add autofix for PLR1714 (#7910)

## Summary

Add autofix for `PLR1714` using tuples.

If added complexity is desired, we can lean into the `set` part by doing
some kind of builtin check on all of the comparator elements for
starters, since we otherwise don't know if something's hashable.

## Test Plan

`cargo test`, and manually.
This commit is contained in:
Steve C 2023-10-11 10:42:21 -04:00 committed by GitHub
parent f670f9b22c
commit 1835d7bb45
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 200 additions and 19 deletions

View file

@ -1,19 +1,21 @@
use std::hash::BuildHasherDefault; use std::hash::BuildHasherDefault;
use std::ops::Deref; use std::ops::Deref;
use ast::ExprContext;
use itertools::{any, Itertools}; use itertools::{any, Itertools};
use rustc_hash::FxHashMap; use rustc_hash::FxHashMap;
use ruff_diagnostics::{Diagnostic, Violation}; use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::comparable::ComparableExpr; use ruff_python_ast::comparable::ComparableExpr;
use ruff_python_ast::hashable::HashableExpr; use ruff_python_ast::hashable::HashableExpr;
use ruff_python_ast::{self as ast, BoolOp, CmpOp, Expr}; use ruff_python_ast::{self as ast, BoolOp, CmpOp, Expr};
use ruff_source_file::Locator; use ruff_source_file::Locator;
use ruff_text_size::{Ranged, TextSize}; use ruff_text_size::{Ranged, TextRange, TextSize};
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
use crate::fix::snippet::SourceCodeSnippet; use crate::fix::snippet::SourceCodeSnippet;
use crate::registry::AsRule;
/// ## What it does /// ## What it does
/// Checks for repeated equality comparisons that can rewritten as a membership /// Checks for repeated equality comparisons that can rewritten as a membership
@ -48,7 +50,7 @@ pub struct RepeatedEqualityComparison {
expression: SourceCodeSnippet, expression: SourceCodeSnippet,
} }
impl Violation for RepeatedEqualityComparison { impl AlwaysFixableViolation for RepeatedEqualityComparison {
#[derive_message_formats] #[derive_message_formats]
fn message(&self) -> String { fn message(&self) -> String {
let RepeatedEqualityComparison { expression } = self; let RepeatedEqualityComparison { expression } = self;
@ -62,6 +64,10 @@ impl Violation for RepeatedEqualityComparison {
) )
} }
} }
fn fix_title(&self) -> String {
format!("Merge multiple comparisons")
}
} }
/// PLR1714 /// PLR1714
@ -115,7 +121,7 @@ pub(crate) fn repeated_equality_comparison(checker: &mut Checker, bool_op: &ast:
.sorted_by_key(|(_, (start, _))| *start) .sorted_by_key(|(_, (start, _))| *start)
{ {
if comparators.len() > 1 { if comparators.len() > 1 {
checker.diagnostics.push(Diagnostic::new( let mut diagnostic = Diagnostic::new(
RepeatedEqualityComparison { RepeatedEqualityComparison {
expression: SourceCodeSnippet::new(merged_membership_test( expression: SourceCodeSnippet::new(merged_membership_test(
value.as_expr(), value.as_expr(),
@ -125,7 +131,30 @@ pub(crate) fn repeated_equality_comparison(checker: &mut Checker, bool_op: &ast:
)), )),
}, },
bool_op.range(), bool_op.range(),
)); );
if checker.patch(diagnostic.kind.rule()) {
let content = checker.generator().expr(&Expr::Compare(ast::ExprCompare {
left: Box::new(value.as_expr().clone()),
ops: match bool_op.op {
BoolOp::Or => vec![CmpOp::In],
BoolOp::And => vec![CmpOp::NotIn],
},
comparators: vec![Expr::Tuple(ast::ExprTuple {
elts: comparators.iter().copied().cloned().collect(),
range: TextRange::default(),
ctx: ExprContext::Load,
})],
range: bool_op.range(),
}));
diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
content,
bool_op.range(),
)));
}
checker.diagnostics.push(diagnostic);
} }
} }
} }

View file

