[pylint] Implement boolean-chained-comparison (R1716)

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

@@ -0,0 +1,120 @@
+# ------------------
+# less than examples
+# ------------------
+
+a = int(input())
+b = int(input())
+c = int(input())
+if a < b and b < c:  # [boolean-chained-comparison]
+    pass
+
+a = int(input())
+b = int(input())
+c = int(input())
+if a < b and b <= c:  # [boolean-chained-comparison]
+    pass
+
+a = int(input())
+b = int(input())
+c = int(input())
+if a <= b and b < c:  # [boolean-chained-comparison]
+    pass
+
+a = int(input())
+b = int(input())
+c = int(input())
+if a <= b and b <= c:  # [boolean-chained-comparison]
+    pass
+
+# ---------------------
+# greater than examples
+# ---------------------
+
+a = int(input())
+b = int(input())
+c = int(input())
+if a > b and b > c:  # [boolean-chained-comparison]
+    pass
+
+a = int(input())
+b = int(input())
+c = int(input())
+if a >= b and b > c:  # [boolean-chained-comparison]
+    pass
+
+a = int(input())
+b = int(input())
+c = int(input())
+if a > b and b >= c:  # [boolean-chained-comparison]
+    pass
+
+a = int(input())
+b = int(input())
+c = int(input())
+if a >= b and b >= c:  # [boolean-chained-comparison]
+    pass
+
+# -----------------------
+# multiple fixes examples
+# -----------------------
+
+a = int(input())
+b = int(input())
+c = int(input())
+d = int(input())
+if a < b and b < c and c < d:  # [boolean-chained-comparison]
+    pass
+
+a = int(input())
+b = int(input())
+c = int(input())
+d = int(input())
+e = int(input())
+if a < b and b < c and c < d and d < e:  # [boolean-chained-comparison]
+    pass
+
+# ------------
+# bad examples
+# ------------
+
+a = int(input())
+b = int(input())
+c = int(input())
+if a > b or b > c:
+    pass
+
+a = int(input())
+b = int(input())
+c = int(input())
+if a > b and b in (1, 2):
+    pass
+
+a = int(input())
+b = int(input())
+c = int(input())
+if a < b and b > c:
+    pass
+
+a = int(input())
+b = int(input())
+c = int(input())
+if a < b and b >= c:
+    pass
+
+a = int(input())
+b = int(input())
+c = int(input())
+if a <= b and b > c:
+    pass
+
+a = int(input())
+b = int(input())
+c = int(input())
+if a <= b and b >= c:
+    pass
+
+a = int(input())
+b = int(input())
+c = int(input())
+if a > b and b < c:
+    pass

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

@@ -1537,6 +1537,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
             }
         }
         Expr::BoolOp(bool_op) => {
+            if checker.enabled(Rule::BooleanChainedComparison) {
+                pylint::rules::boolean_chained_comparison(checker, bool_op);
+            }
             if checker.enabled(Rule::MultipleStartsEndsWith) {
                 flake8_pie::rules::multiple_starts_ends_with(checker, expr);
             }

crates/ruff_linter/src/codes.rs

@@ -257,6 +257,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Pylint, "R1714") => (RuleGroup::Stable, rules::pylint::rules::RepeatedEqualityComparison),
         (Pylint, "R1722") => (RuleGroup::Stable, rules::pylint::rules::SysExitAlias),
         (Pylint, "R1730") => (RuleGroup::Stable, rules::pylint::rules::IfStmtMinMax),
+        (Pylint, "R1716") => (RuleGroup::Preview, rules::pylint::rules::BooleanChainedComparison),
         (Pylint, "R1733") => (RuleGroup::Preview, rules::pylint::rules::UnnecessaryDictIndexLookup),
         (Pylint, "R1736") => (RuleGroup::Stable, rules::pylint::rules::UnnecessaryListIndexLookup),
         (Pylint, "R2004") => (RuleGroup::Stable, rules::pylint::rules::MagicValueComparison),

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

