Tweak self arg not as first argument of a method diagnostic

estebank · estebank · commit 976541884f67 · 2019-05-25T12:05:18.000-07:00
Mention that `self` is only valid on "associated functions"
```
error: unexpected `self` argument in function
  --&gt; $DIR/self-in-function-arg.rs:1:15
   |
LL | fn foo(x:i32, self: i32) -&gt; i32 { self }
   |               ^^^^ not valid as function argument
   |
   = note: `self` is only valid as the first argument of an associated function
```

When it is a method, mention it must be first
```
error: unexpected `self` argument in function
  --&gt; $DIR/trait-fn.rs:4:20
   |
LL |     fn c(foo: u32, self) {}
   |                    ^^^^ must be the first associated function argument
```
diff --git a/src/libsyntax/parse/diagnostics.rs b/src/libsyntax/parse/diagnostics.rs
@@ -594,6 +594,51 @@ impl<'a> Parser<'a> {
         }
     }
 
+    crate fn recover_arg_parse(&mut self) -> PResult<'a, (P<ast::Pat>, P<ast::Ty>)> {
+        let pat = self.parse_pat(Some("argument name"))?;
+        self.expect(&token::Colon)?;
+        let ty = self.parse_ty()?;
+
+        let mut err = self.diagnostic().struct_span_err_with_code(
+            pat.span,
+            "patterns aren't allowed in methods without bodies",
+            DiagnosticId::Error("E0642".into()),
+        );
+        err.span_suggestion_short(
+            pat.span,
+            "give this argument a name or use an underscore to ignore it",
+            "_".to_owned(),
+            Applicability::MachineApplicable,
+        );
+        err.emit();
+
+        // Pretend the pattern is `_`, to avoid duplicate errors from AST validation.
+        let pat = P(Pat {
+            node: PatKind::Wild,
+            span: pat.span,
+            id: ast::DUMMY_NODE_ID
+        });
+        Ok((pat, ty))
+    }
+
+    crate fn recover_bad_self_arg(
+        &mut self,
+        mut arg: ast::Arg,
+        is_trait_item: bool,
+    ) -> PResult<'a, ast::Arg> {
+        let sp = arg.pat.span;
+        arg.ty.node = TyKind::Err;
+        let mut err = self.struct_span_err(sp, "unexpected `self` argument in function");
+        if is_trait_item {
+            err.span_label(sp, "must be the first associated function argument");
+        } else {
+            err.span_label(sp, "not valid as function argument");
+            err.note("`self` is only valid as the first argument of an associated function");
+        }
+        err.emit();
+        Ok(arg)
+    }
+
     crate fn consume_block(&mut self, delim: token::DelimToken) {
         let mut brace_depth = 0;
         loop {
diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs
@@ -1805,50 +1805,23 @@ impl<'a> Parser<'a> {
     }
 
     /// This version of parse arg doesn't necessarily require identifier names.
-    fn parse_arg_general(&mut self, require_name: bool, is_trait_item: bool,
-                         allow_c_variadic: bool) -> PResult<'a, Arg> {
-        if let Ok(Some(_)) = self.parse_self_arg() {
-            let mut err = self.struct_span_err(self.prev_span,
-                "unexpected `self` argument in function");
-            err.span_label(self.prev_span,
-                "`self` is only valid as the first argument of an associated function");
-            return Err(err);
+    fn parse_arg_general(
+        &mut self,
+        require_name: bool,
+        is_trait_item: bool,
+        allow_c_variadic: bool,
+    ) -> PResult<'a, Arg> {
+        if let Ok(Some(arg)) = self.parse_self_arg() {
+            return self.recover_bad_self_arg(arg, is_trait_item);
         }
 
         let (pat, ty) = if require_name || self.is_named_argument() {
-            debug!("parse_arg_general parse_pat (require_name:{})",
-                   require_name);
+            debug!("parse_arg_general parse_pat (require_name:{})", require_name);
             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)
-                // C-style parameter declaration.
-                if self.check_ident() && self.look_ahead(1, |t| {
-                    *t == token::Comma || *t == token::CloseDelim(token::Paren)
-                }) {
-                    let ident = self.parse_ident().unwrap();
-                    let span = pat.span.with_hi(ident.span.hi());
-
-                    err.span_suggestion(
-                        span,
-                        "declare the type after the parameter binding",
-                        String::from("<identifier>: <type>"),
-                        Applicability::HasPlaceholders,
-                    );
-                } else if require_name && is_trait_item {
-                    if let PatKind::Ident(_, ident, _) = pat.node {
-                        err.span_suggestion(
-                            pat.span,
-                            "explicitly ignore parameter",
-                            format!("_: {}", ident),
-                            Applicability::MachineApplicable,
-                        );
-                    }
-
-                    err.note("anonymous parameters are removed in the 2018 edition (see RFC 1685)");
-                }
-
+                self.argument_without_type(&mut err, pat, require_name, is_trait_item);
                 return Err(err);
             }
 
diff --git a/src/test/ui/invalid-self-argument/bare-fn-start.rs b/src/test/ui/invalid-self-argument/bare-fn-start.rs
@@ -1,5 +1,6 @@
 fn a(&self) { }
 //~^ ERROR unexpected `self` argument in function
+//~| NOTE not valid as function argument
 //~| NOTE `self` is only valid as the first argument of an associated function
 
 fn main() { }
diff --git a/src/test/ui/invalid-self-argument/bare-fn-start.stderr b/src/test/ui/invalid-self-argument/bare-fn-start.stderr
@@ -1,8 +1,10 @@
 error: unexpected `self` argument in function
-  --> $DIR/bare-fn-start.rs:1:7
+  --> $DIR/bare-fn-start.rs:1:6
    |
 LL | fn a(&self) { }
-   |       ^^^^ `self` is only valid as the first argument of an associated function
+   |      ^^^^^ not valid as function argument
+   |
+   = note: `self` is only valid as the first argument of an associated function
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/invalid-self-argument/bare-fn.rs b/src/test/ui/invalid-self-argument/bare-fn.rs
@@ -1,5 +1,6 @@
 fn b(foo: u32, &mut self) { }
 //~^ ERROR unexpected `self` argument in function
+//~| NOTE not valid as function argument
 //~| NOTE `self` is only valid as the first argument of an associated function
 
 fn main() { }
diff --git a/src/test/ui/invalid-self-argument/bare-fn.stderr b/src/test/ui/invalid-self-argument/bare-fn.stderr
@@ -1,8 +1,10 @@
 error: unexpected `self` argument in function
-  --> $DIR/bare-fn.rs:1:21
+  --> $DIR/bare-fn.rs:1:16
    |
 LL | fn b(foo: u32, &mut self) { }
-   |                     ^^^^ `self` is only valid as the first argument of an associated function
+   |                ^^^^^^^^^ not valid as function argument
+   |
+   = note: `self` is only valid as the first argument of an associated function
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/invalid-self-argument/trait-fn.rs b/src/test/ui/invalid-self-argument/trait-fn.rs
@@ -3,7 +3,7 @@ struct Foo {}
 impl Foo {
     fn c(foo: u32, self) {}
     //~^ ERROR unexpected `self` argument in function
-    //~| NOTE `self` is only valid as the first argument of an associated function
+    //~| NOTE must be the first associated function argument
 
     fn good(&mut self, foo: u32) {}
 }
diff --git a/src/test/ui/invalid-self-argument/trait-fn.stderr b/src/test/ui/invalid-self-argument/trait-fn.stderr
@@ -2,7 +2,7 @@ error: unexpected `self` argument in function
   --> $DIR/trait-fn.rs:4:20
    |
 LL |     fn c(foo: u32, self) {}
-   |                    ^^^^ `self` is only valid as the first argument of an associated function
+   |                    ^^^^ must be the first associated function argument
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/parser/self-in-function-arg.rs b/src/test/ui/parser/self-in-function-arg.rs
@@ -0,0 +1,3 @@
+fn foo(x:i32, self: i32) -> i32 { self } //~ ERROR unexpected `self` argument in function
+
+fn main() {}
diff --git a/src/test/ui/parser/self-in-function-arg.stderr b/src/test/ui/parser/self-in-function-arg.stderr
@@ -0,0 +1,10 @@
+error: unexpected `self` argument in function
+  --> $DIR/self-in-function-arg.rs:1:15
+   |
+LL | fn foo(x:i32, self: i32) -> i32 { self }
+   |               ^^^^ not valid as function argument
+   |
+   = note: `self` is only valid as the first argument of an associated function
+
+error: aborting due to previous error
+

Original file line number	Diff line number	Diff line change
`@@ -3,7 +3,7 @@ struct Foo {}`
`3`	`3`	`impl Foo {`
`4`	`4`	`fn c(foo: u32, self) {}`
`5`	`5`	//~^ ERROR unexpected `self` argument in function
`6`		- //~\| NOTE `self` is only valid as the first argument of an associated function
	`6`	`+ //~\| NOTE must be the first associated function argument`
`7`	`7`
`8`	`8`	`fn good(&mut self, foo: u32) {}`
`9`	`9`	`}`
Original file line number	Diff line number	Diff line change
@@ -2,7 +2,7 @@ error: unexpected `self` argument in function
`2`	`2`	`--> $DIR/trait-fn.rs:4:20`
`3`	`3`	`\|`
`4`	`4`	`LL \| fn c(foo: u32, self) {}`
`5`		- \| ^^^^ `self` is only valid as the first argument of an associated function
	`5`	`+ \| ^^^^ must be the first associated function argument`
`6`	`6`
`7`	`7`	`error: aborting due to previous error`
`8`	`8`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	+fn foo(x:i32, self: i32) -> i32 { self } //~ ERROR unexpected `self` argument in function
	`2`	`+`
	`3`	`+fn main() {}`