Skip to content

Commit

Permalink
fix(es/resolver): Fix handling of block scoped functions (#5092)
Browse files Browse the repository at this point in the history
  • Loading branch information
Austaras committed Jul 5, 2022
1 parent 6d6e4dd commit 9519e80
Show file tree
Hide file tree
Showing 35 changed files with 830 additions and 280 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
try {
var fx
function fx(){}
} catch {

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@

x the name `fx` is defined multiple times
,-[2:5]
2 | var fx
: ^|
: `-- previous definition of `fx` here
3 | function fx(){}
: ^|
: `-- `fx` redefined here
`----
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
switch (a) {
case 'a':
var foo
function foo() {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@

x the name `foo` is defined multiple times
,-[3:7]
3 | var foo
: ^|^
: `-- previous definition of `foo` here
4 | function foo() {}
: ^|^
: `-- `foo` redefined here
`----
205 changes: 155 additions & 50 deletions crates/swc_ecma_lints/src/rules/duplicate_bindings.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
use std::collections::hash_map::Entry;

use swc_atoms::JsWord;
use swc_common::{
collections::{AHashMap, AHashSet},
errors::HANDLER,
Span,
};
use swc_ecma_ast::*;
use swc_ecma_utils::StmtLike;
use swc_ecma_visit::{noop_visit_type, Visit, VisitWith};

use crate::rule::{visitor_rule, Rule};

pub fn duplicate_bindings() -> Box<dyn Rule> {
visitor_rule(DuplicateBindings {
top_level: true,
..Default::default()
})
}
Expand All @@ -21,6 +22,7 @@ pub fn duplicate_bindings() -> Box<dyn Rule> {
struct BindingInfo {
span: Span,
unique: bool,
is_function: bool,
}

#[derive(Debug, Default)]
Expand All @@ -31,47 +33,29 @@ struct DuplicateBindings {
var_decl_kind: Option<VarDeclKind>,
is_pat_decl: bool,

is_module: bool,

top_level: bool,
/// at the top level of script of function, function behaves like var
/// in other scope it behaves like let
lexical_function: bool,
}

impl DuplicateBindings {
/// Add a binding.
fn add(&mut self, id: &Ident, unique: bool) {
match self.bindings.entry(id.to_id()) {
fn add(&mut self, id: JsWord, info: BindingInfo) {
match self.bindings.entry((id.clone(), info.span.ctxt())) {
Entry::Occupied(mut prev) => {
if unique || prev.get().unique {
let name = &id.sym;

HANDLER.with(|handler| {
handler
.struct_span_err(
id.span,
&format!("the name `{}` is defined multiple times", name),
)
.span_label(
prev.get().span,
&format!("previous definition of `{}` here", name),
)
.span_label(id.span, &format!("`{}` redefined here", name))
.emit();
});
if !(info.is_function && prev.get().is_function)
&& (info.unique || prev.get().unique)
{
emit_error(&id, info.span, prev.get().span);
}

// Next span.
if unique || !prev.get().unique {
*prev.get_mut() = BindingInfo {
span: id.span,
unique,
}
if info.unique || !prev.get().unique {
*prev.get_mut() = info
}
}
Entry::Vacant(e) => {
e.insert(BindingInfo {
span: id.span,
unique,
});
e.insert(info);
}
}
}
Expand All @@ -97,11 +81,34 @@ impl DuplicateBindings {
self.var_decl_kind = old_var_decl_kind;
}

fn visit_with_fn_scope<V: VisitWith<Self>>(&mut self, e: &V) {
let top_level = self.top_level;
self.top_level = false;
e.visit_children_with(self);
self.top_level = top_level;
// this is for the wired case:
// in non strict mode, function in non top level or function scope
// is hoisted, while still error when collides with same level lexical var
fn visit_with_stmt_like<T: StmtLike + VisitWith<Self>>(&mut self, s: &[T]) {
let mut fn_name = AHashMap::default();
for s in s {
if let Some(Stmt::Decl(Decl::Fn(s))) = s.as_stmt() {
if let Some(prev) = fn_name.get(&s.ident.sym) {
emit_error(&s.ident.sym, s.ident.span, *prev)
} else {
fn_name.insert(s.ident.sym.clone(), s.ident.span);
}
}

s.visit_with(self);
}
}

fn visit_with_stmts(&mut self, s: &[Stmt], lexical_function: bool) {
let old = self.lexical_function;
self.lexical_function = lexical_function;

if lexical_function {
self.visit_with_stmt_like(s);
} else {
s.visit_children_with(self);
}
self.lexical_function = old;
}
}

Expand All @@ -112,28 +119,74 @@ impl Visit for DuplicateBindings {
p.visit_children_with(self);

if self.is_pat_decl {
self.add(&p.key, self.is_unique_var_kind());
self.add(
p.key.sym.clone(),
BindingInfo {
span: p.key.span,
unique: self.is_unique_var_kind(),
is_function: false,
},
);
}
}

fn visit_function(&mut self, f: &Function) {
self.visit_with_fn_scope(f)
// in case any new parts is added
let Function {
body,
params,
decorators,
span: _,
is_generator: _,
is_async: _,
type_params: _,
return_type: _,
} = f;
params.visit_with(self);
decorators.visit_with(self);
if let Some(body) = body {
self.visit_with_stmts(&body.stmts, false)
}
}

fn visit_arrow_expr(&mut self, a: &ArrowExpr) {
self.visit_with_fn_scope(a)
let ArrowExpr {
params,
body,
span: _,
is_async: _,
is_generator: _,
type_params: _,
return_type: _,
} = a;
params.visit_with(self);
if let BlockStmtOrExpr::BlockStmt(b) = body {
self.visit_with_stmts(&b.stmts, false)
}
}

fn visit_static_block(&mut self, c: &StaticBlock) {
self.visit_with_stmts(&c.body.stmts, false)
}

fn visit_class(&mut self, c: &Class) {
self.visit_with_fn_scope(c)
// block stmt and case block
fn visit_stmts(&mut self, b: &[Stmt]) {
self.visit_with_stmts(b, true)
}

fn visit_catch_clause(&mut self, c: &CatchClause) {
self.visit_with_kind(c, Some(VarDeclKind::Var))
}

fn visit_class_decl(&mut self, d: &ClassDecl) {
self.add(&d.ident, true);
self.add(
d.ident.sym.clone(),
BindingInfo {
span: d.ident.span,
unique: true,
is_function: false,
},
);

d.visit_children_with(self);
}
Expand All @@ -153,7 +206,14 @@ impl Visit for DuplicateBindings {

fn visit_fn_decl(&mut self, d: &FnDecl) {
if d.function.body.is_some() {
self.add(&d.ident, self.is_module && self.top_level);
self.add(
d.ident.sym.clone(),
BindingInfo {
span: d.ident.span,
unique: self.lexical_function,
is_function: true,
},
);
}

d.visit_children_with(self);
Expand All @@ -171,23 +231,44 @@ impl Visit for DuplicateBindings {
s.visit_children_with(self);

if !self.type_bindings.contains(&s.local.to_id()) {
self.add(&s.local, true);
self.add(
s.local.sym.clone(),
BindingInfo {
span: s.local.span,
unique: true,
is_function: false,
},
);
}
}

fn visit_import_named_specifier(&mut self, s: &ImportNamedSpecifier) {
s.visit_children_with(self);

if !s.is_type_only && !self.type_bindings.contains(&s.local.to_id()) {
self.add(&s.local, true);
self.add(
s.local.sym.clone(),
BindingInfo {
span: s.local.span,
unique: true,
is_function: false,
},
);
}
}

fn visit_import_star_as_specifier(&mut self, s: &ImportStarAsSpecifier) {
s.visit_children_with(self);

if !self.type_bindings.contains(&s.local.to_id()) {
self.add(&s.local, true);
self.add(
s.local.sym.clone(),
BindingInfo {
span: s.local.span,
unique: true,
is_function: false,
},
);
}
}

Expand All @@ -196,16 +277,24 @@ impl Visit for DuplicateBindings {
type_bindings: &mut self.type_bindings,
});

self.is_module = true;
m.visit_children_with(self);
self.lexical_function = true;

self.visit_with_stmt_like(&m.body);
}

fn visit_pat(&mut self, p: &Pat) {
p.visit_children_with(self);

if let Pat::Ident(p) = p {
if self.is_pat_decl {
self.add(&p.id, self.is_unique_var_kind());
self.add(
p.id.sym.clone(),
BindingInfo {
span: p.id.span,
unique: self.is_unique_var_kind(),
is_function: false,
},
);
}
}
}
Expand All @@ -215,7 +304,7 @@ impl Visit for DuplicateBindings {
type_bindings: &mut self.type_bindings,
});

s.visit_children_with(self);
s.body.visit_children_with(self);
}

fn visit_var_decl(&mut self, d: &VarDecl) {
Expand All @@ -240,3 +329,19 @@ impl Visit for TypeCollector<'_> {
}
}
}

fn emit_error(name: &str, span: Span, prev_span: Span) {
HANDLER.with(|handler| {
handler
.struct_span_err(
span,
&format!("the name `{}` is defined multiple times", name),
)
.span_label(
prev_span,
&format!("previous definition of `{}` here", name),
)
.span_label(span, &format!("`{}` redefined here", name))
.emit();
});
}
6 changes: 6 additions & 0 deletions crates/swc_ecma_lints/tests/pass/issue-4907/2/input.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export class A {
static {
var fx
function fx(){}
}
}

1 comment on commit 9519e80

@github-actions
Copy link

Choose a reason for hiding this comment

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

Benchmark

Benchmark suite Current: 9519e80 Previous: 204d742 Ratio
es/full/minify/libraries/antd 1703854935 ns/iter (± 57056341) 1645568957 ns/iter (± 20887065) 1.04
es/full/minify/libraries/d3 435443046 ns/iter (± 12065822) 409586322 ns/iter (± 10587627) 1.06
es/full/minify/libraries/echarts 1651395465 ns/iter (± 19113364) 1613964429 ns/iter (± 23749483) 1.02
es/full/minify/libraries/jquery 101176375 ns/iter (± 8090664) 88593617 ns/iter (± 2852172) 1.14
es/full/minify/libraries/lodash 142153710 ns/iter (± 7054915) 123223614 ns/iter (± 8104909) 1.15
es/full/minify/libraries/moment 54679707 ns/iter (± 3965279) 57572524 ns/iter (± 2470404) 0.95
es/full/minify/libraries/react 19362959 ns/iter (± 1230351) 18979194 ns/iter (± 601944) 1.02
es/full/minify/libraries/terser 601833366 ns/iter (± 9812263) 596416211 ns/iter (± 9483327) 1.01
es/full/minify/libraries/three 550510339 ns/iter (± 5306246) 538408911 ns/iter (± 9380599) 1.02
es/full/minify/libraries/typescript 3586070227 ns/iter (± 49464644) 3402142739 ns/iter (± 64459217) 1.05
es/full/minify/libraries/victory 720875948 ns/iter (± 7730871) 704827816 ns/iter (± 10928979) 1.02
es/full/minify/libraries/vue 141706800 ns/iter (± 4365427) 131182834 ns/iter (± 4165899) 1.08
es/full/codegen/es3 32215 ns/iter (± 365) 31180 ns/iter (± 636) 1.03
es/full/codegen/es5 32421 ns/iter (± 907) 31251 ns/iter (± 779) 1.04
es/full/codegen/es2015 32319 ns/iter (± 335) 31202 ns/iter (± 1753) 1.04
es/full/codegen/es2016 32311 ns/iter (± 730) 31286 ns/iter (± 1127) 1.03
es/full/codegen/es2017 32595 ns/iter (± 1204) 31214 ns/iter (± 1544) 1.04
es/full/codegen/es2018 32661 ns/iter (± 1711) 31321 ns/iter (± 729) 1.04
es/full/codegen/es2019 32363 ns/iter (± 656) 31305 ns/iter (± 909) 1.03
es/full/codegen/es2020 32542 ns/iter (± 713) 31681 ns/iter (± 346) 1.03
es/full/all/es3 179929695 ns/iter (± 8015810) 187827533 ns/iter (± 4162476) 0.96
es/full/all/es5 173238934 ns/iter (± 6093248) 177806990 ns/iter (± 4975691) 0.97
es/full/all/es2015 145498174 ns/iter (± 12831272) 144996657 ns/iter (± 6308711) 1.00
es/full/all/es2016 144401540 ns/iter (± 4936729) 144627665 ns/iter (± 4798638) 1.00
es/full/all/es2017 143211317 ns/iter (± 4982326) 144693521 ns/iter (± 4574964) 0.99
es/full/all/es2018 144238547 ns/iter (± 5157975) 145268118 ns/iter (± 4670133) 0.99
es/full/all/es2019 141589870 ns/iter (± 5010692) 143915531 ns/iter (± 4705254) 0.98
es/full/all/es2020 142000306 ns/iter (± 4042826) 131923862 ns/iter (± 3104832) 1.08
es/full/parser 719839 ns/iter (± 17517) 761028 ns/iter (± 56598) 0.95
es/full/base/fixer 30323 ns/iter (± 573) 28793 ns/iter (± 2972) 1.05
es/full/base/resolver_and_hygiene 87777 ns/iter (± 3506) 87979 ns/iter (± 1405) 1.00
serialization of ast node 213 ns/iter (± 5) 215 ns/iter (± 3) 0.99
serialization of serde 227 ns/iter (± 2) 228 ns/iter (± 5) 1.00

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.