resolve #2842 by keeping track of block-scoped variables when looking for collisions in argument naming

lib/ast.js

@@ -164,16 +164,28 @@ function walk_body(node, visitor) {
     }
 };
 
-var AST_Block = DEFNODE("Block", "body", {
+function clone_block_scope(deep) {
+    var clone = this._clone(deep);
+    if (this.block_scope) {
+        // TODO this is sometimes undefined during compression.
+        // But it should always have a value!
+        clone.block_scope = this.block_scope.clone();
+    }
+    return clone;
+}
+
+var AST_Block = DEFNODE("Block", "body block_scope", {
     $documentation: "A body of statements (usually bracketed)",
     $propdoc: {
-        body: "[AST_Statement*] an array of statements"
+        body: "[AST_Statement*] an array of statements",
+        block_scope: "[AST_Scope] the block scope"
     },
     _walk: function(visitor) {
         return visitor._visit(this, function(){
             walk_body(this, visitor);
         });
-    }
+    },
+    clone: clone_block_scope
 }, AST_Statement);
 
 var AST_BlockStatement = DEFNODE("BlockStatement", null, {
@@ -219,8 +231,12 @@ var AST_LabeledStatement = DEFNODE("LabeledStatement", "label", {
     }
 }, AST_StatementWithBody);
 
-var AST_IterationStatement = DEFNODE("IterationStatement", null, {
+var AST_IterationStatement = DEFNODE("IterationStatement", "block_scope", {
-    $documentation: "Internal class.  All loops inherit from it."
+    $documentation: "Internal class.  All loops inherit from it.",
+    $propdoc: {
+        block_scope: "[AST_Scope] the block scope for this iteration statement."
+    },
+    clone: clone_block_scope
 }, AST_StatementWithBody);
 
 var AST_DWLoop = DEFNODE("DWLoop", "condition", {

lib/compress.js

@@ -4748,7 +4748,7 @@ merge(Compressor.prototype, {
             return return_value(stat);
         }
 
-        function can_inject_args(catches, safe_to_inject) {
+        function can_inject_args(block_scoped, safe_to_inject) {
             for (var i = 0, len = fn.argnames.length; i < len; i++) {
                 var arg = fn.argnames[i];
                 if (arg instanceof AST_DefaultAssign) {
@@ -4762,7 +4762,7 @@ merge(Compressor.prototype, {
                 }
                 if (arg.__unused) continue;
                 if (!safe_to_inject
-                    || catches[arg.name]
+                    || block_scoped[arg.name]
                     || identifier_atom(arg.name)
                     || scope.var_names()[arg.name]) {
                     return false;
@@ -4772,7 +4772,7 @@ merge(Compressor.prototype, {
             return true;
         }
 
-        function can_inject_vars(catches, safe_to_inject) {
+        function can_inject_vars(block_scoped, safe_to_inject) {
             var len = fn.body.length;
             for (var i = 0; i < len; i++) {
                 var stat = fn.body[i];
@@ -4780,7 +4780,7 @@ merge(Compressor.prototype, {
                 if (!safe_to_inject) return false;
                 for (var j = stat.definitions.length; --j >= 0;) {
                     var name = stat.definitions[j].name;
-                    if (catches[name.name]
+                    if (block_scoped[name.name]
                         || identifier_atom(name.name)
                         || scope.var_names()[name.name]) {
                         return false;
@@ -4792,11 +4792,20 @@ merge(Compressor.prototype, {
         }
 
         function can_inject_symbols() {
-            var catches = Object.create(null);
+            var block_scoped = Object.create(null);
             do {
                 scope = compressor.parent(++level);
+                if (scope.is_block_scope() && !(compressor.parent(level - 1) instanceof AST_Scope)) {
+                    if (scope.block_scope) {
+                        // TODO this is sometimes undefined during compression.
+                        // But it should always have a value!
+                        scope.block_scope.variables.each(function (variable) {
+                            block_scoped[variable.name] = true;
+                        });
+                    }
+                }
                 if (scope instanceof AST_Catch) {
-                    catches[scope.argname.name] = true;
+                    block_scoped[scope.argname.name] = true;
                 } else if (scope instanceof AST_IterationStatement) {
                     in_loop = [];
                 } else if (scope instanceof AST_SymbolRef) {
@@ -4805,8 +4814,8 @@ merge(Compressor.prototype, {
             } while (!(scope instanceof AST_Scope) || scope instanceof AST_Arrow);
             var safe_to_inject = !(scope instanceof AST_Toplevel) || compressor.toplevel.vars;
             var inline = compressor.option("inline");
-            if (!can_inject_vars(catches, inline >= 3 && safe_to_inject)) return false;
+            if (!can_inject_vars(block_scoped, inline >= 3 && safe_to_inject)) return false;
-            if (!can_inject_args(catches, inline >= 2 && safe_to_inject)) return false;
+            if (!can_inject_args(block_scoped, inline >= 2 && safe_to_inject)) return false;
             return !in_loop || in_loop.length == 0 || !is_reachable(fn, in_loop);
         }
 

test/compress/functions.js

@@ -2031,6 +2031,29 @@ inline_true: {
     ]
 }
 
+issue_2842: {
+    options = {
+        inline: true,
+        toplevel: true,
+        reduce_vars: true,
+        reduce_funcs: true,
+    }
+    input: {
+        {
+            const data = function (data) {
+                return data[data[0]];
+            }([1, 2, 3]);
+        }
+    }
+    expect: {
+        {
+            const data = function (data) {
+                return data[data[0]];
+            }([1, 2, 3]);
+        }
+    }
+}
+
 use_before_init_in_loop: {
     options = {
         inline: true,

test/mocha.js

@@ -22,3 +22,7 @@ module.exports = function() {
         });
     });
 };
+
+if (module.parent === null) {
+    module.exports();
+}

test/run-tests.js

@@ -18,6 +18,10 @@ if (failures) {
     console.error("!!! " + Object.keys(failed_files).join(", "));
     process.exit(1);
 }
+if (process.argv.length > 2) {
+    // User specified a specific compress/ test, don't run entire test suite
+    return;
+}
 
 var mocha_tests = require("./mocha.js");
 mocha_tests();