Skip to content

Suggest a positional formatting argument instead of a captured argument #100058

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 32 additions & 8 deletions compiler/rustc_builtin_macros/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,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.
Expand All @@ -263,8 +268,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);
Expand Down Expand Up @@ -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;
Expand All @@ -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()?;
Expand All @@ -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 });
}
}
}
Expand Down Expand Up @@ -1180,7 +1185,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> {
Expand Down Expand Up @@ -1270,6 +1275,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);
}
Expand All @@ -1284,7 +1308,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,
Expand Down
35 changes: 35 additions & 0 deletions compiler/rustc_parse_format/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
}
Expand Down Expand Up @@ -313,6 +316,7 @@ impl<'a> Parser<'a> {
label: label.into(),
span,
secondary_label: None,
should_be_replaced_with_positional_argument: false,
});
}

Expand All @@ -336,6 +340,7 @@ impl<'a> Parser<'a> {
label: label.into(),
span,
secondary_label: None,
should_be_replaced_with_positional_argument: false,
});
}

Expand Down Expand Up @@ -407,6 +412,7 @@ impl<'a> Parser<'a> {
label,
span: pos.to(pos),
secondary_label,
should_be_replaced_with_positional_argument: false,
});
None
}
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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('}') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a comment stating "We can only parse foo.bar field access, any deeper nesting, or another type of expression, like method calls, are not supported".

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
Expand Down
18 changes: 18 additions & 0 deletions src/test/ui/fmt/struct-field-as-captured-argument.fixed
Original file line number Diff line number Diff line change
@@ -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
}
18 changes: 18 additions & 0 deletions src/test/ui/fmt/struct-field-as-captured-argument.rs
Original file line number Diff line number Diff line change
@@ -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
}
79 changes: 79 additions & 0 deletions src/test/ui/fmt/struct-field-as-captured-argument.stderr
Original file line number Diff line number Diff line change
@@ -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