From 6605d1578351939ee0e39a13bf68cc9c1708c918 Mon Sep 17 00:00:00 2001 From: Anthony Van de Gejuchte Date: Sun, 10 Jan 2016 23:33:54 +0100 Subject: [PATCH 1/3] Never mangle arguments and keep them in their scope Fixes #892 Helped-by: kzc --- lib/scope.js | 8 ++++++++ test/compress/issue-892.js | 32 ++++++++++++++++++++++++++++++++ test/run-tests.js | 4 ++++ 3 files changed, 44 insertions(+) create mode 100644 test/compress/issue-892.js 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/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", { From 5c4e470d43438e359fbdb93950e5d37d4df45a69 Mon Sep 17 00:00:00 2001 From: Anthony Van de Gejuchte Date: Thu, 14 Jan 2016 22:32:46 +0100 Subject: [PATCH 2/3] Add scope test for arguments --- test/mocha/arguments.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 test/mocha/arguments.js diff --git a/test/mocha/arguments.js b/test/mocha/arguments.js new file mode 100644 index 00000000..294a6c16 --- /dev/null +++ b/test/mocha/arguments.js @@ -0,0 +1,19 @@ +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(); + + // Select 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 From 8439c8ba9813faaea062b64306cbd0b2a448bb20 Mon Sep 17 00:00:00 2001 From: Anthony Van de Gejuchte Date: Fri, 15 Jan 2016 00:04:05 +0100 Subject: [PATCH 3/3] Make arguments test slightly more strict --- test/mocha/arguments.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/mocha/arguments.js b/test/mocha/arguments.js index 294a6c16..089826fc 100644 --- a/test/mocha/arguments.js +++ b/test/mocha/arguments.js @@ -6,7 +6,10 @@ describe("arguments", function() { var ast = UglifyJS.parse("var arguments; var f = function() {arguments.length}"); ast.figure_out_scope(); - // Select symbol in function + // 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);