dan/oxc
1
0
Fork 0
mirror of https://github.com/danbulant/oxc synced 2026-05-24 12:21:58 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

refactor(linter/oxc): improve diagnostic for no-accumulating-spread in loops (#5374)

Browse source 
when reporting diagnotics for code such as

```ts
let foo = {};
for (let i = 0; i < 10; i++) {
    foo = { ...foo, [i]: i };
}
```

we do not currently report **where** the accumulator is defined.
since this is constant for `Array.prototype.reduce`, it is not necessary.
however for loops, it makes sense to add this span to clearly show the user where the accumator is defined.
This commit is contained in:
camc314 2024-08-31 20:52:05 +00:00
parent 024b58506e
commit 69493d2987
2 changed files with 83 additions and 60 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
crates/oxc_linter/src/rules/oxc/no_accumulating_spread.rs
View file

@ -44,10 +44,15 @@ fn reduce_unknown(spread_span: Span, reduce_span: Span) -> OxcDiagnostic {
]) ])
} }
fn loop_spread_diagnostic(spread_span: Span, loop_span: Span) -> OxcDiagnostic { fn loop_spread_diagnostic(
accumulator_decl_span: Span,
spread_span: Span,
loop_span: Span,
) -> OxcDiagnostic {
OxcDiagnostic::warn("Do not spread accumulators in loops") OxcDiagnostic::warn("Do not spread accumulators in loops")
.with_help("Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead.\nUsing spreads within accumulators leads to `O(n^2)` time complexity.") .with_help("Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead.\nUsing spreads within accumulators leads to `O(n^2)` time complexity.")
.with_labels([ .with_labels([
accumulator_decl_span.label("From this accumulator"),
spread_span.label("From this spread"), spread_span.label("From this spread"),
loop_span.label("For this loop") loop_span.label("For this loop")
]) ])
@ -191,9 +196,9 @@ fn check_loop_usage<'a>(
return; return;
} }
if !matches!(declarator.kind(), AstKind::VariableDeclarator(_)) { let AstKind::VariableDeclarator(declarator) = declarator.kind() else {
return; return;
} };
let Some(write_reference) = let Some(write_reference) =
ctx.semantic().symbol_references(referenced_symbol_id).find(|r| r.is_write()) ctx.semantic().symbol_references(referenced_symbol_id).find(|r| r.is_write())
@ -229,7 +234,11 @@ fn check_loop_usage<'a>(
if !parent.kind().span().contains_inclusive(declaration.span) if !parent.kind().span().contains_inclusive(declaration.span)
&& parent.kind().span().contains_inclusive(spread_span) && parent.kind().span().contains_inclusive(spread_span)
{ {
ctx.diagnostic(loop_spread_diagnostic(spread_span, loop_span)); ctx.diagnostic(loop_spread_diagnostic(
declarator.id.span(),
spread_span,
loop_span,
));
} }
} }
} }

126
crates/oxc_linter/src/snapshots/no_accumulating_spread.snap
View file

