diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index 536c739bf161..8732df143715 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -59,6 +59,7 @@ pub enum DepNode { TypeckItemBody(D), Dropck, DropckImpl(D), + UnusedTraitCheck, CheckConst(D), Privacy, IntrinsicCheck(D), @@ -163,6 +164,7 @@ impl DepNode { CheckEntryFn => Some(CheckEntryFn), Variance => Some(Variance), Dropck => Some(Dropck), + UnusedTraitCheck => Some(UnusedTraitCheck), Privacy => Some(Privacy), Reachability => Some(Reachability), DeadCheck => Some(DeadCheck), diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 7eaace91ae9b..68e3e742d031 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -1639,8 +1639,13 @@ pub type FreevarMap = NodeMap>; pub type CaptureModeMap = NodeMap; +pub struct TraitCandidate { + pub def_id: DefId, + pub import_id: Option, +} + // Trait method resolution -pub type TraitMap = NodeMap>; +pub type TraitMap = NodeMap>; // Map from the NodeId of a glob import to a list of items which are actually // imported. diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 5bde6df5123d..c7ae038eb7ad 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -289,6 +289,8 @@ pub struct TyCtxt<'tcx> { // scratch every time. pub freevars: RefCell, + pub maybe_unused_trait_imports: NodeSet, + // Records the type of every item. pub tcache: RefCell>>, @@ -338,6 +340,10 @@ pub struct TyCtxt<'tcx> { /// about. pub used_mut_nodes: RefCell, + /// Set of trait imports actually used in the method resolution. + /// This is used for warning unused imports. + pub used_trait_imports: RefCell, + /// The set of external nominal types whose implementations have been read. /// This is used for lazy resolution of methods. pub populated_external_types: RefCell, @@ -543,6 +549,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>, @@ -581,6 +588,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()), @@ -595,6 +603,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()), diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 2b56366f1c69..53b914936f5c 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -795,6 +795,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, @@ -844,6 +845,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, diff --git a/src/librustc_driver/test.rs b/src/librustc_driver/test.rs index 37f7b31b69cf..53c0e1f79927 100644 --- a/src/librustc_driver/test.rs +++ b/src/librustc_driver/test.rs @@ -132,7 +132,7 @@ fn test_env(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); @@ -143,6 +143,7 @@ fn test_env(source_string: &str, named_region_map.unwrap(), ast_map, freevars, + maybe_unused_trait_imports, region_map, lang_items, index, diff --git a/src/librustc_resolve/check_unused.rs b/src/librustc_resolve/check_unused.rs index 9135b656736a..e213a51fb384 100644 --- a/src/librustc_resolve/check_unused.rs +++ b/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); } } } diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 9ef927b66106..2e2b40fec4b9 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -52,8 +52,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}; @@ -1042,6 +1042,7 @@ pub struct Resolver<'a, 'tcx: 'a> { used_imports: HashSet<(NodeId, Namespace)>, used_crates: HashSet, + maybe_unused_trait_imports: NodeSet, privacy_errors: Vec>, @@ -1137,13 +1138,15 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { export_map: NodeMap(), trait_map: NodeMap(), module_map: module_map, - 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(), + privacy_errors: Vec::new(), arenas: arenas, @@ -1177,7 +1180,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); @@ -1189,7 +1192,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()); } @@ -1492,7 +1494,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) }) @@ -3094,21 +3099,27 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { } } - fn get_traits_containing_item(&mut self, name: Name) -> Vec { + fn get_traits_containing_item(&mut self, name: Name) -> Vec { debug!("(getting traits containing item) looking for '{}'", name); - fn add_trait_info(found_traits: &mut Vec, trait_def_id: DefId, name: Name) { + fn add_trait_info(found_traits: &mut Vec, + trait_def_id: DefId, + import_id: Option, + 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); } } @@ -3131,8 +3142,14 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { for &(trait_name, 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); - self.record_use(trait_name, TypeNS, binding); + 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); + self.record_use(trait_name, binding); } } }; @@ -3506,6 +3523,7 @@ fn err_path_resolution() -> PathResolution { pub struct CrateMap { pub def_map: RefCell, pub freevars: FreevarMap, + pub maybe_unused_trait_imports: NodeSet, pub export_map: ExportMap, pub trait_map: TraitMap, pub glob_map: Option, @@ -3543,6 +3561,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 { diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs index 367cac685098..26d1f50f8c54 100644 --- a/src/librustc_typeck/check/method/mod.rs +++ b/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()); diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index 2defbf0d33e0..4716bab71bfb 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -42,6 +42,7 @@ struct ProbeContext<'a, 'tcx:'a> { inherent_candidates: Vec>, extension_candidates: Vec>, impl_dups: HashSet, + import_id: Option, /// Collects near misses when the candidate functions are missing a `self` keyword and is only /// used for error reporting @@ -67,6 +68,7 @@ struct Candidate<'tcx> { xform_self_ty: Ty<'tcx>, item: ty::ImplOrTraitItem<'tcx>, kind: CandidateKind<'tcx>, + import_id: Option, } #[derive(Debug)] @@ -84,6 +86,7 @@ enum CandidateKind<'tcx> { pub struct Pick<'tcx> { pub item: ty::ImplOrTraitItem<'tcx>, pub kind: PickKind<'tcx>, + pub import_id: Option, // Indicates that the source expression should be autoderef'd N times // @@ -247,6 +250,7 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> { inherent_candidates: Vec::new(), extension_candidates: Vec::new(), impl_dups: HashSet::new(), + import_id: None, steps: Rc::new(steps), opt_simplified_steps: opt_simplified_steps, static_candidates: Vec::new(), @@ -435,7 +439,8 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> { self.inherent_candidates.push(Candidate { xform_self_ty: xform_self_ty, item: item, - kind: InherentImplCandidate(impl_substs, obligations) + kind: InherentImplCandidate(impl_substs, obligations), + import_id: self.import_id, }); } @@ -463,7 +468,8 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> { this.inherent_candidates.push(Candidate { xform_self_ty: xform_self_ty, item: item, - kind: ObjectCandidate + kind: ObjectCandidate, + import_id: this.import_id, }); }); } @@ -533,7 +539,8 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> { this.inherent_candidates.push(Candidate { xform_self_ty: xform_self_ty, item: item, - kind: WhereClauseCandidate(poly_trait_ref) + kind: WhereClauseCandidate(poly_trait_ref), + import_id: this.import_id, }); }); } @@ -577,9 +584,13 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> { let mut duplicates = HashSet::new(); let opt_applicable_traits = self.fcx.ccx.trait_map.get(&expr_id); if let Some(applicable_traits) = opt_applicable_traits { - for &trait_did in applicable_traits { + for trait_candidate in applicable_traits { + let trait_did = trait_candidate.def_id; if duplicates.insert(trait_did) { - self.assemble_extension_candidates_for_trait(trait_did)?; + self.import_id = trait_candidate.import_id; + let result = self.assemble_extension_candidates_for_trait(trait_did); + self.import_id = None; + result?; } } } @@ -679,7 +690,8 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> { self.extension_candidates.push(Candidate { xform_self_ty: xform_self_ty, item: item.clone(), - kind: ExtensionImplCandidate(impl_def_id, impl_substs, obligations) + kind: ExtensionImplCandidate(impl_def_id, impl_substs, obligations), + import_id: self.import_id, }); }); } @@ -754,7 +766,8 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> { self.inherent_candidates.push(Candidate { xform_self_ty: xform_self_ty, item: item.clone(), - kind: TraitCandidate + kind: TraitCandidate, + import_id: self.import_id, }); } @@ -811,7 +824,8 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> { self.extension_candidates.push(Candidate { xform_self_ty: xform_self_ty, item: item.clone(), - kind: TraitCandidate + kind: TraitCandidate, + import_id: self.import_id, }); } } @@ -842,7 +856,8 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> { self.extension_candidates.push(Candidate { xform_self_ty: xform_self_ty, item: item.clone(), - kind: WhereClauseCandidate(poly_bound) + kind: WhereClauseCandidate(poly_bound), + import_id: self.import_id, }); } } @@ -1140,6 +1155,7 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> { Some(Pick { item: probes[0].item.clone(), kind: TraitPick, + import_id: probes[0].import_id, autoderefs: 0, autoref: None, unsize: None @@ -1345,6 +1361,7 @@ impl<'tcx> Candidate<'tcx> { WhereClausePick(trait_ref.clone()) } }, + import_id: self.import_id, autoderefs: 0, autoref: None, unsize: None diff --git a/src/librustc_typeck/check_unused.rs b/src/librustc_typeck/check_unused.rs new file mode 100644 index 000000000000..3c594ebdf0bf --- /dev/null +++ b/src/librustc_typeck/check_unused.rs @@ -0,0 +1,64 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use lint; +use rustc::dep_graph::DepNode; +use rustc::ty::TyCtxt; + +use syntax::ast; +use syntax::codemap::{Span, DUMMY_SP}; + +use rustc::hir; +use rustc::hir::intravisit::Visitor; + +struct UnusedTraitImportVisitor<'a, 'tcx: 'a> { + tcx: &'a TyCtxt<'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: &TyCtxt) { + let _task = tcx.dep_graph.in_task(DepNode::UnusedTraitCheck); + let mut visitor = UnusedTraitImportVisitor { tcx: tcx }; + tcx.map.krate().visit_all_items(&mut visitor); +} diff --git a/src/librustc_typeck/lib.rs b/src/librustc_typeck/lib.rs index c51304120a89..ed393b54f0e2 100644 --- a/src/librustc_typeck/lib.rs +++ b/src/librustc_typeck/lib.rs @@ -122,6 +122,7 @@ use std::cell::RefCell; pub mod diagnostics; pub mod check; +pub mod check_unused; mod rscope; mod astconv; pub mod collect; @@ -363,6 +364,7 @@ pub fn check_crate(tcx: &TyCtxt, trait_map: hir::TraitMap) -> CompileResult { time(time_passes, "drop-impl checking", || check::check_drop_impls(&ccx))?; + check_unused::check_crate(tcx); check_for_entry_fn(&ccx); let err_count = tcx.sess.err_count(); diff --git a/src/test/compile-fail/dep-graph-trait-impl-two-traits-same-method.rs b/src/test/compile-fail/dep-graph-trait-impl-two-traits-same-method.rs index 1afecd80ff5a..5e4f43af669c 100644 --- a/src/test/compile-fail/dep-graph-trait-impl-two-traits-same-method.rs +++ b/src/test/compile-fail/dep-graph-trait-impl-two-traits-same-method.rs @@ -15,6 +15,7 @@ #![feature(rustc_attrs)] #![allow(dead_code)] +#![allow(unused_imports)] fn main() { } diff --git a/src/test/compile-fail/lint-unused-imports.rs b/src/test/compile-fail/lint-unused-imports.rs index 3c1f4b043062..40322f5a5b53 100644 --- a/src/test/compile-fail/lint-unused-imports.rs +++ b/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 {}