Auto merge of rust-lang#11444 - Alexendoo:find-format-args-lifetime-crimes, r=flip1995

bors · bors · commit b27fc10aa890 · 2023-09-14T17:28:51.000Z
Return a value from find_format_args instead of using a callback

r? `@flip1995`

changelog: none
diff --git a/clippy_lints/src/explicit_write.rs b/clippy_lints/src/explicit_write.rs
@@ -57,54 +57,52 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitWrite {
             } else {
                 None
             }
+            && let Some(format_args) = find_format_args(cx, write_arg, ExpnId::root())
         {
-            find_format_args(cx, write_arg, ExpnId::root(), |format_args| {
-                let calling_macro =
-                    // ordering is important here, since `writeln!` uses `write!` internally
-                    if is_expn_of(write_call.span, "writeln").is_some() {
-                        Some("writeln")
-                    } else if is_expn_of(write_call.span, "write").is_some() {
-                        Some("write")
-                    } else {
-                        None
-                    };
-                let prefix = if dest_name == "stderr" {
-                    "e"
-                } else {
-                    ""
-                };
+            // ordering is important here, since `writeln!` uses `write!` internally
+            let calling_macro = if is_expn_of(write_call.span, "writeln").is_some() {
+                Some("writeln")
+            } else if is_expn_of(write_call.span, "write").is_some() {
+                Some("write")
+            } else {
+                None
+            };
+            let prefix = if dest_name == "stderr" {
+                "e"
+            } else {
+                ""
+            };
 
-                // We need to remove the last trailing newline from the string because the
-                // underlying `fmt::write` function doesn't know whether `println!` or `print!` was
-                // used.
-                let (used, sugg_mac) = if let Some(macro_name) = calling_macro {
-                    (
-                        format!("{macro_name}!({dest_name}(), ...)"),
-                        macro_name.replace("write", "print"),
-                    )
-                } else {
-                    (
-                        format!("{dest_name}().write_fmt(...)"),
-                        "print".into(),
-                    )
-                };
-                let mut applicability = Applicability::MachineApplicable;
-                let inputs_snippet = snippet_with_applicability(
-                    cx,
-                    format_args_inputs_span(format_args),
-                    "..",
-                    &mut applicability,
-                );
-                span_lint_and_sugg(
-                    cx,
-                    EXPLICIT_WRITE,
-                    expr.span,
-                    &format!("use of `{used}.unwrap()`"),
-                    "try",
-                    format!("{prefix}{sugg_mac}!({inputs_snippet})"),
-                    applicability,
-                );
-            });
+            // We need to remove the last trailing newline from the string because the
+            // underlying `fmt::write` function doesn't know whether `println!` or `print!` was
+            // used.
+            let (used, sugg_mac) = if let Some(macro_name) = calling_macro {
+                (
+                    format!("{macro_name}!({dest_name}(), ...)"),
+                    macro_name.replace("write", "print"),
+                )
+            } else {
+                (
+                    format!("{dest_name}().write_fmt(...)"),
+                    "print".into(),
+                )
+            };
+            let mut applicability = Applicability::MachineApplicable;
+            let inputs_snippet = snippet_with_applicability(
+                cx,
+                format_args_inputs_span(&format_args),
+                "..",
+                &mut applicability,
+            );
+            span_lint_and_sugg(
+                cx,
+                EXPLICIT_WRITE,
+                expr.span,
+                &format!("use of `{used}.unwrap()`"),
+                "try",
+                format!("{prefix}{sugg_mac}!({inputs_snippet})"),
+                applicability,
+            );
         }
     }
 }
diff --git a/clippy_lints/src/format.rs b/clippy_lints/src/format.rs
@@ -43,14 +43,10 @@ declare_lint_pass!(UselessFormat => [USELESS_FORMAT]);
 
 impl<'tcx> LateLintPass<'tcx> for UselessFormat {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
-        let Some(macro_call) = root_macro_call_first_node(cx, expr) else {
-            return;
-        };
-        if !cx.tcx.is_diagnostic_item(sym::format_macro, macro_call.def_id) {
-            return;
-        }
-
-        find_format_args(cx, expr, macro_call.expn, |format_args| {
+        if let Some(macro_call) = root_macro_call_first_node(cx, expr)
+            && cx.tcx.is_diagnostic_item(sym::format_macro, macro_call.def_id)
+            && let Some(format_args) = find_format_args(cx, expr, macro_call.expn)
+        {
             let mut applicability = Applicability::MachineApplicable;
             let call_site = macro_call.span;
 
@@ -91,7 +87,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessFormat {
                 },
                 _ => {},
             }
-        });
+        }
     }
 }
 
