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

Add rule [consider-using-min-builtin
(R1730)](https://pylint.readthedocs.io/en/latest/user_guide/messages/refactor/consider-using-min-builtin.html)
and [consider-using-max-builtin
(R1731)](https://pylint.readthedocs.io/en/latest/user_guide/messages/refactor/consider-using-max-builtin.html)

See https://github.com/astral-sh/ruff/issues/970 for rules

Test Plan: `cargo test`

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

@@ -0,0 +1,136 @@
+# 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
+
+# Parenthesized expressions
+if value.attr > 3:
+    (
+        value.
+        attr
+    ) = 3

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::IfStmtMinMax) {
+                pylint::rules::if_stmt_min_max(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::IfStmtMinMax),
         (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::IfStmtMinMax, Path::new("if_stmt_min_max.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/if_stmt_min_max.rs

@@ -0,0 +1,196 @@
+use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::comparable::ComparableExpr;
+use ruff_python_ast::parenthesize::parenthesized_range;
+use ruff_python_ast::{self as ast, CmpOp, Stmt};
+use ruff_text_size::Ranged;
+
+use crate::checkers::ast::Checker;
+use crate::fix::snippet::SourceCodeSnippet;
+
+/// ## What it does
+/// Checks for `if` statements that can be replaced with `min()` or `max()`
+/// calls.
+///
+/// ## Why is this bad?
+/// An `if` statement that selects the lesser or greater of two sub-expressions
+/// can be replaced with a `min()` or `max()` call respectively. When possible,
+/// prefer `min()` and `max()`, as they're more concise and readable than the
+/// equivalent `if` statements.
+///
+/// ## Example
+/// ```python
+/// if score > highest_score:
+///     highest_score = score
+/// ```
+///
+/// Use instead:
+/// ```python
+/// highest_score = max(highest_score, score)
+/// ```
+///
+/// ## 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 IfStmtMinMax {
+    min_max: MinMax,
+    replacement: SourceCodeSnippet,
+}
+
+impl Violation for IfStmtMinMax {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
+
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        let Self {
+            min_max,
+            replacement,
+        } = self;
+        if let Some(replacement) = replacement.full_display() {
+            format!("Replace `if` statement with `{replacement}`")
+        } else {
+            format!("Replace `if` statement with `{min_max}` call")
+        }
+    }
+
+    fn fix_title(&self) -> Option<String> {
+        let Self {
+            min_max,
+            replacement,
+        } = self;
+        if let Some(replacement) = replacement.full_display() {
+            Some(format!("Replace with `{replacement}`"))
+        } else {
+            Some(format!("Replace with `{min_max}` call"))
+        }
+    }
+}
+
+/// R1730, R1731
+pub(crate) fn if_stmt_min_max(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 [body @ 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;
+    };
+
+    // Ignore, e.g., `foo < bar < baz`.
+    let [op] = &**ops else {
+        return;
+    };
+
+    // Determine whether to use `min()` or `max()`, and whether to flip the
+    // order of the arguments, which is relevant for breaking ties.
+    let (min_max, flip_args) = match op {
+        CmpOp::Gt => (MinMax::Max, true),
+        CmpOp::GtE => (MinMax::Max, false),
+        CmpOp::Lt => (MinMax::Min, true),
+        CmpOp::LtE => (MinMax::Min, false),
+        _ => return,
+    };
+
+    let [right] = &**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);
+    let body_value_cmp = ComparableExpr::from(body_value);
+    if left_cmp != body_target_cmp || right_statement_cmp != body_value_cmp {
+        return;
+    }
+
+    let (arg1, arg2) = if flip_args {
+        (left.as_ref(), right)
+    } else {
+        (right, left.as_ref())
+    };
+
+    let replacement = format!(
+        "{} = {min_max}({}, {})",
+        checker.locator().slice(
+            parenthesized_range(
+                body_target.into(),
+                body.into(),
+                checker.indexer().comment_ranges(),
+                checker.locator().contents()
+            )
+            .unwrap_or(body_target.range())
+        ),
+        checker.locator().slice(arg1),
+        checker.locator().slice(arg2),
+    );
+
+    let mut diagnostic = Diagnostic::new(
+        IfStmtMinMax {
+            min_max,
+            replacement: SourceCodeSnippet::from_str(replacement.as_str()),
+        },
+        stmt_if.range(),
+    );
+
+    if checker.semantic().is_builtin(min_max.as_str()) {
+        diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
+            replacement,
+            stmt_if.range(),
+        )));
+    }
+
+    checker.diagnostics.push(diagnostic);
+}
+
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+enum MinMax {
+    Min,
+    Max,
+}
+
+impl MinMax {
+    fn as_str(self) -> &'static str {
+        match self {
+            Self::Min => "min",
+            Self::Max => "max",
+        }
+    }
+}
+
+impl std::fmt::Display for MinMax {
+    fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result {
+        write!(fmt, "{}", self.as_str())
+    }
+}

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

