[pylint] Implement if-stmt-min-max (PLR1730, PLR1731)

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

@@ -0,0 +1,129 @@
+# pylint: disable=missing-docstring, invalid-name, too-few-public-methods, redefined-outer-name
+
+value = 10
+value2 = 0
+value3 = 3
+
+# Positive
+if value < 10:  # [max-instead-of-if]
+    value = 10
+
+if value <= 10:  # [max-instead-of-if]
+    value = 10
+
+if value < value2:  # [max-instead-of-if]
+    value = value2
+
+if value > 10:  # [min-instead-of-if]
+    value = 10
+
+if value >= 10:  # [min-instead-of-if]
+    value = 10
+
+if value > value2:  # [min-instead-of-if]
+    value = value2
+
+
+class A:
+    def __init__(self):
+        self.value = 13
+
+
+A1 = A()
+if A1.value < 10:  # [max-instead-of-if]
+    A1.value = 10
+
+if A1.value > 10:  # [min-instead-of-if]
+    A1.value = 10
+
+
+class AA:
+    def __init__(self, value):
+        self.value = value
+
+    def __gt__(self, b):
+        return self.value > b
+
+    def __ge__(self, b):
+        return self.value >= b
+
+    def __lt__(self, b):
+        return self.value < b
+
+    def __le__(self, b):
+        return self.value <= b
+
+
+A1 = AA(0)
+A2 = AA(3)
+
+if A2 < A1:  # [max-instead-of-if]
+    A2 = A1
+
+if A2 <= A1:  # [max-instead-of-if]
+    A2 = A1
+
+if A2 > A1:  # [min-instead-of-if]
+    A2 = A1
+
+if A2 >= A1:  # [min-instead-of-if]
+    A2 = A1
+
+# Negative
+if value < 10:
+    value = 2
+
+if value <= 3:
+    value = 5
+
+if value < 10:
+    value = 2
+    value2 = 3
+
+if value < value2:
+    value = value3
+
+if value < 5:
+    value = value3
+
+if 2 < value <= 3:
+    value = 1
+
+if value < 10:
+    value = 10
+else:
+    value = 3
+
+if value <= 3:
+    value = 5
+elif value == 3:
+    value = 2
+
+if value > 10:
+    value = 2
+
+if value >= 3:
+    value = 5
+
+if value > 10:
+    value = 2
+    value2 = 3
+
+if value > value2:
+    value = value3
+
+if value > 5:
+    value = value3
+
+if 2 > value >= 3:
+    value = 1
+
+if value > 10:
+    value = 10
+else:
+    value = 3
+
+if value >= 3:
+    value = 5
+elif value == 3:
+    value = 2

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

@@ -1117,6 +1117,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
             if checker.enabled(Rule::TooManyBooleanExpressions) {
                 pylint::rules::too_many_boolean_expressions(checker, if_);
             }
