Skip to content
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

add diagnostic for raw identifiers in format string #115611

Merged
merged 1 commit into from
Sep 17, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 4 additions & 2 deletions compiler/rustc_builtin_macros/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ builtin_macros_format_positional_after_named = positional arguments cannot follo
.label = positional arguments must be before named arguments
.named_args = named argument

builtin_macros_format_remove_raw_ident = remove the `r#`

builtin_macros_format_requires_string = requires at least a format string argument

builtin_macros_format_string_invalid = invalid format string: {$desc}
Expand Down Expand Up @@ -165,6 +167,8 @@ builtin_macros_format_unused_arg = {$named ->
builtin_macros_format_unused_args = multiple unused formatting arguments
.label = multiple missing formatting specifiers

builtin_macros_format_use_positional = consider using a positional formatting argument instead

builtin_macros_global_asm_clobber_abi = `clobber_abi` cannot be used with `global_asm!`

builtin_macros_invalid_crate_attribute = invalid crate attribute
Expand Down Expand Up @@ -205,8 +209,6 @@ builtin_macros_requires_cfg_pattern =

builtin_macros_should_panic = functions using `#[should_panic]` must return `()`

builtin_macros_sugg = consider using a positional formatting argument instead

builtin_macros_test_arg_non_lifetime = functions used as tests can not have any non-lifetime generic parameters

builtin_macros_test_args = functions used as tests can not have any arguments
Expand Down
35 changes: 23 additions & 12 deletions compiler/rustc_builtin_macros/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -539,18 +539,29 @@ pub(crate) struct InvalidFormatStringLabel {
}

#[derive(Subdiagnostic)]
#[multipart_suggestion(
builtin_macros_sugg,
style = "verbose",
applicability = "machine-applicable"
)]
pub(crate) struct InvalidFormatStringSuggestion {
#[suggestion_part(code = "{len}")]
pub(crate) captured: Span,
pub(crate) len: String,
#[suggestion_part(code = ", {arg}")]
pub(crate) span: Span,
pub(crate) arg: String,
pub(crate) enum InvalidFormatStringSuggestion {
#[multipart_suggestion(
builtin_macros_format_use_positional,
style = "verbose",
applicability = "machine-applicable"
)]
UsePositional {
#[suggestion_part(code = "{len}")]
captured: Span,
len: String,
#[suggestion_part(code = ", {arg}")]
span: Span,
arg: String,
},
#[suggestion(
builtin_macros_format_remove_raw_ident,
code = "",
applicability = "machine-applicable"
)]
RemoveRawIdent {
#[primary_span]
span: Span,
},
}

#[derive(Diagnostic)]
Expand Down
37 changes: 23 additions & 14 deletions compiler/rustc_builtin_macros/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,20 +260,29 @@ fn make_format_args(
if let Some((label, span)) = err.secondary_label && is_source_literal {
e.label_ = Some(errors::InvalidFormatStringLabel { span: 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));
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 {
captured: captured_arg_span,
len: args.unnamed_args().len().to_string(),
span: span.shrink_to_hi(),
arg,
});
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::RemoveRawIdent(span) => {
if is_source_literal {
let span = fmt_span.from_inner(InnerSpan::new(span.start, span.end));
e.sugg_ = Some(errors::InvalidFormatStringSuggestion::RemoveRawIdent { span })
}
}
}
ecx.emit_err(e);
Expand Down
54 changes: 47 additions & 7 deletions compiler/rustc_parse_format/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,17 @@ 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,
pub suggestion: Suggestion,
}

pub enum Suggestion {
None,
/// Replace inline argument with positional argument:
/// `format!("{foo.bar}")` -> `format!("{}", foo.bar)`
UsePositional,
/// Remove `r#` from identifier:
/// `format!("{r#foo}")` -> `format!("{foo}")`
RemoveRawIdent(InnerSpan),
}

/// The parser structure for interpreting the input format string. This is
Expand Down Expand Up @@ -365,7 +375,7 @@ impl<'a> Parser<'a> {
label: label.into(),
span,
secondary_label: None,
should_be_replaced_with_positional_argument: false,
suggestion: Suggestion::None,
});
}

Expand All @@ -389,7 +399,7 @@ impl<'a> Parser<'a> {
label: label.into(),
span,
secondary_label: None,
should_be_replaced_with_positional_argument: false,
suggestion: Suggestion::None,
});
}

