diff --git a/clippy_lints/src/entry.rs b/clippy_lints/src/entry.rs index a5ec6777b434..6bfaffdddf4d 100644 --- a/clippy_lints/src/entry.rs +++ b/clippy_lints/src/entry.rs @@ -4,7 +4,7 @@ use clippy_utils::ty::is_copy; use clippy_utils::visitors::for_each_expr; use clippy_utils::{ SpanlessEq, can_move_expr_to_closure_no_visit, higher, is_expr_final_block_expr, is_expr_used_or_unified, - peel_hir_expr_while, + peel_hir_expr_while, span_contains_non_whitespace, }; use core::fmt::{self, Write}; use rustc_errors::Applicability; @@ -13,7 +13,7 @@ use rustc_hir::intravisit::{Visitor, walk_body, walk_expr}; use rustc_hir::{Block, Expr, ExprKind, HirId, Pat, Stmt, StmtKind, UnOp}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::declare_lint_pass; -use rustc_span::{DUMMY_SP, Span, SyntaxContext, sym}; +use rustc_span::{BytePos, DUMMY_SP, Pos, Span, SyntaxContext, sym}; use std::ops::ControlFlow; declare_clippy_lint! { @@ -166,7 +166,11 @@ impl<'tcx> LateLintPass<'tcx> for HashMapPass { "if let {}::{entry_kind} = {map_str}.entry({key_str}) {body_str}", map_ty.entry_path(), )) - } else if let Some(insertion) = then_search.as_single_insertion() { + } else if let Some(insertion) = then_search.as_single_insertion() + && let span_in_between = then_expr.span.shrink_to_lo().between(insertion.call.span) + && let span_in_between = span_in_between.with_lo(span_in_between.lo() + BytePos::from_usize(1)) + && !span_contains_non_whitespace(cx, span_in_between, true) + { let value_str = snippet_with_context(cx, insertion.value.span, then_expr.span.ctxt(), "..", &mut app).0; if contains_expr.negated { if insertion.value.can_have_side_effects() { diff --git a/tests/ui/entry.fixed b/tests/ui/entry.fixed index f2df9f0204ea..c621c059c5ab 100644 --- a/tests/ui/entry.fixed +++ b/tests/ui/entry.fixed @@ -249,3 +249,13 @@ mod issue14449 { } fn main() {} + +fn issue15781(m: &mut std::collections::HashMap, k: i32, v: i32) { + fn very_important_fn() {} + m.entry(k).or_insert_with(|| { + //~^ map_entry + #[cfg(test)] + very_important_fn(); + v + }); +} diff --git a/tests/ui/entry.rs b/tests/ui/entry.rs index 166eea417ac2..e60215116950 100644 --- a/tests/ui/entry.rs +++ b/tests/ui/entry.rs @@ -255,3 +255,13 @@ mod issue14449 { } fn main() {} + +fn issue15781(m: &mut std::collections::HashMap, k: i32, v: i32) { + fn very_important_fn() {} + if !m.contains_key(&k) { + //~^ map_entry + #[cfg(test)] + very_important_fn(); + m.insert(k, v); + } +} diff --git a/tests/ui/entry.stderr b/tests/ui/entry.stderr index 009b78d29073..22fbc959782a 100644 --- a/tests/ui/entry.stderr +++ b/tests/ui/entry.stderr @@ -246,5 +246,26 @@ LL + e.insert(42); LL + } | -error: aborting due to 11 previous errors +error: usage of `contains_key` followed by `insert` on a `HashMap` + --> tests/ui/entry.rs:261:5 + | +LL | / if !m.contains_key(&k) { +LL | | +LL | | #[cfg(test)] +LL | | very_important_fn(); +LL | | m.insert(k, v); +LL | | } + | |_____^ + | +help: try + | +LL ~ m.entry(k).or_insert_with(|| { +LL + +LL + #[cfg(test)] +LL + very_important_fn(); +LL + v +LL + }); + | + +error: aborting due to 12 previous errors