Skip to content

Commit 821b2f8

Browse files
TaKO8KintBre
TaKO8Ki
and
ntBre
authored
[refurb] Mark single-item-membership-test fix as always unsafe (FURB171) (#20279)
<!-- Thank you for contributing to Ruff/ty! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? (Please prefix with `[ty]` for ty pull requests.) - Does this pull request include references to any relevant issues? --> ## Summary <!-- What's the purpose of the change? What does it do, and why? --> Fixes #20255 Mark single-item-membership-test fixes as always unsafe - Always set `Applicability::Unsafe` for FURB171 fixes - Update “Fix safety” docs to reflect always-unsafe behavior - Expand tests (not in, nested set/frozenset, commented args) ## Test Plan <!-- How was it tested? --> I have added new test cases to `crates/ruff_linter/resources/test/fixtures/refurb/FURB171_0.py` and `crates/ruff_linter/resources/test/fixtures/refurb/FURB171_1.py`. --------- Co-authored-by: Brent Westbrook <[email protected]>
1 parent 1758f26 commit 821b2f8

File tree

5 files changed

+217
-15
lines changed

5 files changed

+217
-15
lines changed

‎crates/ruff_linter/resources/test/fixtures/refurb/FURB171_0.py‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,3 +117,31 @@ def _():
117117
# After
118118
] and \
119119
0 < 1: ...
120+
121+
# https://github.com/astral-sh/ruff/issues/20255
122+
import math
123+
124+
# NaN behavior differences
125+
if math.nan in [math.nan]:
126+
print("This is True")
127+
128+
if math.nan in (math.nan,):
129+
print("This is True")
130+
131+
if math.nan in {math.nan}:
132+
print("This is True")
133+
134+
# Potential type differences with custom __eq__ methods
135+
class CustomEq:
136+
def __eq__(self, other):
137+
return "custom"
138+
139+
obj = CustomEq()
140+
if obj in [CustomEq()]:
141+
pass
142+
143+
if obj in (CustomEq(),):
144+
pass
145+
146+
if obj in {CustomEq()}:
147+
pass

‎crates/ruff_linter/resources/test/fixtures/refurb/FURB171_1.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,13 @@
5151

5252
if 1 in set((x for x in range(2))):
5353
pass
54+
55+
# https://github.com/astral-sh/ruff/issues/20255
56+
import math
57+
58+
# set() and frozenset() with NaN
59+
if math.nan in set([math.nan]):
60+
print("This should be marked unsafe")
61+
62+
if math.nan in frozenset([math.nan]):
63+
print("This should be marked unsafe")

‎crates/ruff_linter/src/rules/refurb/rules/single_item_membership_test.rs‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use ruff_text_size::Ranged;
66

77
use crate::checkers::ast::Checker;
88
use crate::fix::edits::pad;
9-
use crate::{Applicability, Edit, Fix, FixAvailability, Violation};
9+
use crate::{Edit, Fix, FixAvailability, Violation};
1010

1111
/// ## What it does
1212
/// Checks for membership tests against single-item containers.
@@ -27,13 +27,16 @@ use crate::{Applicability, Edit, Fix, FixAvailability, Violation};
2727
/// ```
2828
///
2929
/// ## Fix safety
30+
/// The fix is always marked as unsafe.
3031
///
31-
/// When the right-hand side is a string, the fix is marked as unsafe.
32-
/// This is because `c in "a"` is true both when `c` is `"a"` and when `c` is the empty string,
33-
/// so the fix can change the behavior of your program in these cases.
32+
/// When the right-hand side is a string, this fix can change the behavior of your program.
33+
/// This is because `c in "a"` is true both when `c` is `"a"` and when `c` is the empty string.
3434
///
35-
/// Additionally, if there are comments within the fix's range,
36-
/// it will also be marked as unsafe.
35+
/// Additionally, converting `in`/`not in` against a single-item container to `==`/`!=` can
36+
/// change runtime behavior: `in` may consider identity (e.g., `NaN`) and always
37+
/// yields a `bool`.
38+
///
39+
/// Comments within the replacement range will also be removed.
3740
///
3841
/// ## References
3942
/// - [Python documentation: Comparisons](https://docs.python.org/3/reference/expressions.html#comparisons)
@@ -100,14 +103,8 @@ pub(crate) fn single_item_membership_test(
100103
expr.range(),
101104
);
102105

103-
let applicability =
104-
if right.is_string_literal_expr() || checker.comment_ranges().intersects(expr.range()) {
105-
Applicability::Unsafe
106-
} else {
107-
Applicability::Safe
108-
};
109-
110-
let fix = Fix::applicable_edit(edit, applicability);
106+
// All supported cases can change runtime behavior; mark as unsafe.
107+
let fix = Fix::unsafe_edit(edit);
111108

112109
checker
113110
.report_diagnostic(SingleItemMembershipTest { membership_test }, expr.range())

