Skip to content

Commit

Permalink
Unrolled build for rust-lang#115611
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#115611 - lukas-code:format!("{r#ident}"), r=compiler-errors

add diagnostic for raw identifiers in format string

Format strings don't support raw identifiers (e.g. `format!("{r#type}")`), but they do support keywords in the format string directly (e.g. `format!("{type}")`). This PR improves the error output when attempting to use a raw identifier in a format string and adds a machine-applicable suggestion to remove the `r#`.

fixes rust-lang#115466
  • Loading branch information
rust-timer committed Sep 17, 2023
2 parents 8ed1d4a + d990eee commit 38ae704
Show file tree
Hide file tree
Showing 6 changed files with 158 additions and 35 deletions.
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

0 comments on commit 38ae704

Please sign in to comment.