From 05b5046633e9f594f955e0365a1219d1a96a5b54 Mon Sep 17 00:00:00 2001 From: Ezra Shaw Date: Fri, 17 Mar 2023 22:27:17 +1300 Subject: [PATCH] feat: implement error recovery in `expected_ident_found` --- .../rustc_parse/src/parser/diagnostics.rs | 84 +++++++++++++------ compiler/rustc_parse/src/parser/item.rs | 8 +- compiler/rustc_parse/src/parser/mod.rs | 13 ++- compiler/rustc_parse/src/parser/pat.rs | 10 ++- tests/ui/parser/ident-recovery.rs | 16 ++++ tests/ui/parser/ident-recovery.stderr | 42 ++++++++++ tests/ui/parser/issues/issue-104088.rs | 31 +++---- tests/ui/parser/issues/issue-104088.stderr | 58 ++++++++----- 8 files changed, 184 insertions(+), 78 deletions(-) create mode 100644 tests/ui/parser/ident-recovery.rs create mode 100644 tests/ui/parser/ident-recovery.stderr 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(); - let suggest_remove_comma = (self.token == token::Comma - && self.look_ahead(1, |t| t.is_ident())) - .then_some(SuggRemoveComma { span: self.token.span }); + // 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)); - 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); + // `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 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() { + if let 2e1 = 123 { + //~^ ERROR mismatched types + } + + 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 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` -} - 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; - | ^ - -error: expected identifier, found `2x` - --> $DIR/issue-104088.rs:11:9 - | -LL | let 2x: i32 = 123; - | ^^ expected identifier - | -help: identifiers cannot start with a number - --> $DIR/issue-104088.rs:11:9 - | -LL | let 2x: i32 = 123; - | ^ +LL | fn 1234test() { + | ^^^^ error: expected identifier, found `23name` - --> $DIR/issue-104088.rs:22:9 + --> $DIR/issue-104088.rs:9:9 | LL | let 23name = 123; | ^^^^^^ expected identifier | help: identifiers cannot start with a number - --> $DIR/issue-104088.rs:22:9 + --> $DIR/issue-104088.rs:9:9 | LL | let 23name = 123; | ^^ +error: expected identifier, found `2x` + --> $DIR/issue-104088.rs:12:9 + | +LL | let 2x: i32 = 123; + | ^^ expected identifier + | +help: identifiers cannot start with a number + --> $DIR/issue-104088.rs:12:9 + | +LL | let 2x: i32 = 123; + | ^ + +error: expected identifier, found `1x` + --> $DIR/issue-104088.rs:15:9 + | +LL | let 1x = 123; + | ^^ expected identifier + | +help: identifiers cannot start with a number + --> $DIR/issue-104088.rs:15:9 + | +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`.