]> git.lizzy.rs Git - rust.git/blobdiff - src/tools/clippy/clippy_lints/src/format_args.rs
Auto merge of #102755 - pcc:data-local-tmp, r=Mark-Simulacrum
[rust.git] / src / tools / clippy / clippy_lints / src / format_args.rs
index 9e1eaf248b73cd4a0f83d46eeaa938d0ae62018c..99bef62f81436d48343eee7a921585b0664270de 100644 (file)
@@ -1,16 +1,18 @@
 use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
-use clippy_utils::is_diag_trait_item;
-use clippy_utils::macros::{is_format_macro, FormatArgsExpn};
+use clippy_utils::macros::FormatParamKind::{Implicit, Named, Numbered, Starred};
+use clippy_utils::macros::{is_format_macro, FormatArgsExpn, FormatParam, FormatParamUsage};
 use clippy_utils::source::snippet_opt;
 use clippy_utils::ty::implements_trait;
+use clippy_utils::{is_diag_trait_item, meets_msrv, msrvs};
 use if_chain::if_chain;
 use itertools::Itertools;
 use rustc_errors::Applicability;
-use rustc_hir::{Expr, ExprKind, HirId};
-use rustc_lint::{LateContext, LateLintPass};
+use rustc_hir::{Expr, ExprKind, HirId, QPath};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::ty::adjustment::{Adjust, Adjustment};
 use rustc_middle::ty::Ty;
-use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_semver::RustcVersion;
+use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::{sym, ExpnData, ExpnKind, Span, Symbol};
 
 declare_clippy_lint! {
     "`to_string` applied to a type that implements `Display` in format args"
 }
 
-declare_lint_pass!(FormatArgs => [FORMAT_IN_FORMAT_ARGS, TO_STRING_IN_FORMAT_ARGS]);
+declare_clippy_lint! {
+    /// ### What it does
+    /// Detect when a variable is not inlined in a format string,
+    /// and suggests to inline it.
+    ///
+    /// ### Why is this bad?
+    /// Non-inlined code is slightly more difficult to read and understand,
+    /// as it requires arguments to be matched against the format string.
+    /// The inlined syntax, where allowed, is simpler.
+    ///
+    /// ### Example
+    /// ```rust
+    /// # let var = 42;
+    /// # let width = 1;
+    /// # let prec = 2;
+    /// format!("{}", var);
+    /// format!("{v:?}", v = var);
+    /// format!("{0} {0}", var);
+    /// format!("{0:1$}", var, width);
+    /// format!("{:.*}", prec, var);
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// # let var = 42;
+    /// # let width = 1;
+    /// # let prec = 2;
+    /// format!("{var}");
+    /// format!("{var:?}");
+    /// format!("{var} {var}");
+    /// format!("{var:width$}");
+    /// format!("{var:.prec$}");
+    /// ```
+    ///
+    /// ### Known Problems
+    ///
+    /// There may be a false positive if the format string is expanded from certain proc macros:
+    ///
+    /// ```ignore
+    /// println!(indoc!("{}"), var);
+    /// ```
+    ///
+    /// If a format string contains a numbered argument that cannot be inlined
+    /// nothing will be suggested, e.g. `println!("{0}={1}", var, 1+2)`.
+    #[clippy::version = "1.65.0"]
+    pub UNINLINED_FORMAT_ARGS,
+    pedantic,
+    "using non-inlined variables in `format!` calls"
+}
+
+impl_lint_pass!(FormatArgs => [FORMAT_IN_FORMAT_ARGS, UNINLINED_FORMAT_ARGS, TO_STRING_IN_FORMAT_ARGS]);
+
+pub struct FormatArgs {
+    msrv: Option<RustcVersion>,
+}
+
+impl FormatArgs {
+    #[must_use]
+    pub fn new(msrv: Option<RustcVersion>) -> Self {
+        Self { msrv }
+    }
+}
 
 impl<'tcx> LateLintPass<'tcx> for FormatArgs {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
@@ -86,9 +148,65 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
                     check_format_in_format_args(cx, outermost_expn_data.call_site, name, arg.param.value);
                     check_to_string_in_format_args(cx, name, arg.param.value);
                 }
+                if meets_msrv(self.msrv, msrvs::FORMAT_ARGS_CAPTURE) {
+                    check_uninlined_args(cx, &format_args, outermost_expn_data.call_site);
+                }
             }
         }
     }
