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

unused import warning should have identifier in first line #37376

Closed
pnkfelix opened this issue Oct 24, 2016 · 3 comments
Closed

unused import warning should have identifier in first line #37376

pnkfelix opened this issue Oct 24, 2016 · 3 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@pnkfelix
Copy link
Member

Currently, the unused_imports lint does not include the identifier that is unused in the first line of its warning output.

Instead, it relies on the span system to highlight the identifier

For example, for the following code:

mod a {
    pub fn foo() { }
    pub fn bar() { }
}

mod b {
    use a::{foo, bar};
}

fn main() {
    a::foo(); a::bar();
}

the produced warning is:

warning: unused import, #[warn(unused_imports)] on by default
 --> <anon>:7:13
  |
7 |     use a::{foo, bar};
  |             ^^^

warning: unused import, #[warn(unused_imports)] on by default
 --> <anon>:7:18
  |
7 |     use a::{foo, bar};
  |                  ^^^

This means that the user who is reviewing the warnings needs to scan down four lines after the warning itself before they can see which identifier is being highlighted. In some contexts, the output may have even flowed off the visible portion of the console output.

Some other similar lints do not suffer from this problem because they include the offending identifier in the first line of the warning. For example, the unused_variables and dead_code lints both include the name of the function or variable in the first line of the warning.

@pnkfelix pnkfelix added A-diagnostics Area: Messages for errors, warnings, and lints E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 24, 2016
@dwillie
Copy link

dwillie commented Oct 25, 2016

I wouldn't mind having a look at this, but it's my first time working on this project – I see this issue is labeled easy, would it be a good first task?

@cramertj
Copy link
Member

@dwillie Sorry, I just saw that you commented after I started work on #37394. It's not done, though-- you're welcome to fork my branch and finish it off, and I can close my current PR.

@dwillie
Copy link

dwillie commented Oct 25, 2016

@cramertj No, no, that's fine :) I hadn't started yet anyway

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Oct 26, 2016
…h-id, r=GuillaumeGomez

Add identifier to unused import warnings

Fix rust-lang#37376.

For some reason, though, I'm getting warnings with messages like "76:9: 76:16: unused import: `self::g`" instead of "unused import: `self::g`". @pnkfelix Any ideas what might be causing this?
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 A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants