Resolver: Handle hoisting (#455)

swc_ecma_transforms:
 - resolver: Handle variable hoisting
 - resolver: Handle function hoisting

ecmascript/transforms/src/compat/es2015/classes/tests.rs

@@ -5490,3 +5490,48 @@ expect(obj.test).toBe(2);
 
 "#
 );
+
+test!(
+    syntax(),
+    |_| spec_tr(),
+    issue_454_followup,
+    "if (true){
+    class Foo extends Bar { }
+}",
+    "if (true) {
+    var Foo = function(_Bar) {
+        _inherits(Foo, _Bar);
+        function Foo() {
+            _classCallCheck(this, Foo);
+            return _possibleConstructorReturn(this, _getPrototypeOf(Foo).apply(this, arguments));
+        }
+        return Foo;
+    }(Bar);
+}"
+);
+
+test!(
+    syntax(),
+    |_| spec_tr(),
+    issue_454_followup_2,
+    "function broken(x, ...foo) {
+  if (true) {
+    class Foo extends Bar { }
+    return hello(...foo)
+  }
+}",
+    "function broken(x, ...foo) {
+    if (true) {
+        var Foo = function(_Bar) {
+            _inherits(Foo, _Bar);
+            function Foo() {
+                _classCallCheck(this, Foo);
+                return _possibleConstructorReturn(this, _getPrototypeOf(Foo).apply(this, \
+     arguments));
+            }
+            return Foo;
+        }(Bar);
+        return hello.apply(void 0, foo);
+    }
+}"
+);

ecmascript/transforms/src/compat/es2015/parameters/tests.rs

