[ruff] Add support for more patterns (RUF055)

crates/ruff_linter/resources/test/fixtures/ruff/RUF055_0.py

@@ -2,7 +2,7 @@
 
 s = "str"
 
-# this should be replaced with s.replace("abc", "")
+# this should be replaced with `s.replace("abc", "")`
 re.sub("abc", "", s)
 
 
@@ -17,25 +17,25 @@ def dashrepl(matchobj):
 
 re.sub("-", dashrepl, "pro----gram-files")
 
-# this one should be replaced with s.startswith("abc") because the Match is
+# this one should be replaced with `s.startswith("abc")` because the Match is
 # used in an if context for its truth value
 if re.match("abc", s):
     pass
 if m := re.match("abc", s):  # this should *not* be replaced
     pass
 re.match("abc", s)  # this should not be replaced because match returns a Match
 
-# this should be replaced with "abc" in s
+# this should be replaced with `"abc" in s`
 if re.search("abc", s):
     pass
 re.search("abc", s)  # this should not be replaced
 
-# this should be replaced with "abc" == s
+# this should be replaced with `"abc" == s`
 if re.fullmatch("abc", s):
     pass
 re.fullmatch("abc", s)  # this should not be replaced
 
-# this should be replaced with s.split("abc")
+# this should be replaced with `s.split("abc")``
 re.split("abc", s)
 
 # these currently should not be modified because the patterns contain regex

crates/ruff_linter/resources/test/fixtures/ruff/RUF055_2.py

@@ -0,0 +1,27 @@
+"""Patterns that don't just involve the call, but rather the parent expression"""
+import re
+
+s = "str"
+
+# this should be replaced with `"abc" not in s`
+re.search("abc", s) is None
+
+
+# this shuold be replaced with `"abc" in s`
+re.search("abc", s) is not None
+
+
+# this should be replaced with `not s.startswith("abc")`
+re.match("abc", s) is None
+
+
+# this should be replaced with `s.startswith("abc")`
+re.match("abc", s) is not None
+
+
+# this should be replaced with `s != "abc"`
+re.fullmatch("abc", s) is None
+
+
+# this should be replaced with `s == "abc"`
+re.fullmatch("abc", s) is not None

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

