From bae4fd93a67ea282905dd7569f31541b5709ba6f Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Thu, 26 Sep 2024 10:00:54 -0500 Subject: [PATCH] Do not offer an invalid fix for PLR1716 where the comparison contains parenthesis --- .../pylint/boolean_chained_comparison.py | 6 +++ .../rules/boolean_chained_comparison.rs | 46 ++++++++++++------- ...PLR1716_boolean_chained_comparison.py.snap | 29 ++++++++++++ 3 files changed, 65 insertions(+), 16 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/boolean_chained_comparison.py b/crates/ruff_linter/resources/test/fixtures/pylint/boolean_chained_comparison.py index 90c87fe3d446f5..60da01bb38bc3f 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/boolean_chained_comparison.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/boolean_chained_comparison.py @@ -118,3 +118,9 @@ c = int(input()) if a > b and b < c: pass + + +# Unfixable due to parenthesis +(a < b) and b < c +((a < b) and b < c) +(a < b) and (b < c) diff --git a/crates/ruff_linter/src/rules/pylint/rules/boolean_chained_comparison.rs b/crates/ruff_linter/src/rules/pylint/rules/boolean_chained_comparison.rs index da8e8ef132b7b8..a539fa47f95962 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/boolean_chained_comparison.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/boolean_chained_comparison.rs @@ -1,5 +1,5 @@ use itertools::Itertools; -use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; +use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::{name::Name, BoolOp, CmpOp, Expr, ExprBoolOp, ExprCompare}; use ruff_text_size::{Ranged, TextRange}; @@ -36,14 +36,16 @@ pub struct BooleanChainedComparison { variable: Name, } -impl AlwaysFixableViolation for BooleanChainedComparison { +impl Violation for BooleanChainedComparison { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + #[derive_message_formats] fn message(&self) -> String { format!("Contains chained boolean comparison that can be simplified") } - fn fix_title(&self) -> String { - "Use a single compare expression".to_string() + fn fix_title(&self) -> Option { + Some("Use a single compare expression".to_string()) } } @@ -59,6 +61,8 @@ pub(crate) fn boolean_chained_comparison(checker: &mut Checker, expr_bool_op: &E return; } + let locator = checker.locator(); + // retrieve all compare statements from expression let compare_expressions = expr_bool_op .values @@ -83,20 +87,30 @@ pub(crate) fn boolean_chained_comparison(checker: &mut Checker, expr_bool_op: &E return None; } - let edit = Edit::range_replacement( - left_compare_right.id().to_string(), - TextRange::new(left_compare_right.start(), right_compare_left.end()), + let replace_range = + TextRange::new(left_compare_right.start(), right_compare_left.end()); + + // It's unsafe to apply the fix if the range contains a parenthesis + let fix = if locator.slice(replace_range).contains(')') { + None + } else { + let edit = + Edit::range_replacement(left_compare_right.id().to_string(), replace_range); + Some(Fix::safe_edit(edit)) + }; + + let mut diagnostic = Diagnostic::new( + BooleanChainedComparison { + variable: left_compare_right.id().clone(), + }, + TextRange::new(left_compare.start(), right_compare.end()), ); - Some( - Diagnostic::new( - BooleanChainedComparison { - variable: left_compare_right.id().clone(), - }, - TextRange::new(left_compare.start(), right_compare.end()), - ) - .with_fix(Fix::safe_edit(edit)), - ) + if let Some(fix) = fix { + diagnostic.set_fix(fix); + } + + Some(diagnostic) }); checker.diagnostics.extend(diagnostics); diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1716_boolean_chained_comparison.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1716_boolean_chained_comparison.py.snap index cf45c0ae9c4ebe..a0ffbc99e9601f 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1716_boolean_chained_comparison.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1716_boolean_chained_comparison.py.snap @@ -260,3 +260,32 @@ boolean_chained_comparison.py:73:24: PLR1716 [*] Contains chained boolean compar 74 74 | pass 75 75 | 76 76 | # ------------ + +boolean_chained_comparison.py:124:2: PLR1716 Contains chained boolean comparison that can be simplified + | +123 | # Unfixable due to parenthesis +124 | (a < b) and b < c + | ^^^^^^^^^^^^^^^^ PLR1716 +125 | ((a < b) and b < c) +126 | (a < b) and (b < c) + | + = help: Use a single compare expression + +boolean_chained_comparison.py:125:3: PLR1716 Contains chained boolean comparison that can be simplified + | +123 | # Unfixable due to parenthesis +124 | (a < b) and b < c +125 | ((a < b) and b < c) + | ^^^^^^^^^^^^^^^^ PLR1716 +126 | (a < b) and (b < c) + | + = help: Use a single compare expression + +boolean_chained_comparison.py:126:2: PLR1716 Contains chained boolean comparison that can be simplified + | +124 | (a < b) and b < c +125 | ((a < b) and b < c) +126 | (a < b) and (b < c) + | ^^^^^^^^^^^^^^^^^ PLR1716 + | + = help: Use a single compare expression