@@ -36,6 +36,10 @@ mod tests {
     #[test_case(Rule::BadStringFormatType, Path::new("bad_string_format_type.py"))]
     #[test_case(Rule::BidirectionalUnicode, Path::new("bidirectional_unicode.py"))]
     #[test_case(Rule::BinaryOpException, Path::new("binary_op_exception.py"))]
+    #[test_case(
+        Rule::BooleanChainedComparison,
+        Path::new("boolean_chained_comparison.py")
+    )]
     #[test_case(Rule::CollapsibleElseIf, Path::new("collapsible_else_if.py"))]
     #[test_case(Rule::CompareToEmptyString, Path::new("compare_to_empty_string.py"))]
     #[test_case(Rule::ComparisonOfConstant, Path::new("comparison_of_constant.py"))]

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

@@ -0,0 +1,120 @@
+use itertools::Itertools;
+use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::{name::Name, BoolOp, CmpOp, Expr, ExprBoolOp, ExprCompare};
+use ruff_text_size::{Ranged, TextRange};
+
+use crate::checkers::ast::Checker;
+
+/// ## What it does
+/// Check for chained boolean operations that can be simplified.
+///
+/// ## Why is this bad?
+/// Refactoring the code will improve readability for these cases.
+///
+/// ## Example
+///
+/// ```python
+/// a = int(input())
+/// b = int(input())
+/// c = int(input())
+/// if a < b and b < c:
+///     pass
+/// ```
+///
+/// Use instead:
+///
+/// ```python
+/// a = int(input())
+/// b = int(input())
+/// c = int(input())
+/// if a < b < c:
+///     pass
+/// ```
+#[violation]
+pub struct BooleanChainedComparison {
+    variable: Name,
+}
+
+impl AlwaysFixableViolation for BooleanChainedComparison {
+    #[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()
+    }
+}
+
+/// PLR1716
+pub(crate) fn boolean_chained_comparison(checker: &mut Checker, expr_bool_op: &ExprBoolOp) {
+    // early exit for non `and` boolean operations
+    if expr_bool_op.op != BoolOp::And {
+        return;
+    }
+
+    // early exit when not all expressions are compare expressions
+    if !expr_bool_op.values.iter().all(Expr::is_compare_expr) {
+        return;
+    }
+
+    // retrieve all compare statements from expression
+    let compare_expressions = expr_bool_op
+        .values
+        .iter()
+        .map(|stmt| stmt.as_compare_expr().unwrap());
+
+    let diagnostics = compare_expressions
+        .tuple_windows()
+        .filter(|(left_compare, right_compare)| {
+            are_compare_expr_simplifiable(left_compare, right_compare)
+        })
+        .filter_map(|(left_compare, right_compare)| {
+            let Expr::Name(left_compare_right) = left_compare.comparators.first()? else {
+                return None;
+            };
+
+            let Expr::Name(right_compare_left) = &*right_compare.left else {
+                return None;
+            };
+
+            if left_compare_right.id() != right_compare_left.id() {
+                return None;
+            }
+
+            let edit = Edit::range_replacement(
+                left_compare_right.id().to_string(),
+                TextRange::new(left_compare_right.start(), right_compare_left.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)),
+            )
+        });
+
+    checker.diagnostics.extend(diagnostics);
+}
+
+/// Checks whether two compare expressions are simplifiable
+fn are_compare_expr_simplifiable(left: &ExprCompare, right: &ExprCompare) -> bool {
+    let [left_operator] = &*left.ops else {
+        return false;
+    };
+
+    let [right_operator] = &*right.ops else {
+        return false;
+    };
+
+    matches!(
+        (left_operator, right_operator),
+        (CmpOp::Lt | CmpOp::LtE, CmpOp::Lt | CmpOp::LtE)
+            | (CmpOp::Gt | CmpOp::GtE, CmpOp::Gt | CmpOp::GtE)
+    )
+}

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

@@ -9,6 +9,7 @@ pub(crate) use bad_string_format_character::BadStringFormatCharacter;
 pub(crate) use bad_string_format_type::*;
 pub(crate) use bidirectional_unicode::*;
 pub(crate) use binary_op_exception::*;
+pub(crate) use boolean_chained_comparison::*;
 pub(crate) use collapsible_else_if::*;
 pub(crate) use compare_to_empty_string::*;
 pub(crate) use comparison_of_constant::*;
@@ -112,6 +113,7 @@ pub(crate) mod bad_string_format_character;
 mod bad_string_format_type;
 mod bidirectional_unicode;
 mod binary_op_exception;
+mod boolean_chained_comparison;
 mod collapsible_else_if;
 mod compare_to_empty_string;
 mod comparison_of_constant;