Auto merge of #37456 - estebank:unused-imports-verbosity, r=jonathandturner

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.
This commit is contained in:
bors 2016-11-11 09:04:17 -08:00 committed by GitHub
commit 5293b913c4
7 changed files with 82 additions and 26 deletions

View File

@ -106,7 +106,7 @@ pub trait IntoEarlyLint {
fn into_early_lint(self, id: LintId) -> EarlyLint;
}
impl<'a> IntoEarlyLint for (Span, &'a str) {
impl<'a, S: Into<MultiSpan>> IntoEarlyLint for (S, &'a str) {
fn into_early_lint(self, id: LintId) -> EarlyLint {
let (span, msg) = self;
let mut diagnostic = Diagnostic::new(errors::Level::Warning, msg);
@ -530,7 +530,10 @@ pub trait LintContext: Sized {
})
}
fn lookup_and_emit(&self, lint: &'static Lint, span: Option<Span>, msg: &str) {
fn lookup_and_emit<S: Into<MultiSpan>>(&self,
lint: &'static Lint,
span: Option<S>,
msg: &str) {
let (level, src) = match self.level_src(lint) {
None => return,
Some(pair) => pair,
@ -553,7 +556,7 @@ pub trait LintContext: Sized {
}
/// Emit a lint at the appropriate level, for a particular span.
fn span_lint(&self, lint: &'static Lint, span: Span, msg: &str) {
fn span_lint<S: Into<MultiSpan>>(&self, lint: &'static Lint, span: S, msg: &str) {
self.lookup_and_emit(lint, Some(span), msg);
}
@ -601,7 +604,7 @@ pub trait LintContext: Sized {
/// Emit a lint at the appropriate level, with no associated span.
fn lint(&self, lint: &'static Lint, msg: &str) {
self.lookup_and_emit(lint, None, msg);
self.lookup_and_emit(lint, None as Option<Span>, msg);
}
/// Merge the lints specified by any lint attributes into the

View File

@ -258,14 +258,15 @@ impl Session {
pub fn unimpl(&self, msg: &str) -> ! {
self.diagnostic().unimpl(msg)
}
pub fn add_lint(&self,
lint: &'static lint::Lint,
id: ast::NodeId,
sp: Span,
msg: String)
pub fn add_lint<S: Into<MultiSpan>>(&self,
lint: &'static lint::Lint,
id: ast::NodeId,
sp: S,
msg: String)
{
self.add_lint_diagnostic(lint, id, (sp, &msg[..]))
}
pub fn add_lint_diagnostic<M>(&self,
lint: &'static lint::Lint,
id: ast::NodeId,

View File

@ -25,13 +25,16 @@ use Resolver;
use Namespace::{TypeNS, ValueNS};
use rustc::lint;
use rustc::util::nodemap::NodeMap;
use syntax::ast::{self, ViewPathGlob, ViewPathList, ViewPathSimple};
use syntax::visit::{self, Visitor};
use syntax_pos::{Span, DUMMY_SP};
use syntax_pos::{Span, MultiSpan, DUMMY_SP};
struct UnusedImportCheckVisitor<'a, 'b: 'a> {
resolver: &'a mut Resolver<'b>,
/// All the (so far) unused imports, grouped path list
unused_imports: NodeMap<NodeMap<Span>>,
}
// Deref and DerefMut impls allow treating UnusedImportCheckVisitor as Resolver.
@ -52,23 +55,21 @@ impl<'a, 'b> DerefMut for UnusedImportCheckVisitor<'a, 'b> {
impl<'a, 'b> UnusedImportCheckVisitor<'a, 'b> {
// We have information about whether `use` (import) directives are actually
// used now. If an import is not used at all, we signal a lint error.
fn check_import(&mut self, id: ast::NodeId, span: Span) {
fn check_import(&mut self, item_id: ast::NodeId, id: ast::NodeId, span: Span) {
if !self.used_imports.contains(&(id, TypeNS)) &&
!self.used_imports.contains(&(id, ValueNS)) {
if self.maybe_unused_trait_imports.contains(&id) {
// Check later.
return;
}
let msg = if let Ok(snippet) = self.session.codemap().span_to_snippet(span) {
format!("unused import: `{}`", snippet)
} else {
"unused import".to_string()
};
self.session.add_lint(lint::builtin::UNUSED_IMPORTS, id, span, msg);
self.unused_imports.entry(item_id).or_insert_with(NodeMap).insert(id, span);
} else {
// This trait import is definitely used, in a way other than
// method resolution.
self.maybe_unused_trait_imports.remove(&id);
if let Some(i) = self.unused_imports.get_mut(&item_id) {
i.remove(&id);
}
}
}
}
@ -98,16 +99,16 @@ impl<'a, 'b> Visitor for UnusedImportCheckVisitor<'a, 'b> {
ast::ItemKind::Use(ref p) => {
match p.node {
ViewPathSimple(..) => {
self.check_import(item.id, p.span)
self.check_import(item.id, item.id, p.span)
}
ViewPathList(_, ref list) => {
for i in list {
self.check_import(i.node.id, i.span);
self.check_import(item.id, i.node.id, i.span);
}
}
ViewPathGlob(_) => {
self.check_import(item.id, p.span)
self.check_import(item.id, item.id, p.span);
}
}
}
@ -117,6 +118,35 @@ impl<'a, 'b> Visitor for UnusedImportCheckVisitor<'a, 'b> {
}
pub fn check_crate(resolver: &mut Resolver, krate: &ast::Crate) {
let mut visitor = UnusedImportCheckVisitor { resolver: resolver };
let mut visitor = UnusedImportCheckVisitor {
resolver: resolver,
unused_imports: NodeMap(),
};
visit::walk_crate(&mut visitor, krate);
for (id, spans) in &visitor.unused_imports {
let len = spans.len();
let mut spans = spans.values().map(|s| *s).collect::<Vec<Span>>();
spans.sort();
let ms = MultiSpan::from_spans(spans.clone());
let mut span_snippets = spans.iter()
.filter_map(|s| {
match visitor.session.codemap().span_to_snippet(*s) {
Ok(s) => Some(format!("`{}`", s)),
_ => None,
}
}).collect::<Vec<String>>();
span_snippets.sort();
let msg = format!("unused import{}{}",
if len > 1 { "s" } else { "" },
if span_snippets.len() > 0 {
format!(": {}", span_snippets.join(", "))
} else {
String::new()
});
visitor.session.add_lint(lint::builtin::UNUSED_IMPORTS,
*id,
ms,
msg);
}
}

View File

@ -52,7 +52,7 @@ pub type FileName = String;
/// able to use many of the functions on spans in codemap and you cannot assume
/// that the length of the span = hi - lo; there may be space in the BytePos
/// range between files.
#[derive(Clone, Copy, Hash, PartialEq, Eq)]
#[derive(Clone, Copy, Hash, PartialEq, Eq, Ord, PartialOrd)]
pub struct Span {
pub lo: BytePos,
pub hi: BytePos,
@ -67,7 +67,7 @@ pub struct Span {
/// the error, and would be rendered with `^^^`.
/// - they can have a *label*. In this case, the label is written next
/// to the mark in the snippet when we render.
#[derive(Clone, Debug, PartialEq, Eq)]
#[derive(Clone, Debug, Hash, PartialEq, Eq)]
pub struct MultiSpan {
primary_spans: Vec<Span>,
span_labels: Vec<(Span, String)>,
@ -254,7 +254,7 @@ impl From<Span> for MultiSpan {
}
}
#[derive(PartialEq, Eq, Clone, Debug, Hash, RustcEncodable, RustcDecodable, Copy)]
#[derive(PartialEq, Eq, Clone, Debug, Hash, RustcEncodable, RustcDecodable, Copy, Ord, PartialOrd)]
pub struct ExpnId(pub u32);
pub const NO_EXPANSION: ExpnId = ExpnId(!0);

View File

@ -17,8 +17,9 @@ use std::mem::*; // shouldn't get errors for not using
// everything imported
// Should get errors for both 'Some' and 'None'
use std::option::Option::{Some, None}; //~ ERROR unused import: `Some`
//~^ ERROR unused import: `None`
use std::option::Option::{Some, None};
//~^ ERROR unused imports: `None`, `Some`
//~| ERROR unused imports: `None`, `Some`
use test::A; //~ ERROR unused import: `test::A`
// Be sure that if we just bring some methods into scope that they're also

View File

@ -0,0 +1,15 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
use std::cmp::{Eq, Ord, min, PartialEq, PartialOrd};
fn main() {
let _ = min(1, 2);
}

View File

@ -0,0 +1,6 @@
warning: unused imports: `Eq`, `Ord`, `PartialEq`, `PartialOrd`, #[warn(unused_imports)] on by default
--> $DIR/multispan-import-lint.rs:11:16
|
11 | use std::cmp::{Eq, Ord, min, PartialEq, PartialOrd};
| ^^ ^^^ ^^^^^^^^^ ^^^^^^^^^^