Skip to content

Commit

Permalink
Rollup merge of rust-lang#56188 - zackmdavis:if_i_may_suggest, r=davi…
Browse files Browse the repository at this point in the history
…dtwco

enum type instead of variant suggestion unification

Fixes rust-lang#56028.

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 rust-lang#56028.

In the matter of message wording, "you can" is redundant (and perhaps too informal); prefer the imperative.

In a second commit, we do some unprincipled special-casing to correct and beautify suggestions for `Option` and `Result` (where the existing code was being confused by their being reexported in the prelude).

r? @davidtwco
  • Loading branch information
Centril authored Dec 23, 2018
2 parents ddab10a + 64ad3e2 commit 5e58651
Show file tree
Hide file tree
Showing 10 changed files with 133 additions and 58 deletions.
56 changes: 38 additions & 18 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3213,23 +3213,43 @@ 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::<Vec<_>>();
.map(|suggestion| {
import_candidate_to_enum_paths(&suggestion)
}).collect::<Vec<_>>();
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_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,
);
}
}
if path.len() == 1 && this.self_type_is_available(span) {
Expand Down Expand Up @@ -5128,8 +5148,8 @@ fn path_names_to_string(path: &Path) -> String {
.collect::<Vec<_>>())
}

/// 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);

Expand All @@ -5140,7 +5160,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)
}


Expand Down
1 change: 1 addition & 0 deletions src/librustc_typeck/check/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 15 additions & 0 deletions src/test/ui/did_you_mean/issue-56028-there-is-an-enum-variant.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
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 }
//~^ ERROR cannot find type `Set` in this scope
//~| ERROR cannot find value `Set` in this scope

fn main() {
setup();
}
Original file line number Diff line number Diff line change
@@ -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`.
2 changes: 1 addition & 1 deletion src/test/ui/enum/enum-variant-type-2.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/issues/issue-17546.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ LL | fn new() -> NoResult<MyEnum, String> {
| --------^^^^^^^^^^^^^^^^
| |
| 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
Expand Down Expand Up @@ -48,7 +48,7 @@ LL | fn newer() -> NoResult<foo::MyEnum, String> {
| --------^^^^^^^^^^^^^^^^^^^^^
| |
| 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

Expand Down
7 changes: 4 additions & 3 deletions src/test/ui/issues/issue-30535.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
18 changes: 10 additions & 8 deletions src/test/ui/issues/issue-35075.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,21 @@ error[E0412]: cannot find type `Foo` in this scope
--> $DIR/issue-35075.rs:12:12
|
LL | inner: Foo<T> //~ 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<T>) //~ 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

Expand Down
34 changes: 18 additions & 16 deletions src/test/ui/issues/issue-35675.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -40,28 +41,29 @@ error[E0573]: expected type, found variant `Ok`
--> $DIR/issue-35675.rs:29:13
|
LL | fn foo() -> Ok {
| ^^ not a type
|
= 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`?
| ^^
| |
| 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
|
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: 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`?
| ^^^^
| |
| not a type
| help: try using the variant's enum: `Option`

error: aborting due to 7 previous errors

Expand Down
16 changes: 6 additions & 10 deletions src/test/ui/variants/variant-used-as-type.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 5e58651

Please sign in to comment.