Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf(es/lints): Avoid needless allocations in no-dupe-args #9041

Merged
merged 14 commits into from
Jun 14, 2024
117 changes: 82 additions & 35 deletions crates/swc_ecma_lints/src/rules/no_dupe_args.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use swc_atoms::JsWord;
use swc_common::{collections::AHashMap, errors::HANDLER, Span};
use std::collections::hash_map::Entry;

use swc_common::{collections::AHashMap, errors::HANDLER};
use swc_ecma_ast::*;
use swc_ecma_utils::find_pat_ids;
use swc_ecma_utils::for_each_binding_ident;
use swc_ecma_visit::{noop_visit_type, Visit, VisitWith};

use crate::rule::{visitor_rule, Rule};
Expand All @@ -13,54 +14,100 @@ pub fn no_dupe_args() -> Box<dyn Rule> {
#[derive(Debug, Default)]
struct NoDupeArgs {}

impl NoDupeArgs {
fn check(&self, param_list: Vec<(JsWord, Span)>) {
let mut variables_map = AHashMap::<JsWord, Span>::default();

param_list.into_iter().for_each(|(js_word, span)| {
if let Some(old_span) = variables_map.insert(js_word.clone(), span) {
HANDLER.with(|handler| {
handler
.struct_span_err(
span,
&format!(
"the name `{}` is bound more than once in this parameter list",
js_word
),
)
.span_label(old_span, "previous definition here".to_string())
.span_label(span, &"used as parameter more than once".to_string())
.emit();
fn error(first: &Ident, second: &Ident) {
HANDLER.with(|handler| {
handler
.struct_span_err(
second.span,
&format!(
"the name `{}` is bound more than once in this parameter list",
first.sym
),
)
.span_label(first.span, "previous definition here".to_string())
.span_label(second.span, &"used as parameter more than once".to_string())
.emit();
});
}

/// This has time complexity of O(n^2), but it's fine as the number of paramters
/// is usually small.
macro_rules! check {
Comment on lines +33 to +34
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably holds true for 99.9%, but maybe we can add another implementation that is used for n > 100 based on a hashmap.

($node:expr) => {{
// This vector allocates only if there are duplicate parameters.
// This is used to handle the case where the same parameter is used 3 or more
// times.
let mut done = vec![];

let mut hash_mode = false;

let mut i1 = 0;
for_each_binding_ident($node, |id1| {
i1 += 1;

if !hash_mode {
let mut i2 = 0;
for_each_binding_ident($node, |id2| {
i2 += 1;

if hash_mode {
return;
} else if i2 >= 100 {
// While iterating for the first `id1`, we detect that there are more than
// 100 identifiers. We switch to hash mode.
hash_mode = true;
}

if i1 >= i2 || done.contains(&i1) {
return;
}

if id1.span.ctxt == id2.span.ctxt && id1.sym == id2.sym {
done.push(i1);

error(id1, id2);
}
});
}
});
}

if hash_mode {
let mut map = AHashMap::default();

for_each_binding_ident($node, |id| {
//
match map.entry((id.sym.clone(), id.span.ctxt)) {
Entry::Occupied(v) => {
error(v.get(), id);
}

Entry::Vacant(v) => {
v.insert(id.clone());
}
}
});
}
}};
}

impl Visit for NoDupeArgs {
noop_visit_type!();

fn visit_function(&mut self, f: &Function) {
let variables: Vec<(JsWord, Span)> = find_pat_ids(&f.params);

self.check(variables);
check!(&f.params);

f.visit_children_with(self);
}

fn visit_arrow_expr(&mut self, arrow_fn: &ArrowExpr) {
let variables: Vec<(JsWord, Span)> = find_pat_ids(&arrow_fn.params);
fn visit_arrow_expr(&mut self, f: &ArrowExpr) {
check!(&f.params);

self.check(variables);

arrow_fn.visit_children_with(self);
f.visit_children_with(self);
}

fn visit_constructor(&mut self, n: &Constructor) {
let variables: Vec<(JsWord, Span)> = find_pat_ids(&n.params);
fn visit_constructor(&mut self, f: &Constructor) {
check!(&f.params);

self.check(variables);

n.visit_children_with(self);
f.visit_children_with(self);
}
}
35 changes: 35 additions & 0 deletions crates/swc_ecma_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2373,6 +2373,8 @@ pub struct DestructuringFinder<I: IdentLike> {
}

/// Finds all **binding** idents of `node`.
///
/// If you want to avoid allocation, use [`for_each_binding_ident`] instead.
pub fn find_pat_ids<T, I: IdentLike>(node: &T) -> Vec<I>
where
T: VisitWith<DestructuringFinder<I>>,
Expand All @@ -2397,6 +2399,39 @@ impl<I: IdentLike> Visit for DestructuringFinder<I> {
fn visit_prop_name(&mut self, _: &PropName) {}
}

/// Finds all **binding** idents of variables.
pub struct BindingIdentifierVisitor<F>
where
F: for<'a> FnMut(&'a Ident),
{
op: F,
}

/// Finds all **binding** idents of `node`. **Any nested identifiers in
/// expressions are ignored**.
pub fn for_each_binding_ident<T, F>(node: &T, op: F)
where
T: VisitWith<BindingIdentifierVisitor<F>>,
F: for<'a> FnMut(&'a Ident),
{
let mut v = BindingIdentifierVisitor { op };
node.visit_with(&mut v);
}

impl<F> Visit for BindingIdentifierVisitor<F>
where
F: for<'a> FnMut(&'a Ident),
{
noop_visit_type!();

/// No-op (we don't care about expressions)
fn visit_expr(&mut self, _: &Expr) {}

fn visit_binding_ident(&mut self, i: &BindingIdent) {
(self.op)(i);
}
}

pub fn is_valid_ident(s: &JsWord) -> bool {
if s.len() == 0 {
return false;
Expand Down
Loading