Skip to content
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

when suggesting to remove a crate, we leave a blank line #51176

Open
nikomatsakis opened this issue May 29, 2018 · 3 comments
Open

when suggesting to remove a crate, we leave a blank line #51176

nikomatsakis opened this issue May 29, 2018 · 3 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. E-help-wanted Call for participation: Help is requested to fix this issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-epoch Working group: Epoch (2018) management

Comments

@nikomatsakis
Copy link
Contributor

As of #51015, the rustfix result for:

#![warn(unused_crates)]
extern crate foo;
fn main() { }

is

#![warn(unused_crates)]

fn main() { }

but not

#![warn(unused_crates)]
fn main() { }
@nikomatsakis nikomatsakis added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-epoch Working group: Epoch (2018) management labels May 29, 2018
@nikomatsakis
Copy link
Contributor Author

Probably the easiest way to fix this is to extend the replacement span by one character if the character after it is \n, somewhere around here:

https://github.com/nikomatsakis/rust/blob/7b7901bf710085a601eed58bf26efb571fe5d25b/src/librustc_typeck/check_unused.rs#L174

@nikomatsakis
Copy link
Contributor Author

Partial diffs here (doesn't work): https://gist.github.com/nikomatsakis/7351f61938624c8e5a9921bbef63add3

One complication: extending to trailing newlines is not enough, we want to be sure that extern is at the start of the line too! i.e., this test:

#![warn(unused_crates)]
/* bar! */ extern crate foo;
fn main() { }

should probably result in

#![warn(unused_crates)]
/* bar! */ 
fn main() { }

not

#![warn(unused_crates)]
/* bar! */ fn main() { }

(Plausibly, though, we might kill trailing whitespace? How much do we care anyway... maybe rustfix wants to do this?)

@estebank estebank added the A-diagnostics Area: Messages for errors, warnings, and lints label May 30, 2018
bors added a commit that referenced this issue Jun 2, 2018
…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
@XAMPPRocky XAMPPRocky added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Oct 2, 2018
@rylev
Copy link
Member

rylev commented Jul 5, 2021

Triage: this is still an issue. Here's the code updated code considering the lint changed since this issues was filed:

#![warn(unused_extern_crates)]
extern crate foo;
fn main() {}

@tmandry tmandry added the E-help-wanted Call for participation: Help is requested to fix this issue. label Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. E-help-wanted Call for participation: Help is requested to fix this issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-epoch Working group: Epoch (2018) management
Projects
None yet
Development

No branches or pull requests

5 participants