Skip to content

Commit

Permalink
Allow repeated-equality-comparison for mixed operations
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jul 17, 2024
1 parent 420f885 commit 72bca7f
Show file tree
Hide file tree
Showing 3 changed files with 119 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,7 @@
foo == "a" or ("c" == bar or "d" == bar) or foo == "b" # Multiple targets

foo == "a" or foo == "b" or "c" != bar and "d" != bar # Multiple targets

foo == "a" or ("c" != bar and "d" != bar) or foo == "b" # Multiple targets

foo == "a" and "c" != bar or foo == "b" and "d" != bar # Multiple targets
Original file line number Diff line number Diff line change
Expand Up @@ -72,30 +72,26 @@ impl AlwaysFixableViolation for RepeatedEqualityComparison {

/// PLR1714
pub(crate) fn repeated_equality_comparison(checker: &mut Checker, bool_op: &ast::ExprBoolOp) {
if bool_op
.values
.iter()
.any(|value| !is_allowed_value(bool_op.op, value, checker.semantic()))
{
return;
}

// Map from expression hash to (starting offset, number of comparisons, list
let mut value_to_comparators: FxHashMap<HashableExpr, (TextSize, Vec<&Expr>, Vec<&Expr>)> =
FxHashMap::with_capacity_and_hasher(bool_op.values.len() * 2, FxBuildHasher);

for value in &bool_op.values {
if !is_allowed_value(bool_op.op, value, checker.semantic()) {
continue;
}

// Enforced via `is_allowed_value`.
let Expr::Compare(ast::ExprCompare {
left, comparators, ..
}) = value
else {
return;
continue;
};

// Enforced via `is_allowed_value`.
let [right] = &**comparators else {
return;
continue;
};

if matches!(left.as_ref(), Expr::Name(_) | Expr::Attribute(_)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,27 @@ repeated_equality_comparison.py:26:1: PLR1714 [*] Consider merging multiple comp
28 28 | # OK
29 29 | foo == "a" and foo == "b" and foo == "c" # `and` mixed with `==`.

repeated_equality_comparison.py:33:1: PLR1714 [*] Consider merging multiple comparisons: `foo in (a, c)`. Use a `set` if the elements are hashable.
|
31 | foo != "a" or foo != "b" or foo != "c" # `or` mixed with `!=`.
32 |
33 | foo == a or foo == b() or foo == c # Call expression.
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1714
34 |
35 | foo != a or foo() != b or foo != c # Call expression.
|
= help: Merge multiple comparisons

Unsafe fix
30 30 |
31 31 | foo != "a" or foo != "b" or foo != "c" # `or` mixed with `!=`.
32 32 |
33 |-foo == a or foo == b() or foo == c # Call expression.
33 |+foo in (a, c) or foo == b() # Call expression.
34 34 |
35 35 | foo != a or foo() != b or foo != c # Call expression.
36 36 |

repeated_equality_comparison.py:59:1: PLR1714 [*] Consider merging multiple comparisons: `foo in ("a", "b")`. Use a `set` if the elements are hashable.
|
57 | sys.platform == "win32" or sys.platform == "emscripten" # sys attributes
Expand Down Expand Up @@ -334,6 +355,27 @@ repeated_equality_comparison.py:59:1: PLR1714 [*] Consider merging multiple comp
61 61 | foo == "a" or ("c" == bar or "d" == bar) or foo == "b" # Multiple targets
62 62 |

repeated_equality_comparison.py:61:1: PLR1714 [*] Consider merging multiple comparisons: `foo in ("a", "b")`. Use a `set` if the elements are hashable.
|
59 | foo == "a" or "c" == bar or foo == "b" or "d" == bar # Multiple targets
60 |
61 | foo == "a" or ("c" == bar or "d" == bar) or foo == "b" # Multiple targets
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1714
62 |
63 | foo == "a" or foo == "b" or "c" != bar and "d" != bar # Multiple targets
|
= help: Merge multiple comparisons

Unsafe fix
58 58 |
59 59 | foo == "a" or "c" == bar or foo == "b" or "d" == bar # Multiple targets
60 60 |
61 |-foo == "a" or ("c" == bar or "d" == bar) or foo == "b" # Multiple targets
61 |+foo in ("a", "b") or ("c" == bar or "d" == bar) # Multiple targets
62 62 |
63 63 | foo == "a" or foo == "b" or "c" != bar and "d" != bar # Multiple targets
64 64 |

repeated_equality_comparison.py:61:16: PLR1714 [*] Consider merging multiple comparisons: `bar in ("c", "d")`. Use a `set` if the elements are hashable.
|
59 | foo == "a" or "c" == bar or foo == "b" or "d" == bar # Multiple targets
Expand All @@ -353,13 +395,37 @@ repeated_equality_comparison.py:61:16: PLR1714 [*] Consider merging multiple com
61 |+foo == "a" or (bar in ("c", "d")) or foo == "b" # Multiple targets
62 62 |
63 63 | foo == "a" or foo == "b" or "c" != bar and "d" != bar # Multiple targets
64 64 |

repeated_equality_comparison.py:63:1: PLR1714 [*] Consider merging multiple comparisons: `foo in ("a", "b")`. Use a `set` if the elements are hashable.
|
61 | foo == "a" or ("c" == bar or "d" == bar) or foo == "b" # Multiple targets
62 |
63 | foo == "a" or foo == "b" or "c" != bar and "d" != bar # Multiple targets
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1714
64 |
65 | foo == "a" or ("c" != bar and "d" != bar) or foo == "b" # Multiple targets
|
= help: Merge multiple comparisons

Unsafe fix
60 60 |
61 61 | foo == "a" or ("c" == bar or "d" == bar) or foo == "b" # Multiple targets
62 62 |
63 |-foo == "a" or foo == "b" or "c" != bar and "d" != bar # Multiple targets
63 |+foo in ("a", "b") or "c" != bar and "d" != bar # Multiple targets
64 64 |
65 65 | foo == "a" or ("c" != bar and "d" != bar) or foo == "b" # Multiple targets
66 66 |

repeated_equality_comparison.py:63:29: PLR1714 [*] Consider merging multiple comparisons: `bar not in ("c", "d")`. Use a `set` if the elements are hashable.
|
61 | foo == "a" or ("c" == bar or "d" == bar) or foo == "b" # Multiple targets
62 |
63 | foo == "a" or foo == "b" or "c" != bar and "d" != bar # Multiple targets
| ^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1714
64 |
65 | foo == "a" or ("c" != bar and "d" != bar) or foo == "b" # Multiple targets
|
= help: Merge multiple comparisons

Expand All @@ -369,3 +435,46 @@ repeated_equality_comparison.py:63:29: PLR1714 [*] Consider merging multiple com
62 62 |
63 |-foo == "a" or foo == "b" or "c" != bar and "d" != bar # Multiple targets
63 |+foo == "a" or foo == "b" or bar not in ("c", "d") # Multiple targets
64 64 |
65 65 | foo == "a" or ("c" != bar and "d" != bar) or foo == "b" # Multiple targets
66 66 |

repeated_equality_comparison.py:65:1: PLR1714 [*] Consider merging multiple comparisons: `foo in ("a", "b")`. Use a `set` if the elements are hashable.
|
63 | foo == "a" or foo == "b" or "c" != bar and "d" != bar # Multiple targets
64 |
65 | foo == "a" or ("c" != bar and "d" != bar) or foo == "b" # Multiple targets
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1714
66 |
67 | foo == "a" and "c" != bar or foo == "b" and "d" != bar # Multiple targets
|
= help: Merge multiple comparisons

Unsafe fix
62 62 |
63 63 | foo == "a" or foo == "b" or "c" != bar and "d" != bar # Multiple targets
64 64 |
65 |-foo == "a" or ("c" != bar and "d" != bar) or foo == "b" # Multiple targets
65 |+foo in ("a", "b") or "c" != bar and "d" != bar # Multiple targets
66 66 |
67 67 | foo == "a" and "c" != bar or foo == "b" and "d" != bar # Multiple targets

repeated_equality_comparison.py:65:16: PLR1714 [*] Consider merging multiple comparisons: `bar not in ("c", "d")`. Use a `set` if the elements are hashable.
|
63 | foo == "a" or foo == "b" or "c" != bar and "d" != bar # Multiple targets
64 |
65 | foo == "a" or ("c" != bar and "d" != bar) or foo == "b" # Multiple targets
| ^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1714
66 |
67 | foo == "a" and "c" != bar or foo == "b" and "d" != bar # Multiple targets
|
= help: Merge multiple comparisons

Unsafe fix
62 62 |
63 63 | foo == "a" or foo == "b" or "c" != bar and "d" != bar # Multiple targets
64 64 |
65 |-foo == "a" or ("c" != bar and "d" != bar) or foo == "b" # Multiple targets
65 |+foo == "a" or (bar not in ("c", "d")) or foo == "b" # Multiple targets
66 66 |
67 67 | foo == "a" and "c" != bar or foo == "b" and "d" != bar # Multiple targets

0 comments on commit 72bca7f

Please sign in to comment.