Skip to content

Commit d0c1605

Browse files
khueykhuey
committed
Make the iter_kv_map lint handle ref/mut annotations.
For the degenerate (`map(|(k, _)| k)`/`map(|(_, v)| v)`) cases a mut annotation is superfluous and a ref annotation won't compile, so no additional handling is required. For cases where the `map` call must be preserved ref/mut annotations should also be presereved so that the map body continues to work as expected.
1 parent 0bca8dd commit d0c1605

File tree

4 files changed

+174
-34
lines changed

4 files changed

+174
-34
lines changed

‎clippy_lints/src/methods/iter_kv_map.rs

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use clippy_utils::source::{snippet, snippet_with_applicability};
66
use clippy_utils::sugg;
77
use clippy_utils::ty::is_type_diagnostic_item;
88
use clippy_utils::visitors::is_local_used;
9-
use rustc_hir::{BindingAnnotation, Body, BorrowKind, Expr, ExprKind, Mutability, Pat, PatKind};
9+
use rustc_hir::{BindingAnnotation, Body, BorrowKind, ByRef, Expr, ExprKind, Mutability, Pat, PatKind};
1010
use rustc_lint::{LateContext, LintContext};
1111
use rustc_middle::ty;
1212
use rustc_span::sym;
@@ -30,9 +30,9 @@ pub(super) fn check<'tcx>(
3030
if let Body {params: [p], value: body_expr, generator_kind: _ } = cx.tcx.hir().body(c.body);
3131
if let PatKind::Tuple([key_pat, val_pat], _) = p.pat.kind;
3232

