Skip to content

Commit 04ad562

Browse files
committed
[pylint] Improve repeated-equality-comparison fix to use a set when all elements are hashable (PLR1714) (#16685)
## Summary This PR promotes the fix improvements for `PLR1714` that were introduced in #14372 to stable. The improvement is that the fix now proposes to use a set if all elements are hashable: ``` foo == "bar" or foo == "baz" or foo == "qux" ``` Gets fixed to ```py foo in {"bar", "baz", "qux"} ``` where it previously always got fixed to a tuple. The new fix was first released in ruff 0.8.0 (Nov last year). This is not a breaking change. The change was preview gated only to get some extra test coverage. There are no open issues or PRs related to this changed fix behavior.
1 parent 9167471 commit 04ad562

4 files changed

+48
-510
lines changed

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -441,10 +441,6 @@ mod tests {
441441
Ok(())
442442
}
443443

444-
#[test_case(
445-
Rule::RepeatedEqualityComparison,
446-
Path::new("repeated_equality_comparison.py")
447-
)]
448444
#[test_case(Rule::BadStrStripCall, Path::new("bad_str_strip_call.py"))]
449445
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
450446
let snapshot = format!(

crates/ruff_linter/src/rules/pylint/rules/repeated_equality_comparison.rs

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,13 @@ use crate::fix::snippet::SourceCodeSnippet;
1515
use crate::Locator;
1616

1717
/// ## What it does
18-
/// Checks for repeated equality comparisons that can rewritten as a membership
18+
/// Checks for repeated equality comparisons that can be rewritten as a membership
1919
/// test.
2020
///
21+
/// This rule will try to determine if the values are hashable
22+
/// and the fix will use a `set` if they are. If unable to determine, the fix
23+
/// will use a `tuple` and suggest the use of a `set`.
24+
///
2125
/// ## Why is this bad?
2226
/// To check if a variable is equal to one of many values, it is common to
2327
/// write a series of equality comparisons (e.g.,
@@ -28,10 +32,6 @@ use crate::Locator;
2832
/// If the items are hashable, use a `set` for efficiency; otherwise, use a
2933
/// `tuple`.
3034
///
31-
/// In [preview], this rule will try to determine if the values are hashable
32-
/// and the fix will use a `set` if they are. If unable to determine, the fix
33-
/// will use a `tuple` and continue to suggest the use of a `set`.
34-
///
3535
/// ## Example
3636
/// ```python
3737
/// foo == "bar" or foo == "baz" or foo == "qux"
@@ -46,8 +46,6 @@ use crate::Locator;
4646
/// - [Python documentation: Comparisons](https://docs.python.org/3/reference/expressions.html#comparisons)
4747
/// - [Python documentation: Membership test operations](https://docs.python.org/3/reference/expressions.html#membership-test-operations)
4848
/// - [Python documentation: `set`](https://docs.python.org/3/library/stdtypes.html#set)
49-
///
50-
/// [preview]: https://docs.astral.sh/ruff/preview/
5149
#[derive(ViolationMetadata)]
5250
pub(crate) struct RepeatedEqualityComparison {
5351
expression: SourceCodeSnippet,
@@ -135,10 +133,9 @@ pub(crate) fn repeated_equality_comparison(checker: &Checker, bool_op: &ast::Exp
135133

136134
// if we can determine that all the values are hashable, we can use a set
137135
// TODO: improve with type inference
138-
let all_hashable = checker.settings.preview.is_enabled()
139-
&& comparators
140-
.iter()
141-
.all(|comparator| comparator.is_literal_expr());
136+
let all_hashable = comparators
137+
.iter()
138+
.all(|comparator| comparator.is_literal_expr());
142139

143140
let mut diagnostic = Diagnostic::new(
144141
RepeatedEqualityComparison {

crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1714_repeated_equality_comparison.py.snap

Lines changed: 40 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
---
22
source: crates/ruff_linter/src/rules/pylint/mod.rs
33
---
4-
repeated_equality_comparison.py:2:1: PLR1714 [*] Consider merging multiple comparisons: `foo in ("a", "b")`. Use a `set` if the elements are hashable.
4+
repeated_equality_comparison.py:2:1: PLR1714 [*] Consider merging multiple comparisons: `foo in {"a", "b"}`.
55
|
66
1 | # Errors.
77
2 | foo == "a" or foo == "b"
@@ -14,12 +14,12 @@ repeated_equality_comparison.py:2:1: PLR1714 [*] Consider merging multiple compa
1414
Unsafe fix
1515
1 1 | # Errors.
1616
2 |-foo == "a" or foo == "b"
17-
2 |+foo in ("a", "b")
17+
2 |+foo in {"a", "b"}
1818
3 3 |
1919
4 4 | foo != "a" and foo != "b"
2020
5 5 |
2121

22-
repeated_equality_comparison.py:4:1: PLR1714 [*] Consider merging multiple comparisons: `foo not in ("a", "b")`. Use a `set` if the elements are hashable.
22+
repeated_equality_comparison.py:4:1: PLR1714 [*] Consider merging multiple comparisons: `foo not in {"a", "b"}`.
2323
|
2424
2 | foo == "a" or foo == "b"
2525
3 |
@@ -35,12 +35,12 @@ repeated_equality_comparison.py:4:1: PLR1714 [*] Consider merging multiple compa
3535
2 2 | foo == "a" or foo == "b"
3636
3 3 |
3737
4 |-foo != "a" and foo != "b"
38-
4 |+foo not in ("a", "b")
38+
4 |+foo not in {"a", "b"}
3939
5 5 |
4040
6 6 | foo == "a" or foo == "b" or foo == "c"
4141
7 7 |
4242

43-
repeated_equality_comparison.py:6:1: PLR1714 [*] Consider merging multiple comparisons: `foo in ("a", "b", "c")`. Use a `set` if the elements are hashable.
43+
repeated_equality_comparison.py:6:1: PLR1714 [*] Consider merging multiple comparisons: `foo in {"a", "b", "c"}`.
4444
|
4545
4 | foo != "a" and foo != "b"
4646
5 |
@@ -56,12 +56,12 @@ repeated_equality_comparison.py:6:1: PLR1714 [*] Consider merging multiple compa
5656
4 4 | foo != "a" and foo != "b"
5757
5 5 |
5858
6 |-foo == "a" or foo == "b" or foo == "c"
59-
6 |+foo in ("a", "b", "c")
59+
6 |+foo in {"a", "b", "c"}
6060
7 7 |
6161
8 8 | foo != "a" and foo != "b" and foo != "c"
6262
9 9 |
6363

64-
repeated_equality_comparison.py:8:1: PLR1714 [*] Consider merging multiple comparisons: `foo not in ("a", "b", "c")`. Use a `set` if the elements are hashable.
64+
repeated_equality_comparison.py:8:1: PLR1714 [*] Consider merging multiple comparisons: `foo not in {"a", "b", "c"}`.
6565
|
6666
6 | foo == "a" or foo == "b" or foo == "c"
6767
7 |
@@ -77,7 +77,7 @@ repeated_equality_comparison.py:8:1: PLR1714 [*] Consider merging multiple compa
7777
6 6 | foo == "a" or foo == "b" or foo == "c"
7878
7 7 |
7979
8 |-foo != "a" and foo != "b" and foo != "c"
80-
8 |+foo not in ("a", "b", "c")
80+
8 |+foo not in {"a", "b", "c"}
8181
9 9 |
8282
10 10 | foo == a or foo == "b" or foo == 3 # Mixed types.
8383
11 11 |
@@ -103,7 +103,7 @@ repeated_equality_comparison.py:10:1: PLR1714 [*] Consider merging multiple comp
103103
12 12 | "a" == foo or "b" == foo or "c" == foo
104104
13 13 |
105105

106-
repeated_equality_comparison.py:12:1: PLR1714 [*] Consider merging multiple comparisons: `foo in ("a", "b", "c")`. Use a `set` if the elements are hashable.
106+
repeated_equality_comparison.py:12:1: PLR1714 [*] Consider merging multiple comparisons: `foo in {"a", "b", "c"}`.
107107
|
108108
10 | foo == a or foo == "b" or foo == 3 # Mixed types.
109109
11 |
@@ -119,12 +119,12 @@ repeated_equality_comparison.py:12:1: PLR1714 [*] Consider merging multiple comp
119119
10 10 | foo == a or foo == "b" or foo == 3 # Mixed types.
120120
11 11 |
121121
12 |-"a" == foo or "b" == foo or "c" == foo
122-
12 |+foo in ("a", "b", "c")
122+
12 |+foo in {"a", "b", "c"}
123123
13 13 |
124124
14 14 | "a" != foo and "b" != foo and "c" != foo
125125
15 15 |
126126

127-
repeated_equality_comparison.py:14:1: PLR1714 [*] Consider merging multiple comparisons: `foo not in ("a", "b", "c")`. Use a `set` if the elements are hashable.
127+
repeated_equality_comparison.py:14:1: PLR1714 [*] Consider merging multiple comparisons: `foo not in {"a", "b", "c"}`.
128128
|
129129
12 | "a" == foo or "b" == foo or "c" == foo
130130
13 |
@@ -140,12 +140,12 @@ repeated_equality_comparison.py:14:1: PLR1714 [*] Consider merging multiple comp
140140
12 12 | "a" == foo or "b" == foo or "c" == foo
141141
13 13 |
142142
14 |-"a" != foo and "b" != foo and "c" != foo
143-
14 |+foo not in ("a", "b", "c")
143+
14 |+foo not in {"a", "b", "c"}
144144
15 15 |
145145
16 16 | "a" == foo or foo == "b" or "c" == foo
146146
17 17 |
147147

148-
repeated_equality_comparison.py:16:1: PLR1714 [*] Consider merging multiple comparisons: `foo in ("a", "b", "c")`. Use a `set` if the elements are hashable.
148+
repeated_equality_comparison.py:16:1: PLR1714 [*] Consider merging multiple comparisons: `foo in {"a", "b", "c"}`.
149149
|
150150
14 | "a" != foo and "b" != foo and "c" != foo
151151
15 |
@@ -161,7 +161,7 @@ repeated_equality_comparison.py:16:1: PLR1714 [*] Consider merging multiple comp
161161
14 14 | "a" != foo and "b" != foo and "c" != foo
162162
15 15 |
163163
16 |-"a" == foo or foo == "b" or "c" == foo
164-
16 |+foo in ("a", "b", "c")
164+
16 |+foo in {"a", "b", "c"}
165165
17 17 |
166166
18 18 | foo == bar or baz == foo or qux == foo
167167
19 19 |
@@ -187,7 +187,7 @@ repeated_equality_comparison.py:18:1: PLR1714 [*] Consider merging multiple comp
187187
20 20 | foo == "a" or "b" == foo or foo == "c"
188188
21 21 |
189189

190-
repeated_equality_comparison.py:20:1: PLR1714 [*] Consider merging multiple comparisons: `foo in ("a", "b", "c")`. Use a `set` if the elements are hashable.
190+
repeated_equality_comparison.py:20:1: PLR1714 [*] Consider merging multiple comparisons: `foo in {"a", "b", "c"}`.
191191
|
192192
18 | foo == bar or baz == foo or qux == foo
193193
19 |
@@ -203,12 +203,12 @@ repeated_equality_comparison.py:20:1: PLR1714 [*] Consider merging multiple comp
203203
18 18 | foo == bar or baz == foo or qux == foo
204204
19 19 |
205205
20 |-foo == "a" or "b" == foo or foo == "c"
206-
20 |+foo in ("a", "b", "c")
206+
20 |+foo in {"a", "b", "c"}
207207
21 21 |
208208
22 22 | foo != "a" and "b" != foo and foo != "c"
209209
23 23 |
210210

211-
repeated_equality_comparison.py:22:1: PLR1714 [*] Consider merging multiple comparisons: `foo not in ("a", "b", "c")`. Use a `set` if the elements are hashable.
211+
repeated_equality_comparison.py:22:1: PLR1714 [*] Consider merging multiple comparisons: `foo not in {"a", "b", "c"}`.
212212
|
213213
20 | foo == "a" or "b" == foo or foo == "c"
214214
21 |
@@ -224,12 +224,12 @@ repeated_equality_comparison.py:22:1: PLR1714 [*] Consider merging multiple comp
224224
20 20 | foo == "a" or "b" == foo or foo == "c"
225225
21 21 |
226226
22 |-foo != "a" and "b" != foo and foo != "c"
227-
22 |+foo not in ("a", "b", "c")
227+
22 |+foo not in {"a", "b", "c"}
228228
23 23 |
229229
24 24 | foo == "a" or foo == "b" or "c" == bar or "d" == bar # Multiple targets
230230
25 25 |
231231

232-
repeated_equality_comparison.py:24:1: PLR1714 [*] Consider merging multiple comparisons: `foo in ("a", "b")`. Use a `set` if the elements are hashable.
232+
repeated_equality_comparison.py:24:1: PLR1714 [*] Consider merging multiple comparisons: `foo in {"a", "b"}`.
233233
|
234234
22 | foo != "a" and "b" != foo and foo != "c"
235235
23 |
@@ -245,12 +245,12 @@ repeated_equality_comparison.py:24:1: PLR1714 [*] Consider merging multiple comp
245245
22 22 | foo != "a" and "b" != foo and foo != "c"
246246
23 23 |
247247
24 |-foo == "a" or foo == "b" or "c" == bar or "d" == bar # Multiple targets
248-
24 |+foo in ("a", "b") or "c" == bar or "d" == bar # Multiple targets
248+
24 |+foo in {"a", "b"} or "c" == bar or "d" == bar # Multiple targets
249249
25 25 |
250250
26 26 | foo.bar == "a" or foo.bar == "b" # Attributes.
251251
27 27 |
252252

253-
repeated_equality_comparison.py:24:1: PLR1714 [*] Consider merging multiple comparisons: `bar in ("c", "d")`. Use a `set` if the elements are hashable.
253+
repeated_equality_comparison.py:24:1: PLR1714 [*] Consider merging multiple comparisons: `bar in {"c", "d"}`.
254254
|
255255
22 | foo != "a" and "b" != foo and foo != "c"
256256
23 |
@@ -266,12 +266,12 @@ repeated_equality_comparison.py:24:1: PLR1714 [*] Consider merging multiple comp
266266
22 22 | foo != "a" and "b" != foo and foo != "c"
267267
23 23 |
268268
24 |-foo == "a" or foo == "b" or "c" == bar or "d" == bar # Multiple targets
269-
24 |+foo == "a" or foo == "b" or bar in ("c", "d") # Multiple targets
269+
24 |+foo == "a" or foo == "b" or bar in {"c", "d"} # Multiple targets
270270
25 25 |
271271
26 26 | foo.bar == "a" or foo.bar == "b" # Attributes.
272272
27 27 |
273273

274-
repeated_equality_comparison.py:26:1: PLR1714 [*] Consider merging multiple comparisons: `foo.bar in ("a", "b")`. Use a `set` if the elements are hashable.
274+
repeated_equality_comparison.py:26:1: PLR1714 [*] Consider merging multiple comparisons: `foo.bar in {"a", "b"}`.
275275
|
276276
24 | foo == "a" or foo == "b" or "c" == bar or "d" == bar # Multiple targets
277277
25 |
@@ -287,12 +287,12 @@ repeated_equality_comparison.py:26:1: PLR1714 [*] Consider merging multiple comp
287287
24 24 | foo == "a" or foo == "b" or "c" == bar or "d" == bar # Multiple targets
288288
25 25 |
289289
26 |-foo.bar == "a" or foo.bar == "b" # Attributes.
290-
26 |+foo.bar in ("a", "b") # Attributes.
290+
26 |+foo.bar in {"a", "b"} # Attributes.
291291
27 27 |
292292
28 28 | # OK
293293
29 29 | foo == "a" and foo == "b" and foo == "c" # `and` mixed with `==`.
294294

295-
repeated_equality_comparison.py:61:16: PLR1714 [*] Consider merging multiple comparisons: `bar in ("c", "d")`. Use a `set` if the elements are hashable.
295+
repeated_equality_comparison.py:61:16: PLR1714 [*] Consider merging multiple comparisons: `bar in {"c", "d"}`.
296296
|
297297
59 | foo == "a" or "c" == bar or foo == "b" or "d" == bar # Multiple targets
298298
60 |
@@ -308,12 +308,12 @@ repeated_equality_comparison.py:61:16: PLR1714 [*] Consider merging multiple com
308308
59 59 | foo == "a" or "c" == bar or foo == "b" or "d" == bar # Multiple targets
309309
60 60 |
310310
61 |-foo == "a" or ("c" == bar or "d" == bar) or foo == "b" # Multiple targets
311-
61 |+foo == "a" or (bar in ("c", "d")) or foo == "b" # Multiple targets
311+
61 |+foo == "a" or (bar in {"c", "d"}) or foo == "b" # Multiple targets
312312
62 62 |
313313
63 63 | foo == "a" or foo == "b" or "c" != bar and "d" != bar # Multiple targets
314314
64 64 |
315315

316-
repeated_equality_comparison.py:63:1: PLR1714 [*] Consider merging multiple comparisons: `foo in ("a", "b")`. Use a `set` if the elements are hashable.
316+
repeated_equality_comparison.py:63:1: PLR1714 [*] Consider merging multiple comparisons: `foo in {"a", "b"}`.
317317
|
318318
61 | foo == "a" or ("c" == bar or "d" == bar) or foo == "b" # Multiple targets
319319
62 |
@@ -329,12 +329,12 @@ repeated_equality_comparison.py:63:1: PLR1714 [*] Consider merging multiple comp
329329
61 61 | foo == "a" or ("c" == bar or "d" == bar) or foo == "b" # Multiple targets
330330
62 62 |
331331
63 |-foo == "a" or foo == "b" or "c" != bar and "d" != bar # Multiple targets
332-
63 |+foo in ("a", "b") or "c" != bar and "d" != bar # Multiple targets
332+
63 |+foo in {"a", "b"} or "c" != bar and "d" != bar # Multiple targets
333333
64 64 |
334334
65 65 | foo == "a" or ("c" != bar and "d" != bar) or foo == "b" # Multiple targets
335335
66 66 |
336336

337-
repeated_equality_comparison.py:63:29: PLR1714 [*] Consider merging multiple comparisons: `bar not in ("c", "d")`. Use a `set` if the elements are hashable.
337+
repeated_equality_comparison.py:63:29: PLR1714 [*] Consider merging multiple comparisons: `bar not in {"c", "d"}`.
338338
|
339339
61 | foo == "a" or ("c" == bar or "d" == bar) or foo == "b" # Multiple targets
340340
62 |
@@ -350,12 +350,12 @@ repeated_equality_comparison.py:63:29: PLR1714 [*] Consider merging multiple com
350350
61 61 | foo == "a" or ("c" == bar or "d" == bar) or foo == "b" # Multiple targets
351351
62 62 |
352352
63 |-foo == "a" or foo == "b" or "c" != bar and "d" != bar # Multiple targets
353-
63 |+foo == "a" or foo == "b" or bar not in ("c", "d") # Multiple targets
353+
63 |+foo == "a" or foo == "b" or bar not in {"c", "d"} # Multiple targets
354354
64 64 |
355355
65 65 | foo == "a" or ("c" != bar and "d" != bar) or foo == "b" # Multiple targets
356356
66 66 |
357357

358-
repeated_equality_comparison.py:65:16: PLR1714 [*] Consider merging multiple comparisons: `bar not in ("c", "d")`. Use a `set` if the elements are hashable.
358+
repeated_equality_comparison.py:65:16: PLR1714 [*] Consider merging multiple comparisons: `bar not in {"c", "d"}`.
359359
|
360360
63 | foo == "a" or foo == "b" or "c" != bar and "d" != bar # Multiple targets
361361
64 |
@@ -371,12 +371,12 @@ repeated_equality_comparison.py:65:16: PLR1714 [*] Consider merging multiple com
371371
63 63 | foo == "a" or foo == "b" or "c" != bar and "d" != bar # Multiple targets
372372
64 64 |
373373
65 |-foo == "a" or ("c" != bar and "d" != bar) or foo == "b" # Multiple targets
374-
65 |+foo == "a" or (bar not in ("c", "d")) or foo == "b" # Multiple targets
374+
65 |+foo == "a" or (bar not in {"c", "d"}) or foo == "b" # Multiple targets
375375
66 66 |
376376
67 67 | foo == "a" and "c" != bar or foo == "b" and "d" != bar # Multiple targets
377377
68 68 |
378378

379-
repeated_equality_comparison.py:69:1: PLR1714 [*] Consider merging multiple comparisons: `foo in (1, True)`. Use a `set` if the elements are hashable.
379+
repeated_equality_comparison.py:69:1: PLR1714 [*] Consider merging multiple comparisons: `foo in {1, True}`.
380380
|
381381
67 | foo == "a" and "c" != bar or foo == "b" and "d" != bar # Multiple targets
382382
68 |
@@ -392,12 +392,12 @@ repeated_equality_comparison.py:69:1: PLR1714 [*] Consider merging multiple comp
392392
67 67 | foo == "a" and "c" != bar or foo == "b" and "d" != bar # Multiple targets
393393
68 68 |
394394
69 |-foo == 1 or foo == True # Different types, same hashed value
395-
69 |+foo in (1, True) # Different types, same hashed value
395+
69 |+foo in {1, True} # Different types, same hashed value
396396
70 70 |
397397
71 71 | foo == 1 or foo == 1.0 # Different types, same hashed value
398398
72 72 |
399399

400-
repeated_equality_comparison.py:71:1: PLR1714 [*] Consider merging multiple comparisons: `foo in (1, 1.0)`. Use a `set` if the elements are hashable.
400+
repeated_equality_comparison.py:71:1: PLR1714 [*] Consider merging multiple comparisons: `foo in {1, 1.0}`.
401401
|
402402
69 | foo == 1 or foo == True # Different types, same hashed value
403403
70 |
@@ -413,12 +413,12 @@ repeated_equality_comparison.py:71:1: PLR1714 [*] Consider merging multiple comp
413413
69 69 | foo == 1 or foo == True # Different types, same hashed value
414414
70 70 |
415415
71 |-foo == 1 or foo == 1.0 # Different types, same hashed value
416-
71 |+foo in (1, 1.0) # Different types, same hashed value
416+
71 |+foo in {1, 1.0} # Different types, same hashed value
417417
72 72 |
418418
73 73 | foo == False or foo == 0 # Different types, same hashed value
419419
74 74 |
420420

421-
repeated_equality_comparison.py:73:1: PLR1714 [*] Consider merging multiple comparisons: `foo in (False, 0)`. Use a `set` if the elements are hashable.
421+
repeated_equality_comparison.py:73:1: PLR1714 [*] Consider merging multiple comparisons: `foo in {False, 0}`.
422422
|
423423
71 | foo == 1 or foo == 1.0 # Different types, same hashed value
424424
72 |
@@ -434,11 +434,11 @@ repeated_equality_comparison.py:73:1: PLR1714 [*] Consider merging multiple comp
434434
71 71 | foo == 1 or foo == 1.0 # Different types, same hashed value
435435
72 72 |
436436
73 |-foo == False or foo == 0 # Different types, same hashed value
437-
73 |+foo in (False, 0) # Different types, same hashed value
437+
73 |+foo in {False, 0} # Different types, same hashed value
438438
74 74 |
439439
75 75 | foo == 0.0 or foo == 0j # Different types, same hashed value
440440

441-
repeated_equality_comparison.py:75:1: PLR1714 [*] Consider merging multiple comparisons: `foo in (0.0, 0j)`. Use a `set` if the elements are hashable.
441+
repeated_equality_comparison.py:75:1: PLR1714 [*] Consider merging multiple comparisons: `foo in {0.0, 0j}`.
442442
|
443443
73 | foo == False or foo == 0 # Different types, same hashed value
444444
74 |
@@ -452,4 +452,4 @@ repeated_equality_comparison.py:75:1: PLR1714 [*] Consider merging multiple comp
452452
73 73 | foo == False or foo == 0 # Different types, same hashed value
453453
74 74 |
454454
75 |-foo == 0.0 or foo == 0j # Different types, same hashed value
455-
75 |+foo in (0.0, 0j) # Different types, same hashed value
455+
75 |+foo in {0.0, 0j} # Different types, same hashed value

0 commit comments

Comments
 (0)