From 1bb957bcf809113ef57b1ddc6181f53d4af9627a Mon Sep 17 00:00:00 2001 From: Boshen Date: Wed, 21 Jun 2023 09:12:29 +0800 Subject: [PATCH] feat(linter): implement `bad_min_max_func` from deepscan --- crates/oxc_ast/src/ast/js.rs | 11 ++ crates/oxc_linter/src/rules.rs | 1 + .../src/rules/deepscan/bad_min_max_func.rs | 151 ++++++++++++++++++ .../src/snapshots/bad_min_max_func.snap | 61 +++++++ 4 files changed, 224 insertions(+) create mode 100644 crates/oxc_linter/src/rules/deepscan/bad_min_max_func.rs create mode 100644 crates/oxc_linter/src/snapshots/bad_min_max_func.snap diff --git a/crates/oxc_ast/src/ast/js.rs b/crates/oxc_ast/src/ast/js.rs index 24cde9f64..0b4a98f70 100644 --- a/crates/oxc_ast/src/ast/js.rs +++ b/crates/oxc_ast/src/ast/js.rs @@ -220,6 +220,17 @@ impl<'a> Expression<'a> { _ => None, } } + + pub fn get_member_expr(&self) -> Option<&MemberExpression<'a>> { + match self.get_inner_expression() { + Expression::ChainExpression(chain_expr) => match &chain_expr.expression { + ChainElement::CallExpression(_) => None, + ChainElement::MemberExpression(member_expr) => Some(member_expr), + }, + Expression::MemberExpression(member_expr) => Some(member_expr), + _ => None, + } + } } /// Identifier Name diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 1f832a803..ac7589ea8 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -32,6 +32,7 @@ oxc_macros::declare_all_lint_rules! { deepscan::bad_comparison_sequence, deepscan::bad_array_method_on_arguments, deepscan::missing_throw, + deepscan::bad_min_max_func, use_isnan, valid_typeof, typescript::isolated_declaration diff --git a/crates/oxc_linter/src/rules/deepscan/bad_min_max_func.rs b/crates/oxc_linter/src/rules/deepscan/bad_min_max_func.rs new file mode 100644 index 000000000..53e829cc1 --- /dev/null +++ b/crates/oxc_linter/src/rules/deepscan/bad_min_max_func.rs @@ -0,0 +1,151 @@ +use oxc_ast::{ + ast::{Argument, CallExpression, Expression}, + AstKind, +}; +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::Error, +}; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; + +use crate::{context::LintContext, rule::Rule, AstNode}; + +#[derive(Debug, Error, Diagnostic)] +#[error("Math.min and Math.max combination leads to constant result")] +#[diagnostic( + severity(warning), + help("This evaluates to {0:?} because of the incorrect `Math.min`/`Math.max` combination") +)] +struct BadMinMaxFuncDiagnostic(f64, #[label] pub Span); + +/// `https://deepscan.io/docs/rules/bad-min-max-func` +#[derive(Debug, Default, Clone)] +pub struct BadMinMaxFunc; + +declare_oxc_lint!( + /// ### What it does + /// + /// Checks whether the clamp function `Math.min(Math.max(x, y), z)` always evaluate to a + /// constant result because the arguments are in the wrong order. + /// + /// ### Example + /// ```javascript + /// Math.min(Math.max(100, x), 0); + /// Math.max(1000, Math.min(0, z)); + /// ``` + BadMinMaxFunc, + correctness +); + +impl Rule for BadMinMaxFunc { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let AstKind::CallExpression(call_expr) = node.kind() else { + return ; + }; + + if let Some((out_min_max, inner_exprs)) = Self::min_max(call_expr) { + for expr in inner_exprs { + if let Some((inner_min_max, ..)) = Self::min_max(expr) { + let constant_result = match (&out_min_max, &inner_min_max) { + (MinMax::Max(max), MinMax::Min(min)) => { + if max > min { + Some(max) + } else { + None + } + } + (MinMax::Min(min), MinMax::Max(max)) => { + if min < max { + Some(min) + } else { + None + } + } + _ => None, + }; + + if let Some(constant) = constant_result { + ctx.diagnostic(BadMinMaxFuncDiagnostic(*constant, call_expr.span)); + } + } + } + } + } +} + +enum MinMax { + Min(f64), + Max(f64), +} + +impl BadMinMaxFunc { + fn min_max<'a>(node: &'a CallExpression<'a>) -> Option<(MinMax, Vec<&'a CallExpression<'a>>)> { + let CallExpression { callee, arguments, .. } = node; + + if let Some(member_expr) = callee.get_member_expr() { + if let Expression::Identifier(ident) = member_expr.object() { + if ident.name != "Math" { + return None; + } + + let number_args = arguments.iter().filter_map(|arg| { + if let Argument::Expression(Expression::NumberLiteral(literal)) = arg { + Some(literal.value) + } else { + None + } + }); + + let min_max = match member_expr.static_property_name() { + Some("max") => MinMax::Max(number_args.fold(f64::NEG_INFINITY, f64::max)), + Some("min") => MinMax::Min(number_args.fold(f64::INFINITY, f64::min)), + _ => return None, + }; + + let mut inner = vec![]; + + for expr in arguments.iter().filter_map(|arg| { + if let Argument::Expression(Expression::CallExpression(expr)) = arg { + Some(&**expr) + } else { + None + } + }) { + inner.push(expr); + } + + return Some((min_max, inner)); + } + } + + None + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + ("Math.max(0, Math.min(100, x))", None), + ("Math.max(Math.min(100, x), 0)", None), + ("Math.min(100, Math.max(0.9, x))", None), + ("Math.min(255.255, Math.max(0, x))", None), + ("Math.min(Math.max(0, x), 255)", None), + ("Math.min(0, Math.min(0, x))", None), + ]; + + let fail = vec![ + ("Math.min(Math.max(100, x), 0)", None), + ("Math.max(255.255, Math.min(0, x))", None), + ("Math.max(Math.min(0, x), 255)", None), + ("Math.max(1000, Math.min(0, z))", None), + ("Math[\"min\"](0, Math.max(100, x))", None), + ("Math.min(Math.max(1000, x), 100, 3)", None), + ("Math.min(0, 5, Math['max'](x, 100, 30))", None), + ("Math.min(Math.max(1e3, x), 1.55e2)", None), + ]; + + Tester::new(BadMinMaxFunc::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/bad_min_max_func.snap b/crates/oxc_linter/src/snapshots/bad_min_max_func.snap new file mode 100644 index 000000000..3a093c2a9 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/bad_min_max_func.snap @@ -0,0 +1,61 @@ +--- +source: crates/oxc_linter/src/tester.rs +expression: bad_min_max_func +--- + ⚠ Math.min and Math.max combination leads to constant result + ╭─[bad_min_max_func.tsx:1:1] + 1 │ Math.min(Math.max(100, x), 0) + · ───────────────────────────── + ╰──── + help: This evaluates to 0.0 because of the incorrect `Math.min`/`Math.max` combination + + ⚠ Math.min and Math.max combination leads to constant result + ╭─[bad_min_max_func.tsx:1:1] + 1 │ Math.max(255.255, Math.min(0, x)) + · ───────────────────────────────── + ╰──── + help: This evaluates to 255.255 because of the incorrect `Math.min`/`Math.max` combination + + ⚠ Math.min and Math.max combination leads to constant result + ╭─[bad_min_max_func.tsx:1:1] + 1 │ Math.max(Math.min(0, x), 255) + · ───────────────────────────── + ╰──── + help: This evaluates to 255.0 because of the incorrect `Math.min`/`Math.max` combination + + ⚠ Math.min and Math.max combination leads to constant result + ╭─[bad_min_max_func.tsx:1:1] + 1 │ Math.max(1000, Math.min(0, z)) + · ────────────────────────────── + ╰──── + help: This evaluates to 1000.0 because of the incorrect `Math.min`/`Math.max` combination + + ⚠ Math.min and Math.max combination leads to constant result + ╭─[bad_min_max_func.tsx:1:1] + 1 │ Math["min"](0, Math.max(100, x)) + · ──────────────────────────────── + ╰──── + help: This evaluates to 0.0 because of the incorrect `Math.min`/`Math.max` combination + + ⚠ Math.min and Math.max combination leads to constant result + ╭─[bad_min_max_func.tsx:1:1] + 1 │ Math.min(Math.max(1000, x), 100, 3) + · ─────────────────────────────────── + ╰──── + help: This evaluates to 3.0 because of the incorrect `Math.min`/`Math.max` combination + + ⚠ Math.min and Math.max combination leads to constant result + ╭─[bad_min_max_func.tsx:1:1] + 1 │ Math.min(0, 5, Math['max'](x, 100, 30)) + · ─────────────────────────────────────── + ╰──── + help: This evaluates to 0.0 because of the incorrect `Math.min`/`Math.max` combination + + ⚠ Math.min and Math.max combination leads to constant result + ╭─[bad_min_max_func.tsx:1:1] + 1 │ Math.min(Math.max(1e3, x), 1.55e2) + · ────────────────────────────────── + ╰──── + help: This evaluates to 155.0 because of the incorrect `Math.min`/`Math.max` combination + +