fix: set-contains-or-insert FP when set is mutated before insert

Author: profetia

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()
 }

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<usize>, v: usize) {
+    if !s.contains(&v) {
+        s.clear();
+        s.insert(v);
+    }
+
+    fn borrow_as_mut(v: usize, s: &mut HashSet<usize>) {
+        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);
+    }
+}

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