Skip to content

Commit 95d5990

Browse files
Fix map_entry suggests wrongly for insert with cfg-ed out code (#15800)
Closes #15781 changelog: [`map_entry`] fix wrong suggestions for insert with cfg-ed out code
2 parents 924f153 + fb92c70 commit 95d5990

File tree

4 files changed

+48
-3
lines changed

4 files changed

+48
-3
lines changed

clippy_lints/src/entry.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use clippy_utils::ty::is_copy;
44
use clippy_utils::visitors::for_each_expr;
55
use clippy_utils::{
66
SpanlessEq, can_move_expr_to_closure_no_visit, desugar_await, higher, is_expr_final_block_expr,
7-
is_expr_used_or_unified, paths, peel_hir_expr_while,
7+
is_expr_used_or_unified, paths, peel_hir_expr_while, span_contains_non_whitespace,
88
};
99
use core::fmt::{self, Write};
1010
use rustc_errors::Applicability;
@@ -167,7 +167,11 @@ impl<'tcx> LateLintPass<'tcx> for HashMapPass {
167167
"if let {}::{entry_kind} = {map_str}.entry({key_str}) {body_str}",
168168
map_ty.entry_path(),
169169
))
170-
} else if let Some(insertion) = then_search.as_single_insertion() {
170+
} else if let Some(insertion) = then_search.as_single_insertion()
171+
&& let span_in_between = then_expr.span.shrink_to_lo().between(insertion.call.span)
172+
&& let span_in_between = span_in_between.split_at(1).1
173+
&& !span_contains_non_whitespace(cx, span_in_between, true)
174+
{
171175
let value_str = snippet_with_context(cx, insertion.value.span, then_expr.span.ctxt(), "..", &mut app).0;
172176
if contains_expr.negated {
173177
if insertion.value.can_have_side_effects() {

tests/ui/entry.fixed

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,3 +273,13 @@ mod issue_16173 {
273273
}
274274

275275
fn main() {}
276+
277+
fn issue15781(m: &mut std::collections::HashMap<i32, i32>, k: i32, v: i32) {
278+
fn very_important_fn() {}
279+
m.entry(k).or_insert_with(|| {
280+
//~^ map_entry
281+
#[cfg(test)]
282+
very_important_fn();
283+
v
284+
});
285+
}

tests/ui/entry.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,3 +279,13 @@ mod issue_16173 {
279279
}
280280

281281
fn main() {}
282+
283+
fn issue15781(m: &mut std::collections::HashMap<i32, i32>, k: i32, v: i32) {
284+
fn very_important_fn() {}
285+
if !m.contains_key(&k) {
286+
//~^ map_entry
287+
#[cfg(test)]
288+
very_important_fn();
289+
m.insert(k, v);
290+
}
291+
}

tests/ui/entry.stderr

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,5 +246,26 @@ LL + e.insert(42);
246246
LL + }
247247
|
248248

249-
error: aborting due to 11 previous errors
249+
error: usage of `contains_key` followed by `insert` on a `HashMap`
250+
--> tests/ui/entry.rs:285:5
251+
|
252+
LL | / if !m.contains_key(&k) {
253+
LL | |
254+
LL | | #[cfg(test)]
255+
LL | | very_important_fn();
256+
LL | | m.insert(k, v);
257+
LL | | }
258+
| |_____^
259+
|
260+
help: try
261+
|
262+
LL ~ m.entry(k).or_insert_with(|| {
263+
LL +
264+
LL + #[cfg(test)]
265+
LL + very_important_fn();
266+
LL + v
267+
LL + });
268+
|
269+
270+
error: aborting due to 12 previous errors
250271

0 commit comments

Comments
 (0)