Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jan 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/refurb/FURB171.py
Original file line number Diff line number Diff line change
Expand Up @@ -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: ...
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: ...
Loading