Skip to content

Commit e560198

Browse files
authoredJun 14, 2024··
perf(es/lints): Avoid needless allocations in no-dupe-args (#9041)
**Description:** I introduced a zero-allocation variant for `swc_ecma_utils::DestructuringFinder` that does not allocate anything.
1 parent 931f752 commit e560198

File tree

4 files changed

+807
-35
lines changed

4 files changed

+807
-35
lines changed
 
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
function foo(
2+
[a0, a1, a2, a3, a4, a5, a6, a7, a8, a9], [b0, b1, b2, b3, b4, b5, b6, b7, b8, b9],
3+
[c0, c1, c2, c3, c4, c5, c6, c7, c8, c9], [d0, d1, d2, d3, d4, d5, d6, d7, d8, d9],
4+
[e0, e1, e2, e3, e4, e5, e6, e7, e8, e9], [f0, f1, f2, f3, f4, f5, f6, f7, f8, f9],
5+
[g0, g1, g2, g3, g4, g5, g6, g7, g8, g9], [h0, h1, h2, h3, h4, h5, h6, h7, h8, h9],
6+
[a0, a1, a2, a3, a4, a5, a6, a7, a8, a9], [b0, b1, b2, b3, b4, b5, b6, b7, b8, b9],
7+
[i0, i1, i2, i3, i4, i5, i6, i7, i8, i9], [j0, j1, j2, j3, j4, j5, j6, j7, j8, j9],
8+
[a0, a1, a2, a3, a4, a5, a6, a7, a8, a9], [b0, b1, b2, b3, b4, b5, b6, b7, b8, b9],) {
9+
10+
}

‎crates/swc/tests/errors/lints/no-dupe-args/hash-mode/output.swc-stderr

+680
Large diffs are not rendered by default.
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
use swc_atoms::JsWord;
2-
use swc_common::{collections::AHashMap, errors::HANDLER, Span};
1+
use std::collections::hash_map::Entry;
2+
3+
use swc_common::{collections::AHashMap, errors::HANDLER};
34
use swc_ecma_ast::*;
4-
use swc_ecma_utils::find_pat_ids;
5+
use swc_ecma_utils::for_each_binding_ident;
56
use swc_ecma_visit::{noop_visit_type, Visit, VisitWith};
67

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

16-
impl NoDupeArgs {
17-
fn check(&self, param_list: Vec<(JsWord, Span)>) {
18-
let mut variables_map = AHashMap::<JsWord, Span>::default();
19-
20-
param_list.into_iter().for_each(|(js_word, span)| {
21-
if let Some(old_span) = variables_map.insert(js_word.clone(), span) {
22-
HANDLER.with(|handler| {
23-
handler
24-
.struct_span_err(
25-
span,
26-
&format!(
27-
"the name `{}` is bound more than once in this parameter list",
28-
js_word
29-
),
30-
)
31-
.span_label(old_span, "previous definition here".to_string())
32-
.span_label(span, &"used as parameter more than once".to_string())
33-
.emit();
17+
fn error(first: &Ident, second: &Ident) {
18+
HANDLER.with(|handler| {
19+
handler
20+
.struct_span_err(
21+
second.span,
22+
&format!(
23+
"the name `{}` is bound more than once in this parameter list",
24+
first.sym
25+
),
26+
)
27+
.span_label(first.span, "previous definition here".to_string())
28+
.span_label(second.span, &"used as parameter more than once".to_string())
29+
.emit();
30+
});
31+
}
32+
33+
/// This has time complexity of O(n^2), but it's fine as the number of paramters
34+
/// is usually small.
35+
macro_rules! check {
36+
($node:expr) => {{
37+
// This vector allocates only if there are duplicate parameters.
38+
// This is used to handle the case where the same parameter is used 3 or more
39+
// times.
40+
let mut done = vec![];
41+
42+
let mut hash_mode = false;
43+
44+
let mut i1 = 0;
45+
for_each_binding_ident($node, |id1| {
46+
i1 += 1;
47+
48+
if !hash_mode {
49+
let mut i2 = 0;
50+
for_each_binding_ident($node, |id2| {
51+
i2 += 1;
52+
53+
if hash_mode {
54+
return;
55+
} else if i2 >= 100 {
56+
// While iterating for the first `id1`, we detect that there are more than
57+
// 100 identifiers. We switch to hash mode.
58+
hash_mode = true;
59+
}
60+
61+
if i1 >= i2 || done.contains(&i1) {
62+
return;
63+
}
64+
65+
if id1.span.ctxt == id2.span.ctxt && id1.sym == id2.sym {
66+
done.push(i1);
67+
68+
error(id1, id2);
69+
}
3470
});
3571
}
3672
});
37-
}
73+
74+
if hash_mode {
75+
let mut map = AHashMap::default();
76+
77+
for_each_binding_ident($node, |id| {
78+
//
79+
match map.entry((id.sym.clone(), id.span.ctxt)) {
80+
Entry::Occupied(v) => {
81+
error(v.get(), id);
82+
}
83+
84+
Entry::Vacant(v) => {
85+
v.insert(id.clone());
86+
}
87+
}
88+
});
89+
}
90+
}};
3891
}
3992

4093
impl Visit for NoDupeArgs {
4194
noop_visit_type!();
4295

4396
fn visit_function(&mut self, f: &Function) {
44-
let variables: Vec<(JsWord, Span)> = find_pat_ids(&f.params);
45-
46-
self.check(variables);
97+
check!(&f.params);
4798

4899
f.visit_children_with(self);
49100
}
50101

51-
fn visit_arrow_expr(&mut self, arrow_fn: &ArrowExpr) {
52-
let variables: Vec<(JsWord, Span)> = find_pat_ids(&arrow_fn.params);
102+
fn visit_arrow_expr(&mut self, f: &ArrowExpr) {
103+
check!(&f.params);
53104

54-
self.check(variables);
55-
56-
arrow_fn.visit_children_with(self);
105+
f.visit_children_with(self);
57106
}
58107

59-
fn visit_constructor(&mut self, n: &Constructor) {
60-
let variables: Vec<(JsWord, Span)> = find_pat_ids(&n.params);
108+
fn visit_constructor(&mut self, f: &Constructor) {
109+
check!(&f.params);
61110

62-
self.check(variables);
63-
64-
n.visit_children_with(self);
111+
f.visit_children_with(self);
65112
}
66113
}

‎crates/swc_ecma_utils/src/lib.rs

+35
Original file line numberDiff line numberDiff line change
@@ -2373,6 +2373,8 @@ pub struct DestructuringFinder<I: IdentLike> {
23732373
}
23742374

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

2402+
/// Finds all **binding** idents of variables.
2403+
pub struct BindingIdentifierVisitor<F>
2404+
where
2405+
F: for<'a> FnMut(&'a Ident),
2406+
{
2407+
op: F,
2408+
}
2409+
2410+
/// Finds all **binding** idents of `node`. **Any nested identifiers in
2411+
/// expressions are ignored**.
2412+
pub fn for_each_binding_ident<T, F>(node: &T, op: F)
2413+
where
2414+
T: VisitWith<BindingIdentifierVisitor<F>>,
2415+
F: for<'a> FnMut(&'a Ident),
2416+
{
2417+
let mut v = BindingIdentifierVisitor { op };
2418+
node.visit_with(&mut v);
2419+
}
2420+
2421+
impl<F> Visit for BindingIdentifierVisitor<F>
2422+
where
2423+
F: for<'a> FnMut(&'a Ident),
2424+
{
2425+
noop_visit_type!();
2426+
2427+
/// No-op (we don't care about expressions)
2428+
fn visit_expr(&mut self, _: &Expr) {}
2429+
2430+
fn visit_binding_ident(&mut self, i: &BindingIdent) {
2431+
(self.op)(i);
2432+
}
2433+
}
2434+
24002435
pub fn is_valid_ident(s: &JsWord) -> bool {
24012436
if s.len() == 0 {
24022437
return false;

0 commit comments

Comments
 (0)
Please sign in to comment.