diff --git a/clippy_lints/src/format_args.rs b/clippy_lints/src/format_args.rs
@@ -186,15 +186,10 @@ impl FormatArgs {
 
 impl<'tcx> LateLintPass<'tcx> for FormatArgs {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
-        let Some(macro_call) = root_macro_call_first_node(cx, expr) else {
-            return;
-        };
-        if !is_format_macro(cx, macro_call.def_id) {
-            return;
-        }
-        let name = cx.tcx.item_name(macro_call.def_id);
-
-        find_format_args(cx, expr, macro_call.expn, |format_args| {
+        if let Some(macro_call) = root_macro_call_first_node(cx, expr)
+            && is_format_macro(cx, macro_call.def_id)
+            && let Some(format_args) = find_format_args(cx, expr, macro_call.expn)
+        {
             for piece in &format_args.template {
                 if let FormatArgsPiece::Placeholder(placeholder) = piece
                     && let Ok(index) = placeholder.argument.index
@@ -206,22 +201,23 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs {
 
                     if placeholder.format_trait != FormatTrait::Display
                         || placeholder.format_options != FormatOptions::default()
-                        || is_aliased(format_args, index)
+                        || is_aliased(&format_args, index)
                     {
                         continue;
                     }
 
                     if let Ok(arg_hir_expr) = arg_expr {
+                        let name = cx.tcx.item_name(macro_call.def_id);
                         check_format_in_format_args(cx, macro_call.span, name, arg_hir_expr);
                         check_to_string_in_format_args(cx, name, arg_hir_expr);
                     }
                 }
             }
 
             if self.msrv.meets(msrvs::FORMAT_ARGS_CAPTURE) {
-                check_uninlined_args(cx, format_args, macro_call.span, macro_call.def_id, self.ignore_mixed);
+                check_uninlined_args(cx, &format_args, macro_call.span, macro_call.def_id, self.ignore_mixed);
             }
-        });
+        }
     }
 
     extract_msrv_attr!(LateContext);
diff --git a/clippy_lints/src/format_impl.rs b/clippy_lints/src/format_impl.rs
@@ -170,30 +170,29 @@ fn check_self_in_format_args<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>,
     if let Some(outer_macro) = root_macro_call_first_node(cx, expr)
         && let macro_def_id = outer_macro.def_id
         && is_format_macro(cx, macro_def_id)
+        && let Some(format_args) = find_format_args(cx, expr, outer_macro.expn)
     {
-        find_format_args(cx, expr, outer_macro.expn, |format_args| {
-            for piece in &format_args.template {
-                if let FormatArgsPiece::Placeholder(placeholder) = piece
-                    && let trait_name = match placeholder.format_trait {
-                        FormatTrait::Display => sym::Display,
-                        FormatTrait::Debug => sym::Debug,
-                        FormatTrait::LowerExp => sym!(LowerExp),
-                        FormatTrait::UpperExp => sym!(UpperExp),
-                        FormatTrait::Octal => sym!(Octal),
-                        FormatTrait::Pointer => sym::Pointer,
-                        FormatTrait::Binary => sym!(Binary),
-                        FormatTrait::LowerHex => sym!(LowerHex),
-                        FormatTrait::UpperHex => sym!(UpperHex),
-                    }
-                    && trait_name == impl_trait.name
-                    && let Ok(index) = placeholder.argument.index
-                    && let Some(arg) = format_args.arguments.all_args().get(index)
-                    && let Ok(arg_expr) = find_format_arg_expr(expr, arg)
-                {
-                    check_format_arg_self(cx, expr.span, arg_expr, impl_trait);
+        for piece in &format_args.template {
+            if let FormatArgsPiece::Placeholder(placeholder) = piece
+                && let trait_name = match placeholder.format_trait {
+                    FormatTrait::Display => sym::Display,
+                    FormatTrait::Debug => sym::Debug,
+                    FormatTrait::LowerExp => sym!(LowerExp),
+                    FormatTrait::UpperExp => sym!(UpperExp),
+                    FormatTrait::Octal => sym!(Octal),
+                    FormatTrait::Pointer => sym::Pointer,
+                    FormatTrait::Binary => sym!(Binary),
+                    FormatTrait::LowerHex => sym!(LowerHex),
+                    FormatTrait::UpperHex => sym!(UpperHex),
                 }
+                && trait_name == impl_trait.name
+                && let Ok(index) = placeholder.argument.index
+                && let Some(arg) = format_args.arguments.all_args().get(index)
+                && let Ok(arg_expr) = find_format_arg_expr(expr, arg)
+            {
+                check_format_arg_self(cx, expr.span, arg_expr, impl_trait);
             }
-        });
+        }
     }
 }
 
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -609,7 +609,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
                 .collect(),
         ))
     });
