Suggest local variables for captured format args

compiler/rustc_builtin_macros/src/errors.rs

@@ -640,18 +640,16 @@ pub(crate) struct InvalidFormatStringLabel {
 
 #[derive(Subdiagnostic)]
 pub(crate) enum InvalidFormatStringSuggestion {
-    #[multipart_suggestion(
-        "consider using a positional formatting argument instead",
+    #[suggestion(
+        "consider putting the value in a local variable instead",
+        code = "{replacement}",
         style = "verbose",
         applicability = "machine-applicable"
     )]
-    UsePositional {
-        #[suggestion_part(code = "{len}")]
-        captured: Span,
-        len: String,
-        #[suggestion_part(code = ", {arg}")]
+    UseLocalVariable {
+        #[primary_span]
         span: Span,
-        arg: String,
+        replacement: String,
     },
     #[suggestion("remove the `r#`", code = "", applicability = "machine-applicable")]
     RemoveRawIdent {

compiler/rustc_builtin_macros/src/format.rs

@@ -18,6 +18,7 @@ use rustc_lint_defs::builtin::NAMED_ARGUMENTS_USED_POSITIONALLY;
 use rustc_lint_defs::{BuiltinLintDiag, LintId};
 use rustc_parse::exp;
 use rustc_parse_format as parse;
+use rustc_span::source_map::SourceMap;
 use rustc_span::{BytePos, ErrorGuaranteed, Ident, InnerSpan, Span, Symbol};
 
 use crate::errors;
@@ -156,10 +157,91 @@ fn parse_args<'a>(ecx: &ExtCtxt<'a>, sp: Span, tts: TokenStream) -> PResult<'a,
     Ok(MacroInput { fmtstr, args, is_direct_literal })
 }
 
