Rollup merge of #100058 - TaKO8Ki:suggest-positional-formatting-argum…

…ent-instead-of-format-args-capture, r=estebank Suggest a positional formatting argument instead of a captured argument This patch fixes a part of #96999. fixes #98241 fixes #97311 r? `@estebank`
rust-lang · Aug 4, 2022 · d3aa757 · d3aa757
2 parents 87dd56f + 8c85c99
commit d3aa757
Show file tree

Hide file tree

Showing 5 changed files with 182 additions and 8 deletions.
diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs
@@ -280,6 +280,11 @@ struct Context<'a, 'b> {
     unused_names_lint: PositionalNamedArgsLint,
 }
 
+pub struct FormatArg {
+    expr: P<ast::Expr>,
+    named: bool,
+}
+
 /// Parses the arguments from the given list of tokens, returning the diagnostic
 /// if there's a parse error so we can continue parsing other format!
 /// expressions.
@@ -293,8 +298,8 @@ fn parse_args<'a>(
     ecx: &mut ExtCtxt<'a>,
     sp: Span,
     tts: TokenStream,
-) -> PResult<'a, (P<ast::Expr>, Vec<P<ast::Expr>>, FxHashMap<Symbol, (usize, Span)>)> {
-    let mut args = Vec::<P<ast::Expr>>::new();
+) -> PResult<'a, (P<ast::Expr>, Vec<FormatArg>, FxHashMap<Symbol, (usize, Span)>)> {
+    let mut args = Vec::<FormatArg>::new();
     let mut names = FxHashMap::<Symbol, (usize, Span)>::default();
 
     let mut p = ecx.new_parser_from_tts(tts);
@@ -362,7 +367,7 @@ fn parse_args<'a>(
                 let e = p.parse_expr()?;
                 if let Some((prev, _)) = names.get(&ident.name) {
                     ecx.struct_span_err(e.span, &format!("duplicate argument named `{}`", ident))
-                        .span_label(args[*prev].span, "previously here")
+                        .span_label(args[*prev].expr.span, "previously here")
                         .span_label(e.span, "duplicate argument")
                         .emit();
                     continue;
@@ -374,7 +379,7 @@ fn parse_args<'a>(
                 // args. And remember the names.
                 let slot = args.len();
                 names.insert(ident.name, (slot, ident.span));
-                args.push(e);
+                args.push(FormatArg { expr: e, named: true });
             }
             _ => {
                 let e = p.parse_expr()?;
@@ -385,11 +390,11 @@ fn parse_args<'a>(
                     );
                     err.span_label(e.span, "positional arguments must be before named arguments");
                     for pos in names.values() {
-                        err.span_label(args[pos.0].span, "named argument");
+                        err.span_label(args[pos.0].expr.span, "named argument");
                     }
                     err.emit();
                 }
-                args.push(e);
+                args.push(FormatArg { expr: e, named: false });
             }
         }
     }
@@ -1214,7 +1219,7 @@ pub fn expand_preparsed_format_args(
     ecx: &mut ExtCtxt<'_>,
     sp: Span,
     efmt: P<ast::Expr>,
-    args: Vec<P<ast::Expr>>,
+    args: Vec<FormatArg>,
     names: FxHashMap<Symbol, (usize, Span)>,
     append_newline: bool,
 ) -> P<ast::Expr> {
@@ -1304,6 +1309,25 @@ pub fn expand_preparsed_format_args(
                 e.span_label(fmt_span.from_inner(InnerSpan::new(span.start, span.end)), label);
             }
         }
+        if err.should_be_replaced_with_positional_argument {
+            let captured_arg_span =
+                fmt_span.from_inner(InnerSpan::new(err.span.start, err.span.end));
+            let positional_args = args.iter().filter(|arg| !arg.named).collect::<Vec<_>>();
+            if let Ok(arg) = ecx.source_map().span_to_snippet(captured_arg_span) {
+                let span = match positional_args.last() {
+                    Some(arg) => arg.expr.span,
+                    None => fmt_sp,
+                };
+                e.multipart_suggestion_verbose(
+                    "consider using a positional formatting argument instead",
+                    vec![
+                        (captured_arg_span, positional_args.len().to_string()),
+                        (span.shrink_to_hi(), format!(", {}", arg)),
+                    ],
+                    Applicability::MachineApplicable,
+                );
+            }
+        }
         e.emit();
         return DummyResult::raw_expr(sp, true);
     }
