Rollup merge of rust-lang#82774 - JohnTitor:bad-diag-for-anon-params-with-ref, r=estebank

JohnTitor · web-flow · commit a16db3dcda83 · 2021-03-17T15:20:48.000+09:00
Fix bad diagnostics for anon params with ref and/or qualified paths Fixes rust-lang#82729 It's easier to review with hiding whitespace changes.
diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs
@@ -640,7 +640,7 @@ impl<'a> Parser<'a> {
                     }
                 }
                 Err(mut err) => {
-                    // We could't parse generic parameters, unlikely to be a turbofish. Rely on
+                    // We couldn't parse generic parameters, unlikely to be a turbofish. Rely on
                     // generic parse error instead.
                     err.cancel();
                     *self = snapshot;
@@ -1242,7 +1242,7 @@ impl<'a> Parser<'a> {
         let is_question = self.eat(&token::Question); // Handle `await? <expr>`.
         let expr = if self.token == token::OpenDelim(token::Brace) {
             // Handle `await { <expr> }`.
-            // This needs to be handled separatedly from the next arm to avoid
+            // This needs to be handled separately from the next arm to avoid
             // interpreting `await { <expr> }?` as `<expr>?.await`.
             self.parse_block_expr(None, self.token.span, BlockCheckMode::Default, AttrVec::new())
         } else {
@@ -1613,42 +1613,82 @@ impl<'a> Parser<'a> {
                 Applicability::HasPlaceholders,
             );
             return Some(ident);
-        } else if let PatKind::Ident(_, ident, _) = pat.kind {
-            if require_name
-                && (self.token == token::Comma
-                    || self.token == token::Lt
-                    || self.token == token::CloseDelim(token::Paren))
-            {
-                // `fn foo(a, b) {}`, `fn foo(a<x>, b<y>) {}` or `fn foo(usize, usize) {}`
-                if first_param {
-                    err.span_suggestion(
-                        pat.span,
-                        "if this is a `self` type, give it a parameter name",
-                        format!("self: {}", ident),
-                        Applicability::MaybeIncorrect,
-                    );
+        } else if require_name
+            && (self.token == token::Comma
+                || self.token == token::Lt
+                || self.token == token::CloseDelim(token::Paren))
+        {
+            let rfc_note = "anonymous parameters are removed in the 2018 edition (see RFC 1685)";
+
+            let (ident, self_sugg, param_sugg, type_sugg) = match pat.kind {
+                PatKind::Ident(_, ident, _) => (
+                    ident,
+                    format!("self: {}", ident),
+                    format!("{}: TypeName", ident),
+                    format!("_: {}", ident),
+                ),
+                // Also catches `fn foo(&a)`.
+                PatKind::Ref(ref pat, mutab)
+                    if matches!(pat.clone().into_inner().kind, PatKind::Ident(..)) =>
+                {
+                    match pat.clone().into_inner().kind {
+                        PatKind::Ident(_, ident, _) => {
+                            let mutab = mutab.prefix_str();
+                            (
+                                ident,
+                                format!("self: &{}{}", mutab, ident),
+                                format!("{}: &{}TypeName", ident, mutab),
+                                format!("_: &{}{}", mutab, ident),
+                            )
+                        }
+                        _ => unreachable!(),
+                    }
                 }
-                // Avoid suggesting that `fn foo(HashMap<u32>)` is fixed with a change to
-                // `fn foo(HashMap: TypeName<u32>)`.
-                if self.token != token::Lt {
-                    err.span_suggestion(
-                        pat.span,
-                        "if this is a parameter name, give it a type",
-                        format!("{}: TypeName", ident),
-                        Applicability::HasPlaceholders,
-                    );
+                _ => {
+                    // Otherwise, try to get a type and emit a suggestion.
+                    if let Some(ty) = pat.to_ty() {
+                        err.span_suggestion_verbose(
+                            pat.span,
+                            "explicitly ignore the parameter name",
+                            format!("_: {}", pprust::ty_to_string(&ty)),
+                            Applicability::MachineApplicable,
+                        );
+                        err.note(rfc_note);
+                    }
+
+                    return None;
                 }
+            };
+
+            // `fn foo(a, b) {}`, `fn foo(a<x>, b<y>) {}` or `fn foo(usize, usize) {}`
+            if first_param {
                 err.span_suggestion(
                     pat.span,
-                    "if this is a type, explicitly ignore the parameter name",
-                    format!("_: {}", ident),
-                    Applicability::MachineApplicable,
+                    "if this is a `self` type, give it a parameter name",
+                    self_sugg,
+                    Applicability::MaybeIncorrect,
                 );
-                err.note("anonymous parameters are removed in the 2018 edition (see RFC 1685)");
-
-                // Don't attempt to recover by using the `X` in `X<Y>` as the parameter name.
-                return if self.token == token::Lt { None } else { Some(ident) };
             }
+            // Avoid suggesting that `fn foo(HashMap<u32>)` is fixed with a change to
+            // `fn foo(HashMap: TypeName<u32>)`.
+            if self.token != token::Lt {
+                err.span_suggestion(
+                    pat.span,
+                    "if this is a parameter name, give it a type",
+                    param_sugg,
+                    Applicability::HasPlaceholders,
+                );
+            }
+            err.span_suggestion(
+                pat.span,
+                "if this is a type, explicitly ignore the parameter name",
+                type_sugg,
+                Applicability::MachineApplicable,
+            );
+            err.note(rfc_note);
+
+            // Don't attempt to recover by using the `X` in `X<Y>` as the parameter name.
+            return if self.token == token::Lt { None } else { Some(ident) };
         }
         None
     }
diff --git a/src/test/ui/anon-params/anon-params-denied-2018.rs b/src/test/ui/anon-params/anon-params-denied-2018.rs
@@ -5,6 +5,20 @@
 trait T {
     fn foo(i32); //~ expected one of `:`, `@`, or `|`, found `)`
 
+    // Also checks with `&`
+    fn foo_with_ref(&mut i32);
+    //~^ ERROR expected one of `:`, `@`, or `|`, found `)`
+
+    fn foo_with_qualified_path(<Bar as T>::Baz);
+    //~^ ERROR expected one of `(`, `...`, `..=`, `..`, `::`, `:`, `{`, or `|`, found `)`
+
+    fn foo_with_qualified_path_and_ref(&<Bar as T>::Baz);
+    //~^ ERROR expected one of `(`, `...`, `..=`, `..`, `::`, `:`, `{`, or `|`, found `)`
+
+    fn foo_with_multiple_qualified_paths(<Bar as T>::Baz, <Bar as T>::Baz);
+    //~^ ERROR expected one of `(`, `...`, `..=`, `..`, `::`, `:`, `{`, or `|`, found `,`
+    //~| ERROR expected one of `(`, `...`, `..=`, `..`, `::`, `:`, `{`, or `|`, found `)`
+
     fn bar_with_default_impl(String, String) {}
     //~^ ERROR expected one of `:`
     //~| ERROR expected one of `:`
diff --git a/src/test/ui/anon-params/anon-params-denied-2018.stderr b/src/test/ui/anon-params/anon-params-denied-2018.stderr
@@ -18,8 +18,76 @@ help: if this is a type, explicitly ignore the parameter name
 LL |     fn foo(_: i32);
    |            ^^^^^^
 
+error: expected one of `:`, `@`, or `|`, found `)`
+  --> $DIR/anon-params-denied-2018.rs:9:29
+   |
+LL |     fn foo_with_ref(&mut i32);
+   |                             ^ expected one of `:`, `@`, or `|`
+   |
+   = note: anonymous parameters are removed in the 2018 edition (see RFC 1685)
+help: if this is a `self` type, give it a parameter name
+   |
+LL |     fn foo_with_ref(self: &mut i32);
+   |                     ^^^^^^^^^^^^^^
+help: if this is a parameter name, give it a type
+   |
+LL |     fn foo_with_ref(i32: &mut TypeName);
+   |                     ^^^^^^^^^^^^^^^^^^
+help: if this is a type, explicitly ignore the parameter name
+   |
+LL |     fn foo_with_ref(_: &mut i32);
+   |                     ^^^^^^^^^^^
+
+error: expected one of `(`, `...`, `..=`, `..`, `::`, `:`, `{`, or `|`, found `)`
+  --> $DIR/anon-params-denied-2018.rs:12:47
+   |
+LL |     fn foo_with_qualified_path(<Bar as T>::Baz);
+   |                                               ^ expected one of 8 possible tokens
+   |
+   = note: anonymous parameters are removed in the 2018 edition (see RFC 1685)
+help: explicitly ignore the parameter name
+   |
+LL |     fn foo_with_qualified_path(_: <Bar as T>::Baz);
+   |                                ^^^^^^^^^^^^^^^^^^
+
+error: expected one of `(`, `...`, `..=`, `..`, `::`, `:`, `{`, or `|`, found `)`
+  --> $DIR/anon-params-denied-2018.rs:15:56
+   |
+LL |     fn foo_with_qualified_path_and_ref(&<Bar as T>::Baz);
+   |                                                        ^ expected one of 8 possible tokens
+   |
+   = note: anonymous parameters are removed in the 2018 edition (see RFC 1685)
+help: explicitly ignore the parameter name
+   |
+LL |     fn foo_with_qualified_path_and_ref(_: &<Bar as T>::Baz);
+   |                                        ^^^^^^^^^^^^^^^^^^^
+
+error: expected one of `(`, `...`, `..=`, `..`, `::`, `:`, `{`, or `|`, found `,`
+  --> $DIR/anon-params-denied-2018.rs:18:57
+   |
+LL |     fn foo_with_multiple_qualified_paths(<Bar as T>::Baz, <Bar as T>::Baz);
+   |                                                         ^ expected one of 8 possible tokens
+   |
+   = note: anonymous parameters are removed in the 2018 edition (see RFC 1685)
+help: explicitly ignore the parameter name
+   |
+LL |     fn foo_with_multiple_qualified_paths(_: <Bar as T>::Baz, <Bar as T>::Baz);
+   |                                          ^^^^^^^^^^^^^^^^^^
+
+error: expected one of `(`, `...`, `..=`, `..`, `::`, `:`, `{`, or `|`, found `)`
+  --> $DIR/anon-params-denied-2018.rs:18:74
+   |
+LL |     fn foo_with_multiple_qualified_paths(<Bar as T>::Baz, <Bar as T>::Baz);
+   |                                                                          ^ expected one of 8 possible tokens
+   |
+   = note: anonymous parameters are removed in the 2018 edition (see RFC 1685)
+help: explicitly ignore the parameter name
+   |
+LL |     fn foo_with_multiple_qualified_paths(<Bar as T>::Baz, _: <Bar as T>::Baz);
+   |                                                           ^^^^^^^^^^^^^^^^^^
+
 error: expected one of `:`, `@`, or `|`, found `,`
-  --> $DIR/anon-params-denied-2018.rs:8:36
+  --> $DIR/anon-params-denied-2018.rs:22:36
    |
 LL |     fn bar_with_default_impl(String, String) {}
    |                                    ^ expected one of `:`, `@`, or `|`
@@ -39,7 +107,7 @@ LL |     fn bar_with_default_impl(_: String, String) {}
    |                              ^^^^^^^^^
 
 error: expected one of `:`, `@`, or `|`, found `)`
-  --> $DIR/anon-params-denied-2018.rs:8:44
+  --> $DIR/anon-params-denied-2018.rs:22:44
    |
 LL |     fn bar_with_default_impl(String, String) {}
    |                                            ^ expected one of `:`, `@`, or `|`
@@ -55,7 +123,7 @@ LL |     fn bar_with_default_impl(String, _: String) {}
    |                                      ^^^^^^^^^
 
 error: expected one of `:`, `@`, or `|`, found `,`
-  --> $DIR/anon-params-denied-2018.rs:13:22
+  --> $DIR/anon-params-denied-2018.rs:27:22
    |
 LL |     fn baz(a:usize, b, c: usize) -> usize {
    |                      ^ expected one of `:`, `@`, or `|`
@@ -70,5 +138,5 @@ help: if this is a type, explicitly ignore the parameter name
 LL |     fn baz(a:usize, _: b, c: usize) -> usize {
    |                     ^^^^
 
-error: aborting due to 4 previous errors
+error: aborting due to 9 previous errors
 
diff --git a/src/test/ui/parser/lifetime-in-pattern.stderr b/src/test/ui/parser/lifetime-in-pattern.stderr
@@ -9,6 +9,20 @@ error: expected one of `:`, `@`, or `|`, found `)`
    |
 LL | fn test(&'a str) {
    |                ^ expected one of `:`, `@`, or `|`
+   |
+   = note: anonymous parameters are removed in the 2018 edition (see RFC 1685)
+help: if this is a `self` type, give it a parameter name
+   |
+LL | fn test(self: &str) {
+   |         ^^^^^^^^^^
+help: if this is a parameter name, give it a type
+   |
+LL | fn test(str: &TypeName) {
+   |         ^^^^^^^^^^^^^^
+help: if this is a type, explicitly ignore the parameter name
+   |
+LL | fn test(_: &str) {
+   |         ^^^^^^^
 
 error: aborting due to 2 previous errors
 
