From 4233a13cebe176139d8d16d18f507b90852ddab0 Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Tue, 2 Aug 2022 20:33:40 +0900 Subject: [PATCH 1/4] suggest a positional formatting argument instead of a captured argument --- compiler/rustc_builtin_macros/src/format.rs | 39 +++++++++++---- compiler/rustc_parse_format/src/lib.rs | 48 ++++++++++++++++--- .../struct-field-as-captured-argument.fixed | 15 ++++++ .../fmt/struct-field-as-captured-argument.rs | 15 ++++++ .../struct-field-as-captured-argument.stderr | 46 ++++++++++++++++++ 5 files changed, 148 insertions(+), 15 deletions(-) create mode 100644 src/test/ui/fmt/struct-field-as-captured-argument.fixed create mode 100644 src/test/ui/fmt/struct-field-as-captured-argument.rs create mode 100644 src/test/ui/fmt/struct-field-as-captured-argument.stderr diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs index f536d0b5900bc..1fdc312603dfc 100644 --- a/compiler/rustc_builtin_macros/src/format.rs +++ b/compiler/rustc_builtin_macros/src/format.rs @@ -250,6 +250,11 @@ struct Context<'a, 'b> { unused_names_lint: PositionalNamedArgsLint, } +pub struct FormatArg { + expr: P, + 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. @@ -263,8 +268,8 @@ fn parse_args<'a>( ecx: &mut ExtCtxt<'a>, sp: Span, tts: TokenStream, -) -> PResult<'a, (P, Vec>, FxHashMap)> { - let mut args = Vec::>::new(); +) -> PResult<'a, (P, Vec, FxHashMap)> { + let mut args = Vec::::new(); let mut names = FxHashMap::::default(); let mut p = ecx.new_parser_from_tts(tts); @@ -332,7 +337,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; @@ -344,7 +349,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()?; @@ -355,11 +360,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 }); } } } @@ -1180,7 +1185,7 @@ pub fn expand_preparsed_format_args( ecx: &mut ExtCtxt<'_>, sp: Span, efmt: P, - args: Vec>, + args: Vec, names: FxHashMap, append_newline: bool, ) -> P { @@ -1270,6 +1275,24 @@ 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::>(); + let mut suggestions = vec![(captured_arg_span, positional_args.len().to_string())]; + 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, + }; + suggestions.push((span.shrink_to_hi(), format!(", {}", arg))) + } + e.multipart_suggestion_verbose( + "consider using a positional formatting argument instead", + suggestions, + Applicability::MachineApplicable, + ); + } e.emit(); return DummyResult::raw_expr(sp, true); } @@ -1284,7 +1307,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 index a7ff9711691fb..4d8bd20aa36ae 100644 --- 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 @@ -228,13 +229,19 @@ impl<'a> Iterator for Parser<'a> { Some(String(self.string(pos + 1))) } else { let arg = self.argument(lbrace_end); - if let Some(rbrace_byte_idx) = self.must_consume('}') { - let lbrace_inner_offset = self.to_span_index(pos); - let rbrace_inner_offset = self.to_span_index(rbrace_byte_idx); - if self.is_literal { - self.arg_places.push( - lbrace_inner_offset.to(InnerOffset(rbrace_inner_offset.0 + 1)), - ); + match self.must_consume('}') { + Some(rbrace_byte_idx) => { + let lbrace_inner_offset = self.to_span_index(pos); + let rbrace_inner_offset = self.to_span_index(rbrace_byte_idx); + if self.is_literal { + self.arg_places.push( + lbrace_inner_offset + .to(InnerOffset(rbrace_inner_offset.0 + 1)), + ); + } + } + None => { + self.suggest_positional_arg_instead_of_captured_arg(arg); } } Some(NextArgument(arg)) @@ -313,6 +320,7 @@ impl<'a> Parser<'a> { label: label.into(), span, secondary_label: None, + should_be_replaced_with_positional_argument: false, }); } @@ -336,6 +344,7 @@ impl<'a> Parser<'a> { label: label.into(), span, secondary_label: None, + should_be_replaced_with_positional_argument: false, }); } @@ -407,6 +416,7 @@ impl<'a> Parser<'a> { label, span: pos.to(pos), secondary_label, + should_be_replaced_with_positional_argument: false, }); None } @@ -434,6 +444,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 +761,29 @@ 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); + 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 new file mode 100644 index 0000000000000..a8d7b44fb3d31 --- /dev/null +++ b/src/test/ui/fmt/struct-field-as-captured-argument.fixed @@ -0,0 +1,15 @@ +// 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 +} diff --git a/src/test/ui/fmt/struct-field-as-captured-argument.rs b/src/test/ui/fmt/struct-field-as-captured-argument.rs new file mode 100644 index 0000000000000..e23c14190b0df --- /dev/null +++ b/src/test/ui/fmt/struct-field-as-captured-argument.rs @@ -0,0 +1,15 @@ +// 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 +} diff --git a/src/test/ui/fmt/struct-field-as-captured-argument.stderr b/src/test/ui/fmt/struct-field-as-captured-argument.stderr new file mode 100644 index 0000000000000..28d3c8f838bb7 --- /dev/null +++ b/src/test/ui/fmt/struct-field-as-captured-argument.stderr @@ -0,0 +1,46 @@ +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: aborting due to 4 previous errors + From a0a2ec332621d35b92633dc4db0451e0c3cb4ab2 Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Wed, 3 Aug 2022 11:28:00 +0900 Subject: [PATCH 2/4] add tests for `Debug` formatters and precision formatters --- .../struct-field-as-captured-argument.fixed | 3 ++ .../fmt/struct-field-as-captured-argument.rs | 3 ++ .../struct-field-as-captured-argument.stderr | 35 ++++++++++++++++++- 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/test/ui/fmt/struct-field-as-captured-argument.fixed b/src/test/ui/fmt/struct-field-as-captured-argument.fixed index a8d7b44fb3d31..f7244f6744f3a 100644 --- a/src/test/ui/fmt/struct-field-as-captured-argument.fixed +++ b/src/test/ui/fmt/struct-field-as-captured-argument.fixed @@ -12,4 +12,7 @@ fn main() { 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 index e23c14190b0df..ab5f2552bd323 100644 --- a/src/test/ui/fmt/struct-field-as-captured-argument.rs +++ b/src/test/ui/fmt/struct-field-as-captured-argument.rs @@ -12,4 +12,7 @@ fn main() { 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 index 28d3c8f838bb7..7ea8b4068f272 100644 --- a/src/test/ui/fmt/struct-field-as-captured-argument.stderr +++ b/src/test/ui/fmt/struct-field-as-captured-argument.stderr @@ -42,5 +42,38 @@ help: consider using a positional formatting argument instead LL | format!("{1} {} {baz}", "aa", foo.field, baz = 3); | ~ +++++++++++ -error: aborting due to 4 previous errors +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 From dcd70c0995a6773ba3f78c072a899c62329f6a41 Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Thu, 4 Aug 2022 11:51:25 +0900 Subject: [PATCH 3/4] return when captured argument is not a struct field --- compiler/rustc_builtin_macros/src/format.rs | 15 +++++++------ compiler/rustc_parse_format/src/lib.rs | 25 ++++++++++----------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs index 1fdc312603dfc..515dedb23c202 100644 --- a/compiler/rustc_builtin_macros/src/format.rs +++ b/compiler/rustc_builtin_macros/src/format.rs @@ -1279,19 +1279,20 @@ pub fn expand_preparsed_format_args( 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::>(); - let mut suggestions = vec![(captured_arg_span, positional_args.len().to_string())]; 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, }; - suggestions.push((span.shrink_to_hi(), format!(", {}", arg))) + 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.multipart_suggestion_verbose( - "consider using a positional formatting argument instead", - suggestions, - Applicability::MachineApplicable, - ); } e.emit(); return DummyResult::raw_expr(sp, true); diff --git a/compiler/rustc_parse_format/src/lib.rs b/compiler/rustc_parse_format/src/lib.rs index 4d8bd20aa36ae..c98c8cae3782e 100644 --- a/compiler/rustc_parse_format/src/lib.rs +++ b/compiler/rustc_parse_format/src/lib.rs @@ -229,20 +229,16 @@ impl<'a> Iterator for Parser<'a> { Some(String(self.string(pos + 1))) } else { let arg = self.argument(lbrace_end); - match self.must_consume('}') { - Some(rbrace_byte_idx) => { - let lbrace_inner_offset = self.to_span_index(pos); - let rbrace_inner_offset = self.to_span_index(rbrace_byte_idx); - if self.is_literal { - self.arg_places.push( - lbrace_inner_offset - .to(InnerOffset(rbrace_inner_offset.0 + 1)), - ); - } - } - None => { - self.suggest_positional_arg_instead_of_captured_arg(arg); + if let Some(rbrace_byte_idx) = self.must_consume('}') { + let lbrace_inner_offset = self.to_span_index(pos); + let rbrace_inner_offset = self.to_span_index(rbrace_byte_idx); + if self.is_literal { + self.arg_places.push( + lbrace_inner_offset.to(InnerOffset(rbrace_inner_offset.0 + 1)), + ); } + } else { + self.suggest_positional_arg_instead_of_captured_arg(arg); } Some(NextArgument(arg)) } @@ -767,6 +763,9 @@ impl<'a> Parser<'a> { let byte_pos = self.to_span_index(end); let start = InnerOffset(byte_pos.0 + 1); let field = self.argument(start); + if !self.consume('}') { + return; + } if let ArgumentNamed(_) = arg.position { if let ArgumentNamed(_) = field.position { self.errors.insert( From 8c85c9936f6bc28b6e8e31bed8f4b1bd95c7e836 Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Thu, 4 Aug 2022 20:43:35 +0900 Subject: [PATCH 4/4] add a comment about what we can parse now --- compiler/rustc_parse_format/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compiler/rustc_parse_format/src/lib.rs b/compiler/rustc_parse_format/src/lib.rs index c98c8cae3782e..4890fade50faf 100644 --- a/compiler/rustc_parse_format/src/lib.rs +++ b/compiler/rustc_parse_format/src/lib.rs @@ -763,6 +763,8 @@ impl<'a> Parser<'a> { 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; }