From 064296cd434d7a293e30b40396106f753fb592b3 Mon Sep 17 00:00:00 2001 From: Cameron Date: Fri, 5 Jan 2024 02:52:31 +0000 Subject: [PATCH] perf(linter) reduce the number of diagnostics for no_sparse_arrays (#1895) 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 --- .../src/rules/eslint/no_sparse_arrays.rs | 65 +++++++++++++++++-- .../src/snapshots/no_sparse_arrays.snap | 49 +++++++++++++- 2 files changed, 106 insertions(+), 8 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_sparse_arrays.rs b/crates/oxc_linter/src/rules/eslint/no_sparse_arrays.rs index c6044452c..1d2b2a322 100644 --- a/crates/oxc_linter/src/rules/eslint/no_sparse_arrays.rs +++ b/crates/oxc_linter/src/rules/eslint/no_sparse_arrays.rs @@ -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::>(); + + 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(); } diff --git a/crates/oxc_linter/src/snapshots/no_sparse_arrays.snap b/crates/oxc_linter/src/snapshots/no_sparse_arrays.snap index 684504d58..0e6160db0 100644 --- a/crates/oxc_linter/src/snapshots/no_sparse_arrays.snap +++ b/crates/oxc_linter/src/snapshots/no_sparse_arrays.snap @@ -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`