Add hints for the case of confusing enum with its variants

[Menschenkindlein]

A solution for #43871. When one uses an enum in a place that accepts variants (e.g., Option(result) instead of Some(result)), suggest one of this enum's variants.

cc @estebank

[alexcrichton]

r? @estebank

[estebank]

It looks good to me, could you add a couple of ui tests?

[estebank]

nitpick, move indentation one char to the left

[estebank]

nitpick, move indentation one char to the left

[Menschenkindlein]

@estebank Could you, please, give me more guidance? I don't really know where is the place for the ui tests, and how to write them.

[oli-obk]

@Menschenkindlein you can add a new file to src/test/ui and follow the error message that you get when running ./x.py test src/test/ui

[estebank]

@Menschenkindlein You can find a bit more of an explanation in COMPILER_TESTS.md, but what you have to do is pretty much what @oli-obk explained. After you've ran ./x.py test src/test/ui, it'll fail (because you don't have an .stderr file in the same place as your new test .rs file). After you run update-all-references.sh you'll have that .stderr file in place so subsequent runs of ./x.py test src/test/ui should succeed.

[Menschenkindlein]

I added some tests. It looks like it doesn't work in tests. When I created a separate project and tested the feature, it worked. Somehow it manages to suggest an empty list of variants.

By the way, is it a reasonable idea to add a suggestion of a variant in other places where they can be used, e.g. when they are used as constructors?

[kennytm]

Hi @estebank, looks like @Menschenkindlein needs some help from you! 😃

[Menschenkindlein]

I assumed that one Def could designate more than one module, and now my code works. But this assumption looks very strange to me. What should I do?

[estebank]

@Menschenkindlein it is slightly odd if you're finding more than one module per Def, but the code so far looks good. Could you try adding some debug!() statements in find_module and run your local build with RUST_LOG=debug ./stage1/bin/rustc file.rs in order to verify that you're indeed ever receiving more than 1 ImportSuggestion?

[Menschenkindlein]

There indeed were two Option modules. Probably, it was so because of reexports: the same module was found by going from two different paths. But the real bug was that I forgot to "populate" the module in collect_enum_variants. Continuing the search in find_module populated it and unintentionally solved the problem.
Now I added self.populate_module_if_necessary(enum_module) and it works!

[estebank]

Awesome @Menschenkindlein! I really like this feature!

r=me once you've fixed ui/did_you_mean/issue-43871-enum-instead-of-variant.stderr

[00:45:25]  error[E0423]: expected function, found enum `Option`
[00:45:25]    --> $DIR/issue-43871-enum-instead-of-variant.rs:14:13
[00:45:25]     |
[00:45:25]  14 |     let x = Option(1);
[00:45:25]     |             ^^^^^^
[00:45:25]     |
[00:45:25]     = note: did you mean to use one of the following variants?
[00:45:25] -           - `std::prelude::v1::Option::None`
[00:45:25] -           - `std::prelude::v1::Option::Some`
[00:45:25] +           - `std::option::Option::None`
[00:45:25] +           - `std::option::Option::Some`
[00:45:25]  
[00:45:25]  error[E0532]: expected tuple struct/variant, found enum `Option`
[00:45:25]    --> $DIR/issue-43871-enum-instead-of-variant.rs:16:12
[00:45:25]     |
[00:45:25]  16 |     if let Option(_) = x {
[00:45:25]     |            ^^^^^^
[00:45:25]     |
[00:45:25]     = note: did you mean to use one of the following variants?
[00:45:25] -           - `std::prelude::v1::Option::None`
[00:45:25] -           - `std::prelude::v1::Option::Some`
[00:45:25] +           - `std::option::Option::None`
[00:45:25] +           - `std::option::Option::Some`
[00:45:25]  
[00:45:25]  error[E0532]: expected tuple struct/variant, found enum `Example`
[00:45:25]    --> $DIR/issue-43871-enum-instead-of-variant.rs:22:12
[00:45:25]     |
[00:45:25]  22 |     if let Example(_) = y {
[00:45:25]     |            ^^^^^^^
[00:45:25]     |
[00:45:25]     = note: did you mean to use one of the following variants?
[00:45:25]             - `Example::Ex`
[00:45:25]             - `Example::NotEx`
[00:45:25]  
[00:45:25]  error: aborting due to 3 previous errors
[00:45:25]  

[oli-obk]

I don't think they are fixable. These are order issues depending on HashMap randomness.

You can turn them into compile-fail tests, since they don't care about order or the full message.

//~ SUGGESTION Option::Some

Should work

[kennytm]

If both modules can appear, you could add a custom normalization rule that maps std::prelude::v1::Option to std::option::Option (as long as None always appear above Some).

// normalize-stderr: "std::prelude::v1::Option" -> "std::option::Option"

[Menschenkindlein]

I implemented find_module using for_each_child. for_each_child_stable should solve the issue with randomness.

[estebank]

@bors r+

[bors]

📌 Commit a3686c6 has been approved by estebank

[oli-obk]

I'm not sure if that solves it. This error is emitted during resolve, right? So we aren't fully resolved yet, thus the paths in find_module can't necessarily find the correct module. I had a similar issue and solved it by storing the information necessary to produce the diagnostic and emit the error after resolve is finished. There's several of these delayed diagnostics in resolve.

[bors]

⌛ Testing commit a3686c6 with merge 9342661...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: estebank
Pushing 9342661 to master...

[Menschenkindlein]

@estebank Thank you for patiently guiding me to the completion of this pull request!