@@ -422,6 +422,7 @@ mod tests {
     #[test_case(Rule::UnrawRePattern, Path::new("RUF039_concat.py"))]
     #[test_case(Rule::UnnecessaryRegularExpression, Path::new("RUF055_0.py"))]
     #[test_case(Rule::UnnecessaryRegularExpression, Path::new("RUF055_1.py"))]
+    #[test_case(Rule::UnnecessaryRegularExpression, Path::new("RUF055_2.py"))]
     #[test_case(Rule::UnnecessaryCastToInt, Path::new("RUF046.py"))]
     #[test_case(Rule::PytestRaisesAmbiguousPattern, Path::new("RUF043.py"))]
     #[test_case(Rule::UnnecessaryRound, Path::new("RUF057.py"))]

crates/ruff_linter/src/rules/ruff/rules/unnecessary_regular_expression.rs

@@ -3,11 +3,11 @@ use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Vi
 use ruff_macros::{derive_message_formats, ViolationMetadata};
 use ruff_python_ast::{
     Arguments, CmpOp, Expr, ExprAttribute, ExprCall, ExprCompare, ExprContext, ExprStringLiteral,
-    Identifier,
+    ExprUnaryOp, Identifier, UnaryOp,
 };
 use ruff_python_semantic::analyze::typing::find_binding_value;
 use ruff_python_semantic::{Modules, SemanticModel};
-use ruff_text_size::TextRange;
+use ruff_text_size::{Ranged, TextRange};
 
 use crate::checkers::ast::Checker;
 
@@ -111,21 +111,32 @@ pub(crate) fn unnecessary_regular_expression(checker: &mut Checker, call: &ExprC
         return;
     }
 
-    // Here we know the pattern is a string literal with no metacharacters, so
-    // we can proceed with the str method replacement
-    let new_expr = re_func.replacement();
+    // Now we know the pattern is a string literal with no metacharacters, so
+    // we can proceed with the str method replacement.
+    //
+    // We first check the complex case, where more than just the call need replacing
+    //    Example: `re.search("abc", s) is not None`  => `"abc" in s`
+    let (mut new_expr, mut call_range) = match re_func.get_parent_replacement(semantic) {
+        Some((expr, range)) => (Some(expr), range),
+        None => (None, TextRange::default()),
+    };
+    // Second, we check the simple case, where only the call needs replacing
+    if new_expr.is_none() {
+        new_expr = re_func.get_call_replacement();
+        call_range = call.range;
+    }
 
     let repl = new_expr.map(|expr| checker.generator().expr(&expr));
     let mut diagnostic = Diagnostic::new(
         UnnecessaryRegularExpression {
             replacement: repl.clone(),
         },
-        call.range,
+        call_range,
     );
 
     if let Some(repl) = repl {
         diagnostic.set_fix(Fix::applicable_edit(
-            Edit::range_replacement(repl, call.range),
+            Edit::range_replacement(repl, call_range),
             if checker
                 .comment_ranges()
                 .has_comments(call, checker.source())
@@ -211,75 +222,130 @@ impl<'a> ReFunc<'a> {
                     string: call.arguments.find_argument_value("string", 2)?,
                 })
             }
-            ("match", 2) if in_if_context => Some(ReFunc {
-                kind: ReFuncKind::Match,
-                pattern: call.arguments.find_argument_value("pattern", 0)?,
-                string: call.arguments.find_argument_value("string", 1)?,
-            }),
-            ("search", 2) if in_if_context => Some(ReFunc {
-                kind: ReFuncKind::Search,
-                pattern: call.arguments.find_argument_value("pattern", 0)?,
-                string: call.arguments.find_argument_value("string", 1)?,
-            }),
-            ("fullmatch", 2) if in_if_context => Some(ReFunc {
-                kind: ReFuncKind::Fullmatch,
-                pattern: call.arguments.find_argument_value("pattern", 0)?,
-                string: call.arguments.find_argument_value("string", 1)?,
-            }),
+            ("match", 2) if in_if_context || get_comparison_to_none(semantic).is_some() => {
+                Some(ReFunc {
+                    kind: ReFuncKind::Match,
+                    pattern: call.arguments.find_argument_value("pattern", 0)?,
+                    string: call.arguments.find_argument_value("string", 1)?,
+                })
+            }
+            ("search", 2) if in_if_context || get_comparison_to_none(semantic).is_some() => {
+                Some(ReFunc {
+                    kind: ReFuncKind::Search,
+                    pattern: call.arguments.find_argument_value("pattern", 0)?,
+                    string: call.arguments.find_argument_value("string", 1)?,
+                })
+            }
+            ("fullmatch", 2) if in_if_context || get_comparison_to_none(semantic).is_some() => {
+                Some(ReFunc {
+                    kind: ReFuncKind::Fullmatch,
+                    pattern: call.arguments.find_argument_value("pattern", 0)?,
+                    string: call.arguments.find_argument_value("string", 1)?,
+                })
+            }
             _ => None,
         }
     }
 
