From 4f57d8746bd9a9d722034684d9ff0c630cc3689d Mon Sep 17 00:00:00 2001 From: kzc Date: Wed, 17 Jan 2018 01:46:23 -0500 Subject: [PATCH] fix various for-of bugs (#2800) - disable `rename` pass on harmony due to problem with for-of loops fixes #2794 --- lib/minify.js | 4 +- lib/output.js | 7 +- test/compress/harmony.js | 196 +++++++++++++++++++++++++++++++++++++++ test/mocha/minify.js | 3 +- 4 files changed, 203 insertions(+), 7 deletions(-) diff --git a/lib/minify.js b/lib/minify.js index 874f6437..ee1d478b 100644 --- a/lib/minify.js +++ b/lib/minify.js @@ -151,7 +151,9 @@ function minify(files, options) { toplevel = toplevel.wrap_commonjs(options.wrap); } if (timings) timings.rename = Date.now(); - if (options.rename) { + // disable rename on harmony due to expand_names bug in for-of loops + // https://github.com/mishoo/UglifyJS2/issues/2794 + if (0 && options.rename) { toplevel.figure_out_scope(options.mangle); toplevel.expand_names(options.mangle); } diff --git a/lib/output.js b/lib/output.js index 139075b3..61a25534 100644 --- a/lib/output.js +++ b/lib/output.js @@ -766,6 +766,7 @@ function OutputStream(options) { || p instanceof AST_Arrow // x => (x, x) || p instanceof AST_DefaultAssign // x => (x = (0, function(){})) || p instanceof AST_Expansion // [...(a, b)] + || p instanceof AST_ForOf && this === p.object // for (e of (foo, bar)) {} ; }); @@ -1046,11 +1047,7 @@ function OutputStream(options) { output.with_parens(function(){ self.init.print(output); output.space(); - if (self instanceof AST_ForOf) { - output.print("of"); - } else { - output.print("in"); - } + output.print(self instanceof AST_ForOf ? "of" : "in"); output.space(); self.object.print(output); }); diff --git a/test/compress/harmony.js b/test/compress/harmony.js index 477d8918..f8120eae 100644 --- a/test/compress/harmony.js +++ b/test/compress/harmony.js @@ -1245,3 +1245,199 @@ issue_2762: { } expect_stdout: "1 2 3" } + +issue_2794_1: { + options = { + collapse_vars: true, + evaluate: true, + inline: true, + passes: 1, + properties: true, + reduce_funcs: true, + reduce_vars: true, + toplevel: false, + side_effects: true, + unused: true, + } + input: { + function foo() { + for (const a of func(value)) { + console.log(a); + } + function func(va) { + return doSomething(va); + } + } + function doSomething(x) { + return [ x, 2 * x, 3 * x ]; + } + const value = 10; + foo(); + } + expect: { + function foo() { + for (const a of (va = value, doSomething(va))) console.log(a); + var va; + } + function doSomething(x) { + return [ x, 2 * x, 3 * x ]; + } + const value = 10; + foo(); + } + expect_stdout: [ + "10", + "20", + "30", + ] + node_version: ">=6" +} + +issue_2794_2: { + mangle = { + toplevel: false, + } + options = { + collapse_vars: true, + evaluate: true, + inline: true, + passes: 1, + properties: true, + reduce_funcs: true, + reduce_vars: true, + toplevel: false, + side_effects: true, + unused: true, + } + input: { + function foo() { + for (const a of func(value)) { + console.log(a); + } + function func(va) { + return doSomething(va); + } + } + function doSomething(x) { + return [ x, 2 * x, 3 * x ]; + } + const value = 10; + foo(); + } + expect: { + function foo() { + for (const n of (o = value, doSomething(o))) console.log(n); + var o; + } + function doSomething(o) { + return [ o, 2 * o, 3 * o ]; + } + const value = 10; + foo(); + } + expect_stdout: [ + "10", + "20", + "30", + ] + node_version: ">=6" +} + +issue_2794_3: { + mangle = { + toplevel: true, + } + options = { + collapse_vars: true, + evaluate: true, + inline: 3, + passes: 3, + properties: true, + reduce_funcs: true, + reduce_vars: true, + toplevel: true, + side_effects: true, + unused: true, + } + input: { + function foo() { + for (const a of func(value)) { + console.log(a); + } + function func(va) { + return doSomething(va); + } + } + function doSomething(x) { + return [ x, 2 * x, 3 * x ]; + } + const value = 10; + foo(); + } + expect: { + (function() { + for (const o of [ 10, 20, 30 ]) console.log(o); + })(); + } + expect_stdout: [ + "10", + "20", + "30", + ] + node_version: ">=6" +} + +issue_2794_4: { + options = {} + input: { + for (var x of ([1, 2], [3, 4])) { + console.log(x); + } + } + expect_exact: "for(var x of([1,2],[3,4]))console.log(x);" + expect_stdout: [ + "3", + "4", + ] + node_version: ">=6" +} + +issue_2794_5: { + mangle = {} + options = { + evaluate: true, + passes: 1, + side_effects: true, + unused: true, + } + input: { + for (var x of ([1, 2], [3, 4])) { + console.log(x); + } + } + expect_exact: "for(var x of[3,4])console.log(x);" + expect_stdout: [ + "3", + "4", + ] + node_version: ">=6" +} + +issue_2794_6: { + options = { + } + input: { + // TODO (or not): have parser flag invalid for-of expression. + // Consider it an uglify extension in the meantime. + for (let e of [1,2], [3,4,5]) { + console.log(e); + } + } + expect_exact: "for(let e of([1,2],[3,4,5]))console.log(e);" + expect_stdout: [ + "3", + "4", + "5", + ] + node_version: ">=6" +} diff --git a/test/mocha/minify.js b/test/mocha/minify.js index b582c4d6..6303d41c 100644 --- a/test/mocha/minify.js +++ b/test/mocha/minify.js @@ -405,7 +405,8 @@ describe("minify", function() { }); }); - describe("rename", function() { + // rename is disabled on harmony due to expand_names bug in for-of loops + if (0) describe("rename", function() { it("Should be repeatable", function() { var code = "!function(x){return x(x)}(y);"; for (var i = 0; i < 2; i++) {