]>
Commit | Line | Data |
---|---|---|
cdc7bbd5 | 1 | use clippy_utils::diagnostics::span_lint; |
353b0b11 | 2 | use clippy_utils::ty::is_interior_mut_ty; |
487cf647 FG |
3 | use clippy_utils::{def_path_def_ids, trait_ref_of_method}; |
4 | use rustc_data_structures::fx::FxHashSet; | |
f20569fa XL |
5 | use rustc_hir as hir; |
6 | use rustc_lint::{LateContext, LateLintPass}; | |
353b0b11 FG |
7 | use rustc_middle::query::Key; |
8 | use rustc_middle::ty::{Adt, Ty}; | |
487cf647 | 9 | use rustc_session::{declare_tool_lint, impl_lint_pass}; |
9ffffee4 | 10 | use rustc_span::def_id::LocalDefId; |
f20569fa | 11 | use rustc_span::source_map::Span; |
94222f64 | 12 | use rustc_span::symbol::sym; |
cdc7bbd5 | 13 | use std::iter; |
f20569fa XL |
14 | |
15 | declare_clippy_lint! { | |
94222f64 XL |
16 | /// ### What it does |
17 | /// Checks for sets/maps with mutable key types. | |
f20569fa | 18 | /// |
94222f64 XL |
19 | /// ### Why is this bad? |
20 | /// All of `HashMap`, `HashSet`, `BTreeMap` and | |
f20569fa XL |
21 | /// `BtreeSet` rely on either the hash or the order of keys be unchanging, |
22 | /// so having types with interior mutability is a bad idea. | |
23 | /// | |
94222f64 | 24 | /// ### Known problems |
c295e0f8 XL |
25 | /// |
26 | /// #### False Positives | |
27 | /// It's correct to use a struct that contains interior mutability as a key, when its | |
28 | /// implementation of `Hash` or `Ord` doesn't access any of the interior mutable types. | |
29 | /// However, this lint is unable to recognize this, so it will often cause false positives in | |
30 | /// theses cases. The `bytes` crate is a great example of this. | |
31 | /// | |
32 | /// #### False Negatives | |
33 | /// For custom `struct`s/`enum`s, this lint is unable to check for interior mutability behind | |
34 | /// indirection. For example, `struct BadKey<'a>(&'a Cell<usize>)` will be seen as immutable | |
35 | /// and cause a false negative if its implementation of `Hash`/`Ord` accesses the `Cell`. | |
36 | /// | |
37 | /// This lint does check a few cases for indirection. Firstly, using some standard library | |
38 | /// types (`Option`, `Result`, `Box`, `Rc`, `Arc`, `Vec`, `VecDeque`, `BTreeMap` and | |
39 | /// `BTreeSet`) directly as keys (e.g. in `HashMap<Box<Cell<usize>>, ()>`) **will** trigger the | |
40 | /// lint, because the impls of `Hash`/`Ord` for these types directly call `Hash`/`Ord` on their | |
41 | /// contained type. | |
42 | /// | |
43 | /// Secondly, the implementations of `Hash` and `Ord` for raw pointers (`*const T` or `*mut T`) | |
44 | /// apply only to the **address** of the contained value. Therefore, interior mutability | |
45 | /// behind raw pointers (e.g. in `HashSet<*mut Cell<usize>>`) can't impact the value of `Hash` | |
46 | /// or `Ord`, and therefore will not trigger this link. For more info, see issue | |
47 | /// [#6745](https://github.com/rust-lang/rust-clippy/issues/6745). | |
f20569fa | 48 | /// |
94222f64 | 49 | /// ### Example |
f20569fa XL |
50 | /// ```rust |
51 | /// use std::cmp::{PartialEq, Eq}; | |
52 | /// use std::collections::HashSet; | |
53 | /// use std::hash::{Hash, Hasher}; | |
54 | /// use std::sync::atomic::AtomicUsize; | |
55 | ///# #[allow(unused)] | |
56 | /// | |
57 | /// struct Bad(AtomicUsize); | |
58 | /// impl PartialEq for Bad { | |
59 | /// fn eq(&self, rhs: &Self) -> bool { | |
60 | /// .. | |
61 | /// ; unimplemented!(); | |
62 | /// } | |
63 | /// } | |
64 | /// | |
65 | /// impl Eq for Bad {} | |
66 | /// | |
67 | /// impl Hash for Bad { | |
68 | /// fn hash<H: Hasher>(&self, h: &mut H) { | |
69 | /// .. | |
70 | /// ; unimplemented!(); | |
71 | /// } | |
72 | /// } | |
73 | /// | |
74 | /// fn main() { | |
75 | /// let _: HashSet<Bad> = HashSet::new(); | |
76 | /// } | |
77 | /// ``` | |
a2a8927a | 78 | #[clippy::version = "1.42.0"] |
f20569fa | 79 | pub MUTABLE_KEY_TYPE, |
136023e0 | 80 | suspicious, |
f20569fa XL |
81 | "Check for mutable `Map`/`Set` key type" |
82 | } | |
83 | ||
487cf647 FG |
84 | #[derive(Clone)] |
85 | pub struct MutableKeyType { | |
86 | ignore_interior_mutability: Vec<String>, | |
87 | ignore_mut_def_ids: FxHashSet<hir::def_id::DefId>, | |
88 | } | |
89 | ||
90 | impl_lint_pass!(MutableKeyType => [ MUTABLE_KEY_TYPE ]); | |
f20569fa XL |
91 | |
92 | impl<'tcx> LateLintPass<'tcx> for MutableKeyType { | |
487cf647 FG |
93 | fn check_crate(&mut self, cx: &LateContext<'tcx>) { |
94 | self.ignore_mut_def_ids.clear(); | |
95 | let mut path = Vec::new(); | |
96 | for ty in &self.ignore_interior_mutability { | |
97 | path.extend(ty.split("::")); | |
98 | for id in def_path_def_ids(cx, &path[..]) { | |
99 | self.ignore_mut_def_ids.insert(id); | |
100 | } | |
101 | path.clear(); | |
102 | } | |
103 | } | |
104 | ||
f20569fa XL |
105 | fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) { |
106 | if let hir::ItemKind::Fn(ref sig, ..) = item.kind { | |
9ffffee4 | 107 | self.check_sig(cx, item.owner_id.def_id, sig.decl); |
f20569fa XL |
108 | } |
109 | } | |
110 | ||
111 | fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::ImplItem<'tcx>) { | |
112 | if let hir::ImplItemKind::Fn(ref sig, ..) = item.kind { | |
2b03887a | 113 | if trait_ref_of_method(cx, item.owner_id.def_id).is_none() { |
9ffffee4 | 114 | self.check_sig(cx, item.owner_id.def_id, sig.decl); |
f20569fa XL |
115 | } |
116 | } | |
117 | } | |
118 | ||
119 | fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::TraitItem<'tcx>) { | |
120 | if let hir::TraitItemKind::Fn(ref sig, ..) = item.kind { | |
9ffffee4 | 121 | self.check_sig(cx, item.owner_id.def_id, sig.decl); |
f20569fa XL |
122 | } |
123 | } | |
124 | ||
125 | fn check_local(&mut self, cx: &LateContext<'_>, local: &hir::Local<'_>) { | |
126 | if let hir::PatKind::Wild = local.pat.kind { | |
127 | return; | |
128 | } | |
487cf647 | 129 | self.check_ty_(cx, local.span, cx.typeck_results().pat_ty(local.pat)); |
f20569fa XL |
130 | } |
131 | } | |
132 | ||
487cf647 FG |
133 | impl MutableKeyType { |
134 | pub fn new(ignore_interior_mutability: Vec<String>) -> Self { | |
135 | Self { | |
136 | ignore_interior_mutability, | |
137 | ignore_mut_def_ids: FxHashSet::default(), | |
138 | } | |
f20569fa | 139 | } |
f20569fa | 140 | |
9ffffee4 FG |
141 | fn check_sig(&self, cx: &LateContext<'_>, fn_def_id: LocalDefId, decl: &hir::FnDecl<'_>) { |
142 | let fn_sig = cx.tcx.fn_sig(fn_def_id).subst_identity(); | |
487cf647 FG |
143 | for (hir_ty, ty) in iter::zip(decl.inputs, fn_sig.inputs().skip_binder()) { |
144 | self.check_ty_(cx, hir_ty.span, *ty); | |
f20569fa | 145 | } |
487cf647 | 146 | self.check_ty_(cx, decl.output.span(), cx.tcx.erase_late_bound_regions(fn_sig.output())); |
f20569fa | 147 | } |
f20569fa | 148 | |
487cf647 FG |
149 | // We want to lint 1. sets or maps with 2. not immutable key types and 3. no unerased |
150 | // generics (because the compiler cannot ensure immutability for unknown types). | |
151 | fn check_ty_<'tcx>(&self, cx: &LateContext<'tcx>, span: Span, ty: Ty<'tcx>) { | |
152 | let ty = ty.peel_refs(); | |
153 | if let Adt(def, substs) = ty.kind() { | |
154 | let is_keyed_type = [sym::HashMap, sym::BTreeMap, sym::HashSet, sym::BTreeSet] | |
155 | .iter() | |
156 | .any(|diag_item| cx.tcx.is_diagnostic_item(*diag_item, def.did())); | |
353b0b11 FG |
157 | if !is_keyed_type { |
158 | return; | |
c295e0f8 | 159 | } |
487cf647 | 160 | |
353b0b11 FG |
161 | let subst_ty = substs.type_at(0); |
162 | // Determines if a type contains interior mutability which would affect its implementation of | |
163 | // [`Hash`] or [`Ord`]. | |
164 | if is_interior_mut_ty(cx, subst_ty) | |
165 | && !matches!(subst_ty.ty_adt_id(), Some(adt_id) if self.ignore_mut_def_ids.contains(&adt_id)) | |
166 | { | |
167 | span_lint(cx, MUTABLE_KEY_TYPE, span, "mutable key type"); | |
168 | } | |
487cf647 | 169 | } |
f20569fa XL |
170 | } |
171 | } |