Merge pull request #918 from avdg/fix-arguments-handling

Never mangle arguments and keep them in their scope

4 files changed, 66 insertions, 0 deletions

diff --git a/lib/scope.js b/lib/scope.js
index 1f0986c4..5e93020f 100644
--- a/lib/scope.js
+++ b/lib/scope.js
@@ -237,6 +237,10 @@ AST_Scope.DEFMETHOD("init_scope_vars", function(nesting){
 AST_Lambda.DEFMETHOD("init_scope_vars", function(){
     AST_Scope.prototype.init_scope_vars.apply(this, arguments);
     this.uses_arguments = false;
+
+    var symbol = new AST_VarDef({ name: "arguments" });
+    var def = new SymbolDef(this, this.variables.size(), symbol);
+    this.variables.set(symbol.name, def);
 });
 
 AST_SymbolRef.DEFMETHOD("reference", function() {
@@ -366,6 +370,10 @@ AST_Toplevel.DEFMETHOD("_default_mangler_options", function(options){
 
 AST_Toplevel.DEFMETHOD("mangle_names", function(options){
     options = this._default_mangler_options(options);
+
+    // Never mangle arguments
+    options.except.push('arguments');
+
     // We only need to mangle declaration nodes.  Special logic wired
     // into the code generator will display the mangled name if it's
     // present (and for AST_SymbolRef-s it'll use the mangled name of
diff --git a/test/compress/issue-892.js b/test/compress/issue-892.js
new file mode 100644
index 00000000..2dab420f
--- /dev/null
+++ b/test/compress/issue-892.js
@@ -0,0 +1,32 @@
+dont_mangle_arguments: {
+    mangle = {
+    };
+    options = {
+        sequences     : true,
+        properties    : true,
+        dead_code     : true,
+        drop_debugger : true,
+        conditionals  : true,
+        comparisons   : true,
+        evaluate      : true,
+        booleans      : true,
+        loops         : true,
+        unused        : true,
+        hoist_funs    : true,
+        keep_fargs    : true,
+        keep_fnames   : false,
+        hoist_vars    : true,
+        if_return     : true,
+        join_vars     : true,
+        cascade       : true,
+        side_effects  : true,
+        negate_iife   : false
+    };
+    input: {
+        (function(){
+            var arguments = arguments, not_arguments = 9;
+            console.log(not_arguments, arguments);
+        })(5,6,7);
+    }
+    expect_exact: "(function(){var arguments=arguments,o=9;console.log(o,arguments)})(5,6,7);"
+}
diff --git a/test/mocha/arguments.js b/test/mocha/arguments.js
new file mode 100644
index 00000000..089826fc
--- /dev/null
+++ b/test/mocha/arguments.js
@@ -0,0 +1,22 @@
+var UglifyJS = require('../../');
+var assert = require("assert");
+
+describe("arguments", function() {
+    it("Should known that arguments in functions are local scoped", function() {
+        var ast = UglifyJS.parse("var arguments; var f = function() {arguments.length}");
+        ast.figure_out_scope();
+
+        // Test scope of `var arguments`
+        assert.strictEqual(ast.find_variable("arguments").global, true);
+
+        // Select arguments symbol in function
+        var symbol = ast.body[1].definitions[0].value.find_variable("arguments");
+
+        assert.strictEqual(symbol.global, false);
+        assert.strictEqual(symbol.scope, ast. // From ast
+            body[1]. // Select 2nd statement (equals to `var f ...`)
+            definitions[0]. // First definition of selected statement
+            value // Select function as scope
+        );
+    });
+});
\ No newline at end of file
diff --git a/test/run-tests.js b/test/run-tests.js
index b9a0f825..fcb1b375 100755
--- a/test/run-tests.js
+++ b/test/run-tests.js
@@ -103,6 +103,10 @@ function run_compress_tests() {
             }
             var output = input.transform(cmp);
             output.figure_out_scope();
+            if (test.mangle) {
+                output.compute_char_frequency(test.mangle);
+                output.mangle_names(test.mangle);
+            }
             output = make_code(output, output_options);
             if (expect != output) {
                 log("!!! failed\n---INPUT---\n{input}\n---OUTPUT---\n{output}\n---EXPECTED---\n{expected}\n\n", {