feat: add useSpread rule #7681
+420
−4
There are no files selected for viewing
This file contains hidden or 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,28 @@ | ||
| --- | ||
| "@biomejs/biome": patch | ||
| --- | ||
|
|
||
| Added the new lint rule, [`useSpread`](https://biomejs.dev/linter/rules/use-spread/), ported from the ESLint rule [`prefer-spread`](https://eslint.org/docs/latest/rules/prefer-spread). | ||
|
|
||
| This rule enforces the use of the **spread syntax** (`...`) over `Function.prototype.apply()` when calling variadic functions, as spread syntax is generally more concise and idiomatic in modern JavaScript (ES2015+). | ||
|
|
||
| The rule provides a safe fix. | ||
|
|
||
| #### Invalid | ||
|
|
||
| ```js | ||
| Math.max.apply(Math, args); | ||
| foo.apply(undefined, args); | ||
| obj.method.apply(obj, args); | ||
| ``` | ||
|
|
||
| #### Valid | ||
|
|
||
| ```js | ||
| Math.max(...args); | ||
| foo(...args); | ||
| obj.method(...args); | ||
|
|
||
| // Allowed: cases where the `this` binding is intentionally changed | ||
| foo.apply(otherObj, args); | ||
| ``` |
This file contains hidden or 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 hidden or 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 hidden or 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,168 @@ | ||
| use crate::JsRuleAction; | ||
| use biome_analyze::{ | ||
| Ast, FixKind, Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule, | ||
| }; | ||
| use biome_console::markup; | ||
| use biome_js_factory::make; | ||
| use biome_js_syntax::{ | ||
| AnyJsCallArgument, AnyJsExpression, AnyJsMemberExpression, JsCallExpression, JsLanguage, T, | ||
| }; | ||
| use biome_rowan::{AstNode, AstSeparatedList, BatchMutationExt, SyntaxToken}; | ||
| use biome_rule_options::use_spread::UseSpreadOptions; | ||
|
|
||
| declare_lint_rule! { | ||
| /// Enforce the use of the spread operator over `.apply()`. | ||
| /// | ||
| /// The `apply()` method is used to call a function with a given `this` value and arguments provided as an array. | ||
| /// The spread operator `...` can be used to achieve the same result, which is more concise and easier to read. | ||
| /// | ||
| /// ## Examples | ||
| /// | ||
| /// ### Invalid | ||
| /// | ||
| /// ```js,expect_diagnostic | ||
| /// foo.apply(null, args); | ||
| /// ``` | ||
| /// | ||
| /// ```js,expect_diagnostic | ||
| /// foo.apply(null, [1, 2, 3]); | ||
| /// ``` | ||
| /// | ||
| /// ```js,expect_diagnostic | ||
| /// foo.apply(undefined, args); | ||
| /// ``` | ||
| /// | ||
| /// ```js,expect_diagnostic | ||
| /// obj.foo.apply(obj, args); | ||
| /// ``` | ||
| /// | ||
| /// ### Valid | ||
| /// | ||
| /// ```js | ||
| /// foo(...args); | ||
| /// | ||
| /// obj.foo(...args); | ||
| /// | ||
| /// foo.apply(obj, [1, 2, 3]); | ||
| /// | ||
| /// ``` | ||
| /// | ||
| pub UseSpread { | ||
| version: "next", | ||
| name: "useSpread", | ||
| language: "js", | ||
| sources: &[RuleSource::Eslint("prefer-spread").same()], | ||
| recommended: true, | ||
| fix_kind: FixKind::Unsafe, | ||
| } | ||
| } | ||
|
|
||
| impl Rule for UseSpread { | ||
| type Query = Ast<JsCallExpression>; | ||
| type State = (AnyJsExpression, AnyJsExpression); | ||
| type Signals = Option<Self::State>; | ||
| type Options = UseSpreadOptions; | ||
|
|
||
| fn run(ctx: &RuleContext<Self>) -> Self::Signals { | ||
| let node = ctx.query(); | ||
| let callee = node.callee().ok()?; | ||
|
|
||
| let member_expr = AnyJsMemberExpression::cast_ref(callee.syntax())?; | ||
| if member_expr.member_name()?.text() != "apply" { | ||
| return None; | ||
| } | ||
|
|
||
| let arguments = node.arguments().ok()?.args(); | ||
| if arguments.len() != 2 { | ||
| return None; | ||
| } | ||
|
|
||
| let first_arg = arguments.first()?.ok()?; | ||
| let this_arg = first_arg.as_any_js_expression()?; | ||
|
|
||
| let second_arg = arguments.last()?.ok()?; | ||
| let spread_candidate = second_arg.as_any_js_expression()?; | ||
|
|
||
| let applied_object = member_expr.object().ok()?; | ||
|
|
||
| let is_same_reference = if let Some(object_member) = | ||
| AnyJsMemberExpression::cast(applied_object.clone().into_syntax()) | ||
|
|
||
| { | ||
| are_nodes_equal(&object_member.object().ok()?, this_arg) | ||
| } else { | ||
| // This handles cases like `foo.apply(null, args)` or `foo.apply(undefined, args)` | ||
| this_arg | ||
| .as_static_value() | ||
| .is_some_and(|v| v.is_null_or_undefined()) | ||
| }; | ||
|
|
||
| if is_same_reference { | ||
| Some((applied_object.clone(), spread_candidate.clone())) | ||
| } else { | ||
| None | ||
| } | ||
| } | ||
|
|
||
| fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> { | ||
| let node = ctx.query(); | ||
| Some(RuleDiagnostic::new( | ||
| rule_category!(), | ||
| node.range(), | ||
| markup! { | ||
| <Emphasis>"apply()"</Emphasis>" is used to call a function with arguments provided as an array." | ||
| }, | ||
| )) | ||
| } | ||
|
|
||
| fn action(ctx: &RuleContext<Self>, state: &Self::State) -> Option<JsRuleAction> { | ||
| let node = ctx.query(); | ||
| let mut mutation = ctx.root().begin(); | ||
|
|
||
| let (object, spread_candidate) = state; | ||
|
|
||
| let new_arguments = make::js_call_arguments( | ||
| make::token(T!['(']), | ||
| make::js_call_argument_list( | ||
| [AnyJsCallArgument::from(make::js_spread( | ||
| make::token(T![...]), | ||
| spread_candidate.clone(), | ||
| ))], | ||
| [], | ||
| ), | ||
| make::token(T![')']), | ||
| ); | ||
|
|
||
| let new_call_expression = make::js_call_expression(object.clone(), new_arguments).build(); | ||
|
|
||
| mutation.replace_node(node.clone(), new_call_expression); | ||
|
|
||
| Some(JsRuleAction::new( | ||
| ctx.metadata().action_category(ctx.category(), ctx.group()), | ||
| ctx.metadata().applicability(), | ||
| markup! { "Use the spread operator." }.to_owned(), | ||
| mutation, | ||
| )) | ||
| } | ||
| } | ||
|
|
||
| fn get_identifier_token(node: &AnyJsExpression) -> Option<SyntaxToken<JsLanguage>> { | ||
| match node { | ||
| AnyJsExpression::JsIdentifierExpression(identifier) => identifier | ||
| .name() | ||
| .ok() | ||
| .and_then(|name| name.value_token().ok()), | ||
| _ => None, | ||
| } | ||
| } | ||
|
|
||
| fn are_nodes_equal(node1: &AnyJsExpression, node2: &AnyJsExpression) -> bool { | ||
| let object_token = get_identifier_token(node1); | ||
| let this_token = get_identifier_token(node2); | ||
|
|
||
| match (object_token, this_token) { | ||
| (Some(object_token), Some(this_token)) => { | ||
| object_token.text_trimmed() == this_token.text_trimmed() | ||
| } | ||
| _ => false, | ||
| } | ||
| } | ||
This file contains hidden or 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,6 @@ | ||
| // should generate diagnostics | ||
|
|
||
| foo.apply(null, args); | ||
| foo.apply(null, [1, 2, 3]); | ||
| foo.apply(undefined, args); | ||
| obj.foo.apply(obj, args); | ||
|
Comment on lines
+3
to
+6
There was a problem hiding this comment. 🛠️ Refactor suggestion | 🟠 Major ❓ Verification inconclusiveTest coverage looks solid. The four test cases cover the main patterns: Since a past review mentioned parentheses breaking detection (supposedly addressed in 6cbc9ad), consider verifying that edge cases like 🏁 Script executed: #!/bin/bash
# Description: Check if parenthesized expression test cases exist
# Expected: Find test cases with parentheses in apply calls
rg -n -A2 -B2 '\(.*\)\.apply' crates/biome_js_analyze/tests/specs/nursery/useSpread/Length of output: 0 Add tests for parenthesized No existing specs cover calls like 🤖 Prompt for AI Agents |
||
This file contains hidden or 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,108 @@ | ||
| --- | ||
| source: crates/biome_js_analyze/tests/spec_tests.rs | ||
| expression: invalid.js | ||
| --- | ||
| # Input | ||
| ```js | ||
| // should generate diagnostics | ||
|
|
||
| foo.apply(null, args); | ||
| foo.apply(null, [1, 2, 3]); | ||
| foo.apply(undefined, args); | ||
| obj.foo.apply(obj, args); | ||
|
|
||
| ``` | ||
|
|
||
| # Diagnostics | ||
| ``` | ||
| invalid.js:3:1 lint/nursery/useSpread FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| i apply() is used to call a function with arguments provided as an array. | ||
|
|
||
| 1 │ // should generate diagnostics | ||
| 2 │ | ||
| > 3 │ foo.apply(null, args); | ||
| │ ^^^^^^^^^^^^^^^^^^^^^ | ||
| 4 │ foo.apply(null, [1, 2, 3]); | ||
| 5 │ foo.apply(undefined, args); | ||
|
|
||
| i Unsafe fix: Use the spread operator. | ||
|
|
||
| 1 1 │ // should generate diagnostics | ||
| 2 2 │ | ||
| 3 │ - foo.apply(null,·args); | ||
| 3 │ + foo(...args); | ||
| 4 4 │ foo.apply(null, [1, 2, 3]); | ||
| 5 5 │ foo.apply(undefined, args); | ||
|
|
||
|
|
||
| ``` | ||
|
|
||
| ``` | ||
| invalid.js:4:1 lint/nursery/useSpread FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| i apply() is used to call a function with arguments provided as an array. | ||
|
|
||
| 3 │ foo.apply(null, args); | ||
| > 4 │ foo.apply(null, [1, 2, 3]); | ||
| │ ^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| 5 │ foo.apply(undefined, args); | ||
| 6 │ obj.foo.apply(obj, args); | ||
|
|
||
| i Unsafe fix: Use the spread operator. | ||
|
|
||
| 2 2 │ | ||
| 3 3 │ foo.apply(null, args); | ||
| 4 │ - foo.apply(null,·[1,·2,·3]); | ||
| 4 │ + foo(...[1,·2,·3]); | ||
| 5 5 │ foo.apply(undefined, args); | ||
| 6 6 │ obj.foo.apply(obj, args); | ||
|
|
||
|
|
||
| ``` | ||
|
|
||
| ``` | ||
| invalid.js:5:1 lint/nursery/useSpread FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| i apply() is used to call a function with arguments provided as an array. | ||
|
|
||
| 3 │ foo.apply(null, args); | ||
| 4 │ foo.apply(null, [1, 2, 3]); | ||
| > 5 │ foo.apply(undefined, args); | ||
| │ ^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| 6 │ obj.foo.apply(obj, args); | ||
| 7 │ | ||
|
|
||
| i Unsafe fix: Use the spread operator. | ||
|
|
||
| 3 3 │ foo.apply(null, args); | ||
| 4 4 │ foo.apply(null, [1, 2, 3]); | ||
| 5 │ - foo.apply(undefined,·args); | ||
| 5 │ + foo(...args); | ||
| 6 6 │ obj.foo.apply(obj, args); | ||
| 7 7 │ | ||
|
|
||
|
|
||
| ``` | ||
|
|
||
| ``` | ||
| invalid.js:6:1 lint/nursery/useSpread FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| i apply() is used to call a function with arguments provided as an array. | ||
|
|
||
| 4 │ foo.apply(null, [1, 2, 3]); | ||
| 5 │ foo.apply(undefined, args); | ||
| > 6 │ obj.foo.apply(obj, args); | ||
| │ ^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| 7 │ | ||
|
|
||
| i Unsafe fix: Use the spread operator. | ||
|
|
||
| 4 4 │ foo.apply(null, [1, 2, 3]); | ||
| 5 5 │ foo.apply(undefined, args); | ||
| 6 │ - obj.foo.apply(obj,·args); | ||
| 6 │ + obj.foo(...args); | ||
| 7 7 │ | ||
|
|
||
|
|
||
| ``` |
This file contains hidden or 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,10 @@ | ||
| /* should not generate diagnostics */ | ||
|
|
||
| // The `this` argument is a specific object, which is not the same as the callee's object. | ||
| foo.apply(obj, [1, 2, 3]); | ||
| foo.apply(obj, args); | ||
| obj.foo.apply(bar, args); | ||
|
|
||
| // The number of arguments is not 2. | ||
| foo.apply(null); | ||
| foo.apply(); |
This file contains hidden or 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,18 @@ | ||
| --- | ||
| source: crates/biome_js_analyze/tests/spec_tests.rs | ||
| expression: valid.js | ||
| --- | ||
| # Input | ||
| ```js | ||
| /* should not generate diagnostics */ | ||
|
|
||
| // The `this` argument is a specific object, which is not the same as the callee's object. | ||
| foo.apply(obj, [1, 2, 3]); | ||
| foo.apply(obj, args); | ||
| obj.foo.apply(bar, args); | ||
|
|
||
| // The number of arguments is not 2. | ||
| foo.apply(null); | ||
| foo.apply(); | ||
|
|
||
| ``` |
This file contains hidden or 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 hidden or 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,6 @@ | ||
| use biome_deserialize_macros::Deserializable; | ||
| use serde::{Deserialize, Serialize}; | ||
| #[derive(Default, Clone, Debug, Deserialize, Deserializable, Eq, PartialEq, Serialize)] | ||
| #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] | ||
| #[serde(rename_all = "camelCase", deny_unknown_fields, default)] | ||
| pub struct UseSpreadOptions {} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.