diff --git a/compiler/rustc_builtin_macros/src/errors.rs b/compiler/rustc_builtin_macros/src/errors.rs index 1343c8ccf4a59..2c4836c35fca6 100644 --- a/compiler/rustc_builtin_macros/src/errors.rs +++ b/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 { diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs index f47dae5eba00a..3741ae60f8b6d 100644 --- a/compiler/rustc_builtin_macros/src/format.rs +++ b/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,6 +157,86 @@ 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 = 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 { + 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, + macro_span: Span, + fmt_span: Span, + err_span: &Range, + fmt_str: &str, + args: &FormatArguments, +) -> Option { + 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(macro_span).ok()?; + let local_name = local_variable_name(fmt_str, args); + let updated_macro = + replace_span_in_snippet(¯o_snippet, captured_span, macro_span, &local_name)?; + + let indent = line_indent(sm, macro_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, @@ -301,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 = macro_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) => { diff --git a/compiler/rustc_parse_format/src/lib.rs b/compiler/rustc_parse_format/src/lib.rs index 2338268a874f0..5eecd271a8667 100644 --- a/compiler/rustc_parse_format/src/lib.rs +++ b/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), @@ -466,7 +466,7 @@ impl<'input> Parser<'input> { ('<' | '^' | '>', _) => self.suggest_format_align(c), (',', _) => self.suggest_unsupported_python_numeric_grouping(), ('=', '}') => self.suggest_rust_debug_printing_macro(), - _ => self.suggest_positional_arg_instead_of_captured_arg(arg), + _ => self.suggest_local_variable_for_captured_arg(arg), } } } @@ -914,7 +914,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; @@ -938,7 +938,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, }, ); } @@ -951,7 +951,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, }, ); } diff --git a/tests/ui/fmt/format-args-non-identifier-diagnostics.fixed b/tests/ui/fmt/format-args-non-identifier-diagnostics.fixed index bd4db9480674c..2a7b1c7911cf4 100644 --- a/tests/ui/fmt/format-args-non-identifier-diagnostics.fixed +++ b/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: . +// identifier is expected in a format arg) to use a local variable instead. +// Issue: . //@ run-rustfix fn main() { let x = (1,); - println!("{0}", x.0); + { + let value = x.0; + println!("{value}") + }; //~^ ERROR invalid format string } diff --git a/tests/ui/fmt/format-args-non-identifier-diagnostics.rs b/tests/ui/fmt/format-args-non-identifier-diagnostics.rs index aab705341f71d..bab3f3ac17771 100644 --- a/tests/ui/fmt/format-args-non-identifier-diagnostics.rs +++ b/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: . +// identifier is expected in a format arg) to use a local variable instead. +// Issue: . //@ run-rustfix fn main() { diff --git a/tests/ui/fmt/format-args-non-identifier-diagnostics.stderr b/tests/ui/fmt/format-args-non-identifier-diagnostics.stderr index af6bb58071fff..38dfd1ba70ffd 100644 --- a/tests/ui/fmt/format-args-non-identifier-diagnostics.stderr +++ b/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 diff --git a/tests/ui/fmt/struct-field-as-captured-argument.fixed b/tests/ui/fmt/struct-field-as-captured-argument.fixed index 0da40737354f7..af67cae024372 100644 --- a/tests/ui/fmt/struct-field-as-captured-argument.fixed +++ b/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 } diff --git a/tests/ui/fmt/struct-field-as-captured-argument.rs b/tests/ui/fmt/struct-field-as-captured-argument.rs index 325b4e3a21878..13087cceb671b 100644 --- a/tests/ui/fmt/struct-field-as-captured-argument.rs +++ b/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 diff --git a/tests/ui/fmt/struct-field-as-captured-argument.stderr b/tests/ui/fmt/struct-field-as-captured-argument.stderr index 388c14f932bbc..eb8ecbf2e0cc0 100644 --- a/tests/ui/fmt/struct-field-as-captured-argument.stderr +++ b/tests/ui/fmt/struct-field-as-captured-argument.stderr @@ -4,10 +4,12 @@ error: invalid format string: field access isn't supported LL | let _ = format!("{foo.field}"); | ^^^^^^^^^ not supported in format string | -help: consider using a positional formatting argument instead +help: consider putting the value in a local variable instead | -LL - let _ = format!("{foo.field}"); -LL + let _ = format!("{0}", foo.field); +LL ~ let _ = { +LL + let value = foo.field; +LL + format!("{value}") +LL ~ }; | error: invalid format string: field access isn't supported @@ -16,71 +18,97 @@ error: invalid format string: field access isn't supported LL | let _ = format!("{foo.field} {} {bar}", "aa"); | ^^^^^^^^^ not supported in format string | -help: consider using a positional formatting argument instead +help: consider putting the value in a local variable instead | -LL - let _ = format!("{foo.field} {} {bar}", "aa"); -LL + let _ = format!("{1} {} {bar}", "aa", foo.field); +LL ~ let _ = { +LL + let value = foo.field; +LL + format!("{value} {} {bar}", "aa") +LL ~ }; | error: invalid format string: field access isn't supported --> $DIR/struct-field-as-captured-argument.rs:13:23 | -LL | let _ = format!("{foo.field} {} {1} {bar}", "aa", "bb"); +LL | let _ = format!("{foo.field:value$} {bar}", value = 1); | ^^^^^^^^^ not supported in format string | -help: consider using a positional formatting argument instead +help: consider putting the value in a local variable instead | -LL - let _ = format!("{foo.field} {} {1} {bar}", "aa", "bb"); -LL + let _ = format!("{2} {} {1} {bar}", "aa", "bb", foo.field); +LL ~ let _ = { +LL + let value_1 = foo.field; +LL + format!("{value_1:value$} {bar}", value = 1) +LL ~ }; | error: invalid format string: field access isn't supported --> $DIR/struct-field-as-captured-argument.rs:14:23 | -LL | let _ = format!("{foo.field} {} {baz}", "aa", baz = 3); +LL | let _ = format!("{foo.field} {} {1} {bar}", "aa", "bb"); | ^^^^^^^^^ not supported in format string | -help: consider using a positional formatting argument instead +help: consider putting the value in a local variable instead | -LL - let _ = format!("{foo.field} {} {baz}", "aa", baz = 3); -LL + let _ = format!("{1} {} {baz}", "aa", foo.field, baz = 3); +LL ~ let _ = { +LL + let value = foo.field; +LL + format!("{value} {} {1} {bar}", "aa", "bb") +LL ~ }; | error: invalid format string: field access isn't supported --> $DIR/struct-field-as-captured-argument.rs:15:23 | -LL | let _ = format!("{foo.field:?} {} {baz}", "aa", baz = 3); +LL | let _ = format!("{foo.field} {} {baz}", "aa", baz = 3); | ^^^^^^^^^ not supported in format string | -help: consider using a positional formatting argument instead +help: consider putting the value in a local variable instead | -LL - let _ = format!("{foo.field:?} {} {baz}", "aa", baz = 3); -LL + let _ = format!("{1:?} {} {baz}", "aa", foo.field, baz = 3); +LL ~ let _ = { +LL + let value = foo.field; +LL + format!("{value} {} {baz}", "aa", baz = 3) +LL ~ }; | error: invalid format string: field access isn't supported --> $DIR/struct-field-as-captured-argument.rs:16:23 | -LL | let _ = format!("{foo.field:#?} {} {baz}", "aa", baz = 3); +LL | let _ = format!("{foo.field:?} {} {baz}", "aa", baz = 3); | ^^^^^^^^^ not supported in format string | -help: consider using a positional formatting argument instead +help: consider putting the value in a local variable instead | -LL - let _ = format!("{foo.field:#?} {} {baz}", "aa", baz = 3); -LL + let _ = format!("{1:#?} {} {baz}", "aa", foo.field, baz = 3); +LL ~ let _ = { +LL + let value = foo.field; +LL + format!("{value:?} {} {baz}", "aa", baz = 3) +LL ~ }; | error: invalid format string: field access isn't supported --> $DIR/struct-field-as-captured-argument.rs:17:23 | +LL | let _ = format!("{foo.field:#?} {} {baz}", "aa", baz = 3); + | ^^^^^^^^^ not supported in format string + | +help: consider putting the value in a local variable instead + | +LL ~ let _ = { +LL + let value = foo.field; +LL + format!("{value:#?} {} {baz}", "aa", baz = 3) +LL ~ }; + | + +error: invalid format string: field access isn't supported + --> $DIR/struct-field-as-captured-argument.rs:18:23 + | LL | let _ = format!("{foo.field:.3} {} {baz}", "aa", baz = 3); | ^^^^^^^^^ not supported in format string | -help: consider using a positional formatting argument instead +help: consider putting the value in a local variable instead | -LL - let _ = format!("{foo.field:.3} {} {baz}", "aa", baz = 3); -LL + let _ = format!("{1:.3} {} {baz}", "aa", foo.field, baz = 3); +LL ~ let _ = { +LL + let value = foo.field; +LL + format!("{value:.3} {} {baz}", "aa", baz = 3) +LL ~ }; | -error: aborting due to 7 previous errors +error: aborting due to 8 previous errors