-    fn replacement(&self) -> Option<Expr> {
+    /// Get replacement for the parent expression.
+    ///     Example: `re.search("abc", s) is None` => `"abc" not in s`
+    fn get_parent_replacement(&self, semantic: &'a SemanticModel) -> Option<(Expr, TextRange)> {
+        let Some((comparison, range)) = get_comparison_to_none(semantic) else {
+            return None;
+        };
+        match self.kind {
+            // pattern in string / pattern not in string
+            ReFuncKind::Search => match comparison {
+                ComparisonToNone::Is => Some((self.compare_expr(CmpOp::NotIn), range)),
+                ComparisonToNone::IsNot => Some((self.compare_expr(CmpOp::In), range)),
+            },
+            // string.startswith(pattern) / not string.startswith(pattern)
+            ReFuncKind::Match => match comparison {
+                ComparisonToNone::Is => Some((
+                    self.method_expr("startswith", vec![self.pattern.clone()], true),
+                    range,
+                )),
+                ComparisonToNone::IsNot => Some((
+                    self.method_expr("startswith", vec![self.pattern.clone()], false),
+                    range,
+                )),
+            },
+            // string == pattern / string != pattern
+            ReFuncKind::Fullmatch => match comparison {
+                ComparisonToNone::Is => Some((self.compare_expr(CmpOp::NotEq), range)),
+                ComparisonToNone::IsNot => Some((self.compare_expr(CmpOp::Eq), range)),
+            },
+            _ => None,
+        }
+    }
+
+    fn get_call_replacement(&self) -> Option<Expr> {
         match self.kind {
             // string.replace(pattern, repl)
             ReFuncKind::Sub { repl } => repl
                 .cloned()
-                .map(|repl| self.method_expr("replace", vec![self.pattern.clone(), repl])),
+                .map(|repl| self.method_expr("replace", vec![self.pattern.clone(), repl], false)),
             // string.startswith(pattern)
-            ReFuncKind::Match => Some(self.method_expr("startswith", vec![self.pattern.clone()])),
+            ReFuncKind::Match => {
+                Some(self.method_expr("startswith", vec![self.pattern.clone()], false))
+            }
             // pattern in string
             ReFuncKind::Search => Some(self.compare_expr(CmpOp::In)),
             // string == pattern
-            ReFuncKind::Fullmatch => Some(Expr::Compare(ExprCompare {
-                range: TextRange::default(),
-                left: Box::new(self.string.clone()),
-                ops: Box::new([CmpOp::Eq]),
-                comparators: Box::new([self.pattern.clone()]),
-            })),
+            ReFuncKind::Fullmatch => Some(self.compare_expr(CmpOp::Eq)),
             // string.split(pattern)
-            ReFuncKind::Split => Some(self.method_expr("split", vec![self.pattern.clone()])),
+            ReFuncKind::Split => Some(self.method_expr("split", vec![self.pattern.clone()], false)),
         }
     }
 
-    /// Return a new compare expr of the form `self.pattern op self.string`
+    /// Return a new compare expr of the form `self.pattern op self.string` (or the reverse)
     fn compare_expr(&self, op: CmpOp) -> Expr {
-        Expr::Compare(ExprCompare {
-            left: Box::new(self.pattern.clone()),
-            ops: Box::new([op]),
-            comparators: Box::new([self.string.clone()]),
-            range: TextRange::default(),
-        })
+        match op {
+            // Example: 'abc' in s
+            CmpOp::In | CmpOp::NotIn => Expr::Compare(ExprCompare {
+                left: Box::new(self.pattern.clone()),
+                ops: Box::new([op]),
+                comparators: Box::new([self.string.clone()]),
+                range: TextRange::default(),
+            }),
+            // Example: s == 'abc'
+            _ => Expr::Compare(ExprCompare {
+                left: Box::new(self.string.clone()),
+                ops: Box::new([op]),
+                comparators: Box::new([self.pattern.clone()]),
+                range: TextRange::default(),
+            }),
+        }
     }
 
     /// Return a new method call expression on `self.string` with `args` like
     /// `self.string.method(args...)`
-    fn method_expr(&self, method: &str, args: Vec<Expr>) -> Expr {
+    fn method_expr(&self, method: &str, args: Vec<Expr>, negate: bool) -> Expr {
         let method = Expr::Attribute(ExprAttribute {
             value: Box::new(self.string.clone()),
             attr: Identifier::new(method, TextRange::default()),
             ctx: ExprContext::Load,
             range: TextRange::default(),
         });
-        Expr::Call(ExprCall {
+        let call_expr = Expr::Call(ExprCall {
             func: Box::new(method),
             arguments: Arguments {
                 args: args.into_boxed_slice(),
                 keywords: Box::new([]),
                 range: TextRange::default(),
             },
             range: TextRange::default(),
-        })
+        });
+
+        if negate {
+            Expr::UnaryOp(ExprUnaryOp {
+                op: UnaryOp::Not,
+                operand: Box::new(call_expr),
+                range: TextRange::default(),
+            })
+        } else {
+            call_expr
+        }
     }
 }
 
@@ -302,3 +368,32 @@ fn resolve_string_literal<'a>(
 
     None
 }
+
+enum ComparisonToNone {
+    Is,
+    IsNot,
+}
+
+/// If the regex call is compared to `None`, return the comparison and its range.
+///    Example: `re.search("abc", s) is None`
+fn get_comparison_to_none(semantic: &SemanticModel) -> Option<(ComparisonToNone, TextRange)> {
+    if let Some(parent_expr) = semantic.current_expression_parent() {
+        if let Expr::Compare(ExprCompare {
+            ops, comparators, ..
+        }) = parent_expr
+        {
+            if ops.len() == 1 {
+                if let Some(right) = comparators.first() {
+                    if right.is_none_literal_expr() {
+                        return match ops[0] {
+                            CmpOp::Is => Some((ComparisonToNone::Is, parent_expr.range())),
+                            CmpOp::IsNot => Some((ComparisonToNone::IsNot, parent_expr.range())),
+                            _ => None,
+                        };
+                    }
+                }
+            }
+        }
+    }
+    None
+}