Skip to content

Commit

Permalink
Confine repeated-equality-comparison-target to names and attributes (#…
Browse files Browse the repository at this point in the history
…6802)

Empirically, Pylint does this, so seems reasonable to follow.
  • Loading branch information
charliermarsh authored Aug 23, 2023
1 parent 1cb1bd7 commit 5f5de52
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@

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

foo.bar == "a" or foo.bar == "b" # Attributes.

# OK
foo == "a" and foo == "b" and foo == "c" # `and` mixed with `==`.

Expand All @@ -45,3 +47,7 @@
foo == bar == "b" or foo == "c" # Multiple comparisons.

foo == foo or foo == bar # Self-comparison.

foo[0] == "a" or foo[0] == "b" # Subscripts.

foo() == "a" or foo() == "b" # Calls.
Original file line number Diff line number Diff line change
Expand Up @@ -96,17 +96,21 @@ pub(crate) fn repeated_equality_comparison_target(
return;
};

let (left_count, left_matches) = value_to_comparators
.entry(left.deref().into())
.or_insert_with(|| (0, Vec::new()));
*left_count += 1;
left_matches.push(right);

let (right_count, right_matches) = value_to_comparators
.entry(right.into())
.or_insert_with(|| (0, Vec::new()));
*right_count += 1;
right_matches.push(left);
if matches!(left.as_ref(), Expr::Name(_) | Expr::Attribute(_)) {
let (left_count, left_matches) = value_to_comparators
.entry(left.deref().into())
.or_insert_with(|| (0, Vec::new()));
*left_count += 1;
left_matches.push(right);
}

if matches!(right, Expr::Name(_) | Expr::Attribute(_)) {
let (right_count, right_matches) = value_to_comparators
.entry(right.into())
.or_insert_with(|| (0, Vec::new()));
*right_count += 1;
right_matches.push(left);
}
}

for (value, (count, comparators)) in value_to_comparators {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ repeated_equality_comparison_target.py:24:1: PLR1714 Consider merging multiple c
24 | foo == "a" or foo == "b" or "c" == bar or "d" == bar # Multiple targets
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1714
25 |
26 | # OK
26 | foo.bar == "a" or foo.bar == "b" # Attributes.
|

repeated_equality_comparison_target.py:24:1: PLR1714 Consider merging multiple comparisons: `bar in ("c", "d")`. Use a `set` if the elements are hashable.
Expand All @@ -127,7 +127,17 @@ repeated_equality_comparison_target.py:24:1: PLR1714 Consider merging multiple c
24 | foo == "a" or foo == "b" or "c" == bar or "d" == bar # Multiple targets
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1714
25 |
26 | # OK
26 | foo.bar == "a" or foo.bar == "b" # Attributes.
|

repeated_equality_comparison_target.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
25 |
26 | foo.bar == "a" or foo.bar == "b" # Attributes.
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLR1714
27 |
28 | # OK
|


0 comments on commit 5f5de52

Please sign in to comment.