Rollup merge of #127350 - veera-sivarajan:bugfix-126311, r=lcnr

Parser: Suggest Placing the Return Type After Function Parameters

Fixes #126311

This PR suggests placing the return type after the function parameters when it's misplaced after a `where` clause.

This also tangentially improves diagnostics for cases like [this](https://github.com/veera-sivarajan/rust/blob/86d6f1312a77997ef994240e716288d61a343a6d/tests/ui/parser/issues/misplaced-return-type-without-where-issue-126311.rs#L1C1-L1C28) and adds doc comments for `parser::AllowPlus`.

Author: matthiaskrgr

compiler/rustc_parse/messages.ftl

@@ -524,6 +524,8 @@ parse_mismatched_closing_delimiter = mismatched closing delimiter: `{$delimiter}
     .label_opening_candidate = closing delimiter possibly meant for this
     .label_unclosed = unclosed delimiter
 
+parse_misplaced_return_type = place the return type after the function parameters
+
 parse_missing_comma_after_match_arm = expected `,` following `match` arm
     .suggestion = missing a comma here to end this `match` arm
 

compiler/rustc_parse/src/errors.rs

@@ -1502,6 +1502,20 @@ pub(crate) struct FnPtrWithGenerics {
     pub sugg: Option<FnPtrWithGenericsSugg>,
 }
 
+#[derive(Subdiagnostic)]
+#[multipart_suggestion(
+    parse_misplaced_return_type,
+    style = "verbose",
+    applicability = "maybe-incorrect"
+)]
+pub(crate) struct MisplacedReturnType {
+    #[suggestion_part(code = " {snippet}")]
+    pub fn_params_end: Span,
+    pub snippet: String,
+    #[suggestion_part(code = "")]
+    pub ret_ty_span: Span,
+}
+
 #[derive(Subdiagnostic)]
 #[multipart_suggestion(parse_suggestion, applicability = "maybe-incorrect")]
 pub(crate) struct FnPtrWithGenericsSugg {
@@ -1516,7 +1530,6 @@ pub(crate) struct FnPtrWithGenericsSugg {
 
 pub(crate) struct FnTraitMissingParen {
     pub span: Span,
-    pub machine_applicable: bool,
 }
 
 impl Subdiagnostic for FnTraitMissingParen {
@@ -1526,16 +1539,11 @@ impl Subdiagnostic for FnTraitMissingParen {
         _: &F,
     ) {
         diag.span_label(self.span, crate::fluent_generated::parse_fn_trait_missing_paren);
-        let applicability = if self.machine_applicable {
-            Applicability::MachineApplicable
-        } else {
-            Applicability::MaybeIncorrect
-        };
         diag.span_suggestion_short(
             self.span.shrink_to_hi(),
             crate::fluent_generated::parse_add_paren,
             "()",
-            applicability,
+            Applicability::MachineApplicable,
         );
     }
 }

compiler/rustc_parse/src/parser/diagnostics.rs

@@ -430,7 +430,7 @@ impl<'a> Parser<'a> {
         &mut self,
         edible: &[TokenKind],
         inedible: &[TokenKind],
-    ) -> PResult<'a, Recovered> {
+    ) -> PResult<'a, ErrorGuaranteed> {
         debug!("expected_one_of_not_found(edible: {:?}, inedible: {:?})", edible, inedible);
         fn tokens_to_string(tokens: &[TokenType]) -> String {
             let mut i = tokens.iter();
@@ -533,7 +533,7 @@ impl<'a> Parser<'a> {
                     sugg: ExpectedSemiSugg::ChangeToSemi(self.token.span),
                 });
                 self.bump();
-                return Ok(Recovered::Yes(guar));
+                return Ok(guar);
             } else if self.look_ahead(0, |t| {
                 t == &token::CloseDelim(Delimiter::Brace)
                     || ((t.can_begin_expr() || t.can_begin_item())
@@ -557,7 +557,7 @@ impl<'a> Parser<'a> {
                     unexpected_token_label: Some(self.token.span),
                     sugg: ExpectedSemiSugg::AddSemi(span),
                 });
-                return Ok(Recovered::Yes(guar));
+                return Ok(guar);
             }
         }
 
@@ -712,7 +712,7 @@ impl<'a> Parser<'a> {
         if self.check_too_many_raw_str_terminators(&mut err) {
             if expected.contains(&TokenType::Token(token::Semi)) && self.eat(&token::Semi) {
                 let guar = err.emit();
-                return Ok(Recovered::Yes(guar));
+                return Ok(guar);
             } else {
                 return Err(err);
             }

compiler/rustc_parse/src/parser/item.rs

@@ -17,6 +17,7 @@ use rustc_span::edit_distance::edit_distance;
 use rustc_span::edition::Edition;
 use rustc_span::source_map;
 use rustc_span::symbol::{kw, sym, Ident, Symbol};
+use rustc_span::ErrorGuaranteed;
 use rustc_span::{Span, DUMMY_SP};
 use std::fmt::Write;
 use std::mem;
@@ -2332,14 +2333,106 @@ impl<'a> Parser<'a> {
                 }
             }
         };
+
+        // Store the end of function parameters to give better diagnostics
+        // inside `parse_fn_body()`.
+        let fn_params_end = self.prev_token.span.shrink_to_hi();
+
         generics.where_clause = self.parse_where_clause()?; // `where T: Ord`
 
+        // `fn_params_end` is needed only when it's followed by a where clause.
+        let fn_params_end =
+            if generics.where_clause.has_where_token { Some(fn_params_end) } else { None };
+
         let mut sig_hi = self.prev_token.span;
-        let body = self.parse_fn_body(attrs, &ident, &mut sig_hi, fn_parse_mode.req_body)?; // `;` or `{ ... }`.
+        // Either `;` or `{ ... }`.
+        let body =
+            self.parse_fn_body(attrs, &ident, &mut sig_hi, fn_parse_mode.req_body, fn_params_end)?;
         let fn_sig_span = sig_lo.to(sig_hi);
         Ok((ident, FnSig { header, decl, span: fn_sig_span }, generics, body))
     }
 
+    /// Provide diagnostics when function body is not found
+    fn error_fn_body_not_found(
+        &mut self,
+        ident_span: Span,
+        req_body: bool,
+        fn_params_end: Option<Span>,
+    ) -> PResult<'a, ErrorGuaranteed> {
+        let expected = if req_body {
+            &[token::OpenDelim(Delimiter::Brace)][..]
+        } else {
+            &[token::Semi, token::OpenDelim(Delimiter::Brace)]
+        };
+        match self.expected_one_of_not_found(&[], expected) {
+            Ok(error_guaranteed) => Ok(error_guaranteed),
+            Err(mut err) => {
+                if self.token.kind == token::CloseDelim(Delimiter::Brace) {
+                    // The enclosing `mod`, `trait` or `impl` is being closed, so keep the `fn` in
+                    // the AST for typechecking.
+                    err.span_label(ident_span, "while parsing this `fn`");
+                    Ok(err.emit())
+                } else if self.token.kind == token::RArrow
+                    && let Some(fn_params_end) = fn_params_end
+                {
+                    // Instead of a function body, the parser has encountered a right arrow
+                    // preceded by a where clause.
+
+                    // Find whether token behind the right arrow is a function trait and
+                    // store its span.
+                    let fn_trait_span =
+                        [sym::FnOnce, sym::FnMut, sym::Fn].into_iter().find_map(|symbol| {
+                            if self.prev_token.is_ident_named(symbol) {
+                                Some(self.prev_token.span)
+                            } else {
+                                None
+                            }
+                        });
+
+                    // Parse the return type (along with the right arrow) and store its span.
+                    // If there's a parse error, cancel it and return the existing error
+                    // as we are primarily concerned with the
+                    // expected-function-body-but-found-something-else error here.
+                    let arrow_span = self.token.span;
+                    let ty_span = match self.parse_ret_ty(
+                        AllowPlus::Yes,
+                        RecoverQPath::Yes,
+                        RecoverReturnSign::Yes,
+                    ) {
+                        Ok(ty_span) => ty_span.span().shrink_to_hi(),
+                        Err(parse_error) => {
+                            parse_error.cancel();
+                            return Err(err);
+                        }
+                    };
+                    let ret_ty_span = arrow_span.to(ty_span);
+
+                    if let Some(fn_trait_span) = fn_trait_span {
+                        // Typo'd Fn* trait bounds such as
+                        // fn foo<F>() where F: FnOnce -> () {}
+                        err.subdiagnostic(errors::FnTraitMissingParen { span: fn_trait_span });
+                    } else if let Ok(snippet) = self.psess.source_map().span_to_snippet(ret_ty_span)
+                    {
+                        // If token behind right arrow is not a Fn* trait, the programmer
+                        // probably misplaced the return type after the where clause like
+                        // `fn foo<T>() where T: Default -> u8 {}`
+                        err.primary_message(
+                            "return type should be specified after the function parameters",
+                        );
+                        err.subdiagnostic(errors::MisplacedReturnType {
+                            fn_params_end,
+                            snippet,
+                            ret_ty_span,
+                        });
+                    }
+                    Err(err)
+                } else {
+                    Err(err)
+                }
+            }
+        }
+    }
+
     /// Parse the "body" of a function.
     /// This can either be `;` when there's no body,
     /// or e.g. a block when the function is a provided one.
@@ -2349,6 +2442,7 @@ impl<'a> Parser<'a> {
         ident: &Ident,
         sig_hi: &mut Span,
         req_body: bool,
+        fn_params_end: Option<Span>,
     ) -> PResult<'a, Option<P<Block>>> {
         let has_semi = if req_body {
             self.token.kind == TokenKind::Semi
@@ -2377,33 +2471,7 @@ impl<'a> Parser<'a> {
             });
             (AttrVec::new(), Some(self.mk_block_err(span, guar)))
         } else {
-            let expected = if req_body {
-                &[token::OpenDelim(Delimiter::Brace)][..]
-            } else {
-                &[token::Semi, token::OpenDelim(Delimiter::Brace)]
-            };
-            if let Err(mut err) = self.expected_one_of_not_found(&[], expected) {
-                if self.token.kind == token::CloseDelim(Delimiter::Brace) {
-                    // The enclosing `mod`, `trait` or `impl` is being closed, so keep the `fn` in
-                    // the AST for typechecking.
-                    err.span_label(ident.span, "while parsing this `fn`");
-                    err.emit();
-                } else {
-                    // check for typo'd Fn* trait bounds such as
-                    // fn foo<F>() where F: FnOnce -> () {}
-                    if self.token.kind == token::RArrow {
-                        let machine_applicable = [sym::FnOnce, sym::FnMut, sym::Fn]
-                            .into_iter()
-                            .any(|s| self.prev_token.is_ident_named(s));
-
-                        err.subdiagnostic(errors::FnTraitMissingParen {
-                            span: self.prev_token.span,
-                            machine_applicable,
-                        });
-                    }
-                    return Err(err);
-                }
-            }
+            self.error_fn_body_not_found(ident.span, req_body, fn_params_end)?;
             (AttrVec::new(), None)
         };
         attrs.extend(inner_attrs);

compiler/rustc_parse/src/parser/mod.rs

@@ -495,6 +495,7 @@ impl<'a> Parser<'a> {
             FatalError.raise();
         } else {
             self.expected_one_of_not_found(edible, inedible)
+                .map(|error_guaranteed| Recovered::Yes(error_guaranteed))
         }
     }
 

compiler/rustc_parse/src/parser/ty.rs

@@ -21,6 +21,11 @@ use rustc_span::symbol::{kw, sym, Ident};
 use rustc_span::{ErrorGuaranteed, Span, Symbol};
 use thin_vec::{thin_vec, ThinVec};
 
+/// Signals whether parsing a type should allow `+`.
+///
+/// For example, let T be the type `impl Default + 'static`
+/// With `AllowPlus::Yes`, T will be parsed successfully
+/// With `AllowPlus::No`, parsing T will return a parse error
 #[derive(Copy, Clone, PartialEq)]
 pub(super) enum AllowPlus {
     Yes,

tests/ui/parser/issues/misplaced-return-type-complex-type-issue-126311.rs

@@ -0,0 +1,5 @@
+fn foo<T>() where T: Default -> impl Default + 'static {}
+//~^ ERROR return type should be specified after the function parameters
+//~| HELP place the return type after the function parameters
+
+fn main() {}

tests/ui/parser/issues/misplaced-return-type-complex-type-issue-126311.stderr

@@ -0,0 +1,14 @@
+error: return type should be specified after the function parameters
+  --> $DIR/misplaced-return-type-complex-type-issue-126311.rs:1:30
+   |
+LL | fn foo<T>() where T: Default -> impl Default + 'static {}
+   |                              ^^ expected one of `(`, `+`, `,`, `::`, `<`, or `{`
+   |
+help: place the return type after the function parameters
+   |
+LL - fn foo<T>() where T: Default -> impl Default + 'static {}
+LL + fn foo<T>() -> impl Default + 'static where T: Default  {}
+   |
+
+error: aborting due to 1 previous error
+

tests/ui/parser/issues/misplaced-return-type-issue-126311.rs

@@ -0,0 +1,5 @@
+fn foo<T>() where T: Default -> u8 {}
+//~^ ERROR return type should be specified after the function parameters
+//~| HELP place the return type after the function parameters
+
+fn main() {}

tests/ui/parser/issues/misplaced-return-type-issue-126311.stderr

@@ -0,0 +1,14 @@
+error: return type should be specified after the function parameters
+  --> $DIR/misplaced-return-type-issue-126311.rs:1:30
+   |
+LL | fn foo<T>() where T: Default -> u8 {}
+   |                              ^^ expected one of `(`, `+`, `,`, `::`, `<`, or `{`
+   |
+help: place the return type after the function parameters
+   |
+LL - fn foo<T>() where T: Default -> u8 {}
+LL + fn foo<T>() -> u8 where T: Default  {}
+   |
+
+error: aborting due to 1 previous error
+

tests/ui/parser/issues/misplaced-return-type-where-in-next-line-issue-126311.rs

@@ -0,0 +1,11 @@
+fn foo<T, K>()
+//~^ HELP place the return type after the function parameters
+where
+    T: Default,
+    K: Clone, -> Result<u8, String>
+//~^ ERROR return type should be specified after the function parameters
+{
+    Ok(0)
+}
+
+fn main() {}

tests/ui/parser/issues/misplaced-return-type-where-in-next-line-issue-126311.stderr

@@ -0,0 +1,17 @@
+error: return type should be specified after the function parameters
+  --> $DIR/misplaced-return-type-where-in-next-line-issue-126311.rs:5:15
+   |
+LL |     K: Clone, -> Result<u8, String>
+   |               ^^ expected one of `{`, lifetime, or type
+   |
+help: place the return type after the function parameters
+   |
+LL ~ fn foo<T, K>() -> Result<u8, String>
+LL |
+LL | where
+LL |     T: Default,
+LL ~     K: Clone, 
+   |
+
+error: aborting due to 1 previous error
+

tests/ui/parser/issues/misplaced-return-type-without-type-issue-126311.rs

@@ -0,0 +1,6 @@
+fn foo<T>() where T: Default -> {
+//~^ ERROR expected one of `(`, `+`, `,`, `::`, `<`, or `{`, found `->`
+    0
+}
+
+fn main() {}

tests/ui/parser/issues/misplaced-return-type-without-type-issue-126311.stderr

@@ -0,0 +1,8 @@
+error: expected one of `(`, `+`, `,`, `::`, `<`, or `{`, found `->`
+  --> $DIR/misplaced-return-type-without-type-issue-126311.rs:1:30
+   |
+LL | fn foo<T>() where T: Default -> {
+   |                              ^^ expected one of `(`, `+`, `,`, `::`, `<`, or `{`
+
+error: aborting due to 1 previous error
+

tests/ui/parser/issues/misplaced-return-type-without-where-issue-126311.rs

@@ -0,0 +1,4 @@
+fn bar<T>() -> u8 -> u64 {}
+//~^ ERROR expected one of `!`, `(`, `+`, `::`, `<`, `where`, or `{`, found `->`
+
+fn main() {}

tests/ui/parser/issues/misplaced-return-type-without-where-issue-126311.stderr

@@ -0,0 +1,8 @@
+error: expected one of `!`, `(`, `+`, `::`, `<`, `where`, or `{`, found `->`
+  --> $DIR/misplaced-return-type-without-where-issue-126311.rs:1:19
+   |
+LL | fn bar<T>() -> u8 -> u64 {}
+   |                   ^^ expected one of 7 possible tokens
+
+error: aborting due to 1 previous error
+