From c55dd5ed74888d533cf8402c6ba3612916ba2885 Mon Sep 17 00:00:00 2001 From: kzc Date: Tue, 12 Apr 2016 09:19:38 -0400 Subject: [PATCH 1/9] Add `passes` compress option. Fix duplicate compress warnings. --- README.md | 3 +++ bin/uglifyjs | 2 +- lib/compress.js | 42 +++++++++++++++++++++++++------- test/compress/issue-1034.js | 48 +++++++++++++------------------------ test/mocha/minify.js | 11 +++++++++ test/run-tests.js | 2 +- tools/node.js | 2 +- 7 files changed, 66 insertions(+), 44 deletions(-) create mode 100644 test/mocha/minify.js diff --git a/README.md b/README.md index 9118d663..041a0709 100644 --- a/README.md +++ b/README.md @@ -356,6 +356,9 @@ to set `true`; it's effectively a shortcut for `foo=true`). compressor from mangling/discarding function names. Useful for code relying on `Function.prototype.name`. +- `passes` -- default `1`. Number of times to run compress. Use an + integer argument larger than 1 to further reduce code size in some cases. + Note: raising the number of passes will increase uglify compress time. ### The `unsafe` option diff --git a/bin/uglifyjs b/bin/uglifyjs index 90197cc4..45c92b50 100755 --- a/bin/uglifyjs +++ b/bin/uglifyjs @@ -423,7 +423,7 @@ async.eachLimit(files, 1, function (file, cb) { if (COMPRESS) { time_it("squeeze", function(){ - TOPLEVEL = TOPLEVEL.transform(compressor); + TOPLEVEL = compressor.compress(TOPLEVEL); }); } diff --git a/lib/compress.js b/lib/compress.js index 3e33c1b4..53618ae1 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -75,18 +75,36 @@ function Compressor(options, false_by_default) { screw_ie8 : false, drop_console : false, angular : false, - warnings : true, - global_defs : {} + global_defs : {}, + passes : 1, }, true); + this.warnings_produced = {}; }; Compressor.prototype = new TreeTransformer; merge(Compressor.prototype, { option: function(key) { return this.options[key] }, - warn: function() { - if (this.options.warnings) - AST_Node.warn.apply(AST_Node, arguments); + compress: function(node) { + var passes = +this.options.passes || 1; + for (var pass = 0; pass < passes && pass < 3; ++pass) { + if (pass > 0) node.clear_opt_flags(); + node = node.transform(this); + } + return node; + }, + warn: function(text, props) { + if (this.options.warnings) { + // only emit unique warnings + var message = string_template(text, props); + if (!(message in this.warnings_produced)) { + this.warnings_produced[message] = true; + AST_Node.warn.apply(AST_Node, arguments); + } + } + }, + clear_warnings: function() { + this.warnings_produced = {}; }, before: function(node, descend, in_list) { if (node._squeezed) return node; @@ -129,6 +147,15 @@ merge(Compressor.prototype, { return this.print_to_string() == node.print_to_string(); }); + AST_Node.DEFMETHOD("clear_opt_flags", function(){ + this.walk(new TreeWalker(function(node){ + if (!(node instanceof AST_Directive || node instanceof AST_Constant)) { + node._squeezed = false; + node._optimized = false; + } + })); + }); + function make_node(ctor, orig, props) { if (!props) props = {}; if (orig) { @@ -392,10 +419,7 @@ merge(Compressor.prototype, { var_defs_removed = true; } // Further optimize statement after substitution. - stat.walk(new TreeWalker(function(node){ - delete node._squeezed; - delete node._optimized; - })); + stat.clear_opt_flags(); compressor.warn("Replacing " + (is_constant ? "constant" : "variable") + " " + var_name + " [{file}:{line},{col}]", node.start); diff --git a/test/compress/issue-1034.js b/test/compress/issue-1034.js index 039af2ed..b91eaced 100644 --- a/test/compress/issue-1034.js +++ b/test/compress/issue-1034.js @@ -39,7 +39,7 @@ non_hoisted_function_after_return_2a: { hoist_funs: false, dead_code: true, conditionals: true, comparisons: true, evaluate: true, booleans: true, loops: true, unused: true, keep_fargs: true, if_return: true, join_vars: true, cascade: true, side_effects: true, - collapse_vars: false + collapse_vars: false, passes: 2 } input: { function foo(x) { @@ -57,20 +57,11 @@ non_hoisted_function_after_return_2a: { } } expect: { - // NOTE: Output is correct, but suboptimal. Not a regression. Can be improved in future. - // This output is run through non_hoisted_function_after_return_2b with same flags. function foo(x) { - if (x) { - return bar(1); - } else { - return bar(2); - var b; - } - var c = bar(3); + return bar(x ? 1 : 2); function bar(x) { return 7 - x; } - return b || c; } } expect_warnings: [ @@ -79,7 +70,12 @@ non_hoisted_function_after_return_2a: { "WARN: Dropping unreachable code [test/compress/issue-1034.js:51,16]", "WARN: Declarations in unreachable code! [test/compress/issue-1034.js:51,16]", "WARN: Dropping unused variable a [test/compress/issue-1034.js:48,20]", - "WARN: Dropping unused function nope [test/compress/issue-1034.js:55,21]" + "WARN: Dropping unused function nope [test/compress/issue-1034.js:55,21]", + "WARN: Dropping unreachable code [test/compress/issue-1034.js:53,12]", + "WARN: Declarations in unreachable code! [test/compress/issue-1034.js:53,12]", + "WARN: Dropping unreachable code [test/compress/issue-1034.js:56,12]", + "WARN: Dropping unused variable b [test/compress/issue-1034.js:51,20]", + "WARN: Dropping unused variable c [test/compress/issue-1034.js:53,16]" ] } @@ -91,7 +87,6 @@ non_hoisted_function_after_return_2b: { collapse_vars: false } input: { - // Note: output of test non_hoisted_function_after_return_2a run through compress again function foo(x) { if (x) { return bar(1); @@ -107,31 +102,20 @@ non_hoisted_function_after_return_2b: { } } expect: { - // the output we would have liked to see from non_hoisted_function_after_return_2a function foo(x) { return bar(x ? 1 : 2); function bar(x) { return 7 - x; } } } expect_warnings: [ - // Notice that some warnings are repeated by multiple compress passes. - // Not a regression. There is room for improvement here. - // Warnings should be cached and only output if unique. - "WARN: Dropping unreachable code [test/compress/issue-1034.js:100,16]", - "WARN: Declarations in unreachable code! [test/compress/issue-1034.js:100,16]", - "WARN: Dropping unreachable code [test/compress/issue-1034.js:100,16]", - "WARN: Declarations in unreachable code! [test/compress/issue-1034.js:100,16]", - "WARN: Dropping unreachable code [test/compress/issue-1034.js:100,16]", - "WARN: Declarations in unreachable code! [test/compress/issue-1034.js:100,16]", - "WARN: Dropping unreachable code [test/compress/issue-1034.js:100,16]", - "WARN: Declarations in unreachable code! [test/compress/issue-1034.js:100,16]", - "WARN: Dropping unreachable code [test/compress/issue-1034.js:102,12]", - "WARN: Declarations in unreachable code! [test/compress/issue-1034.js:102,12]", - "WARN: Dropping unreachable code [test/compress/issue-1034.js:106,12]", - "WARN: Dropping unreachable code [test/compress/issue-1034.js:100,16]", - "WARN: Declarations in unreachable code! [test/compress/issue-1034.js:100,16]", - "WARN: Dropping unused variable b [test/compress/issue-1034.js:100,20]", - "WARN: Dropping unused variable c [test/compress/issue-1034.js:102,16]" + // duplicate warnings no longer emitted + "WARN: Dropping unreachable code [test/compress/issue-1034.js:95,16]", + "WARN: Declarations in unreachable code! [test/compress/issue-1034.js:95,16]", + "WARN: Dropping unreachable code [test/compress/issue-1034.js:97,12]", + "WARN: Declarations in unreachable code! [test/compress/issue-1034.js:97,12]", + "WARN: Dropping unreachable code [test/compress/issue-1034.js:101,12]", + "WARN: Dropping unused variable b [test/compress/issue-1034.js:95,20]", + "WARN: Dropping unused variable c [test/compress/issue-1034.js:97,16]" ] } diff --git a/test/mocha/minify.js b/test/mocha/minify.js new file mode 100644 index 00000000..bbf188c4 --- /dev/null +++ b/test/mocha/minify.js @@ -0,0 +1,11 @@ +var Uglify = require('../../'); +var assert = require("assert"); + +describe("minify", function() { + it("Should test basic sanity of minify with default options", function() { + var js = 'function foo(bar) { if (bar) return 3; else return 7; var u = not_called(); }'; + var result = Uglify.minify(js, {fromString: true}); + assert.strictEqual(result.code, 'function foo(n){return n?3:7}'); + }); +}); + diff --git a/test/run-tests.js b/test/run-tests.js index e9ce3bb2..763575aa 100755 --- a/test/run-tests.js +++ b/test/run-tests.js @@ -110,7 +110,7 @@ function run_compress_tests() { if (test.mangle_props) { input = U.mangle_properties(input, test.mangle_props); } - var output = input.transform(cmp); + var output = cmp.compress(input); output.figure_out_scope(); if (test.mangle) { output.compute_char_frequency(test.mangle); diff --git a/tools/node.js b/tools/node.js index fa8c19dc..ecbf8d3e 100644 --- a/tools/node.js +++ b/tools/node.js @@ -78,7 +78,7 @@ exports.minify = function(files, options) { UglifyJS.merge(compress, options.compress); toplevel.figure_out_scope(); var sq = UglifyJS.Compressor(compress); - toplevel = toplevel.transform(sq); + toplevel = sq.compress(toplevel); } // 3. mangle properties From 4fe630431c37ffb81466959e9ea9797d6d2de21a Mon Sep 17 00:00:00 2001 From: Richard van Velzen Date: Sat, 23 Apr 2016 23:48:33 +0200 Subject: [PATCH 2/9] Hoist functions when reversing if (x) return; ... vs. if (!x) ... Fixes #1052 --- lib/compress.js | 18 ++++++++++++++++-- test/compress/issue-1052.js | 27 +++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 test/compress/issue-1052.js diff --git a/lib/compress.js b/lib/compress.js index 53618ae1..2bcfcf30 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -578,11 +578,13 @@ merge(Compressor.prototype, { CHANGED = true; stat = stat.clone(); stat.condition = stat.condition.negate(compressor); + var body = as_statement_array(stat.alternative).concat(ret); + var funs = extract_functions_from_statement_array(body); stat.body = make_node(AST_BlockStatement, stat, { - body: as_statement_array(stat.alternative).concat(ret) + body: body }); stat.alternative = null; - ret = [ stat.transform(compressor) ]; + ret = funs.concat([ stat.transform(compressor) ]); continue loop; } //--- @@ -840,6 +842,18 @@ merge(Compressor.prototype, { }; + function extract_functions_from_statement_array(statements) { + var funs = []; + for (var i = statements.length - 1; i >= 0; --i) { + var stat = statements[i]; + if (stat instanceof AST_Defun) { + statements.splice(i, 1); + funs.unshift(stat); + } + } + return funs; + } + function extract_declarations_from_unreachable_code(compressor, stat, target) { if (!(stat instanceof AST_Defun)) { compressor.warn("Dropping unreachable code [{file}:{line},{col}]", stat.start); diff --git a/test/compress/issue-1052.js b/test/compress/issue-1052.js new file mode 100644 index 00000000..067eea4a --- /dev/null +++ b/test/compress/issue-1052.js @@ -0,0 +1,27 @@ +hoist_funs_when_handling_if_return_rerversal: { + options = { if_return: true, hoist_funs: false }; + input: { + "use strict"; + + ( function() { + if ( !window ) { + return; + } + + function f() {} + function g() {} + } )(); + } + expect: { + "use strict"; + + ( function() { + function f() {} + function g() {} + + // NOTE: other compression steps will reduce this + // down to just `window`. + if ( window ); + } )(); + } +} From 4d9a0856870fc8feb07771fe2e27d93c8613e857 Mon Sep 17 00:00:00 2001 From: Richard van Velzen Date: Tue, 26 Apr 2016 11:43:03 +0200 Subject: [PATCH 3/9] Add test case for hoisting a single function --- test/compress/issue-1052.js | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/test/compress/issue-1052.js b/test/compress/issue-1052.js index 067eea4a..bad28a8f 100644 --- a/test/compress/issue-1052.js +++ b/test/compress/issue-1052.js @@ -1,8 +1,6 @@ -hoist_funs_when_handling_if_return_rerversal: { +multiple_functions: { options = { if_return: true, hoist_funs: false }; input: { - "use strict"; - ( function() { if ( !window ) { return; @@ -13,8 +11,6 @@ hoist_funs_when_handling_if_return_rerversal: { } )(); } expect: { - "use strict"; - ( function() { function f() {} function g() {} @@ -25,3 +21,23 @@ hoist_funs_when_handling_if_return_rerversal: { } )(); } } + +single_function: { + options = { if_return: true, hoist_funs: false }; + input: { + ( function() { + if ( !window ) { + return; + } + + function f() {} + } )(); + } + expect: { + ( function() { + function f() {} + + if ( window ); + } )(); + } +} From e9224ab4441ddb352566a52b84f1384bf5b8a8d8 Mon Sep 17 00:00:00 2001 From: Richard van Velzen Date: Tue, 26 Apr 2016 11:49:55 +0200 Subject: [PATCH 4/9] Add test cases for slightly more esoteric cases --- test/compress/issue-1052.js | 53 +++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/test/compress/issue-1052.js b/test/compress/issue-1052.js index bad28a8f..0a77f006 100644 --- a/test/compress/issue-1052.js +++ b/test/compress/issue-1052.js @@ -41,3 +41,56 @@ single_function: { } )(); } } + +deeply_nested: { + options = { if_return: true, hoist_funs: false }; + input: { + ( function() { + if ( !window ) { + return; + } + + function f() {} + function g() {} + + if ( !document ) { + return; + } + + function h() {} + } )(); + } + expect: { + ( function() { + function f() {} + function g() {} + + function h() {} + + // NOTE: other compression steps will reduce this + // down to just `window`. + if ( window ) + if (document); + } )(); + } +} + +not_hoisted_when_already_nested: { + options = { if_return: true, hoist_funs: false }; + input: { + ( function() { + if ( !window ) { + return; + } + + if ( foo ) function f() {} + + } )(); + } + expect: { + ( function() { + if ( window ) + if ( foo ) function f() {} + } )(); + } +} From f39fd3d58384d9bafbd18d3c1ec927940eeba458 Mon Sep 17 00:00:00 2001 From: kzc Date: Mon, 25 Apr 2016 00:42:18 -0400 Subject: [PATCH 5/9] Handle CR line endings in comments. Fixes #1050 --- lib/parse.js | 14 ++++++++++++-- test/mocha/line-endings.js | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 test/mocha/line-endings.js diff --git a/lib/parse.js b/lib/parse.js index 18d071f3..be103b67 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -255,6 +255,16 @@ function tokenizer($TEXT, filename, html5_comments, shebang) { return S.text.substr(S.pos, str.length) == str; }; + function find_eol() { + var text = S.text; + for (var i = S.pos, n = S.text.length; i < n; ++i) { + var ch = text[i]; + if (ch == '\n' || ch == '\r') + return i; + } + return -1; + }; + function find(what, signal_eof) { var pos = S.text.indexOf(what, S.pos); if (signal_eof && pos == -1) throw EX_EOF; @@ -410,7 +420,7 @@ function tokenizer($TEXT, filename, html5_comments, shebang) { function skip_line_comment(type) { var regex_allowed = S.regex_allowed; - var i = find("\n"), ret; + var i = find_eol(), ret; if (i == -1) { ret = S.text.substr(S.pos); S.pos = S.text.length; @@ -427,7 +437,7 @@ function tokenizer($TEXT, filename, html5_comments, shebang) { var skip_multiline_comment = with_eof_error("Unterminated multiline comment", function(){ var regex_allowed = S.regex_allowed; var i = find("*/", true); - var text = S.text.substring(S.pos, i); + var text = S.text.substring(S.pos, i).replace(/\r\n|\r/g, '\n'); var a = text.split("\n"), n = a.length; // update stream position S.pos = i + 2; diff --git a/test/mocha/line-endings.js b/test/mocha/line-endings.js new file mode 100644 index 00000000..8fd56a19 --- /dev/null +++ b/test/mocha/line-endings.js @@ -0,0 +1,34 @@ +var Uglify = require('../../'); +var assert = require("assert"); + +describe("line-endings", function() { + var options = { + fromString: true, + mangle: false, + compress: false, + output: { + beautify: false, + comments: /^!/, + } + }; + var expected_code = '/*!one\n2\n3*/\nfunction f(x){if(x)return 3}'; + + it("Should parse LF line endings", function() { + var js = '/*!one\n2\n3*///comment\nfunction f(x) {\n if (x)\n//comment\n return 3;\n}\n'; + var result = Uglify.minify(js, options); + assert.strictEqual(result.code, expected_code); + }); + + it("Should parse CR/LF line endings", function() { + var js = '/*!one\r\n2\r\n3*///comment\r\nfunction f(x) {\r\n if (x)\r\n//comment\r\n return 3;\r\n}\r\n'; + var result = Uglify.minify(js, options); + assert.strictEqual(result.code, expected_code); + }); + + it("Should parse CR line endings", function() { + var js = '/*!one\r2\r3*///comment\rfunction f(x) {\r if (x)\r//comment\r return 3;\r}\r'; + var result = Uglify.minify(js, options); + assert.strictEqual(result.code, expected_code); + }); +}); + From 35bc71662574d6bd537d0460d14e738ae4d3bb19 Mon Sep 17 00:00:00 2001 From: Anthony Van de Gejuchte Date: Tue, 26 Apr 2016 19:20:37 +0200 Subject: [PATCH 6/9] Add node 6 to travis --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 0f65db5b..b91a80e9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,6 +4,7 @@ node_js: - "0.12" - "0.10" - "4" + - "6" matrix: fast_finish: true sudo: false From d2945744f2dd569f4390d3bf3065e8a4e71fbf47 Mon Sep 17 00:00:00 2001 From: kzc Date: Sun, 1 May 2016 00:59:29 -0400 Subject: [PATCH 7/9] Workaround for process.exit() tty output truncation. Fixes #1055 --- tools/node.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tools/node.js b/tools/node.js index ecbf8d3e..8cd3e4be 100644 --- a/tools/node.js +++ b/tools/node.js @@ -1,3 +1,9 @@ +// workaround for tty output truncation upon process.exit() +[process.stdout, process.stderr].forEach(function(stream){ + if (stream._handle && stream._handle.setBlocking) + stream._handle.setBlocking(true); +}); + var path = require("path"); var fs = require("fs"); From 6641dcafb60091f0f944fcd5269ca67cb66ada2a Mon Sep 17 00:00:00 2001 From: Anthony Van de Gejuchte Date: Tue, 3 May 2016 17:41:40 +0200 Subject: [PATCH 8/9] Fix regression causing tests to fail on windows --- test/run-tests.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/run-tests.js b/test/run-tests.js index 763575aa..0a249b9f 100755 --- a/test/run-tests.js +++ b/test/run-tests.js @@ -132,8 +132,10 @@ function run_compress_tests() { beautify: false, quote_style: 2, // force double quote to match JSON }); + warnings_emitted = warnings_emitted.map(function(input) { + return input.split(process.cwd() + path.sep).join("").split(path.sep).join("/"); + }); var actual_warnings = JSON.stringify(warnings_emitted); - actual_warnings = actual_warnings.split(process.cwd() + "/").join(""); if (expected_warnings != actual_warnings) { log("!!! failed\n---INPUT---\n{input}\n---EXPECTED WARNINGS---\n{expected_warnings}\n---ACTUAL WARNINGS---\n{actual_warnings}\n\n", { input: input_code, From a0e03c9df47c411a40bceef02af2ce3dd1a329cc Mon Sep 17 00:00:00 2001 From: kzc Date: Tue, 3 May 2016 15:08:40 -0400 Subject: [PATCH 9/9] Retain comments before AST_Constants during mangle. --- lib/output.js | 2 ++ test/mocha/comment_before_constant.js | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 test/mocha/comment_before_constant.js diff --git a/lib/output.js b/lib/output.js index a59066fc..f8787582 100644 --- a/lib/output.js +++ b/lib/output.js @@ -409,6 +409,7 @@ function OutputStream(options) { AST_Node.DEFMETHOD("print_to_string", function(options){ var s = OutputStream(options); + if (!options) s._readonly = true; this.print(s); return s.get(); }); @@ -416,6 +417,7 @@ function OutputStream(options) { /* -----[ comments ]----- */ AST_Node.DEFMETHOD("add_comments", function(output){ + if (output._readonly) return; var c = output.option("comments"), self = this; var start = self.start; if (start && !start._comments_dumped) { diff --git a/test/mocha/comment_before_constant.js b/test/mocha/comment_before_constant.js new file mode 100644 index 00000000..cfdb6da1 --- /dev/null +++ b/test/mocha/comment_before_constant.js @@ -0,0 +1,27 @@ +var Uglify = require('../../'); +var assert = require("assert"); + +describe("comment before constant", function() { + var js = 'function f() { /*c1*/ var /*c2*/ foo = /*c3*/ false; return foo; }'; + + it("Should test comment before constant is retained and output after mangle.", function() { + var result = Uglify.minify(js, { + fromString: true, + compress: { collapse_vars: false }, + mangle: {}, + output: { comments: true }, + }); + assert.strictEqual(result.code, 'function f(){/*c1*/var/*c2*/n=/*c3*/!1;return n}'); + }); + + it("Should test code works when comments disabled.", function() { + var result = Uglify.minify(js, { + fromString: true, + compress: { collapse_vars: false }, + mangle: {}, + output: {}, + }); + assert.strictEqual(result.code, 'function f(){var n=!1;return n}'); + }); +}); +