From 3986c964481a048100565c8d30b1937ec2eb516d Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Sun, 18 Nov 2018 22:21:38 -0800 Subject: [PATCH 1/2] enum type instead of variant suggestion unification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Weirdly, we were deciding between a help note and a structured suggestion based on whether the import candidate span was a dummy—but we weren't using that span in any case! The dummy-ness of the span (which appears to be a matter of this-crate vs. other-crate definition) isn't the right criterion by which we should decide whether it's germane to mention that "there is an enum variant"; instead, let's use the someness of `def` (which is used as the `has_unexpected_resolution` argument to `error_code`). Since `import_candidate_to_paths` has no other callers, we are free to stop returning the span and rename the function. By using `span_suggestions_`, we leverage the max-suggestions output limit already built in to the emitter, thus resolving #56028. In the matter of message wording, "you can" is redundant (and perhaps too informal); prefer the imperative. --- src/librustc_resolve/lib.rs | 46 +++++++++++-------- .../issue-56028-there-is-an-enum-variant.rs | 13 ++++++ ...ssue-56028-there-is-an-enum-variant.stderr | 38 +++++++++++++++ src/test/ui/enum/enum-variant-type-2.stderr | 2 +- src/test/ui/issues/issue-17546.stderr | 4 +- src/test/ui/issues/issue-30535.stderr | 7 +-- src/test/ui/issues/issue-35075.stderr | 18 ++++---- src/test/ui/issues/issue-35675.stderr | 32 ++++++++----- .../ui/variants/variant-used-as-type.stderr | 16 +++---- 9 files changed, 122 insertions(+), 54 deletions(-) create mode 100644 src/test/ui/did_you_mean/issue-56028-there-is-an-enum-variant.rs create mode 100644 src/test/ui/did_you_mean/issue-56028-there-is-an-enum-variant.stderr diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 794e5741d62ca..8f329283b1a28 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -3213,23 +3213,33 @@ impl<'a> Resolver<'a> { let enum_candidates = this.lookup_import_candidates(ident, ns, is_enum_variant); let mut enum_candidates = enum_candidates.iter() - .map(|suggestion| import_candidate_to_paths(&suggestion)).collect::>(); + .map(|suggestion| { + import_candidate_to_enum_paths(&suggestion) + }).collect::>(); enum_candidates.sort(); - for (sp, variant_path, enum_path) in enum_candidates { - if sp.is_dummy() { - let msg = format!("there is an enum variant `{}`, \ - try using `{}`?", - variant_path, - enum_path); - err.help(&msg); + + if !enum_candidates.is_empty() { + // contextualize for E0412 "cannot find type", but don't belabor the point + // (that it's a variant) for E0573 "expected type, found variant" + let preamble = if def.is_none() { + let others = match enum_candidates.len() { + 1 => String::new(), + 2 => " and 1 other".to_owned(), + n => format!(" and {} others", n) + }; + format!("there is an enum variant `{}`{}; ", + enum_candidates[0].0, others) } else { - err.span_suggestion_with_applicability( - span, - "you can try using the variant's enum", - enum_path, - Applicability::MachineApplicable, - ); - } + String::new() + }; + let msg = format!("{}try using the variant's enum", preamble); + + err.span_suggestions_with_applicability( + span, + &msg, + enum_candidates.into_iter().map(|(_variant, enum_ty)| enum_ty), + Applicability::MachineApplicable, + ); } } if path.len() == 1 && this.self_type_is_available(span) { @@ -5128,8 +5138,8 @@ fn path_names_to_string(path: &Path) -> String { .collect::>()) } -/// Get the path for an enum and the variant from an `ImportSuggestion` for an enum variant. -fn import_candidate_to_paths(suggestion: &ImportSuggestion) -> (Span, String, String) { +/// Get the stringified path for an enum from an `ImportSuggestion` for an enum variant. +fn import_candidate_to_enum_paths(suggestion: &ImportSuggestion) -> (String, String) { let variant_path = &suggestion.path; let variant_path_string = path_names_to_string(variant_path); @@ -5140,7 +5150,7 @@ fn import_candidate_to_paths(suggestion: &ImportSuggestion) -> (Span, String, St }; let enum_path_string = path_names_to_string(&enum_path); - (suggestion.path.span, variant_path_string, enum_path_string) + (variant_path_string, enum_path_string) } diff --git a/src/test/ui/did_you_mean/issue-56028-there-is-an-enum-variant.rs b/src/test/ui/did_you_mean/issue-56028-there-is-an-enum-variant.rs new file mode 100644 index 0000000000000..ec41c41141616 --- /dev/null +++ b/src/test/ui/did_you_mean/issue-56028-there-is-an-enum-variant.rs @@ -0,0 +1,13 @@ +enum PutDown { Set } +enum AffixHeart { Set } +enum CauseToBe { Set } +enum Determine { Set } +enum TableDishesAction { Set } +enum Solidify { Set } +enum UnorderedCollection { Set } + +fn setup() -> Set { Set } + +fn main() { + setup(); +} diff --git a/src/test/ui/did_you_mean/issue-56028-there-is-an-enum-variant.stderr b/src/test/ui/did_you_mean/issue-56028-there-is-an-enum-variant.stderr new file mode 100644 index 0000000000000..6107ca32a5d75 --- /dev/null +++ b/src/test/ui/did_you_mean/issue-56028-there-is-an-enum-variant.stderr @@ -0,0 +1,38 @@ +error[E0412]: cannot find type `Set` in this scope + --> $DIR/issue-56028-there-is-an-enum-variant.rs:9:15 + | +LL | fn setup() -> Set { Set } + | ^^^ not found in this scope +help: there is an enum variant `AffixHeart::Set` and 7 others; try using the variant's enum + | +LL | fn setup() -> AffixHeart { Set } + | ^^^^^^^^^^ +LL | fn setup() -> CauseToBe { Set } + | ^^^^^^^^^ +LL | fn setup() -> Determine { Set } + | ^^^^^^^^^ +LL | fn setup() -> PutDown { Set } + | ^^^^^^^ +and 3 other candidates + +error[E0425]: cannot find value `Set` in this scope + --> $DIR/issue-56028-there-is-an-enum-variant.rs:9:21 + | +LL | fn setup() -> Set { Set } + | ^^^ not found in this scope +help: possible candidates are found in other modules, you can import them into scope + | +LL | use AffixHeart::Set; + | +LL | use CauseToBe::Set; + | +LL | use Determine::Set; + | +LL | use PutDown::Set; + | +and 3 other candidates + +error: aborting due to 2 previous errors + +Some errors occurred: E0412, E0425. +For more information about an error, try `rustc --explain E0412`. diff --git a/src/test/ui/enum/enum-variant-type-2.stderr b/src/test/ui/enum/enum-variant-type-2.stderr index 7a786af71bb3f..eb869f5539f36 100644 --- a/src/test/ui/enum/enum-variant-type-2.stderr +++ b/src/test/ui/enum/enum-variant-type-2.stderr @@ -5,7 +5,7 @@ LL | fn foo(x: Foo::Bar) {} //~ ERROR expected type, found variant `Foo::Bar` | ^^^^^^^^ | | | not a type - | help: you can try using the variant's enum: `Foo` + | help: try using the variant's enum: `Foo` error: aborting due to previous error diff --git a/src/test/ui/issues/issue-17546.stderr b/src/test/ui/issues/issue-17546.stderr index 39f7d5fcc04bc..a8ab5e8eb7745 100644 --- a/src/test/ui/issues/issue-17546.stderr +++ b/src/test/ui/issues/issue-17546.stderr @@ -5,7 +5,7 @@ LL | fn new() -> NoResult { | --------^^^^^^^^^^^^^^^^ | | | did you mean `Result`? - | help: you can try using the variant's enum: `foo::MyEnum` + | help: try using the variant's enum: `foo::MyEnum` error[E0573]: expected type, found variant `Result` --> $DIR/issue-17546.rs:32:17 @@ -48,7 +48,7 @@ LL | fn newer() -> NoResult { | --------^^^^^^^^^^^^^^^^^^^^^ | | | did you mean `Result`? - | help: you can try using the variant's enum: `foo::MyEnum` + | help: try using the variant's enum: `foo::MyEnum` error: aborting due to 4 previous errors diff --git a/src/test/ui/issues/issue-30535.stderr b/src/test/ui/issues/issue-30535.stderr index c3838fdb9cf07..35ef227acfce6 100644 --- a/src/test/ui/issues/issue-30535.stderr +++ b/src/test/ui/issues/issue-30535.stderr @@ -2,9 +2,10 @@ error[E0573]: expected type, found variant `foo::Foo::FooV` --> $DIR/issue-30535.rs:16:8 | LL | _: foo::Foo::FooV //~ ERROR expected type, found variant `foo::Foo::FooV` - | ^^^^^^^^^^^^^^ not a type - | - = help: there is an enum variant `foo::Foo::FooV`, try using `foo::Foo`? + | ^^^^^^^^^^^^^^ + | | + | not a type + | help: try using the variant's enum: `foo::Foo` error: aborting due to previous error diff --git a/src/test/ui/issues/issue-35075.stderr b/src/test/ui/issues/issue-35075.stderr index 9b2f17f038b8f..ead07a5f0c8fe 100644 --- a/src/test/ui/issues/issue-35075.stderr +++ b/src/test/ui/issues/issue-35075.stderr @@ -2,19 +2,21 @@ error[E0412]: cannot find type `Foo` in this scope --> $DIR/issue-35075.rs:12:12 | LL | inner: Foo //~ ERROR cannot find type `Foo` in this scope - | ^^^--- - | | - | not found in this scope - | help: you can try using the variant's enum: `Baz` + | ^^^ not found in this scope +help: there is an enum variant `Baz::Foo`; try using the variant's enum + | +LL | inner: Baz //~ ERROR cannot find type `Foo` in this scope + | ^^^ error[E0412]: cannot find type `Foo` in this scope --> $DIR/issue-35075.rs:16:9 | LL | Foo(Foo) //~ ERROR cannot find type `Foo` in this scope - | ^^^--- - | | - | not found in this scope - | help: you can try using the variant's enum: `Baz` + | ^^^ not found in this scope +help: there is an enum variant `Baz::Foo`; try using the variant's enum + | +LL | Foo(Baz) //~ ERROR cannot find type `Foo` in this scope + | ^^^ error: aborting due to 2 previous errors diff --git a/src/test/ui/issues/issue-35675.stderr b/src/test/ui/issues/issue-35675.stderr index fef8de3a28d9e..2f07ee7124d07 100644 --- a/src/test/ui/issues/issue-35675.stderr +++ b/src/test/ui/issues/issue-35675.stderr @@ -2,10 +2,11 @@ error[E0412]: cannot find type `Apple` in this scope --> $DIR/issue-35675.rs:17:29 | LL | fn should_return_fruit() -> Apple { + | ^^^^^ not found in this scope +help: there is an enum variant `Fruit::Apple`; try using the variant's enum + | +LL | fn should_return_fruit() -> Fruit { | ^^^^^ - | | - | not found in this scope - | help: you can try using the variant's enum: `Fruit` error[E0425]: cannot find function `Apple` in this scope --> $DIR/issue-35675.rs:19:5 @@ -24,7 +25,7 @@ LL | fn should_return_fruit_too() -> Fruit::Apple { | ^^^^^^^^^^^^ | | | not a type - | help: you can try using the variant's enum: `Fruit` + | help: try using the variant's enum: `Fruit` error[E0425]: cannot find function `Apple` in this scope --> $DIR/issue-35675.rs:25:5 @@ -41,27 +42,34 @@ error[E0573]: expected type, found variant `Ok` | LL | fn foo() -> Ok { | ^^ not a type +help: try using the variant's enum | - = help: there is an enum variant `std::prelude::v1::Ok`, try using `std::prelude::v1`? - = help: there is an enum variant `std::result::Result::Ok`, try using `std::result::Result`? +LL | fn foo() -> std::prelude::v1 { + | ^^^^^^^^^^^^^^^^ +LL | fn foo() -> std::result::Result { + | ^^^^^^^^^^^^^^^^^^^ error[E0412]: cannot find type `Variant3` in this scope --> $DIR/issue-35675.rs:34:13 | LL | fn bar() -> Variant3 { - | ^^^^^^^^ - | | - | not found in this scope - | help: you can try using the variant's enum: `x::Enum` + | ^^^^^^^^ not found in this scope +help: there is an enum variant `x::Enum::Variant3`; try using the variant's enum + | +LL | fn bar() -> x::Enum { + | ^^^^^^^ error[E0573]: expected type, found variant `Some` --> $DIR/issue-35675.rs:38:13 | LL | fn qux() -> Some { | ^^^^ not a type +help: try using the variant's enum | - = help: there is an enum variant `std::prelude::v1::Option::Some`, try using `std::prelude::v1::Option`? - = help: there is an enum variant `std::prelude::v1::Some`, try using `std::prelude::v1`? +LL | fn qux() -> std::prelude::v1::Option { + | ^^^^^^^^^^^^^^^^^^^^^^^^ +LL | fn qux() -> std::prelude::v1 { + | ^^^^^^^^^^^^^^^^ error: aborting due to 7 previous errors diff --git a/src/test/ui/variants/variant-used-as-type.stderr b/src/test/ui/variants/variant-used-as-type.stderr index 972fe8a8a616b..c72729923ef6d 100644 --- a/src/test/ui/variants/variant-used-as-type.stderr +++ b/src/test/ui/variants/variant-used-as-type.stderr @@ -3,28 +3,24 @@ error[E0573]: expected type, found variant `Ty::A` | LL | B(Ty::A), | ^^^^^ not a type -help: you can try using the variant's enum - | -LL | B(Ty), - | ^^ -help: you can try using the variant's enum +help: try using the variant's enum | LL | B(E), | ^ +LL | B(Ty), + | ^^ error[E0573]: expected type, found variant `E::A` --> $DIR/variant-used-as-type.rs:27:6 | LL | impl E::A {} | ^^^^ not a type -help: you can try using the variant's enum - | -LL | impl Ty {} - | ^^ -help: you can try using the variant's enum +help: try using the variant's enum | LL | impl E {} | ^ +LL | impl Ty {} + | ^^ error: aborting due to 2 previous errors From 64ad3e2c423f701b856a6780380a0dbb03f90c22 Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Fri, 23 Nov 2018 12:57:03 -0800 Subject: [PATCH 2/2] adjust enum type instead of variant suggestions for prelude enums The present author regrets not thinking of a more eloquent way to do this. --- src/librustc_resolve/lib.rs | 12 +++++++++- src/librustc_typeck/check/demand.rs | 1 + .../issue-56028-there-is-an-enum-variant.rs | 2 ++ src/test/ui/issues/issue-35675.stderr | 22 +++++++------------ 4 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 8f329283b1a28..e543677ef0621 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -3237,7 +3237,17 @@ impl<'a> Resolver<'a> { err.span_suggestions_with_applicability( span, &msg, - enum_candidates.into_iter().map(|(_variant, enum_ty)| enum_ty), + enum_candidates.into_iter() + .map(|(_variant_path, enum_ty_path)| enum_ty_path) + // variants reëxported in prelude doesn't mean `prelude::v1` is the + // type name! FIXME: is there a more principled way to do this that + // would work for other reëxports? + .filter(|enum_ty_path| enum_ty_path != "std::prelude::v1") + // also say `Option` rather than `std::prelude::v1::Option` + .map(|enum_ty_path| { + // FIXME #56861: DRYer prelude filtering + enum_ty_path.trim_start_matches("std::prelude::v1::").to_owned() + }), Applicability::MachineApplicable, ); } diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index db4b68611c51b..996b57f558cc5 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -123,6 +123,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { let sole_field_ty = sole_field.ty(self.tcx, substs); if self.can_coerce(expr_ty, sole_field_ty) { let variant_path = self.tcx.item_path_str(variant.did); + // FIXME #56861: DRYer prelude filtering Some(variant_path.trim_start_matches("std::prelude::v1::").to_string()) } else { None diff --git a/src/test/ui/did_you_mean/issue-56028-there-is-an-enum-variant.rs b/src/test/ui/did_you_mean/issue-56028-there-is-an-enum-variant.rs index ec41c41141616..264cfa449942c 100644 --- a/src/test/ui/did_you_mean/issue-56028-there-is-an-enum-variant.rs +++ b/src/test/ui/did_you_mean/issue-56028-there-is-an-enum-variant.rs @@ -7,6 +7,8 @@ enum Solidify { Set } enum UnorderedCollection { Set } fn setup() -> Set { Set } +//~^ ERROR cannot find type `Set` in this scope +//~| ERROR cannot find value `Set` in this scope fn main() { setup(); diff --git a/src/test/ui/issues/issue-35675.stderr b/src/test/ui/issues/issue-35675.stderr index 2f07ee7124d07..652e1695a85a6 100644 --- a/src/test/ui/issues/issue-35675.stderr +++ b/src/test/ui/issues/issue-35675.stderr @@ -41,13 +41,10 @@ error[E0573]: expected type, found variant `Ok` --> $DIR/issue-35675.rs:29:13 | LL | fn foo() -> Ok { - | ^^ not a type -help: try using the variant's enum - | -LL | fn foo() -> std::prelude::v1 { - | ^^^^^^^^^^^^^^^^ -LL | fn foo() -> std::result::Result { - | ^^^^^^^^^^^^^^^^^^^ + | ^^ + | | + | not a type + | help: try using the variant's enum: `std::result::Result` error[E0412]: cannot find type `Variant3` in this scope --> $DIR/issue-35675.rs:34:13 @@ -63,13 +60,10 @@ error[E0573]: expected type, found variant `Some` --> $DIR/issue-35675.rs:38:13 | LL | fn qux() -> Some { - | ^^^^ not a type -help: try using the variant's enum - | -LL | fn qux() -> std::prelude::v1::Option { - | ^^^^^^^^^^^^^^^^^^^^^^^^ -LL | fn qux() -> std::prelude::v1 { - | ^^^^^^^^^^^^^^^^ + | ^^^^ + | | + | not a type + | help: try using the variant's enum: `Option` error: aborting due to 7 previous errors