['ruff'] Mark fixes as unsafe for new patterns (RUF055)

astral-sh · Jan 28, 2025 · 3d25918 · 3d25918
1 parent 92e84ee
commit 3d25918
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 5 deletions.
diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF055_2.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF055_2.py
@@ -25,3 +25,16 @@
 
 # this should be replaced with `s == "abc"`
 re.fullmatch("abc", s) is not None
+
+
+# this should trigger an unsafe fix because of the presence of a comment (which we'd lose)
+if (
+    re.fullmatch(
+        "a really really really really long string",
+        s,
+    )
+    # with a comment here
+    is None
+):
+    pass
+
diff --git a/crates/ruff_linter/src/rules/ruff/rules/unnecessary_regular_expression.rs b/crates/ruff_linter/src/rules/ruff/rules/unnecessary_regular_expression.rs
@@ -124,12 +124,20 @@ pub(crate) fn unnecessary_regular_expression(checker: &mut Checker, call: &ExprC
     );
 
     if let Some(repl) = repl {
+        let has_comments = match (
+            re_func.comparison_to_none,
+            semantic.current_expression_parent(),
+        ) {
+            (Some(_), Some(parent_expr)) => checker
+                .comment_ranges()
+                .has_comments(parent_expr, checker.source()),
+            _ => checker
+                .comment_ranges()
+                .has_comments(call, checker.source()),
+        };
         diagnostic.set_fix(Fix::applicable_edit(
             Edit::range_replacement(repl, re_func.range),
-            if checker
-                .comment_ranges()
-                .has_comments(call, checker.source())
-            {
+            if has_comments {
                 Applicability::Unsafe
             } else {
                 Applicability::Safe
@@ -167,7 +175,7 @@ impl<'a> ReFunc<'a> {
         func_name: &str,
     ) -> Option<Self> {
         // the proposed fixes for match, search, and fullmatch rely on the
-        // return value only being used for its truth value
+        // return value only being used for its truth value or being compared to None
         let comparison_to_none = get_comparison_to_none(semantic);
         let in_truthy_context = semantic.in_boolean_test() || comparison_to_none.is_some();
 

diff --git a/...rc/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF055_RUF055_2.py.snap b/...rc/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF055_RUF055_2.py.snap
@@ -106,3 +106,37 @@ RUF055_2.py:27:1: RUF055 [*] Plain string pattern passed to `re` function
 26 26 | # this should be replaced with `s == "abc"`
 27    |-re.fullmatch("abc", s) is not None
    27 |+s == "abc"
+28 28 | 
+29 29 | 
+30 30 | # this should trigger an unsafe fix because of the presence of a comment (which we'd lose)
+
+RUF055_2.py:32:5: RUF055 [*] Plain string pattern passed to `re` function
+   |
+30 |   # this should trigger an unsafe fix because of the presence of a comment (which we'd lose)
+31 |   if (
+32 | /     re.fullmatch(
+33 | |         "a really really really really long string",
+34 | |         s,
+35 | |     )
+36 | |     # with a comment here
+37 | |     is None
+   | |___________^ RUF055
+38 |   ):
+39 |       pass
+   |
+   = help: Replace with `s != "a really really really really long string"`
+
+ℹ Unsafe fix
+29 29 | 
+30 30 | # this should trigger an unsafe fix because of the presence of a comment (which we'd lose)
+31 31 | if (
+32    |-    re.fullmatch(
+33    |-        "a really really really really long string",
+34    |-        s,
+35    |-    )
+36    |-    # with a comment here
+37    |-    is None
+   32 |+    s != "a really really really really long string"
+38 33 | ):
+39 34 |     pass
+40 35 |