-
Notifications
You must be signed in to change notification settings - Fork 13k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
suggest candidates for unresolved import #102876
Conversation
r? @fee1-dead (rust-highfive has picked a reviewer for you, use r? to override) |
@bors r+ |
…1-dead suggest candidates for unresolved import Currently we prompt suggestion of candidates(help notes of `use xxx::yyy`) for names which cannot be resolved, but we don't do that for import statements themselves that couldn't be resolved. It seems reasonable to add candidate help information for these statements as well. Fixes rust-lang#102711
…1-dead suggest candidates for unresolved import Currently we prompt suggestion of candidates(help notes of `use xxx::yyy`) for names which cannot be resolved, but we don't do that for import statements themselves that couldn't be resolved. It seems reasonable to add candidate help information for these statements as well. Fixes rust-lang#102711
| | ||
help: a similar name exists in the module | ||
| | ||
LL | use test as y; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's this suggesting? Turning test
into test
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is the original problem of the test module and not a problem caused by this PR
IsPattern::No, | ||
IsImport::Yes, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these mutually exclusive? If so, they probably should be unified into one enum
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two parameters cannot be Yes
at the same time, but can be No
at the same time, or one of them is Yes
, so if they are unified into one enumeration, at least three variants are required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that's what I meant -- the Yes+Yes state being invalid would be better represented as an enum.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense. I think we can rename IsPattern
enum to Mode
and add a new variant in it. Since this PR is already in a rollup, should I just modify this PR or submit a new one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A follow-up is fine.
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#101360 (Point out incompatible closure bounds) - rust-lang#101789 (`let`'s not needed in struct field definitions) - rust-lang#102846 (update to syn-1.0.102) - rust-lang#102871 (rustdoc: clean up overly complex `.trait-impl` CSS selectors) - rust-lang#102876 (suggest candidates for unresolved import) - rust-lang#102888 (Improve rustdoc-gui search-color test) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
…piler-errors unify `IsPattern` and `IsImport` enum in `show_candidates` Follow-up of rust-lang#102876 A binding cannot appear in both pattern and import at the same time, so it makes sense to unify them r? `@compiler-errors`
Currently we prompt suggestion of candidates(help notes of
use xxx::yyy
) for names which cannot be resolved, but we don't do that for import statements themselves that couldn't be resolved. It seems reasonable to add candidate help information for these statements as well.Fixes #102711