From ea355bc6be34f7c3c1da0342ade39593a7f5e494 Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Fri, 5 Mar 2021 04:19:15 +0900 Subject: [PATCH 1/4] Fix bad diagnostics for anon params with ref --- .../rustc_parse/src/parser/diagnostics.rs | 85 ++++++++++++------- .../ui/anon-params/anon-params-denied-2018.rs | 4 + .../anon-params-denied-2018.stderr | 28 +++++- src/test/ui/parser/lifetime-in-pattern.stderr | 14 +++ 4 files changed, 96 insertions(+), 35 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index f4ab3260d1a83..f214b11d5f052 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -1613,42 +1613,65 @@ 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, b) {}` 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, - ); - } - // Avoid suggesting that `fn foo(HashMap)` is fixed with a change to - // `fn foo(HashMap: TypeName)`. - 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, - ); + } else if require_name + && (self.token == token::Comma + || self.token == token::Lt + || self.token == token::CloseDelim(token::Paren)) + { + 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 let PatKind::Ident(_, ident, _) = pat.clone().into_inner().kind { + let mutab = mutab.prefix_str(); + ( + ident, + format!("self: &{}{}", mutab, ident), + format!("{}: &{}TypeName", ident, mutab), + format!("_: &{}{}", mutab, ident), + ) + } else { + return None; + } } + // Ignore other `PatKind`. + _ => return None, + }; + + // `fn foo(a, b) {}`, `fn foo(a, b) {}` 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, + ); + } + // Avoid suggesting that `fn foo(HashMap)` is fixed with a change to + // `fn foo(HashMap: TypeName)`. + 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.note("anonymous parameters are removed in the 2018 edition (see RFC 1685)"); - - // Don't attempt to recover by using the `X` in `X` as the parameter name. - return if self.token == token::Lt { None } else { Some(ident) }; } + err.span_suggestion( + pat.span, + "if this is a type, explicitly ignore the parameter name", + type_sugg, + Applicability::MachineApplicable, + ); + err.note("anonymous parameters are removed in the 2018 edition (see RFC 1685)"); + + // Don't attempt to recover by using the `X` in `X` 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 index 5721f5d235783..a7dfdc8373279 100644 --- 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,10 @@ 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 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 index 840294db0830a..0efb7d424e675 100644 --- 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,28 @@ 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:8:36 + --> $DIR/anon-params-denied-2018.rs:12:36 | LL | fn bar_with_default_impl(String, String) {} | ^ expected one of `:`, `@`, or `|` @@ -39,7 +59,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:12:44 | LL | fn bar_with_default_impl(String, String) {} | ^ expected one of `:`, `@`, or `|` @@ -55,7 +75,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:17:22 | LL | fn baz(a:usize, b, c: usize) -> usize { | ^ expected one of `:`, `@`, or `|` @@ -70,5 +90,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 5 previous errors diff --git a/src/test/ui/parser/lifetime-in-pattern.stderr b/src/test/ui/parser/lifetime-in-pattern.stderr index 71fd3cdf72370..4ffee657cabbe 100644 --- 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 From 8240f1a3d3f0ff3e36c19836ea4d783f29752b0b Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Fri, 5 Mar 2021 14:52:45 +0900 Subject: [PATCH 2/4] Fix bad diagnostics for anon params with qualified paths --- .../rustc_parse/src/parser/diagnostics.rs | 30 ++++++++++++------- .../ui/anon-params/anon-params-denied-2018.rs | 6 ++++ .../anon-params-denied-2018.stderr | 24 ++++++++++++--- 3 files changed, 46 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index f214b11d5f052..975b9cc15bd84 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -1627,18 +1627,28 @@ impl<'a> Parser<'a> { ), // Also catches `fn foo(&a)`. PatKind::Ref(ref pat, mutab) => { - if let PatKind::Ident(_, ident, _) = pat.clone().into_inner().kind { - let mutab = mutab.prefix_str(); - ( - ident, - format!("self: &{}{}", mutab, ident), - format!("{}: &{}TypeName", ident, mutab), - format!("_: &{}{}", mutab, ident), - ) - } else { - return None; + 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), + ) + } + PatKind::Path(..) => { + err.note("anonymous parameters are removed in the 2018 edition (see RFC 1685)"); + return None; + } + _ => return None, } } + // Also catches `fn foo(::Baz)` + PatKind::Path(..) => { + err.note("anonymous parameters are removed in the 2018 edition (see RFC 1685)"); + return None; + } // Ignore other `PatKind`. _ => return 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 index a7dfdc8373279..5487d5c9b032f 100644 --- a/src/test/ui/anon-params/anon-params-denied-2018.rs +++ b/src/test/ui/anon-params/anon-params-denied-2018.rs @@ -9,6 +9,12 @@ trait T { fn foo_with_ref(&mut i32); //~^ ERROR expected one of `:`, `@`, or `|`, found `)` + fn foo_with_qualified_path(::Baz); + //~^ ERROR expected one of `(`, `...`, `..=`, `..`, `::`, `:`, `{`, or `|`, found `)` + + fn foo_with_qualified_path_and_ref(&::Baz); + //~^ 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 index 0efb7d424e675..f57578f017489 100644 --- a/src/test/ui/anon-params/anon-params-denied-2018.stderr +++ b/src/test/ui/anon-params/anon-params-denied-2018.stderr @@ -38,8 +38,24 @@ 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(::Baz); + | ^ expected one of 8 possible tokens + | + = note: anonymous parameters are removed in the 2018 edition (see RFC 1685) + +error: expected one of `(`, `...`, `..=`, `..`, `::`, `:`, `{`, or `|`, found `)` + --> $DIR/anon-params-denied-2018.rs:15:56 + | +LL | fn foo_with_qualified_path_and_ref(&::Baz); + | ^ expected one of 8 possible tokens + | + = note: anonymous parameters are removed in the 2018 edition (see RFC 1685) + error: expected one of `:`, `@`, or `|`, found `,` - --> $DIR/anon-params-denied-2018.rs:12:36 + --> $DIR/anon-params-denied-2018.rs:18:36 | LL | fn bar_with_default_impl(String, String) {} | ^ expected one of `:`, `@`, or `|` @@ -59,7 +75,7 @@ LL | fn bar_with_default_impl(_: String, String) {} | ^^^^^^^^^ error: expected one of `:`, `@`, or `|`, found `)` - --> $DIR/anon-params-denied-2018.rs:12:44 + --> $DIR/anon-params-denied-2018.rs:18:44 | LL | fn bar_with_default_impl(String, String) {} | ^ expected one of `:`, `@`, or `|` @@ -75,7 +91,7 @@ LL | fn bar_with_default_impl(String, _: String) {} | ^^^^^^^^^ error: expected one of `:`, `@`, or `|`, found `,` - --> $DIR/anon-params-denied-2018.rs:17:22 + --> $DIR/anon-params-denied-2018.rs:23:22 | LL | fn baz(a:usize, b, c: usize) -> usize { | ^ expected one of `:`, `@`, or `|` @@ -90,5 +106,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 5 previous errors +error: aborting due to 7 previous errors From 2d99e68940087c7696a5d4919ba1456d6415fb9b Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Wed, 17 Mar 2021 09:49:46 +0900 Subject: [PATCH 3/4] Emit more pretty diagnostics for qualified paths --- .../rustc_parse/src/parser/diagnostics.rs | 35 +++++++++++-------- .../anon-params-denied-2018.stderr | 8 +++++ 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 975b9cc15bd84..77e85c06ff5ae 100644 --- 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? `. let expr = if self.token == token::OpenDelim(token::Brace) { // Handle `await { }`. - // 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 { }?` as `?.await`. self.parse_block_expr(None, self.token.span, BlockCheckMode::Default, AttrVec::new()) } else { @@ -1618,6 +1618,8 @@ impl<'a> Parser<'a> { || 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, @@ -1626,7 +1628,9 @@ impl<'a> Parser<'a> { format!("_: {}", ident), ), // Also catches `fn foo(&a)`. - PatKind::Ref(ref pat, mutab) => { + 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(); @@ -1637,20 +1641,23 @@ impl<'a> Parser<'a> { format!("_: &{}{}", mutab, ident), ) } - PatKind::Path(..) => { - err.note("anonymous parameters are removed in the 2018 edition (see RFC 1685)"); - return None; - } - _ => return None, + _ => unreachable!(), } } - // Also catches `fn foo(::Baz)` - PatKind::Path(..) => { - err.note("anonymous parameters are removed in the 2018 edition (see RFC 1685)"); + _ => { + // 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; } - // Ignore other `PatKind`. - _ => return None, }; // `fn foo(a, b) {}`, `fn foo(a, b) {}` or `fn foo(usize, usize) {}` @@ -1678,7 +1685,7 @@ impl<'a> Parser<'a> { type_sugg, Applicability::MachineApplicable, ); - err.note("anonymous parameters are removed in the 2018 edition (see RFC 1685)"); + err.note(rfc_note); // Don't attempt to recover by using the `X` in `X` as the parameter name. return if self.token == token::Lt { None } else { Some(ident) }; diff --git a/src/test/ui/anon-params/anon-params-denied-2018.stderr b/src/test/ui/anon-params/anon-params-denied-2018.stderr index f57578f017489..39f18dff25082 100644 --- a/src/test/ui/anon-params/anon-params-denied-2018.stderr +++ b/src/test/ui/anon-params/anon-params-denied-2018.stderr @@ -45,6 +45,10 @@ LL | fn foo_with_qualified_path(::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(_: ::Baz); + | ^^^^^^^^^^^^^^^^^^ error: expected one of `(`, `...`, `..=`, `..`, `::`, `:`, `{`, or `|`, found `)` --> $DIR/anon-params-denied-2018.rs:15:56 @@ -53,6 +57,10 @@ LL | fn foo_with_qualified_path_and_ref(&::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(_: &::Baz); + | ^^^^^^^^^^^^^^^^^^^ error: expected one of `:`, `@`, or `|`, found `,` --> $DIR/anon-params-denied-2018.rs:18:36 From 55bdf7f188f1a6ded58b2e28bc2c783ee75b1a60 Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Wed, 17 Mar 2021 11:41:05 +0900 Subject: [PATCH 4/4] Add more test case --- .../ui/anon-params/anon-params-denied-2018.rs | 4 +++ .../anon-params-denied-2018.stderr | 32 ++++++++++++++++--- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/test/ui/anon-params/anon-params-denied-2018.rs b/src/test/ui/anon-params/anon-params-denied-2018.rs index 5487d5c9b032f..95533cf3dfbf1 100644 --- a/src/test/ui/anon-params/anon-params-denied-2018.rs +++ b/src/test/ui/anon-params/anon-params-denied-2018.rs @@ -15,6 +15,10 @@ trait T { fn foo_with_qualified_path_and_ref(&::Baz); //~^ ERROR expected one of `(`, `...`, `..=`, `..`, `::`, `:`, `{`, or `|`, found `)` + fn foo_with_multiple_qualified_paths(::Baz, ::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 index 39f18dff25082..b53640cd65ba9 100644 --- a/src/test/ui/anon-params/anon-params-denied-2018.stderr +++ b/src/test/ui/anon-params/anon-params-denied-2018.stderr @@ -62,8 +62,32 @@ help: explicitly ignore the parameter name LL | fn foo_with_qualified_path_and_ref(_: &::Baz); | ^^^^^^^^^^^^^^^^^^^ +error: expected one of `(`, `...`, `..=`, `..`, `::`, `:`, `{`, or `|`, found `,` + --> $DIR/anon-params-denied-2018.rs:18:57 + | +LL | fn foo_with_multiple_qualified_paths(::Baz, ::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(_: ::Baz, ::Baz); + | ^^^^^^^^^^^^^^^^^^ + +error: expected one of `(`, `...`, `..=`, `..`, `::`, `:`, `{`, or `|`, found `)` + --> $DIR/anon-params-denied-2018.rs:18:74 + | +LL | fn foo_with_multiple_qualified_paths(::Baz, ::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(::Baz, _: ::Baz); + | ^^^^^^^^^^^^^^^^^^ + error: expected one of `:`, `@`, or `|`, found `,` - --> $DIR/anon-params-denied-2018.rs:18:36 + --> $DIR/anon-params-denied-2018.rs:22:36 | LL | fn bar_with_default_impl(String, String) {} | ^ expected one of `:`, `@`, or `|` @@ -83,7 +107,7 @@ LL | fn bar_with_default_impl(_: String, String) {} | ^^^^^^^^^ error: expected one of `:`, `@`, or `|`, found `)` - --> $DIR/anon-params-denied-2018.rs:18:44 + --> $DIR/anon-params-denied-2018.rs:22:44 | LL | fn bar_with_default_impl(String, String) {} | ^ expected one of `:`, `@`, or `|` @@ -99,7 +123,7 @@ LL | fn bar_with_default_impl(String, _: String) {} | ^^^^^^^^^ error: expected one of `:`, `@`, or `|`, found `,` - --> $DIR/anon-params-denied-2018.rs:23:22 + --> $DIR/anon-params-denied-2018.rs:27:22 | LL | fn baz(a:usize, b, c: usize) -> usize { | ^ expected one of `:`, `@`, or `|` @@ -114,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 7 previous errors +error: aborting due to 9 previous errors