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

Group unused import warnings per import list #37456

Merged
merged 1 commit into from
Nov 11, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions src/librustc/lint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand All @@ -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);
}

Expand Down Expand Up @@ -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
Expand Down
11 changes: 6 additions & 5 deletions src/librustc/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
54 changes: 42 additions & 12 deletions src/librustc_resolve/check_unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
}
}
}
}
Expand Down Expand Up @@ -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);
}
}
}
Expand All @@ -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);
}
}
6 changes: 3 additions & 3 deletions src/libsyntax_pos/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)>,
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 3 additions & 2 deletions src/test/compile-fail/lint-unused-imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 15 additions & 0 deletions src/test/ui/span/multispan-import-lint.rs
Original file line number Diff line number Diff line change
@@ -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);
}
6 changes: 6 additions & 0 deletions src/test/ui/span/multispan-import-lint.stderr
Original file line number Diff line number Diff line change
@@ -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};
| ^^ ^^^ ^^^^^^^^^ ^^^^^^^^^^