Skip to content

Commit 9bbea2f

Browse files
committed
fix: map_entry suggests wrongly for insert with cfg-ed out code
1 parent 3ca0738 commit 9bbea2f

File tree

4 files changed

+49
-4
lines changed

4 files changed

+49
-4
lines changed

clippy_lints/src/entry.rs

Lines changed: 7 additions & 3 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, higher, is_expr_final_block_expr, is_expr_used_or_unified,
7-
peel_hir_expr_while,
7+
peel_hir_expr_while, span_contains_non_whitespace,
88
};
99
use core::fmt::{self, Write};
1010
use rustc_errors::Applicability;
@@ -13,7 +13,7 @@ use rustc_hir::intravisit::{Visitor, walk_body, walk_expr};
1313
use rustc_hir::{Block, Expr, ExprKind, HirId, Pat, Stmt, StmtKind, UnOp};
1414
use rustc_lint::{LateContext, LateLintPass};
1515
use rustc_session::declare_lint_pass;
16-
use rustc_span::{DUMMY_SP, Span, SyntaxContext, sym};
16+
use rustc_span::{BytePos, DUMMY_SP, Pos, Span, SyntaxContext, sym};
1717
use std::ops::ControlFlow;
1818

1919
declare_clippy_lint! {
@@ -166,7 +166,11 @@ impl<'tcx> LateLintPass<'tcx> for HashMapPass {
166166
"if let {}::{entry_kind} = {map_str}.entry({key_str}) {body_str}",
167167
map_ty.entry_path(),
168168
))
169-
} else if let Some(insertion) = then_search.as_single_insertion() {
169+
} else if let Some(insertion) = then_search.as_single_insertion()
170+
&& let span_in_between = then_expr.span.shrink_to_lo().between(insertion.call.span)
171+
&& let span_in_between = span_in_between.with_lo(span_in_between.lo() + BytePos::from_usize(1))
172+
&& !span_contains_non_whitespace(cx, span_in_between, true)
173+
{
170174
let value_str = snippet_with_context(cx, insertion.value.span, then_expr.span.ctxt(), "..", &mut app).0;
171175
if contains_expr.negated {
172176
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
@@ -249,3 +249,13 @@ mod issue14449 {
249249
}
250250

251251
fn main() {}
252+
253+
fn issue15781(m: &mut std::collections::HashMap<i32, i32>, k: i32, v: i32) {
254+
fn very_important_fn() {}
255+
m.entry(k).or_insert_with(|| {
256+
//~^ map_entry
257+
#[cfg(test)]
258+
very_important_fn();
259+
v
260+
});
261+
}

tests/ui/entry.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,3 +255,13 @@ mod issue14449 {
255255
}
256256

257257
fn main() {}
258+
259+
fn issue15781(m: &mut std::collections::HashMap<i32, i32>, k: i32, v: i32) {
260+
fn very_important_fn() {}
261+
if !m.contains_key(&k) {
262+
//~^ map_entry
263+
#[cfg(test)]
264+
very_important_fn();
265+
m.insert(k, v);
266+
}
267+
}

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:261: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)