@ -315,141 +315,155 @@ source: crates/oxc_linter/src/tester.rs
Using spreads within accumulators leads to `O(n^2)` time complexity. Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops ⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
╭─[no_accumulating_spread.tsx:1:15] ╭─[no_accumulating_spread.tsx:1:5]
1 │ let foo = []; for (let i = 0; i < 10; i++) { foo = [...foo, i]; } 1 │ let foo = []; for (let i = 0; i < 10; i++) { foo = [...foo, i]; }
· ─┬─ ───┬── · ─┬─ ─┬─ ───┬──
· │ ╰── From this spread · │ │ ╰── From this spread
· ╰── For this loop · │ ╰── For this loop
· ╰── From this accumulator
╰──── ╰────
help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead. help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity. Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops ⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
╭─[no_accumulating_spread.tsx:1:15] ╭─[no_accumulating_spread.tsx:1:5]
1 │ let foo = []; for (const i = 0; i < 10; i++) { foo = [...foo, i]; } 1 │ let foo = []; for (const i = 0; i < 10; i++) { foo = [...foo, i]; }
· ─┬─ ───┬── · ─┬─ ─┬─ ───┬──
· │ ╰── From this spread · │ │ ╰── From this spread
· ╰── For this loop · │ ╰── For this loop
· ╰── From this accumulator
╰──── ╰────
help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead. help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity. Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops ⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
╭─[no_accumulating_spread.tsx:1:15] ╭─[no_accumulating_spread.tsx:1:5]
1 │ let foo = []; for (let i in [1,2,3]) { foo = [...foo, i]; } 1 │ let foo = []; for (let i in [1,2,3]) { foo = [...foo, i]; }
· ─┬─ ───┬── · ─┬─ ─┬─ ───┬──
· │ ╰── From this spread · │ │ ╰── From this spread
· ╰── For this loop · │ ╰── For this loop
· ╰── From this accumulator
╰──── ╰────
help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead. help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity. Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops ⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
╭─[no_accumulating_spread.tsx:1:15] ╭─[no_accumulating_spread.tsx:1:5]
1 │ let foo = []; for (const i in [1,2,3]) { foo = [...foo, i]; } 1 │ let foo = []; for (const i in [1,2,3]) { foo = [...foo, i]; }
· ─┬─ ───┬── · ─┬─ ─┬─ ───┬──
· │ ╰── From this spread · │ │ ╰── From this spread
· ╰── For this loop · │ ╰── For this loop
· ╰── From this accumulator
╰──── ╰────
help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead. help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity. Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops ⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
╭─[no_accumulating_spread.tsx:1:15] ╭─[no_accumulating_spread.tsx:1:5]
1 │ let foo = []; for (let i of [1,2,3]) { foo = [...foo, i]; } 1 │ let foo = []; for (let i of [1,2,3]) { foo = [...foo, i]; }
· ─┬─ ───┬── · ─┬─ ─┬─ ───┬──
· │ ╰── From this spread · │ │ ╰── From this spread
· ╰── For this loop · │ ╰── For this loop
· ╰── From this accumulator
╰──── ╰────
help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead. help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity. Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops ⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
╭─[no_accumulating_spread.tsx:1:15] ╭─[no_accumulating_spread.tsx:1:5]
1 │ let foo = []; for (const i of [1,2,3]) { foo = [...foo, i]; } 1 │ let foo = []; for (const i of [1,2,3]) { foo = [...foo, i]; }
· ─┬─ ───┬── · ─┬─ ─┬─ ───┬──
· │ ╰── From this spread · │ │ ╰── From this spread
· ╰── For this loop · │ ╰── For this loop
· ╰── From this accumulator
╰──── ╰────
help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead. help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity. Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops ⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
╭─[no_accumulating_spread.tsx:1:15] ╭─[no_accumulating_spread.tsx:1:5]
1 │ let foo = []; while (foo.length < 10) { foo = [...foo, foo.length]; } 1 │ let foo = []; while (foo.length < 10) { foo = [...foo, foo.length]; }
· ──┬── ───┬── · ─┬─ ──┬── ───┬──
· │ ╰── From this spread · │ │ ╰── From this spread
· ╰── For this loop · │ ╰── For this loop
· ╰── From this accumulator
╰──── ╰────
help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead. help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity. Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops ⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
╭─[no_accumulating_spread.tsx:1:15] ╭─[no_accumulating_spread.tsx:1:5]
1 │ let foo = {}; for (let i = 0; i < 10; i++) { foo = { ...foo, [i]: i }; } 1 │ let foo = {}; for (let i = 0; i < 10; i++) { foo = { ...foo, [i]: i }; }
· ─┬─ ───┬── · ─┬─ ─┬─ ───┬──
· │ ╰── From this spread · │ │ ╰── From this spread
· ╰── For this loop · │ ╰── For this loop
· ╰── From this accumulator
╰──── ╰────
help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead. help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity. Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops ⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
╭─[no_accumulating_spread.tsx:1:15] ╭─[no_accumulating_spread.tsx:1:5]
1 │ let foo = {}; for (const i = 0; i < 10; i++) { foo = { ...foo, [i]: i }; } 1 │ let foo = {}; for (const i = 0; i < 10; i++) { foo = { ...foo, [i]: i }; }
· ─┬─ ───┬── · ─┬─ ─┬─ ───┬──
· │ ╰── From this spread · │ │ ╰── From this spread
· ╰── For this loop · │ ╰── For this loop
· ╰── From this accumulator
╰──── ╰────
help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead. help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity. Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops ⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
╭─[no_accumulating_spread.tsx:1:15] ╭─[no_accumulating_spread.tsx:1:5]
1 │ let foo = {}; for (let i in [1,2,3]) { foo = { ...foo, [i]: i }; } 1 │ let foo = {}; for (let i in [1,2,3]) { foo = { ...foo, [i]: i }; }
· ─┬─ ───┬── · ─┬─ ─┬─ ───┬──
· │ ╰── From this spread · │ │ ╰── From this spread
· ╰── For this loop · │ ╰── For this loop
· ╰── From this accumulator
╰──── ╰────
help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead. help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity. Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops ⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
╭─[no_accumulating_spread.tsx:1:15] ╭─[no_accumulating_spread.tsx:1:5]
1 │ let foo = {}; for (const i in [1,2,3]) { foo = { ...foo, [i]: i }; } 1 │ let foo = {}; for (const i in [1,2,3]) { foo = { ...foo, [i]: i }; }
· ─┬─ ───┬── · ─┬─ ─┬─ ───┬──
· │ ╰── From this spread · │ │ ╰── From this spread
· ╰── For this loop · │ ╰── For this loop
· ╰── From this accumulator
╰──── ╰────
help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead. help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity. Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops ⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
╭─[no_accumulating_spread.tsx:1:15] ╭─[no_accumulating_spread.tsx:1:5]
1 │ let foo = {}; for (let i of [1,2,3]) { foo = { ...foo, [i]: i }; } 1 │ let foo = {}; for (let i of [1,2,3]) { foo = { ...foo, [i]: i }; }
· ─┬─ ───┬── · ─┬─ ─┬─ ───┬──
· │ ╰── From this spread · │ │ ╰── From this spread
· ╰── For this loop · │ ╰── For this loop
· ╰── From this accumulator
╰──── ╰────
help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead. help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity. Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops ⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
╭─[no_accumulating_spread.tsx:1:15] ╭─[no_accumulating_spread.tsx:1:5]
1 │ let foo = {}; for (const i of [1,2,3]) { foo = { ...foo, [i]: i }; } 1 │ let foo = {}; for (const i of [1,2,3]) { foo = { ...foo, [i]: i }; }
· ─┬─ ───┬── · ─┬─ ─┬─ ───┬──
· │ ╰── From this spread · │ │ ╰── From this spread
· ╰── For this loop · │ ╰── For this loop
· ╰── From this accumulator
╰──── ╰────
help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead. help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity. Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops ⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
╭─[no_accumulating_spread.tsx:1:15] ╭─[no_accumulating_spread.tsx:1:5]
1 │ let foo = {}; while (Object.keys(foo).length < 10) { foo = { ...foo, [Object.keys(foo).length]: Object.keys(foo).length }; } 1 │ let foo = {}; while (Object.keys(foo).length < 10) { foo = { ...foo, [Object.keys(foo).length]: Object.keys(foo).length }; }
· ──┬── ───┬── · ─┬─ ──┬── ───┬──
· │ ╰── From this spread · │ │ ╰── From this spread
· ╰── For this loop · │ ╰── For this loop
· ╰── From this accumulator
╰──── ╰────
help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead. help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity. Using spreads within accumulators leads to `O(n^2)` time complexity.