mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-01 14:21:24 +00:00
[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`.
This commit is contained in:
InSync
GitHub
parent
071862af5a
commit
172f62d8f4
3 changed files with 311 additions and 5 deletions
|
|
@ -46,3 +46,74 @@ if "a" == "a":
|
||||||
|
|
||||||
if 1 in {*[1]}:
|
if 1 in {*[1]}:
|
||||||
pass
|
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: ...
|
||||||
|
|
|
||||||
|
|
@ -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,
|
/// 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.
|
/// 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
|
/// ## References
|
||||||
/// - [Python documentation: Comparisons](https://docs.python.org/3/reference/expressions.html#comparisons)
|
/// - [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)
|
/// - [Python documentation: Membership test operations](https://docs.python.org/3/reference/expressions.html#membership-test-operations)
|
||||||
|
|
@ -98,7 +101,8 @@ pub(crate) fn single_item_membership_test(
|
||||||
expr.range(),
|
expr.range(),
|
||||||
);
|
);
|
||||||
|
|
||||||
let applicability = if right.is_string_literal_expr() {
|
let applicability =
|
||||||
|
if right.is_string_literal_expr() || checker.comment_ranges().intersects(expr.range()) {
|
||||||
Applicability::Unsafe
|
Applicability::Unsafe
|
||||||
} else {
|
} else {
|
||||||
Applicability::Safe
|
Applicability::Safe
|
||||||
|
|
|
||||||
|
|
@ -119,3 +119,234 @@ FURB171.py:18:8: FURB171 [*] Membership test against single-item container
|
||||||
19 19 | print("Check the negated membership test")
|
19 19 | print("Check the negated membership test")
|
||||||
20 20 |
|
20 20 |
|
||||||
21 21 | # Non-errors.
|
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…
Add table
Add a link
Reference in a new issue