33-
let (replacement_kind, binded_ident) = match (&key_pat.kind, &val_pat.kind) {
34-
(key, PatKind::Binding(_, _, value, _)) if pat_is_wild(cx, key, m_arg) => ("value", value),
35-
(PatKind::Binding(_, _, key, _), value) if pat_is_wild(cx, value, m_arg) => ("key", key),
33+
let (replacement_kind, annotation, binded_ident) = match (&key_pat.kind, &val_pat.kind) {
34+
(key, PatKind::Binding(ann, _, value, _)) if pat_is_wild(cx, key, m_arg) => ("value", ann, value),
35+
(PatKind::Binding(ann, _, key, _), value) if pat_is_wild(cx, value, m_arg) => ("key", ann, key),
3636
_ => return,
3737
};
3838

@@ -60,13 +60,23 @@ pub(super) fn check<'tcx>(
6060
applicability,
6161
);
6262
} else {
63+
let ref_annotation = if annotation.0 == ByRef::Yes {
64+
"ref "
65+
} else {
66+
""
67+
};
68+
let mut_annotation = if annotation.1 == Mutability::Mut {
69+
"mut "
70+
} else {
71+
""
72+
};
6373
span_lint_and_sugg(
6474
cx,
6575
ITER_KV_MAP,
6676
expr.span,
6777
&format!("iterating on a map's {replacement_kind}s"),
6878
"try",
69-
format!("{recv_snippet}.{into_prefix}{replacement_kind}s().map(|{binded_ident}| {})",
79+
format!("{recv_snippet}.{into_prefix}{replacement_kind}s().map(|{ref_annotation}{mut_annotation}{binded_ident}| {})",
7080
snippet_with_applicability(cx, body_expr.span, "/* body */", &mut applicability)),
7181
applicability,
7282
);

‎tests/ui/iter_kv_map.fixed

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
// run-rustfix
22

33
#![warn(clippy::iter_kv_map)]
4-
#![allow(clippy::redundant_clone)]
5-
#![allow(clippy::suspicious_map)]
6-
#![allow(clippy::map_identity)]
4+
#![allow(unused_mut, clippy::redundant_clone, clippy::suspicious_map, clippy::map_identity)]
75

86
use std::collections::{BTreeMap, HashMap};
97

108
fn main() {
119
let get_key = |(key, _val)| key;
10+
fn ref_acceptor(v: &u32) -> u32 {
11+
*v
12+
}
1213

1314
let map: HashMap<u32, u32> = HashMap::new();
1415

@@ -36,6 +37,20 @@ fn main() {
3637
let _ = map.keys().map(|key| key * 9).count();
3738
let _ = map.values().map(|value| value * 17).count();
3839

40+
// Preserve the ref in the fix.
41+
let _ = map.clone().into_values().map(|ref val| ref_acceptor(val)).count();
42+
43+
// Preserve the mut in the fix.
44+
let _ = map
45+
.clone().into_values().map(|mut val| {
46+
val += 2;
47+
val
48+
})
49+
.count();
50+
51+
// Don't let a mut interfere.
52+
let _ = map.clone().into_values().count();
53+
3954
let map: BTreeMap<u32, u32> = BTreeMap::new();
4055

4156
let _ = map.keys().collect::<Vec<_>>();
@@ -61,4 +76,18 @@ fn main() {
6176
// Lint
6277
let _ = map.keys().map(|key| key * 9).count();
6378
let _ = map.values().map(|value| value * 17).count();
79+
80+
// Preserve the ref in the fix.
81+
let _ = map.clone().into_values().map(|ref val| ref_acceptor(val)).count();
82+
83+
// Preserve the mut in the fix.
84+
let _ = map
85+
.clone().into_values().map(|mut val| {
86+
val += 2;
87+
val
88+
})
89+
.count();
90+
91+
// Don't let a mut interfere.
92+
let _ = map.clone().into_values().count();
6493
}

‎tests/ui/iter_kv_map.rs

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
// run-rustfix
22

33
#![warn(clippy::iter_kv_map)]
4-
#![allow(clippy::redundant_clone)]
5-
#![allow(clippy::suspicious_map)]
6-
#![allow(clippy::map_identity)]
4+
#![allow(unused_mut, clippy::redundant_clone, clippy::suspicious_map, clippy::map_identity)]
75

86
use std::collections::{BTreeMap, HashMap};
97

108
fn main() {
119
let get_key = |(key, _val)| key;
10+
fn ref_acceptor(v: &u32) -> u32 {
11+
*v
12+
}
1213

1314
let map: HashMap<u32, u32> = HashMap::new();
1415

@@ -36,6 +37,22 @@ fn main() {
3637
let _ = map.iter().map(|(key, _value)| key * 9).count();
3738
let _ = map.iter().map(|(_key, value)| value * 17).count();
3839

40+
// Preserve the ref in the fix.
41+
let _ = map.clone().into_iter().map(|(_, ref val)| ref_acceptor(val)).count();
42+
43+
// Preserve the mut in the fix.
44+
let _ = map
45+
.clone()
46+
.into_iter()
47+
.map(|(_, mut val)| {
48+
val += 2;
49+
val
50+
})
51+
.count();
52+
53+
// Don't let a mut interfere.
54+
let _ = map.clone().into_iter().map(|(_, mut val)| val).count();
55+
3956
let map: BTreeMap<u32, u32> = BTreeMap::new();
4057

4158
let _ = map.iter().map(|(key, _)| key).collect::<Vec<_>>();
@@ -61,4 +78,20 @@ fn main() {
6178
// Lint
6279
let _ = map.iter().map(|(key, _value)| key * 9).count();
6380
let _ = map.iter().map(|(_key, value)| value * 17).count();
81+
82+
// Preserve the ref in the fix.
83+
let _ = map.clone().into_iter().map(|(_, ref val)| ref_acceptor(val)).count();
84+
85+
// Preserve the mut in the fix.
86+
let _ = map
87+
.clone()
88+
.into_iter()
89+
.map(|(_, mut val)| {
90+
val += 2;
91+
val
92+
})
93+
.count();
94+
95+
// Don't let a mut interfere.
96+
let _ = map.clone().into_iter().map(|(_, mut val)| val).count();
6497
}

‎tests/ui/iter_kv_map.stderr

Lines changed: 91 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,136 +1,204 @@
11
error: iterating on a map's keys
2-
--> $DIR/iter_kv_map.rs:15:13
2+
--> $DIR/iter_kv_map.rs:16:13
33
|
44
LL | let _ = map.iter().map(|(key, _)| key).collect::<Vec<_>>();
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.keys()`
66
|
77
= note: `-D clippy::iter-kv-map` implied by `-D warnings`
88

99
error: iterating on a map's values
10-
--> $DIR/iter_kv_map.rs:16:13
10+
--> $DIR/iter_kv_map.rs:17:13
1111
|
1212
LL | let _ = map.iter().map(|(_, value)| value).collect::<Vec<_>>();
1313
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.values()`
1414

1515
error: iterating on a map's values
16-
--> $DIR/iter_kv_map.rs:17:13
16+
--> $DIR/iter_kv_map.rs:18:13
1717
|
1818
LL | let _ = map.iter().map(|(_, v)| v + 2).collect::<Vec<_>>();
1919
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.values().map(|v| v + 2)`
2020

2121
error: iterating on a map's keys
22-
--> $DIR/iter_kv_map.rs:19:13
22+
--> $DIR/iter_kv_map.rs:20:13
2323
|
2424
LL | let _ = map.clone().into_iter().map(|(key, _)| key).collect::<Vec<_>>();
2525
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_keys()`
2626

2727
error: iterating on a map's keys
28-
--> $DIR/iter_kv_map.rs:20:13
28+
--> $DIR/iter_kv_map.rs:21:13
2929
|
3030
LL | let _ = map.clone().into_iter().map(|(key, _)| key + 2).collect::<Vec<_>>();
3131
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_keys().map(|key| key + 2)`
3232

3333
error: iterating on a map's values
34-
--> $DIR/iter_kv_map.rs:22:13
34+
--> $DIR/iter_kv_map.rs:23:13
3535
|
3636
LL | let _ = map.clone().into_iter().map(|(_, val)| val).collect::<Vec<_>>();
3737
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_values()`
3838

3939
error: iterating on a map's values
40-
--> $DIR/iter_kv_map.rs:23:13
40+
--> $DIR/iter_kv_map.rs:24:13
4141
|
4242
LL | let _ = map.clone().into_iter().map(|(_, val)| val + 2).collect::<Vec<_>>();
4343
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_values().map(|val| val + 2)`
4444

4545
error: iterating on a map's values
46-
--> $DIR/iter_kv_map.rs:25:13
46+
--> $DIR/iter_kv_map.rs:26:13
4747
|
4848
LL | let _ = map.clone().iter().map(|(_, val)| val).collect::<Vec<_>>();
4949
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().values()`
5050

5151
error: iterating on a map's keys
52-
--> $DIR/iter_kv_map.rs:26:13
52+
--> $DIR/iter_kv_map.rs:27:13
5353
|
5454
LL | let _ = map.iter().map(|(key, _)| key).filter(|x| *x % 2 == 0).count();
5555
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.keys()`
5656

5757
error: iterating on a map's keys
58-
--> $DIR/iter_kv_map.rs:36:13
58+
--> $DIR/iter_kv_map.rs:37:13
5959
|
6060
LL | let _ = map.iter().map(|(key, _value)| key * 9).count();
6161
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.keys().map(|key| key * 9)`
6262

6363
error: iterating on a map's values
64-
--> $DIR/iter_kv_map.rs:37:13
64+
--> $DIR/iter_kv_map.rs:38:13
6565
|
6666
LL | let _ = map.iter().map(|(_key, value)| value * 17).count();
6767
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.values().map(|value| value * 17)`
6868

69-
error: iterating on a map's keys
69+
error: iterating on a map's values
7070
--> $DIR/iter_kv_map.rs:41:13
7171
|
72+
LL | let _ = map.clone().into_iter().map(|(_, ref val)| ref_acceptor(val)).count();
73+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_values().map(|ref val| ref_acceptor(val))`
74+
75+
error: iterating on a map's values
76+
--> $DIR/iter_kv_map.rs:44:13
77+
|
78+
LL | let _ = map
79+
| _____________^
80+
LL | | .clone()
81+
LL | | .into_iter()
82+
LL | | .map(|(_, mut val)| {
83+
LL | | val += 2;
84+
LL | | val
85+
LL | | })
86+
| |__________^
87+
|
88+
help: try
89+
|
90+
LL ~ let _ = map
91+
LL + .clone().into_values().map(|mut val| {
92+
LL + val += 2;
93+
LL + val
94+
LL + })
95+
|
96+
97+
error: iterating on a map's values
98+
--> $DIR/iter_kv_map.rs:54:13
99+
|
100+
LL | let _ = map.clone().into_iter().map(|(_, mut val)| val).count();
101+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_values()`
102+
103+
error: iterating on a map's keys
104+
--> $DIR/iter_kv_map.rs:58:13
105+
|
72106
LL | let _ = map.iter().map(|(key, _)| key).collect::<Vec<_>>();
73107
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.keys()`
74108

75109
error: iterating on a map's values
76-
--> $DIR/iter_kv_map.rs:42:13
110+
--> $DIR/iter_kv_map.rs:59:13
77111
|
78112
LL | let _ = map.iter().map(|(_, value)| value).collect::<Vec<_>>();
79113
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.values()`
80114

81115
error: iterating on a map's values
82-
--> $DIR/iter_kv_map.rs:43:13
116+
--> $DIR/iter_kv_map.rs:60:13
83117
|
84118
LL | let _ = map.iter().map(|(_, v)| v + 2).collect::<Vec<_>>();
85119
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.values().map(|v| v + 2)`
86120

87121
error: iterating on a map's keys
88-
--> $DIR/iter_kv_map.rs:45:13
122+
--> $DIR/iter_kv_map.rs:62:13
89123
|
90124
LL | let _ = map.clone().into_iter().map(|(key, _)| key).collect::<Vec<_>>();
91125
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_keys()`
92126

93127
error: iterating on a map's keys
94-
--> $DIR/iter_kv_map.rs:46:13
128+
--> $DIR/iter_kv_map.rs:63:13
95129
|
96130
LL | let _ = map.clone().into_iter().map(|(key, _)| key + 2).collect::<Vec<_>>();
97131
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_keys().map(|key| key + 2)`
98132

99133
error: iterating on a map's values
100-
--> $DIR/iter_kv_map.rs:48:13
134+
--> $DIR/iter_kv_map.rs:65:13
101135
|
102136
LL | let _ = map.clone().into_iter().map(|(_, val)| val).collect::<Vec<_>>();
103137
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_values()`
104138

105139
error: iterating on a map's values
106-
--> $DIR/iter_kv_map.rs:49:13
140+
--> $DIR/iter_kv_map.rs:66:13
107141
|
108142
LL | let _ = map.clone().into_iter().map(|(_, val)| val + 2).collect::<Vec<_>>();
109143
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_values().map(|val| val + 2)`
110144

111145
error: iterating on a map's values
112-
--> $DIR/iter_kv_map.rs:51:13
146+
--> $DIR/iter_kv_map.rs:68:13
113147
|
114148
LL | let _ = map.clone().iter().map(|(_, val)| val).collect::<Vec<_>>();
115149
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().values()`
116150

117151
error: iterating on a map's keys
118-
--> $DIR/iter_kv_map.rs:52:13
152+
--> $DIR/iter_kv_map.rs:69:13
119153
|
120154
LL | let _ = map.iter().map(|(key, _)| key).filter(|x| *x % 2 == 0).count();
121155
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.keys()`
122156

123157
error: iterating on a map's keys
124-
--> $DIR/iter_kv_map.rs:62:13
158+
--> $DIR/iter_kv_map.rs:79:13
125159
|
126160
LL | let _ = map.iter().map(|(key, _value)| key * 9).count();
127161
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.keys().map(|key| key * 9)`
128162

129163
error: iterating on a map's values
130-
--> $DIR/iter_kv_map.rs:63:13
164+
--> $DIR/iter_kv_map.rs:80:13
131165
|
132166
LL | let _ = map.iter().map(|(_key, value)| value * 17).count();
133167
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.values().map(|value| value * 17)`
134168

135-
error: aborting due to 22 previous errors
169+
error: iterating on a map's values
170+
--> $DIR/iter_kv_map.rs:83:13
171+
|
172+
LL | let _ = map.clone().into_iter().map(|(_, ref val)| ref_acceptor(val)).count();
173+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_values().map(|ref val| ref_acceptor(val))`
174+
175+
error: iterating on a map's values
176+
--> $DIR/iter_kv_map.rs:86:13
177+
|
178+
LL | let _ = map
179+
| _____________^
180+
LL | | .clone()
181+
LL | | .into_iter()
182+
LL | | .map(|(_, mut val)| {
183+
LL | | val += 2;
184+
LL | | val
185+
LL | | })
186+
| |__________^
187+
|
188+
help: try
189+
|
190+
LL ~ let _ = map
191+
LL + .clone().into_values().map(|mut val| {
192+
LL + val += 2;
193+
LL + val
194+
LL + })
195+
|
196+
197+
error: iterating on a map's values
198+
--> $DIR/iter_kv_map.rs:96:13
199+
|
200+
LL | let _ = map.clone().into_iter().map(|(_, mut val)| val).count();
201+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_values()`
202+
203+
error: aborting due to 28 previous errors
136204

0 commit comments

Comments
 (0)