-
Notifications
You must be signed in to change notification settings - Fork 13.8k
Fix duplicated crate keyword in suggested path #115876
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
Fix duplicated crate keyword in suggested path #115876
Conversation
(rustbot has picked a reviewer for you, use r? to override) |
ba9093a
to
1ca042a
Compare
pub mod utils { | ||
pub fn f() { | ||
let x = crate::linux::system::Y; | ||
//~^ ERROR unresolved import |
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 didn't find a way to ensure that in the help message, we have the correct suggested path to prevent regressions from happening. If someone knows how I could do that, it'd be awesome!
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.
You could // run-rustfix
which checks that the applied suggestion works
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.
Oh nice! Doing that right away. Thanks!
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.
Unfortunately it doesn't work because there are multiple suggestions:
thread '[ui] tests/ui/imports/issue-115858-duplicated-crate-diag.rs' panicked at src/tools/compiletest/src/runtest.rs:3830:17:
failed to apply suggestions for "/home/imperio/rust/rust/tests/ui/imports/issue-115858-duplicated-crate-diag.rs" with rustfix: Could not replace range 309...322 in file -- maybe parts of it were already replaced?
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.
Do you know maybe how to check this @estebank ?
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.
@GuillaumeGomez We should change the logic so that the last two suggestions aren't emitted if this one is. rustfix can't deal with multiple exclusive suggestions.
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.
You mean changing the logic behind the compiler error help messages or rustfix? In any case, what do you have in mind?
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.
The compiler is the one responsible to not emit unnecessary suggestions. I'd need to look at the code more closely to see how to signal that the more specific suggestion has already been added to the diagnostic, to not add the later ones.
r? estebank |
r? compiler |
@GuillaumeGomez @rustbot label: +S-inactive |
Fixes #115858.
This only appears starting the 2018 edition because of this.