Use MultiSpan for unused imports

src/librustc/lint/context.rs

@@ -41,7 +41,7 @@ use std::default::Default as StdDefault;
 use std::mem;
 use syntax::ast_util::{self, IdVisitingOperation};
 use syntax::attr::{self, AttrMetaMethods};
-use syntax::codemap::Span;
+use syntax::codemap::{Span, MultiSpan};
 use syntax::errors::DiagnosticBuilder;
 use syntax::parse::token::InternedString;
 use syntax::ast;
@@ -398,20 +398,20 @@ pub fn gather_attr(attr: &ast::Attribute)
 /// in trans that run after the main lint pass is finished. Most
 /// lints elsewhere in the compiler should call
 /// `Session::add_lint()` instead.
-pub fn raw_emit_lint(sess: &Session,
+pub fn raw_emit_lint<S: Into<MultiSpan>>(sess: &Session,
                      lints: &LintStore,
                      lint: &'static Lint,
                      lvlsrc: LevelSource,
-                     span: Option<Span>,
+                     span: Option<S>,
                      msg: &str) {
     raw_struct_lint(sess, lints, lint, lvlsrc, span, msg).emit();
 }
 
-pub fn raw_struct_lint<'a>(sess: &'a Session,
+pub fn raw_struct_lint<'a, S: Into<MultiSpan>>(sess: &'a Session,
                            lints: &LintStore,
                            lint: &'static Lint,
                            lvlsrc: LevelSource,
-                           span: Option<Span>,
+                           span: Option<S>,
                            msg: &str)
                            -> DiagnosticBuilder<'a> {
     let (mut level, source) = lvlsrc;
@@ -442,10 +442,15 @@ pub fn raw_struct_lint<'a>(sess: &'a Session,
     // For purposes of printing, we can treat forbid as deny.
     if level == Forbid { level = Deny; }
 
-    let mut err = match (level, span) {
-        (Warn, Some(sp)) => sess.struct_span_warn(sp, &msg[..]),
+    let span = span.map(|s| s.into());
+    let mut err = match (level, span.clone()) {
+        (Warn, Some(sp)) => {
+            sess.struct_span_warn(sp, &msg[..])
+        },
         (Warn, None)     => sess.struct_warn(&msg[..]),
-        (Deny, Some(sp)) => sess.struct_span_err(sp, &msg[..]),
+        (Deny, Some(sp)) => {
+            sess.struct_span_err(sp, &msg[..])
+        },
         (Deny, None)     => sess.struct_err(&msg[..]),
         _ => sess.bug("impossible level in raw_emit_lint"),
     };
@@ -458,7 +463,7 @@ pub fn raw_struct_lint<'a>(sess: &'a Session,
         let citation = format!("for more information, see {}",
                                future_incompatible.reference);
         if let Some(sp) = span {
-            err.fileline_warn(sp, &explanation);
+            err.fileline_warn(sp.clone(), &explanation);
             err.fileline_note(sp, &citation);
         } else {
             err.warn(&explanation);
@@ -497,7 +502,7 @@ pub trait LintContext: Sized {
         })
     }
 
