diff --git a/clippy_lints/src/mut_key.rs b/clippy_lints/src/mut_key.rs index 8a79786d4fc..d48c4c419ae 100644 --- a/clippy_lints/src/mut_key.rs +++ b/clippy_lints/src/mut_key.rs @@ -1,5 +1,5 @@ use crate::utils::{match_def_path, paths, span_lint, trait_ref_of_method, walk_ptrs_ty}; -use rustc::ty::{Adt, Dynamic, Opaque, Param, RawPtr, Ref, Ty, TypeAndMut}; +use rustc::ty::{Adt, Array, RawPtr, Ref, Slice, Tuple, Ty, TypeAndMut}; use rustc_hir as hir; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -101,21 +101,24 @@ fn check_ty<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, span: Span, ty: Ty<'tcx>) { if [&paths::HASHMAP, &paths::BTREEMAP, &paths::HASHSET, &paths::BTREESET] .iter() .any(|path| match_def_path(cx, def.did, &**path)) + && is_mutable_type(cx, substs.type_at(0), span) { - let key_type = concrete_type(substs.type_at(0)); - if let Some(key_type) = key_type { - if !key_type.is_freeze(cx.tcx, cx.param_env, span) { - span_lint(cx, MUTABLE_KEY_TYPE, span, "mutable key type"); - } - } + span_lint(cx, MUTABLE_KEY_TYPE, span, "mutable key type"); } } } -fn concrete_type(ty: Ty<'_>) -> Option> { +fn is_mutable_type<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'tcx>, span: Span) -> bool { match ty.kind { - RawPtr(TypeAndMut { ty: inner_ty, .. }) | Ref(_, inner_ty, _) => concrete_type(inner_ty), - Dynamic(..) | Opaque(..) | Param(..) => None, - _ => Some(ty), + RawPtr(TypeAndMut { ty: inner_ty, mutbl }) | Ref(_, inner_ty, mutbl) => { + mutbl == hir::Mutability::Mut || is_mutable_type(cx, inner_ty, span) + }, + Slice(inner_ty) => is_mutable_type(cx, inner_ty, span), + Array(inner_ty, size) => { + size.try_eval_usize(cx.tcx, cx.param_env).map_or(true, |u| u != 0) && is_mutable_type(cx, inner_ty, span) + }, + Tuple(..) => ty.tuple_fields().any(|ty| is_mutable_type(cx, ty, span)), + Adt(..) => cx.tcx.layout_of(cx.param_env.and(ty)).is_ok() && !ty.is_freeze(cx.tcx, cx.param_env, span), + _ => false, } } diff --git a/tests/ui/mut_key.rs b/tests/ui/mut_key.rs index 5ec9b05f517..d45cf8278a8 100644 --- a/tests/ui/mut_key.rs +++ b/tests/ui/mut_key.rs @@ -1,3 +1,5 @@ +#![allow(clippy::implicit_hasher)] + use std::collections::{HashMap, HashSet}; use std::hash::{Hash, Hasher}; use std::sync::atomic::{AtomicUsize, Ordering::Relaxed}; @@ -29,9 +31,27 @@ fn should_not_take_this_arg(m: &mut HashMap, _n: usize) -> HashSet) {} +fn this_is_ok(_m: &mut HashMap) {} + +#[allow(unused)] +trait Trait { + type AssociatedType; + + fn trait_fn(&self, set: std::collections::HashSet); +} + +fn generics_are_ok_too(_m: &mut HashSet) { + // nothing to see here, move along +} + +fn tuples(_m: &mut HashMap<((), U), ()>) {} + +fn tuples_bad(_m: &mut HashMap<(Key, U), bool>) {} fn main() { let _ = should_not_take_this_arg(&mut HashMap::new(), 1); this_is_ok(&mut HashMap::new()); + tuples::(&mut HashMap::new()); + tuples::<()>(&mut HashMap::new()); + tuples_bad::<()>(&mut HashMap::new()); } diff --git a/tests/ui/mut_key.stderr b/tests/ui/mut_key.stderr index ebdbfe99022..5af28f18d3d 100644 --- a/tests/ui/mut_key.stderr +++ b/tests/ui/mut_key.stderr @@ -1,5 +1,5 @@ error: mutable key type - --> $DIR/mut_key.rs:27:32 + --> $DIR/mut_key.rs:29:32 | LL | fn should_not_take_this_arg(m: &mut HashMap, _n: usize) -> HashSet { | ^^^^^^^^^^^^^^^^^^^^^^^^ @@ -7,16 +7,22 @@ LL | fn should_not_take_this_arg(m: &mut HashMap, _n: usize) -> Hash = note: `#[deny(clippy::mutable_key_type)]` on by default error: mutable key type - --> $DIR/mut_key.rs:27:72 + --> $DIR/mut_key.rs:29:72 | LL | fn should_not_take_this_arg(m: &mut HashMap, _n: usize) -> HashSet { | ^^^^^^^^^^^^ error: mutable key type - --> $DIR/mut_key.rs:28:5 + --> $DIR/mut_key.rs:30:5 | LL | let _other: HashMap = HashMap::new(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 3 previous errors +error: mutable key type + --> $DIR/mut_key.rs:49:22 + | +LL | fn tuples_bad(_m: &mut HashMap<(Key, U), bool>) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 4 previous errors