Rollup merge of #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 #115466

Author: Dylan-DPC

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

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)]

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);

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,
                         },
                     );
                 }

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);
+}

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
+