@@ -21,6 +21,7 @@ pub(crate) use eq_without_hash::*;
 pub(crate) use global_at_module_level::*;
 pub(crate) use global_statement::*;
 pub(crate) use global_variable_not_assigned::*;
+pub(crate) use if_stmt_min_max::*;
 pub(crate) use import_outside_top_level::*;
 pub(crate) use import_private_name::*;
 pub(crate) use import_self::*;
@@ -116,6 +117,7 @@ mod eq_without_hash;
 mod global_at_module_level;
 mod global_statement;
 mod global_variable_not_assigned;
+mod if_stmt_min_max;
 mod import_outside_top_level;
 mod import_private_name;
 mod import_self;

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

@@ -0,0 +1,296 @@
+---
+source: crates/ruff_linter/src/rules/pylint/mod.rs
+---
+if_stmt_min_max.py:8:1: PLR1730 [*] Replace `if` statement with `value = min(value, 10)`
+   |
+ 7 |   # Positive
+ 8 | / if value < 10:  # [max-instead-of-if]
+ 9 | |     value = 10
+   | |______________^ PLR1730
+10 |   
+11 |   if value <= 10:  # [max-instead-of-if]
+   |
+   = help: Replace with `value = min(value, 10)`
+
+ℹ Safe fix
+5  5  | value3 = 3
+6  6  | 
+7  7  | # Positive
+8     |-if value < 10:  # [max-instead-of-if]
+9     |-    value = 10
+   8  |+value = min(value, 10)
+10 9  | 
+11 10 | if value <= 10:  # [max-instead-of-if]
+12 11 |     value = 10
+
+if_stmt_min_max.py:11:1: PLR1730 [*] Replace `if` statement with `value = min(10, value)`
+   |
+ 9 |       value = 10
+10 |   
+11 | / if value <= 10:  # [max-instead-of-if]
+12 | |     value = 10
+   | |______________^ PLR1730
+13 |   
+14 |   if value < value2:  # [max-instead-of-if]
+   |
+   = help: Replace with `value = min(10, value)`
+
+ℹ Safe fix
+8  8  | if value < 10:  # [max-instead-of-if]
+9  9  |     value = 10
+10 10 | 
+11    |-if value <= 10:  # [max-instead-of-if]
+12    |-    value = 10
+   11 |+value = min(10, value)
+13 12 | 
+14 13 | if value < value2:  # [max-instead-of-if]
+15 14 |     value = value2
+
+if_stmt_min_max.py:14:1: PLR1730 [*] Replace `if` statement with `value = min(value, value2)`
+   |
+12 |       value = 10
+13 |   
+14 | / if value < value2:  # [max-instead-of-if]
+15 | |     value = value2
+   | |__________________^ PLR1730
+16 |   
+17 |   if value > 10:  # [min-instead-of-if]
+   |
+   = help: Replace with `value = min(value, value2)`
+
+ℹ Safe fix
+11 11 | if value <= 10:  # [max-instead-of-if]
+12 12 |     value = 10
+13 13 | 
+14    |-if value < value2:  # [max-instead-of-if]
+15    |-    value = value2
+   14 |+value = min(value, value2)
+16 15 | 
+17 16 | if value > 10:  # [min-instead-of-if]
+18 17 |     value = 10
+
+if_stmt_min_max.py:17:1: PLR1730 [*] Replace `if` statement with `value = max(value, 10)`
+   |
+15 |       value = value2
+16 |   
+17 | / if value > 10:  # [min-instead-of-if]
+18 | |     value = 10
+   | |______________^ PLR1730
+19 |   
+20 |   if value >= 10:  # [min-instead-of-if]
+   |
+   = help: Replace with `value = max(value, 10)`
+
+ℹ Safe fix
+14 14 | if value < value2:  # [max-instead-of-if]
+15 15 |     value = value2
+16 16 | 
+17    |-if value > 10:  # [min-instead-of-if]
+18    |-    value = 10
+   17 |+value = max(value, 10)
+19 18 | 
+20 19 | if value >= 10:  # [min-instead-of-if]
+21 20 |     value = 10
+
+if_stmt_min_max.py:20:1: PLR1730 [*] Replace `if` statement with `value = max(10, value)`
+   |
+18 |       value = 10
+19 |   
+20 | / if value >= 10:  # [min-instead-of-if]
+21 | |     value = 10
+   | |______________^ PLR1730
+22 |   
+23 |   if value > value2:  # [min-instead-of-if]
+   |
+   = help: Replace with `value = max(10, value)`
+
+ℹ Safe fix
+17 17 | if value > 10:  # [min-instead-of-if]
+18 18 |     value = 10
+19 19 | 
+20    |-if value >= 10:  # [min-instead-of-if]
+21    |-    value = 10
+   20 |+value = max(10, value)
+22 21 | 
+23 22 | if value > value2:  # [min-instead-of-if]
+24 23 |     value = value2
+
+if_stmt_min_max.py:23:1: PLR1730 [*] Replace `if` statement with `value = max(value, value2)`
+   |
+21 |       value = 10
+22 |   
+23 | / if value > value2:  # [min-instead-of-if]
+24 | |     value = value2
+   | |__________________^ PLR1730
+   |
+   = help: Replace with `value = max(value, value2)`
+
+ℹ Safe fix
+20 20 | if value >= 10:  # [min-instead-of-if]
+21 21 |     value = 10
+22 22 | 
+23    |-if value > value2:  # [min-instead-of-if]
+24    |-    value = value2
+   23 |+value = max(value, value2)
+25 24 | 
+26 25 | 
+27 26 | class A:
+
+if_stmt_min_max.py:33:1: PLR1730 [*] Replace `if` statement with `A1.value = min(A1.value, 10)`
+   |
+32 |   A1 = A()
+33 | / if A1.value < 10:  # [max-instead-of-if]
+34 | |     A1.value = 10
+   | |_________________^ PLR1730
+35 |   
+36 |   if A1.value > 10:  # [min-instead-of-if]
+   |
+   = help: Replace with `A1.value = min(A1.value, 10)`
+
+ℹ Safe fix
+30 30 | 
+31 31 | 
+32 32 | A1 = A()
+33    |-if A1.value < 10:  # [max-instead-of-if]
+34    |-    A1.value = 10
+   33 |+A1.value = min(A1.value, 10)
+35 34 | 
+36 35 | if A1.value > 10:  # [min-instead-of-if]
+37 36 |     A1.value = 10
+
+if_stmt_min_max.py:36:1: PLR1730 [*] Replace `if` statement with `A1.value = max(A1.value, 10)`
+   |
+34 |       A1.value = 10
+35 |   
+36 | / if A1.value > 10:  # [min-instead-of-if]
+37 | |     A1.value = 10
+   | |_________________^ PLR1730
+   |
+   = help: Replace with `A1.value = max(A1.value, 10)`
+
+ℹ Safe fix
+33 33 | if A1.value < 10:  # [max-instead-of-if]
+34 34 |     A1.value = 10
+35 35 | 
+36    |-if A1.value > 10:  # [min-instead-of-if]
+37    |-    A1.value = 10
+   36 |+A1.value = max(A1.value, 10)
+38 37 | 
+39 38 | 
+40 39 | class AA:
+
+if_stmt_min_max.py:60:1: PLR1730 [*] Replace `if` statement with `A2 = min(A2, A1)`
+   |
+58 |   A2 = AA(3)
+59 |   
+60 | / if A2 < A1:  # [max-instead-of-if]
+61 | |     A2 = A1
+   | |___________^ PLR1730
+62 |   
+63 |   if A2 <= A1:  # [max-instead-of-if]
+   |
+   = help: Replace with `A2 = min(A2, A1)`
+
+ℹ Safe fix
+57 57 | A1 = AA(0)
+58 58 | A2 = AA(3)
+59 59 | 
+60    |-if A2 < A1:  # [max-instead-of-if]
+61    |-    A2 = A1
+   60 |+A2 = min(A2, A1)
+62 61 | 
+63 62 | if A2 <= A1:  # [max-instead-of-if]
+64 63 |     A2 = A1
+
+if_stmt_min_max.py:63:1: PLR1730 [*] Replace `if` statement with `A2 = min(A1, A2)`
+   |
+61 |       A2 = A1
+62 |   
+63 | / if A2 <= A1:  # [max-instead-of-if]
+64 | |     A2 = A1
+   | |___________^ PLR1730
+65 |   
+66 |   if A2 > A1:  # [min-instead-of-if]
+   |
+   = help: Replace with `A2 = min(A1, A2)`
+
+ℹ Safe fix
+60 60 | if A2 < A1:  # [max-instead-of-if]
+61 61 |     A2 = A1
+62 62 | 
+63    |-if A2 <= A1:  # [max-instead-of-if]
+64    |-    A2 = A1
+   63 |+A2 = min(A1, A2)
+65 64 | 
+66 65 | if A2 > A1:  # [min-instead-of-if]
+67 66 |     A2 = A1
+
+if_stmt_min_max.py:66:1: PLR1730 [*] Replace `if` statement with `A2 = max(A2, A1)`
+   |
+64 |       A2 = A1
+65 |   
+66 | / if A2 > A1:  # [min-instead-of-if]
+67 | |     A2 = A1
+   | |___________^ PLR1730
+68 |   
+69 |   if A2 >= A1:  # [min-instead-of-if]
+   |
+   = help: Replace with `A2 = max(A2, A1)`
+
+ℹ Safe fix
+63 63 | if A2 <= A1:  # [max-instead-of-if]
+64 64 |     A2 = A1
+65 65 | 
+66    |-if A2 > A1:  # [min-instead-of-if]
+67    |-    A2 = A1
+   66 |+A2 = max(A2, A1)
+68 67 | 
+69 68 | if A2 >= A1:  # [min-instead-of-if]
+70 69 |     A2 = A1
+
+if_stmt_min_max.py:69:1: PLR1730 [*] Replace `if` statement with `A2 = max(A1, A2)`
+   |
+67 |       A2 = A1
+68 |   
+69 | / if A2 >= A1:  # [min-instead-of-if]
+70 | |     A2 = A1
+   | |___________^ PLR1730
+71 |   
+72 |   # Negative
+   |
+   = help: Replace with `A2 = max(A1, A2)`
+
+ℹ Safe fix
+66 66 | if A2 > A1:  # [min-instead-of-if]
+67 67 |     A2 = A1
+68 68 | 
+69    |-if A2 >= A1:  # [min-instead-of-if]
+70    |-    A2 = A1
+   69 |+A2 = max(A1, A2)
+71 70 | 
+72 71 | # Negative
+73 72 | if value < 10:
+
+if_stmt_min_max.py:132:1: PLR1730 [*] Replace `if` statement with `max` call
+    |
+131 |   # Parenthesized expressions
+132 | / if value.attr > 3:
+133 | |     (
+134 | |         value.
+135 | |         attr
+136 | |     ) = 3
+    | |_________^ PLR1730
+    |
+    = help: Replace with `max` call
+
+ℹ Safe fix
+129 129 |     value = 2
+130 130 | 
+131 131 | # Parenthesized expressions
+132     |-if value.attr > 3:
+133     |-    (
+    132 |+(
+134 133 |         value.
+135 134 |         attr
+136     |-    ) = 3
+    135 |+    ) = max(value.attr, 3)

ruff.schema.json

@@ -3355,6 +3355,7 @@
         "PLR172",
         "PLR1722",
         "PLR173",
+        "PLR1730",
         "PLR1733",
         "PLR1736",
         "PLR2",