+
+    extract_msrv_attr!(LateContext);
+}
+
+fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_site: Span) {
+    if args.format_string.span.from_expansion() {
+        return;
+    }
+
+    let mut fixes = Vec::new();
+    // If any of the arguments are referenced by an index number,
+    // and that argument is not a simple variable and cannot be inlined,
+    // we cannot remove any other arguments in the format string,
+    // because the index numbers might be wrong after inlining.
+    // Example of an un-inlinable format:  print!("{}{1}", foo, 2)
+    if !args.params().all(|p| check_one_arg(args, &p, &mut fixes)) || fixes.is_empty() {
+        return;
+    }
+
+    // Temporarily ignore multiline spans: https://github.com/rust-lang/rust/pull/102729#discussion_r988704308
+    if fixes.iter().any(|(span, _)| cx.sess().source_map().is_multiline(*span)) {
+        return;
+    }
+
+    span_lint_and_then(
+        cx,
+        UNINLINED_FORMAT_ARGS,
+        call_site,
+        "variables can be used directly in the `format!` string",
+        |diag| {
+            diag.multipart_suggestion("change this to", fixes, Applicability::MachineApplicable);
+        },
+    );
+}
+
+fn check_one_arg(args: &FormatArgsExpn<'_>, param: &FormatParam<'_>, fixes: &mut Vec<(Span, String)>) -> bool {
+    if matches!(param.kind, Implicit | Starred | Named(_) | Numbered)
+        && let ExprKind::Path(QPath::Resolved(None, path)) = param.value.kind
+        && let [segment] = path.segments
+        && let Some(arg_span) = args.value_with_prev_comma_span(param.value.hir_id)
+    {
+        let replacement = match param.usage {
+            FormatParamUsage::Argument => segment.ident.name.to_string(),
+            FormatParamUsage::Width => format!("{}$", segment.ident.name),
+            FormatParamUsage::Precision => format!(".{}$", segment.ident.name),
+        };
+        fixes.push((param.span, replacement));
+        fixes.push((arg_span, String::new()));
+        true  // successful inlining, continue checking
+    } else {
+        // if we can't inline a numbered argument, we can't continue
+        param.kind != Numbered
+    }
 }
 
 fn outermost_expn_data(expn_data: ExpnData) -> ExpnData {
@@ -117,11 +235,10 @@ fn check_format_in_format_args(
         cx,
         FORMAT_IN_FORMAT_ARGS,
         call_site,
-        &format!("`format!` in `{}!` args", name),
+        &format!("`format!` in `{name}!` args"),
         |diag| {
             diag.help(&format!(
-                "combine the `format!(..)` arguments with the outer `{}!(..)` call",
-                name
+                "combine the `format!(..)` arguments with the outer `{name}!(..)` call"
             ));
             diag.help("or consider changing `format!` to `format_args!`");
         },
@@ -149,8 +266,7 @@ fn check_to_string_in_format_args(cx: &LateContext<'_>, name: Symbol, value: &Ex
                     TO_STRING_IN_FORMAT_ARGS,
                     value.span.with_lo(receiver.span.hi()),
                     &format!(
-                        "`to_string` applied to a type that implements `Display` in `{}!` args",
-                        name
+                        "`to_string` applied to a type that implements `Display` in `{name}!` args"
                     ),
                     "remove this",
                     String::new(),
@@ -162,16 +278,13 @@ fn check_to_string_in_format_args(cx: &LateContext<'_>, name: Symbol, value: &Ex
                     TO_STRING_IN_FORMAT_ARGS,
                     value.span,
                     &format!(
-                        "`to_string` applied to a type that implements `Display` in `{}!` args",
-                        name
+                        "`to_string` applied to a type that implements `Display` in `{name}!` args"
                     ),
                     "use this",
                     format!(
-                        "{}{:*>width$}{}",
+                        "{}{:*>n_needed_derefs$}{receiver_snippet}",
                         if needs_ref { "&" } else { "" },
-                        "",
-                        receiver_snippet,
-                        width = n_needed_derefs
+                        ""
                     ),
                     Applicability::MachineApplicable,
                 );
@@ -180,7 +293,7 @@ fn check_to_string_in_format_args(cx: &LateContext<'_>, name: Symbol, value: &Ex
     }
 }
 
-// Returns true if `hir_id` is referred to by multiple format params
+/// Returns true if `hir_id` is referred to by multiple format params
 fn is_aliased(args: &FormatArgsExpn<'_>, hir_id: HirId) -> bool {
     args.params().filter(|param| param.value.hir_id == hir_id).at_most_one().is_err()
 }