mirror of
https://github.com/danbulant/oxc
synced 2026-05-19 04:08:41 +00:00
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:
parent
69493d2987
commit
8d781e7dff
2 changed files with 49 additions and 22 deletions
|
|
@ -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!(),
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
Loading…
Reference in a new issue