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