diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB171_0.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB171_0.py index 109145cb5c..e021ec9635 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB171_0.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB171_0.py @@ -117,3 +117,31 @@ def _(): # After ] and \ 0 < 1: ... + +# https://github.com/astral-sh/ruff/issues/20255 +import math + +# NaN behavior differences +if math.nan in [math.nan]: + print("This is True") + +if math.nan in (math.nan,): + print("This is True") + +if math.nan in {math.nan}: + print("This is True") + +# Potential type differences with custom __eq__ methods +class CustomEq: + def __eq__(self, other): + return "custom" + +obj = CustomEq() +if obj in [CustomEq()]: + pass + +if obj in (CustomEq(),): + pass + +if obj in {CustomEq()}: + pass diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB171_1.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB171_1.py index 41109f6cfa..77dfe266c3 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB171_1.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB171_1.py @@ -51,3 +51,13 @@ if 1 in set(1,2): if 1 in set((x for x in range(2))): pass + +# https://github.com/astral-sh/ruff/issues/20255 +import math + +# set() and frozenset() with NaN +if math.nan in set([math.nan]): + print("This should be marked unsafe") + +if math.nan in frozenset([math.nan]): + print("This should be marked unsafe") 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 index baecccb9e8..de6bdae0b3 100644 --- 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 @@ -6,7 +6,7 @@ use ruff_text_size::Ranged; use crate::checkers::ast::Checker; use crate::fix::edits::pad; -use crate::{Applicability, Edit, Fix, FixAvailability, Violation}; +use crate::{Edit, Fix, FixAvailability, Violation}; /// ## What it does /// Checks for membership tests against single-item containers. @@ -27,13 +27,16 @@ use crate::{Applicability, Edit, Fix, FixAvailability, Violation}; /// ``` /// /// ## Fix safety +/// The fix is always marked as unsafe. /// -/// When the right-hand side is a string, the fix is marked as unsafe. -/// 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. +/// When the right-hand side is a string, this fix can change the behavior of your program. +/// This is because `c in "a"` is true both when `c` is `"a"` and when `c` is the empty string. /// -/// Additionally, if there are comments within the fix's range, -/// it will also be marked as unsafe. +/// Additionally, converting `in`/`not in` against a single-item container to `==`/`!=` can +/// change runtime behavior: `in` may consider identity (e.g., `NaN`) and always +/// yields a `bool`. +/// +/// Comments within the replacement range will also be removed. /// /// ## References /// - [Python documentation: Comparisons](https://docs.python.org/3/reference/expressions.html#comparisons) @@ -100,14 +103,8 @@ pub(crate) fn single_item_membership_test( expr.range(), ); - 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); + // All supported cases can change runtime behavior; mark as unsafe. + let fix = Fix::unsafe_edit(edit); checker .report_diagnostic(SingleItemMembershipTest { membership_test }, expr.range()) diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB171_FURB171_0.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB171_FURB171_0.py.snap index 49277079fc..9e38c7e109 100644 --- a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB171_FURB171_0.py.snap +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB171_FURB171_0.py.snap @@ -18,6 +18,7 @@ help: Convert to equality test 4 | print("Single-element tuple") 5 | 6 | if 1 in [1]: +note: This is an unsafe fix and may change runtime behavior FURB171 [*] Membership test against single-item container --> FURB171_0.py:6:4 @@ -37,6 +38,7 @@ help: Convert to equality test 7 | print("Single-element list") 8 | 9 | if 1 in {1}: +note: This is an unsafe fix and may change runtime behavior FURB171 [*] Membership test against single-item container --> FURB171_0.py:9:4 @@ -56,6 +58,7 @@ help: Convert to equality test 10 | print("Single-element set") 11 | 12 | if "a" in "a": +note: This is an unsafe fix and may change runtime behavior FURB171 [*] Membership test against single-item container --> FURB171_0.py:12:4 @@ -95,6 +98,7 @@ help: Convert to inequality test 16 | print("Check `not in` membership test") 17 | 18 | if not 1 in (1,): +note: This is an unsafe fix and may change runtime behavior FURB171 [*] Membership test against single-item container --> FURB171_0.py:18:8 @@ -114,6 +118,7 @@ help: Convert to equality test 19 | print("Check the negated membership test") 20 | 21 | # Non-errors. +note: This is an unsafe fix and may change runtime behavior FURB171 [*] Membership test against single-item container --> FURB171_0.py:52:5 @@ -344,4 +349,122 @@ help: Convert to inequality test - ] and \ 113 + if foo != bar and \ 114 | 0 < 1: ... +115 | +116 | # https://github.com/astral-sh/ruff/issues/20255 +note: This is an unsafe fix and may change runtime behavior + +FURB171 [*] Membership test against single-item container + --> FURB171_0.py:125:4 + | +124 | # NaN behavior differences +125 | if math.nan in [math.nan]: + | ^^^^^^^^^^^^^^^^^^^^^^ +126 | print("This is True") + | +help: Convert to equality test +122 | import math +123 | +124 | # NaN behavior differences + - if math.nan in [math.nan]: +125 + if math.nan == math.nan: +126 | print("This is True") +127 | +128 | if math.nan in (math.nan,): +note: This is an unsafe fix and may change runtime behavior + +FURB171 [*] Membership test against single-item container + --> FURB171_0.py:128:4 + | +126 | print("This is True") +127 | +128 | if math.nan in (math.nan,): + | ^^^^^^^^^^^^^^^^^^^^^^^ +129 | print("This is True") + | +help: Convert to equality test +125 | if math.nan in [math.nan]: +126 | print("This is True") +127 | + - if math.nan in (math.nan,): +128 + if math.nan == math.nan: +129 | print("This is True") +130 | +131 | if math.nan in {math.nan}: +note: This is an unsafe fix and may change runtime behavior + +FURB171 [*] Membership test against single-item container + --> FURB171_0.py:131:4 + | +129 | print("This is True") +130 | +131 | if math.nan in {math.nan}: + | ^^^^^^^^^^^^^^^^^^^^^^ +132 | print("This is True") + | +help: Convert to equality test +128 | if math.nan in (math.nan,): +129 | print("This is True") +130 | + - if math.nan in {math.nan}: +131 + if math.nan == math.nan: +132 | print("This is True") +133 | +134 | # Potential type differences with custom __eq__ methods +note: This is an unsafe fix and may change runtime behavior + +FURB171 [*] Membership test against single-item container + --> FURB171_0.py:140:4 + | +139 | obj = CustomEq() +140 | if obj in [CustomEq()]: + | ^^^^^^^^^^^^^^^^^^^ +141 | pass + | +help: Convert to equality test +137 | return "custom" +138 | +139 | obj = CustomEq() + - if obj in [CustomEq()]: +140 + if obj == CustomEq(): +141 | pass +142 | +143 | if obj in (CustomEq(),): +note: This is an unsafe fix and may change runtime behavior + +FURB171 [*] Membership test against single-item container + --> FURB171_0.py:143:4 + | +141 | pass +142 | +143 | if obj in (CustomEq(),): + | ^^^^^^^^^^^^^^^^^^^^ +144 | pass + | +help: Convert to equality test +140 | if obj in [CustomEq()]: +141 | pass +142 | + - if obj in (CustomEq(),): +143 + if obj == CustomEq(): +144 | pass +145 | +146 | if obj in {CustomEq()}: +note: This is an unsafe fix and may change runtime behavior + +FURB171 [*] Membership test against single-item container + --> FURB171_0.py:146:4 + | +144 | pass +145 | +146 | if obj in {CustomEq()}: + | ^^^^^^^^^^^^^^^^^^^ +147 | pass + | +help: Convert to equality test +143 | if obj in (CustomEq(),): +144 | pass +145 | + - if obj in {CustomEq()}: +146 + if obj == CustomEq(): +147 | pass note: This is an unsafe fix and may change runtime behavior diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB171_FURB171_1.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB171_FURB171_1.py.snap index a662206459..ece6d276be 100644 --- a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB171_FURB171_1.py.snap +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB171_FURB171_1.py.snap @@ -18,6 +18,7 @@ help: Convert to equality test 4 | print("Single-element set") 5 | 6 | if 1 in set((1,)): +note: This is an unsafe fix and may change runtime behavior FURB171 [*] Membership test against single-item container --> FURB171_1.py:6:4 @@ -37,6 +38,7 @@ help: Convert to equality test 7 | print("Single-element set") 8 | 9 | if 1 in set({1}): +note: This is an unsafe fix and may change runtime behavior FURB171 [*] Membership test against single-item container --> FURB171_1.py:9:4 @@ -56,6 +58,7 @@ help: Convert to equality test 10 | print("Single-element set") 11 | 12 | if 1 in frozenset([1]): +note: This is an unsafe fix and may change runtime behavior FURB171 [*] Membership test against single-item container --> FURB171_1.py:12:4 @@ -75,6 +78,7 @@ help: Convert to equality test 13 | print("Single-element set") 14 | 15 | if 1 in frozenset((1,)): +note: This is an unsafe fix and may change runtime behavior FURB171 [*] Membership test against single-item container --> FURB171_1.py:15:4 @@ -94,6 +98,7 @@ help: Convert to equality test 16 | print("Single-element set") 17 | 18 | if 1 in frozenset({1}): +note: This is an unsafe fix and may change runtime behavior FURB171 [*] Membership test against single-item container --> FURB171_1.py:18:4 @@ -113,6 +118,7 @@ help: Convert to equality test 19 | print("Single-element set") 20 | 21 | if 1 in set(set([1])): +note: This is an unsafe fix and may change runtime behavior FURB171 [*] Membership test against single-item container --> FURB171_1.py:21:4 @@ -131,4 +137,42 @@ help: Convert to equality test 21 + if 1 == 1: 22 | print('Recursive solution') 23 | -24 | +24 | +note: This is an unsafe fix and may change runtime behavior + +FURB171 [*] Membership test against single-item container + --> FURB171_1.py:59:4 + | +58 | # set() and frozenset() with NaN +59 | if math.nan in set([math.nan]): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +60 | print("This should be marked unsafe") + | +help: Convert to equality test +56 | import math +57 | +58 | # set() and frozenset() with NaN + - if math.nan in set([math.nan]): +59 + if math.nan == math.nan: +60 | print("This should be marked unsafe") +61 | +62 | if math.nan in frozenset([math.nan]): +note: This is an unsafe fix and may change runtime behavior + +FURB171 [*] Membership test against single-item container + --> FURB171_1.py:62:4 + | +60 | print("This should be marked unsafe") +61 | +62 | if math.nan in frozenset([math.nan]): + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +63 | print("This should be marked unsafe") + | +help: Convert to equality test +59 | if math.nan in set([math.nan]): +60 | print("This should be marked unsafe") +61 | + - if math.nan in frozenset([math.nan]): +62 + if math.nan == math.nan: +63 | print("This should be marked unsafe") +note: This is an unsafe fix and may change runtime behavior