-    store.register_early_pass(|| Box::new(utils::format_args_collector::FormatArgsCollector));
+    store.register_early_pass(|| Box::<utils::format_args_collector::FormatArgsCollector>::default());
     store.register_late_pass(|_| Box::new(utils::dump_hir::DumpHir));
     store.register_late_pass(|_| Box::new(utils::author::Author));
     let await_holding_invalid_types = conf.await_holding_invalid_types.clone();
diff --git a/clippy_lints/src/methods/expect_fun_call.rs b/clippy_lints/src/methods/expect_fun_call.rs
@@ -131,13 +131,12 @@ pub(super) fn check<'tcx>(
 
     let mut applicability = Applicability::MachineApplicable;
 
-    //Special handling for `format!` as arg_root
+    // Special handling for `format!` as arg_root
     if let Some(macro_call) = root_macro_call_first_node(cx, arg_root) {
-        if !cx.tcx.is_diagnostic_item(sym::format_macro, macro_call.def_id) {
-            return;
-        }
-        find_format_args(cx, arg_root, macro_call.expn, |format_args| {
-            let span = format_args_inputs_span(format_args);
+        if cx.tcx.is_diagnostic_item(sym::format_macro, macro_call.def_id)
+            && let Some(format_args) = find_format_args(cx, arg_root, macro_call.expn)
+        {
+            let span = format_args_inputs_span(&format_args);
             let sugg = snippet_with_applicability(cx, span, "..", &mut applicability);
             span_lint_and_sugg(
                 cx,
@@ -148,7 +147,7 @@ pub(super) fn check<'tcx>(
                 format!("unwrap_or_else({closure_args} panic!({sugg}))"),
                 applicability,
             );
-        });
+        }
         return;
     }
 
diff --git a/clippy_lints/src/utils/format_args_collector.rs b/clippy_lints/src/utils/format_args_collector.rs
@@ -1,12 +1,15 @@
-use clippy_utils::macros::collect_ast_format_args;
+use clippy_utils::macros::AST_FORMAT_ARGS;
 use clippy_utils::source::snippet_opt;
 use itertools::Itertools;
-use rustc_ast::{Expr, ExprKind, FormatArgs};
+use rustc_ast::{Crate, Expr, ExprKind, FormatArgs};
+use rustc_data_structures::fx::FxHashMap;
 use rustc_lexer::{tokenize, TokenKind};
 use rustc_lint::{EarlyContext, EarlyLintPass};
-use rustc_session::{declare_lint_pass, declare_tool_lint};
-use rustc_span::hygiene;
+use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_span::{hygiene, Span};
 use std::iter::once;
+use std::mem;
+use std::rc::Rc;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -17,7 +20,12 @@ declare_clippy_lint! {
     "collects `format_args` AST nodes for use in later lints"
 }
 