Expand Down Expand Up @@ -493,7 +503,7 @@ impl<'a> Parser<'a> {
label,
span: pos.to(pos),
secondary_label,
should_be_replaced_with_positional_argument: false,
suggestion: Suggestion::None,
});

None
Expand Down Expand Up @@ -573,7 +583,37 @@ impl<'a> Parser<'a> {
Some(ArgumentIs(i))
} else {
match self.cur.peek() {
Some(&(_, c)) if rustc_lexer::is_id_start(c) => Some(ArgumentNamed(self.word())),
Some(&(lo, c)) if rustc_lexer::is_id_start(c) => {
let word = self.word();

// Recover from `r#ident` in format strings.
// FIXME: use a let chain
if word == "r" {
if let Some((pos, '#')) = self.cur.peek() {
if self.input[pos + 1..]
.chars()
.next()
.is_some_and(rustc_lexer::is_id_start)
{
self.cur.next();
let word = self.word();
let prefix_span = self.span(lo, lo + 2);
let full_span = self.span(lo, lo + 2 + word.len());
self.errors.insert(0, ParseError {
description: "raw identifiers are not supported".to_owned(),
note: Some("identifiers in format strings can be keywords and don't need to be prefixed with `r#`".to_string()),
label: "raw identifier used here".to_owned(),
span: full_span,
secondary_label: None,
suggestion: Suggestion::RemoveRawIdent(prefix_span),
});
return Some(ArgumentNamed(word));
}
}
}

Some(ArgumentNamed(word))
}

// This is an `ArgumentNext`.
// Record the fact and do the resolution after parsing the
Expand Down Expand Up @@ -841,7 +881,7 @@ impl<'a> Parser<'a> {
label: "expected `?` to occur after `:`".to_owned(),
span: pos.to(pos),
secondary_label: None,
should_be_replaced_with_positional_argument: false,
suggestion: Suggestion::None,
},
);
}
Expand All @@ -867,7 +907,7 @@ impl<'a> Parser<'a> {
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,
suggestion: Suggestion::UsePositional,
},
);
}
Expand Down
17 changes: 17 additions & 0 deletions tests/ui/fmt/raw-idents.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Regression test for https://github.com/rust-lang/rust/issues/115466

// The "identifier" in format strings is parsed as an IDENTIFIER_OR_KEYWORD, not an IDENTIFIER.
// Test that there is an actionable diagnostic if a RAW_IDENTIFIER is used instead.

fn main() {
let r#type = "foobar";
println!("It is {r#type}"); //~ ERROR: invalid format string: raw identifiers are not supported
println!(r##"It still is {r#type}"##); //~ ERROR: invalid format string: raw identifiers are not supported
println!(concat!("{r#", "type}")); //~ ERROR: invalid format string: raw identifiers are not supported
println!("{\x72\x23type:?}"); //~ ERROR: invalid format string: raw identifiers are not supported

// OK
println!("{type}");
println!("{let}", let = r#type);
println!("{let}", r#let = r#type);
}
44 changes: 44 additions & 0 deletions tests/ui/fmt/raw-idents.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
error: invalid format string: raw identifiers are not supported
--> $DIR/raw-idents.rs:8:22
|
LL | println!("It is {r#type}");
| --^^^^
| |
| raw identifier used here in format string
| help: remove the `r#`
|
= note: identifiers in format strings can be keywords and don't need to be prefixed with `r#`

error: invalid format string: raw identifiers are not supported
--> $DIR/raw-idents.rs:9:31
|
LL | println!(r##"It still is {r#type}"##);
| --^^^^
| |
| raw identifier used here in format string
| help: remove the `r#`
|
= note: identifiers in format strings can be keywords and don't need to be prefixed with `r#`

error: invalid format string: raw identifiers are not supported
--> $DIR/raw-idents.rs:10:14
|
LL | println!(concat!("{r#", "type}"));
| ^^^^^^^^^^^^^^^^^^^^^^^ raw identifier used here in format string
|
= note: identifiers in format strings can be keywords and don't need to be prefixed with `r#`
= note: this error originates in the macro `concat` (in Nightly builds, run with -Z macro-backtrace for more info)

error: invalid format string: raw identifiers are not supported
--> $DIR/raw-idents.rs:11:16
|
LL | println!("{\x72\x23type:?}");
| --------^^^^
| |
| raw identifier used here in format string
| help: remove the `r#`
|
= note: identifiers in format strings can be keywords and don't need to be prefixed with `r#`

error: aborting due to 4 previous errors