use reexport::*;
use rustc::hir;
use rustc::hir::*;
-use rustc::hir::intravisit::{walk_ty, FnKind, NestedVisitorMap, Visitor};
+use rustc::hir::intravisit::{walk_body, walk_expr, walk_ty, FnKind, NestedVisitorMap, Visitor};
use rustc::lint::*;
use rustc::ty::{self, Ty, TyCtxt};
use rustc::ty::subst::Substs;
use std::cmp::Ordering;
+use std::collections::BTreeMap;
+use std::borrow::Cow;
use syntax::ast::{FloatTy, IntTy, UintTy};
use syntax::attr::IntType;
use syntax::codemap::Span;
use utils::{comparisons, higher, in_external_macro, in_macro, last_path_segment, match_def_path, match_path,
- opt_def_id, snippet, snippet_opt, span_help_and_lint, span_lint, span_lint_and_sugg, type_size};
+ opt_def_id, same_tys, snippet, snippet_opt, span_help_and_lint, span_lint, span_lint_and_sugg,
+ span_lint_and_then, type_size};
use utils::paths;
/// Handles all the linting of funky types
match *qpath {
QPath::Resolved(Some(ref ty), ref p) => {
check_ty(cx, ty, is_local);
- for ty in p.segments
- .iter()
- .flat_map(|seg| seg.parameters.as_ref()
- .map_or_else(|| [].iter(),
- |params| params.types.iter()))
- {
+ for ty in p.segments.iter().flat_map(|seg| {
+ seg.parameters
+ .as_ref()
+ .map_or_else(|| [].iter(), |params| params.types.iter())
+ }) {
check_ty(cx, ty, is_local);
}
},
- QPath::Resolved(None, ref p) => for ty in p.segments
- .iter()
- .flat_map(|seg| seg.parameters.as_ref()
- .map_or_else(|| [].iter(),
- |params| params.types.iter()))
- {
+ QPath::Resolved(None, ref p) => for ty in p.segments.iter().flat_map(|seg| {
+ seg.parameters
+ .as_ref()
+ .map_or_else(|| [].iter(), |params| params.types.iter())
+ }) {
check_ty(cx, ty, is_local);
},
QPath::TypeRelative(ref ty, ref seg) => {
let opt = snippet_opt(cx, op.span);
let sugg = if let Some(ref snip) = opt {
if should_strip_parens(op, snip) {
- &snip[1..snip.len()-1]
+ &snip[1..snip.len() - 1]
} else {
snip.as_str()
}
}
}
}
+
+/// **What it does:** Checks for `impl` or `fn` missing generalization over
+/// different hashers and implicitly defaulting to the default hashing
+/// algorithm (SipHash). This lint ignores private free-functions.
+///
+/// **Why is this bad?** `HashMap` or `HashSet` with custom hashers cannot be
+/// used with them.
+///
+/// **Known problems:** Suggestions for replacing constructors are not always
+/// accurate.
+///
+/// **Example:**
+/// ```rust
+/// impl<K: Hash + Eq, V> Serialize for HashMap<K, V> { ... }
+///
+/// pub foo(map: &mut HashMap<i32, i32>) { .. }
+/// ```
+declare_lint! {
+ pub IMPLICIT_HASHER,
+ Warn,
+ "missing generalization over different hashers"
+}
+
+pub struct ImplicitHasher;
+
+impl LintPass for ImplicitHasher {
+ fn get_lints(&self) -> LintArray {
+ lint_array!(IMPLICIT_HASHER)
+ }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImplicitHasher {
+ #[allow(cast_possible_truncation)]
+ fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item) {
+ if let ItemImpl(_, _, _, ref generics, _, ref ty, ref items) = item.node {
+ let mut vis = ImplicitHasherTypeVisitor::new(cx);
+ vis.visit_ty(ty);
+
+ for target in vis.found {
+ let generics_snip = snippet(cx, generics.span, "");
+ let generics_snip_trimmed = if generics_snip.len() == 0 {
+ ""
+ } else {
+ // trim `<` `>`
+ &generics_snip[1..generics_snip.len() - 1]
+ };
+ let generics_span = generics.span.substitute_dummy({
+ let pos = snippet_opt(cx, item.span.until(target.span()))
+ .and_then(|snip| {
+ Some(item.span.lo() + ::syntax_pos::BytePos(snip.find("impl")? as u32 + 4))
+ })
+ .expect("failed to create span for type arguments");
+ Span::new(pos, pos, item.span.data().ctxt)
+ });
+
+ let mut vis = ImplicitHasherConstructorVisitor::new(cx, target.clone());
+ for item in items.iter().map(|item| cx.tcx.hir.impl_item(item.id)) {
+ vis.visit_impl_item(item);
+ }
+
+ span_lint_and_then(
+ cx,
+ IMPLICIT_HASHER,
+ target.span(),
+ &format!("impl for `{}` should be generarized over different hashers", target.type_name()),
+ move |db| {
+ db.span_suggestion(
+ generics_span,
+ "consider adding a type parameter",
+ format!(
+ "<{}{}S: ::std::hash::BuildHasher{}>",
+ generics_snip_trimmed,
+ if generics_snip_trimmed.is_empty() {
+ ""
+ } else {
+ ", "
+ },
+ if vis.suggestions.is_empty() {
+ ""
+ } else {
+ // request users to add `Default` bound so that generic constructors can be used
+ " + Default"
+ },
+ ),
+ );
+
+ db.span_suggestion(
+ target.span(),
+ "...and change the type to",
+ format!("{}<{}, S>", target.type_name(), target.type_arguments(),),
+ );
+
+ for (span, sugg) in vis.suggestions {
+ db.span_suggestion(span, "...and use generic constructor here", sugg);
+ }
+ },
+ );
+ }
+ }
+
+ if let ItemFn(ref decl, .., ref generics, body) = item.node {
+ if item.vis != Public {
+ return;
+ }
+
+ for ty in &decl.inputs {
+ let mut vis = ImplicitHasherTypeVisitor::new(cx);
+ vis.visit_ty(ty);
+
+ for target in vis.found {
+ let generics_snip = snippet(cx, generics.span, "");
+ let generics_snip_trimmed = if generics_snip.len() == 0 {
+ ""
+ } else {
+ // trim `<` `>`
+ &generics_snip[1..generics_snip.len() - 1]
+ };
+ let generics_span = generics.span.substitute_dummy({
+ let pos = snippet_opt(cx, item.span.until(ty.span))
+ .and_then(|snip| {
+ let i = snip.find("fn")?;
+ Some(item.span.lo() + ::syntax_pos::BytePos(i as u32 + (&snip[i..]).find('(')? as u32))
+ })
+ .expect("failed to create span for type parameters");
+ Span::new(pos, pos, item.span.data().ctxt)
+ });
+
+ let mut ctr_vis = ImplicitHasherConstructorVisitor::new(cx, target.clone());
+ ctr_vis.visit_body(cx.tcx.hir.body(body));
+ assert!(ctr_vis.suggestions.is_empty());
+
+ span_lint_and_then(
+ cx,
+ IMPLICIT_HASHER,
+ target.span(),
+ &format!(
+ "parameter of type `{}` should be generarized over different hashers",
+ target.type_name()
+ ),
+ move |db| {
+ db.span_suggestion(
+ generics_span,
+ "consider adding a type parameter",
+ format!(
+ "<{}{}S: ::std::hash::BuildHasher>",
+ generics_snip_trimmed,
+ if generics_snip_trimmed.is_empty() {
+ ""
+ } else {
+ ", "
+ },
+ ),
+ );
+
+ db.span_suggestion(
+ target.span(),
+ "...and change the type to",
+ format!("{}<{}, S>", target.type_name(), target.type_arguments(),),
+ );
+ },
+ );
+ }
+ }
+ }
+ }
+}
+
+#[derive(Clone)]
+enum ImplicitHasherType<'tcx> {
+ HashMap(Span, Ty<'tcx>, Cow<'static, str>, Cow<'static, str>),
+ HashSet(Span, Ty<'tcx>, Cow<'static, str>),
+}
+
+impl<'tcx> ImplicitHasherType<'tcx> {
+ /// Checks that `ty` is a target type without a BuildHasher.
+ fn new<'a>(cx: &LateContext<'a, 'tcx>, hir_ty: &hir::Ty) -> Option<Self> {
+ if let TyPath(QPath::Resolved(None, ref path)) = hir_ty.node {
+ let params = &path.segments.last().as_ref()?.parameters.as_ref()?.types;
+ let params_len = params.len();
+
+ let ty = cx.tcx.type_of(opt_def_id(path.def)?);
+
+ if match_path(path, &paths::HASHMAP) && params_len == 2 {
+ Some(ImplicitHasherType::HashMap(
+ hir_ty.span,
+ ty,
+ snippet(cx, params[0].span, "K"),
+ snippet(cx, params[1].span, "V"),
+ ))
+ } else if match_path(path, &paths::HASHSET) && params_len == 1 {
+ Some(ImplicitHasherType::HashSet(hir_ty.span, ty, snippet(cx, params[0].span, "T")))
+ } else {
+ None
+ }
+ } else {
+ None
+ }
+ }
+
+ fn type_name(&self) -> &'static str {
+ match *self {
+ ImplicitHasherType::HashMap(..) => "HashMap",
+ ImplicitHasherType::HashSet(..) => "HashSet",
+ }
+ }
+
+ fn type_arguments(&self) -> String {
+ match *self {
+ ImplicitHasherType::HashMap(.., ref k, ref v) => format!("{}, {}", k, v),
+ ImplicitHasherType::HashSet(.., ref t) => format!("{}", t),
+ }
+ }
+
+ fn ty(&self) -> Ty<'tcx> {
+ match *self {
+ ImplicitHasherType::HashMap(_, ty, ..) | ImplicitHasherType::HashSet(_, ty, ..) => ty,
+ }
+ }
+
+ fn span(&self) -> Span {
+ match *self {
+ ImplicitHasherType::HashMap(span, ..) | ImplicitHasherType::HashSet(span, ..) => span,
+ }
+ }
+}
+
+struct ImplicitHasherTypeVisitor<'a, 'tcx: 'a> {
+ cx: &'a LateContext<'a, 'tcx>,
+ found: Vec<ImplicitHasherType<'tcx>>,
+}
+
+impl<'a, 'tcx: 'a> ImplicitHasherTypeVisitor<'a, 'tcx> {
+ fn new(cx: &'a LateContext<'a, 'tcx>) -> Self {
+ Self { cx, found: vec![] }
+ }
+}
+
+impl<'a, 'tcx: 'a> Visitor<'tcx> for ImplicitHasherTypeVisitor<'a, 'tcx> {
+ fn visit_ty(&mut self, t: &'tcx hir::Ty) {
+ if let Some(target) = ImplicitHasherType::new(self.cx, t) {
+ self.found.push(target);
+ }
+
+ walk_ty(self, t);
+ }
+
+ fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
+ NestedVisitorMap::None
+ }
+}
+
+/// Looks for default-hasher-dependent constructors like `HashMap::new`.
+struct ImplicitHasherConstructorVisitor<'a, 'tcx: 'a> {
+ cx: &'a LateContext<'a, 'tcx>,
+ body: Option<BodyId>,
+ target: ImplicitHasherType<'tcx>,
+ suggestions: BTreeMap<Span, String>,
+}
+
+impl<'a, 'tcx: 'a> ImplicitHasherConstructorVisitor<'a, 'tcx> {
+ fn new(cx: &'a LateContext<'a, 'tcx>, target: ImplicitHasherType<'tcx>) -> Self {
+ Self {
+ cx,
+ body: None,
+ target,
+ suggestions: BTreeMap::new(),
+ }
+ }
+}
+
+impl<'a, 'tcx: 'a> Visitor<'tcx> for ImplicitHasherConstructorVisitor<'a, 'tcx> {
+ fn visit_body(&mut self, body: &'tcx Body) {
+ self.body = Some(body.id());
+ walk_body(self, body);
+ }
+
+ fn visit_expr(&mut self, e: &'tcx Expr) {
+ if_let_chain!{[
+ let Some(body) = self.body,
+ let ExprCall(ref fun, ref args) = e.node,
+ let ExprPath(QPath::TypeRelative(ref ty, ref method)) = fun.node,
+ let TyPath(QPath::Resolved(None, ref ty_path)) = ty.node,
+ ], {
+ if same_tys(self.cx, self.cx.tcx.body_tables(body).expr_ty(e), self.target.ty()) {
+ return;
+ }
+
+ if match_path(ty_path, &paths::HASHMAP) {
+ if method.name == "new" {
+ self.suggestions
+ .insert(e.span, "HashMap::default()".to_string());
+ } else if method.name == "with_capacity" {
+ self.suggestions.insert(
+ e.span,
+ format!(
+ "HashMap::with_capacity_and_hasher({}, Default::default())",
+ snippet(self.cx, args[0].span, "capacity"),
+ ),
+ );
+ }
+ } else if match_path(ty_path, &paths::HASHSET) {
+ if method.name == "new" {
+ self.suggestions
+ .insert(e.span, "HashSet::default()".to_string());
+ } else if method.name == "with_capacity" {
+ self.suggestions.insert(
+ e.span,
+ format!(
+ "HashSet::with_capacity_and_hasher({}, Default::default())",
+ snippet(self.cx, args[0].span, "capacity"),
+ ),
+ );
+ }
+ }
+ }}
+
+ walk_expr(self, e);
+ }
+
+ fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
+ NestedVisitorMap::OnlyBodies(&self.cx.tcx.hir)
+ }
+}
--- /dev/null
+#![allow(unused)]
+//#![feature(plugin)]#![plugin(clippy)]
+use std::collections::{HashMap, HashSet};
+use std::cmp::Eq;
+use std::hash::{Hash, BuildHasher};
+
+trait Foo<T>: Sized {
+ fn make() -> (Self, Self);
+}
+
+impl<K: Hash + Eq, V> Foo<i8> for HashMap<K, V> {
+ fn make() -> (Self, Self) {
+ // OK, don't suggest to modify these
+ let _: HashMap<i32, i32> = HashMap::new();
+ let _: HashSet<i32> = HashSet::new();
+
+ (HashMap::new(), HashMap::with_capacity(10))
+ }
+}
+impl<K: Hash + Eq, V> Foo<i8> for (HashMap<K, V>,) {
+ fn make() -> (Self, Self) {
+ ((HashMap::new(),), (HashMap::with_capacity(10),))
+ }
+}
+impl Foo<i16> for HashMap<String, String> {
+ fn make() -> (Self, Self) {
+ (HashMap::new(), HashMap::with_capacity(10))
+ }
+}
+
+impl<K: Hash + Eq, V, S: BuildHasher + Default> Foo<i32> for HashMap<K, V, S> {
+ fn make() -> (Self, Self) {
+ (HashMap::default(), HashMap::with_capacity_and_hasher(10, Default::default()))
+ }
+}
+impl<S: BuildHasher + Default> Foo<i64> for HashMap<String, String, S> {
+ fn make() -> (Self, Self) {
+ (HashMap::default(), HashMap::with_capacity_and_hasher(10, Default::default()))
+ }
+}
+
+
+impl<T: Hash + Eq> Foo<i8> for HashSet<T> {
+ fn make() -> (Self, Self) {
+ (HashSet::new(), HashSet::with_capacity(10))
+ }
+}
+impl Foo<i16> for HashSet<String> {
+ fn make() -> (Self, Self) {
+ (HashSet::new(), HashSet::with_capacity(10))
+ }
+}
+
+impl<T: Hash + Eq, S: BuildHasher + Default> Foo<i32> for HashSet<T, S> {
+ fn make() -> (Self, Self) {
+ (HashSet::default(), HashSet::with_capacity_and_hasher(10, Default::default()))
+ }
+}
+impl<S: BuildHasher + Default> Foo<i64> for HashSet<String, S> {
+ fn make() -> (Self, Self) {
+ (HashSet::default(), HashSet::with_capacity_and_hasher(10, Default::default()))
+ }
+}
+
+pub fn foo(_map: &mut HashMap<i32, i32>, _set: &mut HashSet<i32>) {
+}
+
+fn main() {}
--- /dev/null
+error: impl for `HashMap` should be generarized over different hashers
+ --> $DIR/implicit_hasher.rs:11:35
+ |
+11 | impl<K: Hash + Eq, V> Foo<i8> for HashMap<K, V> {
+ | ^^^^^^^^^^^^^
+ |
+ = note: `-D implicit-hasher` implied by `-D warnings`
+help: consider adding a type parameter
+ |
+11 | impl<K: Hash + Eq, V, S: ::std::hash::BuildHasher + Default> Foo<i8> for HashMap<K, V> {
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: ...and change the type to
+ |
+11 | impl<K: Hash + Eq, V> Foo<i8> for HashMap<K, V, S> {
+ | ^^^^^^^^^^^^^^^^
+help: ...and use generic constructor here
+ |
+14 | let _: HashMap<i32, i32> = HashMap::default();
+ | ^^^^^^^^^^^^^^^^^^
+help: ...and use generic constructor here
+ |
+15 | let _: HashSet<i32> = HashSet::default();
+ | ^^^^^^^^^^^^^^^^^^
+help: ...and use generic constructor here
+ |
+17 | (HashMap::default(), HashMap::with_capacity(10))
+ | ^^^^^^^^^^^^^^^^^^
+help: ...and use generic constructor here
+ |
+17 | (HashMap::new(), HashMap::with_capacity_and_hasher(10, Default::default()))
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: impl for `HashMap` should be generarized over different hashers
+ --> $DIR/implicit_hasher.rs:20:36
+ |
+20 | impl<K: Hash + Eq, V> Foo<i8> for (HashMap<K, V>,) {
+ | ^^^^^^^^^^^^^
+ |
+help: consider adding a type parameter
+ |
+20 | impl<K: Hash + Eq, V, S: ::std::hash::BuildHasher + Default> Foo<i8> for (HashMap<K, V>,) {
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: ...and change the type to
+ |
+20 | impl<K: Hash + Eq, V> Foo<i8> for (HashMap<K, V, S>,) {
+ | ^^^^^^^^^^^^^^^^
+help: ...and use generic constructor here
+ |
+22 | ((HashMap::default(),), (HashMap::with_capacity(10),))
+ | ^^^^^^^^^^^^^^^^^^
+help: ...and use generic constructor here
+ |
+22 | ((HashMap::new(),), (HashMap::with_capacity_and_hasher(10, Default::default()),))
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: impl for `HashMap` should be generarized over different hashers
+ --> $DIR/implicit_hasher.rs:25:19
+ |
+25 | impl Foo<i16> for HashMap<String, String> {
+ | ^^^^^^^^^^^^^^^^^^^^^^^
+ |
+help: consider adding a type parameter
+ |
+25 | impl<S: ::std::hash::BuildHasher + Default> Foo<i16> for HashMap<String, String> {
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: ...and change the type to
+ |
+25 | impl Foo<i16> for HashMap<String, String, S> {
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: ...and use generic constructor here
+ |
+27 | (HashMap::default(), HashMap::with_capacity(10))
+ | ^^^^^^^^^^^^^^^^^^
+help: ...and use generic constructor here
+ |
+27 | (HashMap::new(), HashMap::with_capacity_and_hasher(10, Default::default()))
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: impl for `HashSet` should be generarized over different hashers
+ --> $DIR/implicit_hasher.rs:43:32
+ |
+43 | impl<T: Hash + Eq> Foo<i8> for HashSet<T> {
+ | ^^^^^^^^^^
+ |
+help: consider adding a type parameter
+ |
+43 | impl<T: Hash + Eq, S: ::std::hash::BuildHasher + Default> Foo<i8> for HashSet<T> {
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: ...and change the type to
+ |
+43 | impl<T: Hash + Eq> Foo<i8> for HashSet<T, S> {
+ | ^^^^^^^^^^^^^
+help: ...and use generic constructor here
+ |
+45 | (HashSet::default(), HashSet::with_capacity(10))
+ | ^^^^^^^^^^^^^^^^^^
+help: ...and use generic constructor here
+ |
+45 | (HashSet::new(), HashSet::with_capacity_and_hasher(10, Default::default()))
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: impl for `HashSet` should be generarized over different hashers
+ --> $DIR/implicit_hasher.rs:48:19
+ |
+48 | impl Foo<i16> for HashSet<String> {
+ | ^^^^^^^^^^^^^^^
+ |
+help: consider adding a type parameter
+ |
+48 | impl<S: ::std::hash::BuildHasher + Default> Foo<i16> for HashSet<String> {
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: ...and change the type to
+ |
+48 | impl Foo<i16> for HashSet<String, S> {
+ | ^^^^^^^^^^^^^^^^^^
+help: ...and use generic constructor here
+ |
+50 | (HashSet::default(), HashSet::with_capacity(10))
+ | ^^^^^^^^^^^^^^^^^^
+help: ...and use generic constructor here
+ |
+50 | (HashSet::new(), HashSet::with_capacity_and_hasher(10, Default::default()))
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: parameter of type `HashMap` should be generarized over different hashers
+ --> $DIR/implicit_hasher.rs:65:23
+ |
+65 | pub fn foo(_map: &mut HashMap<i32, i32>, _set: &mut HashSet<i32>) {
+ | ^^^^^^^^^^^^^^^^^
+ |
+help: consider adding a type parameter
+ |
+65 | pub fn foo<S: ::std::hash::BuildHasher>(_map: &mut HashMap<i32, i32>, _set: &mut HashSet<i32>) {
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: ...and change the type to
+ |
+65 | pub fn foo(_map: &mut HashMap<i32, i32, S>, _set: &mut HashSet<i32>) {
+ | ^^^^^^^^^^^^^^^^^^^^
+
+error: parameter of type `HashSet` should be generarized over different hashers
+ --> $DIR/implicit_hasher.rs:65:53
+ |
+65 | pub fn foo(_map: &mut HashMap<i32, i32>, _set: &mut HashSet<i32>) {
+ | ^^^^^^^^^^^^
+ |
+help: consider adding a type parameter
+ |
+65 | pub fn foo<S: ::std::hash::BuildHasher>(_map: &mut HashMap<i32, i32>, _set: &mut HashSet<i32>) {
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: ...and change the type to
+ |
+65 | pub fn foo(_map: &mut HashMap<i32, i32>, _set: &mut HashSet<i32, S>) {
+ | ^^^^^^^^^^^^^^^
+
projects / rust.git / commitdiff