From fbff7db5eadfca6da4682e6bf7e7c181621360ae Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 22 Feb 2023 17:33:59 -0500 Subject: [PATCH] Avoid suggesting 'is' for constant literals --- .../fixtures/pycodestyle/constant_literals.py | 14 +-- .../pycodestyle/rules/literal_comparisons.rs | 110 +++++++++--------- ...pycodestyle__tests__constant_literals.snap | 44 ------- 3 files changed, 58 insertions(+), 110 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/pycodestyle/constant_literals.py b/crates/ruff/resources/test/fixtures/pycodestyle/constant_literals.py index 5f3e7222772c4..4c8dc7d10b856 100644 --- a/crates/ruff/resources/test/fixtures/pycodestyle/constant_literals.py +++ b/crates/ruff/resources/test/fixtures/pycodestyle/constant_literals.py @@ -17,20 +17,16 @@ pass ### -# Unfixable errors +# Non-errors ### -if "abc" == None: # E711 +if "abc" == None: pass -if None == "abc": # E711 +if None == "abc": pass -if "abc" == False: # E712 +if "abc" == False: pass -if False == "abc": # E712 +if False == "abc": pass - -### -# Non-errors -### if "def" == "abc": pass if False is None: diff --git a/crates/ruff/src/rules/pycodestyle/rules/literal_comparisons.rs b/crates/ruff/src/rules/pycodestyle/rules/literal_comparisons.rs index 3a64f4e6813ec..36d46ae124785 100644 --- a/crates/ruff/src/rules/pycodestyle/rules/literal_comparisons.rs +++ b/crates/ruff/src/rules/pycodestyle/rules/literal_comparisons.rs @@ -102,68 +102,72 @@ pub fn literal_comparisons( // Check `left`. let mut comparator = left; let next = &comparators[0]; - if check_none_comparisons - && matches!( - comparator.node, - ExprKind::Constant { - value: Constant::None, - kind: None - } - ) - { - if matches!(op, Cmpop::Eq) { - let diagnostic = - Diagnostic::new(NoneComparison(op.into()), Range::from_located(comparator)); - if checker.patch(diagnostic.kind.rule()) && !helpers::is_constant_non_singleton(next) { - bad_ops.insert(0, Cmpop::Is); - } - diagnostics.push(diagnostic); - } - if matches!(op, Cmpop::NotEq) { - let diagnostic = - Diagnostic::new(NoneComparison(op.into()), Range::from_located(comparator)); - if checker.patch(diagnostic.kind.rule()) && !helpers::is_constant_non_singleton(next) { - bad_ops.insert(0, Cmpop::IsNot); - } - diagnostics.push(diagnostic); - } - } - if check_true_false_comparisons { - if let ExprKind::Constant { - value: Constant::Bool(value), - kind: None, - } = comparator.node + if !helpers::is_constant_non_singleton(next) { + if check_none_comparisons + && matches!( + comparator.node, + ExprKind::Constant { + value: Constant::None, + kind: None + } + ) { if matches!(op, Cmpop::Eq) { - let diagnostic = Diagnostic::new( - TrueFalseComparison(value, op.into()), - Range::from_located(comparator), - ); - if checker.patch(diagnostic.kind.rule()) - && !helpers::is_constant_non_singleton(next) - { + let diagnostic = + Diagnostic::new(NoneComparison(op.into()), Range::from_located(comparator)); + if checker.patch(diagnostic.kind.rule()) { bad_ops.insert(0, Cmpop::Is); } diagnostics.push(diagnostic); } if matches!(op, Cmpop::NotEq) { - let diagnostic = Diagnostic::new( - TrueFalseComparison(value, op.into()), - Range::from_located(comparator), - ); - if checker.patch(diagnostic.kind.rule()) - && !helpers::is_constant_non_singleton(next) - { + let diagnostic = + Diagnostic::new(NoneComparison(op.into()), Range::from_located(comparator)); + if checker.patch(diagnostic.kind.rule()) { bad_ops.insert(0, Cmpop::IsNot); } diagnostics.push(diagnostic); } } + + if check_true_false_comparisons { + if let ExprKind::Constant { + value: Constant::Bool(value), + kind: None, + } = comparator.node + { + if matches!(op, Cmpop::Eq) { + let diagnostic = Diagnostic::new( + TrueFalseComparison(value, op.into()), + Range::from_located(comparator), + ); + if checker.patch(diagnostic.kind.rule()) { + bad_ops.insert(0, Cmpop::Is); + } + diagnostics.push(diagnostic); + } + if matches!(op, Cmpop::NotEq) { + let diagnostic = Diagnostic::new( + TrueFalseComparison(value, op.into()), + Range::from_located(comparator), + ); + if checker.patch(diagnostic.kind.rule()) { + bad_ops.insert(0, Cmpop::IsNot); + } + diagnostics.push(diagnostic); + } + } + } } // Check each comparator in order. for (idx, (op, next)) in izip!(ops, comparators).enumerate() { + if helpers::is_constant_non_singleton(comparator) { + comparator = next; + continue; + } + if check_none_comparisons && matches!( next.node, @@ -176,9 +180,7 @@ pub fn literal_comparisons( if matches!(op, Cmpop::Eq) { let diagnostic = Diagnostic::new(NoneComparison(op.into()), Range::from_located(next)); - if checker.patch(diagnostic.kind.rule()) - && !helpers::is_constant_non_singleton(comparator) - { + if checker.patch(diagnostic.kind.rule()) { bad_ops.insert(idx, Cmpop::Is); } diagnostics.push(diagnostic); @@ -186,9 +188,7 @@ pub fn literal_comparisons( if matches!(op, Cmpop::NotEq) { let diagnostic = Diagnostic::new(NoneComparison(op.into()), Range::from_located(next)); - if checker.patch(diagnostic.kind.rule()) - && !helpers::is_constant_non_singleton(comparator) - { + if checker.patch(diagnostic.kind.rule()) { bad_ops.insert(idx, Cmpop::IsNot); } diagnostics.push(diagnostic); @@ -206,9 +206,7 @@ pub fn literal_comparisons( TrueFalseComparison(value, op.into()), Range::from_located(next), ); - if checker.patch(diagnostic.kind.rule()) - && !helpers::is_constant_non_singleton(comparator) - { + if checker.patch(diagnostic.kind.rule()) { bad_ops.insert(idx, Cmpop::Is); } diagnostics.push(diagnostic); @@ -218,9 +216,7 @@ pub fn literal_comparisons( TrueFalseComparison(value, op.into()), Range::from_located(next), ); - if checker.patch(diagnostic.kind.rule()) - && !helpers::is_constant_non_singleton(comparator) - { + if checker.patch(diagnostic.kind.rule()) { bad_ops.insert(idx, Cmpop::IsNot); } diagnostics.push(diagnostic); diff --git a/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__constant_literals.snap b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__constant_literals.snap index 29ba88e251fdc..fe0e3cea6eb5a 100644 --- a/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__constant_literals.snap +++ b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__constant_literals.snap @@ -164,48 +164,4 @@ expression: diagnostics row: 16 column: 16 parent: ~ -- kind: - NoneComparison: Eq - location: - row: 22 - column: 12 - end_location: - row: 22 - column: 16 - fix: ~ - parent: ~ -- kind: - NoneComparison: Eq - location: - row: 24 - column: 3 - end_location: - row: 24 - column: 7 - fix: ~ - parent: ~ -- kind: - TrueFalseComparison: - - false - - Eq - location: - row: 26 - column: 12 - end_location: - row: 26 - column: 17 - fix: ~ - parent: ~ -- kind: - TrueFalseComparison: - - false - - Eq - location: - row: 28 - column: 3 - end_location: - row: 28 - column: 8 - fix: ~ - parent: ~