@@ -1195,7 +1195,7 @@ function d(thing) {
 
 test!(
     ::swc_ecma_parser::Syntax::default(),
-    |_| chain!(Classes, tr(), crate::compat::es2015::spread()),
+    |_| chain!(tr(), Classes, crate::compat::es2015::spread()),
     rest_nested_iife,
     r#"function broken(x, ...foo) {
   if (true) {
@@ -1209,7 +1209,7 @@ test!(
         foo[_key - 1] = arguments[_key];
     }
     if (true) {
-        var Foo = function(_Bar) {
+        let Foo = function(_Bar) {
             _inherits(Foo, _Bar);
             function Foo() {
                 _classCallCheck(this, Foo);

ecmascript/transforms/src/resolver/mod.rs

@@ -4,6 +4,7 @@ use crate::{
 };
 use ast::*;
 use hashbrown::HashSet;
+use std::cell::RefCell;
 use swc_atoms::JsWord;
 use swc_common::{Fold, FoldWith, Mark, SyntaxContext};
 
@@ -12,6 +13,8 @@ mod tests;
 
 const LOG: bool = false;
 
+/// TODO: Split this into two struct
+
 pub fn resolver() -> impl Pass + 'static {
     Resolver::new(
         Mark::fresh(Mark::root()),
@@ -30,6 +33,7 @@ struct Scope<'a> {
 
     /// All references declared in this scope
     declared_symbols: HashSet<JsWord>,
+    hoisted_symbols: RefCell<HashSet<JsWord>>,
 }
 
 impl<'a> Default for Scope<'a> {
@@ -44,11 +48,18 @@ impl<'a> Scope<'a> {
             parent,
             kind,
             declared_symbols: Default::default(),
+            hoisted_symbols: Default::default(),
         }
     }
 }
 
-pub struct Resolver<'a> {
+/// # Phases
+///
+/// ## Hoisting phase
+///
+/// ## Resolving phase
+struct Resolver<'a> {
+    hoist: bool,
     mark: Mark,
     current: Scope<'a>,
     cur_defining: Option<(JsWord, Mark)>,
@@ -58,6 +69,7 @@ pub struct Resolver<'a> {
 impl<'a> Resolver<'a> {
     fn new(mark: Mark, current: Scope<'a>, cur_defining: Option<(JsWord, Mark)>) -> Self {
         Resolver {
+            hoist: false,
             mark,
             current,
             cur_defining,
@@ -70,7 +82,7 @@ impl<'a> Resolver<'a> {
         let mut scope = Some(&self.current);
 
         while let Some(cur) = scope {
-            if cur.declared_symbols.contains(sym) {
+            if cur.declared_symbols.contains(sym) || cur.hoisted_symbols.borrow().contains(sym) {
                 if mark == Mark::root() {
                     return None;
                 }
@@ -108,18 +120,34 @@ impl<'a> Resolver<'a> {
             (true, self.mark)
         };
 
+        let mut mark = mark;
+
         if should_insert {
+            if self.hoist {
+                let mut cursor = Some(&self.current);
+
+                while let Some(c) = cursor {
+                    if c.kind == ScopeKind::Fn {
+                        c.hoisted_symbols.borrow_mut().insert(ident.sym.clone());
+                        break;
+                    }
+                    cursor = c.parent;
+                    mark = mark.parent();
+                }
+            } else {
                 self.current.declared_symbols.insert(ident.sym.clone());
             }
+        }
 
         Ident {
             span: if mark == Mark::root() {
                 ident.span
             } else {
+                let span = ident.span.apply_mark(mark);
                 if cfg!(debug_assertions) && LOG {
-                    eprintln!("\t-> {:?}", mark);
+                    eprintln!("\t-> {:?}", span.ctxt());
                 }
-                ident.span.apply_mark(mark)
+                span
             },
             sym: ident.sym,
             ..ident
@@ -129,25 +157,14 @@ impl<'a> Resolver<'a> {
 
 impl<'a> Fold<Function> for Resolver<'a> {
     fn fold(&mut self, mut f: Function) -> Function {
-        let child_mark = Mark::fresh(self.mark);
+        self.ident_type = IdentType::Ref;
+        f.decorators = f.decorators.fold_with(self);
 
-        // Child folder
-        let mut folder = Resolver::new(
-            child_mark,
-            Scope::new(ScopeKind::Fn, Some(&self.current)),
-            self.cur_defining.take(),
-        );
+        self.ident_type = IdentType::Binding;
+        f.params = f.params.fold_with(self);
 
-        folder.ident_type = IdentType::Ref;
-        f.decorators = f.decorators.fold_with(&mut folder);
-
-        folder.ident_type = IdentType::Binding;
-        f.params = f.params.fold_with(&mut folder);
-
-        folder.ident_type = IdentType::Ref;
-        f.body = f.body.map(|stmt| stmt.fold_children(&mut folder));
-
-        self.cur_defining = folder.cur_defining;
+        self.ident_type = IdentType::Ref;
+        f.body = f.body.map(|stmt| stmt.fold_children(self));
 
         f
     }
@@ -177,7 +194,17 @@ impl<'a> Fold<FnExpr> for Resolver<'a> {
             None
         };
 
-        let function = e.function.fold_with(self);
+        let child_mark = Mark::fresh(self.mark);
+
+        // Child folder
+        let mut folder = Resolver::new(
+            child_mark,
+            Scope::new(ScopeKind::Fn, Some(&self.current)),
+            self.cur_defining.take(),
+        );
+        let function = e.function.fold_with(&mut folder);
+
+        self.cur_defining = folder.cur_defining;
 
         FnExpr { ident, function }
     }
@@ -185,14 +212,23 @@ impl<'a> Fold<FnExpr> for Resolver<'a> {
 
 impl<'a> Fold<FnDecl> for Resolver<'a> {
     fn fold(&mut self, node: FnDecl) -> FnDecl {
-        let ident = self.fold_binding_ident(node.ident);
+        // We don't fold this as Hoister handles this.
+        let ident = node.ident;
 
-        let old = self.cur_defining.take();
-        self.cur_defining = Some((ident.sym.clone(), ident.span.ctxt().remove_mark()));
+        let function = {
+            let child_mark = Mark::fresh(self.mark);
 
-        let function = node.function.fold_with(self);
+            // Child folder
+            let mut folder = Resolver::new(
+                child_mark,
+                Scope::new(ScopeKind::Fn, Some(&self.current)),
+                None,
+            );
 
-        self.cur_defining = old;
+            folder.cur_defining = Some((ident.sym.clone(), ident.span.ctxt().remove_mark()));
+
+            node.function.fold_with(&mut folder)
+        };
 
         FnDecl {
             ident,
@@ -256,6 +292,19 @@ impl<'a> Fold<VarDeclarator> for Resolver<'a> {
     }
 }
 
+impl Fold<VarDecl> for Resolver<'_> {
+    fn fold(&mut self, decl: VarDecl) -> VarDecl {
+        let old_hoist = self.hoist;
+
+        self.hoist = VarDeclKind::Var == decl.kind;
+        let decls = decl.decls.fold_with(self);
+
+        self.hoist = old_hoist;
+
+        VarDecl { decls, ..decl }
+    }
+}
+
 impl<'a> Fold<Ident> for Resolver<'a> {
     fn fold(&mut self, i: Ident) -> Ident {
         match self.ident_type {
@@ -272,14 +321,12 @@ impl<'a> Fold<Ident> for Resolver<'a> {
                 }
 
                 if let Some(mark) = self.mark_for(&sym) {
+                    let span = span.apply_mark(mark);
+
                     if cfg!(debug_assertions) && LOG {
-                        eprintln!("\t -> {:?}", mark);
-                    }
-                    Ident {
-                        sym,
-                        span: span.apply_mark(mark),
-                        ..i
+                        eprintln!("\t -> {:?}", span.ctxt());
                     }
+                    Ident { sym, span, ..i }
                 } else {
                     if cfg!(debug_assertions) && LOG {
                         eprintln!("\t -> Unresolved");
@@ -310,3 +357,51 @@ impl<'a> Fold<ArrowExpr> for Resolver<'a> {
         ArrowExpr { params, body, ..e }
     }
 }
+
+impl Fold<Vec<Stmt>> for Resolver<'_> {
+    fn fold(&mut self, stmts: Vec<Stmt>) -> Vec<Stmt> {
+        if self.current.kind != ScopeKind::Fn {
+            return stmts.fold_children(self);
+        }
+
+        // Phase 1: Handle hoisting
+        let stmts = {
+            let mut hoister = Hoister { resolver: self };
+            stmts.fold_children(&mut hoister)
+        };
+
+        // Phase 2.
+        stmts.fold_children(self)
+    }
+}
+
+impl Fold<Vec<ModuleItem>> for Resolver<'_> {
+    fn fold(&mut self, stmts: Vec<ModuleItem>) -> Vec<ModuleItem> {
+        if self.current.kind != ScopeKind::Fn {
+            return stmts.fold_children(self);
+        }
+
+        // Phase 1: Handle hoisting
+        let stmts = {
+            let mut hoister = Hoister { resolver: self };
+            stmts.fold_children(&mut hoister)
+        };
+
+        // Phase 2.
+        stmts.fold_children(self)
+    }
+}
+
+/// The folder which handles function hoisting.
+struct Hoister<'a, 'b> {
+    resolver: &'a mut Resolver<'b>,
+}
+
+impl Fold<FnDecl> for Hoister<'_, '_> {
+    fn fold(&mut self, node: FnDecl) -> FnDecl {
+        self.resolver.hoist = false;
+        let ident = self.resolver.fold_binding_ident(node.ident);
+
+        FnDecl { ident, ..node }
+    }
+}

ecmascript/transforms/src/resolver/tests.rs

@@ -86,7 +86,8 @@ to!(
             _classCallCheck(this, ConstructorScoping);
             var bar;
             {
-                var bar;
+                let bar;
+                use(bar);
             }
         }
         ",
@@ -96,6 +97,7 @@ to!(
             var bar;
             {
                 var bar1;
+                use(bar1);
             }
         }
         "
@@ -659,8 +661,7 @@ class Foo {
 
 identical!(
     issue_308,
-    "function bar(props) {
-}
+    "function bar(props) {}
 var Foo = function Foo() {
     _classCallCheck(this, Foo);
     super();
@@ -672,6 +673,23 @@ var Foo = function Foo() {
 "
 );
 
+identical!(
+    issue_308_2,
+    "
+function wrapper(){
+    function bar(props) {}
+    var Foo = function Foo() {
+        _classCallCheck(this, Foo);
+        super();
+        _defineProperty(this, 'onBar', ()=>{
+            bar();
+        });
+        bar();
+    };   
+}
+"
+);
+
 identical!(
     issue_369_1,
     "export function input(name) {
@@ -751,3 +769,85 @@ to!(
         return _setPrototypeOf(o, p);
     }"
 );
+
+to!(
+    issue_454_1,
+    "var a = 2;
+function foo() {
+  try {
+    var a = 1;
+    a;
+  } catch (err) {
+    // ignored
+  }
+  a;
+}",
+    "var a = 2;
+function foo() {
+    try {
+        var a1 = 1;
+        a1;
+    } catch (err) {
+    }
+    a1;
+}"
+);
+
+to!(
+    issue_454_2,
+    "function a() {}
+function foo() {
+  function b() {
+    a();
+  }
+  function a() {}
+}",
+    "function a1() {
+}
+function foo() {
+    function b() {
+        a2();
+    }
+    function a2() {
+    }
+}"
+);
+
+to!(
+    issue_454_3,
+    "function a() {}
+function foo() {
+    function b() {
+        a();
+    }
+    function a() {
+        b();
+    }
+}",
+    "function a1() {
+}
+function foo() {
+    function b() {
+        a2();
+    }
+    function a2() {
+        b();
+    }
+}"
+);
+
+identical!(
+    regression_of_454,
+    "function broken(x) {
+        var Foo = function(_Bar) {
+            _inherits(Foo, _Bar);
+            function Foo() {
+                _classCallCheck(this, Foo);
+                return _possibleConstructorReturn(this, _getPrototypeOf(Foo).apply(this, \
+     arguments));
+            }
+            return Foo;
+        }(Bar);
+}
+"
+);

ecmascript/transforms/src/util/mod.rs

@@ -67,7 +67,7 @@ pub trait ModuleItemLike: StmtLike {
     }
 }
 
-pub trait StmtLike: Sized {
+pub trait StmtLike: Sized + 'static {
     fn try_into_stmt(self) -> Result<Stmt, Self>;
     fn as_stmt(&self) -> Option<&Stmt>;
     fn from_stmt(stmt: Stmt) -> Self;