feat(linter/oxc): differentiate between array/object in no-accumulating-spread loop diagnostic (#5375)

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 differentiate the diagnostic message between object/array.
this PR changes this to help the user fix ths issue
This commit is contained in:
camc314 2024-08-31 20:52:05 +00:00
parent 69493d2987
commit 8d781e7dff
2 changed files with 49 additions and 22 deletions

View file

@ -44,13 +44,26 @@ fn reduce_unknown(spread_span: Span, reduce_span: Span) -> OxcDiagnostic {
])
}
fn loop_spread_diagnostic(
fn loop_spread_likely_object_diagnostic(
accumulator_decl_span: Span,
spread_span: Span,
loop_span: Span,
) -> OxcDiagnostic {
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()` to mutate the accumulator instead.\nUsing spreads within accumulators leads to `O(n^2)` time complexity.")
.with_labels([
accumulator_decl_span.label("From this accumulator"),
spread_span.label("From this spread"),
loop_span.label("For this loop")
])
}
fn loop_spread_likely_array_diagnostic(
accumulator_decl_span: Span,
spread_span: Span,
loop_span: Span,
) -> OxcDiagnostic {
OxcDiagnostic::warn("Do not spread accumulators in loops")
.with_help("Consider using `Array.prototype.push()` to mutate the accumulator instead.\nUsing spreads within accumulators leads to `O(n^2)` time complexity.")
.with_labels([
accumulator_decl_span.label("From this accumulator"),
spread_span.label("From this spread"),
@ -221,7 +234,8 @@ fn check_loop_usage<'a>(
return;
};
match assignment_expression.right.get_inner_expression() {
let assignment_expression_right_inner_expr = assignment_expression.right.get_inner_expression();
match assignment_expression_right_inner_expr {
Expression::ArrayExpression(array_expr)
if array_expr.span.contains_inclusive(spread_span) => {}
Expression::ObjectExpression(object_expr)
@ -234,11 +248,24 @@ fn check_loop_usage<'a>(
if !parent.kind().span().contains_inclusive(declaration.span)
&& parent.kind().span().contains_inclusive(spread_span)
{
ctx.diagnostic(loop_spread_diagnostic(
declarator.id.span(),
spread_span,
loop_span,
));
match assignment_expression_right_inner_expr {
Expression::ArrayExpression(_) => {
ctx.diagnostic(loop_spread_likely_array_diagnostic(
declarator.id.span(),
spread_span,
loop_span,
));
}
Expression::ObjectExpression(_) => {
ctx.diagnostic(loop_spread_likely_object_diagnostic(
declarator.id.span(),
spread_span,
loop_span,
));
}
// we check above that the expression is either an array or object expression
_ => unreachable!(),
}
}
}
}

View file

@ -322,7 +322,7 @@ source: crates/oxc_linter/src/tester.rs
· │ ╰── For this loop
· ╰── From this accumulator
╰────
help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead.
help: Consider using `Array.prototype.push()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
@ -333,7 +333,7 @@ source: crates/oxc_linter/src/tester.rs
· │ ╰── For this loop
· ╰── From this accumulator
╰────
help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead.
help: Consider using `Array.prototype.push()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
@ -344,7 +344,7 @@ source: crates/oxc_linter/src/tester.rs
· │ ╰── For this loop
· ╰── From this accumulator
╰────
help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead.
help: Consider using `Array.prototype.push()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
@ -355,7 +355,7 @@ source: crates/oxc_linter/src/tester.rs
· │ ╰── For this loop
· ╰── From this accumulator
╰────
help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead.
help: Consider using `Array.prototype.push()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
@ -366,7 +366,7 @@ source: crates/oxc_linter/src/tester.rs
· │ ╰── For this loop
· ╰── From this accumulator
╰────
help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead.
help: Consider using `Array.prototype.push()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
@ -377,7 +377,7 @@ source: crates/oxc_linter/src/tester.rs
· │ ╰── For this loop
· ╰── From this accumulator
╰────
help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead.
help: Consider using `Array.prototype.push()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
@ -388,7 +388,7 @@ source: crates/oxc_linter/src/tester.rs
· │ ╰── For this loop
· ╰── From this accumulator
╰────
help: Consider using `Object.assign()` or `Array.prototype.push()` to mutate the accumulator instead.
help: Consider using `Array.prototype.push()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
@ -399,7 +399,7 @@ source: crates/oxc_linter/src/tester.rs
· │ ╰── 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()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
@ -410,7 +410,7 @@ source: crates/oxc_linter/src/tester.rs
· │ ╰── 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()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
@ -421,7 +421,7 @@ source: crates/oxc_linter/src/tester.rs
· │ ╰── 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()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
@ -432,7 +432,7 @@ source: crates/oxc_linter/src/tester.rs
· │ ╰── 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()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
@ -443,7 +443,7 @@ source: crates/oxc_linter/src/tester.rs
· │ ╰── 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()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
@ -454,7 +454,7 @@ source: crates/oxc_linter/src/tester.rs
· │ ╰── 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()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity.
⚠ oxc(no-accumulating-spread): Do not spread accumulators in loops
@ -465,5 +465,5 @@ source: crates/oxc_linter/src/tester.rs
· │ ╰── 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()` to mutate the accumulator instead.
Using spreads within accumulators leads to `O(n^2)` time complexity.