[refurb] Mark fix as unsafe if there are comments (FURB171) (#15832)

## Summary Resolves #10063 and follow-up to #15521. The fix is now marked as unsafe if there are any comments within its range. Tests are adapted from that of #15521. ## Test Plan `cargo nextest run` and `cargo insta test`.
astral-sh · Jan 30, 2025 · 172f62d · 172f62d
1 parent 071862a
commit 172f62d
Show file tree

Hide file tree

Showing 3 changed files with 311 additions and 5 deletions.
diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB171.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB171.py
@@ -46,3 +46,74 @@
 
 if 1 in {*[1]}:
     pass
+
+
+# https://github.com/astral-sh/ruff/issues/10063
+_ = a in (
+    # Foo
+    b,
+)
+
+_ = a in (  # Foo1
+    (  # Foo2
+    # Foo3
+        (  # Tuple
+            (  # Bar
+       (b
+        # Bar
+        )
+  )
+       # Foo4
+            # Foo5
+     ,
+       )
+        # Foo6
+    )
+)
+
+foo = (
+    lorem()
+        .ipsum()
+        .dolor(lambda sit: sit in (
+            # Foo1
+            # Foo2
+            amet,
+        ))
+)
+
+foo = (
+    lorem()
+        .ipsum()
+        .dolor(lambda sit: sit in (
+            (
+                # Foo1
+                # Foo2
+                amet
+            ),
+        ))
+)
+
+foo = lorem() \
+    .ipsum() \
+    .dolor(lambda sit: sit in (
+        # Foo1
+        # Foo2
+        amet,
+    ))
+
+def _():
+    if foo not \
+        in [
+        # Before
+        bar
+        # After
+    ]: ...
+
+def _():
+    if foo not \
+        in [
+        # Before
+        bar
+        # After
+    ] and \
+        0 < 1: ...
diff --git a/crates/ruff_linter/src/rules/refurb/rules/single_item_membership_test.rs b/crates/ruff_linter/src/rules/refurb/rules/single_item_membership_test.rs
@@ -31,6 +31,9 @@ use crate::fix::edits::pad;
 /// This is because `c in "a"` is true both when `c` is `"a"` and when `c` is the empty string,
 /// so the fix can change the behavior of your program in these cases.
 ///
+/// Additionally, if there are comments within the fix's range,
+/// it will also be marked as unsafe.
+///
 /// ## References
 /// - [Python documentation: Comparisons](https://docs.python.org/3/reference/expressions.html#comparisons)
 /// - [Python documentation: Membership test operations](https://docs.python.org/3/reference/expressions.html#membership-test-operations)
@@ -98,11 +101,12 @@ pub(crate) fn single_item_membership_test(
         expr.range(),
     );
 
-    let applicability = if right.is_string_literal_expr() {
-        Applicability::Unsafe
-    } else {
-        Applicability::Safe
-    };
+    let applicability =
+        if right.is_string_literal_expr() || checker.comment_ranges().intersects(expr.range()) {
+            Applicability::Unsafe
+        } else {
+            Applicability::Safe
+        };
 
     let fix = Fix::applicable_edit(edit, applicability);
 

diff --git a/...ter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB171_FURB171.py.snap b/...ter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB171_FURB171.py.snap
@@ -119,3 +119,234 @@ FURB171.py:18:8: FURB171 [*] Membership test against single-item container
 19 19 |     print("Check the negated membership test")
 20 20 | 
 21 21 | # Non-errors.
+
+FURB171.py:52:5: FURB171 [*] Membership test against single-item container
+   |
+51 |   # https://github.com/astral-sh/ruff/issues/10063
+52 |   _ = a in (
+   |  _____^
+53 | |     # Foo
+54 | |     b,
+55 | | )
+   | |_^ FURB171
+56 |
+57 |   _ = a in (  # Foo1
+   |
+   = help: Convert to equality test
+
+ℹ Unsafe fix
+49 49 | 
+50 50 | 
+51 51 | # https://github.com/astral-sh/ruff/issues/10063
+52    |-_ = a in (
+53    |-    # Foo
+54    |-    b,
+55    |-)
+   52 |+_ = a == b
+56 53 | 
+57 54 | _ = a in (  # Foo1
+58 55 |     (  # Foo2
+
+FURB171.py:57:5: FURB171 [*] Membership test against single-item container
+   |
+55 |   )
+56 |
+57 |   _ = a in (  # Foo1
+   |  _____^
+58 | |     (  # Foo2
+59 | |     # Foo3
+60 | |         (  # Tuple
+61 | |             (  # Bar
+62 | |        (b
+63 | |         # Bar
+64 | |         )
+65 | |   )
+66 | |        # Foo4
+67 | |             # Foo5
+68 | |      ,
+69 | |        )
+70 | |         # Foo6
+71 | |     )
+72 | | )
+   | |_^ FURB171
+73 |
+74 |   foo = (
+   |
+   = help: Convert to equality test
+
+ℹ Unsafe fix
+54 54 |     b,
+55 55 | )
+56 56 | 
+57    |-_ = a in (  # Foo1
+58    |-    (  # Foo2
+59    |-    # Foo3
+60    |-        (  # Tuple
+61    |-            (  # Bar
+   57 |+_ = a == (  # Bar
+62 58 |        (b
+63 59 |         # Bar
+64 60 |         )
+65 61 |   )
+66    |-       # Foo4
+67    |-            # Foo5
+68    |-     ,
+69    |-       )
+70    |-        # Foo6
+71    |-    )
+72    |-)
+73 62 | 
+74 63 | foo = (
+75 64 |     lorem()
+
+FURB171.py:77:28: FURB171 [*] Membership test against single-item container
+   |
+75 |       lorem()
+76 |           .ipsum()
+77 |           .dolor(lambda sit: sit in (
+   |  ____________________________^
+78 | |             # Foo1
+79 | |             # Foo2
+80 | |             amet,
+81 | |         ))
+   | |_________^ FURB171
+82 |   )
+   |
+   = help: Convert to equality test
+
+ℹ Unsafe fix
+74 74 | foo = (
+75 75 |     lorem()
+76 76 |         .ipsum()
+77    |-        .dolor(lambda sit: sit in (
+78    |-            # Foo1
+79    |-            # Foo2
+80    |-            amet,
+81    |-        ))
+   77 |+        .dolor(lambda sit: sit == amet)
+82 78 | )
+83 79 | 
+84 80 | foo = (
+
+FURB171.py:87:28: FURB171 [*] Membership test against single-item container
+   |
+85 |       lorem()
+86 |           .ipsum()
+87 |           .dolor(lambda sit: sit in (
+   |  ____________________________^
+88 | |             (
+89 | |                 # Foo1
+90 | |                 # Foo2
+91 | |                 amet
+92 | |             ),
+93 | |         ))
+   | |_________^ FURB171
+94 |   )
+   |
+   = help: Convert to equality test
+
+ℹ Unsafe fix
+84 84 | foo = (
+85 85 |     lorem()
+86 86 |         .ipsum()
+87    |-        .dolor(lambda sit: sit in (
+88    |-            (
+   87 |+        .dolor(lambda sit: sit == (
+89 88 |                 # Foo1
+90 89 |                 # Foo2
+91 90 |                 amet
+92    |-            ),
+93    |-        ))
+   91 |+            ))
+94 92 | )
+95 93 | 
+96 94 | foo = lorem() \
+
+FURB171.py:98:24: FURB171 [*] Membership test against single-item container
+    |
+ 96 |   foo = lorem() \
+ 97 |       .ipsum() \
+ 98 |       .dolor(lambda sit: sit in (
+    |  ________________________^
+ 99 | |         # Foo1
+100 | |         # Foo2
+101 | |         amet,
+102 | |     ))
+    | |_____^ FURB171
+103 |
+104 |   def _():
+    |
+    = help: Convert to equality test
+
+ℹ Unsafe fix
+95  95  | 
+96  96  | foo = lorem() \
+97  97  |     .ipsum() \
+98      |-    .dolor(lambda sit: sit in (
+99      |-        # Foo1
+100     |-        # Foo2
+101     |-        amet,
+102     |-    ))
+    98  |+    .dolor(lambda sit: sit == amet)
+103 99  | 
+104 100 | def _():
+105 101 |     if foo not \
+
+FURB171.py:105:8: FURB171 [*] Membership test against single-item container
+    |
+104 |   def _():
+105 |       if foo not \
+    |  ________^
+106 | |         in [
+107 | |         # Before
+108 | |         bar
+109 | |         # After
+110 | |     ]: ...
+    | |_____^ FURB171
+111 |
+112 |   def _():
+    |
+    = help: Convert to inequality test
+
+ℹ Unsafe fix
+102 102 |     ))
+103 103 | 
+104 104 | def _():
+105     |-    if foo not \
+106     |-        in [
+107     |-        # Before
+108     |-        bar
+109     |-        # After
+110     |-    ]: ...
+    105 |+    if foo != bar: ...
+111 106 | 
+112 107 | def _():
+113 108 |     if foo not \
+
+FURB171.py:113:8: FURB171 [*] Membership test against single-item container
+    |
+112 |   def _():
+113 |       if foo not \
+    |  ________^
+114 | |         in [
+115 | |         # Before
+116 | |         bar
+117 | |         # After
+118 | |     ] and \
+    | |_____^ FURB171
+119 |           0 < 1: ...
+    |
+    = help: Convert to inequality test
+
+ℹ Unsafe fix
+110 110 |     ]: ...
+111 111 | 
+112 112 | def _():
+113     |-    if foo not \
+114     |-        in [
+115     |-        # Before
+116     |-        bar
+117     |-        # After
+118     |-    ] and \
+    113 |+    if foo != bar and \
+119 114 |         0 < 1: ...