Warn unused trait imports

Author: sanxiyn

src/librustc/dep_graph/dep_node.rs

@@ -59,6 +59,7 @@ pub enum DepNode<D: Clone + Debug> {
     TypeckItemBody(D),
     Dropck,
     DropckImpl(D),
+    UnusedTraitCheck,
     CheckConst(D),
     Privacy,
     IntrinsicCheck(D),
@@ -164,6 +165,7 @@ impl<D: Clone + Debug> DepNode<D> {
             CheckEntryFn => Some(CheckEntryFn),
             Variance => Some(Variance),
             Dropck => Some(Dropck),
+            UnusedTraitCheck => Some(UnusedTraitCheck),
             Privacy => Some(Privacy),
             Reachability => Some(Reachability),
             DeadCheck => Some(DeadCheck),

src/librustc/hir/mod.rs

@@ -1637,8 +1637,13 @@ pub type FreevarMap = NodeMap<Vec<Freevar>>;
 
 pub type CaptureModeMap = NodeMap<CaptureClause>;
 
+pub struct TraitCandidate {
+    pub def_id: DefId,
+    pub import_id: Option<NodeId>,
+}
+
 // Trait method resolution
-pub type TraitMap = NodeMap<Vec<DefId>>;
+pub type TraitMap = NodeMap<Vec<TraitCandidate>>;
 
 // Map from the NodeId of a glob import to a list of items which are actually
 // imported.

src/librustc/ty/context.rs

@@ -285,6 +285,8 @@ pub struct TyCtxt<'tcx> {
     // scratch every time.
     pub freevars: RefCell<FreevarMap>,
 
+    pub maybe_unused_trait_imports: NodeSet,
+
     // Records the type of every item.
     pub tcache: RefCell<DepTrackingMap<maps::Tcache<'tcx>>>,
 
@@ -334,6 +336,10 @@ pub struct TyCtxt<'tcx> {
     /// about.
     pub used_mut_nodes: RefCell<NodeSet>,
 
+    /// Set of trait imports actually used in the 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>,
@@ -524,6 +530,7 @@ impl<'tcx> TyCtxt<'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>,
@@ -560,6 +567,7 @@ impl<'tcx> TyCtxt<'tcx> {
             fulfilled_predicates: RefCell::new(fulfilled_predicates),
             map: map,
             freevars: RefCell::new(freevars),
+            maybe_unused_trait_imports: maybe_unused_trait_imports,
             tcache: RefCell::new(DepTrackingMap::new(dep_graph.clone())),
             rcache: RefCell::new(FnvHashMap()),
             tc_cache: RefCell::new(FnvHashMap()),
@@ -574,6 +582,7 @@ impl<'tcx> TyCtxt<'tcx> {
             impl_items: RefCell::new(DepTrackingMap::new(dep_graph.clone())),
             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

@@ -775,6 +775,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,
         glob_map,
@@ -824,6 +825,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
                                named_region_map,
                                hir_map,
                                freevars,
+                               maybe_unused_trait_imports,
                                region_map,
                                lang_items,
                                index,

src/librustc_driver/test.rs

@@ -129,7 +129,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, &ast_map, &def_map.borrow());
     let region_map = region::resolve_crate(&sess, &ast_map);
@@ -140,6 +140,7 @@ fn test_env<F>(source_string: &str,
                                named_region_map.unwrap(),
                                ast_map,
                                freevars,
+                               maybe_unused_trait_imports,
                                region_map,
                                lang_items,
                                index,

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 acutal check in librustc_typeck/check_unused.rs.
 
 use std::ops::{Deref, DerefMut};
 
@@ -55,10 +57,18 @@ impl<'a, 'b, 'tcx> UnusedImportCheckVisitor<'a, 'b, 'tcx> {
     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

@@ -53,8 +53,8 @@ use rustc::hir::def_id::DefId;
 use rustc::hir::pat_util::pat_bindings;
 use rustc::ty;
 use rustc::ty::subst::{ParamSpace, FnSpace, TypeSpace};
-use rustc::hir::{Freevar, FreevarMap, TraitMap, GlobMap};
-use rustc::util::nodemap::{NodeMap, FnvHashMap, FnvHashSet};
+use rustc::hir::{Freevar, FreevarMap, TraitCandidate, TraitMap, GlobMap};
+use rustc::util::nodemap::{NodeMap, NodeSet, FnvHashMap, FnvHashSet};
 
 use syntax::ast::{self, FloatTy};
 use syntax::ast::{CRATE_NODE_ID, Name, NodeId, CrateNum, IntTy, UintTy};
@@ -1091,6 +1091,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>>,
@@ -1181,13 +1182,15 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             export_map: NodeMap(),
             trait_map: NodeMap(),
             module_map: NodeMap(),
-            used_imports: HashSet::new(),
-            used_crates: HashSet::new(),
 
             emit_errors: true,
             make_glob_map: make_glob_map == MakeGlobMap::Yes,
             glob_map: NodeMap(),
 
+            used_imports: HashSet::new(),
+            used_crates: HashSet::new(),
+            maybe_unused_trait_imports: NodeSet(),
+
             callback: None,
             resolved: false,
             privacy_errors: Vec::new(),
@@ -1230,7 +1233,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
     }
 
     #[inline]
-    fn record_use(&mut self, name: Name, ns: Namespace, binding: &'a NameBinding<'a>) {
+    fn record_use(&mut self, name: Name, binding: &'a NameBinding<'a>) {
         // track extern crates for unused_extern_crate lint
         if let Some(DefId { krate, .. }) = binding.module().and_then(ModuleS::def_id) {
             self.used_crates.insert(krate);
@@ -1242,7 +1245,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             _ => return,
         };
 
-        self.used_imports.insert((directive.id, ns));
         if let Some(error) = privacy_error.as_ref() {
             self.privacy_errors.push((**error).clone());
         }
@@ -1553,7 +1555,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             false => module.resolve_name(name, namespace, false),
         }.and_then(|binding| {
             if record_used {
-                self.record_use(name, namespace, binding);
+                if let NameBindingKind::Import { directive, .. } = binding.kind {
+                    self.used_imports.insert((directive.id, namespace));
+                }
+                self.record_use(name, binding);
             }
             Success(binding)
         })
@@ -3215,21 +3220,27 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         }
     }
 
-    fn get_traits_containing_item(&mut self, name: Name) -> Vec<DefId> {
+    fn get_traits_containing_item(&mut self, name: Name) -> Vec<TraitCandidate> {
         debug!("(getting traits containing item) looking for '{}'", name);
 
-        fn add_trait_info(found_traits: &mut Vec<DefId>, trait_def_id: DefId, name: Name) {
+        fn add_trait_info(found_traits: &mut Vec<TraitCandidate>,
+                          trait_def_id: DefId,
+                          import_id: Option<NodeId>,
+                          name: Name) {
             debug!("(adding trait info) found trait {:?} for method '{}'",
                    trait_def_id,
                    name);
-            found_traits.push(trait_def_id);
+            found_traits.push(TraitCandidate {
+                def_id: trait_def_id,
+                import_id: import_id,
+            });
         }
 
         let mut found_traits = Vec::new();
         // Look for the current trait.
         if let Some((trait_def_id, _)) = self.current_trait_ref {
             if self.trait_item_map.contains_key(&(name, trait_def_id)) {
-                add_trait_info(&mut found_traits, trait_def_id, name);
+                add_trait_info(&mut found_traits, trait_def_id, None, name);
             }
         }
 
@@ -3252,9 +3263,15 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                 for binding in traits.as_ref().unwrap().iter() {
                     let trait_def_id = binding.def().unwrap().def_id();
                     if self.trait_item_map.contains_key(&(name, trait_def_id)) {
-                        add_trait_info(&mut found_traits, trait_def_id, name);
+                        let mut import_id = None;
+                        if let NameBindingKind::Import { directive, .. } = binding.kind {
+                            let id = directive.id;
+                            self.maybe_unused_trait_imports.insert(id);
+                            import_id = Some(id);
+                        }
+                        add_trait_info(&mut found_traits, trait_def_id, import_id, name);
                         let trait_name = self.get_trait_name(trait_def_id);
-                        self.record_use(trait_name, TypeNS, binding);
+                        self.record_use(trait_name, binding);
                     }
                 }
             };
@@ -3634,6 +3651,7 @@ fn err_path_resolution() -> PathResolution {
 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 glob_map: Option<GlobMap>,
@@ -3671,6 +3689,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,
         glob_map: if resolver.make_glob_map {

src/librustc_typeck/check/method/mod.rs

@@ -128,6 +128,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 = 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))
 }
 
@@ -339,8 +344,12 @@ pub fn resolve_ufcs<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
 {
     let mode = probe::Mode::Path;
     let pick = probe::probe(fcx, span, mode, method_name, self_ty, expr_id)?;
-    let def = pick.item.def();
 
+    if let Some(import_id) = pick.import_id {
+        fcx.tcx().used_trait_imports.borrow_mut().insert(import_id);
+    }
+
+    let def = pick.item.def();
     if let probe::InherentImplPick = pick.kind {
         if !pick.item.vis().is_accessible_from(fcx.body_id, &fcx.tcx().map) {
             let msg = format!("{} `{}` is private", def.kind_name(), &method_name.as_str());