From 81384f505990fe438f2daa25c5a11142747aa6d9 Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Fri, 26 Jul 2024 00:14:41 +0000 Subject: [PATCH] perf(linter): avoid unnecessary work in `nextjs:no_duplicate_head` rule (#4465) In `nextjs:no_duplicate_head` rule, avoid allocating a `Vec` unless more than one `` is found (uncommon case). Also, #4464 made getting span of a `Reference` a little more expensive. Avoid this work unless it's needed (which it generally won't be). --- .../src/rules/nextjs/no_duplicate_head.rs | 52 ++++++++++++++----- 1 file changed, 40 insertions(+), 12 deletions(-) diff --git a/crates/oxc_linter/src/rules/nextjs/no_duplicate_head.rs b/crates/oxc_linter/src/rules/nextjs/no_duplicate_head.rs index 6182840ab074b..ef05f50fdad25 100644 --- a/crates/oxc_linter/src/rules/nextjs/no_duplicate_head.rs +++ b/crates/oxc_linter/src/rules/nextjs/no_duplicate_head.rs @@ -1,6 +1,7 @@ use oxc_ast::AstKind; use oxc_diagnostics::{LabeledSpan, OxcDiagnostic}; use oxc_macros::declare_oxc_lint; +use oxc_span::GetSpan; use crate::{context::LintContext, rule::Rule}; @@ -56,19 +57,46 @@ impl Rule for NoDuplicateHead { return; } + // 1 x `` is fine, more than 1 is not. + // Avoid allocating a `Vec`, or looking up span in common case + // where only a single `` is found. + let mut first_node_id = None; + let mut labels = vec![]; let nodes = ctx.nodes(); - let labels = symbols - .get_resolved_references(symbol_id) - .filter(|r| r.is_read()) - .filter(|r| { - let kind = nodes.ancestors(r.node_id()).nth(2).map(|node_id| nodes.kind(node_id)); - matches!(kind, Some(AstKind::JSXOpeningElement(_))) - }) - .map(|reference| ctx.semantic().reference_span(reference)) - .map(LabeledSpan::underline) - .collect::>(); - - if labels.len() <= 1 { + let get_label = |node_id| { + let span = nodes.kind(node_id).span(); + LabeledSpan::underline(span) + }; + + for reference in symbols.get_resolved_references(symbol_id) { + if !reference.is_read() { + continue; + } + + if !matches!( + nodes.ancestors(reference.node_id()).nth(2).map(|node_id| nodes.kind(node_id)), + Some(AstKind::JSXOpeningElement(_)) + ) { + continue; + } + + let node_id = reference.node_id(); + #[allow(clippy::unnecessary_unwrap)] + if first_node_id.is_none() { + // First `` found + first_node_id = Some(node_id); + } else if labels.is_empty() { + // 2nd `` found - populate `labels` with both + let first_node_id = first_node_id.unwrap(); + labels.extend([get_label(first_node_id), get_label(node_id)]); + } else { + // Further `` found - add to `node_ids` + labels.push(get_label(node_id)); + } + } + + // `labels` is empty if 0 or 1 `` found + if labels.is_empty() { return; }