‎crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB171_FURB171_0.py.snap‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ help: Convert to equality test
1818
4 | print("Single-element tuple")
1919
5 |
2020
6 | if 1 in [1]:
21+
note: This is an unsafe fix and may change runtime behavior
2122

2223
FURB171 [*] Membership test against single-item container
2324
--> FURB171_0.py:6:4
@@ -37,6 +38,7 @@ help: Convert to equality test
3738
7 | print("Single-element list")
3839
8 |
3940
9 | if 1 in {1}:
41+
note: This is an unsafe fix and may change runtime behavior
4042

4143
FURB171 [*] Membership test against single-item container
4244
--> FURB171_0.py:9:4
@@ -56,6 +58,7 @@ help: Convert to equality test
5658
10 | print("Single-element set")
5759
11 |
5860
12 | if "a" in "a":
61+
note: This is an unsafe fix and may change runtime behavior
5962

6063
FURB171 [*] Membership test against single-item container
6164
--> FURB171_0.py:12:4
@@ -95,6 +98,7 @@ help: Convert to inequality test
9598
16 | print("Check `not in` membership test")
9699
17 |
97100
18 | if not 1 in (1,):
101+
note: This is an unsafe fix and may change runtime behavior
98102

99103
FURB171 [*] Membership test against single-item container
100104
--> FURB171_0.py:18:8
@@ -114,6 +118,7 @@ help: Convert to equality test
114118
19 | print("Check the negated membership test")
115119
20 |
116120
21 | # Non-errors.
121+
note: This is an unsafe fix and may change runtime behavior
117122

118123
FURB171 [*] Membership test against single-item container
119124
--> FURB171_0.py:52:5
@@ -344,4 +349,122 @@ help: Convert to inequality test
344349
- ] and \
345350
113 + if foo != bar and \
346351
114 | 0 < 1: ...
352+
115 |
353+
116 | # https://github.com/astral-sh/ruff/issues/20255
354+
note: This is an unsafe fix and may change runtime behavior
355+
356+
FURB171 [*] Membership test against single-item container
357+
--> FURB171_0.py:125:4
358+
|
359+
124 | # NaN behavior differences
360+
125 | if math.nan in [math.nan]:
361+
| ^^^^^^^^^^^^^^^^^^^^^^
362+
126 | print("This is True")
363+
|
364+
help: Convert to equality test
365+
122 | import math
366+
123 |
367+
124 | # NaN behavior differences
368+
- if math.nan in [math.nan]:
369+
125 + if math.nan == math.nan:
370+
126 | print("This is True")
371+
127 |
372+
128 | if math.nan in (math.nan,):
373+
note: This is an unsafe fix and may change runtime behavior
374+
375+
FURB171 [*] Membership test against single-item container
376+
--> FURB171_0.py:128:4
377+
|
378+
126 | print("This is True")
379+
127 |
380+
128 | if math.nan in (math.nan,):
381+
| ^^^^^^^^^^^^^^^^^^^^^^^
382+
129 | print("This is True")
383+
|
384+
help: Convert to equality test
385+
125 | if math.nan in [math.nan]:
386+
126 | print("This is True")
387+
127 |
388+
- if math.nan in (math.nan,):
389+
128 + if math.nan == math.nan:
390+
129 | print("This is True")
391+
130 |
392+
131 | if math.nan in {math.nan}:
393+
note: This is an unsafe fix and may change runtime behavior
394+
395+
FURB171 [*] Membership test against single-item container
396+
--> FURB171_0.py:131:4
397+
|
398+
129 | print("This is True")
399+
130 |
400+
131 | if math.nan in {math.nan}:
401+
| ^^^^^^^^^^^^^^^^^^^^^^
402+
132 | print("This is True")
403+
|
404+
help: Convert to equality test
405+
128 | if math.nan in (math.nan,):
406+
129 | print("This is True")
407+
130 |
408+
- if math.nan in {math.nan}:
409+
131 + if math.nan == math.nan:
410+
132 | print("This is True")
411+
133 |
412+
134 | # Potential type differences with custom __eq__ methods
413+
note: This is an unsafe fix and may change runtime behavior
414+
415+
FURB171 [*] Membership test against single-item container
416+
--> FURB171_0.py:140:4
417+
|
418+
139 | obj = CustomEq()
419+
140 | if obj in [CustomEq()]:
420+
| ^^^^^^^^^^^^^^^^^^^
421+
141 | pass
422+
|
423+
help: Convert to equality test
424+
137 | return "custom"
425+
138 |
426+
139 | obj = CustomEq()
427+
- if obj in [CustomEq()]:
428+
140 + if obj == CustomEq():
429+
141 | pass
430+
142 |
431+
143 | if obj in (CustomEq(),):
432+
note: This is an unsafe fix and may change runtime behavior
433+
434+
FURB171 [*] Membership test against single-item container
435+
--> FURB171_0.py:143:4
436+
|
437+
141 | pass
438+
142 |
439+
143 | if obj in (CustomEq(),):
440+
| ^^^^^^^^^^^^^^^^^^^^
441+
144 | pass
442+
|
443+
help: Convert to equality test
444+
140 | if obj in [CustomEq()]:
445+
141 | pass
446+
142 |
447+
- if obj in (CustomEq(),):
448+
143 + if obj == CustomEq():
449+
144 | pass
450+
145 |
451+
146 | if obj in {CustomEq()}:
452+
note: This is an unsafe fix and may change runtime behavior
453+
454+
FURB171 [*] Membership test against single-item container
455+
--> FURB171_0.py:146:4
456+
|
457+
144 | pass
458+
145 |
459+
146 | if obj in {CustomEq()}:
460+
| ^^^^^^^^^^^^^^^^^^^
461+
147 | pass
462+
|
463+
help: Convert to equality test
464+
143 | if obj in (CustomEq(),):
465+
144 | pass
466+
145 |
467+
- if obj in {CustomEq()}:
468+
146 + if obj == CustomEq():
469+
147 | pass
347470
note: This is an unsafe fix and may change runtime behavior

