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.

Sign up for GitHub

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

Jump to bottom

Group unused import warnings per import list #37456

Merged
merged 1 commit into from
Nov 11, 2016
Merged

Group unused import warnings per import list #37456

merged 1 commit into from
Nov 11, 2016

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Oct 28, 2016 •
edited
Loading

Given a file

use std::collections::{BinaryHeap, BTreeMap, BTreeSet};

fn main() {}

Show a single warning, instead of three for each unused import:

warning: unused imports, #[warn(unused_imports)] on by default
 --> file2.rs:1:24
  |
1 | use std::collections::{BinaryHeap, BTreeMap, BTreeSet};
  |                        ^^^^^^^^^^  ^^^^^^^^  ^^^^^^^^

Include support for lints pointing at MultilineSpans, instead of just
Spans.

Fixes #16132.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @eddyb (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@eddyb
Copy link
Member

eddyb commented Oct 28, 2016

r? @jonathandturner

@rust-highfive rust-highfive assigned sophiajt and unassigned eddyb Oct 28, 2016
@estebank estebank force-pushed the unused-imports-verbosity branch 5 times, most recently from 1fcc56c to 35540ac Compare October 29, 2016 07:11
@frewsxcv
Copy link
Member

Might want to check https://s3.amazonaws.com/archive.travis-ci.org/jobs/171581156/log.txt for the test failures

@estebank estebank force-pushed the unused-imports-verbosity branch 2 times, most recently from ffbeb3c to c336f15 Compare October 30, 2016 01:41
eddyb
eddyb reviewed Oct 30, 2016
View reviewed changes
src/librustc_resolve/check_unused.rs


struct UnusedImportCheckVisitor<'a, 'b: 'a> {
resolver: &'a mut Resolver<'b>,
/// All the (so far) unused imports, grouped path list
unused_imports: HashMap<ast::NodeId, HashMap<ast::NodeId, Span>>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to use NodeMap<NodeMap<Span>> here (from rustc::util::nodemap).

@estebank estebank force-pushed the unused-imports-verbosity branch 4 times, most recently from c96b6a2 to 8c745d6 Compare October 31, 2016 02:36
@sophiajt
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Oct 31, 2016

📌 Commit 8c745d6 has been approved by jonathandturner

@bors
Copy link
Contributor

bors commented Oct 31, 2016

☔ The latest upstream changes (presumably #37191) made this pull request unmergeable. Please resolve the merge conflicts.

@estebank estebank force-pushed the unused-imports-verbosity branch from 8c745d6 to 2a82785 Compare October 31, 2016 23:37
@estebank
Copy link
Contributor Author

@jonathandturner rebased to fix merge conflict and while I was at it, squashed to clean commit history.

@bors
Copy link
Contributor

bors commented Nov 1, 2016

☔ The latest upstream changes (presumably #37497) made this pull request unmergeable. Please resolve the merge conflicts.

@estebank estebank force-pushed the unused-imports-verbosity branch from 2a82785 to 0c47923 Compare November 1, 2016 18:15
@bors
Copy link
Contributor

bors commented Nov 4, 2016

☔ The latest upstream changes (presumably #37167) made this pull request unmergeable. Please resolve the merge conflicts.

@estebank
Group unused import warnings per path list
a820d99 
Given a file

```rust
use std::collections::{BinaryHeap, BTreeMap, BTreeSet};

fn main() {}
```

Show a single warning, instead of three for each unused import:

```nocode
warning: unused imports, #[warn(unused_imports)] on by default
 --> foo.rs:1:24
  |
1 | use std::collections::{BinaryHeap, BTreeMap, BTreeSet};
  |                        ^^^^^^^^^^  ^^^^^^^^  ^^^^^^^^
```

Include support for lints pointing at `MultilineSpan`s, instead of just
`Span`s.
@estebank estebank force-pushed the unused-imports-verbosity branch from 0c47923 to a820d99 Compare November 9, 2016 15:41
@alexcrichton
Copy link
Member

@bors: r=jonathandturner

(assuming nothing changed, but @jonathandturner feel free to correct me!)

btw I'm pretty stoked about this change!

@bors
Copy link
Contributor

bors commented Nov 10, 2016

📌 Commit a820d99 has been approved by jonathandturner

@brson brson added the relnotes Marks issues that should be documented in the release notes of the next release. label Nov 11, 2016
@brson
Copy link
Contributor

brson commented Nov 11, 2016

Sweet fix for an ancient bug!

@bors
Copy link
Contributor

bors commented Nov 11, 2016

⌛ Testing commit a820d99 with merge 5293b91...

bors added a commit that referenced this pull request Nov 11, 2016
@bors
Auto merge of #37456 - estebank:unused-imports-verbosity, r=jonathand…
5293b91 
…turner

Group unused import warnings per import list

Given a file

``` rust
use std::collections::{BinaryHeap, BTreeMap, BTreeSet};

fn main() {}
```

Show a single warning, instead of three for each unused import:

``` nocode
warning: unused imports, #[warn(unused_imports)] on by default
 --> file2.rs:1:24
  |
1 | use std::collections::{BinaryHeap, BTreeMap, BTreeSet};
  |                        ^^^^^^^^^^  ^^^^^^^^  ^^^^^^^^
```

Include support for lints pointing at `MultilineSpan`s, instead of just
`Span`s.

Fixes #16132.
@bors
Copy link
Contributor

bors commented Nov 11, 2016

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-cargotest, auto-linux-64-cross-freebsd, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-opt-rustbuild, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-opt, auto-mac-64-opt-rustbuild, auto-mac-cross-ios-opt, auto-win-gnu-32-opt, auto-win-gnu-32-opt-rustbuild, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-cargotest, auto-win-msvc-64-opt, auto-win-msvc-64-opt-rustbuild
Approved by: jonathandturner
Pushing 5293b91 to master...

@bors bors merged commit a820d99 into rust-lang:master Nov 11, 2016
@emberian
Copy link
Member

\o/ you're awesome, @estebank

@estebank estebank deleted the unused-imports-verbosity branch November 9, 2023 05:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eddyb eddyb eddyb left review comments

Assignees

@sophiajt sophiajt

Labels
relnotes Marks issues that should be documented in the release notes of the next release.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@estebank @rust-highfive @eddyb @frewsxcv @sophiajt @bors @alexcrichton @brson @emberian