-declare_lint_pass!(FormatArgsCollector => [FORMAT_ARGS_COLLECTOR]);
+#[derive(Default)]
+pub struct FormatArgsCollector {
+    format_args: FxHashMap<Span, Rc<FormatArgs>>,
+}
+
+impl_lint_pass!(FormatArgsCollector => [FORMAT_ARGS_COLLECTOR]);
 
 impl EarlyLintPass for FormatArgsCollector {
     fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
@@ -26,9 +34,17 @@ impl EarlyLintPass for FormatArgsCollector {
                 return;
             }
 
-            collect_ast_format_args(expr.span, args);
+            self.format_args
+                .insert(expr.span.with_parent(None), Rc::new((**args).clone()));
         }
     }
+
+    fn check_crate_post(&mut self, _: &EarlyContext<'_>, _: &Crate) {
+        AST_FORMAT_ARGS.with(|ast_format_args| {
+            let result = ast_format_args.set(mem::take(&mut self.format_args));
+            debug_assert!(result.is_ok());
+        });
+    }
 }
 
 /// Detects if the format string or an argument has its span set by a proc macro to something inside
diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs
@@ -304,23 +304,23 @@ impl<'tcx> LateLintPass<'tcx> for Write {
             _ => return,
         }
 
-        find_format_args(cx, expr, macro_call.expn, |format_args| {
+        if let Some(format_args) = find_format_args(cx, expr, macro_call.expn) {
             // ignore `writeln!(w)` and `write!(v, some_macro!())`
             if format_args.span.from_expansion() {
                 return;
             }
 
             match diag_name {
                 sym::print_macro | sym::eprint_macro | sym::write_macro => {
-                    check_newline(cx, format_args, &macro_call, name);
+                    check_newline(cx, &format_args, &macro_call, name);
                 },
                 sym::println_macro | sym::eprintln_macro | sym::writeln_macro => {
-                    check_empty_string(cx, format_args, &macro_call, name);
+                    check_empty_string(cx, &format_args, &macro_call, name);
                 },
                 _ => {},
             }
 
-            check_literal(cx, format_args, name);
+            check_literal(cx, &format_args, name);
 
             if !self.in_debug_impl {
                 for piece in &format_args.template {
@@ -334,7 +334,7 @@ impl<'tcx> LateLintPass<'tcx> for Write {
                     }
                 }
             }
-        });
+        }
     }
 }
 
diff --git a/clippy_utils/src/macros.rs b/clippy_utils/src/macros.rs

Original file line number	Diff line number	Diff line change
`@@ -304,23 +304,23 @@ impl<'tcx> LateLintPass<'tcx> for Write {`
`304`	`304`	`_ => return,`
`305`	`305`	`}`
`306`	`306`
`307`		`- find_format_args(cx, expr, macro_call.expn, \|format_args\| {`
	`307`	`+ if let Some(format_args) = find_format_args(cx, expr, macro_call.expn) {`
`308`	`308`	// ignore `writeln!(w)` and `write!(v, some_macro!())`
`309`	`309`	`if format_args.span.from_expansion() {`
`310`	`310`	`return;`
`311`	`311`	`}`
`312`	`312`
`313`	`313`	`match diag_name {`
`314`	`314`	`sym::print_macro \| sym::eprint_macro \| sym::write_macro => {`
`315`		`- check_newline(cx, format_args, &macro_call, name);`
	`315`	`+ check_newline(cx, &format_args, &macro_call, name);`
`316`	`316`	`},`
`317`	`317`	`sym::println_macro \| sym::eprintln_macro \| sym::writeln_macro => {`
`318`		`- check_empty_string(cx, format_args, &macro_call, name);`
	`318`	`+ check_empty_string(cx, &format_args, &macro_call, name);`
`319`	`319`	`},`
`320`	`320`	`_ => {},`
`321`	`321`	`}`
`322`	`322`
`323`		`- check_literal(cx, format_args, name);`
	`323`	`+ check_literal(cx, &format_args, name);`
`324`	`324`
`325`	`325`	`if !self.in_debug_impl {`
`326`	`326`	`for piece in &format_args.template {`
`@@ -334,7 +334,7 @@ impl<'tcx> LateLintPass<'tcx> for Write {`
`334`	`334`	`}`
`335`	`335`	`}`
`336`	`336`	`}`
`337`		`- });`
	`337`	`+ }`
`338`	`338`	`}`
`339`	`339`	`}`
`340`	`340`