Skip to content

Commit 9d3cad9

Browse files
authored
[refurb] Add coverage of set and frozenset calls (FURB171) (#18035)
## Summary Adds coverage of using set(...) in addition to `{...} in SingleItemMembershipTest. Fixes #15792 (and replaces the old PR #15793) <!-- What's the purpose of the change? What does it do, and why? --> ## Test Plan Updated unit test and snapshot. Steps to reproduce are in the issue linked above. <!-- How was it tested? -->
1 parent 7df79cf commit 9d3cad9

File tree

6 files changed

+232
-17
lines changed

6 files changed

+232
-17
lines changed
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
# Errors.
2+
3+
if 1 in set([1]):
4+
print("Single-element set")
5+
6+
if 1 in set((1,)):
7+
print("Single-element set")
8+
9+
if 1 in set({1}):
10+
print("Single-element set")
11+
12+
if 1 in frozenset([1]):
13+
print("Single-element set")
14+
15+
if 1 in frozenset((1,)):
16+
print("Single-element set")
17+
18+
if 1 in frozenset({1}):
19+
print("Single-element set")
20+
21+
if 1 in set(set([1])):
22+
print('Recursive solution')
23+
24+
25+
26+
# Non-errors.
27+
28+
if 1 in set((1, 2)):
29+
pass
30+
31+
if 1 in set([1, 2]):
32+
pass
33+
34+
if 1 in set({1, 2}):
35+
pass
36+
37+
if 1 in frozenset((1, 2)):
38+
pass
39+
40+
if 1 in frozenset([1, 2]):
41+
pass
42+
43+
if 1 in frozenset({1, 2}):
44+
pass
45+
46+
if 1 in set(1,):
47+
pass
48+
49+
if 1 in set(1,2):
50+
pass
51+
52+
if 1 in set((x for x in range(2))):
53+
pass

crates/ruff_linter/src/rules/refurb/mod.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ mod tests {
3535
#[test_case(Rule::UnnecessaryFromFloat, Path::new("FURB164.py"))]
3636
#[test_case(Rule::PrintEmptyString, Path::new("FURB105.py"))]
3737
#[test_case(Rule::ImplicitCwd, Path::new("FURB177.py"))]
38-
#[test_case(Rule::SingleItemMembershipTest, Path::new("FURB171.py"))]
38+
#[test_case(Rule::SingleItemMembershipTest, Path::new("FURB171_0.py"))]
39+
#[test_case(Rule::SingleItemMembershipTest, Path::new("FURB171_1.py"))]
3940
#[test_case(Rule::BitCount, Path::new("FURB161.py"))]
4041
#[test_case(Rule::IntOnSlicedStr, Path::new("FURB166.py"))]
4142
#[test_case(Rule::RegexFlagAlias, Path::new("FURB167.py"))]

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

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use ruff_macros::{ViolationMetadata, derive_message_formats};
22
use ruff_python_ast::helpers::generate_comparison;
33
use ruff_python_ast::{self as ast, CmpOp, Expr, ExprStringLiteral};
4+
use ruff_python_semantic::SemanticModel;
45
use ruff_text_size::Ranged;
56

67
use crate::checkers::ast::Checker;
@@ -78,8 +79,8 @@ pub(crate) fn single_item_membership_test(
7879
_ => return,
7980
};
8081

81-
// Check if the right-hand side is a single-item object.
82-
let Some(item) = single_item(right) else {
82+
// Check if the right-hand side is a single-item object
83+
let Some(item) = single_item(right, checker.semantic()) else {
8384
return;
8485
};
8586

@@ -115,7 +116,7 @@ pub(crate) fn single_item_membership_test(
115116

116117
/// Return the single item wrapped in `Some` if the expression contains a single
117118
/// item, otherwise return `None`.
118-
fn single_item(expr: &Expr) -> Option<&Expr> {
119+
fn single_item<'a>(expr: &'a Expr, semantic: &'a SemanticModel) -> Option<&'a Expr> {
119120
match expr {
120121
Expr::List(ast::ExprList { elts, .. })
121122
| Expr::Tuple(ast::ExprTuple { elts, .. })
@@ -124,6 +125,19 @@ fn single_item(expr: &Expr) -> Option<&Expr> {
124125
[item] => Some(item),
125126
_ => None,
126127
},
128+
Expr::Call(ast::ExprCall {
129+
func,
130+
arguments,
131+
range: _,
132+
}) => {
133+
if arguments.len() != 1 || !is_set_method(func, semantic) {
134+
return None;
135+
}
136+
137+
arguments
138+
.find_positional(0)
139+
.and_then(|arg| single_item(arg, semantic))
140+
}
127141
string_expr @ Expr::StringLiteral(ExprStringLiteral { value: string, .. })
128142
if string.chars().count() == 1 =>
129143
{
@@ -133,6 +147,12 @@ fn single_item(expr: &Expr) -> Option<&Expr> {
133147
}
134148
}
135149

150+
fn is_set_method(func: &Expr, semantic: &SemanticModel) -> bool {
151+
["set", "frozenset"]
152+
.iter()
153+
.any(|s| semantic.match_builtin_expr(func, s))
154+
}
155+
136156
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
137157
enum MembershipTest {
138158
/// Ex) `1 in [1]`
Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
---
22
source: crates/ruff_linter/src/rules/refurb/mod.rs
33
---
4-
FURB171.py:3:4: FURB171 [*] Membership test against single-item container
4+
FURB171_0.py:3:4: FURB171 [*] Membership test against single-item container
55
|
66
1 | # Errors.
77
2 |
@@ -20,7 +20,7 @@ FURB171.py:3:4: FURB171 [*] Membership test against single-item container
2020
5 5 |
2121
6 6 | if 1 in [1]:
2222

23-
FURB171.py:6:4: FURB171 [*] Membership test against single-item container
23+
FURB171_0.py:6:4: FURB171 [*] Membership test against single-item container
2424
|
2525
4 | print("Single-element tuple")
2626
5 |
@@ -40,7 +40,7 @@ FURB171.py:6:4: FURB171 [*] Membership test against single-item container
4040
8 8 |
4141
9 9 | if 1 in {1}:
4242

43-
FURB171.py:9:4: FURB171 [*] Membership test against single-item container
43+
FURB171_0.py:9:4: FURB171 [*] Membership test against single-item container
4444
|
4545
7 | print("Single-element list")
4646
8 |
@@ -60,7 +60,7 @@ FURB171.py:9:4: FURB171 [*] Membership test against single-item container
6060
11 11 |
6161
12 12 | if "a" in "a":
6262

63-
FURB171.py:12:4: FURB171 [*] Membership test against single-item container
63+
FURB171_0.py:12:4: FURB171 [*] Membership test against single-item container
6464
|
6565
10 | print("Single-element set")
6666
11 |
@@ -80,7 +80,7 @@ FURB171.py:12:4: FURB171 [*] Membership test against single-item container
8080
14 14 |
8181
15 15 | if 1 not in (1,):
8282

83-
FURB171.py:15:4: FURB171 [*] Membership test against single-item container
83+
FURB171_0.py:15:4: FURB171 [*] Membership test against single-item container
8484
|
8585
13 | print("Single-element string")
8686
14 |
@@ -100,7 +100,7 @@ FURB171.py:15:4: FURB171 [*] Membership test against single-item container
100100
17 17 |
101101
18 18 | if not 1 in (1,):
102102

103-
FURB171.py:18:8: FURB171 [*] Membership test against single-item container
103+
FURB171_0.py:18:8: FURB171 [*] Membership test against single-item container
104104
|
105105
16 | print("Check `not in` membership test")
106106
17 |
@@ -120,7 +120,7 @@ FURB171.py:18:8: FURB171 [*] Membership test against single-item container
120120
20 20 |
121121
21 21 | # Non-errors.
122122

123-
FURB171.py:52:5: FURB171 [*] Membership test against single-item container
123+
FURB171_0.py:52:5: FURB171 [*] Membership test against single-item container
124124
|
125125
51 | # https://github.com/astral-sh/ruff/issues/10063
126126
52 | _ = a in (
@@ -147,7 +147,7 @@ FURB171.py:52:5: FURB171 [*] Membership test against single-item container
147147
57 54 | _ = a in ( # Foo1
148148
58 55 | ( # Foo2
149149

150-
FURB171.py:57:5: FURB171 [*] Membership test against single-item container
150+
FURB171_0.py:57:5: FURB171 [*] Membership test against single-item container
151151
|
152152
55 | )
153153
56 |
@@ -199,7 +199,7 @@ FURB171.py:57:5: FURB171 [*] Membership test against single-item container
199199
74 63 | foo = (
200200
75 64 | lorem()
201201

202-
FURB171.py:77:28: FURB171 [*] Membership test against single-item container
202+
FURB171_0.py:77:28: FURB171 [*] Membership test against single-item container
203203
|
204204
75 | lorem()
205205
76 | .ipsum()
@@ -228,7 +228,7 @@ FURB171.py:77:28: FURB171 [*] Membership test against single-item container
228228
83 79 |
229229
84 80 | foo = (
230230

231-
FURB171.py:87:28: FURB171 [*] Membership test against single-item container
231+
FURB171_0.py:87:28: FURB171 [*] Membership test against single-item container
232232
|
233233
85 | lorem()
234234
86 | .ipsum()
@@ -262,7 +262,7 @@ FURB171.py:87:28: FURB171 [*] Membership test against single-item container
262262
95 93 |
263263
96 94 | foo = lorem() \
264264

265-
FURB171.py:98:24: FURB171 [*] Membership test against single-item container
265+
FURB171_0.py:98:24: FURB171 [*] Membership test against single-item container
266266
|
267267
96 | foo = lorem() \
268268
97 | .ipsum() \
@@ -292,7 +292,7 @@ FURB171.py:98:24: FURB171 [*] Membership test against single-item container
292292
104 100 | def _():
293293
105 101 | if foo not \
294294

295-
FURB171.py:105:8: FURB171 [*] Membership test against single-item container
295+
FURB171_0.py:105:8: FURB171 [*] Membership test against single-item container
296296
|
297297
104 | def _():
298298
105 | if foo not \
@@ -323,7 +323,7 @@ FURB171.py:105:8: FURB171 [*] Membership test against single-item container
323323
112 107 | def _():
324324
113 108 | if foo not \
325325

326-
FURB171.py:113:8: FURB171 [*] Membership test against single-item container
326+
FURB171_0.py:113:8: FURB171 [*] Membership test against single-item container
327327
|
328328
112 | def _():
329329
113 | if foo not \
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
---
2+
source: crates/ruff_linter/src/rules/refurb/mod.rs
3+
---
4+
FURB171_1.py:3:4: FURB171 [*] Membership test against single-item container
5+
|
6+
1 | # Errors.
7+
2 |
8+
3 | if 1 in set([1]):
9+
| ^^^^^^^^^^^^^ FURB171
10+
4 | print("Single-element set")
11+
|
12+
= help: Convert to equality test
13+
14+
Safe fix
15+
1 1 | # Errors.
16+
2 2 |
17+
3 |-if 1 in set([1]):
18+
3 |+if 1 == 1:
19+
4 4 | print("Single-element set")
20+
5 5 |
21+
6 6 | if 1 in set((1,)):
22+
23+
FURB171_1.py:6:4: FURB171 [*] Membership test against single-item container
24+
|
25+
4 | print("Single-element set")
26+
5 |
27+
6 | if 1 in set((1,)):
28+
| ^^^^^^^^^^^^^^ FURB171
29+
7 | print("Single-element set")
30+
|
31+
= help: Convert to equality test
32+
33+
Safe fix
34+
3 3 | if 1 in set([1]):
35+
4 4 | print("Single-element set")
36+
5 5 |
37+
6 |-if 1 in set((1,)):
38+
6 |+if 1 == 1:
39+
7 7 | print("Single-element set")
40+
8 8 |
41+
9 9 | if 1 in set({1}):
42+
43+
FURB171_1.py:9:4: FURB171 [*] Membership test against single-item container
44+
|
45+
7 | print("Single-element set")
46+
8 |
47+
9 | if 1 in set({1}):
48+
| ^^^^^^^^^^^^^ FURB171
49+
10 | print("Single-element set")
50+
|
51+
= help: Convert to equality test
52+
53+
Safe fix
54+
6 6 | if 1 in set((1,)):
55+
7 7 | print("Single-element set")
56+
8 8 |
57+
9 |-if 1 in set({1}):
58+
9 |+if 1 == 1:
59+
10 10 | print("Single-element set")
60+
11 11 |
61+
12 12 | if 1 in frozenset([1]):
62+
63+
FURB171_1.py:12:4: FURB171 [*] Membership test against single-item container
64+
|
65+
10 | print("Single-element set")
66+
11 |
67+
12 | if 1 in frozenset([1]):
68+
| ^^^^^^^^^^^^^^^^^^^ FURB171
69+
13 | print("Single-element set")
70+
|
71+
= help: Convert to equality test
72+
73+
Safe fix
74+
9 9 | if 1 in set({1}):
75+
10 10 | print("Single-element set")
76+
11 11 |
77+
12 |-if 1 in frozenset([1]):
78+
12 |+if 1 == 1:
79+
13 13 | print("Single-element set")
80+
14 14 |
81+
15 15 | if 1 in frozenset((1,)):
82+
83+
FURB171_1.py:15:4: FURB171 [*] Membership test against single-item container
84+
|
85+
13 | print("Single-element set")
86+
14 |
87+
15 | if 1 in frozenset((1,)):
88+
| ^^^^^^^^^^^^^^^^^^^^ FURB171
89+
16 | print("Single-element set")
90+
|
91+
= help: Convert to equality test
92+
93+
Safe fix
94+
12 12 | if 1 in frozenset([1]):
95+
13 13 | print("Single-element set")
96+
14 14 |
97+
15 |-if 1 in frozenset((1,)):
98+
15 |+if 1 == 1:
99+
16 16 | print("Single-element set")
100+
17 17 |
101+
18 18 | if 1 in frozenset({1}):
102+
103+
FURB171_1.py:18:4: FURB171 [*] Membership test against single-item container
104+
|
105+
16 | print("Single-element set")
106+
17 |
107+
18 | if 1 in frozenset({1}):
108+
| ^^^^^^^^^^^^^^^^^^^ FURB171
109+
19 | print("Single-element set")
110+
|
111+
= help: Convert to equality test
112+
113+
Safe fix
114+
15 15 | if 1 in frozenset((1,)):
115+
16 16 | print("Single-element set")
116+
17 17 |
117+
18 |-if 1 in frozenset({1}):
118+
18 |+if 1 == 1:
119+
19 19 | print("Single-element set")
120+
20 20 |
121+
21 21 | if 1 in set(set([1])):
122+
123+
FURB171_1.py:21:4: FURB171 [*] Membership test against single-item container
124+
|
125+
19 | print("Single-element set")
126+
20 |
127+
21 | if 1 in set(set([1])):
128+
| ^^^^^^^^^^^^^^^^^^ FURB171
129+
22 | print('Recursive solution')
130+
|
131+
= help: Convert to equality test
132+
133+
Safe fix
134+
18 18 | if 1 in frozenset({1}):
135+
19 19 | print("Single-element set")
136+
20 20 |
137+
21 |-if 1 in set(set([1])):
138+
21 |+if 1 == 1:
139+
22 22 | print('Recursive solution')
140+
23 23 |
141+
24 24 |

0 commit comments

Comments
 (0)