improve parse item fallback

Author: Orion Gonzalez

compiler/rustc_ast_pretty/src/pprust/mod.rs

@@ -7,7 +7,7 @@ use std::borrow::Cow;
 use rustc_ast as ast;
 use rustc_ast::token::{Nonterminal, Token, TokenKind};
 use rustc_ast::tokenstream::{TokenStream, TokenTree};
-pub use state::{print_crate, AnnNode, Comments, PpAnn, PrintState, State};
+pub use state::{AnnNode, Comments, PpAnn, PrintState, State, print_crate};
 
 pub fn nonterminal_to_string(nt: &Nonterminal) -> String {
     State::new().nonterminal_to_string(nt)

compiler/rustc_parse/src/parser/item.rs

@@ -10,15 +10,15 @@ use rustc_ast::util::case::Case;
 use rustc_ast::{self as ast};
 use rustc_ast_pretty::pprust;
 use rustc_errors::codes::*;
-use rustc_errors::{struct_span_code_err, Applicability, PResult, StashKey};
+use rustc_errors::{Applicability, PResult, StashKey, struct_span_code_err};
 use rustc_span::edit_distance::edit_distance;
 use rustc_span::edition::Edition;
-use rustc_span::symbol::{kw, sym, Ident, Symbol};
-use rustc_span::{source_map, ErrorGuaranteed, Span, DUMMY_SP};
-use thin_vec::{thin_vec, ThinVec};
+use rustc_span::symbol::{Ident, Symbol, kw, sym};
+use rustc_span::{DUMMY_SP, ErrorGuaranteed, Span, source_map};
+use thin_vec::{ThinVec, thin_vec};
 use tracing::debug;
 
-use super::diagnostics::{dummy_arg, ConsumeClosingDelim};
+use super::diagnostics::{ConsumeClosingDelim, dummy_arg};
 use super::ty::{AllowPlus, RecoverQPath, RecoverReturnSign};
 use super::{
     AttemptLocalParseRecovery, AttrWrapper, FollowedByType, ForceCollect, Parser, PathStyle,
@@ -75,6 +75,8 @@ impl<'a> Parser<'a> {
             items.push(item);
         }
 
+        // The last token should be `term`: either EOF or `}`. If it's not that means that we've had an error
+        // parsing an item
         if !self.eat(term) {
             if !self.maybe_consume_incorrect_semicolon(items.last().map(|x| &**x)) {
                 let err = self.fallback_incorrect_item();
@@ -90,12 +92,15 @@ impl<'a> Parser<'a> {
     /// Tries to parse the item as a statement to provide further diagnostics.
     fn fallback_incorrect_item(&mut self) -> rustc_errors::Diag<'a> {
         let token_str = super::token_descr(&self.token);
-        let mut err = self
-            .dcx()
-            .struct_span_err(self.token.span, format!("expected item, found {token_str}"));
+        let token_span = self.token.span;
+        let mut err =
+            self.dcx().struct_span_err(token_span, format!("expected item, found {token_str}"));
+
+        let mut do_default_diag = true;
 
         match self.parse_full_stmt(AttemptLocalParseRecovery::No) {
             Ok(Some(stmt)) => {
+                do_default_diag = false;
                 let span = stmt.span;
                 match &stmt.kind {
                     StmtKind::Let(_) => {
@@ -121,16 +126,22 @@ impl<'a> Parser<'a> {
                         );
                     }
                     StmtKind::Item(_) | StmtKind::MacCall(_) => {
-                        unreachable!("These should be valid.")
+                        unreachable!("These should be valid items!")
                     }
                 };
             }
-            Ok(None) => {} // It's not a statement, not much we can do.
+            // It's not a statement, we can't do much recovery.
+            Ok(None) => {}
             Err(e) => {
                 // We don't really care about an error parsing this statement.
                 e.cancel();
             }
         }
+
+        if do_default_diag {
+            err.span_label(token_span, "expected item");
+        }
+
         err.note("for a full list of items that can appear in modules, see <https://doc.rust-lang.org/reference/items.html>");
 
         err
@@ -490,11 +501,7 @@ impl<'a> Parser<'a> {
             None
         };
 
-        if let Some(err) = err {
-            Err(self.dcx().create_err(err))
-        } else {
-            Ok(())
-        }
+        if let Some(err) = err { Err(self.dcx().create_err(err)) } else { Ok(()) }
     }
 
     fn parse_item_builtin(&mut self) -> PResult<'a, Option<ItemInfo>> {
@@ -1154,11 +1161,7 @@ impl<'a> Parser<'a> {
     }
 
     fn parse_rename(&mut self) -> PResult<'a, Option<Ident>> {
-        if self.eat_keyword(kw::As) {
-            self.parse_ident_or_underscore().map(Some)
-        } else {
-            Ok(None)
-        }
+        if self.eat_keyword(kw::As) { self.parse_ident_or_underscore().map(Some) } else { Ok(None) }
     }
 
     fn parse_ident_or_underscore(&mut self) -> PResult<'a, Ident> {
@@ -1241,7 +1244,7 @@ impl<'a> Parser<'a> {
         mut safety: Safety,
     ) -> PResult<'a, ItemInfo> {
         let abi = self.parse_abi(); // ABI?
-                                    // FIXME: This recovery should be tested better.
+        // FIXME: This recovery should be tested better.
         if safety == Safety::Default
             && self.token.is_keyword(kw::Unsafe)
             && self.look_ahead(1, |t| *t == token::OpenDelim(Delimiter::Brace))
@@ -1943,10 +1946,10 @@ impl<'a> Parser<'a> {
                 // Try to recover extra trailing angle brackets
                 if let TyKind::Path(_, Path { segments, .. }) = &a_var.ty.kind {
                     if let Some(last_segment) = segments.last() {
-                        let guar = self.check_trailing_angle_brackets(
-                            last_segment,
-                            &[&token::Comma, &token::CloseDelim(Delimiter::Brace)],
-                        );
+                        let guar = self.check_trailing_angle_brackets(last_segment, &[
+                            &token::Comma,
+                            &token::CloseDelim(Delimiter::Brace),
+                        ]);
                         if let Some(_guar) = guar {
                             // Handle a case like `Vec<u8>>,` where we can continue parsing fields
                             // after the comma
@@ -2153,7 +2156,7 @@ impl<'a> Parser<'a> {
                 self.unexpected()?;
             }
             let body = self.parse_token_tree(); // `MacBody`
-                                                // Convert `MacParams MacBody` into `{ MacParams => MacBody }`.
+            // Convert `MacParams MacBody` into `{ MacParams => MacBody }`.
             let bspan = body.span();
             let arrow = TokenTree::token_alone(token::FatArrow, pspan.between(bspan)); // `=>`
             let tokens = TokenStream::new(vec![params, arrow, body]);

tests/ui/parser/issues/issue-113110-non-item-at-module-root.stderr

@@ -2,8 +2,9 @@ error: expected item, found `5`
   --> $DIR/issue-113110-non-item-at-module-root.rs:1:2
    |
 LL |  5
-   |  ^ expected item
+   |  ^ unexpected expression
    |
+   = help: consider putting it inside a function: fn foo() { 5 }
    = note: for a full list of items that can appear in modules, see <https://doc.rust-lang.org/reference/items.html>
 
 error: aborting due to 1 previous error

tests/ui/parser/issues/issue-62913.stderr

@@ -16,8 +16,9 @@ error: expected item, found `"\u\"`
   --> $DIR/issue-62913.rs:1:1
    |
 LL | "\u\"
-   | ^^^^^^ expected item
+   | ^^^^^^ unexpected expression
    |
+   = help: consider putting it inside a function: fn foo() { "\u\" }
    = note: for a full list of items that can appear in modules, see <https://doc.rust-lang.org/reference/items.html>
 
 error: aborting due to 3 previous errors

tests/ui/parser/recover/pub-let-outside-fn.rs

@@ -0,0 +1,5 @@
+mod m {
+    pub let answer = 42;
+    //~^ ERROR visibility `pub` is not followed by an item
+    //~| ERROR expected item, found keyword `let`
+}

tests/ui/parser/recover/pub-let-outside-fn.stderr

@@ -0,0 +1,21 @@
+error: visibility `pub` is not followed by an item
+  --> $DIR/pub-let-outside-fn.rs:2:5
+   |
+LL |     pub let answer = 42;
+   |     ^^^ the visibility
+   |
+   = help: you likely meant to define an item, e.g., `pub fn foo() {}`
+
+error: expected item, found keyword `let`
+  --> $DIR/pub-let-outside-fn.rs:2:9
+   |
+LL |     pub let answer = 42;
+   |         ^^^-------------
+   |         |
+   |         unexpected `let` binding outside of a function
+   |
+   = help: consider using `const` or `static` instead of `let` for global variables, or put it inside of a function:  fn foo() { let answer = 42; }
+   = note: for a full list of items that can appear in modules, see <https://doc.rust-lang.org/reference/items.html>
+
+error: aborting due to 2 previous errors
+

tests/ui/parser/shebang/shebang-doc-comment.stderr

@@ -2,8 +2,11 @@ error: expected item, found `[`
   --> $DIR/shebang-doc-comment.rs:2:1
    |
 LL | [allow(unused_variables)]
-   | ^ expected item
+   | ^------------------------
+   | |
+   | unexpected expression
    |
+   = help: consider putting it inside a function: fn foo() { [allow(unused_variables)] }
    = note: for a full list of items that can appear in modules, see <https://doc.rust-lang.org/reference/items.html>
 
 error: aborting due to 1 previous error

tests/ui/parser/suggest-const-for-global-var.stderr

@@ -2,7 +2,12 @@ error: expected item, found keyword `let`
   --> $DIR/suggest-const-for-global-var.rs:1:1
    |
 LL | let X: i32 = 12;
-   | ^^^ consider using `const` or `static` instead of `let` for global variables
+   | ^^^-------------
+   | |
+   | unexpected `let` binding outside of a function
+   |
+   = help: consider using `const` or `static` instead of `let` for global variables, or put it inside of a function:  fn foo() { let X: i32 = 12; }
+   = note: for a full list of items that can appear in modules, see <https://doc.rust-lang.org/reference/items.html>
 
 error: aborting due to 1 previous error