Skip to content

Commit 0bfc9c9

Browse files
JarchoJarcho
committed
Minor cleanup of map_entry and a few additional tests.
1 parent e1cbbef commit 0bfc9c9

8 files changed

+136
-85
lines changed

‎clippy_lints/src/entry.rs

+11-6
Original file line numberDiff line numberDiff line change
@@ -530,13 +530,17 @@ impl InsertSearchResults<'tcx> {
530530
app,
531531
));
532532
if is_expr_used_or_unified(cx.tcx, insertion.call) {
533-
res.push_str("Some(e.insert(");
534-
res.push_str(&snippet_with_context(cx, insertion.value.span, ctxt, "..", app).0);
535-
res.push_str("))");
533+
let _ = write!(
534+
res,
535+
"Some(e.insert({}))",
536+
snippet_with_context(cx, insertion.value.span, ctxt, "..", app).0
537+
);
536538
} else {
537-
res.push_str("e.insert(");
538-
res.push_str(&snippet_with_context(cx, insertion.value.span, ctxt, "..", app).0);
539-
res.push(')');
539+
let _ = write!(
540+
res,
541+
"e.insert({})",
542+
snippet_with_context(cx, insertion.value.span, ctxt, "..", app).0
543+
);
540544
}
541545
span = span.trim_start(insertion.call.span).unwrap_or(DUMMY_SP);
542546
}
@@ -607,6 +611,7 @@ impl InsertSearchResults<'tcx> {
607611
res
608612
}
609613
}
614+
610615
fn find_insert_calls(
611616
cx: &LateContext<'tcx>,
612617
contains_expr: &ContainsExpr<'tcx>,

‎tests/ui/entry.fixed

+53-5
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#![allow(unused, clippy::needless_pass_by_value, clippy::collapsible_if)]
44
#![warn(clippy::map_entry)]
5+
#![feature(asm)]
56

67
use std::collections::{BTreeMap, HashMap};
78
use std::hash::Hash;
@@ -10,11 +11,19 @@ macro_rules! m {
1011
($e:expr) => {{ $e }};
1112
}
1213

14+
macro_rules! insert {
15+
($map:expr, $key:expr, $val:expr) => {
16+
$map.insert($key, $val)
17+
};
18+
}
19+
1320
fn foo() {}
1421

15-
fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2: V) {
22+
fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, m2: &mut HashMap<K, V>, k: K, k2: K, v: V, v2: V) {
23+
// or_insert(v)
1624
m.entry(k).or_insert(v);
1725

26+
// semicolon on insert, use or_insert_with(..)
1827
m.entry(k).or_insert_with(|| {
1928
if true {
2029
v
@@ -23,6 +32,7 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
2332
}
2433
});
2534

35+
// semicolon on if, use or_insert_with(..)
2636
m.entry(k).or_insert_with(|| {
2737
if true {
2838
v
@@ -31,6 +41,7 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
3141
}
3242
});
3343

44+
// early return, use if let
3445
if let std::collections::hash_map::Entry::Vacant(e) = m.entry(k) {
3546
if true {
3647
e.insert(v);
@@ -40,11 +51,13 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
4051
}
4152
}
4253

54+
// use or_insert_with(..)
4355
m.entry(k).or_insert_with(|| {
4456
foo();
4557
v
4658
});
4759

60+
// semicolon on insert and match, use or_insert_with(..)
4861
m.entry(k).or_insert_with(|| {
4962
match 0 {
5063
1 if true => {
@@ -56,18 +69,17 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
5669
}
5770
});
5871

72+
// one branch doesn't insert, use if let
5973
if let std::collections::hash_map::Entry::Vacant(e) = m.entry(k) {
6074
match 0 {
61-
0 => {},
62-
1 => {
63-
e.insert(v);
64-
},
75+
0 => foo(),
6576
_ => {
6677
e.insert(v2);
6778
},
6879
};
6980
}
7081

82+
// use or_insert_with
7183
m.entry(k).or_insert_with(|| {
7284
foo();
7385
match 0 {
@@ -94,10 +106,46 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
94106
}
95107
});
96108

109+
// ok, insert in loop
110+
if !m.contains_key(&k) {
111+
for _ in 0..2 {
112+
m.insert(k, v);
113+
}
114+
}
115+
116+
// macro_expansion test, use or_insert(..)
97117
m.entry(m!(k)).or_insert_with(|| m!(v));
118+
119+
// ok, map used before insertion
120+
if !m.contains_key(&k) {
121+
let _ = m.len();
122+
m.insert(k, v);
123+
}
124+
125+
// ok, inline asm
126+
if !m.contains_key(&k) {
127+
unsafe { asm!("nop") }
128+
m.insert(k, v);
129+
}
130+
131+
// ok, different keys.
132+
if !m.contains_key(&k) {
133+
m.insert(k2, v);
134+
}
135+
136+
// ok, different maps
137+
if !m.contains_key(&k) {
138+
m2.insert(k, v);
139+
}
140+
141+
// ok, insert in macro
142+
if !m.contains_key(&k) {
143+
insert!(m, k, v);
144+
}
98145
}
99146

100147
fn btree_map<K: Eq + Ord + Copy, V: Copy>(m: &mut BTreeMap<K, V>, k: K, v: V, v2: V) {
148+
// insert then do something, use if let
101149
if let std::collections::btree_map::Entry::Vacant(e) = m.entry(k) {
102150
e.insert(v);
103151
foo();

‎tests/ui/entry.rs

+53-5
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#![allow(unused, clippy::needless_pass_by_value, clippy::collapsible_if)]
44
#![warn(clippy::map_entry)]
5+
#![feature(asm)]
56

67
use std::collections::{BTreeMap, HashMap};
78
use std::hash::Hash;
@@ -10,13 +11,21 @@ macro_rules! m {
1011
($e:expr) => {{ $e }};
1112
}
1213

14+
macro_rules! insert {
15+
($map:expr, $key:expr, $val:expr) => {
16+
$map.insert($key, $val)
17+
};
18+
}
19+
1320
fn foo() {}
1421

15-
fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2: V) {
22+
fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, m2: &mut HashMap<K, V>, k: K, k2: K, v: V, v2: V) {
23+
// or_insert(v)
1624
if !m.contains_key(&k) {
1725
m.insert(k, v);
1826
}
1927

28+
// semicolon on insert, use or_insert_with(..)
2029
if !m.contains_key(&k) {
2130
if true {
2231
m.insert(k, v);
@@ -25,6 +34,7 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
2534
}
2635
}
2736

37+
// semicolon on if, use or_insert_with(..)
2838
if !m.contains_key(&k) {
2939
if true {
3040
m.insert(k, v)
@@ -33,6 +43,7 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
3343
};
3444
}
3545

46+
// early return, use if let
3647
if !m.contains_key(&k) {
3748
if true {
3849
m.insert(k, v);
@@ -42,11 +53,13 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
4253
}
4354
}
4455

56+
// use or_insert_with(..)
4557
if !m.contains_key(&k) {
4658
foo();
4759
m.insert(k, v);
4860
}
4961

62+
// semicolon on insert and match, use or_insert_with(..)
5063
if !m.contains_key(&k) {
5164
match 0 {
5265
1 if true => {
@@ -58,18 +71,17 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
5871
};
5972
}
6073

74+
// one branch doesn't insert, use if let
6175
if !m.contains_key(&k) {
6276
match 0 {
63-
0 => {},
64-
1 => {
65-
m.insert(k, v);
66-
},
77+
0 => foo(),
6778
_ => {
6879
m.insert(k, v2);
6980
},
7081
};
7182
}
7283

84+
// use or_insert_with
7385
if !m.contains_key(&k) {
7486
foo();
7587
match 0 {
@@ -96,12 +108,48 @@ fn hash_map<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2:
96108
}
97109
}
98110

111+
// ok, insert in loop
112+
if !m.contains_key(&k) {
113+
for _ in 0..2 {
114+
m.insert(k, v);
115+
}
116+
}
117+
118+
// macro_expansion test, use or_insert(..)
99119
if !m.contains_key(&m!(k)) {
100120
m.insert(m!(k), m!(v));
101121
}
122+
123+
// ok, map used before insertion
124+
if !m.contains_key(&k) {
125+
let _ = m.len();
126+
m.insert(k, v);
127+
}
128+
129+
// ok, inline asm
130+
if !m.contains_key(&k) {
131+
unsafe { asm!("nop") }
132+
m.insert(k, v);
133+
}
134+
135+
// ok, different keys.
136+
if !m.contains_key(&k) {
137+
m.insert(k2, v);
138+
}
139+
140+
// ok, different maps
141+
if !m.contains_key(&k) {
142+
m2.insert(k, v);
143+
}
144+
145+
// ok, insert in macro
146+
if !m.contains_key(&k) {
147+
insert!(m, k, v);
148+
}
102149
}
103150

104151
fn btree_map<K: Eq + Ord + Copy, V: Copy>(m: &mut BTreeMap<K, V>, k: K, v: V, v2: V) {
152+
// insert then do something, use if let
105153
if !m.contains_key(&k) {
106154
m.insert(k, v);
107155
foo();

‎tests/ui/entry.stderr

+15-15
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: usage of `contains_key` followed by `insert` on a `HashMap`
2-
--> $DIR/entry.rs:16:5
2+
--> $DIR/entry.rs:24:5
33
|
44
LL | / if !m.contains_key(&k) {
55
LL | | m.insert(k, v);
@@ -9,7 +9,7 @@ LL | | }
99
= note: `-D clippy::map-entry` implied by `-D warnings`
1010

1111
error: usage of `contains_key` followed by `insert` on a `HashMap`
12-
--> $DIR/entry.rs:20:5
12+
--> $DIR/entry.rs:29:5
1313
|
1414
LL | / if !m.contains_key(&k) {
1515
LL | | if true {
@@ -31,7 +31,7 @@ LL | }
3131
...
3232

3333
error: usage of `contains_key` followed by `insert` on a `HashMap`
34-
--> $DIR/entry.rs:28:5
34+
--> $DIR/entry.rs:38:5
3535
|
3636
LL | / if !m.contains_key(&k) {
3737
LL | | if true {
@@ -53,7 +53,7 @@ LL | }
5353
...
5454

5555
error: usage of `contains_key` followed by `insert` on a `HashMap`
56-
--> $DIR/entry.rs:36:5
56+
--> $DIR/entry.rs:47:5
5757
|
5858
LL | / if !m.contains_key(&k) {
5959
LL | | if true {
@@ -75,7 +75,7 @@ LL | return;
7575
...
7676

7777
error: usage of `contains_key` followed by `insert` on a `HashMap`
78-
--> $DIR/entry.rs:45:5
78+
--> $DIR/entry.rs:57:5
7979
|
8080
LL | / if !m.contains_key(&k) {
8181
LL | | foo();
@@ -92,7 +92,7 @@ LL | });
9292
|
9393

9494
error: usage of `contains_key` followed by `insert` on a `HashMap`
95-
--> $DIR/entry.rs:50:5
95+
--> $DIR/entry.rs:63:5
9696
|
9797
LL | / if !m.contains_key(&k) {
9898
LL | | match 0 {
@@ -114,12 +114,12 @@ LL | _ => {
114114
...
115115

116116
error: usage of `contains_key` followed by `insert` on a `HashMap`
117-
--> $DIR/entry.rs:61:5
117+
--> $DIR/entry.rs:75:5
118118
|
119119
LL | / if !m.contains_key(&k) {
120120
LL | | match 0 {
121-
LL | | 0 => {},
122-
LL | | 1 => {
121+
LL | | 0 => foo(),
122+
LL | | _ => {
123123
... |
124124
LL | | };
125125
LL | | }
@@ -129,14 +129,14 @@ help: try this
129129
|
130130
LL | if let std::collections::hash_map::Entry::Vacant(e) = m.entry(k) {
131131
LL | match 0 {
132-
LL | 0 => {},
133-
LL | 1 => {
134-
LL | e.insert(v);
132+
LL | 0 => foo(),
133+
LL | _ => {
134+
LL | e.insert(v2);
135135
LL | },
136136
...
137137

138138
error: usage of `contains_key` followed by `insert` on a `HashMap`
139-
--> $DIR/entry.rs:73:5
139+
--> $DIR/entry.rs:85:5
140140
|
141141
LL | / if !m.contains_key(&k) {
142142
LL | | foo();
@@ -158,15 +158,15 @@ LL | },
158158
...
159159

160160
error: usage of `contains_key` followed by `insert` on a `HashMap`
161-
--> $DIR/entry.rs:99:5
161+
--> $DIR/entry.rs:119:5
162162
|
163163
LL | / if !m.contains_key(&m!(k)) {
164164
LL | | m.insert(m!(k), m!(v));
165165
LL | | }
166166
| |_____^ help: try this: `m.entry(m!(k)).or_insert_with(|| m!(v));`
167167

168168
error: usage of `contains_key` followed by `insert` on a `BTreeMap`
169-
--> $DIR/entry.rs:105:5
169+
--> $DIR/entry.rs:153:5
170170
|
171171
LL | / if !m.contains_key(&k) {
172172
LL | | m.insert(k, v);

0 commit comments

Comments
 (0)