feat(linter): add unicorn/prefer-math-min-max (#6621)

[unicorn/prefer-math-min-max](https://github.com/sindresorhus/eslint-plugin-unicorn/blob/v56.0.0/docs/rules/prefer-math-min-max.md)
of #684

---------

Co-authored-by: Brian Liu <brian@redflagsdating.com>
Co-authored-by: Don Isaac <donald.isaac@gmail.com>
Co-authored-by: Cam McHenry <camchenry@users.noreply.github.com>

Author: 4 people

crates/oxc_linter/src/rules.rs

@@ -334,6 +334,7 @@ mod unicorn {
     pub mod prefer_event_target;
     pub mod prefer_includes;
     pub mod prefer_logical_operator_over_ternary;
+    pub mod prefer_math_min_max;
     pub mod prefer_math_trunc;
     pub mod prefer_modern_dom_apis;
     pub mod prefer_modern_math_apis;
@@ -909,6 +910,7 @@ oxc_macros::declare_all_lint_rules! {
     unicorn::prefer_event_target,
     unicorn::prefer_includes,
     unicorn::prefer_logical_operator_over_ternary,
+    unicorn::prefer_math_min_max,
     unicorn::prefer_math_trunc,
     unicorn::prefer_modern_dom_apis,
     unicorn::prefer_modern_math_apis,

crates/oxc_linter/src/rules/unicorn/prefer_math_min_max.rs

@@ -0,0 +1,264 @@
+use std::borrow::Cow;
+
+use oxc_ast::{
+    ast::{BinaryExpression, BinaryOperator, Expression, UnaryOperator},
+    AstKind,
+};
+use oxc_diagnostics::OxcDiagnostic;
+use oxc_macros::declare_oxc_lint;
+use oxc_span::Span;
+
+use crate::{context::LintContext, rule::Rule, utils::is_same_reference, AstNode};
+
+fn prefer_math_min_max_diagnostic(span: Span) -> OxcDiagnostic {
+    OxcDiagnostic::warn(
+        "Prefer `Math.min()` and `Math.max()` over ternaries for simple comparisons.",
+    )
+    .with_label(span)
+}
+
+#[derive(Debug, Default, Clone)]
+pub struct PreferMathMinMax;
+
+#[derive(Debug)]
+enum TypeOptions {
+    Min,
+    Max,
+    Unknown,
+}
+
+declare_oxc_lint!(
+    /// ### What it does
+    ///
+    /// Prefers use of [`Math.min()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/min)
+    /// and [`Math.max()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/max) instead of
+    /// ternary expressions when performing simple comparisons for more concise, easier to understand, and less prone to errors.
+    ///
+    /// ### Examples
+    ///
+    /// Examples of **incorrect** code for this rule:
+    /// ```javascript
+    /// height > 50 ? 50 : height;
+    /// height > 50 ? height : 50;
+    /// ```
+    ///
+    /// Examples of **correct** code for this rule:
+    /// ```javascript
+    /// Math.min(height, 50);
+    /// Math.max(height, 50);
+    /// ```
+    PreferMathMinMax,
+    pedantic,
+    fix
+);
+
+impl Rule for PreferMathMinMax {
+    fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
+        let AstKind::ConditionalExpression(conditional_expr) = node.kind() else {
+            return;
+        };
+
+        let Expression::BinaryExpression(test_expr) = &conditional_expr.test else {
+            return;
+        };
+
+        let condition_type =
+            is_min_max(test_expr, &conditional_expr.consequent, &conditional_expr.alternate, ctx);
+
+        if matches!(condition_type, TypeOptions::Unknown) {
+            return;
+        }
+
+        ctx.diagnostic_with_fix(prefer_math_min_max_diagnostic(conditional_expr.span), |fixer| {
+            let Some(consequent) = get_expr_value(&conditional_expr.consequent) else {
+                return fixer.noop();
+            };
+            let Some(alternate) = get_expr_value(&conditional_expr.alternate) else {
+                return fixer.noop();
+            };
+
+            match condition_type {
+                TypeOptions::Max => fixer.replace(
+                    conditional_expr.span,
+                    Cow::Owned(format!("Math.max({consequent}, {alternate})")),
+                ),
+                TypeOptions::Min => fixer.replace(
+                    conditional_expr.span,
+                    Cow::Owned(format!("Math.min({consequent}, {alternate})")),
+                ),
+                TypeOptions::Unknown => unreachable!(),
+            }
+        });
+    }
+}
+
+fn get_expr_value(expr: &Expression) -> Option<String> {
+    match expr {
+        Expression::NumericLiteral(lit) => Some(lit.to_string()),
+        Expression::UnaryExpression(lit) => {
+            let mut unary_str: String = String::from(lit.operator.as_str());
+
+            let Some(unary_lit) = get_expr_value(&lit.argument) else {
+                return Some(unary_str.to_string());
+            };
+
+            unary_str.push_str(unary_lit.as_str());
+
+            Some(unary_str.to_string())
+        }
+        Expression::Identifier(identifier) => Some(identifier.name.to_string()),
+        _ => None,
+    }
+}
+
+fn is_same_expression(left: &Expression, right: &Expression, ctx: &LintContext) -> bool {
+    if is_same_reference(left, right, ctx) {
+        return true;
+    }
+
+    match (left, right) {
+        (Expression::UnaryExpression(left_expr), Expression::UnaryExpression(right_expr)) => {
+            left_expr.operator == UnaryOperator::UnaryNegation
+                && right_expr.operator == UnaryOperator::UnaryNegation
+                && is_same_reference(&left_expr.argument, &right_expr.argument, ctx)
+        }
+        _ => false,
+    }
+}
+
+fn is_min_max(
+    condition: &BinaryExpression,
+    consequent: &Expression,
+    alternate: &Expression,
+    ctx: &LintContext,
+) -> TypeOptions {
+    let is_matched = matches!(
+        (condition.left.get_inner_expression(), condition.right.get_inner_expression()),
+        (Expression::NumericLiteral(_) | Expression::UnaryExpression(_), Expression::Identifier(_))
+            | (
+                Expression::Identifier(_),
+                Expression::NumericLiteral(_) | Expression::UnaryExpression(_)
+            )
+    );
+
+    if !condition.operator.is_compare() || !is_matched {
+        return TypeOptions::Unknown;
+    }
+
+    if is_same_expression(&condition.left, consequent, ctx)
+        && is_same_expression(&condition.right, alternate, ctx)
+    {
+        if matches!(condition.operator, BinaryOperator::LessThan | BinaryOperator::LessEqualThan) {
+            return TypeOptions::Min;
+        }
+        return TypeOptions::Max;
+    } else if is_same_expression(&condition.left, alternate, ctx)
+        && is_same_expression(&condition.right, consequent, ctx)
+    {
+        if matches!(condition.operator, BinaryOperator::LessThan | BinaryOperator::LessEqualThan) {
+            return TypeOptions::Max;
+        }
+        return TypeOptions::Min;
+    }
+
+    TypeOptions::Unknown
+}
+
+#[test]
+fn test() {
+    use crate::tester::Tester;
+
+    let pass = vec![
+        r"const foo = height ? height : 50;",
+        r"const foo = height == 'success' ? height : 'failed';",
+        r"Math.min(height, 50);",
+        r"Math.max(height, 50);",
+        r"Math.min(50, height);",
+        r"Math.max(-50, height);",
+        r"const foo = height < 50n ? height : 50n;",
+    ];
+
+    let fail: Vec<&str> = vec![
+        r"const foo = height < 50 ? height : 50;",
+        r"const foo = height <= -50 ? height : -50;",
+        r"const foo = 150.34 < height ? 150.34 : height;",
+        r"const foo = -2.34 <= height ? -2.34 : height;",
+        r"const foo = height > 10e3 ? 10e3 : height;",
+        r"const foo = height >= -10e3 ? -10e3 : height;",
+        r"const foo = 50 > height ? height : 50;",
+        r"const foo = 50 >= height ? height : 50;",
+        r"return height > 100 ? height : 100;",
+        r"return height >= 50 ? height : 50;",
+        r"const foo = (50 > height ? 50 : height) || 0;",
+        r"const foo = (50 >= height ? 50 : height) || 0;",
+        r"return (height < 50 ? 50 : height) || 100;",
+        r"return (height <= 50 ? 50 : height) || 200;",
+        r"const foo = 50 < height ? height: 50;",
+        r"const foo = 50 <= height ? height: 50;",
+    ];
+
+    let fix = vec![
+        (r"const foo = height < 100 ? height : 100;", r"const foo = Math.min(height, 100);", None),
+        (
+            r"const foo = height <= -100 ? height : -100;",
+            r"const foo = Math.min(height, -100);",
+            None,
+        ),
+        (
+            r"const foo = 150.34 < height ? 150.34 : height;",
+            r"const foo = Math.min(150.34, height);",
+            None,
+        ),
+        (
+            r"const foo = -0.34 <= height ? -0.34 : height;",
+            r"const foo = Math.min(-0.34, height);",
+            None,
+        ),
+        (
+            r"const foo = height > 10e3 ? 10e3 : height;",
+            r"const foo = Math.min(10e3, height);",
+            None,
+        ),
+        (
+            r"const foo = height >= -10e3 ? -10e3 : height;",
+            r"const foo = Math.min(-10e3, height);",
+            None,
+        ),
+        (
+            r"const foo = 10e3 > height ? height : 10e3;",
+            r"const foo = Math.min(height, 10e3);",
+            None,
+        ),
+        (
+            r"const foo = 10e3 >= height ? height : 10e3;",
+            r"const foo = Math.min(height, 10e3);",
+            None,
+        ),
+        ("return height > 100 ? height : 100;", "return Math.max(height, 100);", None),
+        ("return height >= 50 ? height : 50;", "return Math.max(height, 50);", None),
+        (
+            "return (10e3 > height ? 10e3 : height) || 200;",
+            "return (Math.max(10e3, height)) || 200;",
+            None,
+        ),
+        (
+            "return (-10e3 >= height ? -10e3 : height) || 200;",
+            "return (Math.max(-10e3, height)) || 200;",
+            None,
+        ),
+        (
+            "return (height < 2.99 ? 2.99 : height) || 0.99;",
+            "return (Math.max(2.99, height)) || 0.99;",
+            None,
+        ),
+        (
+            "return (height <= -0.99 ? -0.99 : height) || -3.99;",
+            "return (Math.max(-0.99, height)) || -3.99;",
+            None,
+        ),
+        ("return 10e6 < height ? height : 10e6;", "return Math.max(height, 10e6);", None),
+        ("return -10e4 <= height ? height : -10e4;", "return Math.max(height, -10e4);", None),
+    ];
+
+    Tester::new(PreferMathMinMax::NAME, pass, fail).expect_fix(fix).test_and_snapshot();
+}

crates/oxc_linter/src/snapshots/prefer_math_min_max.snap

@@ -0,0 +1,114 @@
+---
+source: crates/oxc_linter/src/tester.rs
+---
+  ⚠ eslint-plugin-unicorn(prefer-math-min-max): Prefer `Math.min()` and `Math.max()` over ternaries for simple comparisons.
+   ╭─[prefer_math_min_max.tsx:1:13]
+ 1 │ const foo = height < 50 ? height : 50;
+   ·             ─────────────────────────
+   ╰────
+  help: Replace `height < 50 ? height : 50` with `Math.min(height, 50)`.
+
+  ⚠ eslint-plugin-unicorn(prefer-math-min-max): Prefer `Math.min()` and `Math.max()` over ternaries for simple comparisons.
+   ╭─[prefer_math_min_max.tsx:1:13]
+ 1 │ const foo = height <= -50 ? height : -50;
+   ·             ────────────────────────────
+   ╰────
+  help: Replace `height <= -50 ? height : -50` with `Math.min(height, -50)`.
+
+  ⚠ eslint-plugin-unicorn(prefer-math-min-max): Prefer `Math.min()` and `Math.max()` over ternaries for simple comparisons.
+   ╭─[prefer_math_min_max.tsx:1:13]
+ 1 │ const foo = 150.34 < height ? 150.34 : height;
+   ·             ─────────────────────────────────
+   ╰────
+  help: Replace `150.34 < height ? 150.34 : height` with `Math.min(150.34, height)`.
+
+  ⚠ eslint-plugin-unicorn(prefer-math-min-max): Prefer `Math.min()` and `Math.max()` over ternaries for simple comparisons.
+   ╭─[prefer_math_min_max.tsx:1:13]
+ 1 │ const foo = -2.34 <= height ? -2.34 : height;
+   ·             ────────────────────────────────
+   ╰────
+  help: Replace `-2.34 <= height ? -2.34 : height` with `Math.min(-2.34, height)`.
+
+  ⚠ eslint-plugin-unicorn(prefer-math-min-max): Prefer `Math.min()` and `Math.max()` over ternaries for simple comparisons.
+   ╭─[prefer_math_min_max.tsx:1:13]
+ 1 │ const foo = height > 10e3 ? 10e3 : height;
+   ·             ─────────────────────────────
+   ╰────
+  help: Replace `height > 10e3 ? 10e3 : height` with `Math.min(10e3, height)`.
+
+  ⚠ eslint-plugin-unicorn(prefer-math-min-max): Prefer `Math.min()` and `Math.max()` over ternaries for simple comparisons.
+   ╭─[prefer_math_min_max.tsx:1:13]
+ 1 │ const foo = height >= -10e3 ? -10e3 : height;
+   ·             ────────────────────────────────
+   ╰────
+  help: Replace `height >= -10e3 ? -10e3 : height` with `Math.min(-10e3, height)`.
+
+  ⚠ eslint-plugin-unicorn(prefer-math-min-max): Prefer `Math.min()` and `Math.max()` over ternaries for simple comparisons.
+   ╭─[prefer_math_min_max.tsx:1:13]
+ 1 │ const foo = 50 > height ? height : 50;
+   ·             ─────────────────────────
+   ╰────
+  help: Replace `50 > height ? height : 50` with `Math.min(height, 50)`.
+
+  ⚠ eslint-plugin-unicorn(prefer-math-min-max): Prefer `Math.min()` and `Math.max()` over ternaries for simple comparisons.
+   ╭─[prefer_math_min_max.tsx:1:13]
+ 1 │ const foo = 50 >= height ? height : 50;
+   ·             ──────────────────────────
+   ╰────
+  help: Replace `50 >= height ? height : 50` with `Math.min(height, 50)`.
+
+  ⚠ eslint-plugin-unicorn(prefer-math-min-max): Prefer `Math.min()` and `Math.max()` over ternaries for simple comparisons.
+   ╭─[prefer_math_min_max.tsx:1:8]
+ 1 │ return height > 100 ? height : 100;
+   ·        ───────────────────────────
+   ╰────
+  help: Replace `height > 100 ? height : 100` with `Math.max(height, 100)`.
+
+  ⚠ eslint-plugin-unicorn(prefer-math-min-max): Prefer `Math.min()` and `Math.max()` over ternaries for simple comparisons.
+   ╭─[prefer_math_min_max.tsx:1:8]
+ 1 │ return height >= 50 ? height : 50;
+   ·        ──────────────────────────
+   ╰────
+  help: Replace `height >= 50 ? height : 50` with `Math.max(height, 50)`.
+
+  ⚠ eslint-plugin-unicorn(prefer-math-min-max): Prefer `Math.min()` and `Math.max()` over ternaries for simple comparisons.
+   ╭─[prefer_math_min_max.tsx:1:14]
+ 1 │ const foo = (50 > height ? 50 : height) || 0;
+   ·              ─────────────────────────
+   ╰────
+  help: Replace `50 > height ? 50 : height` with `Math.max(50, height)`.
+
+  ⚠ eslint-plugin-unicorn(prefer-math-min-max): Prefer `Math.min()` and `Math.max()` over ternaries for simple comparisons.
+   ╭─[prefer_math_min_max.tsx:1:14]
+ 1 │ const foo = (50 >= height ? 50 : height) || 0;
+   ·              ──────────────────────────
+   ╰────
+  help: Replace `50 >= height ? 50 : height` with `Math.max(50, height)`.
+
+  ⚠ eslint-plugin-unicorn(prefer-math-min-max): Prefer `Math.min()` and `Math.max()` over ternaries for simple comparisons.
+   ╭─[prefer_math_min_max.tsx:1:9]
+ 1 │ return (height < 50 ? 50 : height) || 100;
+   ·         ─────────────────────────
+   ╰────
+  help: Replace `height < 50 ? 50 : height` with `Math.max(50, height)`.
+
+  ⚠ eslint-plugin-unicorn(prefer-math-min-max): Prefer `Math.min()` and `Math.max()` over ternaries for simple comparisons.
+   ╭─[prefer_math_min_max.tsx:1:9]
+ 1 │ return (height <= 50 ? 50 : height) || 200;
+   ·         ──────────────────────────
+   ╰────
+  help: Replace `height <= 50 ? 50 : height` with `Math.max(50, height)`.
+
+  ⚠ eslint-plugin-unicorn(prefer-math-min-max): Prefer `Math.min()` and `Math.max()` over ternaries for simple comparisons.
+   ╭─[prefer_math_min_max.tsx:1:13]
+ 1 │ const foo = 50 < height ? height: 50;
+   ·             ────────────────────────
+   ╰────
+  help: Replace `50 < height ? height: 50` with `Math.max(height, 50)`.
+
+  ⚠ eslint-plugin-unicorn(prefer-math-min-max): Prefer `Math.min()` and `Math.max()` over ternaries for simple comparisons.
+   ╭─[prefer_math_min_max.tsx:1:13]
+ 1 │ const foo = 50 <= height ? height: 50;
+   ·             ─────────────────────────
+   ╰────
+  help: Replace `50 <= height ? height: 50` with `Math.max(height, 50)`.