Do not offer an invalid fix for PLR1716 when the comparisons contain parenthesis

crates/ruff_linter/resources/test/fixtures/pylint/boolean_chained_comparison.py

@@ -118,3 +118,11 @@
 c = int(input())
 if a > b and b < c:
     pass
+
+
+# Unfixable due to parentheses.
+(a < b) and b < c
+a < b and (b < c)
+((a < b) and b < c)
+(a < b) and (b < c)
+(((a < b))) and (b < c)

crates/ruff_linter/src/rules/pylint/rules/boolean_chained_comparison.rs

@@ -1,7 +1,9 @@
 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_python_ast::{
+    name::Name, parenthesize::parenthesized_range, BoolOp, CmpOp, Expr, ExprBoolOp, ExprCompare,
+};
 use ruff_text_size::{Ranged, TextRange};
 
 use crate::checkers::ast::Checker;
@@ -36,14 +38,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<String> {
+        Some("Use a single compare expression".to_string())
     }
 }
 
@@ -59,6 +63,9 @@ pub(crate) fn boolean_chained_comparison(checker: &mut Checker, expr_bool_op: &E
         return;
     }
 
+    let locator = checker.locator();
+    let comment_ranges = checker.comment_ranges();
+
     // retrieve all compare statements from expression
     let compare_expressions = expr_bool_op
         .values
@@ -83,20 +90,47 @@ 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 left_has_paren = parenthesized_range(
+                left_compare.into(),
+                expr_bool_op.into(),
+                comment_ranges,
+                locator.contents(),
+            )
+            .is_some();
 
-            Some(
-                Diagnostic::new(
-                    BooleanChainedComparison {
-                        variable: left_compare_right.id().clone(),
-                    },
-                    TextRange::new(left_compare.start(), right_compare.end()),
-                )
-                .with_fix(Fix::safe_edit(edit)),
+            let right_has_paren = parenthesized_range(
+                right_compare.into(),
+                expr_bool_op.into(),
+                comment_ranges,
+                locator.contents(),
             )
+            .is_some();
+
+            // Do not offer a fix if there are any parentheses
+            // TODO: We can support a fix here, we just need to be careful to balance the
+            // parentheses which requires a more sophisticated edit
+            let fix = if left_has_paren || right_has_paren {
+                None
+            } else {
+                let edit = Edit::range_replacement(
+                    left_compare_right.id().to_string(),
+                    TextRange::new(left_compare_right.start(), right_compare_left.end()),
+                );
+                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()),
+            );
+
+            if let Some(fix) = fix {
+                diagnostic.set_fix(fix);
+            }
+
+            Some(diagnostic)
         });
 
     checker.diagnostics.extend(diagnostics);

crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1716_boolean_chained_comparison.py.snap

@@ -260,3 +260,54 @@ 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 parentheses.
+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:1: PLR1716 Contains chained boolean comparison that can be simplified
+    |
+123 | # Unfixable due to parentheses.
+124 | (a < b) and b < c
+125 | a < b and (b < c)
+    | ^^^^^^^^^^^^^^^^ PLR1716
+126 | ((a < b) and b < c)
+127 | (a < b) and (b < c)
+    |
+    = help: Use a single compare expression
+
+boolean_chained_comparison.py:126:3: 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
+127 | (a < b) and (b < c)
+128 | (((a < b))) and (b < c)
+    |
+    = help: Use a single compare expression
+
+boolean_chained_comparison.py:127:2: PLR1716 Contains chained boolean comparison that can be simplified
+    |
+125 | a < b and (b < c)
+126 | ((a < b) and b < c)
+127 | (a < b) and (b < c)
+    |  ^^^^^^^^^^^^^^^^^ PLR1716
+128 | (((a < b))) and (b < c)
+    |
+    = help: Use a single compare expression
+
+boolean_chained_comparison.py:128:4: PLR1716 Contains chained boolean comparison that can be simplified
+    |
+126 | ((a < b) and b < c)
+127 | (a < b) and (b < c)
+128 | (((a < b))) and (b < c)
+    |    ^^^^^^^^^^^^^^^^^^^ PLR1716
+    |
+    = help: Use a single compare expression