-
Notifications
You must be signed in to change notification settings - Fork 14k
merge unused-extern-crate and unnecessary-extern-crate lints #51015
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
merge unused-extern-crate and unnecessary-extern-crate lints #51015
Conversation
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
Seems like a fine idea to me! I haven't personally looked too closely at this lint yet for removing |
|
☔ The latest upstream changes (presumably #50879) made this pull request unmergeable. Please resolve the merge conflicts. |
3ee1606 to
8eedba0
Compare
|
ok I added the |
8eedba0 to
295046d
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
295046d to
f2ddd1d
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
r=me when this is passing travis! |
3583475 to
85bf5b0
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
85bf5b0 to
da0e605
Compare
da0e605 to
b0d6b75
Compare
|
This is gonna break clippy cc @Manishearth and @oli-obk — but pretty trivial change. |
|
@bors r=acrichto |
|
📌 Commit b0d6b75 has been approved by |
|
🔒 Merge conflict |
|
☔ The latest upstream changes (presumably #50929) made this pull request unmergeable. Please resolve the merge conflicts. |
b0d6b75 to
d3b30b1
Compare
|
@bors r=alexcrichton |
|
📌 Commit d3b30b1 has been approved by |
|
@nikomatsakis shall this lint also work with unused extern crates from the --extern flag to rustc? |
|
@andjo403 that would be a nice extension; it doesn't now though. Ideally, we'd have some way to point into the |
|
💔 Test failed - status-travis |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
1 similar comment
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
🔒 Merge conflict |
We first collect unused crates into a map and then walk all extern crates in crate order.
7b7901b to
b37cc85
Compare
|
@bors r=alexcrichton |
|
📌 Commit b37cc85 has been approved by |
…diom, r=alexcrichton merge unused-extern-crate and unnecessary-extern-crate lints Extend the `unused_extern_crates` lint to offer a suggestion to remove the extern crate and remove the `unnecessary_extern_crate` lint. Still a few minor issues to fix: - [x] this *does* now leave a blank line... (defer to #51176) - idea: extend the span to be replaced by 1 character if the next character is a `\n` - [x] what about macros? do we need to watch out for that? (defer to #48704) - [x] also it doesn't work for `extern crate foo; fn main() { foo::bar(); }` - this is subtle: the `foo` might be shadowing a glob import too, can't always remove - defer to #51177 - [x] we also don't do the `pub use` rewrite thang (#51013) Spun off from #51010 Fixes #50672 r? @alexcrichton
|
☀️ Test successful - status-appveyor, status-travis |
|
Does |
| fn main() {} | ||
| fn main() { | ||
| unsafe { a::getpid(); } | ||
| unsafe { b::getpid(); } |
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 are these calls testing? As per #49219 (comment), these don't exist on wasm32, is there a suitable replacement for libc?
Or should I just ignore the test on wasm32 (ideally I wouldn't).
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 suspect that they make a, b, etc used, so they report "extern crate is not idiomatic" rather than "extern crate is unused".
AFAIK, libc has universally available names like c_int or c_void, so they can be used instead.
type A = a::c_int; // Makes `a` usedThere 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.
No, wasm32 has an empty libc last I checked. Maybe we can flip alloc and libc, as I expect alloc to not be empty on wasm32?
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.
alloc would do too.
Or a new custom crate in the auxiliary dir.
Extend the
unused_extern_crateslint to offer a suggestion to remove the extern crate and remove theunnecessary_extern_cratelint.Still a few minor issues to fix:
\nextern crate foo; fn main() { foo::bar(); }foomight be shadowing a glob import too, can't always removepub userewrite thang (unused_extern_crateslint does not suggest rewriting crates to auseorpub use#51013)Spun off from #51010
Fixes #50672
r? @alexcrichton