[refurb] Implement check-and-remove-from-set rule (FURB132)

crates/ruff/resources/test/fixtures/refurb/FURB132.py

@@ -0,0 +1,80 @@
+from typing import Set
+from some.package.name import foo, bar
+
+
+s = set()
+s2 = {}
+s3: set[int] = foo()
+
+# these should match
+
+# FURB132
+if "x" in s:
+    s.remove("x")
+
+
+# FURB132
+if "x" in s2:
+    s2.remove("x")
+
+
+# FURB132
+if "x" in s3:
+    s3.remove("x")
+
+
+var = "y"
+# FURB132
+if var in s:
+    s.remove(var)
+
+
+if f"{var}:{var}" in s:
+    s.remove(f"{var}:{var}")
+
+
+def identity(x):
+    return x
+
+
+# FURB132
+if identity("x") in s2:
+    s2.remove(identity("x"))
+
+
+# these should not
+
+if "x" in s:
+    s.remove("y")
+
+s.discard("x")
+
+s2 = set()
+
+if "x" in s:
+    s2.remove("x")
+
+if "x" in s:
+    s.remove("x")
+    print("removed item")
+
+if bar() in s:
+    # bar() might have a side effect
+    s.remove(bar())
+
+if "x" in s:
+    s.remove("x")
+else:
+    print("not found")
+
+class Container:
+    def remove(self, item) -> None:
+        return
+
+    def __contains__(self, other) -> bool:
+        return True
+
+c = Container()
+
+if "x" in c:
+    c.remove("x")

crates/ruff/src/checkers/ast/analyze/statement.rs

@@ -1057,6 +1057,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
                     checker.diagnostics.push(diagnostic);
                 }
             }
