-
-
Notifications
You must be signed in to change notification settings - Fork 471
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(biome_js_analyze): noConstantMathMinMaxClamp (#2404)
- Loading branch information
Showing
19 changed files
with
850 additions
and
51 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
206 changes: 206 additions & 0 deletions
206
crates/biome_js_analyze/src/lint/nursery/no_constant_math_min_max_clamp.rs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,206 @@ | ||
| use std::{cmp::Ordering, str::FromStr}; | ||
|
|
||
| use biome_analyze::{ | ||
| context::RuleContext, declare_rule, ActionCategory, FixKind, Rule, RuleDiagnostic, RuleSource, | ||
| }; | ||
| use biome_console::markup; | ||
| use biome_diagnostics::Applicability; | ||
| use biome_js_semantic::SemanticModel; | ||
| use biome_js_syntax::{ | ||
| global_identifier, AnyJsExpression, AnyJsLiteralExpression, AnyJsMemberExpression, | ||
| JsCallExpression, JsNumberLiteralExpression, | ||
| }; | ||
| use biome_rowan::{AstNode, BatchMutationExt}; | ||
|
|
||
| use crate::{services::semantic::Semantic, JsRuleAction}; | ||
|
|
||
| declare_rule! { | ||
| /// Disallow the use of `Math.min` and `Math.max` to clamp a value where the result itself is constant. | ||
| /// | ||
| /// ## Examples | ||
| /// | ||
| /// ### Invalid | ||
| /// | ||
| /// ```js,expect_diagnostic | ||
| /// Math.min(0, Math.max(100, x)); | ||
| /// ``` | ||
| /// | ||
| /// ```js,expect_diagnostic | ||
| /// Math.max(100, Math.min(0, x)); | ||
| /// ``` | ||
| /// ### Valid | ||
| /// | ||
| /// ```js | ||
| /// Math.min(100, Math.max(0, x)); | ||
| /// ``` | ||
| /// | ||
| pub NoConstantMathMinMaxClamp { | ||
| version: "next", | ||
| name: "noConstantMathMinMaxClamp", | ||
| sources: &[RuleSource::Clippy("min_max")], | ||
| recommended: false, | ||
| fix_kind: FixKind::Unsafe, | ||
| } | ||
| } | ||
|
|
||
| impl Rule for NoConstantMathMinMaxClamp { | ||
| type Query = Semantic<JsCallExpression>; | ||
| type State = (JsNumberLiteralExpression, JsNumberLiteralExpression); | ||
| type Signals = Option<Self::State>; | ||
| type Options = (); | ||
|
|
||
| fn run(ctx: &RuleContext<Self>) -> Self::Signals { | ||
| let node = ctx.query(); | ||
| let model = ctx.model(); | ||
|
|
||
| let outer_call = get_math_min_or_max_call(node, model)?; | ||
|
|
||
| let inner_call = get_math_min_or_max_call( | ||
| outer_call | ||
| .other_expression_argument | ||
| .as_js_call_expression()?, | ||
| model, | ||
| )?; | ||
|
|
||
| if outer_call.kind == inner_call.kind { | ||
| return None; | ||
| } | ||
|
|
||
| match ( | ||
| outer_call.kind, | ||
| outer_call | ||
| .constant_argument | ||
| .as_number()? | ||
| .partial_cmp(&inner_call.constant_argument.as_number()?), | ||
| ) { | ||
| (MinMaxKind::Min, Some(Ordering::Less)) | ||
| | (MinMaxKind::Max, Some(Ordering::Greater)) => { | ||
| Some((outer_call.constant_argument, inner_call.constant_argument)) | ||
| } | ||
| _ => None, | ||
| } | ||
| } | ||
|
|
||
| fn diagnostic(ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> { | ||
| let node = ctx.query(); | ||
|
|
||
| Some( | ||
| RuleDiagnostic::new( | ||
| rule_category!(), | ||
| node.range(), | ||
| markup! { | ||
| "This "<Emphasis>"Math.min/Math.max"</Emphasis>" combination leads to a constant result." | ||
| } | ||
| ).detail( | ||
| state.0.range(), | ||
| markup! { | ||
| "It always evaluates to "<Emphasis>{state.0.text()}</Emphasis>"." | ||
| } | ||
| ) | ||
| ) | ||
| } | ||
|
|
||
| fn action(ctx: &RuleContext<Self>, state: &Self::State) -> Option<JsRuleAction> { | ||
| let mut mutation = ctx.root().begin(); | ||
|
|
||
| mutation.replace_node(state.0.clone(), state.1.clone()); | ||
| mutation.replace_node(state.1.clone(), state.0.clone()); | ||
|
|
||
| Some(JsRuleAction { | ||
| mutation, | ||
| message: markup! {"Swap "<Emphasis>{state.0.text()}</Emphasis>" with "<Emphasis>{state.1.text()}</Emphasis>"."} | ||
| .to_owned(), | ||
| category: ActionCategory::QuickFix, | ||
| applicability: Applicability::MaybeIncorrect, | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| #[derive(PartialEq, Eq, Debug, Clone, Copy)] | ||
| enum MinMaxKind { | ||
| Min, | ||
| Max, | ||
| } | ||
|
|
||
| impl FromStr for MinMaxKind { | ||
| type Err = &'static str; | ||
|
|
||
| fn from_str(s: &str) -> Result<Self, Self::Err> { | ||
| match s { | ||
| "min" => Ok(MinMaxKind::Min), | ||
| "max" => Ok(MinMaxKind::Max), | ||
| _ => Err("Value not supported for math min max kind"), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #[derive(Debug, Clone)] | ||
| struct MathMinOrMaxCall { | ||
| kind: MinMaxKind, | ||
| constant_argument: JsNumberLiteralExpression, | ||
| other_expression_argument: AnyJsExpression, | ||
| } | ||
|
|
||
| fn get_math_min_or_max_call( | ||
| call_expression: &JsCallExpression, | ||
| model: &SemanticModel, | ||
| ) -> Option<MathMinOrMaxCall> { | ||
| let callee = call_expression.callee().ok()?.omit_parentheses(); | ||
| let member_expr = AnyJsMemberExpression::cast_ref(callee.syntax())?; | ||
|
|
||
| let member_name = member_expr.member_name()?; | ||
| let member_name = member_name.text(); | ||
|
|
||
| let min_or_max = MinMaxKind::from_str(member_name).ok()?; | ||
|
|
||
| let object = member_expr.object().ok()?.omit_parentheses(); | ||
| let (reference, name) = global_identifier(&object)?; | ||
|
|
||
| if name.text() != "Math" || model.binding(&reference).is_some() { | ||
| return None; | ||
| } | ||
|
|
||
| let arguments = call_expression.arguments().ok()?.args(); | ||
| let mut iter = arguments.into_iter(); | ||
|
|
||
| let first_argument = iter.next()?.ok()?; | ||
| let first_argument = first_argument.as_any_js_expression()?; | ||
|
|
||
| let second_argument = iter.next()?.ok()?; | ||
| let second_argument = second_argument.as_any_js_expression()?; | ||
|
|
||
| // `Math.min` and `Math.max` are variadic functions. | ||
| // We give up if they have more than 2 arguments. | ||
| if iter.next().is_some() { | ||
| return None; | ||
| } | ||
|
|
||
| match (first_argument, second_argument) { | ||
| ( | ||
| any_expression, | ||
| AnyJsExpression::AnyJsLiteralExpression( | ||
| AnyJsLiteralExpression::JsNumberLiteralExpression(constant_value), | ||
| ), | ||
| ) | ||
| | ( | ||
| AnyJsExpression::AnyJsLiteralExpression( | ||
| AnyJsLiteralExpression::JsNumberLiteralExpression(constant_value), | ||
| ), | ||
| any_expression, | ||
| ) => { | ||
| // The non-number literal argument must either be a call expression or an identifier expression. | ||
| if any_expression.as_js_call_expression().is_none() | ||
| && any_expression.as_js_identifier_expression().is_none() | ||
| { | ||
| return None; | ||
| } | ||
|
|
||
| Some(MathMinOrMaxCall { | ||
| kind: min_or_max, | ||
| constant_argument: constant_value.clone(), | ||
| other_expression_argument: any_expression.clone(), | ||
| }) | ||
| } | ||
| _ => None, | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
19 changes: 19 additions & 0 deletions
19
crates/biome_js_analyze/tests/specs/nursery/noConstantMathMinMaxClamp/invalid.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| Math.min(0, Math.max(100, x)); | ||
|
|
||
| Math.max(100, Math.min(0, x)); | ||
|
|
||
| Math.max(100, Math.min(x, 0)); | ||
|
|
||
| window.Math.min(0, window.Math.max(100, x)); | ||
|
|
||
| window.Math.min(0, Math.max(100, x)); | ||
|
|
||
| Math.min(0, window.Math.max(100, x)); | ||
|
|
||
| globalThis.Math.min(0, globalThis.Math.max(100, x)); | ||
|
|
||
| globalThis.Math.min(0, Math.max(100, x)); | ||
|
|
||
| Math.min(0, globalThis.Math.max(100, x)); | ||
|
|
||
| foo(Math.min(0, Math.max(100, x))); |
Oops, something went wrong.