Warn unused trait imports

Author: sanxiyn

src/librustc/middle/ty/context.rs

@@ -271,6 +271,7 @@ pub struct ctxt<'tcx> {
 
     pub map: ast_map::Map<'tcx>,
     pub freevars: RefCell<FreevarMap>,
+    pub maybe_unused_trait_imports: NodeSet,
     pub tcache: RefCell<DefIdMap<ty::TypeScheme<'tcx>>>,
     pub rcache: RefCell<FnvHashMap<ty::CReaderCacheKey, Ty<'tcx>>>,
     pub tc_cache: RefCell<FnvHashMap<Ty<'tcx>, ty::contents::TypeContents>>,
@@ -306,6 +307,10 @@ pub struct ctxt<'tcx> {
     /// about.
     pub used_mut_nodes: RefCell<NodeSet>,
 
+    /// Set of trait imports actually used in method resolution.
+    /// This is used for warning unused imports.
+    pub used_trait_imports: RefCell<NodeSet>,
+
     /// The set of external nominal types whose implementations have been read.
     /// This is used for lazy resolution of methods.
     pub populated_external_types: RefCell<DefIdSet>,
@@ -475,6 +480,7 @@ impl<'tcx> ctxt<'tcx> {
                                  named_region_map: resolve_lifetime::NamedRegionMap,
                                  map: ast_map::Map<'tcx>,
                                  freevars: FreevarMap,
+                                 maybe_unused_trait_imports: NodeSet,
                                  region_maps: RegionMaps,
                                  lang_items: middle::lang_items::LanguageItems,
                                  stability: stability::Index<'tcx>,
@@ -508,6 +514,7 @@ impl<'tcx> ctxt<'tcx> {
             fulfilled_predicates: RefCell::new(traits::FulfilledPredicates::new()),
             map: map,
             freevars: RefCell::new(freevars),
+            maybe_unused_trait_imports: maybe_unused_trait_imports,
             tcache: RefCell::new(DefIdMap()),
             rcache: RefCell::new(FnvHashMap()),
             tc_cache: RefCell::new(FnvHashMap()),
@@ -522,6 +529,7 @@ impl<'tcx> ctxt<'tcx> {
             impl_items: RefCell::new(DefIdMap()),
             used_unsafe: RefCell::new(NodeSet()),
             used_mut_nodes: RefCell::new(NodeSet()),
+            used_trait_imports: RefCell::new(NodeSet()),
             populated_external_types: RefCell::new(DefIdSet()),
             populated_external_primitive_impls: RefCell::new(DefIdSet()),
             extern_const_statics: RefCell::new(DefIdMap()),

src/librustc_driver/driver.rs

@@ -703,6 +703,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
     let resolve::CrateMap {
         def_map,
         freevars,
+        maybe_unused_trait_imports,
         export_map,
         trait_map,
         external_exports,
@@ -741,6 +742,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
                                named_region_map,
                                ast_map,
                                freevars,
+                               maybe_unused_trait_imports,
                                region_map,
                                lang_items,
                                stability::Index::new(krate),

src/librustc_driver/test.rs

@@ -128,7 +128,7 @@ fn test_env<F>(source_string: &str,
 
     // run just enough stuff to build a tcx:
     let lang_items = lang_items::collect_language_items(&sess, &ast_map);
-    let resolve::CrateMap { def_map, freevars, .. } =
+    let resolve::CrateMap { def_map, freevars, maybe_unused_trait_imports, .. } =
         resolve::resolve_crate(&sess, &ast_map, resolve::MakeGlobMap::No);
     let named_region_map = resolve_lifetime::krate(&sess, krate, &def_map.borrow());
     let region_map = region::resolve_crate(&sess, krate);
@@ -138,6 +138,7 @@ fn test_env<F>(source_string: &str,
                                named_region_map,
                                ast_map,
                                freevars,
+                               maybe_unused_trait_imports,
                                region_map,
                                lang_items,
                                stability::Index::new(krate),

src/librustc_resolve/check_unused.rs

@@ -16,6 +16,8 @@
 // resolve data structures and because it finalises the privacy information for
 // `use` directives.
 //
+// Unused trait imports can't be checked until the method resolution. We save
+// candidates here, and do the actual check in librustc_typeck/check_unused.rs.
 
 use std::ops::{Deref, DerefMut};
 
@@ -104,13 +106,21 @@ impl<'a, 'b, 'tcx> UnusedImportCheckVisitor<'a, 'b, 'tcx> {
         };
     }
 
-    fn check_import(&self, id: ast::NodeId, span: Span) {
+    fn check_import(&mut self, 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;
+            }
             self.session.add_lint(lint::builtin::UNUSED_IMPORTS,
                                   id,
                                   span,
                                   "unused import".to_string());
+        } else {
+            // This trait import is definitely used, in a way other than
+            // method resolution.
+            self.maybe_unused_trait_imports.remove(&id);
         }
     }
 }

src/librustc_resolve/lib.rs

@@ -61,7 +61,7 @@ use rustc::middle::pat_util::pat_bindings;
 use rustc::middle::privacy::*;
 use rustc::middle::subst::{ParamSpace, FnSpace, TypeSpace};
 use rustc::middle::ty::{Freevar, FreevarMap, TraitCandidate, TraitMap, GlobMap};
-use rustc::util::nodemap::{NodeMap, DefIdSet, FnvHashMap};
+use rustc::util::nodemap::{NodeMap, NodeSet, DefIdSet, FnvHashMap};
 
 use syntax::ast;
 use syntax::ast::{CRATE_NODE_ID, Ident, Name, NodeId, CrateNum, TyIs, TyI8, TyI16, TyI32, TyI64};
@@ -1141,6 +1141,7 @@ pub struct Resolver<'a, 'tcx: 'a> {
 
     used_imports: HashSet<(NodeId, Namespace)>,
     used_crates: HashSet<CrateNum>,
+    maybe_unused_trait_imports: NodeSet,
 
     // Callback function for intercepting walks
     callback: Option<Box<Fn(hir_map::Node, &mut bool) -> bool>>,
@@ -1192,14 +1193,16 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             freevars_seen: NodeMap(),
             export_map: NodeMap(),
             trait_map: NodeMap(),
-            used_imports: HashSet::new(),
-            used_crates: HashSet::new(),
             external_exports: DefIdSet(),
 
             emit_errors: true,
             make_glob_map: make_glob_map == MakeGlobMap::Yes,
             glob_map: HashMap::new(),
 
+            used_imports: HashSet::new(),
+            used_crates: HashSet::new(),
+            maybe_unused_trait_imports: NodeSet(),
+
             callback: None,
             resolved: false,
         }
@@ -3669,7 +3672,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                 if self.trait_item_map.contains_key(&(name, did)) {
                     let id = import.type_ns.id;
                     add_trait_info(&mut found_traits, did, Some(id), name);
-                    self.used_imports.insert((id, TypeNS));
+                    self.maybe_unused_trait_imports.insert(id);
                     let trait_name = self.get_trait_name(did);
                     self.record_import_use(id, trait_name);
                     if let Some(DefId{krate: kid, ..}) = target.target_module.def_id() {
@@ -3820,6 +3823,7 @@ fn module_to_string(module: &Module) -> String {
 pub struct CrateMap {
     pub def_map: RefCell<DefMap>,
     pub freevars: FreevarMap,
+    pub maybe_unused_trait_imports: NodeSet,
     pub export_map: ExportMap,
     pub trait_map: TraitMap,
     pub external_exports: ExternalExports,
@@ -3848,6 +3852,7 @@ pub fn resolve_crate<'a, 'tcx>(session: &'a Session,
     CrateMap {
         def_map: resolver.def_map,
         freevars: resolver.freevars,
+        maybe_unused_trait_imports: resolver.maybe_unused_trait_imports,
         export_map: resolver.export_map,
         trait_map: resolver.trait_map,
         external_exports: resolver.external_exports,

src/librustc_typeck/check/method/mod.rs

@@ -125,6 +125,11 @@ pub fn lookup<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
     let mode = probe::Mode::MethodCall;
     let self_ty = fcx.infcx().resolve_type_vars_if_possible(&self_ty);
     let pick = try!(probe::probe(fcx, span, mode, method_name, self_ty, call_expr.id));
+
+    if let Some(import_id) = pick.import_id {
+        fcx.tcx().used_trait_imports.borrow_mut().insert(import_id);
+    }
+
     Ok(confirm::confirm(fcx, span, self_expr, call_expr, self_ty, pick, supplied_method_types))
 }
 
@@ -337,6 +342,11 @@ pub fn resolve_ufcs<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
 {
     let mode = probe::Mode::Path;
     let pick = try!(probe::probe(fcx, span, mode, method_name, self_ty, expr_id));
+
+    if let Some(import_id) = pick.import_id {
+        fcx.tcx().used_trait_imports.borrow_mut().insert(import_id);
+    }
+
     let def_id = pick.item.def_id();
     let mut lp = LastMod(AllPublic);
     if let probe::InherentImplPick = pick.kind {

src/librustc_typeck/check_unused.rs

@@ -0,0 +1,62 @@
+// Copyright 2015 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 lint;
+use middle::ty;
+
+use syntax::ast;
+use syntax::codemap::{Span, DUMMY_SP};
+
+use rustc_front::hir;
+use rustc_front::intravisit::Visitor;
+
+struct UnusedTraitImportVisitor<'a, 'tcx: 'a> {
+    tcx: &'a ty::ctxt<'tcx>,
+}
+
+impl<'a, 'tcx> UnusedTraitImportVisitor<'a, 'tcx> {
+    fn check_import(&self, id: ast::NodeId, span: Span) {
+        if !self.tcx.maybe_unused_trait_imports.contains(&id) {
+            return;
+        }
+        if self.tcx.used_trait_imports.borrow().contains(&id) {
+            return;
+        }
+        self.tcx.sess.add_lint(lint::builtin::UNUSED_IMPORTS,
+                               id,
+                               span,
+                               "unused import".to_string());
+    }
+}
+
+impl<'a, 'tcx, 'v> Visitor<'v> for UnusedTraitImportVisitor<'a, 'tcx> {
+    fn visit_item(&mut self, item: &hir::Item) {
+        if item.vis == hir::Public || item.span == DUMMY_SP {
+            return;
+        }
+        if let hir::ItemUse(ref path) = item.node {
+            match path.node {
+                hir::ViewPathSimple(..) | hir::ViewPathGlob(..) => {
+                    self.check_import(item.id, path.span);
+                }
+                hir::ViewPathList(_, ref path_list) => {
+                    for path_item in path_list {
+                        self.check_import(path_item.node.id(), path_item.span);
+                    }
+                }
+            }
+        }
+    }
+}
+
+pub fn check_crate(tcx: &ty::ctxt) {
+    let mut visitor = UnusedTraitImportVisitor { tcx: tcx };
+    tcx.map.krate().visit_all_items(&mut visitor);
+}

src/librustc_typeck/lib.rs

@@ -120,6 +120,7 @@ use std::cell::RefCell;
 pub mod diagnostics;
 
 pub mod check;
+pub mod check_unused;
 mod rscope;
 mod astconv;
 pub mod collect;
@@ -361,6 +362,7 @@ pub fn check_crate(tcx: &ty::ctxt, trait_map: ty::TraitMap) {
     time(time_passes, "wf checking (new)", ||
         check::check_wf_new(&ccx));
 
+    check_unused::check_crate(tcx);
     check_for_entry_fn(&ccx);
     tcx.sess.abort_if_errors();
 }

src/test/compile-fail/lint-unused-imports.rs

@@ -24,6 +24,8 @@ use test::A;       //~ ERROR unused import
 // Be sure that if we just bring some methods into scope that they're also
 // counted as being used.
 use test::B;
+// But only when actually used: do not get confused by the method with the same name.
+use test::B2; //~ ERROR unused import
 
 // Make sure this import is warned about when at least one of its imported names
 // is unused
@@ -37,6 +39,7 @@ mod test2 {
 mod test {
     pub trait A { fn a(&self) {} }
     pub trait B { fn b(&self) {} }
+    pub trait B2 { fn b(&self) {} }
     pub struct C;
     impl A for C {}
     impl B for C {}