Rollup merge of #91531 - notriddle:notriddle/issue-87647-expected-semicolon, r=estebank

Do not add `;` to expected tokens list when it's wrong

There's a few spots where semicolons are checked for to do error recovery, and should not be suggested (or checked for other stuff).

Fixes #87647

compiler/rustc_parse/src/parser/attr.rs

@@ -1,4 +1,4 @@
-use super::{AttrWrapper, Capturing, ForceCollect, Parser, PathStyle};
+use super::{AttrWrapper, Capturing, FnParseMode, ForceCollect, Parser, PathStyle};
 use rustc_ast as ast;
 use rustc_ast::attr;
 use rustc_ast::token::{self, Nonterminal};
@@ -177,7 +177,7 @@ impl<'a> Parser<'a> {
             AttrWrapper::empty(),
             true,
             false,
-            |_| true,
+            FnParseMode { req_name: |_| true, req_body: true },
             ForceCollect::No,
         ) {
             Ok(Some(item)) => {

compiler/rustc_parse/src/parser/diagnostics.rs

@@ -1129,7 +1129,8 @@ impl<'a> Parser<'a> {
     }
 
     pub fn maybe_consume_incorrect_semicolon(&mut self, items: &[P<Item>]) -> bool {
-        if self.eat(&token::Semi) {
+        if self.token.kind == TokenKind::Semi {
+            self.bump();
             let mut err = self.struct_span_err(self.prev_token.span, "expected item, found `;`");
             err.span_suggestion_short(
                 self.prev_token.span,

compiler/rustc_parse/src/parser/item.rs

@@ -78,16 +78,17 @@ pub(super) type ItemInfo = (Ident, ItemKind);
 
 impl<'a> Parser<'a> {
     pub fn parse_item(&mut self, force_collect: ForceCollect) -> PResult<'a, Option<P<Item>>> {
-        self.parse_item_(|_| true, force_collect).map(|i| i.map(P))
+        let fn_parse_mode = FnParseMode { req_name: |_| true, req_body: true };
+        self.parse_item_(fn_parse_mode, force_collect).map(|i| i.map(P))
     }
 
     fn parse_item_(
         &mut self,
-        req_name: ReqName,
+        fn_parse_mode: FnParseMode,
         force_collect: ForceCollect,
     ) -> PResult<'a, Option<Item>> {
         let attrs = self.parse_outer_attributes()?;
-        self.parse_item_common(attrs, true, false, req_name, force_collect)
+        self.parse_item_common(attrs, true, false, fn_parse_mode, force_collect)
     }
 
     pub(super) fn parse_item_common(
@@ -95,7 +96,7 @@ impl<'a> Parser<'a> {
         attrs: AttrWrapper,
         mac_allowed: bool,
         attrs_allowed: bool,
-        req_name: ReqName,
+        fn_parse_mode: FnParseMode,
         force_collect: ForceCollect,
     ) -> PResult<'a, Option<Item>> {
         // Don't use `maybe_whole` so that we have precise control
@@ -113,7 +114,8 @@ impl<'a> Parser<'a> {
         let mut unclosed_delims = vec![];
         let item =
             self.collect_tokens_trailing_token(attrs, force_collect, |this: &mut Self, attrs| {
-                let item = this.parse_item_common_(attrs, mac_allowed, attrs_allowed, req_name);
+                let item =
+                    this.parse_item_common_(attrs, mac_allowed, attrs_allowed, fn_parse_mode);
                 unclosed_delims.append(&mut this.unclosed_delims);
                 Ok((item?, TrailingToken::None))
             })?;
@@ -127,12 +129,13 @@ impl<'a> Parser<'a> {
         mut attrs: Vec<Attribute>,
         mac_allowed: bool,
         attrs_allowed: bool,
-        req_name: ReqName,
+        fn_parse_mode: FnParseMode,
     ) -> PResult<'a, Option<Item>> {
         let lo = self.token.span;
         let vis = self.parse_visibility(FollowedByType::No)?;
         let mut def = self.parse_defaultness();
-        let kind = self.parse_item_kind(&mut attrs, mac_allowed, lo, &vis, &mut def, req_name)?;
+        let kind =
+            self.parse_item_kind(&mut attrs, mac_allowed, lo, &vis, &mut def, fn_parse_mode)?;
         if let Some((ident, kind)) = kind {
             self.error_on_unconsumed_default(def, &kind);
             let span = lo.to(self.prev_token.span);
@@ -192,7 +195,7 @@ impl<'a> Parser<'a> {
         lo: Span,
         vis: &Visibility,
         def: &mut Defaultness,
-        req_name: ReqName,
+        fn_parse_mode: FnParseMode,
     ) -> PResult<'a, Option<ItemInfo>> {
         let def_final = def == &Defaultness::Final;
         let mut def = || mem::replace(def, Defaultness::Final);
@@ -219,7 +222,7 @@ impl<'a> Parser<'a> {
             (Ident::empty(), ItemKind::Use(tree))
         } else if self.check_fn_front_matter(def_final) {
             // FUNCTION ITEM
-            let (ident, sig, generics, body) = self.parse_fn(attrs, req_name, lo)?;
+            let (ident, sig, generics, body) = self.parse_fn(attrs, fn_parse_mode, lo)?;
             (ident, ItemKind::Fn(Box::new(Fn { defaultness: def(), sig, generics, body })))
         } else if self.eat_keyword(kw::Extern) {
             if self.eat_keyword(kw::Crate) {
@@ -733,23 +736,26 @@ impl<'a> Parser<'a> {
         &mut self,
         force_collect: ForceCollect,
     ) -> PResult<'a, Option<Option<P<AssocItem>>>> {
-        self.parse_assoc_item(|_| true, force_collect)
+        let fn_parse_mode = FnParseMode { req_name: |_| true, req_body: true };
+        self.parse_assoc_item(fn_parse_mode, force_collect)
     }
 
     pub fn parse_trait_item(
         &mut self,
         force_collect: ForceCollect,
     ) -> PResult<'a, Option<Option<P<AssocItem>>>> {
-        self.parse_assoc_item(|edition| edition >= Edition::Edition2018, force_collect)
+        let fn_parse_mode =
+            FnParseMode { req_name: |edition| edition >= Edition::Edition2018, req_body: false };
+        self.parse_assoc_item(fn_parse_mode, force_collect)
     }
 
     /// Parses associated items.
     fn parse_assoc_item(
         &mut self,
-        req_name: ReqName,
+        fn_parse_mode: FnParseMode,
         force_collect: ForceCollect,
     ) -> PResult<'a, Option<Option<P<AssocItem>>>> {
-        Ok(self.parse_item_(req_name, force_collect)?.map(
+        Ok(self.parse_item_(fn_parse_mode, force_collect)?.map(
             |Item { attrs, id, span, vis, ident, kind, tokens }| {
                 let kind = match AssocItemKind::try_from(kind) {
                     Ok(kind) => kind,
@@ -944,7 +950,8 @@ impl<'a> Parser<'a> {
         &mut self,
         force_collect: ForceCollect,
     ) -> PResult<'a, Option<Option<P<ForeignItem>>>> {
-        Ok(self.parse_item_(|_| true, force_collect)?.map(
+        let fn_parse_mode = FnParseMode { req_name: |_| true, req_body: false };
+        Ok(self.parse_item_(fn_parse_mode, force_collect)?.map(
             |Item { attrs, id, span, vis, ident, kind, tokens }| {
                 let kind = match ForeignItemKind::try_from(kind) {
                     Ok(kind) => kind,
@@ -1484,7 +1491,8 @@ impl<'a> Parser<'a> {
         if !is_raw && ident.is_reserved() {
             let err = if self.check_fn_front_matter(false) {
                 // We use `parse_fn` to get a span for the function
-                if let Err(mut db) = self.parse_fn(&mut Vec::new(), |_| true, lo) {
+                let fn_parse_mode = FnParseMode { req_name: |_| true, req_body: true };
+                if let Err(mut db) = self.parse_fn(&mut Vec::new(), fn_parse_mode, lo) {
                     db.delay_as_bug();
                 }
                 let mut err = self.struct_span_err(
@@ -1698,25 +1706,82 @@ impl<'a> Parser<'a> {
 /// The parsing configuration used to parse a parameter list (see `parse_fn_params`).
 ///
 /// The function decides if, per-parameter `p`, `p` must have a pattern or just a type.
+///
+/// This function pointer accepts an edition, because in edition 2015, trait declarations
+/// were allowed to omit parameter names. In 2018, they became required.
 type ReqName = fn(Edition) -> bool;
 
+/// Parsing configuration for functions.
+///
+/// The syntax of function items is slightly different within trait definitions,
+/// impl blocks, and modules. It is still parsed using the same code, just with
+/// different flags set, so that even when the input is wrong and produces a parse
+/// error, it still gets into the AST and the rest of the parser and
+/// type checker can run.
+#[derive(Clone, Copy)]
+pub(crate) struct FnParseMode {
+    /// A function pointer that decides if, per-parameter `p`, `p` must have a
+    /// pattern or just a type. This field affects parsing of the parameters list.
+    ///
+    /// ```text
+    /// fn foo(alef: A) -> X { X::new() }
+    ///        -----^^ affects parsing this part of the function signature
+    ///        |
+    ///        if req_name returns false, then this name is optional
+    ///
+    /// fn bar(A) -> X;
+    ///        ^
+    ///        |
+    ///        if req_name returns true, this is an error
+    /// ```
+    ///
+    /// Calling this function pointer should only return false if:
+    ///
+    ///   * The item is being parsed inside of a trait definition.
+    ///     Within an impl block or a module, it should always evaluate
+    ///     to true.
+    ///   * The span is from Edition 2015. In particular, you can get a
+    ///     2015 span inside a 2021 crate using macros.
+    pub req_name: ReqName,
+    /// If this flag is set to `true`, then plain, semicolon-terminated function
+    /// prototypes are not allowed here.
+    ///
+    /// ```text
+    /// fn foo(alef: A) -> X { X::new() }
+    ///                      ^^^^^^^^^^^^
+    ///                      |
+    ///                      this is always allowed
+    ///
+    /// fn bar(alef: A, bet: B) -> X;
+    ///                             ^
+    ///                             |
+    ///                             if req_body is set to true, this is an error
+    /// ```
+    ///
+    /// This field should only be set to false if the item is inside of a trait
+    /// definition or extern block. Within an impl block or a module, it should
+    /// always be set to true.
+    pub req_body: bool,
+}
+
 /// Parsing of functions and methods.
 impl<'a> Parser<'a> {
     /// Parse a function starting from the front matter (`const ...`) to the body `{ ... }` or `;`.
     fn parse_fn(
         &mut self,
         attrs: &mut Vec<Attribute>,
-        req_name: ReqName,
+        fn_parse_mode: FnParseMode,
         sig_lo: Span,
     ) -> PResult<'a, (Ident, FnSig, Generics, Option<P<Block>>)> {
         let header = self.parse_fn_front_matter()?; // `const ... fn`
         let ident = self.parse_ident()?; // `foo`
         let mut generics = self.parse_generics()?; // `<'a, T, ...>`
-        let decl = self.parse_fn_decl(req_name, AllowPlus::Yes, RecoverReturnSign::Yes)?; // `(p: u8, ...)`
+        let decl =
+            self.parse_fn_decl(fn_parse_mode.req_name, AllowPlus::Yes, RecoverReturnSign::Yes)?; // `(p: u8, ...)`
         generics.where_clause = self.parse_where_clause()?; // `where T: Ord`
 
         let mut sig_hi = self.prev_token.span;
-        let body = self.parse_fn_body(attrs, &ident, &mut sig_hi)?; // `;` or `{ ... }`.
+        let body = self.parse_fn_body(attrs, &ident, &mut sig_hi, fn_parse_mode.req_body)?; // `;` or `{ ... }`.
         let fn_sig_span = sig_lo.to(sig_hi);
         Ok((ident, FnSig { header, decl, span: fn_sig_span }, generics, body))
     }
@@ -1729,9 +1794,17 @@ impl<'a> Parser<'a> {
         attrs: &mut Vec<Attribute>,
         ident: &Ident,
         sig_hi: &mut Span,
+        req_body: bool,
     ) -> PResult<'a, Option<P<Block>>> {
-        let (inner_attrs, body) = if self.eat(&token::Semi) {
+        let has_semi = if req_body {
+            self.token.kind == TokenKind::Semi
+        } else {
+            // Only include `;` in list of expected tokens if body is not required
+            self.check(&TokenKind::Semi)
+        };
+        let (inner_attrs, body) = if has_semi {
             // Include the trailing semicolon in the span of the signature
+            self.expect_semi()?;
             *sig_hi = self.prev_token.span;
             (Vec::new(), None)
         } else if self.check(&token::OpenDelim(token::Brace)) || self.token.is_whole_block() {
@@ -1752,9 +1825,12 @@ impl<'a> Parser<'a> {
                 .emit();
             (Vec::new(), Some(self.mk_block_err(span)))
         } else {
-            if let Err(mut err) =
+            let expected = if req_body {
-                self.expected_one_of_not_found(&[], &[token::Semi, token::OpenDelim(token::Brace)])
+                &[token::OpenDelim(token::Brace)][..]
-            {
+            } else {
+                &[token::Semi, token::OpenDelim(token::Brace)]
+            };
+            if let Err(mut err) = self.expected_one_of_not_found(&[], &expected) {
                 if self.token.kind == token::CloseDelim(token::Brace) {
                     // The enclosing `mod`, `trait` or `impl` is being closed, so keep the `fn` in
                     // the AST for typechecking.

compiler/rustc_parse/src/parser/mod.rs

@@ -14,6 +14,7 @@ use crate::lexer::UnmatchedBrace;
 pub use attr_wrapper::AttrWrapper;
 pub use diagnostics::AttemptLocalParseRecovery;
 use diagnostics::Error;
+pub(crate) use item::FnParseMode;
 pub use pat::{RecoverColon, RecoverComma};
 pub use path::PathStyle;
 

compiler/rustc_parse/src/parser/stmt.rs

@@ -4,7 +4,9 @@ use super::expr::LhsExpr;
 use super::pat::RecoverComma;
 use super::path::PathStyle;
 use super::TrailingToken;
-use super::{AttrWrapper, BlockMode, ForceCollect, Parser, Restrictions, SemiColonMode};
+use super::{
+    AttrWrapper, BlockMode, FnParseMode, ForceCollect, Parser, Restrictions, SemiColonMode,
+};
 use crate::maybe_whole;
 
 use rustc_ast as ast;
@@ -79,9 +81,13 @@ impl<'a> Parser<'a> {
             } else {
                 self.parse_stmt_path_start(lo, attrs)
             }?
-        } else if let Some(item) =
+        } else if let Some(item) = self.parse_item_common(
-            self.parse_item_common(attrs.clone(), false, true, |_| true, force_collect)?
+            attrs.clone(),
-        {
+            false,
+            true,
+            FnParseMode { req_name: |_| true, req_body: true },
+            force_collect,
+        )? {
             // FIXME: Bad copy of attrs
             self.mk_stmt(lo.to(item.span), StmtKind::Item(P(item)))
         } else if self.eat(&token::Semi) {

src/test/ui/fn/fn-recover-return-sign2.rs

@@ -3,6 +3,6 @@
 
 fn foo() => impl Fn() => bool {
     //~^ ERROR return types are denoted using `->`
-    //~| ERROR expected one of `+`, `->`, `::`, `;`, `where`, or `{`, found `=>`
+    //~| ERROR expected one of `+`, `->`, `::`, `where`, or `{`, found `=>`
     unimplemented!()
 }

src/test/ui/fn/fn-recover-return-sign2.stderr

@@ -4,11 +4,11 @@ error: return types are denoted using `->`
 LL | fn foo() => impl Fn() => bool {
    |          ^^ help: use `->` instead
 
-error: expected one of `+`, `->`, `::`, `;`, `where`, or `{`, found `=>`
+error: expected one of `+`, `->`, `::`, `where`, or `{`, found `=>`
   --> $DIR/fn-recover-return-sign2.rs:4:23
    |
 LL | fn foo() => impl Fn() => bool {
-   |                       ^^ expected one of `+`, `->`, `::`, `;`, `where`, or `{`
+   |                       ^^ expected one of `+`, `->`, `::`, `where`, or `{`
 
 error: aborting due to 2 previous errors
 

src/test/ui/parser/issues/issue-24780.rs

@@ -2,7 +2,7 @@
 // to happen in #24780. For example, following should be an error:
 // expected one of ..., `>`, ... found `>`.
 
-fn foo() -> Vec<usize>> { //~ ERROR expected one of `!`, `+`, `::`, `;`, `where`, or `{`, found `>`
+fn foo() -> Vec<usize>> { //~ ERROR expected one of `!`, `+`, `::`, `where`, or `{`, found `>`
     Vec::new()
 }
 

src/test/ui/parser/issues/issue-24780.stderr

@@ -1,8 +1,8 @@
-error: expected one of `!`, `+`, `::`, `;`, `where`, or `{`, found `>`
+error: expected one of `!`, `+`, `::`, `where`, or `{`, found `>`
   --> $DIR/issue-24780.rs:5:23
    |
 LL | fn foo() -> Vec<usize>> {
-   |                       ^ expected one of `!`, `+`, `::`, `;`, `where`, or `{`
+   |                       ^ expected one of `!`, `+`, `::`, `where`, or `{`
 
 error: aborting due to previous error
 

src/test/ui/parser/issues/issue-58856-1.rs

@@ -2,7 +2,7 @@ impl A {
     //~^ ERROR cannot find type `A` in this scope
     fn b(self>
     //~^ ERROR expected one of `)`, `,`, or `:`, found `>`
-    //~| ERROR expected one of `->`, `;`, `where`, or `{`, found `>`
+    //~| ERROR expected one of `->`, `where`, or `{`, found `>`
 }
 
 fn main() {}

src/test/ui/parser/issues/issue-58856-1.stderr

@@ -6,14 +6,14 @@ LL |     fn b(self>
    |         |
    |         unclosed delimiter
 
-error: expected one of `->`, `;`, `where`, or `{`, found `>`
+error: expected one of `->`, `where`, or `{`, found `>`
   --> $DIR/issue-58856-1.rs:3:14
    |
 LL | impl A {
    |        - while parsing this item list starting here
 LL |
 LL |     fn b(self>
-   |              ^ expected one of `->`, `;`, `where`, or `{`
+   |              ^ expected one of `->`, `where`, or `{`
 ...
 LL | }
    | - the item list ends here

src/test/ui/parser/issues/issue-84148-1.stderr

@@ -13,11 +13,11 @@ LL | fn f(t:for<>t?)
    |              expected one of `(`, `)`, `+`, `,`, `::`, or `<`
    |              help: missing `,`
 
-error: expected one of `->`, `;`, `where`, or `{`, found `<eof>`
+error: expected one of `->`, `where`, or `{`, found `<eof>`
   --> $DIR/issue-84148-1.rs:1:15
    |
 LL | fn f(t:for<>t?)
-   |               ^ expected one of `->`, `;`, `where`, or `{`
+   |               ^ expected one of `->`, `where`, or `{`
 
 error: aborting due to 3 previous errors
 

src/test/ui/parser/issues/issue-84148-2.stderr

@@ -21,11 +21,11 @@ LL | fn f(t:for<>t?
    |              expected one of `(`, `)`, `+`, `,`, `::`, or `<`
    |              help: missing `,`
 
-error: expected one of `->`, `;`, `where`, or `{`, found `<eof>`
+error: expected one of `->`, `where`, or `{`, found `<eof>`
   --> $DIR/issue-84148-2.rs:4:16
    |
 LL | fn f(t:for<>t?
-   |                ^ expected one of `->`, `;`, `where`, or `{`
+   |                ^ expected one of `->`, `where`, or `{`
 
 error: aborting due to 4 previous errors
 

src/test/ui/parser/issues/issue-87635.rs

@@ -2,8 +2,8 @@ struct Foo {}
 
 impl Foo {
     pub fn bar()
-    //~^ ERROR: expected `;`, found `}`
+    //~^ ERROR: associated function in `impl` without body
-    //~| ERROR: associated function in `impl` without body
 }
+//~^ERROR expected one of `->`, `where`, or `{`, found `}`
 
 fn main() {}

src/test/ui/parser/issues/issue-87635.stderr

@@ -1,11 +1,13 @@
-error: expected `;`, found `}`
+error: expected one of `->`, `where`, or `{`, found `}`
-  --> $DIR/issue-87635.rs:4:17
+  --> $DIR/issue-87635.rs:6:1
    |
 LL |     pub fn bar()
-   |                 ^ help: add `;` here
+   |            ---  - expected one of `->`, `where`, or `{`
-...
+   |            |
+   |            while parsing this `fn`
+LL |
 LL | }
-   | - unexpected token
+   | ^ unexpected token
 
 error: associated function in `impl` without body
   --> $DIR/issue-87635.rs:4:5

src/test/ui/parser/missing_right_paren.stderr

@@ -22,11 +22,11 @@ error: expected one of `:` or `|`, found `)`
 LL | fn main((ؼ
    |           ^ expected one of `:` or `|`
 
-error: expected one of `->`, `;`, `where`, or `{`, found `<eof>`
+error: expected one of `->`, `where`, or `{`, found `<eof>`
   --> $DIR/missing_right_paren.rs:3:11
    |
 LL | fn main((ؼ
-   |           ^ expected one of `->`, `;`, `where`, or `{`
+   |           ^ expected one of `->`, `where`, or `{`
 
 error: aborting due to 4 previous errors
 

src/test/ui/suggestions/suggest-semicolon-for-fn-in-extern-block.fixed

@@ -0,0 +1,9 @@
+// run-rustfix
+
+#[allow(dead_code)]
+
+extern "C" {
+  fn foo(); //~ERROR expected `;`
+}
+
+fn main() {}

src/test/ui/suggestions/suggest-semicolon-for-fn-in-extern-block.rs

@@ -0,0 +1,9 @@
+// run-rustfix
+
+#[allow(dead_code)]
+
+extern "C" {
+  fn foo() //~ERROR expected `;`
+}
+
+fn main() {}

src/test/ui/suggestions/suggest-semicolon-for-fn-in-extern-block.stderr

@@ -0,0 +1,10 @@
+error: expected `;`, found `}`
+  --> $DIR/suggest-semicolon-for-fn-in-extern-block.rs:6:11
+   |
+LL |   fn foo()
+   |           ^ help: add `;` here
+LL | }
+   | - unexpected token
+
+error: aborting due to previous error
+