Skip to content

Commit 9dc0610

Browse files
committed
map_entry: don't suggest when {e.insert(_); None } would be required
1 parent 816cc54 commit 9dc0610

File tree

8 files changed

+216
-204
lines changed

8 files changed

+216
-204
lines changed

clippy_lints/src/entry.rs

Lines changed: 142 additions & 120 deletions
Large diffs are not rendered by default.

tests/ui/entry_unfixable.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,4 +90,11 @@ mod issue13306 {
9090
}
9191
}
9292

93+
fn issue15307<K: Eq + Hash, V>(mut m: HashMap<K, V>, k: K, v: V) {
94+
if !m.contains_key(&k) {
95+
//~^ map_entry
96+
assert!(m.insert(k, v).is_none());
97+
}
98+
}
99+
93100
fn main() {}

tests/ui/entry_unfixable.stderr

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,5 +42,16 @@ LL | | }
4242
|
4343
= help: consider using the `Entry` API: https://doc.rust-lang.org/std/collections/struct.HashMap.html#entry-api
4444

45-
error: aborting due to 3 previous errors
45+
error: usage of `contains_key` followed by `insert` on a `HashMap`
46+
--> tests/ui/entry_unfixable.rs:94:5
47+
|
48+
LL | / if !m.contains_key(&k) {
49+
LL | |
50+
LL | | assert!(m.insert(k, v).is_none());
51+
LL | | }
52+
| |_____^
53+
|
54+
= help: consider using the `Entry` API: https://doc.rust-lang.org/std/collections/struct.HashMap.html#entry-api
55+
56+
error: aborting due to 4 previous errors
4657

tests/ui/entry_with_else.fixed

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -54,25 +54,6 @@ fn insert_if_absent0<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K,
5454
e.insert(v2);
5555
}
5656
}
57-
58-
match m.entry(k) {
59-
std::collections::hash_map::Entry::Occupied(mut e) => {
60-
//~^ map_entry
61-
if true { Some(e.insert(v)) } else { Some(e.insert(v2)) }
62-
}
63-
std::collections::hash_map::Entry::Vacant(e) => {
64-
e.insert(v);
65-
None
66-
}
67-
};
68-
69-
if let std::collections::hash_map::Entry::Occupied(mut e) = m.entry(k) {
70-
//~^ map_entry
71-
foo();
72-
Some(e.insert(v))
73-
} else {
74-
None
75-
};
7657
}
7758

7859
fn main() {}

tests/ui/entry_with_else.rs

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -45,21 +45,6 @@ fn insert_if_absent0<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K,
4545
} else {
4646
m.insert(k, v2);
4747
}
48-
49-
if m.contains_key(&k) {
50-
//~^ map_entry
51-
if true { m.insert(k, v) } else { m.insert(k, v2) }
52-
} else {
53-
m.insert(k, v)
54-
};
55-
56-
if m.contains_key(&k) {
57-
//~^ map_entry
58-
foo();
59-
m.insert(k, v)
60-
} else {
61-
None
62-
};
6348
}
6449

6550
fn main() {}

tests/ui/entry_with_else.stderr

Lines changed: 1 addition & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -114,53 +114,5 @@ LL + }
114114
LL + }
115115
|
116116

117-
error: usage of `contains_key` followed by `insert` on a `HashMap`
118-
--> tests/ui/entry_with_else.rs:49:5
119-
|
120-
LL | / if m.contains_key(&k) {
121-
LL | |
122-
LL | | if true { m.insert(k, v) } else { m.insert(k, v2) }
123-
LL | | } else {
124-
LL | | m.insert(k, v)
125-
LL | | };
126-
| |_____^
127-
|
128-
help: try
129-
|
130-
LL ~ match m.entry(k) {
131-
LL + std::collections::hash_map::Entry::Occupied(mut e) => {
132-
LL +
133-
LL + if true { Some(e.insert(v)) } else { Some(e.insert(v2)) }
134-
LL + }
135-
LL + std::collections::hash_map::Entry::Vacant(e) => {
136-
LL + e.insert(v);
137-
LL + None
138-
LL + }
139-
LL ~ };
140-
|
141-
142-
error: usage of `contains_key` followed by `insert` on a `HashMap`
143-
--> tests/ui/entry_with_else.rs:56:5
144-
|
145-
LL | / if m.contains_key(&k) {
146-
LL | |
147-
LL | | foo();
148-
LL | | m.insert(k, v)
149-
LL | | } else {
150-
LL | | None
151-
LL | | };
152-
| |_____^
153-
|
154-
help: try
155-
|
156-
LL ~ if let std::collections::hash_map::Entry::Occupied(mut e) = m.entry(k) {
157-
LL +
158-
LL + foo();
159-
LL + Some(e.insert(v))
160-
LL + } else {
161-
LL + None
162-
LL ~ };
163-
|
164-
165-
error: aborting due to 7 previous errors
117+
error: aborting due to 5 previous errors
166118

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
//@no-rustfix
2+
#![warn(clippy::map_entry)]
3+
4+
use std::collections::HashMap;
5+
use std::hash::Hash;
6+
7+
fn foo() {}
8+
9+
fn insert_if_absent0<K: Eq + Hash + Copy, V: Copy>(m: &mut HashMap<K, V>, k: K, v: V, v2: V) {
10+
if m.contains_key(&k) {
11+
//~^ map_entry
12+
if true { m.insert(k, v) } else { m.insert(k, v2) }
13+
} else {
14+
m.insert(k, v)
15+
};
16+
17+
// example from Known problems
18+
if !m.contains_key(&k) {
19+
//~^ map_entry
20+
m.insert(k, v)
21+
} else {
22+
None
23+
};
24+
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
error: usage of `contains_key` followed by `insert` on a `HashMap`
2+
--> tests/ui/entry_with_else_unfixable.rs:10:5
3+
|
4+
LL | / if m.contains_key(&k) {
5+
LL | |
6+
LL | | if true { m.insert(k, v) } else { m.insert(k, v2) }
7+
LL | | } else {
8+
LL | | m.insert(k, v)
9+
LL | | };
10+
| |_____^
11+
|
12+
= help: consider using the `Entry` API: https://doc.rust-lang.org/std/collections/struct.HashMap.html#entry-api
13+
= note: `-D clippy::map-entry` implied by `-D warnings`
14+
= help: to override `-D warnings` add `#[allow(clippy::map_entry)]`
15+
16+
error: usage of `contains_key` followed by `insert` on a `HashMap`
17+
--> tests/ui/entry_with_else_unfixable.rs:18:5
18+
|
19+
LL | / if !m.contains_key(&k) {
20+
LL | |
21+
LL | | m.insert(k, v)
22+
LL | | } else {
23+
LL | | None
24+
LL | | };
25+
| |_____^
26+
|
27+
= help: consider using the `Entry` API: https://doc.rust-lang.org/std/collections/struct.HashMap.html#entry-api
28+
29+
error: aborting due to 2 previous errors
30+

0 commit comments

Comments
 (0)