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

perf(linter) reduce the number of diagnostics for no_sparse_arrays (#1895)

Browse source 
Partially address #1879

See there for more info. but tldr:

Printing a lot of diagnostics takes a lot of time - a lot longer than it
takes to create them

we speed up printing by producing less diagnostics.

we do this by making `no-sparse-arrays` report 1x per array instead of
1x per violation.

This does have the overhead of now every ArrayElement is processed 2x
(1x when we loop through in the rule, 1x when we loop through the ast).
But the perf reduction is negligble compared to the per production of
producing thousands of diagnostics vs 1 diagnostic.

This means that linting nodejs/node now takes ~1.5 seconds vs ~2:30
previously
This commit is contained in:
Cameron 2024-01-05 02:52:31 +00:00 committed by GitHub
parent 5f29e3f9f6
commit 064296cd43
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 106 additions and 8 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

65
crates/oxc_linter/src/rules/eslint/no_sparse_arrays.rs
View file

@ -1,3 +1,4 @@
use miette::{miette, LabeledSpan};
use oxc_ast::{ast::ArrayExpressionElement, AstKind};
use oxc_diagnostics::{
miette::{self, Diagnostic},
@ -36,9 +37,47 @@ declare_oxc_lint!(
impl Rule for NoSparseArrays {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
if let AstKind::ArrayExpressionElement(ArrayExpressionElement::Elision(span)) = node.kind()
{
ctx.diagnostic(NoSparseArraysDiagnostic(Span::new(span.start, span.start)));
if let AstKind::ArrayExpression(array_expr) = node.kind() {
let violations = array_expr
.elements
.iter()
.filter_map(|el| match el {
ArrayExpressionElement::Elision(span) => Some(span),
_ => None,
})
.map(|span| {
LabeledSpan::at(
(span.start as usize)..(span.start as usize),
"unexpected comma",
)
})
.collect::<Vec<_>>();
if !violations.is_empty() {
if violations.len() < 10 {
ctx.diagnostic(miette!(
labels = violations,
help = "remove the comma or insert `undefined`",
"eslint(no-sparse-arrays): Unexpected comma in middle of array"
));
} else {
let span = if (array_expr.span.end - array_expr.span.start) < 50 {
LabeledSpan::at(array_expr.span, "the array here")
} else {
LabeledSpan::at(
(array_expr.span.start as usize)..(array_expr.span.start as usize),
"the array starting here",
)
};
ctx.diagnostic(miette!(
labels = vec![span],
help = "remove the comma or insert `undefined`",
"eslint(no-sparse-arrays): {} unexpected commas in middle of array",
violations.len()
));
}
}
}
}
}
@ -47,9 +86,23 @@ impl Rule for NoSparseArrays {
fn test() {
use crate::tester::Tester;
let pass = vec![("var a = [ 1, 2, ]", None)];
let pass = vec!["var a = [ 1, 2, ]"];
let fail = vec![("var a = [,];", None), ("var a = [ 1,, 2];", None)];
let fail = vec![
"var a = [,];",
"var a = [ 1,, 2];",
"var a = [ 1,,,, 2];",
"var a = [ 1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,, 2];",
"var a = [ 1, , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , 2];",
"var a = [ 1, , , , , , , , , , , , , , , , , , , , , , , , , , hello, , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , 2];",
"var a = [ 1, , , , , , , , , , , , , , , , , , , , , , , , , ,
hello, , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , ,
, , , , , , , , , , , , , , , , , , , 2];",
];
Tester::new(NoSparseArrays::NAME, pass, fail).test_and_snapshot();
Tester::new_without_config(NoSparseArrays::NAME, pass, fail).test_and_snapshot();
}

49
crates/oxc_linter/src/snapshots/no_sparse_arrays.snap
View file

@ -2,17 +2,62 @@
source: crates/oxc_linter/src/tester.rs
expression: no_sparse_arrays
---
⚠ eslint(no-sparse-arrays): Unexpected comma in middle of array
× eslint(no-sparse-arrays): Unexpected comma in middle of array
╭─[no_sparse_arrays.tsx:1:1]
1 │ var a = [,];
· ▲
· ╰── unexpected comma
╰────
help: remove the comma or insert `undefined`
⚠ eslint(no-sparse-arrays): Unexpected comma in middle of array
× eslint(no-sparse-arrays): Unexpected comma in middle of array
╭─[no_sparse_arrays.tsx:1:1]
1 │ var a = [ 1,, 2];
· ▲
· ╰── unexpected comma
╰────
help: remove the comma or insert `undefined`
× eslint(no-sparse-arrays): Unexpected comma in middle of array
╭─[no_sparse_arrays.tsx:1:1]
1 │ var a = [ 1,,,, 2];
· ▲▲▲
· ││╰── unexpected comma
· │╰── unexpected comma
· ╰── unexpected comma
╰────
help: remove the comma or insert `undefined`
× eslint(no-sparse-arrays): 30 unexpected commas in middle of array
╭─[no_sparse_arrays.tsx:1:1]
1 │ var a = [ 1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,, 2];
· ──────────────────┬──────────────────
· ╰── the array here
╰────
help: remove the comma or insert `undefined`
× eslint(no-sparse-arrays): 83 unexpected commas in middle of array
╭─[no_sparse_arrays.tsx:1:1]
1 │ var a = [ 1, , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , 2];
· ▲
· ╰── the array starting here
╰────
help: remove the comma or insert `undefined`
× eslint(no-sparse-arrays): 82 unexpected commas in middle of array
╭─[no_sparse_arrays.tsx:1:1]
1 │ var a = [ 1, , , , , , , , , , , , , , , , , , , , , , , , , , hello, , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , 2];
· ▲
· ╰── the array starting here
╰────
help: remove the comma or insert `undefined`
× eslint(no-sparse-arrays): 81 unexpected commas in middle of array
╭─[no_sparse_arrays.tsx:1:1]
1 │ var a = [ 1, , , , , , , , , , , , , , , , , , , , , , , , , ,
· ▲
· ╰── the array starting here
2 │
╰────
help: remove the comma or insert `undefined`