@ -1,7 +1,7 @@
--- ---
source: crates/ruff_linter/src/rules/pylint/mod.rs source: crates/ruff_linter/src/rules/pylint/mod.rs
--- ---
repeated_equality_comparison.py:2:1: PLR1714 Consider merging multiple comparisons: `foo in ("a", "b")`. Use a `set` if the elements are hashable. repeated_equality_comparison.py:2:1: PLR1714 [*] Consider merging multiple comparisons: `foo in ("a", "b")`. Use a `set` if the elements are hashable.
| |
1 | # Errors. 1 | # Errors.
2 | foo == "a" or foo == "b" 2 | foo == "a" or foo == "b"
@ -9,8 +9,17 @@ repeated_equality_comparison.py:2:1: PLR1714 Consider merging multiple compariso
3 | 3 |
4 | foo != "a" and foo != "b" 4 | foo != "a" and foo != "b"
| |
= help: Merge multiple comparisons
repeated_equality_comparison.py:4:1: PLR1714 Consider merging multiple comparisons: `foo not in ("a", "b")`. Use a `set` if the elements are hashable. Suggested fix
1 1 | # Errors.
2 |-foo == "a" or foo == "b"
2 |+foo in ("a", "b")
3 3 |
4 4 | foo != "a" and foo != "b"
5 5 |
repeated_equality_comparison.py:4:1: PLR1714 [*] Consider merging multiple comparisons: `foo not in ("a", "b")`. Use a `set` if the elements are hashable.
| |
2 | foo == "a" or foo == "b" 2 | foo == "a" or foo == "b"
3 | 3 |
@ -19,8 +28,19 @@ repeated_equality_comparison.py:4:1: PLR1714 Consider merging multiple compariso
5 | 5 |
6 | foo == "a" or foo == "b" or foo == "c" 6 | foo == "a" or foo == "b" or foo == "c"
| |
= help: Merge multiple comparisons
repeated_equality_comparison.py:6:1: PLR1714 Consider merging multiple comparisons: `foo in ("a", "b", "c")`. Use a `set` if the elements are hashable. Suggested fix
1 1 | # Errors.
2 2 | foo == "a" or foo == "b"
3 3 |
4 |-foo != "a" and foo != "b"
4 |+foo not in ("a", "b")
5 5 |
6 6 | foo == "a" or foo == "b" or foo == "c"
7 7 |
repeated_equality_comparison.py:6:1: PLR1714 [*] Consider merging multiple comparisons: `foo in ("a", "b", "c")`. Use a `set` if the elements are hashable.
| |
4 | foo != "a" and foo != "b" 4 | foo != "a" and foo != "b"
5 | 5 |
@ -29,8 +49,19 @@ repeated_equality_comparison.py:6:1: PLR1714 Consider merging multiple compariso
7 | 7 |
8 | foo != "a" and foo != "b" and foo != "c" 8 | foo != "a" and foo != "b" and foo != "c"
| |
= help: Merge multiple comparisons
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. Suggested fix
3 3 |
4 4 | foo != "a" and foo != "b"
5 5 |
6 |-foo == "a" or foo == "b" or foo == "c"
6 |+foo in ("a", "b", "c")
7 7 |
8 8 | foo != "a" and foo != "b" and foo != "c"
9 9 |
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.
| |
6 | foo == "a" or foo == "b" or foo == "c" 6 | foo == "a" or foo == "b" or foo == "c"
7 | 7 |
@ -39,8 +70,19 @@ repeated_equality_comparison.py:8:1: PLR1714 Consider merging multiple compariso
9 | 9 |
10 | foo == a or foo == "b" or foo == 3 # Mixed types. 10 | foo == a or foo == "b" or foo == 3 # Mixed types.
| |
= help: Merge multiple comparisons
repeated_equality_comparison.py:10:1: PLR1714 Consider merging multiple comparisons: `foo in (a, "b", 3)`. Use a `set` if the elements are hashable. Suggested fix
5 5 |
6 6 | foo == "a" or foo == "b" or foo == "c"
7 7 |
8 |-foo != "a" and foo != "b" and foo != "c"
8 |+foo not in ("a", "b", "c")
9 9 |
10 10 | foo == a or foo == "b" or foo == 3 # Mixed types.
11 11 |
repeated_equality_comparison.py:10:1: PLR1714 [*] Consider merging multiple comparisons: `foo in (a, "b", 3)`. Use a `set` if the elements are hashable.
| |
8 | foo != "a" and foo != "b" and foo != "c" 8 | foo != "a" and foo != "b" and foo != "c"
9 | 9 |
@ -49,8 +91,19 @@ repeated_equality_comparison.py:10:1: PLR1714 Consider merging multiple comparis
11 | 11 |
12 | "a" == foo or "b" == foo or "c" == foo 12 | "a" == foo or "b" == foo or "c" == foo
| |
= help: Merge multiple comparisons
repeated_equality_comparison.py:12:1: PLR1714 Consider merging multiple comparisons: `foo in ("a", "b", "c")`. Use a `set` if the elements are hashable. Suggested fix
7 7 |
8 8 | foo != "a" and foo != "b" and foo != "c"
9 9 |
10 |-foo == a or foo == "b" or foo == 3 # Mixed types.
10 |+foo in (a, "b", 3) # Mixed types.
11 11 |
12 12 | "a" == foo or "b" == foo or "c" == foo
13 13 |
repeated_equality_comparison.py:12:1: PLR1714 [*] Consider merging multiple comparisons: `foo in ("a", "b", "c")`. Use a `set` if the elements are hashable.
| |
10 | foo == a or foo == "b" or foo == 3 # Mixed types. 10 | foo == a or foo == "b" or foo == 3 # Mixed types.
11 | 11 |
@ -59,8 +112,19 @@ repeated_equality_comparison.py:12:1: PLR1714 Consider merging multiple comparis
13 | 13 |
14 | "a" != foo and "b" != foo and "c" != foo 14 | "a" != foo and "b" != foo and "c" != foo
| |
= help: Merge multiple comparisons
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. Suggested fix
9 9 |
10 10 | foo == a or foo == "b" or foo == 3 # Mixed types.
11 11 |
12 |-"a" == foo or "b" == foo or "c" == foo
12 |+foo in ("a", "b", "c")
13 13 |
14 14 | "a" != foo and "b" != foo and "c" != foo
15 15 |
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.
| |
12 | "a" == foo or "b" == foo or "c" == foo 12 | "a" == foo or "b" == foo or "c" == foo
13 | 13 |
@ -69,8 +133,19 @@ repeated_equality_comparison.py:14:1: PLR1714 Consider merging multiple comparis
15 | 15 |
16 | "a" == foo or foo == "b" or "c" == foo 16 | "a" == foo or foo == "b" or "c" == foo
| |
= help: Merge multiple comparisons
repeated_equality_comparison.py:16:1: PLR1714 Consider merging multiple comparisons: `foo in ("a", "b", "c")`. Use a `set` if the elements are hashable. Suggested fix
11 11 |
12 12 | "a" == foo or "b" == foo or "c" == foo
13 13 |
14 |-"a" != foo and "b" != foo and "c" != foo
14 |+foo not in ("a", "b", "c")
15 15 |
16 16 | "a" == foo or foo == "b" or "c" == foo
17 17 |
repeated_equality_comparison.py:16:1: PLR1714 [*] Consider merging multiple comparisons: `foo in ("a", "b", "c")`. Use a `set` if the elements are hashable.
| |
14 | "a" != foo and "b" != foo and "c" != foo 14 | "a" != foo and "b" != foo and "c" != foo
15 | 15 |
@ -79,8 +154,19 @@ repeated_equality_comparison.py:16:1: PLR1714 Consider merging multiple comparis
17 | 17 |
18 | foo == bar or baz == foo or qux == foo 18 | foo == bar or baz == foo or qux == foo
| |
= help: Merge multiple comparisons
repeated_equality_comparison.py:18:1: PLR1714 Consider merging multiple comparisons: `foo in (bar, baz, qux)`. Use a `set` if the elements are hashable. Suggested fix
13 13 |
14 14 | "a" != foo and "b" != foo and "c" != foo
15 15 |
16 |-"a" == foo or foo == "b" or "c" == foo
16 |+foo in ("a", "b", "c")
17 17 |
18 18 | foo == bar or baz == foo or qux == foo
19 19 |
repeated_equality_comparison.py:18:1: PLR1714 [*] Consider merging multiple comparisons: `foo in (bar, baz, qux)`. Use a `set` if the elements are hashable.
| |
16 | "a" == foo or foo == "b" or "c" == foo 16 | "a" == foo or foo == "b" or "c" == foo
17 | 17 |
@ -89,8 +175,19 @@ repeated_equality_comparison.py:18:1: PLR1714 Consider merging multiple comparis
19 | 19 |
20 | foo == "a" or "b" == foo or foo == "c" 20 | foo == "a" or "b" == foo or foo == "c"
| |
= help: Merge multiple comparisons
repeated_equality_comparison.py:20:1: PLR1714 Consider merging multiple comparisons: `foo in ("a", "b", "c")`. Use a `set` if the elements are hashable. Suggested fix
15 15 |
16 16 | "a" == foo or foo == "b" or "c" == foo
17 17 |
18 |-foo == bar or baz == foo or qux == foo
18 |+foo in (bar, baz, qux)
19 19 |
20 20 | foo == "a" or "b" == foo or foo == "c"
21 21 |
repeated_equality_comparison.py:20:1: PLR1714 [*] Consider merging multiple comparisons: `foo in ("a", "b", "c")`. Use a `set` if the elements are hashable.
| |
18 | foo == bar or baz == foo or qux == foo 18 | foo == bar or baz == foo or qux == foo
19 | 19 |
@ -99,8 +196,19 @@ repeated_equality_comparison.py:20:1: PLR1714 Consider merging multiple comparis
21 | 21 |
22 | foo != "a" and "b" != foo and foo != "c" 22 | foo != "a" and "b" != foo and foo != "c"
| |
= help: Merge multiple comparisons
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. Suggested fix
17 17 |
18 18 | foo == bar or baz == foo or qux == foo
19 19 |
20 |-foo == "a" or "b" == foo or foo == "c"
20 |+foo in ("a", "b", "c")
21 21 |
22 22 | foo != "a" and "b" != foo and foo != "c"
23 23 |
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.
| |
20 | foo == "a" or "b" == foo or foo == "c" 20 | foo == "a" or "b" == foo or foo == "c"
21 | 21 |
@ -109,8 +217,19 @@ repeated_equality_comparison.py:22:1: PLR1714 Consider merging multiple comparis
23 | 23 |
24 | foo == "a" or foo == "b" or "c" == bar or "d" == bar # Multiple targets 24 | foo == "a" or foo == "b" or "c" == bar or "d" == bar # Multiple targets
| |
= help: Merge multiple comparisons
repeated_equality_comparison.py:24:1: PLR1714 Consider merging multiple comparisons: `foo in ("a", "b")`. Use a `set` if the elements are hashable. Suggested fix
19 19 |
20 20 | foo == "a" or "b" == foo or foo == "c"
21 21 |
22 |-foo != "a" and "b" != foo and foo != "c"
22 |+foo not in ("a", "b", "c")
23 23 |
24 24 | foo == "a" or foo == "b" or "c" == bar or "d" == bar # Multiple targets
25 25 |
repeated_equality_comparison.py:24:1: PLR1714 [*] Consider merging multiple comparisons: `foo in ("a", "b")`. Use a `set` if the elements are hashable.
| |
22 | foo != "a" and "b" != foo and foo != "c" 22 | foo != "a" and "b" != foo and foo != "c"
23 | 23 |
@ -119,8 +238,19 @@ repeated_equality_comparison.py:24:1: PLR1714 Consider merging multiple comparis
25 | 25 |
26 | foo.bar == "a" or foo.bar == "b" # Attributes. 26 | foo.bar == "a" or foo.bar == "b" # Attributes.
| |
= help: Merge multiple comparisons
repeated_equality_comparison.py:24:1: PLR1714 Consider merging multiple comparisons: `bar in ("c", "d")`. Use a `set` if the elements are hashable. Suggested fix
21 21 |
22 22 | foo != "a" and "b" != foo and foo != "c"
23 23 |
24 |-foo == "a" or foo == "b" or "c" == bar or "d" == bar # Multiple targets
24 |+foo in ("a", "b") # Multiple targets
25 25 |
26 26 | foo.bar == "a" or foo.bar == "b" # Attributes.
27 27 |
repeated_equality_comparison.py:24:1: PLR1714 [*] Consider merging multiple comparisons: `bar in ("c", "d")`. Use a `set` if the elements are hashable.
| |
22 | foo != "a" and "b" != foo and foo != "c" 22 | foo != "a" and "b" != foo and foo != "c"
23 | 23 |
@ -129,8 +259,19 @@ repeated_equality_comparison.py:24:1: PLR1714 Consider merging multiple comparis
25 | 25 |
26 | foo.bar == "a" or foo.bar == "b" # Attributes. 26 | foo.bar == "a" or foo.bar == "b" # Attributes.
| |
= help: Merge multiple comparisons
repeated_equality_comparison.py:26:1: PLR1714 Consider merging multiple comparisons: `foo.bar in ("a", "b")`. Use a `set` if the elements are hashable. Suggested fix
21 21 |
22 22 | foo != "a" and "b" != foo and foo != "c"
23 23 |
24 |-foo == "a" or foo == "b" or "c" == bar or "d" == bar # Multiple targets
24 |+bar in ("c", "d") # Multiple targets
25 25 |
26 26 | foo.bar == "a" or foo.bar == "b" # Attributes.
27 27 |
repeated_equality_comparison.py:26:1: PLR1714 [*] Consider merging multiple comparisons: `foo.bar in ("a", "b")`. Use a `set` if the elements are hashable.
| |
24 | foo == "a" or foo == "b" or "c" == bar or "d" == bar # Multiple targets 24 | foo == "a" or foo == "b" or "c" == bar or "d" == bar # Multiple targets
25 | 25 |
@ -139,5 +280,16 @@ repeated_equality_comparison.py:26:1: PLR1714 Consider merging multiple comparis
27 | 27 |
28 | # OK 28 | # OK
| |
= help: Merge multiple comparisons
Suggested fix
23 23 |
24 24 | foo == "a" or foo == "b" or "c" == bar or "d" == bar # Multiple targets
25 25 |
26 |-foo.bar == "a" or foo.bar == "b" # Attributes.
26 |+foo.bar in ("a", "b") # Attributes.
27 27 |
28 28 | # OK
29 29 | foo == "a" and foo == "b" and foo == "c" # `and` mixed with `==`.