+            if checker.enabled(Rule::MinMaxInsteadOfIf) {
+                pylint::rules::min_max_instead_of_if(checker, if_);
+            }
             if checker.source_type.is_stub() {
                 if checker.any_enabled(&[
                     Rule::UnrecognizedVersionInfoCheck,

crates/ruff_linter/src/codes.rs

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

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

@@ -47,6 +47,7 @@ mod tests {
     #[test_case(Rule::EqWithoutHash, Path::new("eq_without_hash.py"))]
     #[test_case(Rule::EmptyComment, Path::new("empty_comment.py"))]
     #[test_case(Rule::ManualFromImport, Path::new("import_aliasing.py"))]
+    #[test_case(Rule::MinMaxInsteadOfIf, Path::new("min_max_instead_of_if.py"))]
     #[test_case(Rule::SingleStringSlots, Path::new("single_string_slots.py"))]
     #[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_0.py"))]
     #[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_1.py"))]

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

@@ -0,0 +1,142 @@
+use std::ops::Deref;
+
+use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::comparable::ComparableExpr;
+use ruff_python_ast::{self as ast, Arguments, CmpOp, ExprContext, Stmt};
+use ruff_text_size::{Ranged, TextRange};
+
+use crate::checkers::ast::Checker;
+
+/// ## What it does
+/// Check for an if node that can be refactored as a min/max python builtin.
+///
+/// ## Why is this bad?
+/// An if block where the test and assignment have the same structure can
+/// be expressed more concisely by using the python builtin min/max function.
+///
+/// ## Example
+/// ```python
+/// if value < 10:
+///     value = 10
+/// ```
+///
+/// Use instead:
+/// ```python
+/// value = max(value, 10)
+/// ```
+///
+/// ## References
+/// - [Python documentation: max function](https://docs.python.org/3/library/functions.html#max)
+/// - [Python documentation: min function](https://docs.python.org/3/library/functions.html#min)
+#[violation]
+pub struct MinMaxInsteadOfIf {
+    contents: String,
+}
+
+impl Violation for MinMaxInsteadOfIf {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        let MinMaxInsteadOfIf { contents } = self;
+        format!("Consider using `{contents}` instead of unnecessary if block")
+    }
+}
+
+/// R1730 (and also R1731)
+pub(crate) fn min_max_instead_of_if(checker: &mut Checker, stmt_if: &ast::StmtIf) {
+    let ast::StmtIf {
+        test,
+        body,
+        elif_else_clauses,
+        range: _,
+    } = stmt_if;
+
+    if !elif_else_clauses.is_empty() {
+        return;
+    }
+
+    let [Stmt::Assign(ast::StmtAssign {
+        targets: body_targets,
+        value: body_value,
+        ..
+    })] = body.as_slice()
+    else {
+        return;
+    };
+    let [body_target] = body_targets.as_slice() else {
+        return;
+    };
+
+    let Some(ast::ExprCompare {
+        ops,
+        left,
+        comparators,
+        ..
+    }) = test.as_compare_expr()
+    else {
+        return;
+    };
+
+    if !(!body_target.is_subscript_expr() && !left.is_subscript_expr()) {
+        return;
+    }
+
+    let ([op], [right_statement]) = (&**ops, &**comparators) else {
+        return;
+    };
+
+    let min_or_max = match op {
+        CmpOp::Gt | CmpOp::GtE => MinMax::Min,
+        CmpOp::Lt | CmpOp::LtE => MinMax::Max,
+        _ => return,
+    };
+
+    let left_cmp = ComparableExpr::from(left);
+    let body_target_cmp = ComparableExpr::from(body_target);
+    let right_statement_cmp = ComparableExpr::from(right_statement);
+    let body_value_cmp = ComparableExpr::from(body_value);
+    if left_cmp != body_target_cmp || right_statement_cmp != body_value_cmp {
+        return;
+    }
+
+    let func_node = ast::ExprName {
+        id: min_or_max.as_str().into(),
+        ctx: ExprContext::Load,
+        range: TextRange::default(),
+    };
+    let value_node = ast::ExprCall {
+        func: Box::new(func_node.into()),
+        arguments: Arguments {
+            args: Box::from([body_target.clone(), body_value.deref().clone()]),
+            keywords: Box::from([]),
+            range: TextRange::default(),
+        },
+        range: TextRange::default(),
+    };
+    let assign_node = ast::StmtAssign {
+        targets: vec![body_target.clone()],
+        value: Box::new(value_node.into()),
+        range: TextRange::default(),
+    };
+    let diagnostic = Diagnostic::new(
+        MinMaxInsteadOfIf {
+            contents: checker.generator().stmt(&assign_node.into()),
+        },
+        stmt_if.range(),
+    );
+    checker.diagnostics.push(diagnostic);
+}
+
+enum MinMax {
+    Min,
+    Max,
+}
+
+impl MinMax {
+    fn as_str(&self) -> &'static str {
+        match self {
+            Self::Min => "min",
+            Self::Max => "max",
+        }
+    }
+}

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

@@ -37,6 +37,7 @@ pub(crate) use load_before_global_declaration::*;
 pub(crate) use logging::*;
 pub(crate) use magic_value_comparison::*;
 pub(crate) use manual_import_from::*;
+pub(crate) use min_max_instead_of_if::*;
 pub(crate) use misplaced_bare_raise::*;
 pub(crate) use modified_iterating_set::*;
 pub(crate) use named_expr_without_context::*;
@@ -132,6 +133,7 @@ mod load_before_global_declaration;
 mod logging;
 mod magic_value_comparison;
 mod manual_import_from;
+mod min_max_instead_of_if;
 mod misplaced_bare_raise;
 mod modified_iterating_set;
 mod named_expr_without_context;