From dcccb6059ee4223bdeee8ebbad7bd95aae19d6d9 Mon Sep 17 00:00:00 2001 From: "Alex Lam S.L" Date: Thu, 28 Jun 2018 18:16:49 +0800 Subject: [PATCH] implement `directives` (#3203) fixes #3166 --- README.md | 2 + lib/compress.js | 5 +- test/compress/directives.js | 3 + test/compress/functions.js | 43 +++-- test/mocha/cli.js | 6 +- test/mocha/directives.js | 305 +++++++++++++++++++++++------------- test/mocha/glob.js | 6 +- test/mocha/minify.js | 12 +- test/mocha/sourcemaps.js | 8 +- test/mocha/spidermonkey.js | 2 +- 10 files changed, 251 insertions(+), 141 deletions(-) diff --git a/README.md b/README.md index 7d82296cc..50bea3100 100644 --- a/README.md +++ b/README.md @@ -666,6 +666,8 @@ If you're using the `X-SourceMap` header instead, you can just omit `sourceMap.u enabled `compress` transforms. Useful when you only want to enable a few `compress` options while disabling the rest. +- `directives` (default: `true`) -- remove redundant or non-standard directives + - `drop_console` (default: `false`) -- Pass `true` to discard calls to `console.*` functions. If you wish to drop a specific function call such as `console.info` and/or retain side effects from function arguments diff --git a/lib/compress.js b/lib/compress.js index be60a4a91..8d45b8491 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -58,6 +58,7 @@ function Compressor(options, false_by_default) { conditionals : !false_by_default, dead_code : !false_by_default, defaults : true, + directives : !false_by_default, drop_console : false, drop_debugger : !false_by_default, ecma : 5, @@ -3211,8 +3212,10 @@ merge(Compressor.prototype, { /* -----[ optimizers ]----- */ + var directives = ["use asm", "use strict"]; OPT(AST_Directive, function(self, compressor) { - if (compressor.has_directive(self.value) !== self) { + if (compressor.option("directives") + && (!member(self.value, directives) || compressor.has_directive(self.value) !== self)) { return make_node(AST_EmptyStatement, self); } return self; diff --git a/test/compress/directives.js b/test/compress/directives.js index 51587b951..d1af95666 100644 --- a/test/compress/directives.js +++ b/test/compress/directives.js @@ -1,4 +1,7 @@ class_directives_compression: { + options = { + directives: true, + } input: { class foo { foo() { diff --git a/test/compress/functions.js b/test/compress/functions.js index e1c2cf6d5..bf6226ed9 100644 --- a/test/compress/functions.js +++ b/test/compress/functions.js @@ -2246,6 +2246,37 @@ issue_2898: { expect_stdout: "2" } +deduplicate_parenthesis: { + input: { + ({}).a = b; + (({}).a = b)(); + (function() {}).a = b; + ((function() {}).a = b)(); + } + expect_exact: "({}).a=b;({}.a=b)();(function(){}).a=b;(function(){}.a=b)();" +} + +issue_3166: { + options = { + directives: true, + } + input: { + "foo"; + "use strict"; + function f() { + "use strict"; + "bar"; + "use asm"; + } + } + expect: { + "use strict"; + function f() { + "use asm"; + } + } +} + issue_3016_1: { options = { inline: true, @@ -2507,6 +2538,7 @@ issue_3125: { drop_lone_use_strict: { options = { + directives: true, side_effects: true, } input: { @@ -2537,6 +2569,7 @@ drop_lone_use_strict: { drop_lone_use_strict_arrows_1: { options = { side_effects: true, + directives: true, } input: { var f0 = () => 0; @@ -2594,13 +2627,3 @@ drop_lone_use_strict_arrows_2: { } node_version: ">=6" } - -deduplicate_parenthesis: { - input: { - ({}).a = b; - (({}).a = b)(); - (function() {}).a = b; - ((function() {}).a = b)(); - } - expect_exact: "({}).a=b;({}.a=b)();(function(){}).a=b;(function(){}.a=b)();" -} diff --git a/test/mocha/cli.js b/test/mocha/cli.js index f46046084..8c363e4da 100644 --- a/test/mocha/cli.js +++ b/test/mocha/cli.js @@ -9,7 +9,7 @@ function read(path) { describe("bin/uglifyjs", function() { var uglifyjscmd = '"' + process.argv[0] + '" bin/uglifyjs'; - it("should produce a functional build when using --self", function (done) { + it("Should produce a functional build when using --self", function (done) { this.timeout(120000); var command = uglifyjscmd + ' --self -mc ecma='; @@ -70,7 +70,7 @@ describe("bin/uglifyjs", function() { done(); }); }); - it("should not append source map to output when not using --source-map url=inline", function (done) { + it("Should not append source map to output when not using --source-map url=inline", function (done) { var command = uglifyjscmd + ' test/input/issue-1323/sample.js'; exec(command, function (err, stdout) { @@ -80,7 +80,7 @@ describe("bin/uglifyjs", function() { done(); }); }); - it("should not consider source map file content as source map file name (issue #2082)", function (done) { + it("Should not consider source map file content as source map file name (issue #2082)", function (done) { var command = [ uglifyjscmd, "test/input/issue-2082/sample.js", diff --git a/test/mocha/directives.js b/test/mocha/directives.js index 6bc5077ef..9e71a7672 100644 --- a/test/mocha/directives.js +++ b/test/mocha/directives.js @@ -2,14 +2,12 @@ var assert = require("assert"); var UglifyJS = require("../node"); describe("Directives", function() { - it ("Should allow tokenizer to store directives state", function() { + it("Should allow tokenizer to store directives state", function() { var tokenizer = UglifyJS.tokenizer("", "foo.js"); - // Stack level 0 assert.strictEqual(tokenizer.has_directive("use strict"), false); assert.strictEqual(tokenizer.has_directive("use asm"), false); assert.strictEqual(tokenizer.has_directive("use thing"), false); - // Stack level 2 tokenizer.push_directives_stack(); tokenizer.push_directives_stack(); @@ -17,7 +15,6 @@ describe("Directives", function() { assert.strictEqual(tokenizer.has_directive("use strict"), true); assert.strictEqual(tokenizer.has_directive("use asm"), false); assert.strictEqual(tokenizer.has_directive("use thing"), false); - // Stack level 3 tokenizer.push_directives_stack(); tokenizer.add_directive("use strict"); @@ -25,13 +22,11 @@ describe("Directives", function() { assert.strictEqual(tokenizer.has_directive("use strict"), true); assert.strictEqual(tokenizer.has_directive("use asm"), true); assert.strictEqual(tokenizer.has_directive("use thing"), false); - // Stack level 2 tokenizer.pop_directives_stack(); assert.strictEqual(tokenizer.has_directive("use strict"), true); assert.strictEqual(tokenizer.has_directive("use asm"), false); assert.strictEqual(tokenizer.has_directive("use thing"), false); - // Stack level 3 tokenizer.push_directives_stack(); tokenizer.add_directive("use thing"); @@ -39,26 +34,22 @@ describe("Directives", function() { assert.strictEqual(tokenizer.has_directive("use strict"), true); assert.strictEqual(tokenizer.has_directive("use asm"), false); // Directives are strict! assert.strictEqual(tokenizer.has_directive("use thing"), true); - // Stack level 2 tokenizer.pop_directives_stack(); assert.strictEqual(tokenizer.has_directive("use strict"), true); assert.strictEqual(tokenizer.has_directive("use asm"), false); assert.strictEqual(tokenizer.has_directive("use thing"), false); - // Stack level 1 tokenizer.pop_directives_stack(); assert.strictEqual(tokenizer.has_directive("use strict"), false); assert.strictEqual(tokenizer.has_directive("use asm"), false); assert.strictEqual(tokenizer.has_directive("use thing"), false); - // Stack level 0 tokenizer.pop_directives_stack(); assert.strictEqual(tokenizer.has_directive("use strict"), false); 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) { @@ -105,43 +96,6 @@ describe("Directives", function() { 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"], @@ -183,18 +137,122 @@ describe("Directives", function() { test_directive(tokenizer, tests[i]); } + + [ + [ + '"use strict"\n', + [ "use strict"], + [ "use asm"] + ], + [ + '"use\\\nstrict";', + [], + [ "use strict", "use\nstrict", "use \nstrict", "use asm" ] + ], + [ + '"use strict"\n"use asm"\n"use bar"\n', + [ "use strict", "use asm", "use bar" ], + [ "use foo", "use\\x20strict" ] + ], + [ + '"use \\\nstrict";"use strict";', + [], + [ "use strict", "use\nstrict", "use \nstrict", "use asm" ] + ], + [ + '"\\76";', + [], + [ ">", "\\76" ] + ], + [ + // no ; or newline + '"use strict"', + [], + [ "use strict", "use\nstrict", "use \nstrict", "use asm" ] + ], + [ + ';"use strict"', + [], + [ "use strict", "use\nstrict", "use \nstrict", "use asm" ] + ], + // Duplicate above code but put it in a function + [ + 'function foo() {"use strict"\n', + [ "use strict" ], + [ "use asm" ] + ], + [ + 'function foo() {"use\\\nstrict";', + [], + [ "use strict", "use\nstrict", "use \nstrict", "use asm" ] + ], + [ + 'function foo() {"use strict"\n"use asm"\n"use bar"\n', + [ "use strict", "use asm", "use bar" ], + [ "use foo", "use\\x20strict" ] + ], + [ + 'function foo() {"use \\\nstrict";"use strict";', + [], + [ "use strict", "use\nstrict", "use \nstrict", "use asm" ] + ], + [ + 'var foo = function() {"\\76";', + [], + [ ">", "\\76" ] + ], + [ + 'var foo = function() {"use strict"', // no ; or newline + [], + [ "use strict", "use\nstrict", "use \nstrict", "use asm" ] + ], + [ + 'var foo = function() {;"use strict"', + [], + [ "use strict", "use\nstrict", "use \nstrict", "use asm" ] + ], + // Special cases + [ + '"1";"2";"3";"4";;"5"', + [ "1", "2", "3", "4" ], + [ "5", "6", "use strict", "use asm" ] + ], + [ + 'if(1){"use strict";', + [], + [ "use strict", "use\nstrict", "use \nstrict", "use asm" ] + ], + [ + '"use strict";try{"use asm";', + [ "use strict" ], + [ "use\nstrict", "use \nstrict", "use asm" ] + ], + ].forEach(function(test) { + var tokenizer = UglifyJS.tokenizer(test[0] + "]", "foo.js"); + assert.throws(function() { + UglifyJS.parse(tokenizer); + }, function(e) { + return e instanceof UglifyJS.JS_Parse_Error + && e.message === "Unexpected token: punc (])" + }, test[0]); + test[1].forEach(function(directive) { + assert.strictEqual(tokenizer.has_directive(directive), true, directive + " in " + test[0]); + }); + test[2].forEach(function(fake_directive) { + assert.strictEqual(tokenizer.has_directive(fake_directive), false, fake_directive + " in " + test[0]); + }); + }); }); - it("Should test EXPECT_DIRECTIVE RegExp", function() { [ - ["", 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] + [ "", 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 ], ].forEach(function(test) { var out = UglifyJS.OutputStream(); out.print(test[0]); @@ -202,19 +260,33 @@ describe("Directives", function() { assert.strictEqual(out.get() === test[0] + ';""', test[1], test[0]); }); }); - it("Should only print 2 semicolons spread over 2 lines in beautify mode", function() { - assert.strictEqual( - UglifyJS.minify( - '"use strict";\'use strict\';"use strict";"use strict";;\'use strict\';console.log(\'use strict\');', - {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\');' - ); + var result = UglifyJS.minify([ + '"use strict";', + "'use strict';", + '"use strict";', + '"use strict";;', + "'use strict';", + "console.log('use strict');" + ].join(""), { + compress: false, + output: { + beautify: true, + quote_style: 3 + } + }); + if (result.error) throw result.error; + assert.strictEqual(result.code, [ + '"use strict";', + "'use strict';", + '"use strict";', + '"use strict";', + ";'use strict';", + "console.log('use strict');" + ].join("\n\n")); }); - it("Should not add double semicolons in non-scoped block statements to avoid strings becoming directives", function() { - var tests = [ + [ [ '{"use\x20strict"}', '{"use strict"}' @@ -231,26 +303,27 @@ describe("Directives", function() { 'if(1){"use\x20strict"} else {"use strict"}', 'if(1){"use strict"}else{"use strict"}' ] - ]; - - for (var i = 0; i < tests.length; i++) { - assert.strictEqual( - UglifyJS.minify(tests[i][0], {compress: false, mangle: false}).code, - tests[i][1], - tests[i][0] - ); - } + ].forEach(function(test) { + var result = UglifyJS.minify(test[0], { + compress: false, + mangle: false + }); + if (result.error) throw result.error; + assert.strictEqual(result.code, test[1], test[0]); + }); }); - it("Should add double semicolon when relying on automatic semicolon insertion", function() { - var code = UglifyJS.minify('"use strict";"use\\x20strict";', - {output: {semicolons: false}, compress: false} - ).code; - assert.strictEqual(code, '"use strict";;"use strict"\n'); + var result = UglifyJS.minify('"use strict";"use\\x20strict";', { + compress: false, + output: { + semicolons: false + } + }); + if (result.error) throw result.error; + assert.strictEqual(result.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";', @@ -347,46 +420,54 @@ describe("Directives", function() { 3, "'\"use strict\"';", ], - ]; - for (var i = 0; i < tests.length; i++) { - assert.strictEqual( - UglifyJS.minify(tests[i][0], {output:{quote_style: tests[i][1]}, compress: false}).code, - tests[i][2], - tests[i][0] + " using mode " + tests[i][1] - ); - } + ].forEach(function(test) { + var result = UglifyJS.minify(test[0], { + compress: false, + output: { + quote_style: test[1] + } + }); + if (result.error) throw result.error; + assert.strictEqual(result.code, test[2], test[0] + " using mode " + test[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");', + '"use strict";doSomething("foo");' + ], + [ + // Nothing gets optimised in the compressor because "use asm" is the first statement + '"use asm";"use\\x20strict";1+1;', + // Yet, the parser noticed that "use strict" wasn't a directive + '"use asm";;"use strict";1+1;', + ], + [ 'function f(){ "use strict" }', + 'function f(){}' + ], + [ 'function f(){ "use asm" }', + 'function f(){"use asm"}' + ], + [ 'function f(){ "use nondirective" }', + 'function f(){}' + ], + [ 'function f(){ ;"use strict" }', - 'function f(){ "use \n"; }', + 'function f(){}' ], [ - // 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 - 'function f(){"use strict"}', - 'function f(){"use asm"}', - 'function f(){"use nondirective"}', - 'function f(){}', - 'function f(){}', - ] - ]; - - for (var i = 0; i < tests.length; i++) { - assert.strictEqual( - UglifyJS.minify(tests[i][0]).code, - tests[i][1], - tests[i][0] - ); - } + 'function f(){ "use \\n"; }', + 'function f(){}' + ], + ].forEach(function(test) { + var result = UglifyJS.minify(test[0]); + if (result.error) throw result.error; + assert.strictEqual(result.code, test[1], test[0]); + }); }); it("Should be detect implicit usages of strict mode from tree walker", function() { var tests = [ diff --git a/test/mocha/glob.js b/test/mocha/glob.js index 58c40cf0d..0f437c34f 100644 --- a/test/mocha/glob.js +++ b/test/mocha/glob.js @@ -35,7 +35,7 @@ describe("bin/uglifyjs with input file globs", function() { done(); }); }); - it("should throw with non-matching glob string", function(done) { + it("Should throw with non-matching glob string", function(done) { var command = uglifyjscmd + ' "test/input/issue-1242/blah.*"'; exec(command, function(err, stdout, stderr) { @@ -53,7 +53,7 @@ describe("bin/uglifyjs with input file globs", function() { done(); }); }); - it("should handle special characters in glob string", function(done) { + it("Should handle special characters in glob string", function(done) { var command = uglifyjscmd + ' "test/input/issue-1632/^{*}[???](*)+$.??" -cm'; exec(command, function(err, stdout) { @@ -63,7 +63,7 @@ describe("bin/uglifyjs with input file globs", function() { done(); }); }); - it("should handle array of glob strings - matching and otherwise", function(done) { + it("Should handle array of glob strings - matching and otherwise", function(done) { var dir = "test/input/issue-1242"; var command = uglifyjscmd + ' "' + [ path.join(dir, "b*.es5"), diff --git a/test/mocha/minify.js b/test/mocha/minify.js index aefcd2afa..28eeae325 100644 --- a/test/mocha/minify.js +++ b/test/mocha/minify.js @@ -281,7 +281,7 @@ describe("minify", function() { }); describe("#__PURE__", function() { - it("should drop #__PURE__ hint after use", function() { + it("Should drop #__PURE__ hint after use", function() { var result = UglifyJS.minify('//@__PURE__ comment1 #__PURE__ comment2\n foo(), bar();', { output: { comments: "all", @@ -291,7 +291,7 @@ describe("minify", function() { var code = result.code; assert.strictEqual(code, "// comment1 comment2\nbar();"); }); - it("should drop #__PURE__ hint if function is retained", function() { + it("Should drop #__PURE__ hint if function is retained", function() { var result = UglifyJS.minify("var a = /*#__PURE__*/(function(){ foo(); })();", { output: { comments: "all", @@ -304,7 +304,7 @@ describe("minify", function() { }); describe("JS_Parse_Error", function() { - it("should return syntax error", function() { + it("Should return syntax error", function() { var result = UglifyJS.minify("function f(a{}"); var err = result.error; assert.ok(err instanceof Error); @@ -313,7 +313,7 @@ describe("minify", function() { assert.strictEqual(err.line, 1); assert.strictEqual(err.col, 12); }); - it("should reject duplicated label name", function() { + it("Should reject duplicated label name", function() { var result = UglifyJS.minify("L:{L:{}}"); var err = result.error; assert.ok(err instanceof Error); @@ -325,7 +325,7 @@ describe("minify", function() { }); describe("global_defs", function() { - it("should throw for non-trivial expressions", function() { + it("Should throw for non-trivial expressions", function() { var result = UglifyJS.minify("alert(42);", { compress: { global_defs: { @@ -337,7 +337,7 @@ describe("minify", function() { assert.ok(err instanceof Error); assert.strictEqual(err.stack.split(/\n/)[0], "SyntaxError: Unexpected token: keyword (debugger)"); }); - it("should skip inherited properties", function() { + it("Should skip inherited properties", function() { var foo = Object.create({ skip: this }); foo.bar = 42; var result = UglifyJS.minify("alert(FOO);", { diff --git a/test/mocha/sourcemaps.js b/test/mocha/sourcemaps.js index 15a3392f9..a285580aa 100644 --- a/test/mocha/sourcemaps.js +++ b/test/mocha/sourcemaps.js @@ -53,7 +53,6 @@ describe("sourcemaps", function() { assert.strictEqual(map.version, 3); assert.deepEqual(map.names, [ "x" ]); }); - it("Should give correct names", function() { var map = source_map([ "({", @@ -67,7 +66,6 @@ describe("sourcemaps", function() { ].join("\n")); assert.deepEqual(map.names, [ "enabled", "x" ]); }); - it("Should mark array/object literals", function() { var result = UglifyJS.minify([ "var obj = {};", @@ -80,7 +78,6 @@ describe("sourcemaps", function() { assert.strictEqual(result.code, "({}).wat([]);"); assert.strictEqual(result.map, '{"version":3,"sources":["0"],"names":["wat"],"mappings":"CAAU,IACNA,IAAI"}'); }); - it("Should give correct sourceRoot", function() { var code = "console.log(42);"; var result = UglifyJS.minify(code, { @@ -162,6 +159,9 @@ describe("sourcemaps", function() { }); it("Should work with max_line_len", function() { var result = UglifyJS.minify(read("./test/input/issue-505/input.js"), { + compress: { + directives: false, + }, output: { max_line_len: 20 }, @@ -211,7 +211,6 @@ describe("sourcemaps", function() { var map = prepare_map(orig); assert.equal(map.sourceContentFor("index.js"), orig.sourcesContent[0]); }); - it("Should copy sourcesContent if sources are relative", function() { var relativeMap = get_map(); relativeMap.sources = ['./index.js']; @@ -220,7 +219,6 @@ describe("sourcemaps", function() { assert.equal(map.sourcesContent.length, 1); assert.equal(map.sourceContentFor("index.js"), relativeMap.sourcesContent[0]); }); - it("Should not have invalid mappings from inputSourceMap", function() { var map = prepare_map(get_map()); // The original source has only 2 lines, check that mappings don't have more lines diff --git a/test/mocha/spidermonkey.js b/test/mocha/spidermonkey.js index 1b7d25cec..21926d346 100644 --- a/test/mocha/spidermonkey.js +++ b/test/mocha/spidermonkey.js @@ -6,7 +6,7 @@ var escodegen = require("escodegen"); var UglifyJS = require("../.."); describe("spidermonkey export/import sanity test", function() { - it("should produce a functional build when using --self with spidermonkey", function(done) { + it("Should produce a functional build when using --self with spidermonkey", function(done) { this.timeout(60000); var uglifyjs = '"' + process.argv[0] + '" bin/uglifyjs';