+            if checker.enabled(Rule::CheckAndRemoveFromSet) {
+                refurb::rules::check_and_remove_from_set(checker, if_);
+            }
             if checker.source_type.is_stub() {
                 if checker.any_enabled(&[
                     Rule::UnrecognizedVersionInfoCheck,

crates/ruff/src/codes.rs

@@ -868,6 +868,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         // refurb
         (Refurb, "113") => (RuleGroup::Unspecified, rules::refurb::rules::RepeatedAppend),
         (Refurb, "131") => (RuleGroup::Unspecified, rules::refurb::rules::DeleteFullSlice),
+        (Refurb, "132") => (RuleGroup::Unspecified, rules::refurb::rules::CheckAndRemoveFromSet),
 
         _ => return None,
     })

crates/ruff/src/rules/refurb/helpers.rs

@@ -6,8 +6,8 @@ use ruff_python_semantic::{Binding, BindingKind};
 use crate::checkers::ast::Checker;
 
 trait TypeChecker {
-    const EXPR_TYPE: PythonType;
     fn match_annotation(checker: &Checker, annotation: &Expr) -> bool;
+    fn match_initializer(checker: &Checker, initializer: &Expr) -> bool;
 }
 
 fn check_type<'a, T: TypeChecker>(checker: &'a Checker, binding: &'a Binding, name: &str) -> bool {
@@ -17,11 +17,7 @@ fn check_type<'a, T: TypeChecker>(checker: &'a Checker, binding: &'a Binding, na
     match binding.kind {
         BindingKind::Assignment => match stmt {
             Stmt::Assign(ast::StmtAssign { value, .. }) => {
-                let value_type: ResolvedPythonType = value.as_ref().into();
-                let ResolvedPythonType::Atom(candidate) = value_type else {
-                    return false;
-                };
-                candidate == T::EXPR_TYPE
+                T::match_initializer(checker, value.as_ref())
             }
             Stmt::AnnAssign(ast::StmtAnnAssign { annotation, .. }) => {
                 T::match_annotation(checker, annotation.as_ref())
@@ -50,9 +46,10 @@ fn check_type<'a, T: TypeChecker>(checker: &'a Checker, binding: &'a Binding, na
     }
 }
 
-trait AnnotationTypeChecker {
+trait BuiltinTypeChecker {
     const TYPING_NAME: &'static str;
     const BUILTIN_TYPE_NAME: &'static str;
+    const EXPR_TYPE: PythonType;
 
     fn match_annotation(checker: &Checker, annotation: &Expr) -> bool {
         let Expr::Subscript(ast::ExprSubscript { value, .. }) = annotation else {
@@ -64,6 +61,25 @@ trait AnnotationTypeChecker {
                 .match_typing_expr(value, Self::TYPING_NAME)
     }
 
+    fn match_initializer(checker: &Checker, initializer: &Expr) -> bool {
+        Self::match_expr_type(initializer) || Self::match_builtin_constructor(checker, initializer)
+    }
+
+    fn match_expr_type(initializer: &Expr) -> bool {
+        let init_type: ResolvedPythonType = initializer.into();
+        match init_type {
+            ResolvedPythonType::Atom(atom) => atom == Self::EXPR_TYPE,
+            _ => false,
+        }
+    }
+
+    fn match_builtin_constructor(checker: &Checker, initializer: &Expr) -> bool {
+        let Expr::Call(ast::ExprCall { func, .. }) = initializer else {
+            return false;
+        };
+        Self::match_builtin_type(checker, func.as_ref())
+    }
+
     fn match_builtin_type(checker: &Checker, type_expr: &Expr) -> bool {
         let Expr::Name(ast::ExprName { id, .. }) = type_expr else {
             return false;
@@ -72,32 +88,38 @@ trait AnnotationTypeChecker {
     }
 }
 
+impl<T: BuiltinTypeChecker> TypeChecker for T {
+    fn match_annotation(checker: &Checker, annotation: &Expr) -> bool {
+        <Self as BuiltinTypeChecker>::match_annotation(checker, annotation)
+    }
+
+    fn match_initializer(checker: &Checker, initializer: &Expr) -> bool {
+        <Self as BuiltinTypeChecker>::match_initializer(checker, initializer)
+    }
+}
+
 struct ListChecker;
 
-impl AnnotationTypeChecker for ListChecker {
+impl BuiltinTypeChecker for ListChecker {
     const TYPING_NAME: &'static str = "List";
     const BUILTIN_TYPE_NAME: &'static str = "list";
-}
-
-impl TypeChecker for ListChecker {
     const EXPR_TYPE: PythonType = PythonType::List;
-    fn match_annotation(checker: &Checker, annotation: &Expr) -> bool {
-        <Self as AnnotationTypeChecker>::match_annotation(checker, annotation)
-    }
 }
 
 struct DictChecker;
 
-impl AnnotationTypeChecker for DictChecker {
+impl BuiltinTypeChecker for DictChecker {
     const TYPING_NAME: &'static str = "Dict";
     const BUILTIN_TYPE_NAME: &'static str = "dict";
+    const EXPR_TYPE: PythonType = PythonType::Dict;
 }
 
-impl TypeChecker for DictChecker {
-    const EXPR_TYPE: PythonType = PythonType::Dict;
-    fn match_annotation(checker: &Checker, annotation: &Expr) -> bool {
-        <Self as AnnotationTypeChecker>::match_annotation(checker, annotation)
-    }
+struct SetChecker;
+
+impl BuiltinTypeChecker for SetChecker {
+    const TYPING_NAME: &'static str = "Set";
+    const BUILTIN_TYPE_NAME: &'static str = "set";
+    const EXPR_TYPE: PythonType = PythonType::Set;
 }
 
 pub(super) fn is_list<'a>(checker: &'a Checker, binding: &'a Binding, name: &str) -> bool {
@@ -108,6 +130,10 @@ pub(super) fn is_dict<'a>(checker: &'a Checker, binding: &'a Binding, name: &str
     check_type::<DictChecker>(checker, binding, name)
 }
 
+pub(super) fn is_set<'a>(checker: &'a Checker, binding: &'a Binding, name: &str) -> bool {
+    check_type::<SetChecker>(checker, binding, name)
+}
+
 #[inline]
 fn find_parameter_by_name<'a>(
     parameters: &'a Parameters,

crates/ruff/src/rules/refurb/mod.rs

@@ -15,6 +15,7 @@ mod tests {
 
     #[test_case(Rule::RepeatedAppend, Path::new("FURB113.py"))]
     #[test_case(Rule::DeleteFullSlice, Path::new("FURB131.py"))]
+    #[test_case(Rule::CheckAndRemoveFromSet, Path::new("FURB132.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
         let diagnostics = test_path(