Skip to content

Commit b406db6

Browse files
kedevkedematipico
andauthored
feat: add useSpread rule (#7681)
Co-authored-by: Emanuele Stoppa <[email protected]>
1 parent 4872ca3 commit b406db6

File tree

14 files changed

+440
-21
lines changed

14 files changed

+440
-21
lines changed

.changeset/proud-bananas-smoke.md

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
---
2+
"@biomejs/biome": patch
3+
---
4+
5+
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).
6+
7+
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+).
8+
9+
The rule provides a safe fix.
10+
11+
#### Invalid
12+
13+
```js
14+
Math.max.apply(Math, args);
15+
foo.apply(undefined, args);
16+
obj.method.apply(obj, args);
17+
```
18+
19+
#### Valid
20+
21+
```js
22+
Math.max(...args);
23+
foo(...args);
24+
obj.method(...args);
25+
26+
// Allowed: cases where the `this` binding is intentionally changed
27+
foo.apply(otherObj, args);
28+
```

crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs

Lines changed: 12 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/biome_configuration/src/analyzer/linter/rules.rs

Lines changed: 42 additions & 20 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/biome_diagnostics_categories/src/categories.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,7 @@ define_categories! {
209209
"lint/nursery/useQwikMethodUsage": "https://biomejs.dev/linter/rules/use-qwik-method-usage",
210210
"lint/nursery/useQwikValidLexicalScope": "https://biomejs.dev/linter/rules/use-qwik-valid-lexical-scope",
211211
"lint/nursery/useSortedClasses": "https://biomejs.dev/linter/rules/use-sorted-classes",
212+
"lint/nursery/useSpread": "https://biomejs.dev/linter/rules/no-spread",
212213
"lint/nursery/useUniqueGraphqlOperationName": "https://biomejs.dev/linter/rules/use-unique-graphql-operation-name",
213214
"lint/nursery/useVueDefineMacrosOrder": "https://biomejs.dev/linter/rules/use-vue-define-macros-order",
214215
"lint/nursery/useVueMultiWordComponentNames": "https://biomejs.dev/linter/rules/use-vue-multi-word-component-names",

crates/biome_js_analyze/src/lint/nursery.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ pub mod use_max_params;
3434
pub mod use_qwik_method_usage;
3535
pub mod use_qwik_valid_lexical_scope;
3636
pub mod use_sorted_classes;
37+
pub mod use_spread;
3738
pub mod use_vue_define_macros_order;
3839
pub mod use_vue_multi_word_component_names;
39-
declare_lint_group! { pub Nursery { name : "nursery" , rules : [self :: no_continue :: NoContinue , self :: no_deprecated_imports :: NoDeprecatedImports , self :: no_empty_source :: NoEmptySource , self :: no_floating_promises :: NoFloatingPromises , self :: no_for_in :: NoForIn , self :: no_import_cycles :: NoImportCycles , self :: no_increment_decrement :: NoIncrementDecrement , self :: no_jsx_literals :: NoJsxLiterals , self :: no_misused_promises :: NoMisusedPromises , self :: no_next_async_client_component :: NoNextAsyncClientComponent , self :: no_parameters_only_used_in_recursion :: NoParametersOnlyUsedInRecursion , self :: no_react_forward_ref :: NoReactForwardRef , self :: no_shadow :: NoShadow , self :: no_unknown_attribute :: NoUnknownAttribute , self :: no_unnecessary_conditions :: NoUnnecessaryConditions , self :: no_unresolved_imports :: NoUnresolvedImports , self :: no_unused_expressions :: NoUnusedExpressions , self :: no_useless_catch_binding :: NoUselessCatchBinding , self :: no_useless_undefined :: NoUselessUndefined , self :: no_vue_data_object_declaration :: NoVueDataObjectDeclaration , self :: no_vue_duplicate_keys :: NoVueDuplicateKeys , self :: no_vue_reserved_keys :: NoVueReservedKeys , self :: no_vue_reserved_props :: NoVueReservedProps , self :: use_array_sort_compare :: UseArraySortCompare , self :: use_consistent_arrow_return :: UseConsistentArrowReturn , self :: use_exhaustive_switch_cases :: UseExhaustiveSwitchCases , self :: use_explicit_type :: UseExplicitType , self :: use_max_params :: UseMaxParams , self :: use_qwik_method_usage :: UseQwikMethodUsage , self :: use_qwik_valid_lexical_scope :: UseQwikValidLexicalScope , self :: use_sorted_classes :: UseSortedClasses , self :: use_vue_define_macros_order :: UseVueDefineMacrosOrder , self :: use_vue_multi_word_component_names :: UseVueMultiWordComponentNames ,] } }
40+
declare_lint_group! { pub Nursery { name : "nursery" , rules : [self :: no_continue :: NoContinue , self :: no_deprecated_imports :: NoDeprecatedImports , self :: no_empty_source :: NoEmptySource , self :: no_floating_promises :: NoFloatingPromises , self :: no_for_in :: NoForIn , self :: no_import_cycles :: NoImportCycles , self :: no_increment_decrement :: NoIncrementDecrement , self :: no_jsx_literals :: NoJsxLiterals , self :: no_misused_promises :: NoMisusedPromises , self :: no_next_async_client_component :: NoNextAsyncClientComponent , self :: no_parameters_only_used_in_recursion :: NoParametersOnlyUsedInRecursion , self :: no_react_forward_ref :: NoReactForwardRef , self :: no_shadow :: NoShadow , self :: no_unknown_attribute :: NoUnknownAttribute , self :: no_unnecessary_conditions :: NoUnnecessaryConditions , self :: no_unresolved_imports :: NoUnresolvedImports , self :: no_unused_expressions :: NoUnusedExpressions , self :: no_useless_catch_binding :: NoUselessCatchBinding , self :: no_useless_undefined :: NoUselessUndefined , self :: no_vue_data_object_declaration :: NoVueDataObjectDeclaration , self :: no_vue_duplicate_keys :: NoVueDuplicateKeys , self :: no_vue_reserved_keys :: NoVueReservedKeys , self :: no_vue_reserved_props :: NoVueReservedProps , self :: use_array_sort_compare :: UseArraySortCompare , self :: use_consistent_arrow_return :: UseConsistentArrowReturn , self :: use_exhaustive_switch_cases :: UseExhaustiveSwitchCases , self :: use_explicit_type :: UseExplicitType , self :: use_max_params :: UseMaxParams , self :: use_qwik_method_usage :: UseQwikMethodUsage , self :: use_qwik_valid_lexical_scope :: UseQwikValidLexicalScope , self :: use_sorted_classes :: UseSortedClasses , self :: use_spread :: UseSpread , self :: use_vue_define_macros_order :: UseVueDefineMacrosOrder , self :: use_vue_multi_word_component_names :: UseVueMultiWordComponentNames ,] } }
Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
use crate::JsRuleAction;
2+
use biome_analyze::{
3+
Ast, FixKind, Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule,
4+
};
5+
use biome_console::markup;
6+
use biome_js_factory::make;
7+
use biome_js_syntax::{
8+
AnyJsCallArgument, AnyJsExpression, AnyJsMemberExpression, JsCallExpression, JsLanguage, T,
9+
};
10+
use biome_rowan::{AstNode, AstSeparatedList, BatchMutationExt, SyntaxToken};
11+
use biome_rule_options::use_spread::UseSpreadOptions;
12+
13+
declare_lint_rule! {
14+
/// Enforce the use of the spread operator over `.apply()`.
15+
///
16+
/// The `apply()` method is used to call a function with a given `this` value and arguments provided as an array.
17+
/// The spread operator `...` can be used to achieve the same result, which is more concise and easier to read.
18+
///
19+
/// ## Examples
20+
///
21+
/// ### Invalid
22+
///
23+
/// ```js,expect_diagnostic
24+
/// foo.apply(null, args);
25+
/// ```
26+
///
27+
/// ```js,expect_diagnostic
28+
/// foo.apply(null, [1, 2, 3]);
29+
/// ```
30+
///
31+
/// ```js,expect_diagnostic
32+
/// foo.apply(undefined, args);
33+
/// ```
34+
///
35+
/// ```js,expect_diagnostic
36+
/// obj.foo.apply(obj, args);
37+
/// ```
38+
///
39+
/// ### Valid
40+
///
41+
/// ```js
42+
/// foo(...args);
43+
///
44+
/// obj.foo(...args);
45+
///
46+
/// foo.apply(obj, [1, 2, 3]);
47+
///
48+
/// ```
49+
///
50+
pub UseSpread {
51+
version: "next",
52+
name: "useSpread",
53+
language: "js",
54+
sources: &[RuleSource::Eslint("prefer-spread").same()],
55+
recommended: true,
56+
fix_kind: FixKind::Unsafe,
57+
}
58+
}
59+
60+
impl Rule for UseSpread {
61+
type Query = Ast<JsCallExpression>;
62+
type State = (AnyJsExpression, AnyJsExpression);
63+
type Signals = Option<Self::State>;
64+
type Options = UseSpreadOptions;
65+
66+
fn run(ctx: &RuleContext<Self>) -> Self::Signals {
67+
let node = ctx.query();
68+
let callee = node.callee().ok()?;
69+
70+
let member_expr = AnyJsMemberExpression::cast_ref(callee.syntax())?;
71+
if member_expr.member_name()?.text() != "apply" {
72+
return None;
73+
}
74+
75+
let arguments = node.arguments().ok()?.args();
76+
if arguments.len() != 2 {
77+
return None;
78+
}
79+
80+
let first_arg = arguments.first()?.ok()?;
81+
let this_arg = first_arg.as_any_js_expression()?;
82+
83+
let second_arg = arguments.last()?.ok()?;
84+
let spread_candidate = second_arg.as_any_js_expression()?;
85+
86+
let applied_object = member_expr.object().ok()?;
87+
88+
let is_same_reference = if let Some(object_member) =
89+
AnyJsMemberExpression::cast(applied_object.clone().into_syntax())
90+
{
91+
are_nodes_equal(&object_member.object().ok()?, this_arg)
92+
} else {
93+
// This handles cases like `foo.apply(null, args)` or `foo.apply(undefined, args)`
94+
this_arg
95+
.as_static_value()
96+
.is_some_and(|v| v.is_null_or_undefined())
97+
};
98+
99+
if is_same_reference {
100+
Some((applied_object.clone(), spread_candidate.clone()))
101+
} else {
102+
None
103+
}
104+
}
105+
106+
fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> {
107+
let node = ctx.query();
108+
Some(RuleDiagnostic::new(
109+
rule_category!(),
110+
node.range(),
111+
markup! {
112+
<Emphasis>"apply()"</Emphasis>" is used to call a function with arguments provided as an array."
113+
},
114+
))
115+
}
116+
117+
fn action(ctx: &RuleContext<Self>, state: &Self::State) -> Option<JsRuleAction> {
118+
let node = ctx.query();
119+
let mut mutation = ctx.root().begin();
120+
121+
let (object, spread_candidate) = state;
122+
123+
let new_arguments = make::js_call_arguments(
124+
make::token(T!['(']),
125+
make::js_call_argument_list(
126+
[AnyJsCallArgument::from(make::js_spread(
127+
make::token(T![...]),
128+
spread_candidate.clone(),
129+
))],
130+
[],
131+
),
132+
make::token(T![')']),
133+
);
134+
135+
let new_call_expression = make::js_call_expression(object.clone(), new_arguments).build();
136+
137+
mutation.replace_node(node.clone(), new_call_expression);
138+
139+
Some(JsRuleAction::new(
140+
ctx.metadata().action_category(ctx.category(), ctx.group()),
141+
ctx.metadata().applicability(),
142+
markup! { "Use the spread operator." }.to_owned(),
143+
mutation,
144+
))
145+
}
146+
}
147+
148+
fn get_identifier_token(node: &AnyJsExpression) -> Option<SyntaxToken<JsLanguage>> {
149+
match node {
150+
AnyJsExpression::JsIdentifierExpression(identifier) => identifier
151+
.name()
152+
.ok()
153+
.and_then(|name| name.value_token().ok()),
154+
_ => None,
155+
}
156+
}
157+
158+
fn are_nodes_equal(node1: &AnyJsExpression, node2: &AnyJsExpression) -> bool {
159+
let object_token = get_identifier_token(node1);
160+
let this_token = get_identifier_token(node2);
161+
162+
match (object_token, this_token) {
163+
(Some(object_token), Some(this_token)) => {
164+
object_token.text_trimmed() == this_token.text_trimmed()
165+
}
166+
_ => false,
167+
}
168+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// should generate diagnostics
2+
3+
foo.apply(null, args);
4+
foo.apply(null, [1, 2, 3]);
5+
foo.apply(undefined, args);
6+
obj.foo.apply(obj, args);
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
---
2+
source: crates/biome_js_analyze/tests/spec_tests.rs
3+
expression: invalid.js
4+
---
5+
# Input
6+
```js
7+
// should generate diagnostics
8+
9+
foo.apply(null, args);
10+
foo.apply(null, [1, 2, 3]);
11+
foo.apply(undefined, args);
12+
obj.foo.apply(obj, args);
13+
14+
```
15+
16+
# Diagnostics
17+
```
18+
invalid.js:3:1 lint/nursery/useSpread FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
19+
20+
i apply() is used to call a function with arguments provided as an array.
21+
22+
1 │ // should generate diagnostics
23+
2 │
24+
> 3 │ foo.apply(null, args);
25+
│ ^^^^^^^^^^^^^^^^^^^^^
26+
4 │ foo.apply(null, [1, 2, 3]);
27+
5 │ foo.apply(undefined, args);
28+
29+
i Unsafe fix: Use the spread operator.
30+
31+
1 1 │ // should generate diagnostics
32+
2 2 │
33+
3 │ - foo.apply(null,·args);
34+
3 │ + foo(...args);
35+
4 4 │ foo.apply(null, [1, 2, 3]);
36+
5 5 │ foo.apply(undefined, args);
37+
38+
39+
```
40+
41+
```
42+
invalid.js:4:1 lint/nursery/useSpread FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
43+
44+
i apply() is used to call a function with arguments provided as an array.
45+
46+
3 │ foo.apply(null, args);
47+
> 4 │ foo.apply(null, [1, 2, 3]);
48+
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^
49+
5 │ foo.apply(undefined, args);
50+
6 │ obj.foo.apply(obj, args);
51+
52+
i Unsafe fix: Use the spread operator.
53+
54+
2 2 │
55+
3 3 │ foo.apply(null, args);
56+
4 │ - foo.apply(null,·[1,·2,·3]);
57+
4 │ + foo(...[1,·2,·3]);
58+
5 5 │ foo.apply(undefined, args);
59+
6 6 │ obj.foo.apply(obj, args);
60+
61+
62+
```
63+
64+
```
65+
invalid.js:5:1 lint/nursery/useSpread FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
66+
67+
i apply() is used to call a function with arguments provided as an array.
68+
69+
3 │ foo.apply(null, args);
70+
4 │ foo.apply(null, [1, 2, 3]);
71+
> 5 │ foo.apply(undefined, args);
72+
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^
73+
6 │ obj.foo.apply(obj, args);
74+
7 │
75+
76+
i Unsafe fix: Use the spread operator.
77+
78+
3 3 │ foo.apply(null, args);
79+
4 4 │ foo.apply(null, [1, 2, 3]);
80+
5 │ - foo.apply(undefined,·args);
81+
5 │ + foo(...args);
82+
6 6 │ obj.foo.apply(obj, args);
83+
7 7 │
84+
85+
86+
```
87+
88+
```
89+
invalid.js:6:1 lint/nursery/useSpread FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
90+
91+
i apply() is used to call a function with arguments provided as an array.
92+
93+
4 │ foo.apply(null, [1, 2, 3]);
94+
5 │ foo.apply(undefined, args);
95+
> 6 │ obj.foo.apply(obj, args);
96+
│ ^^^^^^^^^^^^^^^^^^^^^^^^
97+
7 │
98+
99+
i Unsafe fix: Use the spread operator.
100+
101+
4 4 │ foo.apply(null, [1, 2, 3]);
102+
5 5 │ foo.apply(undefined, args);
103+
6 │ - obj.foo.apply(obj,·args);
104+
6 │ + obj.foo(...args);
105+
7 7 │
106+
107+
108+
```
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
/* should not generate diagnostics */
2+
3+
// The `this` argument is a specific object, which is not the same as the callee's object.
4+
foo.apply(obj, [1, 2, 3]);
5+
foo.apply(obj, args);
6+
obj.foo.apply(bar, args);
7+
8+
// The number of arguments is not 2.
9+
foo.apply(null);
10+
foo.apply();
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
source: crates/biome_js_analyze/tests/spec_tests.rs
3+
expression: valid.js
4+
---
5+
# Input
6+
```js
7+
/* should not generate diagnostics */
8+
9+
// The `this` argument is a specific object, which is not the same as the callee's object.
10+
foo.apply(obj, [1, 2, 3]);
11+
foo.apply(obj, args);
12+
obj.foo.apply(bar, args);
13+
14+
// The number of arguments is not 2.
15+
foo.apply(null);
16+
foo.apply();
17+
18+
```

0 commit comments

Comments
 (0)