fix #445 by adjusting AST_Switch's scope

terser · Sep 7, 2019 · d0c5217 · d0c5217
1 parent 11094c1
commit d0c5217
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 15 deletions.
diff --git a/lib/scope.js b/lib/scope.js
@@ -81,6 +81,7 @@ import {
     AST_Sequence,
     AST_String,
     AST_Sub,
+    AST_Switch,
     AST_SwitchBranch,
     AST_Symbol,
     AST_SymbolBlockDeclaration,
@@ -213,7 +214,21 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options) {
                     for_scopes.push(scope);
                 }
             }
-            descend();
+
+            if (node instanceof AST_Switch) {
+                // XXX: HACK! Ensure the switch expression gets the correct scope (the parent scope) and the body gets the contained scope
+                // AST_Switch has a scope within the body, but it itself "is a block scope"
+                // This means the switched expression has to belong to the outer scope
+                // while the body inside belongs to the switch itself.
+                // This is pretty nasty and warrants an AST change similar to AST_Try (read above)
+                const the_block_scope = scope;
+                scope = save_scope;
+                node.expression.walk(tw);
+                scope = the_block_scope;
+                node.body.forEach(node => node.walk(tw));
+            } else {
+                descend();
+            }
             scope = save_scope;
             return true;
         }
@@ -276,9 +291,12 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options) {
             // This deals with the name of the class being available
             // inside the class.
             mark_export((node.scope = defun.parent_scope).def_function(node, defun), 1);
-        } else if (node instanceof AST_SymbolVar
+        } else if (
+            node instanceof AST_SymbolVar
             || node instanceof AST_SymbolLet
-            || node instanceof AST_SymbolConst) {
+            || node instanceof AST_SymbolConst
+            || node instanceof AST_SymbolCatch
+        ) {
             var def;
             if (node instanceof AST_SymbolBlockDeclaration) {
                 def = scope.def_variable(node, null);
@@ -309,8 +327,6 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options) {
                     node.reference(options);
                 }
             }
-        } else if (node instanceof AST_SymbolCatch) {
-            scope.def_variable(node);
         } else if (node instanceof AST_LabelRef) {
             var sym = labels.get(node.name);
             if (!sym) throw new Error(string_template("Undefined label {name} [{line},{col}]", {
@@ -415,8 +431,7 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options) {
     // https://github.com/mishoo/UglifyJS2/issues/1753
     // https://bugs.webkit.org/show_bug.cgi?id=171041
     if (options.safari10) {
-        for (var i = 0; i < for_scopes.length; i++) {
-            var scope = for_scopes[i];
+        for (const scope of for_scopes) {
             scope.parent_scope.variables.forEach(function(def) {
                 push_uniq(scope.enclosed, def);
             });

diff --git a/lib/transform.js b/lib/transform.js
@@ -91,19 +91,19 @@ import {
 
 function def_transform(node, descend) {
     node.DEFMETHOD("transform", function(tw, in_list) {
-        var x, y;
+        let transformed = undefined;
         tw.push(this);
-        if (tw.before) x = tw.before(this, descend, in_list);
-        if (x === undefined) {
-            x = this;
-            descend(x, tw);
+        if (tw.before) transformed = tw.before(this, descend, in_list);
+        if (transformed === undefined) {
+            transformed = this;
+            descend(transformed, tw);
             if (tw.after) {
-                y = tw.after(x, in_list);
-                if (y !== undefined) x = y;
+                const after_ret = tw.after(transformed, in_list);
+                if (after_ret !== undefined) transformed = after_ret;
             }
         }
         tw.pop();
-        return x;
+        return transformed;
     });
 }
 

diff --git a/test/compress/.eslintrc b/test/compress/.eslintrc
@@ -1,6 +1,8 @@
 {
     "globals": {
         "options": true,
+        "mangle": true,
+        "rename": true,
         "beautify": true,
         "console": false,
         "window": false,

diff --git a/test/compress/switch.js b/test/compress/switch.js
@@ -864,3 +864,24 @@ issue_1750: {
     }
     expect_stdout: "0 2"
 }
+
+issue_445: {
+    mangle = true;
+    input: {
+        const leak = () => {}
+
+        function scan() {
+            let len = leak();
+            let ch = 0;
+            switch (ch = 123) {
+                case "never-reached":
+                    const ch = leak();
+                    leak(ch);
+            }
+            return len === 123 ? "FAIL" : "PASS";
+        }
+
+        console.log(scan());
+    }
+    expect_stdout: "PASS"
+}