fix & enhance collapse_vars (#4447)

fixes #4446

lib/compress.js

@@ -1542,7 +1542,7 @@ merge(Compressor.prototype, {
                     && (scan_lhs && lhs.equivalent_to(node)
                         || scan_rhs && (hit_rhs = scan_rhs(node, this)))) {
                     if (!can_replace || stop_if_hit && (hit_rhs || !lhs_local || !replace_all)) {
-                        if (!hit_rhs || !value_def) abort = true;
+                        if (!hit_rhs && !value_def) abort = true;
                         return node;
                     }
                     if (is_lhs(node, parent)) {
@@ -1595,14 +1595,28 @@ merge(Compressor.prototype, {
                     can_replace = false;
                     descend(node, scanner);
                     can_replace = replace;
-                    return node;
+                    return signal_abort(node);
+                }
+                // Scan but don't replace inside destructuring LHS
+                if (node instanceof AST_Assign && node.left instanceof AST_Destructured) {
+                    var replace = can_replace;
+                    can_replace = false;
+                    node.left = node.left.transform(scanner);
+                    can_replace = replace;
+                    node.right = node.right.transform(scanner);
+                    return signal_abort(node);
+                }
+                // Scan but don't replace inside default value
+                if (node instanceof AST_DefaultValue) {
+                    node.name = node.name.transform(scanner);
+                    var replace = can_replace;
+                    can_replace = false;
+                    node.value = node.value.transform(scanner);
+                    can_replace = replace;
+                    return signal_abort(node);
                 }
                 return handle_custom_scan_order(node, scanner);
-            }, function(node) {
+            }, signal_abort);
-                if (abort) return;
-                if (stop_after === node) abort = true;
-                if (stop_if_hit === node) stop_if_hit = null;
-            });
             var multi_replacer = new TreeTransformer(function(node) {
                 if (abort) return node;
                 // Skip nodes before `candidate` as quickly as possible
@@ -1723,6 +1737,13 @@ merge(Compressor.prototype, {
                 }
             }
 
+            function signal_abort(node) {
+                if (abort) return node;
+                if (stop_after === node) abort = true;
+                if (stop_if_hit === node) stop_if_hit = null;
+                return node;
+            }
+
             function handle_custom_scan_order(node, tt) {
                 if (!(node instanceof AST_BlockScope)) return;
                 // Skip (non-executed) functions
@@ -1760,6 +1781,13 @@ merge(Compressor.prototype, {
                 }
             }
 
+            function is_direct_assignment(node, parent) {
+                if (parent instanceof AST_Assign) return parent.operator == "=" && parent.left === node;
+                if (parent instanceof AST_DefaultValue) return parent.name === node;
+                if (parent instanceof AST_DestructuredArray) return true;
+                if (parent instanceof AST_DestructuredKeyVal) return parent.value === node;
+            }
+
             function should_stop(node, parent) {
                 if (node === rvalue) return true;
                 if (parent instanceof AST_For) return node !== parent.init;
@@ -1773,7 +1801,7 @@ merge(Compressor.prototype, {
                 }
                 if (node instanceof AST_Debugger) return true;
                 if (node instanceof AST_Defun) return funarg && lhs.name === node.name.name;
-                if (node instanceof AST_Destructured) return parent instanceof AST_Assign;
+                if (node instanceof AST_Destructured) return (in_try || !lhs_local) && parent instanceof AST_Assign;
                 if (node instanceof AST_DestructuredKeyVal) return node.key instanceof AST_Node;
                 if (node instanceof AST_DWLoop) return true;
                 if (node instanceof AST_LoopControl) return true;
@@ -1784,7 +1812,7 @@ merge(Compressor.prototype, {
                             return !(parent instanceof AST_PropAccess && parent.expression === node)
                                 && is_arguments(node.definition());
                         }
-                    } else if (parent instanceof AST_Assign && parent.operator == "=" && parent.left === node) {
+                    } else if (is_direct_assignment(node, parent)) {
                         return false;
                     }
                     if (!replace_all) return true;
@@ -1864,9 +1892,7 @@ merge(Compressor.prototype, {
                 }
                 if (node instanceof AST_Spread) return true;
                 if (node instanceof AST_SymbolRef) {
-                    if (symbol_in_lvalues(node, parent)) {
+                    if (symbol_in_lvalues(node, parent)) return !is_direct_assignment(node, parent);
-                        return !(parent instanceof AST_Assign && parent.operator == "=" && parent.left === node);
-                    }
                     if (side_effects && may_modify(node)) return true;
                     var def = node.definition();
                     return (in_try || def.scope.resolve() !== scope) && !can_drop_symbol(node);

test/compress/collapse_vars.js

@@ -4273,8 +4273,8 @@ issue_2436_14: {
         var b = {};
         (function() {
             a && function(c, d) {
-                console.log(c, d);
+                console.log(b, d);
-            }(b, a);
+            }(0, a);
         })();
     }
     expect_stdout: true

test/compress/default-values.js

@@ -974,3 +974,41 @@ issue_4444: {
     expect_stdout: "PASS"
     node_version: ">=6"
 }
+
+issue_4446_1: {
+    options = {
+        collapse_vars: true,
+    }
+    input: {
+        a = 42;
+        [ b = 42 ] = [ "PASS" ];
+        c = 42;
+        console.log(b, a);
+    }
+    expect: {
+        [ b = 42 ] = [ "PASS" ];
+        c = a = 42;
+        console.log(b, a);
+    }
+    expect_stdout: "PASS 42"
+    node_version: ">=6"
+}
+
+issue_4446_2: {
+    options = {
+        collapse_vars: true,
+    }
+    input: {
+        a = 42;
+        var [ b = 42 ] = [ "PASS" ];
+        c = 42;
+        console.log(b, a);
+    }
+    expect: {
+        var [ b = 42 ] = [ "PASS" ];
+        c = a = 42;
+        console.log(b, a);
+    }
+    expect_stdout: "PASS 42"
+    node_version: ">=6"
+}

test/compress/destructured.js

@@ -2146,3 +2146,21 @@ issue_4436_undefined: {
     expect_stdout: true
     node_version: ">=6"
 }
+
+issue_4446: {
+    options = {
+        collapse_vars: true,
+    }
+    input: {
+        a = "PASS";
+        var a = [ a[0] ] = [ a ];
+        console.log(a[0]);
+    }
+    expect: {
+        a = "PASS";
+        var a = [ a[0] ] = [ a ];
+        console.log(a[0]);
+    }
+    expect_stdout: "PASS"
+    node_version: ">=6"
+}

test/reduce.js

@@ -153,6 +153,20 @@ module.exports = function reduce_test(testcase, minify_options, reduce_options)
                     node.left,
                     node.right,
                 ][ permute & 1 ];
+                if (expr instanceof U.AST_Destructured) expr = expr.transform(new U.TreeTransformer(function(node, descend) {
+                    if (node instanceof U.AST_DefaultValue) return new U.AST_Assign({
+                        operator: "=",
+                        left: node.name.transform(this),
+                        right: node.value,
+                        start: {},
+                    });
+                    if (node instanceof U.AST_DestructuredKeyVal) return new U.AST_ObjectKeyVal(node);
+                    if (node instanceof U.AST_Destructured) {
+                        node = new (node instanceof U.AST_DestructuredArray ? U.AST_Array : U.AST_Object)(node);
+                        descend(node, this);
+                    }
+                    return node;
+                }));
                 CHANGED = true;
                 return permute < 2 ? expr : wrap_with_console_log(expr);
             }

test/ufuzz/index.js

@@ -1680,18 +1680,20 @@ function log(options) {
         }
     }
     errorln("//-------------------------------------------------------------");
-    var reduce_options = JSON.parse(options);
+    if (!ok) {
-    reduce_options.validate = true;
+        var reduce_options = JSON.parse(options);
-    var reduced = reduce_test(original_code, reduce_options, {
+        reduce_options.validate = true;
-        verbose: false,
+        var reduced = reduce_test(original_code, reduce_options, {
-    }).code;
+            verbose: false,
-    if (reduced) {
+        }).code;
-        errorln();
+        if (reduced) {
-        errorln("// reduced test case (output will differ)");
+            errorln();
-        errorln();
+            errorln("// reduced test case (output will differ)");
-        errorln(reduced);
+            errorln();
-        errorln();
+            errorln(reduced);
-        errorln("//-------------------------------------------------------------");
+            errorln();
+            errorln("//-------------------------------------------------------------");
+        }
     }
     errorln("minify(options):");
     errorln(JSON.stringify(JSON.parse(options), null, 2));

test/ufuzz/job.js

@@ -70,7 +70,7 @@ function run() {
 
     function trap(data) {
         stderr += data;
-        if (~stderr.indexOf("\nminify(options):\n")) {
+        if (~stderr.indexOf("!!!!!! Failed... round ")) {
             process.exitCode = 1;
             child.stderr.removeListener("data", trap);
         }