From 5f5de52abad75ab535314e537531754a76a2d5b4 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 22 Aug 2023 23:56:37 -0400 Subject: [PATCH] Confine repeated-equality-comparison-target to names and attributes (#6802) Empirically, Pylint does this, so seems reasonable to follow. --- .../repeated_equality_comparison_target.py | 6 +++++ .../repeated_equality_comparison_target.rs | 26 +++++++++++-------- ...epeated_equality_comparison_target.py.snap | 14 ++++++++-- 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/pylint/repeated_equality_comparison_target.py b/crates/ruff/resources/test/fixtures/pylint/repeated_equality_comparison_target.py index 747e0b53312e1..6c3f694ba87cd 100644 --- a/crates/ruff/resources/test/fixtures/pylint/repeated_equality_comparison_target.py +++ b/crates/ruff/resources/test/fixtures/pylint/repeated_equality_comparison_target.py @@ -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 `==`. @@ -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. diff --git a/crates/ruff/src/rules/pylint/rules/repeated_equality_comparison_target.rs b/crates/ruff/src/rules/pylint/rules/repeated_equality_comparison_target.rs index cdb0ebe83fdfc..a426c8e1ca7ba 100644 --- a/crates/ruff/src/rules/pylint/rules/repeated_equality_comparison_target.rs +++ b/crates/ruff/src/rules/pylint/rules/repeated_equality_comparison_target.rs @@ -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 { diff --git a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR1714_repeated_equality_comparison_target.py.snap b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR1714_repeated_equality_comparison_target.py.snap index 5c5f1bb57ea35..40373cea07ca5 100644 --- a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR1714_repeated_equality_comparison_target.py.snap +++ b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR1714_repeated_equality_comparison_target.py.snap @@ -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. @@ -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 |