From bb9c9707aa6b4c625ad985798aea879080411ce1 Mon Sep 17 00:00:00 2001 From: Anthony Van de Gejuchte Date: Sun, 12 Jun 2016 18:58:20 +0200 Subject: [PATCH 01/10] Don't allow with statements in strict mode --- lib/parse.js | 3 +++ test/mocha/with.js | 16 ++++++++++++++++ 2 files changed, 19 insertions(+) create mode 100644 test/mocha/with.js diff --git a/lib/parse.js b/lib/parse.js index 4530c2d9..2c007965 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -928,6 +928,9 @@ function parse($TEXT, options) { return tmp = const_(), semicolon(), tmp; case "with": + if (S.input.has_directive("use strict")) { + croak("Strict mode may not include a with statement"); + } return new AST_With({ expression : parenthesised(), body : statement() diff --git a/test/mocha/with.js b/test/mocha/with.js new file mode 100644 index 00000000..d284f1c2 --- /dev/null +++ b/test/mocha/with.js @@ -0,0 +1,16 @@ +var assert = require("assert"); +var uglify = require("../../"); + +describe("With", function() { + it ("Should throw syntaxError when using with statement in strict mode", function() { + var code = '"use strict";\nthrow NotEarlyError;\nwith ({}) { }'; + var test = function() { + uglify.parse(code); + } + var error = function(e) { + return e instanceof uglify.JS_Parse_Error && + e.message === "Strict mode may not include a with statement"; + } + assert.throws(test, error); + }); +}); \ No newline at end of file From 5c4cfaa0a75317582c979f9b50c1e562fbcfa40d Mon Sep 17 00:00:00 2001 From: Anthony Van de Gejuchte Date: Sun, 12 Jun 2016 17:34:05 +0200 Subject: [PATCH 02/10] Re-add parens after new expression in beautify mode --- lib/output.js | 4 ++- test/mocha/new.js | 62 +++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 55 insertions(+), 11 deletions(-) diff --git a/lib/output.js b/lib/output.js index 7ddee484..6eae2f1f 100644 --- a/lib/output.js +++ b/lib/output.js @@ -1287,7 +1287,9 @@ function OutputStream(options) { // self should be AST_New. decide if we want to show parens or not. function need_constructor_parens(self, output) { // Always print parentheses with arguments - return self.args.length > 0; + if (self.args.length > 0) return true; + + return output.option("beautify"); }; function best_of(a) { diff --git a/test/mocha/new.js b/test/mocha/new.js index 8c0f24bc..083b9964 100644 --- a/test/mocha/new.js +++ b/test/mocha/new.js @@ -2,7 +2,49 @@ var assert = require("assert"); var uglify = require("../../"); describe("New", function() { - it("Should attach callback parens after some tokens", function() { + it("Should add trailing parentheses for new expressions with zero arguments in beautify mode", function() { + var tests = [ + "new x(1);", + "new x;", + "new new x;", + "new (function(foo){this.foo=foo;})(1);", + "new (function(foo){this.foo=foo;})();", + "new (function test(foo){this.foo=foo;})(1);", + "new (function test(foo){this.foo=foo;})();", + "new true;", + "new (0);", + "new (!0);", + "new (bar = function(foo) {this.foo=foo;})(123);", + "new (bar = function(foo) {this.foo=foo;})();" + ]; + var expected = [ + "new x(1);", + "new x();", + "new new x()();", + "new function(foo) {\n this.foo = foo;\n}(1);", + "new function(foo) {\n this.foo = foo;\n}();", + "new function test(foo) {\n this.foo = foo;\n}(1);", + "new function test(foo) {\n this.foo = foo;\n}();", + "new true();", + "new 0();", + "new (!0)();", + "new (bar = function(foo) {\n this.foo = foo;\n})(123);", + "new (bar = function(foo) {\n this.foo = foo;\n})();" + ]; + for (var i = 0; i < tests.length; i++) { + assert.strictEqual( + uglify.minify(tests[i], { + fromString: true, + output: {beautify: true}, + compress: false, + mangle: false + }).code, + expected[i] + ); + } + }); + + it("Should not add trailing parentheses for new expressions with zero arguments in non-beautify mode", function() { var tests = [ "new x(1);", "new x;", @@ -20,22 +62,22 @@ describe("New", function() { var expected = [ "new x(1);", "new x;", - "new (new x);", - "new function(foo) {\n this.foo = foo;\n}(1);", - "new function(foo) {\n this.foo = foo;\n};", - "new function test(foo) {\n this.foo = foo;\n}(1);", - "new function test(foo) {\n this.foo = foo;\n};", + "new(new x);", + "new function(foo){this.foo=foo}(1);", + "new function(foo){this.foo=foo};", + "new function test(foo){this.foo=foo}(1);", + "new function test(foo){this.foo=foo};", "new true;", "new 0;", - "new (!0);", - "new (bar = function(foo) {\n this.foo = foo;\n})(123);", - "new (bar = function(foo) {\n this.foo = foo;\n});" + "new(!0);", + "new(bar=function(foo){this.foo=foo})(123);", + "new(bar=function(foo){this.foo=foo});" ]; for (var i = 0; i < tests.length; i++) { assert.strictEqual( uglify.minify(tests[i], { fromString: true, - output: {beautify: true}, + output: {beautify: false}, compress: false, mangle: false }).code, From d7971ba0e439eb042fd880762bc7fa77ce1f3e73 Mon Sep 17 00:00:00 2001 From: Anthony Van de Gejuchte Date: Mon, 13 Jun 2016 18:19:06 +0200 Subject: [PATCH 03/10] Fix test262 failures related to <, <=, in and instanceof Fixed-by: @kzc --- README.md | 13 +++++-- lib/compress.js | 10 ++--- test/compress/asm.js | 2 +- test/compress/comparing.js | 76 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 92 insertions(+), 9 deletions(-) create mode 100644 test/compress/comparing.js diff --git a/README.md b/README.md index 041a0709..b0ac7121 100644 --- a/README.md +++ b/README.md @@ -291,12 +291,19 @@ to set `true`; it's effectively a shortcut for `foo=true`). - `unsafe` (default: false) -- apply "unsafe" transformations (discussion below) +- `unsafe_comps` (default: false) -- Reverse `<` and `<=` to `>` and `>=` to + allow improved compression. This might be unsafe when an at least one of two + operands is an object with computed values due the use of methods like `get`, + or `valueOf`. This could cause change in execution order after operands in the + comparison are switching. Compression only works if both `comparisons` and + `unsafe_comps` are both set to true. + - `conditionals` -- apply optimizations for `if`-s and conditional expressions - `comparisons` -- apply certain optimizations to binary nodes, for example: - `!(a <= b) → a > b` (only when `unsafe`), attempts to negate binary nodes, - e.g. `a = !b && !c && !d && !e → a=!(b||c||d||e)` etc. + `!(a <= b) → a > b` (only when `unsafe_comps`), attempts to negate binary + nodes, e.g. `a = !b && !c && !d && !e → a=!(b||c||d||e)` etc. - `evaluate` -- attempt to evaluate constant expressions @@ -415,7 +422,7 @@ them. If you are targeting < ES6 environments, use `/** @const */ var`. #### Conditional compilation, API -You can also use conditional compilation via the programmatic API. With the difference that the +You can also use conditional compilation via the programmatic API. With the difference that the property name is `global_defs` and is a compressor property: ```js diff --git a/lib/compress.js b/lib/compress.js index 4e04e961..3200a66f 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -1079,8 +1079,6 @@ merge(Compressor.prototype, { case "<=" : return ev(left, c) <= ev(right, c); case ">" : return ev(left, c) > ev(right, c); case ">=" : return ev(left, c) >= ev(right, c); - case "in" : return ev(left, c) in ev(right, c); - case "instanceof" : return ev(left, c) instanceof ev(right, c); } throw def; }); @@ -2502,9 +2500,11 @@ merge(Compressor.prototype, { }); self = best_of(self, negated); } - switch (self.operator) { - case "<": reverse(">"); break; - case "<=": reverse(">="); break; + if (compressor.option("unsafe_comps")) { + switch (self.operator) { + case "<": reverse(">"); break; + case "<=": reverse(">="); break; + } } } if (self.operator == "+" && self.right instanceof AST_String diff --git a/test/compress/asm.js b/test/compress/asm.js index c3018485..f392e78e 100644 --- a/test/compress/asm.js +++ b/test/compress/asm.js @@ -92,7 +92,7 @@ asm_mixed: { function logSum(start, end) { start = 0 | start, end = 0 | end; var sum = 0, p = 0, q = 0; - for (p = start << 3, q = end << 3; (0 | q) > (0 | p); p = p + 8 | 0) sum += +log(values[p >> 3]); + for (p = start << 3, q = end << 3; (0 | p) < (0 | q); p = p + 8 | 0) sum += +log(values[p >> 3]); return +sum; } function geometricMean(start, end) { diff --git a/test/compress/comparing.js b/test/compress/comparing.js new file mode 100644 index 00000000..c51fac31 --- /dev/null +++ b/test/compress/comparing.js @@ -0,0 +1,76 @@ +keep_comparisons: { + options = { + comparisons: true, + unsafe_comps: false + } + input: { + var obj1 = { + valueOf: function() {triggeredFirst();} + } + var obj2 = { + valueOf: function() {triggeredSecond();} + } + var result1 = obj1 <= obj2; + var result2 = obj1 < obj2; + var result3 = obj1 >= obj2; + var result4 = obj1 > obj2; + } + expect: { + var obj1 = { + valueOf: function() {triggeredFirst();} + } + var obj2 = { + valueOf: function() {triggeredSecond();} + } + var result1 = obj1 <= obj2; + var result2 = obj1 < obj2; + var result3 = obj1 >= obj2; + var result4 = obj1 > obj2; + } +} + +keep_comparisons_with_unsafe_comps: { + options = { + comparisons: true, + unsafe_comps: true + } + input: { + var obj1 = { + valueOf: function() {triggeredFirst();} + } + var obj2 = { + valueOf: function() {triggeredSecond();} + } + var result1 = obj1 <= obj2; + var result2 = obj1 < obj2; + var result3 = obj1 >= obj2; + var result4 = obj1 > obj2; + } + expect: { + var obj1 = { + valueOf: function() {triggeredFirst();} + } + var obj2 = { + valueOf: function() {triggeredSecond();} + } + var result1 = obj2 >= obj1; + var result2 = obj2 > obj1; + var result3 = obj1 >= obj2; + var result4 = obj1 > obj2; + } +} + +dont_change_in_or_instanceof_expressions: { + input: { + 1 in 1; + null in null; + 1 instanceof 1; + null instanceof null; + } + expect: { + 1 in 1; + null in null; + 1 instanceof 1; + null instanceof null; + } +} \ No newline at end of file From 2149bfb7071dedbcd42d2e245bb9c2f6b41a43bc Mon Sep 17 00:00:00 2001 From: Anthony Van de Gejuchte Date: Mon, 13 Jun 2016 21:11:08 +0200 Subject: [PATCH 04/10] Don't mix strings with directives in output * Don't interpret strings with escaped content as directive * Don't interpret strings after empty statement as directive * Adapt output to prevent strings being represent as directive * Introduce UGLIFY_DEBUG to allow internal testing like EXPECT_DIRECTIVE --- README.md | 2 +- lib/output.js | 42 ++++- lib/parse.js | 8 +- test/mocha/directives.js | 309 +++++++++++++++++++++++++++++++++++ test/mocha/string-literal.js | 4 +- test/run-tests.js | 2 + tools/exports.js | 4 + tools/node.js | 5 +- 8 files changed, 361 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index b0ac7121..51d0b629 100644 --- a/README.md +++ b/README.md @@ -452,7 +452,7 @@ can pass additional arguments that control the code output: objects - `space-colon` (default `true`) -- insert a space after the colon signs - `ascii-only` (default `false`) -- escape Unicode characters in strings and - regexps + regexps (affects directives with non-ascii characters becoming invalid) - `inline-script` (default `false`) -- escape the slash in occurrences of ` 0) output.with_block(function(){ - display_body(body, false, output); + display_body(body, false, output, allow_directives); }); else output.print("{}"); }; @@ -779,7 +807,7 @@ function OutputStream(options) { }); }); output.space(); - print_bracketed(self.body, output); + print_bracketed(self.body, output, true); }); DEFPRINT(AST_Lambda, function(self, output){ self._do_print(output); @@ -1185,7 +1213,7 @@ function OutputStream(options) { output.print(self.getValue()); }); DEFPRINT(AST_String, function(self, output){ - output.print_string(self.getValue(), self.quote); + output.print_string(self.getValue(), self.quote, in_directive); }); DEFPRINT(AST_Number, function(self, output){ if (use_asm && self.start.raw != null) { diff --git a/lib/parse.js b/lib/parse.js index 2c007965..9b5a142b 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -815,9 +815,10 @@ function parse($TEXT, options) { handle_regexp(); switch (S.token.type) { case "string": - if (S.in_directives) { - if (is_token(peek(), "punc", ";") || peek().nlb) { - S.input.add_directive(S.token.raw.substr(1, S.token.raw.length - 2)); + var dir = false; + if (S.in_directives === true) { + if ((is_token(peek(), "punc", ";") || peek().nlb) && S.token.raw.indexOf("\\") === -1) { + S.input.add_directive(S.token.value); } else { S.in_directives = false; } @@ -855,6 +856,7 @@ function parse($TEXT, options) { case "(": return simple_statement(); case ";": + S.in_directives = false; next(); return new AST_EmptyStatement(); default: diff --git a/test/mocha/directives.js b/test/mocha/directives.js index 4433e429..604b0329 100644 --- a/test/mocha/directives.js +++ b/test/mocha/directives.js @@ -58,4 +58,313 @@ describe("Directives", function() { assert.strictEqual(tokenizer.has_directive("use asm"), false); assert.strictEqual(tokenizer.has_directive("use thing"), false); }); + + it("Should know which strings are directive and which ones are not", function() { + var test_directive = function(tokenizer, test) { + test.directives.map(function(directive) { + assert.strictEqual(tokenizer.has_directive(directive), true, directive + " in " + test.input); + }); + test.non_directives.map(function(fake_directive) { + assert.strictEqual(tokenizer.has_directive(fake_directive), false, fake_directive + " in " + test.input); + }); + } + + var tests = [ + { + input: '"use strict"\n', + directives: ["use strict"], + non_directives: ["use asm"] + }, + { + input: '"use\\\nstrict";', + directives: [], + non_directives: ["use strict", "use\nstrict", "use \nstrict", "use asm"] + }, + { + input: '"use strict"\n"use asm"\n"use bar"\n', + directives: ["use strict", "use asm", "use bar"], + non_directives: ["use foo", "use\\x20strict"] + }, + { + input: '"use \\\nstrict";"use strict";', + directives: [], + non_directives: ["use strict", "use\nstrict", "use \nstrict", "use asm"] + }, + { + input: '"\\76";', + directives: [], + non_directives: [">", "\\76"] + }, + { + input: '"use strict"', // no ; or newline + directives: [], + non_directives: ["use strict", "use\nstrict", "use \nstrict", "use asm"] + }, + { + input: ';"use strict"', + directives: [], + non_directives: ["use strict", "use\nstrict", "use \nstrict", "use asm"] + }, + // Duplicate above code but put it in a function + { + input: 'function foo() {"use strict"\n', + directives: ["use strict"], + non_directives: ["use asm"] + }, + { + input: 'function foo() {"use\\\nstrict";', + directives: [], + non_directives: ["use strict", "use\nstrict", "use \nstrict", "use asm"] + }, + { + input: 'function foo() {"use strict"\n"use asm"\n"use bar"\n', + directives: ["use strict", "use asm", "use bar"], + non_directives: ["use foo", "use\\x20strict"] + }, + { + input: 'function foo() {"use \\\nstrict";"use strict";', + directives: [], + non_directives: ["use strict", "use\nstrict", "use \nstrict", "use asm"] + }, + { + input: 'var foo = function() {"\\76";', + directives: [], + non_directives: [">", "\\76"] + }, + { + input: 'var foo = function() {"use strict"', // no ; or newline + directives: [], + non_directives: ["use strict", "use\nstrict", "use \nstrict", "use asm"] + }, + { + input: 'var foo = function() {;"use strict"', + directives: [], + non_directives: ["use strict", "use\nstrict", "use \nstrict", "use asm"] + }, + // Special cases + { + input: '"1";"2";"3";"4";;"5"', + directives: ["1", "2", "3", "4"], + non_directives: ["5", "6", "use strict", "use asm"] + }, + { + input: 'if(1){"use strict";', + directives: [], + non_directives: ["use strict", "use\nstrict", "use \nstrict", "use asm"] + }, + { + input: '"use strict";try{"use asm";', + directives: ["use strict"], + non_directives: ["use\nstrict", "use \nstrict", "use asm"] + } + ]; + + for (var i = 0; i < tests.length; i++) { + // Fail parser deliberately to get state at failure + var tokenizer = uglify.tokenizer(tests[i].input + "]", "foo.js"); + + try { + var parser = uglify.parse(tokenizer); + throw new Error("Expected parser to fail"); + } catch (e) { + assert.strictEqual(e instanceof uglify.JS_Parse_Error, true); + assert.strictEqual(e.message, "Unexpected token: punc (])"); + } + + test_directive(tokenizer, tests[i]); + } + }); + + it("Should test EXPECT_DIRECTIVE RegExp", function() { + var tests = [ + ["", true], + ["'test';", true], + ["'test';;", true], + ["'tests';\n", true], + ["'tests'", false], + ["'tests'; \n\t", true], + ["'tests';\n\n", true], + ["\n\n\"use strict\";\n\n", true] + ]; + + for (var i = 0; i < tests.length; i++) { + assert.strictEqual(uglify.EXPECT_DIRECTIVE.test(tests[i][0]), tests[i][1], tests[i][0]); + } + }); + + it("Should only print 2 semicolons spread over 2 lines in beautify mode", function() { + assert.strictEqual( + uglify.minify( + '"use strict";\'use strict\';"use strict";"use strict";;\'use strict\';console.log(\'use strict\');', + {fromString: true, output: {beautify: true, quote_style: 3}, compress: false} + ).code, + '"use strict";\n\n\'use strict\';\n\n"use strict";\n\n"use strict";\n\n;\'use strict\';\n\nconsole.log(\'use strict\');' + ); + }); + + it("Should not add double semicolons in non-scoped block statements to avoid strings becoming directives", function() { + var tests = [ + [ + '{"use\x20strict"}', + '{"use strict"}' + ], + [ + 'function foo(){"use\x20strict";}', // Valid place for directives + 'function foo(){"use strict"}' + ], + [ + 'try{"use\x20strict"}catch(e){}finally{"use\x20strict"}', + 'try{"use strict"}catch(e){}finally{"use strict"}' + ], + [ + 'if(1){"use\x20strict"} else {"use strict"}', + 'if(1){"use strict"}else{"use strict"}' + ] + ]; + + for (var i = 0; i < tests.length; i++) { + assert.strictEqual( + uglify.minify(tests[i][0], {fromString: true, quote_style: 3, compress: false}).code, + tests[i][1], + tests[i][0] + ); + } + }); + + it("Should add double semicolon when relying on automatic semicolon insertion", function() { + var code = uglify.minify('"use strict";"use\\x20strict";', + {fromString: true, output: {semicolons: false}, compress: false} + ).code; + assert.strictEqual(code, '"use strict";;"use strict"\n'); + }); + + it("Should check quote style of directives", function() { + var tests = [ + // 0. Prefer double quotes, unless string contains more double quotes than single quotes + [ + '"testing something";', + 0, + '"testing something";' + ], + [ + "'use strict';", + 0, + '"use strict";' + ], + [ + '"\\\'use strict\\\'";', // Not a directive as it contains quotes + 0, + ';"\'use strict\'";', + ], + [ + "'\"use strict\"';", + 0, + "'\"use strict\"';", + ], + // 1. Always use single quote + [ + '"testing something";', + 1, + "'testing something';" + ], + [ + "'use strict';", + 1, + "'use strict';" + ], + [ + '"\'use strict\'";', + 1, + // Intentionally causes directive breakage at cost of less logic, usage should be rare anyway + "'\\'use strict\\'';", + ], + [ + "'\\'use strict\\'';", // Not a valid directive + 1, + "'\\'use strict\\'';" // But no ; necessary as directive stays invalid + ], + [ + "'\"use strict\"';", + 1, + "'\"use strict\"';", + ], + // 2. Always use double quote + [ + '"testing something";', + 2, + '"testing something";' + ], + [ + "'use strict';", + 2, + '"use strict";' + ], + [ + '"\'use strict\'";', + 2, + "\"'use strict'\";", + ], + [ + "'\"use strict\"';", + 2, + // Intentionally causes directive breakage at cost of less logic, usage should be rare anyway + '"\\\"use strict\\\"";', + ], + [ + '"\\"use strict\\"";', // Not a valid directive + 2, + '"\\"use strict\\"";' // But no ; necessary as directive stays invalid + ], + // 3. Always use original + [ + '"testing something";', + 3, + '"testing something";' + ], + [ + "'use strict';", + 3, + "'use strict';", + ], + [ + '"\'use strict\'";', + 3, + '"\'use strict\'";', + ], + [ + "'\"use strict\"';", + 3, + "'\"use strict\"';", + ], + ]; + for (var i = 0; i < tests.length; i++) { + assert.strictEqual( + uglify.minify(tests[i][0], {fromString: true, output:{quote_style: tests[i][1]}, compress: false}).code, + tests[i][2], + tests[i][0] + " using mode " + tests[i][1] + ); + } + }); + it("Should be able to compress without side effects", function() { + // NOTE: the "use asm" directive disables any optimisation after being defined + var tests = [ + [ + '"use strict";"use strict";"use strict";"use foo";"use strict";;"use sloppy";doSomething("foo");', + '"use strict";"use foo";doSomething("foo");' + ], + [ + // Nothing gets optimised in the compressor because "use asm" is the first statement + '"use asm";"use\\x20strict";1+1;', + '"use asm";;"use strict";1+1;' // Yet, the parser noticed that "use strict" wasn't a directive + ] + ]; + + for (var i = 0; i < tests.length; i++) { + assert.strictEqual( + uglify.minify(tests[i][0], {fromString: true, compress: {collapse_vars: true, side_effects: true}}).code, + tests[i][1], + tests[i][0] + ); + } + }); }); \ No newline at end of file diff --git a/test/mocha/string-literal.js b/test/mocha/string-literal.js index b363a07f..ea984213 100644 --- a/test/mocha/string-literal.js +++ b/test/mocha/string-literal.js @@ -59,13 +59,13 @@ describe("String literals", function() { it("Should not throw error outside strict mode if string contains escaped octalIntegerLiteral", function() { var tests = [ - ['"\\76";', '">";'], + ['"\\76";', ';">";'], ['"\\0"', '"\\0";'], ['"\\08"', '"\\08";'], ['"\\008"', '"\\08";'], ['"\\0008"', '"\\08";'], ['"use strict" === "use strict";\n"\\76";', '"use strict"==="use strict";">";'], - // ['"use\\\n strict";\n"\\07";', '"use\\\n strict";\n"\\u0007";'] // TODO No way to store this content literally yet as directive + ['"use\\\n strict";\n"\\07";', ';"use strict";"\07";'] ]; for (var test in tests) { diff --git a/test/run-tests.js b/test/run-tests.js index 6614b2a5..5fc69c69 100755 --- a/test/run-tests.js +++ b/test/run-tests.js @@ -1,5 +1,7 @@ #! /usr/bin/env node +global.UGLIFY_DEBUG = true; + var U = require("../tools/node"); var path = require("path"); var fs = require("fs"); diff --git a/tools/exports.js b/tools/exports.js index 09acc13e..a481143c 100644 --- a/tools/exports.js +++ b/tools/exports.js @@ -17,3 +17,7 @@ exports["string_template"] = string_template; exports["tokenizer"] = tokenizer; exports["is_identifier"] = is_identifier; exports["SymbolDef"] = SymbolDef; + +if (DEBUG) { + exports["EXPECT_DIRECTIVE"] = EXPECT_DIRECTIVE; +} diff --git a/tools/node.js b/tools/node.js index 8cd3e4be..39976371 100644 --- a/tools/node.js +++ b/tools/node.js @@ -25,11 +25,12 @@ var FILES = exports.FILES = [ var UglifyJS = exports; -new Function("MOZ_SourceMap", "exports", FILES.map(function(file){ +new Function("MOZ_SourceMap", "exports", "DEBUG", FILES.map(function(file){ return fs.readFileSync(file, "utf8"); }).join("\n\n"))( require("source-map"), - UglifyJS + UglifyJS, + !!global.UGLIFY_DEBUG ); UglifyJS.AST_Node.warn_function = function(txt) { From 6c99816855b650c6804a67f4891c339a3e8970f4 Mon Sep 17 00:00:00 2001 From: Anthony Van de Gejuchte Date: Sat, 18 Jun 2016 17:28:22 +0200 Subject: [PATCH 05/10] Normalize error messages --- lib/parse.js | 32 ++++++++++++++++---------------- test/mocha/directives.js | 4 ++-- test/mocha/getter-setter.js | 2 +- test/mocha/with.js | 2 +- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/lib/parse.js b/lib/parse.js index 9b5a142b..a573234c 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -762,14 +762,14 @@ function parse($TEXT, options) { function unexpected(token) { if (token == null) token = S.token; - token_error(token, "Unexpected token: " + token.type + " (" + token.value + ")"); + token_error(token, "SyntaxError: Unexpected token: " + token.type + " (" + token.value + ")"); }; function expect_token(type, val) { if (is(type, val)) { return next(); } - token_error(S.token, "Unexpected token " + S.token.type + " «" + S.token.value + "»" + ", expected " + type + " «" + val + "»"); + token_error(S.token, "SyntaxError: Unexpected token " + S.token.type + " «" + S.token.value + "»" + ", expected " + type + " «" + val + "»"); }; function expect(punc) { return expect_token("punc", punc); }; @@ -898,7 +898,7 @@ function parse($TEXT, options) { case "return": if (S.in_function == 0 && !options.bare_returns) - croak("'return' outside of function"); + croak("SyntaxError: 'return' outside of function"); return new AST_Return({ value: ( is("punc", ";") ? (next(), null) @@ -915,7 +915,7 @@ function parse($TEXT, options) { case "throw": if (S.token.nlb) - croak("Illegal newline after 'throw'"); + croak("SyntaxError: Illegal newline after 'throw'"); return new AST_Throw({ value: (tmp = expression(true), semicolon(), tmp) }); @@ -931,7 +931,7 @@ function parse($TEXT, options) { case "with": if (S.input.has_directive("use strict")) { - croak("Strict mode may not include a with statement"); + croak("SyntaxError: Strict mode may not include a with statement"); } return new AST_With({ expression : parenthesised(), @@ -951,7 +951,7 @@ function parse($TEXT, options) { // syntactically incorrect if it contains a // LabelledStatement that is enclosed by a // LabelledStatement with the same Identifier as label. - croak("Label " + label.name + " defined twice"); + croak("SyntaxError: Label " + label.name + " defined twice"); } expect(":"); S.labels.push(label); @@ -964,7 +964,7 @@ function parse($TEXT, options) { label.references.forEach(function(ref){ if (ref instanceof AST_Continue) { ref = ref.label.start; - croak("Continue label `" + label.name + "` refers to non-IterationStatement.", + croak("SyntaxError: Continue label `" + label.name + "` refers to non-IterationStatement.", ref.line, ref.col, ref.pos); } }); @@ -984,11 +984,11 @@ function parse($TEXT, options) { if (label != null) { ldef = find_if(function(l){ return l.name == label.name }, S.labels); if (!ldef) - croak("Undefined label " + label.name); + croak("SyntaxError: Undefined label " + label.name); label.thedef = ldef; } else if (S.in_loop == 0) - croak(type.TYPE + " not inside a loop or switch"); + croak("SyntaxError: " + type.TYPE + " not inside a loop or switch"); semicolon(); var stat = new type({ label: label }); if (ldef) ldef.references.push(stat); @@ -1004,7 +1004,7 @@ function parse($TEXT, options) { : expression(true, true); if (is("operator", "in")) { if (init instanceof AST_Var && init.definitions.length > 1) - croak("Only one variable declaration allowed in for..in loop"); + croak("SyntaxError: Only one variable declaration allowed in for..in loop"); next(); return for_in(init); } @@ -1154,7 +1154,7 @@ function parse($TEXT, options) { }); } if (!bcatch && !bfinally) - croak("Missing catch/finally blocks"); + croak("SyntaxError: Missing catch/finally blocks"); return new AST_Try({ body : body, bcatch : bcatch, @@ -1248,8 +1248,8 @@ function parse($TEXT, options) { break; case "operator": if (!is_identifier_string(tok.value)) { - throw new JS_Parse_Error("Invalid getter/setter name: " + tok.value, - tok.file, tok.line, tok.col, tok.pos); + croak("SyntaxError: Invalid getter/setter name: " + tok.value, + tok.line, tok.col, tok.pos); } ret = _make_symbol(AST_SymbolRef); break; @@ -1399,7 +1399,7 @@ function parse($TEXT, options) { function as_symbol(type, noerror) { if (!is("name")) { - if (!noerror) croak("Name expected"); + if (!noerror) croak("SyntaxError: Name expected"); return null; } var sym = _make_symbol(type); @@ -1463,7 +1463,7 @@ function parse($TEXT, options) { function make_unary(ctor, op, expr) { if ((op == "++" || op == "--") && !is_assignable(expr)) - croak("Invalid use of " + op + " operator"); + croak("SyntaxError: Invalid use of " + op + " operator"); return new ctor({ operator: op, expression: expr }); }; @@ -1527,7 +1527,7 @@ function parse($TEXT, options) { end : prev() }); } - croak("Invalid assignment"); + croak("SyntaxError: Invalid assignment"); } return left; }; diff --git a/test/mocha/directives.js b/test/mocha/directives.js index 604b0329..45f454bf 100644 --- a/test/mocha/directives.js +++ b/test/mocha/directives.js @@ -168,7 +168,7 @@ describe("Directives", function() { throw new Error("Expected parser to fail"); } catch (e) { assert.strictEqual(e instanceof uglify.JS_Parse_Error, true); - assert.strictEqual(e.message, "Unexpected token: punc (])"); + assert.strictEqual(e.message, "SyntaxError: Unexpected token: punc (])"); } test_directive(tokenizer, tests[i]); @@ -367,4 +367,4 @@ describe("Directives", function() { ); } }); -}); \ No newline at end of file +}); diff --git a/test/mocha/getter-setter.js b/test/mocha/getter-setter.js index 641a2026..a292fa00 100644 --- a/test/mocha/getter-setter.js +++ b/test/mocha/getter-setter.js @@ -71,7 +71,7 @@ describe("Getters and setters", function() { var fail = function(data) { return function (e) { return e instanceof UglifyJS.JS_Parse_Error && - e.message === "Invalid getter/setter name: " + data.operator; + e.message === "SyntaxError: Invalid getter/setter name: " + data.operator; }; }; diff --git a/test/mocha/with.js b/test/mocha/with.js index d284f1c2..2e758d1e 100644 --- a/test/mocha/with.js +++ b/test/mocha/with.js @@ -9,7 +9,7 @@ describe("With", function() { } var error = function(e) { return e instanceof uglify.JS_Parse_Error && - e.message === "Strict mode may not include a with statement"; + e.message === "SyntaxError: Strict mode may not include a with statement"; } assert.throws(test, error); }); From e645ba84cfc950183a222c2cce2ea6b94fe51226 Mon Sep 17 00:00:00 2001 From: Shrey Banga Date: Thu, 5 May 2016 13:44:59 -0700 Subject: [PATCH 06/10] Respect quote style in object literals The option added in fbbaa42ee55a7f753f7cab9b1a905ccf73cf26d5 wasn't being respected inside object literals, so quoted property names would still be stripped out with this option. This is mostly a corner-case, but useful when the output is passed to something like the Closure compiler, where quoted property names can be used to prevent mangling. --- README.md | 16 +++++++++-- bin/uglifyjs | 24 ++++++++++++----- lib/output.js | 9 +++++-- lib/propmangle.js | 16 +++++++---- test/compress/properties.js | 51 +++++++++++++++++++++++++++++++++++ test/mocha/minify.js | 53 ++++++++++++++++++++++++++++++++++++- test/run-tests.js | 6 ++++- 7 files changed, 158 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 51d0b629..293608e7 100644 --- a/README.md +++ b/README.md @@ -133,7 +133,16 @@ The available options are: --reserved-file File containing reserved names --reserve-domprops Make (most?) DOM properties reserved for --mangle-props - --mangle-props Mangle property names + --mangle-props Mangle property names (default `0`). Set to + `true` or `1` to mangle all property names. Set + to `unquoted` or `2` to only mangle unquoted + property names. Mode `2` also enables the + `keep_quoted_props` beautifier option to + preserve the quotes around property names and + disables the `properties` compressor option to + prevent rewriting quoted properties with dot + notation. You can override these by setting + them explicitly on the command line. --mangle-regex Only mangle property names matching the regex --name-cache File to hold mangled names mappings --pure-funcs List of functions that can be safely removed if @@ -479,6 +488,8 @@ can pass additional arguments that control the code output: - `1` -- always use single quotes - `2` -- always use double quotes - `3` -- always use the original quotes +- `keep_quoted_props` (default `false`) -- when turned on, prevents stripping + quotes from property names in object literals. ### Keeping copyright notices or other comments @@ -667,7 +678,8 @@ Other options: ##### mangleProperties options - - `regex` — Pass a RegExp to only mangle certain names (maps to the `--mange-regex` CLI arguments option) + - `regex` — Pass a RegExp to only mangle certain names (maps to the `--mangle-regex` CLI arguments option) + - `ignore_quoted` – Only mangle unquoted property names (maps to the `--mangle-props 2` CLI arguments option) We could add more options to `UglifyJS.minify` — if you need additional functionality please suggest! diff --git a/bin/uglifyjs b/bin/uglifyjs index 45c92b50..b7009426 100755 --- a/bin/uglifyjs +++ b/bin/uglifyjs @@ -69,7 +69,7 @@ You need to pass an argument to this option to specify the name that your module .describe("quotes", "Quote style (0 - auto, 1 - single, 2 - double, 3 - original)") .describe("reserved-file", "File containing reserved names") .describe("reserve-domprops", "Make (most?) DOM properties reserved for --mangle-props") - .describe("mangle-props", "Mangle property names") + .describe("mangle-props", "Mangle property names (0 - disabled, 1 - mangle all properties, 2 - mangle unquoted properies)") .describe("mangle-regex", "Only mangle property names matching the regex") .describe("name-cache", "File to hold mangled names mappings") .describe("pure-funcs", "List of functions that can be safely removed if their return value is not used") @@ -125,7 +125,6 @@ You need to pass an argument to this option to specify the name that your module .boolean("noerr") .boolean("bare-returns") .boolean("keep-fnames") - .boolean("mangle-props") .boolean("reserve-domprops") .wrap(80) @@ -213,12 +212,24 @@ if (ARGS.quotes === true) { ARGS.quotes = 3; } +if (ARGS.mangle_props === true) { + ARGS.mangle_props = 1; +} else if (ARGS.mangle_props === "unquoted") { + ARGS.mangle_props = 2; +} + var OUTPUT_OPTIONS = { beautify : BEAUTIFY ? true : false, preamble : ARGS.preamble || null, quote_style : ARGS.quotes != null ? ARGS.quotes : 0 }; +if (ARGS.mangle_props == 2) { + OUTPUT_OPTIONS.keep_quoted_props = true; + if (COMPRESS && !("properties" in COMPRESS)) + COMPRESS.properties = false; +} + if (ARGS.screw_ie8) { if (COMPRESS) COMPRESS.screw_ie8 = true; if (MANGLE) MANGLE.screw_ie8 = true; @@ -401,10 +412,11 @@ async.eachLimit(files, 1, function (file, cb) { } TOPLEVEL = UglifyJS.mangle_properties(TOPLEVEL, { - reserved : reserved, - cache : cache, - only_cache : !ARGS.mangle_props, - regex : regex + reserved : reserved, + cache : cache, + only_cache : !ARGS.mangle_props, + regex : regex, + ignore_quoted : ARGS.mangle_props == 2 }); writeNameCache("props", cache); })(); diff --git a/lib/output.js b/lib/output.js index 1fa9899f..10465e21 100644 --- a/lib/output.js +++ b/lib/output.js @@ -66,7 +66,8 @@ function OutputStream(options) { preserve_line : false, screw_ie8 : false, preamble : null, - quote_style : 0 + quote_style : 0, + keep_quoted_props: false }, true); var indentation = 0; @@ -1173,7 +1174,11 @@ function OutputStream(options) { && parseFloat(key) >= 0) { output.print(make_num(key)); } else if (RESERVED_WORDS(key) ? output.option("screw_ie8") : is_identifier_string(key)) { - output.print_name(key); + if (quote && output.option("keep_quoted_props")) { + output.print_string(key, quote); + } else { + output.print_name(key); + } } else { output.print_string(key, quote); } diff --git a/lib/propmangle.js b/lib/propmangle.js index 840bda91..08043d73 100644 --- a/lib/propmangle.js +++ b/lib/propmangle.js @@ -65,7 +65,8 @@ function mangle_properties(ast, options) { reserved : null, cache : null, only_cache : false, - regex : null + regex : null, + ignore_quoted : false }); var reserved = options.reserved; @@ -81,6 +82,7 @@ function mangle_properties(ast, options) { } var regex = options.regex; + var ignore_quoted = options.ignore_quoted; var names_to_mangle = []; var unmangleable = []; @@ -88,7 +90,8 @@ function mangle_properties(ast, options) { // step 1: find candidates to mangle ast.walk(new TreeWalker(function(node){ if (node instanceof AST_ObjectKeyVal) { - add(node.key); + if (!(ignore_quoted && node.quote)) + add(node.key); } else if (node instanceof AST_ObjectProperty) { // setter or getter, since KeyVal is handled above @@ -101,7 +104,8 @@ function mangle_properties(ast, options) { } else if (node instanceof AST_Sub) { if (this.parent() instanceof AST_Assign) { - addStrings(node.property); + if (!ignore_quoted) + addStrings(node.property); } } })); @@ -109,7 +113,8 @@ function mangle_properties(ast, options) { // step 2: transform the tree, renaming properties return ast.transform(new TreeTransformer(function(node){ if (node instanceof AST_ObjectKeyVal) { - node.key = mangle(node.key); + if (!(ignore_quoted && node.quote)) + node.key = mangle(node.key); } else if (node instanceof AST_ObjectProperty) { // setter or getter @@ -119,7 +124,8 @@ function mangle_properties(ast, options) { node.property = mangle(node.property); } else if (node instanceof AST_Sub) { - node.property = mangleStrings(node.property); + if (!ignore_quoted) + node.property = mangleStrings(node.property); } // else if (node instanceof AST_String) { // if (should_mangle(node.value)) { diff --git a/test/compress/properties.js b/test/compress/properties.js index 39470738..574c5142 100644 --- a/test/compress/properties.js +++ b/test/compress/properties.js @@ -72,3 +72,54 @@ evaluate_length: { a = ("foo" + b).length; } } + +mangle_properties: { + mangle_props = { + ignore_quoted: false + }; + input: { + a["foo"] = "bar"; + a.color = "red"; + x = {"bar": 10}; + } + expect: { + a["a"] = "bar"; + a.b = "red"; + x = {c: 10}; + } +} + +mangle_unquoted_properties: { + mangle_props = { + ignore_quoted: true + } + beautify = { + beautify: false, + quote_style: 3, + keep_quoted_props: true, + } + input: { + function f1() { + a["foo"] = "bar"; + a.color = "red"; + x = {"bar": 10}; + } + function f2() { + a.foo = "bar"; + a['color'] = "red"; + x = {bar: 10}; + } + } + expect: { + function f1() { + a["foo"] = "bar"; + a.a = "red"; + x = {"bar": 10}; + } + function f2() { + a.b = "bar"; + a['color'] = "red"; + x = {c: 10}; + } + } +} diff --git a/test/mocha/minify.js b/test/mocha/minify.js index bbf188c4..02d31558 100644 --- a/test/mocha/minify.js +++ b/test/mocha/minify.js @@ -7,5 +7,56 @@ describe("minify", function() { var result = Uglify.minify(js, {fromString: true}); assert.strictEqual(result.code, 'function foo(n){return n?3:7}'); }); -}); + describe("keep_quoted_props", function() { + it("Should preserve quotes in object literals", function() { + var js = 'var foo = {"x": 1, y: 2, \'z\': 3};'; + var result = Uglify.minify(js, { + fromString: true, output: { + keep_quoted_props: true + }}); + assert.strictEqual(result.code, 'var foo={"x":1,y:2,"z":3};'); + }); + + it("Should preserve quote styles when quote_style is 3", function() { + var js = 'var foo = {"x": 1, y: 2, \'z\': 3};'; + var result = Uglify.minify(js, { + fromString: true, output: { + keep_quoted_props: true, + quote_style: 3 + }}); + assert.strictEqual(result.code, 'var foo={"x":1,y:2,\'z\':3};'); + }); + + it("Should not preserve quotes in object literals when disabled", function() { + var js = 'var foo = {"x": 1, y: 2, \'z\': 3};'; + var result = Uglify.minify(js, { + fromString: true, output: { + keep_quoted_props: false, + quote_style: 3 + }}); + assert.strictEqual(result.code, 'var foo={x:1,y:2,z:3};'); + }); + }); + + describe("mangleProperties", function() { + it("Shouldn't mangle quoted properties", function() { + var js = 'a["foo"] = "bar"; a.color = "red"; x = {"bar": 10};'; + var result = Uglify.minify(js, { + fromString: true, + compress: { + properties: false + }, + mangleProperties: { + ignore_quoted: true + }, + output: { + keep_quoted_props: true, + quote_style: 3 + } + }); + assert.strictEqual(result.code, + 'a["foo"]="bar",a.a="red",x={"bar":10};'); + }); + }); +}); diff --git a/test/run-tests.js b/test/run-tests.js index 5fc69c69..0fdee6f1 100755 --- a/test/run-tests.js +++ b/test/run-tests.js @@ -108,7 +108,11 @@ function run_compress_tests() { expect = test.expect_exact; } var input = as_toplevel(test.input); - var input_code = make_code(test.input, { beautify: true }); + var input_code = make_code(test.input, { + beautify: true, + quote_style: 3, + keep_quoted_props: true + }); if (test.mangle_props) { input = U.mangle_properties(input, test.mangle_props); } From fc1abd1c1152de0a297705d124e65d186181de10 Mon Sep 17 00:00:00 2001 From: Asia Date: Sat, 18 Jun 2016 22:17:51 +0200 Subject: [PATCH 07/10] Document the except option to mangle Added documentation for the `except` option to the `mangle` option in the API reference. --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 293608e7..2ab9de62 100644 --- a/README.md +++ b/README.md @@ -676,6 +676,10 @@ Other options: - `parse` (default {}) — pass an object if you wish to specify some additional [parser options][parser]. (not all options available... see below) +##### mangle + + - `except` - pass an array of identifiers that should be excluded from mangling + ##### mangleProperties options - `regex` — Pass a RegExp to only mangle certain names (maps to the `--mangle-regex` CLI arguments option) From 55c592dd43bc72a2be6cdfbcce05d2645eed0656 Mon Sep 17 00:00:00 2001 From: Richard van Velzen Date: Sun, 19 Jun 2016 21:56:06 +0200 Subject: [PATCH 08/10] v2.6.3 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 748e04fb..423a3e5d 100644 --- a/package.json +++ b/package.json @@ -4,7 +4,7 @@ "homepage": "http://lisperator.net/uglifyjs", "author": "Mihai Bazon (http://lisperator.net/)", "license": "BSD-2-Clause", - "version": "2.6.2", + "version": "2.6.3", "engines": { "node": ">=0.8.0" }, From aa82027a1721ee7233e92ab1b9f9ced43a8f17cf Mon Sep 17 00:00:00 2001 From: Richard van Velzen Date: Mon, 20 Jun 2016 08:39:46 +0200 Subject: [PATCH 09/10] Don't assume DEBUG is defined when exporting --self Potential fix for #1148 --- tools/exports.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/exports.js b/tools/exports.js index a481143c..54aa23e8 100644 --- a/tools/exports.js +++ b/tools/exports.js @@ -18,6 +18,6 @@ exports["tokenizer"] = tokenizer; exports["is_identifier"] = is_identifier; exports["SymbolDef"] = SymbolDef; -if (DEBUG) { +if (typeof DEBUG !== "undefined" && DEBUG) { exports["EXPECT_DIRECTIVE"] = EXPECT_DIRECTIVE; } From 85fbf86d7b6517161004c4f7dbfd15df4c282e3e Mon Sep 17 00:00:00 2001 From: Anthony Van de Gejuchte Date: Mon, 20 Jun 2016 14:18:47 +0200 Subject: [PATCH 10/10] Keep master in sync with harmony * Do not mangle when no mangle is required * Improve use_asm reset while printing code --- lib/output.js | 6 +++--- test/mocha/directives.js | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/output.js b/lib/output.js index 10465e21..5db1356c 100644 --- a/lib/output.js +++ b/lib/output.js @@ -404,7 +404,7 @@ function OutputStream(options) { AST_Node.DEFMETHOD("print", function(stream, force_parens){ var self = this, generator = self._codegen, prev_use_asm = use_asm; - if (self instanceof AST_Directive && self.value == "use asm") { + if (self instanceof AST_Directive && self.value == "use asm" && stream.parent() instanceof AST_Scope) { use_asm = true; } function doit() { @@ -419,7 +419,7 @@ function OutputStream(options) { doit(); } stream.pop_node(); - if (self instanceof AST_Lambda) { + if (self instanceof AST_Scope) { use_asm = prev_use_asm; } }); @@ -1221,7 +1221,7 @@ function OutputStream(options) { output.print_string(self.getValue(), self.quote, in_directive); }); DEFPRINT(AST_Number, function(self, output){ - if (use_asm && self.start.raw != null) { + if (use_asm && self.start && self.start.raw != null) { output.print(self.start.raw); } else { output.print(make_num(self.getValue())); diff --git a/test/mocha/directives.js b/test/mocha/directives.js index 45f454bf..82594758 100644 --- a/test/mocha/directives.js +++ b/test/mocha/directives.js @@ -224,7 +224,7 @@ describe("Directives", function() { for (var i = 0; i < tests.length; i++) { assert.strictEqual( - uglify.minify(tests[i][0], {fromString: true, quote_style: 3, compress: false}).code, + uglify.minify(tests[i][0], {fromString: true, quote_style: 3, compress: false, mangle: false}).code, tests[i][1], tests[i][0] );