rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.43k stars 12.6k forks source link

Consider skipping 'unused import' lint for traits when errors have occured #71017

Open Aaron1011 opened 4 years ago

Aaron1011 commented 4 years ago

The following code:

trait Foo {
    fn dummy(&self) {}
}
impl Foo for bool {}

mod bar {
    use super::Foo;
    fn contains_error() {
        let a = missing_method();
        a.dummy();
    }
}

gives the following output:

error[E0425]: cannot find function `missing_method` in this scope
 --> src/lib.rs:9:17
  |
9 |         let a = missing_method();
  |                 ^^^^^^^^^^^^^^ not found in this scope

warning: unused import: `super::Foo`
 --> src/lib.rs:7:9
  |
7 |     use super::Foo;
  |         ^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

I think the 'unused import' lint firing is questionable. If the trait in question is being imported only for its methods, then the user will never actually write down the trait name in their code. As a result, introducing a seemingly unrelated compilation error during development may cause a method to no longer be resolved (a.dummy() in this example), resulting in the trait appearing unused.

However, the fact that the trait appears unused here is only because we weren't able to perform method resolution. If the user decides to remove the import, they'll end up with a new compilation error after fixing the original one, requiring them to revert their change.

I think we should suppress the unused_imports for traits if we encountered any errors before method resolution (or maybe before type-checking). The lint will still fire if there are other errors (e.g. borrowcheck) or no errors, but we won't give users this kind of false positive.

RalfJung commented 4 years ago

I agree. I find I often have tons of "unused" warnings when working on code that doesn't compile yet due to various type errors, and many of them go away when the type errors are resolved.