‎crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB171_FURB171_1.py.snap‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ help: Convert to equality test
1818
4 | print("Single-element set")
1919
5 |
2020
6 | if 1 in set((1,)):
21+
note: This is an unsafe fix and may change runtime behavior
2122

2223
FURB171 [*] Membership test against single-item container
2324
--> FURB171_1.py:6:4
@@ -37,6 +38,7 @@ help: Convert to equality test
3738
7 | print("Single-element set")
3839
8 |
3940
9 | if 1 in set({1}):
41+
note: This is an unsafe fix and may change runtime behavior
4042

4143
FURB171 [*] Membership test against single-item container
4244
--> FURB171_1.py:9:4
@@ -56,6 +58,7 @@ help: Convert to equality test
5658
10 | print("Single-element set")
5759
11 |
5860
12 | if 1 in frozenset([1]):
61+
note: This is an unsafe fix and may change runtime behavior
5962

6063
FURB171 [*] Membership test against single-item container
6164
--> FURB171_1.py:12:4
@@ -75,6 +78,7 @@ help: Convert to equality test
7578
13 | print("Single-element set")
7679
14 |
7780
15 | if 1 in frozenset((1,)):
81+
note: This is an unsafe fix and may change runtime behavior
7882

7983
FURB171 [*] Membership test against single-item container
8084
--> FURB171_1.py:15:4
@@ -94,6 +98,7 @@ help: Convert to equality test
9498
16 | print("Single-element set")
9599
17 |
96100
18 | if 1 in frozenset({1}):
101+
note: This is an unsafe fix and may change runtime behavior
97102

98103
FURB171 [*] Membership test against single-item container
99104
--> FURB171_1.py:18:4
@@ -113,6 +118,7 @@ help: Convert to equality test
113118
19 | print("Single-element set")
114119
20 |
115120
21 | if 1 in set(set([1])):
121+
note: This is an unsafe fix and may change runtime behavior
116122

117123
FURB171 [*] Membership test against single-item container
118124
--> FURB171_1.py:21:4
@@ -131,4 +137,42 @@ help: Convert to equality test
131137
21 + if 1 == 1:
132138
22 | print('Recursive solution')
133139
23 |
134-
24 |
140+
24 |
141+
note: This is an unsafe fix and may change runtime behavior
142+
143+
FURB171 [*] Membership test against single-item container
144+
--> FURB171_1.py:59:4
145+
|
146+
58 | # set() and frozenset() with NaN
147+
59 | if math.nan in set([math.nan]):
148+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
149+
60 | print("This should be marked unsafe")
150+
|
151+
help: Convert to equality test
152+
56 | import math
153+
57 |
154+
58 | # set() and frozenset() with NaN
155+
- if math.nan in set([math.nan]):
156+
59 + if math.nan == math.nan:
157+
60 | print("This should be marked unsafe")
158+
61 |
159+
62 | if math.nan in frozenset([math.nan]):
160+
note: This is an unsafe fix and may change runtime behavior
161+
162+
FURB171 [*] Membership test against single-item container
163+
--> FURB171_1.py:62:4
164+
|
165+
60 | print("This should be marked unsafe")
166+
61 |
167+
62 | if math.nan in frozenset([math.nan]):
168+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
169+
63 | print("This should be marked unsafe")
170+
|
171+
help: Convert to equality test
172+
59 | if math.nan in set([math.nan]):
173+
60 | print("This should be marked unsafe")
174+
61 |
175+
- if math.nan in frozenset([math.nan]):
176+
62 + if math.nan == math.nan:
177+
63 | print("This should be marked unsafe")
178+
note: This is an unsafe fix and may change runtime behavior

0 commit comments

Comments
 (0)