Fix hygiene bug (#109)

ecmascript/transforms/src/compat/es2015/block_scoping.rs

@@ -21,7 +21,7 @@ impl<'a> BlockFolder<'a> {
         let mut scope = Some(&self.current);
 
         while let Some(cur) = scope {
-            if cur.declared_symbols.contains(sym) {
+            if cur.declared_symbols.contains_key(sym) {
                 return Some(mark);
             }
             mark = mark.parent();
@@ -70,10 +70,13 @@ impl<'a> Fold<Pat> for BlockFolder<'a> {
     fn fold(&mut self, pat: Pat) -> Pat {
         match pat {
             Pat::Ident(ident) => {
-                self.current.declared_symbols.insert(ident.sym.clone());
+                self.current
+                    .declared_symbols
+                    .insert(ident.sym.clone(), ident.span.ctxt());
+
                 let ident = Ident {
-                    sym: ident.sym,
                     span: ident.span.apply_mark(self.mark),
+                    sym: ident.sym,
                 };
                 return Pat::Ident(ident);
             }
@@ -87,18 +90,6 @@ impl<'a> Fold<Pat> for BlockFolder<'a> {
 impl<'a> Fold<Expr> for BlockFolder<'a> {
     fn fold(&mut self, expr: Expr) -> Expr {
         match expr {
-            Expr::Ident(Ident { sym, span }) => {
-                if let Some(mark) = self.mark_for(&sym) {
-                    Expr::Ident(Ident {
-                        sym,
-                        span: span.apply_mark(mark),
-                    })
-                } else {
-                    // Cannot resolve reference. (TODO: Report error)
-                    Expr::Ident(Ident { sym, span })
-                }
-            }
-
             // Leftmost one of a member expression shoukld be resolved.
             Expr::Member(me) => Expr::Member(MemberExpr {
                 obj: me.obj.fold_with(self),
@@ -109,9 +100,35 @@ impl<'a> Fold<Expr> for BlockFolder<'a> {
     }
 }
 
+impl<'a> Fold<VarDeclarator> for BlockFolder<'a> {
+    fn fold(&mut self, decl: VarDeclarator) -> VarDeclarator {
+        VarDeclarator {
+            // order is important
+            init: decl.init.fold_children(self),
+            name: decl.name.fold_with(self),
+            ..decl
+        }
+    }
+}
+
+impl<'a> Fold<Ident> for BlockFolder<'a> {
+    fn fold(&mut self, Ident { span, sym }: Ident) -> Ident {
+        if let Some(mark) = self.mark_for(&sym) {
+            Ident {
+                sym,
+                span: span.apply_mark(mark),
+            }
+        } else {
+            // Cannot resolve reference. (TODO: Report error)
+            Ident { sym, span }
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
+    use swc_common::SyntaxContext;
 
     #[test]
     fn test_mark_for() {
@@ -124,16 +141,25 @@ mod tests {
             let folder1 = BlockFolder::new(mark1, Scope::new(ScopeKind::Block, None));
             let mut folder2 =
                 BlockFolder::new(mark2, Scope::new(ScopeKind::Block, Some(&folder1.current)));
-            folder2.current.declared_symbols.insert("foo".into());
+            folder2
+                .current
+                .declared_symbols
+                .insert("foo".into(), SyntaxContext::empty());
 
             let mut folder3 =
                 BlockFolder::new(mark3, Scope::new(ScopeKind::Block, Some(&folder2.current)));
-            folder3.current.declared_symbols.insert("bar".into());
+            folder3
+                .current
+                .declared_symbols
+                .insert("bar".into(), SyntaxContext::empty());
             assert_eq!(folder3.mark_for(&"bar".into()), Some(mark3));
 
             let mut folder4 =
                 BlockFolder::new(mark4, Scope::new(ScopeKind::Block, Some(&folder3.current)));
-            folder4.current.declared_symbols.insert("foo".into());
+            folder4
+                .current
+                .declared_symbols
+                .insert("foo".into(), SyntaxContext::empty());
 
             assert_eq!(folder4.mark_for(&"foo".into()), Some(mark4));
             assert_eq!(folder4.mark_for(&"bar".into()), Some(mark3));
@@ -429,4 +455,56 @@ expect(a).toBe(2);"#
         use(a1);
     }"#
     );
+
+    test!(
+        ::swc_ecma_parser::Syntax::Es2019,
+        block_scoping(),
+        shorthand,
+        r#"let a = 'foo';
+    function foo() {
+        let a = 'bar';
+        use({a});
+    }"#,
+        r#"var a = 'foo';
+    function foo() {
+        var a1 = 'bar';
+        use({a: a1});
+    }"#
+    );
+
+    test!(
+        ::swc_ecma_parser::Syntax::Es2019,
+        block_scoping(),
+        same_level,
+        r#"
+        var a = 'foo';
+        var a = 'bar';
+        "#,
+        r#"
+        var a = 'foo';
+        var a = 'bar';
+        "#
+    );
+
+    test!(
+        ::swc_ecma_parser::Syntax::Es2019,
+        block_scoping(),
+        class_block,
+        r#"
+    var Foo = function(_Bar) {
+            _inherits(Foo, _Bar);
+            function Foo() {
+            }
+            return Foo;
+        }(Bar);
+    "#,
+        r#"
+    var Foo = function(_Bar) {
+            _inherits(Foo, _Bar);
+            function Foo() {
+            }
+            return Foo;
+        }(Bar);
+        "#
+    );
 }

ecmascript/transforms/src/hygiene.rs

@@ -12,9 +12,15 @@ impl Hygiene {
         }
 
         if !scope.is_declared(&ident.sym) {
-            // First symbol
+            // first symbol
 
-            scope.declared_symbols.insert(ident.sym.clone());
+            scope
+                .declared_symbols
+                .insert(ident.sym.clone(), ident.span.ctxt());
+            return;
+        }
+        if scope.declared_symbols.get(&ident.sym) == Some(&ident.span.ctxt()) {
+            // skip if previous symbol is declared on the same level.
             return;
         }
 
@@ -345,4 +351,58 @@ mod tests {
             ",
         );
     }
+
+    #[test]
+    fn shorthand() {
+        test(
+            |tester| {
+                let mark1 = Mark::fresh(Mark::root());
+                let mark2 = Mark::fresh(mark1);
+
+                Ok(vec![
+                    tester
+                        .parse_stmt("actual1.js", "let a = 1;")?
+                        .fold_with(&mut marker(&[("a", mark1)])),
+                    tester
+                        .parse_stmt(
+                            "actual2.js",
+                            "function foo() {
+                                let a = 2;
+                                use({ a })
+                            }",
+                        )?
+                        .fold_with(&mut marker(&[("a", mark2)])),
+                ])
+            },
+            "
+            let a = 1;
+            function foo() {
+                let a1 = 2;
+                use({ a: a1 })
+            }
+            ",
+        );
+    }
+
+    #[test]
+    fn same_level() {
+        test(
+            |tester| {
+                let mark1 = Mark::fresh(Mark::root());
+
+                Ok(vec![
+                    tester
+                        .parse_stmt("actual1.js", "var a = 1;")?
+                        .fold_with(&mut marker(&[("a", mark1)])),
+                    tester
+                        .parse_stmt("actual2.js", "var a = 1;")?
+                        .fold_with(&mut marker(&[("a", mark1)])),
+                ])
+            },
+            "
+            var a = 1;
+            var a = 1;
+            ",
+        );
+    }
 }

ecmascript/transforms/src/scope.rs

@@ -1,5 +1,5 @@
 use ast::*;
-use fnv::FnvHashSet;
+use fnv::{FnvHashMap, FnvHashSet};
 use std::cell::{Cell, RefCell};
 use swc_atoms::JsWord;
 use swc_common::{Fold, FoldWith, SyntaxContext};
@@ -37,7 +37,7 @@ pub struct Scope<'a> {
     /// All references declared in this scope
     pub declared_refs: FnvHashSet<Ident>,
 
-    pub declared_symbols: FnvHashSet<JsWord>,
+    pub declared_symbols: FnvHashMap<JsWord, SyntaxContext>,
     /* /// All children of the this scope
      * pub children: Vec<Scope<'a>>, */
     pub(crate) ops: RefCell<Vec<ScopeOp>>,
@@ -45,24 +45,54 @@ pub struct Scope<'a> {
 
 pub struct Operator<'a>(&'a [ScopeOp]);
 
-impl<'a> Fold<Ident> for Operator<'a> {
-    fn fold(&mut self, ident: Ident) -> Ident {
+impl<'a> Fold<Prop> for Operator<'a> {
+    fn fold(&mut self, prop: Prop) -> Prop {
+        match prop {
+            Prop::Shorthand(i) => {
+                // preserve key
+                match self.rename_ident(i.clone()) {
+                    Ok(renamed) => Prop::KeyValue(KeyValueProp {
+                        key: PropName::Ident(Ident {
+                            // clear mark
+                            span: i.span.with_ctxt(SyntaxContext::empty()),
+                            ..i
+                        }),
+                        value: box Expr::Ident(renamed),
+                    }),
+                    Err(i) => Prop::Shorthand(i),
+                }
+            }
+            _ => prop.fold_children(self),
+        }
+    }
+}
+
+impl<'a> Operator<'a> {
+    /// Returns `Ok(renamed_ident)` if ident should be renamed.
+    fn rename_ident(&mut self, ident: Ident) -> Result<Ident, Ident> {
         for op in self.0 {
             match *op {
                 ScopeOp::Rename { ref from, ref to }
                     if *from.sym == ident.sym && from.span.ctxt() == ident.span.ctxt() =>
                 {
-                    return Ident {
+                    return Ok(Ident {
                         // Clear mark
                         span: ident.span.with_ctxt(SyntaxContext::empty()),
                         sym: to.clone(),
-                    };
+                    });
                 }
                 _ => {}
             }
         }
+        Err(ident)
+    }
+}
 
-        ident
+impl<'a> Fold<Ident> for Operator<'a> {
+    fn fold(&mut self, ident: Ident) -> Ident {
+        match self.rename_ident(ident) {
+            Ok(i) | Err(i) => i,
+        }
     }
 }
 
@@ -93,7 +123,7 @@ impl<'a> Scope<'a> {
     }
 
     pub(crate) fn is_declared(&self, sym: &JsWord) -> bool {
-        if self.declared_symbols.contains(sym) {
+        if self.declared_symbols.contains_key(sym) {
             return true;
         }
         for op in self.ops.borrow().iter() {