Auto merge of #55451 - estebank:arg-doc, r=pnkfelix

Custom diagnostic when trying to doc comment argument

When writing

```
pub fn f(
    /// Comment
    id: u8,
) {}
```

Produce a targeted diagnostic

```
error: documentation comments cannot be applied to method arguments
  --> $DIR/fn-arg-doc-comment.rs:2:5
   |
LL |     /// Comment
   |     ^^^^^^^^^^^ doc comments are not allowed here
```

Fix #54801.

Author: bors

src/libsyntax/ext/expand.rs

@@ -1032,7 +1032,7 @@ impl<'a> Parser<'a> {
                 }
             },
             AstFragmentKind::Ty => AstFragment::Ty(self.parse_ty()?),
-            AstFragmentKind::Pat => AstFragment::Pat(self.parse_pat()?),
+            AstFragmentKind::Pat => AstFragment::Pat(self.parse_pat(None)?),
         })
     }
 

src/libsyntax/ext/quote.rs

@@ -419,7 +419,7 @@ pub fn parse_item_panic(parser: &mut Parser) -> Option<P<Item>> {
 }
 
 pub fn parse_pat_panic(parser: &mut Parser) -> P<Pat> {
-    panictry!(parser.parse_pat())
+    panictry!(parser.parse_pat(None))
 }
 
 pub fn parse_arm_panic(parser: &mut Parser) -> Arm {

src/libsyntax/ext/tt/macro_parser.rs

@@ -888,7 +888,7 @@ fn parse_nt<'a>(p: &mut Parser<'a>, sp: Span, name: &str) -> Nonterminal {
                 FatalError.raise();
             }
         },
-        "pat" => token::NtPat(panictry!(p.parse_pat())),
+        "pat" => token::NtPat(panictry!(p.parse_pat(None))),
         "expr" => token::NtExpr(panictry!(p.parse_expr())),
         "literal" => token::NtLiteral(panictry!(p.parse_literal_maybe_minus())),
         "ty" => token::NtTy(panictry!(p.parse_ty())),

src/libsyntax/parse/parser.rs

@@ -1790,6 +1790,35 @@ impl<'a> Parser<'a> {
         self.look_ahead(offset + 1, |t| t == &token::Colon)
     }
 
+    /// Skip unexpected attributes and doc comments in this position and emit an appropriate error.
+    fn eat_incorrect_doc_comment(&mut self, applied_to: &str) {
+        if let token::DocComment(_) = self.token {
+            let mut err = self.diagnostic().struct_span_err(
+                self.span,
+                &format!("documentation comments cannot be applied to {}", applied_to),
+            );
+            err.span_label(self.span, "doc comments are not allowed here");
+            err.emit();
+            self.bump();
+        } else if self.token == token::Pound && self.look_ahead(1, |t| {
+            *t == token::OpenDelim(token::Bracket)
+        }) {
+            let lo = self.span;
+            // Skip every token until next possible arg.
+            while self.token != token::CloseDelim(token::Bracket) {
+                self.bump();
+            }
+            let sp = lo.to(self.span);
+            self.bump();
+            let mut err = self.diagnostic().struct_span_err(
+                sp,
+                &format!("attributes cannot be applied to {}", applied_to),
+            );
+            err.span_label(sp, "attributes are not allowed here");
+            err.emit();
+        }
+    }
+
     /// This version of parse arg doesn't necessarily require
     /// identifier names.
     fn parse_arg_general(&mut self, require_name: bool) -> PResult<'a, Arg> {
