Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn unused trait imports #30021

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/libcore/num/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
use self::wrapping::OverflowingOps;

use char::CharExt;
use cmp::{Eq, PartialOrd};
use cmp::PartialOrd;
use convert::From;
use fmt;
use intrinsics;
Expand Down
1 change: 0 additions & 1 deletion src/libcore/option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@
use self::Option::*;

use clone::Clone;
use cmp::{Eq, Ord};
use default::Default;
use iter::ExactSizeIterator;
use iter::{Iterator, DoubleEndedIterator, FromIterator, IntoIterator};
Expand Down
2 changes: 0 additions & 2 deletions src/libcore/str/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ use self::pattern::{Searcher, ReverseSearcher, DoubleEndedSearcher};

use char::{self, CharExt};
use clone::Clone;
use cmp::Eq;
use convert::AsRef;
use default::Default;
use fmt;
Expand Down Expand Up @@ -1180,7 +1179,6 @@ Section: Trait implementations
mod traits {
use cmp::{self, Ordering, Ord, PartialEq, PartialOrd, Eq};
use cmp::Ordering::{Less, Greater};
use iter::Iterator;
use option::Option;
use option::Option::Some;
use ops;
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/lint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use middle::privacy::AccessLevels;
use middle::ty::{self, Ty};
use session::{early_error, Session};
use lint::{Level, LevelSource, Lint, LintId, LintArray, LintPass};
use lint::{EarlyLintPass, EarlyLintPassObject, LateLintPass, LateLintPassObject};
use lint::{EarlyLintPassObject, LateLintPass, LateLintPassObject};
use lint::{Default, CommandLine, Node, Allow, Warn, Deny, Forbid};
use lint::builtin;
use util::nodemap::FnvHashMap;
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use lint;

use std::collections::HashSet;
use syntax::{ast, codemap};
use syntax::attr::{self, AttrMetaMethods};
use syntax::attr;

// Any local node that may call something in its body block should be
// explored. For example, if it's a live NodeItem that is a
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/implicator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use middle::infer::{InferCtxt, GenericKind};
use middle::subst::Substs;
use middle::traits;
use middle::ty::{self, RegionEscape, ToPredicate, Ty};
use middle::ty::fold::{TypeFoldable, TypeFolder};
use middle::ty::fold::TypeFoldable;

use syntax::ast;
use syntax::codemap::Span;
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/infer/combine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ use middle::ty::{IntType, UintType};
use middle::ty::{self, Ty};
use middle::ty::error::TypeError;
use middle::ty::fold::{TypeFolder, TypeFoldable};
use middle::ty::relate::{Relate, RelateResult, TypeRelation};
use middle::ty::relate::{RelateResult, TypeRelation};

use syntax::ast;
use syntax::codemap::Span;
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use middle::ty::adjustment;
use middle::ty::{TyVid, IntVid, FloatVid, RegionVid};
use middle::ty::{self, Ty, HasTypeFlags};
use middle::ty::error::{ExpectedFound, TypeError, UnconstrainedNumeric};
use middle::ty::fold::{TypeFolder, TypeFoldable};
use middle::ty::fold::TypeFoldable;
use middle::ty::relate::{Relate, RelateResult, TypeRelation};
use rustc_data_structures::unify::{self, UnificationTable};
use std::cell::{RefCell, Ref};
Expand Down
8 changes: 8 additions & 0 deletions src/librustc/middle/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>,
Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -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()),
Expand All @@ -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()),
Expand Down
10 changes: 7 additions & 3 deletions src/librustc/middle/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,13 @@ use middle::lang_items::{FnTraitLangItem, FnMutTraitLangItem, FnOnceTraitLangIte
use middle::subst::{self, ParamSpace, Subst, Substs, VecPerParamSpace};
use middle::traits;
use middle::ty;
use middle::ty::fold::TypeFolder;
use middle::ty::walk::TypeWalker;
use util::common::memoized;
use util::nodemap::{NodeMap, NodeSet, DefIdMap};
use util::nodemap::FnvHashMap;

use serialize::{Encodable, Encoder, Decodable, Decoder};
use std::borrow::{Borrow, Cow};
use std::borrow::Cow;
use std::cell::{Cell, RefCell};
use std::hash::{Hash, Hasher};
use std::iter;
Expand Down Expand Up @@ -2752,8 +2751,13 @@ pub type FreevarMap = NodeMap<Vec<Freevar>>;

pub type CaptureModeMap = NodeMap<hir::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.
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use std::cmp;
use std::hash::{Hash, SipHasher, Hasher};
use std::rc::Rc;
use syntax::ast::{self, Name};
use syntax::attr::{self, AttrMetaMethods, SignedInt, UnsignedInt};
use syntax::attr::{self, SignedInt, UnsignedInt};
use syntax::codemap::Span;

use rustc_front::hir;
Expand Down
1 change: 0 additions & 1 deletion src/librustc/session/filesearch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ pub use self::FileMatch::*;
use std::collections::HashSet;
use std::env;
use std::fs;
use std::io::prelude::*;
use std::path::{Path, PathBuf};

use session::search_paths::{SearchPaths, PathKind};
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_driver/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_driver/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_lint/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use std::cmp;
use std::{i8, i16, i32, i64, u8, u16, u32, u64, f32, f64};

use syntax::{abi, ast};
use syntax::attr::{self, AttrMetaMethods};
use syntax::attr;
use syntax::codemap::{self, Span};
use syntax::feature_gate::{emit_feature_err, GateIssue};
use syntax::ast::{TyIs, TyUs, TyI8, TyU8, TyI16, TyU16, TyI32, TyU32, TyI64, TyU64};
Expand Down
1 change: 0 additions & 1 deletion src/librustc_metadata/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ use std::u32;
use syntax::abi;
use syntax::ast::{self, NodeId, Name, CRATE_NODE_ID, CrateNum};
use syntax::attr;
use syntax::attr::AttrMetaMethods;
use syntax::diagnostic::SpanHandler;
use syntax::parse::token::special_idents;
use syntax;
Expand Down
37 changes: 22 additions & 15 deletions src/librustc_resolve/check_unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -62,13 +64,7 @@ impl<'a, 'b, 'tcx> UnusedImportCheckVisitor<'a, 'b, 'tcx> {
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.check_import(id, span);

let mut def_map = self.def_map.borrow_mut();
let path_res = if let Some(r) = def_map.get_mut(&id) {
Expand Down Expand Up @@ -109,6 +105,24 @@ impl<'a, 'b, 'tcx> UnusedImportCheckVisitor<'a, 'b, 'tcx> {
type_used: t_used,
};
}

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

impl<'a, 'b, 'v, 'tcx> Visitor<'v> for UnusedImportCheckVisitor<'a, 'b, 'tcx> {
Expand Down Expand Up @@ -144,14 +158,7 @@ impl<'a, 'b, 'v, 'tcx> Visitor<'v> for UnusedImportCheckVisitor<'a, 'b, 'tcx> {
}
}
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.check_import(item.id, p.span);
}
}
}
Expand Down
34 changes: 22 additions & 12 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,12 @@ use rustc::middle::def_id::DefId;
use rustc::middle::pat_util::pat_bindings;
use rustc::middle::privacy::*;
use rustc::middle::subst::{ParamSpace, FnSpace, TypeSpace};
use rustc::middle::ty::{Freevar, FreevarMap, TraitMap, GlobMap};
use rustc::util::nodemap::{NodeMap, DefIdSet, FnvHashMap};
use rustc::middle::ty::{Freevar, FreevarMap, TraitCandidate, TraitMap, GlobMap};
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};
use syntax::ast::{TyUs, TyU8, TyU16, TyU32, TyU64, TyF64, TyF32};
use syntax::attr::AttrMetaMethods;
use syntax::parse::token::{self, special_names, special_idents};
use syntax::codemap::{self, Span, Pos};
use syntax::util::lev_distance::{lev_distance, max_suggestion_distance};
Expand Down Expand Up @@ -1142,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>>,
Expand Down Expand Up @@ -1193,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,
}
Expand Down Expand Up @@ -3609,14 +3611,20 @@ 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();
Expand All @@ -3626,7 +3634,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
match self.current_trait_ref {
Some((trait_def_id, _)) => {
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);
}
}
None => {} // Nothing to do.
Expand All @@ -3646,7 +3654,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
_ => continue,
};
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);
}
}
}
Expand All @@ -3662,9 +3670,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
Some(..) | None => continue,
};
if self.trait_item_map.contains_key(&(name, did)) {
add_trait_info(&mut found_traits, did, name);
let id = import.type_ns.id;
self.used_imports.insert((id, TypeNS));
add_trait_info(&mut found_traits, did, Some(id), name);
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() {
Expand Down Expand Up @@ -3815,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,
Expand Down Expand Up @@ -3843,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,
Expand Down
Loading