+fn local_variable_name(fmt_str: &str, args: &FormatArguments) -> String {
+    let explicit_named_args: FxHashSet<Symbol> = args
+        .explicit_args()
+        .iter()
+        .filter_map(|arg| match arg.kind {
+            FormatArgumentKind::Named(ident) => Some(ident.name),
+            _ => None,
+        })
+        .collect();
+    let base = "value";
+    if !fmt_str.contains(&format!("{{{base}"))
+        && !explicit_named_args.contains(&Symbol::intern(base))
+    {
+        return base.to_string();
+    }
+
+    for i in 1.. {
+        let candidate = format!("{base}_{i}");
+        if !fmt_str.contains(&format!("{{{candidate}"))
+            && !explicit_named_args.contains(&Symbol::intern(&candidate))
+        {
+            return candidate;
+        }
+    }
+
+    base.to_string()
+}
+
+fn line_indent(sm: &SourceMap, span: Span) -> String {
+    let Ok(prev) = sm.span_to_prev_source(span) else {
+        return String::new();
+    };
+    let line = prev.rsplit_once('\n').map(|(_, line)| line).unwrap_or(prev.as_str());
+    line.chars().take_while(|c| c.is_whitespace()).collect()
+}
+
+fn replace_span_in_snippet(
+    snippet: &str,
+    inner_span: Span,
+    outer_span: Span,
+    replacement: &str,
+) -> Option<String> {
+    if !outer_span.contains(inner_span) {
+        return None;
+    }
+
+    let lo = (inner_span.lo().0 - outer_span.lo().0) as usize;
+    let hi = (inner_span.hi().0 - outer_span.lo().0) as usize;
+    if hi > snippet.len() || !snippet.is_char_boundary(lo) || !snippet.is_char_boundary(hi) {
+        return None;
+    }
+
+    let mut updated = snippet.to_string();
+    updated.replace_range(lo..hi, replacement);
+    Some(updated)
+}
+
+fn build_local_variable_replacement(
+    sm: &SourceMap,
+    mac_span: Span,
+    fmt_span: Span,
+    err_span: &Range<usize>,
+    fmt_str: &str,
+    args: &FormatArguments,
+) -> Option<String> {
+    let captured_span = fmt_span.from_inner(InnerSpan::new(err_span.start, err_span.end));
+    let captured_snippet = sm.span_to_snippet(captured_span).ok()?;
+    let macro_snippet = sm.span_to_snippet(mac_span).ok()?;
+    let local_name = local_variable_name(fmt_str, args);
+    let updated_macro =
+        replace_span_in_snippet(&macro_snippet, captured_span, mac_span, &local_name)?;
+
+    let indent = line_indent(sm, mac_span);
+    let inner_indent = format!("{indent}    ");
+
+    Some(format!(
+        "{{\n{inner_indent}let {local_name} = {captured_snippet};\n{inner_indent}{updated_macro}\n{indent}}}"
+    ))
+}
+
 fn make_format_args(
     ecx: &mut ExtCtxt<'_>,
     input: MacroInput,
     append_newline: bool,
+    mac_span: Span,
 ) -> ExpandResult<Result<FormatArgs, ErrorGuaranteed>, ()> {
     let msg = "format argument must be a string literal";
     let unexpanded_fmt_span = input.fmtstr.span;
@@ -300,20 +382,23 @@ fn make_format_args(
         }
         match err.suggestion {
             parse::Suggestion::None => {}
-            parse::Suggestion::UsePositional => {
-                let captured_arg_span =
-                    fmt_span.from_inner(InnerSpan::new(err.span.start, err.span.end));
-                if let Ok(arg) = ecx.source_map().span_to_snippet(captured_arg_span) {
-                    let span = match args.unnamed_args().last() {
-                        Some(arg) => arg.expr.span,
-                        None => fmt_span,
-                    };
-                    e.sugg_ = Some(errors::InvalidFormatStringSuggestion::UsePositional {
-                        captured: captured_arg_span,
-                        len: args.unnamed_args().len().to_string(),
-                        span: span.shrink_to_hi(),
-                        arg,
-                    });
+            parse::Suggestion::UseLocalVariable => {
+                if is_source_literal {
+                    let mac_callsite = mac_span.source_callsite();
+                    let replacement = build_local_variable_replacement(
+                        ecx.source_map(),
+                        mac_callsite,
+                        fmt_span,
+                        &err.span,
+                        fmt_str,
+                        &args,
+                    );
+                    if let Some(replacement) = replacement {
+                        e.sugg_ = Some(errors::InvalidFormatStringSuggestion::UseLocalVariable {
+                            span: mac_callsite,
+                            replacement,
+                        });
+                    }
                 }
             }
             parse::Suggestion::RemoveRawIdent(span) => {
@@ -1048,7 +1133,7 @@ fn expand_format_args_impl<'cx>(
     sp = ecx.with_def_site_ctxt(sp);
     ExpandResult::Ready(match parse_args(ecx, sp, tts) {
         Ok(input) => {
-            let ExpandResult::Ready(mac) = make_format_args(ecx, input, nl) else {
+            let ExpandResult::Ready(mac) = make_format_args(ecx, input, nl, sp) else {
                 return ExpandResult::Retry(());
             };
             match mac {

compiler/rustc_parse_format/src/lib.rs

@@ -173,9 +173,9 @@ pub struct ParseError {
 
 pub enum Suggestion {
     None,
-    /// Replace inline argument with positional argument:
-    /// `format!("{foo.bar}")` -> `format!("{}", foo.bar)`
-    UsePositional,
+    /// Replace inline argument with a local variable:
+    /// `format!("{foo.bar}")` -> `{ let value = foo.bar; format!("{value}") }`
+    UseLocalVariable,
     /// Remove `r#` from identifier:
     /// `format!("{r#foo}")` -> `format!("{foo}")`
     RemoveRawIdent(Range<usize>),
@@ -462,7 +462,7 @@ impl<'input> Parser<'input> {
                 ('?', _) => self.suggest_format_debug(),
                 ('<' | '^' | '>', _) => self.suggest_format_align(c),
                 (',', _) => self.suggest_unsupported_python_numeric_grouping(),
-                _ => self.suggest_positional_arg_instead_of_captured_arg(arg),
+                _ => self.suggest_local_variable_for_captured_arg(arg),
             }
         }
     }
@@ -889,7 +889,7 @@ impl<'input> Parser<'input> {
         }
     }
 
-    fn suggest_positional_arg_instead_of_captured_arg(&mut self, arg: &Argument<'_>) {
+    fn suggest_local_variable_for_captured_arg(&mut self, arg: &Argument<'_>) {
         // If the argument is not an identifier, it is not a field access.
         if !arg.is_identifier() {
             return;
@@ -913,7 +913,7 @@ impl<'input> Parser<'input> {
                                 label: "not supported".to_string(),
                                 span: arg.position_span.start..field.position_span.end,
                                 secondary_label: None,
-                                suggestion: Suggestion::UsePositional,
+                                suggestion: Suggestion::UseLocalVariable,
                             },
                         );
                     }
@@ -926,7 +926,7 @@ impl<'input> Parser<'input> {
                                 label: "not supported".to_string(),
                                 span: arg.position_span.start..field.position_span.end,
                                 secondary_label: None,
-                                suggestion: Suggestion::UsePositional,
+                                suggestion: Suggestion::UseLocalVariable,
                             },
                         );
                     }

tests/ui/fmt/format-args-non-identifier-diagnostics.fixed

@@ -1,10 +1,13 @@
 // Checks that there is a suggestion for simple tuple index access expression (used where an
-// identifier is expected in a format arg) to use positional arg instead.
-// Issue: <https://github.com/rust-lang/rust/issues/122535>.
+// identifier is expected in a format arg) to use a local variable instead.
+// Issue: <https://github.com/rust-lang/rust/issues/114108>.
 //@ run-rustfix
 
 fn main() {
     let x = (1,);
-    println!("{0}", x.0);
+    {
+        let value = x.0;
+        println!("{value}")
+    };
     //~^ ERROR invalid format string
 }

tests/ui/fmt/format-args-non-identifier-diagnostics.rs

@@ -1,6 +1,6 @@
 // Checks that there is a suggestion for simple tuple index access expression (used where an
-// identifier is expected in a format arg) to use positional arg instead.
-// Issue: <https://github.com/rust-lang/rust/issues/122535>.
+// identifier is expected in a format arg) to use a local variable instead.
+// Issue: <https://github.com/rust-lang/rust/issues/114108>.
 //@ run-rustfix
 
 fn main() {

tests/ui/fmt/format-args-non-identifier-diagnostics.stderr

@@ -4,10 +4,12 @@ error: invalid format string: tuple index access isn't supported
 LL |     println!("{x.0}");
    |                ^^^ not supported in format string
    |
-help: consider using a positional formatting argument instead
+help: consider putting the value in a local variable instead
    |
-LL -     println!("{x.0}");
-LL +     println!("{0}", x.0);
+LL ~     {
+LL +         let value = x.0;
+LL +         println!("{value}")
+LL ~     };
    |
 
 error: aborting due to 1 previous error

tests/ui/fmt/struct-field-as-captured-argument.fixed

@@ -8,11 +8,36 @@ struct Foo {
 fn main() {
     let foo = Foo { field: 0 };
     let bar = 3;
-    let _ = format!("{0}", foo.field); //~ ERROR invalid format string: field access isn't supported
-    let _ = format!("{1} {} {bar}", "aa", foo.field); //~ ERROR invalid format string: field access isn't supported
-    let _ = format!("{2} {} {1} {bar}", "aa", "bb", foo.field); //~ ERROR invalid format string: field access isn't supported
-    let _ = format!("{1} {} {baz}", "aa", foo.field, baz = 3); //~ ERROR invalid format string: field access isn't supported
-    let _ = format!("{1:?} {} {baz}", "aa", foo.field, baz = 3); //~ ERROR invalid format string: field access isn't supported
-    let _ = format!("{1:#?} {} {baz}", "aa", foo.field, baz = 3); //~ ERROR invalid format string: field access isn't supported
-    let _ = format!("{1:.3} {} {baz}", "aa", foo.field, baz = 3); //~ ERROR invalid format string: field access isn't supported
+    let _ = {
+        let value = foo.field;
+        format!("{value}")
+    }; //~ ERROR invalid format string: field access isn't supported
+    let _ = {
+        let value = foo.field;
+        format!("{value} {} {bar}", "aa")
+    }; //~ ERROR invalid format string: field access isn't supported
+    let _ = {
+        let value_1 = foo.field;
+        format!("{value_1:value$} {bar}", value = 1)
+    }; //~ ERROR invalid format string: field access isn't supported
+    let _ = {
+        let value = foo.field;
+        format!("{value} {} {1} {bar}", "aa", "bb")
+    }; //~ ERROR invalid format string: field access isn't supported
+    let _ = {
+        let value = foo.field;
+        format!("{value} {} {baz}", "aa", baz = 3)
+    }; //~ ERROR invalid format string: field access isn't supported
+    let _ = {
+        let value = foo.field;
+        format!("{value:?} {} {baz}", "aa", baz = 3)
+    }; //~ ERROR invalid format string: field access isn't supported
+    let _ = {
+        let value = foo.field;
+        format!("{value:#?} {} {baz}", "aa", baz = 3)
+    }; //~ ERROR invalid format string: field access isn't supported
+    let _ = {
+        let value = foo.field;
+        format!("{value:.3} {} {baz}", "aa", baz = 3)
+    }; //~ ERROR invalid format string: field access isn't supported
 }

tests/ui/fmt/struct-field-as-captured-argument.rs

@@ -10,6 +10,7 @@ fn main() {
     let bar = 3;
     let _ = format!("{foo.field}"); //~ ERROR invalid format string: field access isn't supported
     let _ = format!("{foo.field} {} {bar}", "aa"); //~ ERROR invalid format string: field access isn't supported
+    let _ = format!("{foo.field:value$} {bar}", value = 1); //~ ERROR invalid format string: field access isn't supported
     let _ = format!("{foo.field} {} {1} {bar}", "aa", "bb"); //~ ERROR invalid format string: field access isn't supported
     let _ = format!("{foo.field} {} {baz}", "aa", baz = 3); //~ ERROR invalid format string: field access isn't supported
     let _ = format!("{foo.field:?} {} {baz}", "aa", baz = 3); //~ ERROR invalid format string: field access isn't supported