Warn unused trait imports

This commit is contained in:
Seo Sanghyeon 2016-04-19 22:43:10 +09:00
parent 7ad1900e1c
commit 24d86137f5
13 changed files with 169 additions and 25 deletions

View File

@ -59,6 +59,7 @@ pub enum DepNode<D: Clone + Debug> {
TypeckItemBody(D),
Dropck,
DropckImpl(D),
UnusedTraitCheck,
CheckConst(D),
Privacy,
IntrinsicCheck(D),
@ -163,6 +164,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),

View File

@ -1639,8 +1639,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.

View File

@ -289,6 +289,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>>>,
@ -338,6 +340,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>,
@ -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()),

View File

@ -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,

View File

@ -132,7 +132,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);
@ -143,6 +143,7 @@ fn test_env<F>(source_string: &str,
named_region_map.unwrap(),
ast_map,
freevars,
maybe_unused_trait_imports,
region_map,
lang_items,
index,

View File

@ -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);
}
}
}

View File

@ -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<CrateNum>,
maybe_unused_trait_imports: NodeSet,
privacy_errors: Vec<PrivacyError<'a>>,
@ -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<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);
}
}
@ -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<DefMap>,
pub freevars: FreevarMap,
pub maybe_unused_trait_imports: NodeSet,
pub export_map: ExportMap,
pub trait_map: TraitMap,
pub glob_map: Option<GlobMap>,
@ -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 {

View File

@ -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());

View File

@ -42,6 +42,7 @@ struct ProbeContext<'a, 'tcx:'a> {
inherent_candidates: Vec<Candidate<'tcx>>,
extension_candidates: Vec<Candidate<'tcx>>,
impl_dups: HashSet<DefId>,
import_id: Option<ast::NodeId>,
/// 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<ast::NodeId>,
}
#[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<ast::NodeId>,
// 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

View File

@ -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 <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 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);
}

View File

@ -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();

View File

@ -15,6 +15,7 @@
#![feature(rustc_attrs)]
#![allow(dead_code)]
#![allow(unused_imports)]
fn main() { }

View File

@ -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 {}