feat: implement error recovery in `expected_ident_found`

8 files changed, 175 insertions, 69 deletions

diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs
index 47d11084915..9544afd3d6d 100644
--- a/compiler/rustc_parse/src/parser/diagnostics.rs
+++ b/compiler/rustc_parse/src/parser/diagnostics.rs
@@ -269,13 +269,18 @@ impl<'a> Parser<'a> {
     }
 
     /// Emits an error with suggestions if an identifier was expected but not found.
-    pub(super) fn expected_ident_found(&mut self) -> DiagnosticBuilder<'a, ErrorGuaranteed> {
+    ///
+    /// Returns a possibly recovered identifier.
+    pub(super) fn expected_ident_found(
+        &mut self,
+        recover: bool,
+    ) -> PResult<'a, (Ident, /* is_raw */ bool)> {
         if let TokenKind::DocComment(..) = self.prev_token.kind {
-            return DocCommentDoesNotDocumentAnything {
+            return Err(DocCommentDoesNotDocumentAnything {
                 span: self.prev_token.span,
                 missing_comma: None,
             }
-            .into_diagnostic(&self.sess.span_diagnostic);
+            .into_diagnostic(&self.sess.span_diagnostic));
         }
 
         let valid_follow = &[
@@ -290,34 +295,51 @@ impl<'a> Parser<'a> {
             TokenKind::CloseDelim(Delimiter::Parenthesis),
         ];
 
-        let suggest_raw = match self.token.ident() {
-            Some((ident, false))
-                if ident.is_raw_guess()
-                    && self.look_ahead(1, |t| valid_follow.contains(&t.kind)) =>
-            {
-                Some(SuggEscapeIdentifier {
-                    span: ident.span.shrink_to_lo(),
-                    // `Symbol::to_string()` is different from `Symbol::into_diagnostic_arg()`,
-                    // which uses `Symbol::to_ident_string()` and "helpfully" adds an implicit `r#`
-                    ident_name: ident.name.to_string(),
-                })
-            }
-            _ => None,
-        };
+        let mut recovered_ident = None;
+        // we take this here so that the correct original token is retained in
+        // the diagnostic, regardless of eager recovery.
+        let bad_token = self.token.clone();
+
+        // suggest prepending a keyword in identifier position with `r#`
+        let suggest_raw = if let Some((ident, false)) = self.token.ident()
+            && ident.is_raw_guess()
+            && self.look_ahead(1, |t| valid_follow.contains(&t.kind))
+        {
+            recovered_ident = Some((ident, true));
+
+            // `Symbol::to_string()` is different from `Symbol::into_diagnostic_arg()`,
+            // which uses `Symbol::to_ident_string()` and "helpfully" adds an implicit `r#`
+            let ident_name = ident.name.to_string();
+
+            Some(SuggEscapeIdentifier {
+                span: ident.span.shrink_to_lo(),
+                ident_name
+            })
+        } else { None };
+
+        let suggest_remove_comma =
+            if self.token == token::Comma && self.look_ahead(1, |t| t.is_ident()) {
+                if recover {
+                    self.bump();
+                    recovered_ident = self.ident_or_err(false).ok();
+                };
+
+                Some(SuggRemoveComma { span: bad_token.span })
+            } else {
+                None
+            };
 
-        let suggest_remove_comma = (self.token == token::Comma
-            && self.look_ahead(1, |t| t.is_ident()))
-        .then_some(SuggRemoveComma { span: self.token.span });
+        let help_cannot_start_number = self.is_lit_bad_ident().map(|(len, valid_portion)| {
+            let (invalid, valid) = self.token.span.split_at(len as u32);
 
-        let help_cannot_start_number = self.is_lit_bad_ident().map(|(len, _valid_portion)| {
-            let (invalid, _valid) = self.token.span.split_at(len as u32);
+            recovered_ident = Some((Ident::new(valid_portion, valid), false));
 
             HelpIdentifierStartsWithNumber { num_span: invalid }
         });
 
         let err = ExpectedIdentifier {
-            span: self.token.span,
-            token: self.token.clone(),
+            span: bad_token.span,
+            token: bad_token,
             suggest_raw,
             suggest_remove_comma,
             help_cannot_start_number,
@@ -326,6 +348,7 @@ impl<'a> Parser<'a> {
 
         // if the token we have is a `<`
         // it *might* be a misplaced generic
+        // FIXME: could we recover with this?
         if self.token == token::Lt {
             // all keywords that could have generic applied
             let valid_prev_keywords =
@@ -376,7 +399,16 @@ impl<'a> Parser<'a> {
             }
         }
 
-        err
+        if let Some(recovered_ident) = recovered_ident && recover {
+            err.emit();
+            Ok(recovered_ident)
+        } else {
+            Err(err)
+        }
+    }
+
+    pub(super) fn expected_ident_found_err(&mut self) -> DiagnosticBuilder<'a, ErrorGuaranteed> {
+        self.expected_ident_found(false).unwrap_err()
     }
 
     /// Checks if the current token is a integer or float literal and looks like
@@ -392,7 +424,7 @@ impl<'a> Parser<'a> {
             kind: token::LitKind::Integer | token::LitKind::Float,
             symbol,
             suffix,
-        }) = self.token.uninterpolate().kind
+        }) = self.token.kind
             && rustc_ast::MetaItemLit::from_token(&self.token).is_none()
         {
             Some((symbol.as_str().len(), suffix.unwrap()))
diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs
index b7415dc8fb8..ae8fe90e9d6 100644
--- a/compiler/rustc_parse/src/parser/item.rs
+++ b/compiler/rustc_parse/src/parser/item.rs
@@ -1181,7 +1181,7 @@ impl<'a> Parser<'a> {
         defaultness: Defaultness,
     ) -> PResult<'a, ItemInfo> {
         let impl_span = self.token.span;
-        let mut err = self.expected_ident_found();
+        let mut err = self.expected_ident_found_err();
 
         // Only try to recover if this is implementing a trait for a type
         let mut impl_info = match self.parse_item_impl(attrs, defaultness) {
@@ -1776,7 +1776,7 @@ impl<'a> Parser<'a> {
                     Err(err) => {
                         err.cancel();
                         self.restore_snapshot(snapshot);
-                        self.expected_ident_found()
+                        self.expected_ident_found_err()
                     }
                 }
             } else if self.eat_keyword(kw::Struct) {
@@ -1792,11 +1792,11 @@ impl<'a> Parser<'a> {
                     Err(err) => {
                         err.cancel();
                         self.restore_snapshot(snapshot);
-                        self.expected_ident_found()
+                        self.expected_ident_found_err()
                     }
                 }
             } else {
-                let mut err = self.expected_ident_found();
+                let mut err = self.expected_ident_found_err();
                 if self.eat_keyword_noexpect(kw::Let)
                     && let removal_span = self.prev_token.span.until(self.token.span)
                     && let Ok(ident) = self.parse_ident_common(false)
diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs
index 6991520895d..53c25a80c4b 100644
--- a/compiler/rustc_parse/src/parser/mod.rs
+++ b/compiler/rustc_parse/src/parser/mod.rs
@@ -553,8 +553,9 @@ impl<'a> Parser<'a> {
 
     fn parse_ident_common(&mut self, recover: bool) -> PResult<'a, Ident> {
         let (ident, is_raw) = self.ident_or_err(recover)?;
+
         if !is_raw && ident.is_reserved() {
-            let mut err = self.expected_ident_found();
+            let mut err = self.expected_ident_found_err();
             if recover {
                 err.emit();
             } else {
@@ -565,12 +566,16 @@ impl<'a> Parser<'a> {
         Ok(ident)
     }
 
-    fn ident_or_err(&mut self, _recover: bool) -> PResult<'a, (Ident, /* is_raw */ bool)> {
-        let result = self.token.ident().ok_or_else(|| self.expected_ident_found());
+    fn ident_or_err(&mut self, recover: bool) -> PResult<'a, (Ident, /* is_raw */ bool)> {
+        let result = self.token.ident().ok_or_else(|| self.expected_ident_found(recover));
 
         let (ident, is_raw) = match result {
             Ok(ident) => ident,
-            Err(err) => return Err(err),
+            Err(err) => match err {
+                // we recovered!
+                Ok(ident) => ident,
+                Err(err) => return Err(err),
+            },
         };
 
         Ok((ident, is_raw))
diff --git a/compiler/rustc_parse/src/parser/pat.rs b/compiler/rustc_parse/src/parser/pat.rs
index d9af2415848..2246002f5d3 100644
--- a/compiler/rustc_parse/src/parser/pat.rs
+++ b/compiler/rustc_parse/src/parser/pat.rs
@@ -391,7 +391,13 @@ impl<'a> Parser<'a> {
             } else {
                 PatKind::Lit(const_expr)
             }
-        } else if self.can_be_ident_pat() || self.is_lit_bad_ident().is_some() {
+        // Don't eagerly error on semantically invalid tokens when matching
+        // declarative macros, as the input to those doesn't have to be
+        // semantically valid. For attribute/derive proc macros this is not the
+        // case, so doing the recovery for them is fine.
+        } else if self.can_be_ident_pat()
+            || (self.is_lit_bad_ident().is_some() && self.may_recover())
+        {
             // Parse `ident @ pat`
             // This can give false positives and parse nullary enums,
             // they are dealt with later in resolve.
@@ -590,7 +596,7 @@ impl<'a> Parser<'a> {
         // Make sure we don't allow e.g. `let mut $p;` where `$p:pat`.
         if let token::Interpolated(nt) = &self.token.kind {
             if let token::NtPat(_) = **nt {
-                self.expected_ident_found().emit();
+                self.expected_ident_found_err().emit();
             }
         }
 
diff --git a/tests/ui/parser/ident-recovery.rs b/tests/ui/parser/ident-recovery.rs
new file mode 100644
index 00000000000..7575372b940
--- /dev/null
+++ b/tests/ui/parser/ident-recovery.rs
@@ -0,0 +1,16 @@
+fn ,comma() {
+    //~^ ERROR expected identifier, found `,`
+    struct Foo {
+        x: i32,,
+        //~^ ERROR expected identifier, found `,`
+        y: u32,
+    }
+}
+
+fn break() {
+//~^ ERROR expected identifier, found keyword `break`
+    let continue = 5;
+    //~^ ERROR expected identifier, found keyword `continue`
+}
+
+fn main() {}
diff --git a/tests/ui/parser/ident-recovery.stderr b/tests/ui/parser/ident-recovery.stderr
new file mode 100644
index 00000000000..e9a55026d12
--- /dev/null
+++ b/tests/ui/parser/ident-recovery.stderr
@@ -0,0 +1,42 @@
+error: expected identifier, found `,`
+  --> $DIR/ident-recovery.rs:1:4
+   |
+LL | fn ,comma() {
+   |    ^
+   |    |
+   |    expected identifier
+   |    help: remove this comma
+
+error: expected identifier, found `,`
+  --> $DIR/ident-recovery.rs:4:16
+   |
+LL |         x: i32,,
+   |                ^
+   |                |
+   |                expected identifier
+   |                help: remove this comma
+
+error: expected identifier, found keyword `break`
+  --> $DIR/ident-recovery.rs:10:4
+   |
+LL | fn break() {
+   |    ^^^^^ expected identifier, found keyword
+   |
+help: escape `break` to use it as an identifier
+   |
+LL | fn r#break() {
+   |    ++
+
+error: expected identifier, found keyword `continue`
+  --> $DIR/ident-recovery.rs:12:9
+   |
+LL |     let continue = 5;
+   |         ^^^^^^^^ expected identifier, found keyword
+   |
+help: escape `continue` to use it as an identifier
+   |
+LL |     let r#continue = 5;
+   |         ++
+
+error: aborting due to 4 previous errors
+
diff --git a/tests/ui/parser/issues/issue-104088.rs b/tests/ui/parser/issues/issue-104088.rs
index 86988c8cd21..3dc636b6a33 100644
--- a/tests/ui/parser/issues/issue-104088.rs
+++ b/tests/ui/parser/issues/issue-104088.rs
@@ -1,26 +1,19 @@
-fn test() {
+fn 1234test() {
+//~^ ERROR expected identifier, found `1234test`
     if let 123 = 123 { println!("yes"); }
-}
-
-fn test_2() {
-    let 1x = 123;
-    //~^ ERROR expected identifier, found `1x`
-}
-
-fn test_3() {
-    let 2x: i32 = 123;
-    //~^ ERROR expected identifier, found `2x`
-}
 
-fn test_4() {
     if let 2e1 = 123 {
         //~^ ERROR mismatched types
     }
-}
 
-fn test_5() {
     let 23name = 123;
     //~^ ERROR expected identifier, found `23name`
+
+    let 2x: i32 = 123;
+    //~^ ERROR expected identifier, found `2x`
+
+    let 1x = 123;
+    //~^ ERROR expected identifier, found `1x`
 }
 
 fn main() {}
diff --git a/tests/ui/parser/issues/issue-104088.stderr b/tests/ui/parser/issues/issue-104088.stderr
index 08ea1c891c1..8b751759d69 100644
--- a/tests/ui/parser/issues/issue-104088.stderr
+++ b/tests/ui/parser/issues/issue-104088.stderr
@@ -1,47 +1,59 @@
-error: expected identifier, found `1x`
-  --> $DIR/issue-104088.rs:6:9
+error: expected identifier, found `1234test`
+  --> $DIR/issue-104088.rs:1:4
    |
-LL |     let 1x = 123;
-   |         ^^ expected identifier
+LL | fn 1234test() {
+   |    ^^^^^^^^ expected identifier
    |
 help: identifiers cannot start with a number
-  --> $DIR/issue-104088.rs:6:9
+  --> $DIR/issue-104088.rs:1:4
    |
-LL |     let 1x = 123;
-   |         ^
+LL | fn 1234test() {
+   |    ^^^^
+
+error: expected identifier, found `23name`
+  --> $DIR/issue-104088.rs:9:9
+   |
+LL |     let 23name = 123;
+   |         ^^^^^^ expected identifier
+   |
+help: identifiers cannot start with a number
+  --> $DIR/issue-104088.rs:9:9
+   |
+LL |     let 23name = 123;
+   |         ^^
 
 error: expected identifier, found `2x`
-  --> $DIR/issue-104088.rs:11:9
+  --> $DIR/issue-104088.rs:12:9
    |
 LL |     let 2x: i32 = 123;
    |         ^^ expected identifier
    |
 help: identifiers cannot start with a number
-  --> $DIR/issue-104088.rs:11:9
+  --> $DIR/issue-104088.rs:12:9
    |
 LL |     let 2x: i32 = 123;
    |         ^
 
-error: expected identifier, found `23name`
-  --> $DIR/issue-104088.rs:22:9
+error: expected identifier, found `1x`
+  --> $DIR/issue-104088.rs:15:9
    |
-LL |     let 23name = 123;
-   |         ^^^^^^ expected identifier
+LL |     let 1x = 123;
+   |         ^^ expected identifier
    |
 help: identifiers cannot start with a number
-  --> $DIR/issue-104088.rs:22:9
+  --> $DIR/issue-104088.rs:15:9
    |
-LL |     let 23name = 123;
-   |         ^^
+LL |     let 1x = 123;
+   |         ^
 
 error[E0308]: mismatched types
-  --> $DIR/issue-104088.rs:16:12
+  --> $DIR/issue-104088.rs:5:12
    |
 LL |     if let 2e1 = 123 {
    |            ^^^   --- this expression has type `{integer}`
    |            |
    |            expected integer, found floating-point number
 
-error: aborting due to 4 previous errors
+error: aborting due to 5 previous errors
 
 For more information about this error, try `rustc --explain E0308`.