diag: improve handling of patterns in fn ptr and trait decl

Author: Ezrashaw

compiler/rustc_ast_passes/messages.ftl

@@ -222,8 +222,10 @@ ast_passes_out_of_order_params = {$param_ord} parameters must be declared prior
 
 ast_passes_pattern_in_bodiless = patterns aren't allowed in functions without bodies
     .label = pattern not allowed in function without body
+    .suggestion = give this argument a name or use an underscore to ignore it
 
 ast_passes_pattern_in_fn_pointer = patterns aren't allowed in function pointer types
+    .suggestion = give this argument a name or use an underscore to ignore it
 
 ast_passes_pattern_in_foreign = patterns aren't allowed in foreign function declarations
     .label = pattern not allowed in foreign function

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -788,8 +788,11 @@ impl<'a> AstValidator<'a> {
             TyKind::BareFn(bfty) => {
                 self.check_bare_fn_safety(bfty.decl_span, bfty.safety);
                 self.check_fn_decl(&bfty.decl, SelfSemantic::No);
-                Self::check_decl_no_pat(&bfty.decl, |span, _, _| {
-                    self.dcx().emit_err(errors::PatternFnPointer { span });
+                Self::check_decl_no_pat(&bfty.decl, |span, ident, _| {
+                    // `self` in bare fn ptr is already an error; don't add an extra one.
+                    if !ident.is_some_and(|ident| ident.name == kw::SelfLower) {
+                        self.dcx().emit_err(errors::PatternFnPointer { span });
+                    }
                 });
                 if let Extern::Implicit(extern_span) = bfty.ext {
                     self.handle_missing_abi(extern_span, ty.id);

compiler/rustc_ast_passes/src/errors.rs

@@ -404,6 +404,7 @@ impl Subdiagnostic for EmptyLabelManySpans {
 #[diag(ast_passes_pattern_in_fn_pointer, code = E0561)]
 pub(crate) struct PatternFnPointer {
     #[primary_span]
+    #[suggestion(code = "_", style = "verbose")]
     pub span: Span,
 }
 
@@ -685,6 +686,7 @@ pub(crate) struct PatternInForeign {
 pub(crate) struct PatternInBodiless {
     #[primary_span]
     #[label]
+    #[suggestion(code = "_", style = "verbose")]
     pub span: Span,
 }
 

compiler/rustc_parse/messages.ftl

@@ -753,8 +753,9 @@ parse_path_found_c_variadic_params = `Trait(...)` syntax does not support c_vari
 parse_path_found_named_params = `Trait(...)` syntax does not support named parameters
     .suggestion = remove the parameter name
 
-parse_pattern_method_param_without_body = patterns aren't allowed in methods without bodies
-    .suggestion = give this argument a name or use an underscore to ignore it
+parse_pattern_in_trait_fn_in_2015 = patterns aren't allowed in trait methods in the 2015 edition
+    .note = upgrading editions will fix this error
+    .suggestion = in this edition, give this argument a name or use an underscore to ignore it
 
 parse_pattern_on_wrong_side_of_at = pattern on wrong side of `@`
     .label_pattern = pattern on the left, should be on the right

compiler/rustc_parse/src/errors.rs

@@ -1490,10 +1490,11 @@ pub(crate) struct AttributeOnParamType {
 }
 
 #[derive(Diagnostic)]
-#[diag(parse_pattern_method_param_without_body, code = E0642)]
-pub(crate) struct PatternMethodParamWithoutBody {
+#[diag(parse_pattern_in_trait_fn_in_2015)]
+#[note]
+pub(crate) struct PatternInTraitFnIn2015 {
     #[primary_span]
-    #[suggestion(code = "_", applicability = "machine-applicable", style = "verbose")]
+    #[suggestion(code = "_", style = "verbose")]
     pub span: Span,
 }
 

compiler/rustc_parse/src/parser/diagnostics.rs

@@ -38,7 +38,7 @@ use crate::errors::{
     DocCommentOnParamType, DoubleColonInBound, ExpectedIdentifier, ExpectedSemi, ExpectedSemiSugg,
     GenericParamsWithoutAngleBrackets, GenericParamsWithoutAngleBracketsSugg,
     HelpIdentifierStartsWithNumber, HelpUseLatestEdition, InInTypo, IncorrectAwait,
-    IncorrectSemicolon, IncorrectUseOfAwait, IncorrectUseOfUse, PatternMethodParamWithoutBody,
+    IncorrectSemicolon, IncorrectUseOfAwait, IncorrectUseOfUse, PatternInTraitFnIn2015,
     QuestionMarkInType, QuestionMarkInTypeSugg, SelfParamNotFirst, StructLiteralBodyWithoutPath,
     StructLiteralBodyWithoutPathSugg, SuggAddMissingLetStmt, SuggEscapeIdentifier, SuggRemoveComma,
     TernaryOperator, TernaryOperatorSuggestion, UnexpectedConstInGenericParam,
@@ -2346,16 +2346,28 @@ impl<'a> Parser<'a> {
         None
     }
 
-    pub(super) fn recover_arg_parse(&mut self) -> PResult<'a, (P<ast::Pat>, P<ast::Ty>)> {
-        let pat = self.parse_pat_no_top_alt(Some(Expected::ArgumentName), None)?;
+    pub(super) fn recover_arg_parse(
+        &mut self,
+        emit_error: bool,
+    ) -> PResult<'a, (P<ast::Pat>, P<ast::Ty>)> {
+        let mut pat = self.parse_pat_no_top_alt(Some(Expected::ArgumentName), None)?;
         self.expect(exp!(Colon))?;
         let ty = self.parse_ty()?;
 
-        self.dcx().emit_err(PatternMethodParamWithoutBody { span: pat.span });
+        // Don't emit an error if we can emit a better one during AST passes (i.e., with patterns in
+        // bare fn ptrs). This branch is only missed if are guaranteed to emit an error later.
+        if emit_error {
+            self.dcx().emit_err(PatternInTraitFnIn2015 { span: pat.span });
+
+            // Pretend the pattern is `_`, to avoid duplicate errors from AST validation.
+            pat = P(Pat {
+                kind: PatKind::Wild,
+                span: pat.span,
+                id: ast::DUMMY_NODE_ID,
+                tokens: None,
+            });
+        }
 
-        // Pretend the pattern is `_`, to avoid duplicate errors from AST validation.
-        let pat =
-            P(Pat { kind: PatKind::Wild, span: pat.span, id: ast::DUMMY_NODE_ID, tokens: None });
         Ok((pat, ty))
     }
 

compiler/rustc_parse/src/parser/item.rs

@@ -2932,6 +2932,8 @@ impl<'a> Parser<'a> {
     /// Parses the parameter list of a function, including the `(` and `)` delimiters.
     pub(super) fn parse_fn_params(&mut self, req_name: ReqName) -> PResult<'a, ThinVec<Param>> {
         let mut first_param = true;
+        let is_bare_fn_ptr = self.prev_token.is_keyword(kw::Fn);
+
         // Parse the arguments, starting out with `self` being allowed...
         if self.token != TokenKind::OpenParen
         // might be typo'd trait impl, handled elsewhere
@@ -2946,22 +2948,23 @@ impl<'a> Parser<'a> {
         let (mut params, _) = self.parse_paren_comma_seq(|p| {
             p.recover_vcs_conflict_marker();
             let snapshot = p.create_snapshot_for_diagnostic();
-            let param = p.parse_param_general(req_name, first_param, true).or_else(|e| {
-                let guar = e.emit();
-                // When parsing a param failed, we should check to make the span of the param
-                // not contain '(' before it.
-                // For example when parsing `*mut Self` in function `fn oof(*mut Self)`.
-                let lo = if let TokenKind::OpenParen = p.prev_token.kind {
-                    p.prev_token.span.shrink_to_hi()
-                } else {
-                    p.prev_token.span
-                };
-                p.restore_snapshot(snapshot);
-                // Skip every token until next possible arg or end.
-                p.eat_to_tokens(&[exp!(Comma), exp!(CloseParen)]);
-                // Create a placeholder argument for proper arg count (issue #34264).
-                Ok(dummy_arg(Ident::new(sym::dummy, lo.to(p.prev_token.span)), guar))
-            });
+            let param =
+                p.parse_param_general(req_name, first_param, true, is_bare_fn_ptr).or_else(|e| {
+                    let guar = e.emit();
+                    // When parsing a param failed, we should check to make the span of the param
+                    // not contain '(' before it.
+                    // For example when parsing `*mut Self` in function `fn oof(*mut Self)`.
+                    let lo = if let TokenKind::OpenParen = p.prev_token.kind {
+                        p.prev_token.span.shrink_to_hi()
+                    } else {
+                        p.prev_token.span
+                    };
+                    p.restore_snapshot(snapshot);
+                    // Skip every token until next possible arg or end.
+                    p.eat_to_tokens(&[exp!(Comma), exp!(CloseParen)]);
+                    // Create a placeholder argument for proper arg count (issue #34264).
+                    Ok(dummy_arg(Ident::new(sym::dummy, lo.to(p.prev_token.span)), guar))
+                });
             // ...now that we've parsed the first argument, `self` is no longer allowed.
             first_param = false;
             param
@@ -2975,11 +2978,13 @@ impl<'a> Parser<'a> {
     ///
     /// - `self` is syntactically allowed when `first_param` holds.
     /// - `recover_arg_parse` is used to recover from a failed argument parse.
+    /// - `is_bare_fn_ptr` is used to improve diagnostics for bare fn ptrs.
     pub(super) fn parse_param_general(
         &mut self,
         req_name: ReqName,
         first_param: bool,
         recover_arg_parse: bool,
+        is_bare_fn_ptr: bool,
     ) -> PResult<'a, Param> {
         let lo = self.token.span;
         let attrs = self.parse_outer_attributes()?;
@@ -3040,6 +3045,7 @@ impl<'a> Parser<'a> {
                         ty = this.unexpected_any();
                     }
                 }
+
                 match ty {
                     Ok(ty) => {
                         let pat = this.mk_pat(ty.span, PatKind::Missing);
@@ -3052,7 +3058,7 @@ impl<'a> Parser<'a> {
                         // Recover from attempting to parse the argument as a type without pattern.
                         err.cancel();
                         this.restore_snapshot(parser_snapshot_before_ty);
-                        this.recover_arg_parse()?
+                        this.recover_arg_parse(!is_bare_fn_ptr)?
                     }
                     Err(err) => return Err(err),
                 }

compiler/rustc_parse/src/parser/path.rs

@@ -400,7 +400,7 @@ impl<'a> Parser<'a> {
 
                     let dcx = self.dcx();
                     let parse_params_result = self.parse_paren_comma_seq(|p| {
-                        let param = p.parse_param_general(|_| false, false, false);
+                        let param = p.parse_param_general(|_| false, false, false, false);
                         param.map(move |param| {
                             if !matches!(param.pat.kind, PatKind::Missing) {
                                 dcx.emit_err(FnPathFoundNamedParams {

tests/ui/error-codes/E0642-2015.fixed

@@ -0,0 +1,19 @@
+//@ run-rustfix
+
+#![allow(unused)] // for rustfix
+
+#[derive(Clone, Copy)]
+struct S;
+
+trait T {
+    fn foo(_: (i32, i32)); //~ ERROR patterns aren't allowed in trait methods in the 2015 edition
+
+    fn bar(_: (i32, i32)) {} //~ ERROR patterns aren't allowed in trait methods in the 2015 edition
+    fn method(_: S) {} //~ ERROR patterns aren't allowed in trait methods in the 2015 edition
+
+    fn f(&ident: &S) {} // ok
+    fn g(&&ident: &&S) {} // ok
+    fn h(mut ident: S) {} // ok
+}
+
+fn main() {}

tests/ui/error-codes/E0642-2015.rs

@@ -0,0 +1,19 @@
+//@ run-rustfix
+
+#![allow(unused)] // for rustfix
+
+#[derive(Clone, Copy)]
+struct S;
+
+trait T {
+    fn foo((x, y): (i32, i32)); //~ ERROR patterns aren't allowed in trait methods in the 2015 edition
+
+    fn bar((x, y): (i32, i32)) {} //~ ERROR patterns aren't allowed in trait methods in the 2015 edition
+    fn method(S { .. }: S) {} //~ ERROR patterns aren't allowed in trait methods in the 2015 edition
+
+    fn f(&ident: &S) {} // ok
+    fn g(&&ident: &&S) {} // ok
+    fn h(mut ident: S) {} // ok
+}
+
+fn main() {}