From f242c46a0a423e3b088c6826636394fd21f788a1 Mon Sep 17 00:00:00 2001 From: Linshu Yang Date: Sun, 2 Nov 2025 19:17:30 +0000 Subject: [PATCH] fix: `set-contains-or-insert` FP when set is mutated before `insert` --- clippy_lints/src/set_contains_or_insert.rs | 21 ++++++++++++++++++--- tests/ui/set_contains_or_insert.rs | 21 +++++++++++++++++++++ tests/ui/set_contains_or_insert.stderr | 11 ++++++++++- 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/set_contains_or_insert.rs b/clippy_lints/src/set_contains_or_insert.rs index 688da33a1777..7482bac4c7b4 100644 --- a/clippy_lints/src/set_contains_or_insert.rs +++ b/clippy_lints/src/set_contains_or_insert.rs @@ -112,6 +112,16 @@ fn try_parse_op_call<'tcx>( None } +fn is_set_mutated<'tcx>(cx: &LateContext<'tcx>, contains_expr: &OpExpr<'tcx>, expr: &'tcx Expr<'_>) -> bool { + // Guard on type to avoid useless potentially expansive `SpanlessEq` checks + cx.typeck_results().expr_ty_adjusted(expr).is_mutable_ptr() + && matches!( + cx.typeck_results().expr_ty(expr).peel_refs().opt_diag_name(cx), + Some(sym::HashSet | sym::BTreeSet) + ) + && SpanlessEq::new(cx).eq_expr(contains_expr.receiver, expr.peel_borrows()) +} + fn find_insert_calls<'tcx>( cx: &LateContext<'tcx>, contains_expr: &OpExpr<'tcx>, @@ -122,9 +132,14 @@ fn find_insert_calls<'tcx>( && SpanlessEq::new(cx).eq_expr(contains_expr.receiver, insert_expr.receiver) && SpanlessEq::new(cx).eq_expr(contains_expr.value, insert_expr.value) { - ControlFlow::Break(insert_expr) - } else { - ControlFlow::Continue(()) + return ControlFlow::Break(Some(insert_expr)); } + + if is_set_mutated(cx, contains_expr, e) { + return ControlFlow::Break(None); + } + + ControlFlow::Continue(()) }) + .flatten() } diff --git a/tests/ui/set_contains_or_insert.rs b/tests/ui/set_contains_or_insert.rs index 575cfda139a4..ac1d74f8afa4 100644 --- a/tests/ui/set_contains_or_insert.rs +++ b/tests/ui/set_contains_or_insert.rs @@ -164,3 +164,24 @@ fn main() { should_not_warn_hashset(); should_not_warn_btreeset(); } + +fn issue15990(s: &mut HashSet, v: usize) { + if !s.contains(&v) { + s.clear(); + s.insert(v); + } + + fn borrow_as_mut(v: usize, s: &mut HashSet) { + s.clear(); + } + if !s.contains(&v) { + borrow_as_mut(v, s); + s.insert(v); + } + + if !s.contains(&v) { + //~^ set_contains_or_insert + let _readonly_access = s.contains(&v); + s.insert(v); + } +} diff --git a/tests/ui/set_contains_or_insert.stderr b/tests/ui/set_contains_or_insert.stderr index 3152b1136458..3b06b63182ab 100644 --- a/tests/ui/set_contains_or_insert.stderr +++ b/tests/ui/set_contains_or_insert.stderr @@ -127,5 +127,14 @@ LL | LL | borrow_set.insert(value); | ^^^^^^^^^^^^^ -error: aborting due to 14 previous errors +error: usage of `HashSet::insert` after `HashSet::contains` + --> tests/ui/set_contains_or_insert.rs:182:11 + | +LL | if !s.contains(&v) { + | ^^^^^^^^^^^^ +... +LL | s.insert(v); + | ^^^^^^^^^ + +error: aborting due to 15 previous errors