From 19d1358b316092b104a78d316aec8a755fbcfcd4 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Thu, 3 Oct 2024 19:29:36 +0000 Subject: [PATCH] print cause chain in `mutable_key_type` --- clippy_lints/src/mut_key.rs | 13 ++++++-- clippy_utils/src/ty.rs | 48 ++++++++++++++++------------- tests/ui/mut_key.stderr | 60 +++++++++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 23 deletions(-) diff --git a/clippy_lints/src/mut_key.rs b/clippy_lints/src/mut_key.rs index 8118c14bd4a2..745f81d1c51a 100644 --- a/clippy_lints/src/mut_key.rs +++ b/clippy_lints/src/mut_key.rs @@ -1,9 +1,10 @@ use clippy_config::Conf; -use clippy_utils::diagnostics::span_lint; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::trait_ref_of_method; use clippy_utils::ty::InteriorMut; use rustc_hir as hir; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::print::with_forced_trimmed_paths; use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_session::impl_lint_pass; use rustc_span::Span; @@ -132,8 +133,14 @@ impl<'tcx> MutableKeyType<'tcx> { ) { let subst_ty = args.type_at(0); - if self.interior_mut.is_interior_mut_ty(cx, subst_ty) { - span_lint(cx, MUTABLE_KEY_TYPE, span, "mutable key type"); + if let Some(chain) = self.interior_mut.interior_mut_ty_chain(cx, subst_ty) { + span_lint_and_then(cx, MUTABLE_KEY_TYPE, span, "mutable key type", |diag| { + for ty in chain.iter().rev() { + diag.note(with_forced_trimmed_paths!(format!( + "... because it contains `{ty}`, which has interior mutability" + ))); + } + }); } } } diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs index 775a98fe361d..a99f821f3b0a 100644 --- a/clippy_utils/src/ty.rs +++ b/clippy_utils/src/ty.rs @@ -1168,7 +1168,7 @@ pub fn make_normalized_projection<'tcx>( pub struct InteriorMut<'tcx> { ignored_def_ids: FxHashSet, ignore_pointers: bool, - tys: FxHashMap, Option>, + tys: FxHashMap, Option<&'tcx ty::List>>>, } impl<'tcx> InteriorMut<'tcx> { @@ -1194,25 +1194,24 @@ impl<'tcx> InteriorMut<'tcx> { } } - /// Check if given type has inner mutability such as [`std::cell::Cell`] or - /// [`std::cell::RefCell`] etc. - pub fn is_interior_mut_ty(&mut self, cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { + /// Check if given type has interior mutability such as [`std::cell::Cell`] or + /// [`std::cell::RefCell`] etc. and if it does, returns a chain of types that causes + /// this type to be interior mutable + pub fn interior_mut_ty_chain(&mut self, cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<&'tcx ty::List>> { match self.tys.entry(ty) { - Entry::Occupied(o) => return *o.get() == Some(true), + Entry::Occupied(o) => return *o.get(), // Temporarily insert a `None` to break cycles Entry::Vacant(v) => v.insert(None), }; - let interior_mut = match *ty.kind() { - ty::RawPtr(inner_ty, _) if !self.ignore_pointers => self.is_interior_mut_ty(cx, inner_ty), - ty::Ref(_, inner_ty, _) | ty::Slice(inner_ty) => self.is_interior_mut_ty(cx, inner_ty), - ty::Array(inner_ty, size) => { - size.try_eval_target_usize(cx.tcx, cx.param_env) - .map_or(true, |u| u != 0) - && self.is_interior_mut_ty(cx, inner_ty) + let chain = match *ty.kind() { + ty::RawPtr(inner_ty, _) if !self.ignore_pointers => self.interior_mut_ty_chain(cx, inner_ty), + ty::Ref(_, inner_ty, _) | ty::Slice(inner_ty) => self.interior_mut_ty_chain(cx, inner_ty), + ty::Array(inner_ty, size) if size.try_eval_target_usize(cx.tcx, cx.param_env) != Some(0) => { + self.interior_mut_ty_chain(cx, inner_ty) }, - ty::Tuple(fields) => fields.iter().any(|ty| self.is_interior_mut_ty(cx, ty)), - ty::Adt(def, _) if def.is_unsafe_cell() => true, + ty::Tuple(fields) => fields.iter().find_map(|ty| self.interior_mut_ty_chain(cx, ty)), + ty::Adt(def, _) if def.is_unsafe_cell() => Some(ty::List::empty()), ty::Adt(def, args) => { let is_std_collection = matches!( cx.tcx.get_diagnostic_name(def.did()), @@ -1231,19 +1230,28 @@ impl<'tcx> InteriorMut<'tcx> { if is_std_collection || def.is_box() { // Include the types from std collections that are behind pointers internally - args.types().any(|ty| self.is_interior_mut_ty(cx, ty)) + args.types().find_map(|ty| self.interior_mut_ty_chain(cx, ty)) } else if self.ignored_def_ids.contains(&def.did()) || def.is_phantom_data() { - false + None } else { def.all_fields() - .any(|f| self.is_interior_mut_ty(cx, f.ty(cx.tcx, args))) + .find_map(|f| self.interior_mut_ty_chain(cx, f.ty(cx.tcx, args))) } }, - _ => false, + _ => None, }; - self.tys.insert(ty, Some(interior_mut)); - interior_mut + chain.map(|chain| { + let list = cx.tcx.mk_type_list_from_iter(chain.iter().chain([ty])); + self.tys.insert(ty, Some(list)); + list + }) + } + + /// Check if given type has interior mutability such as [`std::cell::Cell`] or + /// [`std::cell::RefCell`] etc. + pub fn is_interior_mut_ty(&mut self, cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { + self.interior_mut_ty_chain(cx, ty).is_some() } } diff --git a/tests/ui/mut_key.stderr b/tests/ui/mut_key.stderr index 5ad9aad2d0a5..8698ed4fd678 100644 --- a/tests/ui/mut_key.stderr +++ b/tests/ui/mut_key.stderr @@ -4,6 +4,9 @@ error: mutable key type LL | fn should_not_take_this_arg(m: &mut HashMap, _n: usize) -> HashSet { | ^^^^^^^^^^^^^^^^^^^^^^^^ | + = note: ... because it contains `Key`, which has interior mutability + = note: ... because it contains `AtomicUsize`, which has interior mutability + = note: ... because it contains `UnsafeCell`, which has interior mutability = note: `-D clippy::mutable-key-type` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::mutable_key_type)]` @@ -12,84 +15,141 @@ error: mutable key type | LL | fn should_not_take_this_arg(m: &mut HashMap, _n: usize) -> HashSet { | ^^^^^^^^^^^^ + | + = note: ... because it contains `Key`, which has interior mutability + = note: ... because it contains `AtomicUsize`, which has interior mutability + = note: ... because it contains `UnsafeCell`, which has interior mutability error: mutable key type --> tests/ui/mut_key.rs:35:5 | LL | let _other: HashMap = HashMap::new(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: ... because it contains `Key`, which has interior mutability + = note: ... because it contains `AtomicUsize`, which has interior mutability + = note: ... because it contains `UnsafeCell`, which has interior mutability error: mutable key type --> tests/ui/mut_key.rs:63:22 | LL | fn tuples_bad(_m: &mut HashMap<(Key, U), bool>) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: ... because it contains `(Key, U)`, which has interior mutability + = note: ... because it contains `Key`, which has interior mutability + = note: ... because it contains `AtomicUsize`, which has interior mutability + = note: ... because it contains `UnsafeCell`, which has interior mutability error: mutable key type --> tests/ui/mut_key.rs:76:5 | LL | let _map = HashMap::, usize>::new(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: ... because it contains `Cell`, which has interior mutability + = note: ... because it contains `UnsafeCell`, which has interior mutability error: mutable key type --> tests/ui/mut_key.rs:78:5 | LL | let _map = HashMap::<&mut Cell, usize>::new(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: ... because it contains `&mut Cell`, which has interior mutability + = note: ... because it contains `Cell`, which has interior mutability + = note: ... because it contains `UnsafeCell`, which has interior mutability error: mutable key type --> tests/ui/mut_key.rs:81:5 | LL | let _map = HashMap::>, usize>::new(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: ... because it contains `Vec>`, which has interior mutability + = note: ... because it contains `Cell`, which has interior mutability + = note: ... because it contains `UnsafeCell`, which has interior mutability error: mutable key type --> tests/ui/mut_key.rs:83:5 | LL | let _map = HashMap::, ()>, usize>::new(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: ... because it contains `BTreeMap, ()>`, which has interior mutability + = note: ... because it contains `Cell`, which has interior mutability + = note: ... because it contains `UnsafeCell`, which has interior mutability error: mutable key type --> tests/ui/mut_key.rs:85:5 | LL | let _map = HashMap::>, usize>::new(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: ... because it contains `BTreeMap<(), Cell>`, which has interior mutability + = note: ... because it contains `Cell`, which has interior mutability + = note: ... because it contains `UnsafeCell`, which has interior mutability error: mutable key type --> tests/ui/mut_key.rs:87:5 | LL | let _map = HashMap::>, usize>::new(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: ... because it contains `BTreeSet>`, which has interior mutability + = note: ... because it contains `Cell`, which has interior mutability + = note: ... because it contains `UnsafeCell`, which has interior mutability error: mutable key type --> tests/ui/mut_key.rs:89:5 | LL | let _map = HashMap::>, usize>::new(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: ... because it contains `Option>`, which has interior mutability + = note: ... because it contains `Cell`, which has interior mutability + = note: ... because it contains `UnsafeCell`, which has interior mutability error: mutable key type --> tests/ui/mut_key.rs:91:5 | LL | let _map = HashMap::>>, usize>::new(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: ... because it contains `Option>>`, which has interior mutability + = note: ... because it contains `Vec>`, which has interior mutability + = note: ... because it contains `Cell`, which has interior mutability + = note: ... because it contains `UnsafeCell`, which has interior mutability error: mutable key type --> tests/ui/mut_key.rs:94:5 | LL | let _map = HashMap::>, usize>::new(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: ... because it contains `Box>`, which has interior mutability + = note: ... because it contains `Cell`, which has interior mutability + = note: ... because it contains `UnsafeCell`, which has interior mutability error: mutable key type --> tests/ui/mut_key.rs:96:5 | LL | let _map = HashMap::>, usize>::new(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: ... because it contains `Rc>`, which has interior mutability + = note: ... because it contains `Cell`, which has interior mutability + = note: ... because it contains `UnsafeCell`, which has interior mutability error: mutable key type --> tests/ui/mut_key.rs:98:5 | LL | let _map = HashMap::>, usize>::new(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: ... because it contains `Arc>`, which has interior mutability + = note: ... because it contains `Cell`, which has interior mutability + = note: ... because it contains `UnsafeCell`, which has interior mutability error: aborting due to 15 previous errors