add diagnostic for raw identifiers in format string

rust-lang · Sep 6, 2023 · d990eee · d990eee
1 parent a0c28cd
commit d990eee
Show file tree

Hide file tree

Showing 6 changed files with 158 additions and 35 deletions.
diff --git a/compiler/rustc_builtin_macros/messages.ftl b/compiler/rustc_builtin_macros/messages.ftl
@@ -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}
@@ -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
@@ -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

diff --git a/compiler/rustc_builtin_macros/src/errors.rs b/compiler/rustc_builtin_macros/src/errors.rs
@@ -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)]

diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs
@@ -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);

diff --git a/compiler/rustc_parse_format/src/lib.rs b/compiler/rustc_parse_format/src/lib.rs
@@ -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
@@ -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,
         });
     }
 
@@ -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,
         });
     }
 
@@ -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
@@ -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
@@ -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,
                 },
             );
         }
@@ -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,
                         },
                     );
                 }

diff --git a/tests/ui/fmt/raw-idents.rs b/tests/ui/fmt/raw-idents.rs
@@ -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);
 }
diff --git a/tests/ui/fmt/raw-idents.stderr b/tests/ui/fmt/raw-idents.stderr
@@ -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