@@ -1318,7 +1342,7 @@ pub fn expand_preparsed_format_args(
 
     let mut cx = Context {
         ecx,
-        args,
+        args: args.into_iter().map(|arg| arg.expr).collect(),
         num_captured_args: 0,
         arg_types,
         arg_unique_types,

diff --git a/compiler/rustc_parse_format/src/lib.rs b/compiler/rustc_parse_format/src/lib.rs
@@ -175,6 +175,7 @@ pub struct ParseError {
     pub label: string::String,
     pub span: InnerSpan,
     pub secondary_label: Option<(string::String, InnerSpan)>,
+    pub should_be_replaced_with_positional_argument: bool,
 }
 
 /// The parser structure for interpreting the input format string. This is
@@ -236,6 +237,8 @@ impl<'a> Iterator for Parser<'a> {
                                     lbrace_inner_offset.to(InnerOffset(rbrace_inner_offset.0 + 1)),
                                 );
                             }
+                        } else {
+                            self.suggest_positional_arg_instead_of_captured_arg(arg);
                         }
                         Some(NextArgument(arg))
                     }
@@ -313,6 +316,7 @@ impl<'a> Parser<'a> {
             label: label.into(),
             span,
             secondary_label: None,
+            should_be_replaced_with_positional_argument: false,
         });
     }
 
@@ -336,6 +340,7 @@ impl<'a> Parser<'a> {
             label: label.into(),
             span,
             secondary_label: None,
+            should_be_replaced_with_positional_argument: false,
         });
     }
 
@@ -407,6 +412,7 @@ impl<'a> Parser<'a> {
                     label,
                     span: pos.to(pos),
                     secondary_label,
+                    should_be_replaced_with_positional_argument: false,
                 });
                 None
             }
@@ -434,6 +440,7 @@ impl<'a> Parser<'a> {
                     label,
                     span: pos.to(pos),
                     secondary_label,
+                    should_be_replaced_with_positional_argument: false,
                 });
             } else {
                 self.err(description, format!("expected `{:?}`", c), pos.to(pos));
@@ -750,6 +757,34 @@ impl<'a> Parser<'a> {
         }
         if found { Some(cur) } else { None }
     }
+
+    fn suggest_positional_arg_instead_of_captured_arg(&mut self, arg: Argument<'a>) {
+        if let Some(end) = self.consume_pos('.') {
+            let byte_pos = self.to_span_index(end);
+            let start = InnerOffset(byte_pos.0 + 1);
+            let field = self.argument(start);
+            // We can only parse `foo.bar` field access, any deeper nesting,
+            // or another type of expression, like method calls, are not supported
+            if !self.consume('}') {
+                return;
+            }
+            if let ArgumentNamed(_) = arg.position {
+                if let ArgumentNamed(_) = field.position {
+                    self.errors.insert(
+                        0,
+                        ParseError {
+                            description: "field access isn't supported".to_string(),
+                            note: None,
+                            label: "not supported".to_string(),
+                            span: InnerSpan::new(arg.position_span.start, field.position_span.end),
+                            secondary_label: None,
+                            should_be_replaced_with_positional_argument: true,
+                        },
+                    );
+                }
+            }
+        }
+    }
 }
 
 /// Finds the indices of all characters that have been processed and differ between the actual