@@ -1798,7 +1827,8 @@ impl<'a> Parser<'a> {
         let (pat, ty) = if require_name || self.is_named_argument() {
             debug!("parse_arg_general parse_pat (require_name:{})",
                    require_name);
-            let pat = self.parse_pat()?;
+            self.eat_incorrect_doc_comment("method arguments");
+            let pat = self.parse_pat(Some("argument name"))?;
 
             if let Err(mut err) = self.expect(&token::Colon) {
                 // If we find a pattern followed by an identifier, it could be an (incorrect)
@@ -1820,10 +1850,12 @@ impl<'a> Parser<'a> {
                 return Err(err);
             }
 
+            self.eat_incorrect_doc_comment("a method argument's type");
             (pat, self.parse_ty()?)
         } else {
             debug!("parse_arg_general ident_to_pat");
             let parser_snapshot_before_ty = self.clone();
+            self.eat_incorrect_doc_comment("a method argument's type");
             let mut ty = self.parse_ty();
             if ty.is_ok() && self.token == token::Colon {
                 // This wasn't actually a type, but a pattern looking like a type,
@@ -1845,7 +1877,7 @@ impl<'a> Parser<'a> {
                     // Recover from attempting to parse the argument as a type without pattern.
                     err.cancel();
                     mem::replace(self, parser_snapshot_before_ty);
-                    let pat = self.parse_pat()?;
+                    let pat = self.parse_pat(Some("argument name"))?;
                     self.expect(&token::Colon)?;
                     let ty = self.parse_ty()?;
 
@@ -1883,7 +1915,7 @@ impl<'a> Parser<'a> {
 
     /// Parse an argument in a lambda header e.g. |arg, arg|
     fn parse_fn_block_arg(&mut self) -> PResult<'a, Arg> {
-        let pat = self.parse_pat()?;
+        let pat = self.parse_pat(Some("argument name"))?;
         let t = if self.eat(&token::Colon) {
             self.parse_ty()?
         } else {
@@ -2440,7 +2472,11 @@ impl<'a> Parser<'a> {
                     return Ok(self.mk_expr(lo.to(hi), ex, attrs));
                 }
                 if self.eat_keyword(keywords::Match) {
-                    return self.parse_match_expr(attrs);
+                    let match_sp = self.prev_span;
+                    return self.parse_match_expr(attrs).map_err(|mut err| {
+                        err.span_label(match_sp, "while parsing this match expression");
+                        err
+                    });
                 }
                 if self.eat_keyword(keywords::Unsafe) {
                     return self.parse_block_expr(
@@ -3746,7 +3782,7 @@ impl<'a> Parser<'a> {
                                   "`..` can only be used once per tuple or tuple struct pattern");
                 }
             } else if !self.check(&token::CloseDelim(token::Paren)) {
-                fields.push(self.parse_pat()?);
+                fields.push(self.parse_pat(None)?);
             } else {
                 break
             }
@@ -3802,7 +3838,7 @@ impl<'a> Parser<'a> {
                 }
             }
 
-            let subpat = self.parse_pat()?;
+            let subpat = self.parse_pat(None)?;
             if before_slice && self.eat(&token::DotDot) {
                 slice = Some(subpat);
                 before_slice = false;
@@ -3827,7 +3863,7 @@ impl<'a> Parser<'a> {
             // Parsing a pattern of the form "fieldname: pat"
             let fieldname = self.parse_field_name()?;
             self.bump();
-            let pat = self.parse_pat()?;
+            let pat = self.parse_pat(None)?;
             hi = pat.span;
             (pat, fieldname, false)
         } else {
@@ -4029,7 +4065,7 @@ impl<'a> Parser<'a> {
     /// "top-level" patterns in a match arm, `for` loop, `let`, &c. (in contrast
     /// to subpatterns within such).
     fn parse_top_level_pat(&mut self) -> PResult<'a, P<Pat>> {
-        let pat = self.parse_pat()?;
+        let pat = self.parse_pat(None)?;
         if self.token == token::Comma {
             // An unexpected comma after a top-level pattern is a clue that the
             // user (perhaps more accustomed to some other language) forgot the
@@ -4061,13 +4097,17 @@ impl<'a> Parser<'a> {
     }
 
     /// Parse a pattern.
-    pub fn parse_pat(&mut self) -> PResult<'a, P<Pat>> {
-        self.parse_pat_with_range_pat(true)
+    pub fn parse_pat(&mut self, expected: Option<&'static str>) -> PResult<'a, P<Pat>> {
+        self.parse_pat_with_range_pat(true, expected)
     }
 
     /// Parse a pattern, with a setting whether modern range patterns e.g. `a..=b`, `a..b` are
     /// allowed.
-    fn parse_pat_with_range_pat(&mut self, allow_range_pat: bool) -> PResult<'a, P<Pat>> {
+    fn parse_pat_with_range_pat(
+        &mut self,
+        allow_range_pat: bool,
+        expected: Option<&'static str>,
+    ) -> PResult<'a, P<Pat>> {
         maybe_whole!(self, NtPat, |x| x);
 
         let lo = self.span;
@@ -4083,7 +4123,7 @@ impl<'a> Parser<'a> {
                     err.span_label(self.span, "unexpected lifetime");
                     return Err(err);
                 }
-                let subpat = self.parse_pat_with_range_pat(false)?;
+                let subpat = self.parse_pat_with_range_pat(false, expected)?;
                 pat = PatKind::Ref(subpat, mutbl);
             }
             token::OpenDelim(token::Paren) => {
@@ -4129,7 +4169,7 @@ impl<'a> Parser<'a> {
                 pat = self.parse_pat_ident(BindingMode::ByRef(mutbl))?;
             } else if self.eat_keyword(keywords::Box) {
                 // Parse box pat
-                let subpat = self.parse_pat_with_range_pat(false)?;
+                let subpat = self.parse_pat_with_range_pat(false, None)?;
                 pat = PatKind::Box(subpat);
             } else if self.token.is_ident() && !self.token.is_reserved_ident() &&
                       self.parse_as_ident() {
@@ -4229,9 +4269,14 @@ impl<'a> Parser<'a> {
                     }
                     Err(mut err) => {
                         self.cancel(&mut err);
-                        let msg = format!("expected pattern, found {}", self.this_token_descr());
+                        let expected = expected.unwrap_or("pattern");
+                        let msg = format!(
+                            "expected {}, found {}",
+                            expected,
+                            self.this_token_descr(),
+                        );
                         let mut err = self.fatal(&msg);
-                        err.span_label(self.span, "expected pattern");
+                        err.span_label(self.span, format!("expected {}", expected));
                         return Err(err);
                     }
                 }
@@ -4275,7 +4320,7 @@ impl<'a> Parser<'a> {
                        -> PResult<'a, PatKind> {
         let ident = self.parse_ident()?;
         let sub = if self.eat(&token::At) {
-            Some(self.parse_pat()?)
+            Some(self.parse_pat(Some("binding pattern"))?)
         } else {
             None
         };

src/libsyntax/util/parser_testing.rs

@@ -68,7 +68,7 @@ pub fn string_to_item (source_str : String) -> Option<P<ast::Item>> {
 pub fn string_to_pat(source_str: String) -> P<ast::Pat> {
     let ps = ParseSess::new(FilePathMapping::empty());
     with_error_checking_parse(source_str, &ps, |p| {
-        p.parse_pat()
+        p.parse_pat(None)
     })
 }
 

src/test/ui/label/label_break_value_illegal_uses.stderr

@@ -27,7 +27,9 @@ error: expected one of `.`, `?`, `{`, or an operator, found `'b`
   --> $DIR/label_break_value_illegal_uses.rs:28:17
    |
 LL |     match false 'b: {} //~ ERROR expected one of `.`, `?`, `{`, or an operator
-   |                 ^^ expected one of `.`, `?`, `{`, or an operator here
+   |     -----       ^^ expected one of `.`, `?`, `{`, or an operator here
+   |     |
+   |     while parsing this match expression
 
 error: aborting due to 4 previous errors
 

src/test/ui/parser/fn-arg-doc-comment.rs

@@ -0,0 +1,37 @@
+pub fn f(
+    /// Comment
+    //~^ ERROR documentation comments cannot be applied to method arguments
+    //~| NOTE doc comments are not allowed here
+    id: u8,
+    /// Other
+    //~^ ERROR documentation comments cannot be applied to method arguments
+    //~| NOTE doc comments are not allowed here
+    a: u8,
+) {}
+
+fn foo(#[allow(dead_code)] id: i32) {}
+//~^ ERROR attributes cannot be applied to method arguments
+//~| NOTE attributes are not allowed here
+
+fn bar(id: #[allow(dead_code)] i32) {}
+//~^ ERROR attributes cannot be applied to a method argument's type
+//~| NOTE attributes are not allowed here
+
+fn main() {
+    // verify that the parser recovered and properly typechecked the args
+    f("", "");
+    //~^ ERROR mismatched types
+    //~| NOTE expected u8, found reference
+    //~| NOTE expected
+    //~| ERROR mismatched types
+    //~| NOTE expected u8, found reference
+    //~| NOTE expected
+    foo("");
+    //~^ ERROR mismatched types
+    //~| NOTE expected i32, found reference
+    //~| NOTE expected
+    bar("");
+    //~^ ERROR mismatched types
+    //~| NOTE expected i32, found reference
+    //~| NOTE expected
+}

src/test/ui/parser/fn-arg-doc-comment.stderr

@@ -0,0 +1,63 @@
+error: documentation comments cannot be applied to method arguments
+  --> $DIR/fn-arg-doc-comment.rs:2:5
+   |
+LL |     /// Comment
+   |     ^^^^^^^^^^^ doc comments are not allowed here
+
+error: documentation comments cannot be applied to method arguments
+  --> $DIR/fn-arg-doc-comment.rs:6:5
+   |
+LL |     /// Other
+   |     ^^^^^^^^^ doc comments are not allowed here
+
+error: attributes cannot be applied to method arguments
+  --> $DIR/fn-arg-doc-comment.rs:12:8
+   |
+LL | fn foo(#[allow(dead_code)] id: i32) {}
+   |        ^^^^^^^^^^^^^^^^^^^ attributes are not allowed here
+
+error: attributes cannot be applied to a method argument's type
+  --> $DIR/fn-arg-doc-comment.rs:16:12
+   |
+LL | fn bar(id: #[allow(dead_code)] i32) {}
+   |            ^^^^^^^^^^^^^^^^^^^ attributes are not allowed here
+
+error[E0308]: mismatched types
+  --> $DIR/fn-arg-doc-comment.rs:22:7
+   |
+LL |     f("", "");
+   |       ^^ expected u8, found reference
+   |
+   = note: expected type `u8`
+              found type `&'static str`
+
+error[E0308]: mismatched types
+  --> $DIR/fn-arg-doc-comment.rs:22:11
+   |
+LL |     f("", "");
+   |           ^^ expected u8, found reference
+   |
+   = note: expected type `u8`
+              found type `&'static str`
+
+error[E0308]: mismatched types
+  --> $DIR/fn-arg-doc-comment.rs:29:9
+   |
+LL |     foo("");
+   |         ^^ expected i32, found reference
+   |
+   = note: expected type `i32`
+              found type `&'static str`
+
+error[E0308]: mismatched types
+  --> $DIR/fn-arg-doc-comment.rs:33:9
+   |
+LL |     bar("");
+   |         ^^ expected i32, found reference
+   |
+   = note: expected type `i32`
+              found type `&'static str`
+
+error: aborting due to 8 previous errors
+
+For more information about this error, try `rustc --explain E0308`.

src/test/ui/parser/issue-33413.rs

@@ -11,5 +11,6 @@
 // compile-flags: -Z parse-only
 
 impl S {
-    fn f(*, a: u8) -> u8 {} //~ ERROR expected pattern, found `*`
+    fn f(*, a: u8) -> u8 {}
+    //~^ ERROR expected argument name, found `*`
 }

src/test/ui/parser/issue-33413.stderr

@@ -1,8 +1,8 @@
-error: expected pattern, found `*`
+error: expected argument name, found `*`
   --> $DIR/issue-33413.rs:14:10
    |
-LL |     fn f(*, a: u8) -> u8 {} //~ ERROR expected pattern, found `*`
-   |          ^ expected pattern
+LL |     fn f(*, a: u8) -> u8 {}
+   |          ^ expected argument name
 
 error: aborting due to previous error
 

src/test/ui/parser/match-refactor-to-expr.rs

@@ -12,7 +12,7 @@
 
 fn main() {
     let foo =
-        match
+        match //~ NOTE while parsing this match expression
         Some(4).unwrap_or_else(5)
         //~^ NOTE expected one of `.`, `?`, `{`, or an operator here
         ; //~ NOTE unexpected token

src/test/ui/parser/match-refactor-to-expr.stderr

@@ -1,8 +1,11 @@
 error: expected one of `.`, `?`, `{`, or an operator, found `;`
   --> $DIR/match-refactor-to-expr.rs:18:9
    |
-LL |         match
-   |         ----- help: try removing this `match`
+LL |         match //~ NOTE while parsing this match expression
+   |         -----
+   |         |
+   |         while parsing this match expression
+   |         help: try removing this `match`
 LL |         Some(4).unwrap_or_else(5)
    |                                  - expected one of `.`, `?`, `{`, or an operator here
 LL |         //~^ NOTE expected one of `.`, `?`, `{`, or an operator here

src/test/ui/parser/removed-syntax-mode.rs

@@ -10,4 +10,5 @@
 
 // compile-flags: -Z parse-only
 
-fn f(+x: isize) {} //~ ERROR expected pattern, found `+`
+fn f(+x: isize) {}
+//~^ ERROR expected argument name, found `+`