-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Don't suggest changing extern crate w/ alias to use. #60252
Conversation
This commit adds a test that causes a suggestion to replace `extern crate` with `use` when doing so would cause a compliation error, as the new name of the import would not be added to the prelude if a `use` was used.
This commit stops `unused_extern_crates` lints from occuring on `extern crate` statements that alias the crate as the suggestion to change to a `use` statement would result in the aliased name no longer being added to the prelude, thereby causing compilation errors if other imports expected this to be the case.
r? @eddyb (rust_highfive has picked a reviewer for you, use r? to override) |
// expecting it. | ||
if extern_crate.orig_name.is_some() { | ||
continue; | ||
} |
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 would have liked to come up with a more robust check here that would only omit the lint if there was an instance of the extern crate alias being used from the prelude, but I couldn't work out a nice way to achieve this.
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.
Personally that doesn't seem necessary, and somewhat out of scope for this lint -- that seems more like an "unused extern crate" lint rather than an idiom lint.
r? @estebank |
@bors r+ This looks fine to me based on the updated tests, so I'm going to approve this. If necessary we can always revert. |
📌 Commit 8869bc5 has been approved by |
⌛ Testing commit 8869bc5 with merge c6f3bdb8eefd2ad16b4820121b323d6d4e7bce6c... |
Don't suggest changing extern crate w/ alias to use. Fixes #57672.
☀️ Test successful - checks-travis, status-appveyor |
Fixes #57672.