diff --git a/src/test/ui/fmt/struct-field-as-captured-argument.fixed b/src/test/ui/fmt/struct-field-as-captured-argument.fixed
@@ -0,0 +1,18 @@
+// run-rustfix
+
+#[derive(Debug)]
+struct Foo {
+    field: usize,
+}
+
+fn main() {
+    let foo = Foo { field: 0 };
+    let bar = 3;
+    format!("{0}", foo.field); //~ ERROR invalid format string: field access isn't supported
+    format!("{1} {} {bar}", "aa", foo.field); //~ ERROR invalid format string: field access isn't supported
+    format!("{2} {} {1} {bar}", "aa", "bb", foo.field); //~ ERROR invalid format string: field access isn't supported
+    format!("{1} {} {baz}", "aa", foo.field, baz = 3); //~ ERROR invalid format string: field access isn't supported
+    format!("{1:?} {} {baz}", "aa", foo.field, baz = 3); //~ ERROR invalid format string: field access isn't supported
+    format!("{1:#?} {} {baz}", "aa", foo.field, baz = 3); //~ ERROR invalid format string: field access isn't supported
+    format!("{1:.3} {} {baz}", "aa", foo.field, baz = 3); //~ ERROR invalid format string: field access isn't supported
+}
diff --git a/src/test/ui/fmt/struct-field-as-captured-argument.rs b/src/test/ui/fmt/struct-field-as-captured-argument.rs
@@ -0,0 +1,18 @@
+// run-rustfix
+
+#[derive(Debug)]
+struct Foo {
+    field: usize,
+}
+
+fn main() {
+    let foo = Foo { field: 0 };
+    let bar = 3;
+    format!("{foo.field}"); //~ ERROR invalid format string: field access isn't supported
+    format!("{foo.field} {} {bar}", "aa"); //~ ERROR invalid format string: field access isn't supported
+    format!("{foo.field} {} {1} {bar}", "aa", "bb"); //~ ERROR invalid format string: field access isn't supported
+    format!("{foo.field} {} {baz}", "aa", baz = 3); //~ ERROR invalid format string: field access isn't supported
+    format!("{foo.field:?} {} {baz}", "aa", baz = 3); //~ ERROR invalid format string: field access isn't supported
+    format!("{foo.field:#?} {} {baz}", "aa", baz = 3); //~ ERROR invalid format string: field access isn't supported
+    format!("{foo.field:.3} {} {baz}", "aa", baz = 3); //~ ERROR invalid format string: field access isn't supported
+}
diff --git a/src/test/ui/fmt/struct-field-as-captured-argument.stderr b/src/test/ui/fmt/struct-field-as-captured-argument.stderr
@@ -0,0 +1,79 @@
+error: invalid format string: field access isn't supported
+  --> $DIR/struct-field-as-captured-argument.rs:11:15
+   |
+LL |     format!("{foo.field}");
+   |               ^^^^^^^^^ not supported in format string
+   |
+help: consider using a positional formatting argument instead
+   |
+LL |     format!("{0}", foo.field);
+   |               ~  +++++++++++
+
+error: invalid format string: field access isn't supported
+  --> $DIR/struct-field-as-captured-argument.rs:12:15
+   |
+LL |     format!("{foo.field} {} {bar}", "aa");
+   |               ^^^^^^^^^ not supported in format string
+   |
+help: consider using a positional formatting argument instead
+   |
+LL |     format!("{1} {} {bar}", "aa", foo.field);
+   |               ~                 +++++++++++
+
+error: invalid format string: field access isn't supported
+  --> $DIR/struct-field-as-captured-argument.rs:13:15
+   |
+LL |     format!("{foo.field} {} {1} {bar}", "aa", "bb");
+   |               ^^^^^^^^^ not supported in format string
+   |
+help: consider using a positional formatting argument instead
+   |
+LL |     format!("{2} {} {1} {bar}", "aa", "bb", foo.field);
+   |               ~                           +++++++++++
+
+error: invalid format string: field access isn't supported
+  --> $DIR/struct-field-as-captured-argument.rs:14:15
+   |
+LL |     format!("{foo.field} {} {baz}", "aa", baz = 3);
+   |               ^^^^^^^^^ not supported in format string
+   |
+help: consider using a positional formatting argument instead
+   |
+LL |     format!("{1} {} {baz}", "aa", foo.field, baz = 3);
+   |               ~                 +++++++++++
+
+error: invalid format string: field access isn't supported
+  --> $DIR/struct-field-as-captured-argument.rs:15:15
+   |
+LL |     format!("{foo.field:?} {} {baz}", "aa", baz = 3);
+   |               ^^^^^^^^^ not supported in format string
+   |
+help: consider using a positional formatting argument instead
+   |
+LL |     format!("{1:?} {} {baz}", "aa", foo.field, baz = 3);
+   |               ~                   +++++++++++
+
+error: invalid format string: field access isn't supported
+  --> $DIR/struct-field-as-captured-argument.rs:16:15
+   |
+LL |     format!("{foo.field:#?} {} {baz}", "aa", baz = 3);
+   |               ^^^^^^^^^ not supported in format string
+   |
+help: consider using a positional formatting argument instead
+   |
+LL |     format!("{1:#?} {} {baz}", "aa", foo.field, baz = 3);
+   |               ~                    +++++++++++
+
+error: invalid format string: field access isn't supported
+  --> $DIR/struct-field-as-captured-argument.rs:17:15
+   |
+LL |     format!("{foo.field:.3} {} {baz}", "aa", baz = 3);
+   |               ^^^^^^^^^ not supported in format string
+   |
+help: consider using a positional formatting argument instead
+   |
+LL |     format!("{1:.3} {} {baz}", "aa", foo.field, baz = 3);
+   |               ~                    +++++++++++
+
+error: aborting due to 7 previous errors
+