Skip to content

Commit bb7a15c

Browse files
emilyinuredyc3
andauthored
fix: only flag object assign in accumulators, if it is allocating a new object for each iter (#7078)
Co-authored-by: Carson McManus <[email protected]>
1 parent e9f068e commit bb7a15c

File tree

5 files changed

+217
-71
lines changed

5 files changed

+217
-71
lines changed

.changeset/wet-jeans-repair.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
"@biomejs/biome": patch
3+
---
4+
5+
Fixed [#6675](https://github.com/biomejs/biome/issues/6675): Now only flags
6+
noAccumulatingSpread on Object.assign when a new object is being allocated on
7+
each iteration. Before, all cases using Object.assign with reduce parameters
8+
were warned despite not making new allocations.
9+
10+
The following code will no longer be a false positive:
11+
12+
```js
13+
foo.reduce((acc, bar) => Object.assign(acc, bar), {})
14+
```
15+
16+
The following cases which **do** make new allocations will continue to warn:
17+
18+
```js
19+
foo.reduce((acc, bar) => Object.assign({}, acc, bar), {})
20+
```

crates/biome_js_analyze/src/lint/performance/no_accumulating_spread.rs

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,19 +43,24 @@ declare_lint_rule! {
4343
///
4444
/// ```js,expect_diagnostic
4545
/// var a = ['a', 'b', 'c'];
46-
/// a.reduce((acc, val) => Object.assign(acc, val), []);
46+
/// a.reduce((acc, val) => {return Object.assign([], acc, val);}, []);
4747
/// ```
4848
///
49-
/// ```js,expect_diagnostic
49+
/// ### Valid
50+
///
51+
/// ```js
5052
/// var a = ['a', 'b', 'c'];
51-
/// a.reduce((acc, val) => {return Object.assign(acc, val);}, []);
53+
/// a.reduce((acc, val) => {acc.push(val); return acc}, []);
5254
/// ```
5355
///
54-
/// ### Valid
56+
/// ```js
57+
/// var a = ['a', 'b', 'c'];
58+
/// a.reduce((acc, val) => Object.assign(acc, val), []);
59+
/// ```
5560
///
5661
/// ```js
5762
/// var a = ['a', 'b', 'c'];
58-
/// a.reduce((acc, val) => {acc.push(val); return acc}, []);
63+
/// a.reduce((acc, val) => {return Object.assign(acc, val);}, []);
5964
/// ```
6065
///
6166
pub NoAccumulatingSpread {
@@ -200,9 +205,21 @@ fn handle_object_assign(node: &JsStaticMemberExpression, model: &SemanticModel)
200205

201206
let call_expression = node.parent::<JsCallExpression>()?;
202207
let arguments = call_expression.arguments().ok()?;
203-
let reference = arguments
204-
.args()
205-
.first()?
208+
209+
let mut arg_iter = arguments.args().iter();
210+
211+
let first = arg_iter.next()?.ok()?;
212+
let first = first.as_any_js_expression()?;
213+
214+
if first.as_js_object_expression().is_none()
215+
&& first.as_js_array_expression().is_none()
216+
&& first.as_js_new_expression().is_none()
217+
{
218+
return None;
219+
}
220+
221+
let reference = arg_iter
222+
.next()?
206223
.ok()?
207224
.as_any_js_expression()?
208225
.as_js_identifier_expression()?

crates/biome_js_analyze/tests/specs/performance/noAccumulatingSpread/invalid.jsonc

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,6 @@
1515
"foo.reduce((acc, bar) => {return [...acc, ...bar];}, [])",
1616
"foo.reduceRight((acc, bar) => {return [...acc, ...bar];}, [])",
1717

18-
// Array - Arrow return with assign
19-
"foo.reduce((acc, bar) => Object.assign(acc, bar), [])",
20-
"foo.reduceRight((acc, bar) => Object.assign(acc, bar), [])",
21-
22-
// Array - Body return with Assign
23-
"foo.reduce((acc, bar) => { return Object.assign(acc, bar) }, [])",
24-
"foo.reduceRight((acc, bar) => { return Object.assign(acc, bar) }, [])",
2518

2619
// Object - Arrow return
2720
"foo.reduce((acc, bar) => ({...acc, [bar.key]: bar.value}), {})",
@@ -39,11 +32,27 @@
3932
"foo.reduce((acc, bar) => {return {...acc, ...bar};}, {})",
4033
"foo.reduceRight((acc, bar) => {return {...acc, ...bar};}, {})",
4134

35+
// Array - Arrow return with assign
36+
"foo.reduce((acc, bar) => Object.assign({}, acc, bar), [])",
37+
"foo.reduceRight((acc, bar) => Object.assign({}, acc, bar), [])",
38+
39+
// Array - Body return with Assign
40+
"foo.reduce((acc, bar) => { return Object.assign([], acc, bar) }, [])",
41+
"foo.reduceRight((acc, bar) => { return Object.assign([], acc, bar) }, [])",
42+
4243
// Object - Arrow return with assign
43-
"foo.reduce((acc, bar) => Object.assign(acc, bar), {})",
44-
"foo.reduceRight((acc, bar) => Object.assign(acc, bar), {})",
44+
"foo.reduce((acc, bar) => Object.assign([], acc, bar), {})",
45+
"foo.reduceRight((acc, bar) => Object.assign([], acc, bar), {})",
4546

4647
// Object - Body return with Assign
47-
"foo.reduce((acc, bar) => { return Object.assign(acc, bar) }, {})",
48-
"foo.reduceRight((acc, bar) => { return Object.assign(acc, bar) }, {})"
48+
"foo.reduce((acc, bar) => { return Object.assign([], acc, bar) }, {})",
49+
"foo.reduceRight((acc, bar) => { return Object.assign([], acc, bar) }, {})",
50+
51+
// Array - Body return with Assign new
52+
"foo.reduce((acc, bar) => { return Object.assign(new Array(1), acc, bar) }, [])",
53+
"foo.reduceRight((acc, bar) => { return Object.assign(new Array(1), acc, bar) }, [])",
54+
55+
// Object - Arrow return with assign new
56+
"foo.reduce((acc, bar) => Object.assign(new Array(1), acc, bar), {})",
57+
"foo.reduceRight((acc, bar) => Object.assign(new Array(1), acc, bar), {})"
4958
]

0 commit comments

Comments
 (0)