-    fn lookup_and_emit(&self, lint: &'static Lint, span: Option<Span>, msg: &str) {
+    fn lookup_and_emit(&self, lint: &'static Lint, span: Option<MultiSpan>, msg: &str) {
         let (level, src) = match self.level_src(lint) {
             None => return,
             Some(pair) => pair,
@@ -520,8 +525,8 @@ 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) {
-        self.lookup_and_emit(lint, Some(span), msg);
+    fn span_lint<S: Into<MultiSpan>>(&self, lint: &'static Lint, span: S, msg: &str) {
+        self.lookup_and_emit(lint, Some(span.into()), msg);
     }
 
     fn struct_span_lint(&self,
@@ -1258,8 +1263,8 @@ pub fn check_crate(tcx: &ty::ctxt, access_levels: &AccessLevels) {
     // If we missed any lints added to the session, then there's a bug somewhere
     // in the iteration code.
     for (id, v) in tcx.sess.lints.borrow().iter() {
-        for &(lint, span, ref msg) in v {
-            tcx.sess.span_bug(span,
+        for &(lint, ref span, ref msg) in v {
+            tcx.sess.span_bug(span.clone(),
                               &format!("unprocessed lint {} at {}: {}",
                                        lint.as_str(), tcx.map.node_to_string(*id), *msg))
         }
@@ -1292,8 +1297,8 @@ pub fn check_ast_crate(sess: &Session, krate: &ast::Crate) {
     // If we missed any lints added to the session, then there's a bug somewhere
     // in the iteration code.
     for (_, v) in sess.lints.borrow().iter() {
-        for &(lint, span, ref msg) in v {
-            sess.span_bug(span,
+        for &(lint, ref span, ref msg) in v {
+            sess.span_bug(span.clone(),
                           &format!("unprocessed lint {}: {}",
                                    lint.as_str(), *msg))
         }

src/librustc/session/mod.rs

@@ -57,7 +57,7 @@ pub struct Session {
     pub local_crate_source_file: Option<PathBuf>,
     pub working_dir: PathBuf,
     pub lint_store: RefCell<lint::LintStore>,
-    pub lints: RefCell<NodeMap<Vec<(lint::LintId, Span, String)>>>,
+    pub lints: RefCell<NodeMap<Vec<(lint::LintId, MultiSpan, String)>>>,
     pub plugin_llvm_passes: RefCell<Vec<String>>,
     pub plugin_attributes: RefCell<Vec<(String, AttributeType)>>,
     pub crate_types: RefCell<Vec<config::CrateType>>,
@@ -236,18 +236,18 @@ impl Session {
     pub fn unimpl(&self, msg: &str) -> ! {
         self.diagnostic().unimpl(msg)
     }
-    pub fn add_lint(&self,
+    pub fn add_lint<S: Into<MultiSpan>>(&self,
                     lint: &'static lint::Lint,
                     id: ast::NodeId,
-                    sp: Span,
+                    sp: S,
                     msg: String) {
         let lint_id = lint::LintId::of(lint);
         let mut lints = self.lints.borrow_mut();
         match lints.get_mut(&id) {
-            Some(arr) => { arr.push((lint_id, sp, msg)); return; }
+            Some(arr) => { arr.push((lint_id, sp.into(), msg)); return; }
             None => {}
         }
-        lints.insert(id, vec!((lint_id, sp, msg)));
+        lints.insert(id, vec!((lint_id, sp.into(), msg)));
     }
     pub fn reserve_node_ids(&self, count: ast::NodeId) -> ast::NodeId {
         let id = self.next_node_id.get();

src/librustc_resolve/check_unused.rs

@@ -18,21 +18,23 @@
 //
 
 use std::ops::{Deref, DerefMut};
+use std::collections::HashMap;
 
 use Resolver;
 use Namespace::{TypeNS, ValueNS};
 
 use rustc::lint;
 use rustc::middle::privacy::{DependsOn, LastImport, Used, Unused};
 use syntax::ast;
-use syntax::codemap::{Span, DUMMY_SP};
+use syntax::codemap::{Span, MultiSpan, DUMMY_SP};
 
 use rustc_front::hir;
 use rustc_front::hir::{ViewPathGlob, ViewPathList, ViewPathSimple};
 use rustc_front::intravisit::Visitor;
 
 struct UnusedImportCheckVisitor<'a, 'b: 'a, 'tcx: 'b> {
     resolver: &'a mut Resolver<'b, 'tcx>,
+    unused_imports: HashMap<ast::NodeId, Vec<Span>>,
 }
 
 // Deref and DerefMut impls allow treating UnusedImportCheckVisitor as Resolver.
@@ -58,16 +60,13 @@ impl<'a, 'b, 'tcx> UnusedImportCheckVisitor<'a, 'b, 'tcx> {
     // only check imports and namespaces which are used. In particular, this
     // means that if an import could name either a public or private item, we
     // will check the correct thing, dependent on how the import is used.
-    fn finalize_import(&mut self, id: ast::NodeId, span: Span) {
+    fn finalize_import(&mut self, item_id: ast::NodeId, id: ast::NodeId, span: Span) {
         debug!("finalizing import uses for {:?}",
                self.session.codemap().span_to_snippet(span));
 
         if !self.used_imports.contains(&(id, TypeNS)) &&
            !self.used_imports.contains(&(id, ValueNS)) {
-            self.session.add_lint(lint::builtin::UNUSED_IMPORTS,
-                                  id,
-                                  span,
-                                  "unused import".to_string());
+               self.unused_imports.entry(item_id).or_insert_with(Vec::new).push(span);
         }
 
         let mut def_map = self.def_map.borrow_mut();
@@ -135,22 +134,19 @@ impl<'a, 'b, 'v, 'tcx> Visitor<'v> for UnusedImportCheckVisitor<'a, 'b, 'tcx> {
             hir::ItemUse(ref p) => {
                 match p.node {
                     ViewPathSimple(_, _) => {
-                        self.finalize_import(item.id, p.span)
+                        self.finalize_import(item.id, item.id, p.span)
                     }
 
                     ViewPathList(_, ref list) => {
                         for i in list {
-                            self.finalize_import(i.node.id(), i.span);
+                            self.finalize_import(item.id, i.node.id(), i.span);
                         }
                     }
                     ViewPathGlob(_) => {
                         if !self.used_imports.contains(&(item.id, TypeNS)) &&
                            !self.used_imports.contains(&(item.id, ValueNS)) {
-                            self.session
-                                .add_lint(lint::builtin::UNUSED_IMPORTS,
-                                          item.id,
-                                          p.span,
-                                          "unused import".to_string());
+                           self.unused_imports.entry(item.id).or_insert_with(Vec::new)
+                                                             .push(item.span);
                         }
                     }
                 }
@@ -161,6 +157,20 @@ impl<'a, 'b, 'v, 'tcx> Visitor<'v> for UnusedImportCheckVisitor<'a, 'b, 'tcx> {
 }
 
 pub fn check_crate(resolver: &mut Resolver, krate: &hir::Crate) {
-    let mut visitor = UnusedImportCheckVisitor { resolver: resolver };
+    let mut visitor = UnusedImportCheckVisitor { resolver: resolver,
+                                                 unused_imports: HashMap::new() };
     krate.visit_all_items(&mut visitor);
+
+    for (id, spans) in &visitor.unused_imports {
+        let ms = spans.iter().fold(MultiSpan::new(), |mut acc, &sp| { acc.push_merge(sp); acc });
+
+        visitor.session.add_lint(lint::builtin::UNUSED_IMPORTS,
+                                 *id,
+                                 ms,
+                                 if spans.len() > 1  {
+                                     "unused imports".to_string()
+                                 } else {
+                                     "unused import".to_string()
+                                 });
+    }
 }

src/libsyntax/codemap.rs

@@ -364,6 +364,14 @@ impl MultiSpan {
     }
 }
 
+impl fmt::Debug for MultiSpan {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f,
+               "MultiSpan ({})",
+               self.spans.iter().map(|s| format!("{:?}", s)).collect::<Vec<_>>().join(", "))
+    }
+}
+
 impl From<Span> for MultiSpan {
     fn from(span: Span) -> MultiSpan {
         MultiSpan { spans: vec![span] }