From: Alex Lam S.L Date: Thu, 10 Dec 2020 22:59:21 +0000 (+0000) Subject: fix corner cases with spread syntax (#4358) X-Git-Url: https://git.ndcode.org/public/gitweb.cgi?a=commitdiff_plain;h=57105b299ec582bc731b58002703faa297f10063;p=UglifyJS.git fix corner cases with spread syntax (#4358) --- diff --git a/README.md b/README.md index 5718014d..f1c51a80 100644 --- a/README.md +++ b/README.md @@ -1175,6 +1175,8 @@ To allow for better optimizations, the compiler makes various assumptions: - Object properties can be added, removed and modified (not prevented with `Object.defineProperty()`, `Object.defineProperties()`, `Object.freeze()`, `Object.preventExtensions()` or `Object.seal()`). +- Iteration order of keys over an object which contains spread syntax in later + versions of Chrome and Node.js may be altered. - When `toplevel` is enabled, UglifyJS effectively assumes input code is wrapped within `function(){ ... }`, thus forbids aliasing of declared global variables: ```js diff --git a/lib/compress.js b/lib/compress.js index fbf6036c..d4b911ee 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -3662,6 +3662,20 @@ merge(Compressor.prototype, { ], }); + // Accomodate when compress option evaluate=false + // as well as the common constant expressions !0 and -1 + (function(def) { + def(AST_Node, return_false); + def(AST_Constant, return_true); + def(AST_RegExp, return_false); + var unaryPrefix = makePredicate("! ~ - + void"); + def(AST_UnaryPrefix, function() { + return unaryPrefix[this.operator] && this.expression instanceof AST_Constant; + }); + })(function(node, func) { + node.DEFMETHOD("is_constant", func); + }); + // methods to evaluate a constant expression (function(def) { // If the node has been successfully reduced to a constant, @@ -3686,18 +3700,6 @@ merge(Compressor.prototype, { if (typeof val == "function" || typeof val == "object") return this; return val; }); - var unaryPrefix = makePredicate("! ~ - + void"); - AST_Node.DEFMETHOD("is_constant", function() { - // Accomodate when compress option evaluate=false - // as well as the common constant expressions !0 and -1 - if (this instanceof AST_Constant) { - return !(this instanceof AST_RegExp); - } else { - return this instanceof AST_UnaryPrefix - && this.expression instanceof AST_Constant - && unaryPrefix[this.operator]; - } - }); var scan_modified = new TreeWalker(function(node) { if (node instanceof AST_Assign) modified(node.left); if (node instanceof AST_Unary && unary_arithmetic[node.operator]) modified(node.expression); @@ -6453,20 +6455,24 @@ merge(Compressor.prototype, { } }); var values = trim(exprs, compressor, first_in_statement, function(node, compressor, first_in_statement) { - var exp = node.expression; - if (exp instanceof AST_Object) return node; - if (exp instanceof AST_PropAccess) return node; + var exp = node.expression.tail_node(); if (exp instanceof AST_SymbolRef) { exp = exp.fixed_value(); if (!exp) return node; - if (exp instanceof AST_SymbolRef) return node; - if (exp instanceof AST_PropAccess) return node; - if (!(exp instanceof AST_Object)) return null; - return all(exp.properties, function(prop) { + exp = exp.tail_node(); + } + if (exp instanceof AST_Array + || exp.TYPE == "Binary" && !lazy_op[exp.operator] + || exp instanceof AST_Constant + || exp instanceof AST_Lambda + || exp instanceof AST_Object && all(exp.properties, function(prop) { return !(prop instanceof AST_ObjectGetter || prop instanceof AST_Spread); - }) ? null : node; + }) + || exp instanceof AST_This + || exp instanceof AST_Unary) { + return node.expression.drop_side_effect_free(compressor, first_in_statement); } - return exp.drop_side_effect_free(compressor, first_in_statement); + return node; }); if (!values) return null; if (values === exprs && !all(values, function(node) { @@ -10043,26 +10049,36 @@ merge(Compressor.prototype, { OPT(AST_Object, function(self, compressor) { if (!compressor.option("objects")) return self; var changed = false; - var computed_int = false; - var has_computed = false; + var found = false; + var generated = false; var keep_duplicate = compressor.has_directive("use strict"); var keys = new Dictionary(); var values = []; self.properties.forEach(function(prop) { if (!(prop instanceof AST_Spread)) return process(prop); + found = true; var exp = prop.expression; - if (exp instanceof AST_Object && all(exp.properties, function(node) { - return node instanceof AST_ObjectKeyVal; + if (compressor.option("spread") && exp instanceof AST_Object && all(exp.properties, function(prop) { + return !(prop instanceof AST_ObjectGetter || prop instanceof AST_Spread); })) { changed = true; - has_computed = true; - exp.properties.forEach(process); + exp.properties.forEach(function(prop) { + if (prop instanceof AST_ObjectKeyVal) process(prop); + }); } else { + generated = true; flush(); values.push(prop); } }); flush(); + if (!changed) return self; + if (found && generated && values.length == 1) { + var value = values[0]; + if (value instanceof AST_ObjectProperty && value.key instanceof AST_Number) { + value.key = "" + value.key.value; + } + } return changed ? make_node(AST_Object, self, { properties: values }) : self; @@ -10071,8 +10087,8 @@ merge(Compressor.prototype, { keys.each(function(props) { if (props.length == 1) return values.push(props[0]); changed = true; - var tail = keep_duplicate && props.pop(); - values.push(make_node(AST_ObjectKeyVal, self, { + var tail = keep_duplicate && !generated && props.pop(); + values.push(props.length == 1 ? props[0] : make_node(AST_ObjectKeyVal, self, { key: props[0].key, value: make_sequence(self, props.map(function(prop) { return prop.value; @@ -10086,9 +10102,13 @@ merge(Compressor.prototype, { function process(prop) { var key = prop.key; if (key instanceof AST_Node) { - has_computed = true; + found = true; key = key.evaluate(compressor); - if (key !== prop.key) key = prop.key = "" + key; + if (key === prop.key) { + generated = true; + } else { + key = prop.key = "" + key; + } } if (prop instanceof AST_ObjectKeyVal && typeof key == "string") { if (prop.value.has_side_effects(compressor)) flush(); @@ -10097,8 +10117,8 @@ merge(Compressor.prototype, { flush(); values.push(prop); } - if (has_computed && !computed_int && typeof key == "string" && /^[0-9]+$/.test(key)) { - computed_int = true; + if (found && !generated && typeof key == "string" && /^[0-9]+$/.test(key)) { + generated = true; prop.key = make_node(AST_Number, prop, { value: +key }); diff --git a/test/compress/objects.js b/test/compress/objects.js index 5d3653d0..5e59726d 100644 --- a/test/compress/objects.js +++ b/test/compress/objects.js @@ -360,3 +360,30 @@ issue_4269_4: { expect_stdout: "PASS" node_version: ">=4" } + +issue_4269_5: { + options = { + evaluate: true, + objects: true, + } + input: { + console.log({ + get 42() { + return "FAIL"; + }, + [console]: "bar", + 42: "PASS", + }[42]); + } + expect: { + console.log({ + get 42() { + return "FAIL"; + }, + [console]: "bar", + 42: "PASS", + }[42]); + } + expect_stdout: "PASS" + node_version: ">=4" +} diff --git a/test/compress/spread.js b/test/compress/spread.js index aaa8a7f7..32ed0b86 100644 --- a/test/compress/spread.js +++ b/test/compress/spread.js @@ -250,43 +250,119 @@ reduce_vars_2: { node_version: ">=6" } -drop_object: { +keep_getter_1: { options = { side_effects: true, } input: { - ({ ...console.log("PASS") }); + ({ + ...{ + get p() { + console.log("PASS"); + }, + }, + get q() { + console.log("FAIL"); + }, + }); } expect: { - console.log("PASS"); + ({ + ...{ + get p() { + console.log("PASS"); + }, + }, + }); } expect_stdout: "PASS" node_version: ">=8" } -keep_getter: { +keep_getter_2: { options = { side_effects: true, } input: { ({ - ...{ + ...(console.log("foo"), { get p() { - console.log("PASS"); + console.log("bar"); }, - }, - get q() { - console.log("FAIL"); - }, + }), }); } expect: { ({ - ...{ + ...(console.log("foo"), { get p() { - console.log("PASS"); + console.log("bar"); }, + }), + }); + } + expect_stdout: [ + "foo", + "bar", + ] + node_version: ">=8" +} + +keep_getter_3: { + options = { + side_effects: true, + } + input: { + ({ + ...function() { + return { + get p() { + console.log("PASS"); + }, + }; + }(), + }); + } + expect: { + ({ + ...function() { + return { + get p() { + console.log("PASS"); + }, + }; + }(), + }); + } + expect_stdout: "PASS" + node_version: ">=8" +} + +keep_getter_4: { + options = { + reduce_vars: true, + side_effects: true, + toplevel: true, + } + input: { + var o = { + get p() { + console.log("PASS"); + }, + }; + ({ + q: o, + ...o, + }); + } + expect: { + var o = { + get p() { + console.log("PASS"); }, + }; + ({ + ...o, }); } expect_stdout: "PASS" @@ -341,6 +417,179 @@ keep_accessor: { node_version: ">=8" } +object_key_order_1: { + options = { + objects: true, + spread: true, + } + input: { + var o = { + ...{}, + a: 1, + b: 2, + a: 3, + }; + for (var k in o) + console.log(k, o[k]); + } + expect: { + var o = { + a: (1, 3), + b: 2, + }; + for (var k in o) + console.log(k, o[k]); + } + expect_stdout: [ + "a 3", + "b 2", + ] + node_version: ">=8 <=10" +} + +object_key_order_2: { + options = { + objects: true, + spread: true, + } + input: { + var o = { + a: 1, + ...{}, + b: 2, + a: 3, + }; + for (var k in o) + console.log(k, o[k]); + } + expect: { + var o = { + a: (1, 3), + b: 2, + }; + for (var k in o) + console.log(k, o[k]); + } + expect_stdout: [ + "a 3", + "b 2", + ] + node_version: ">=8" +} + +object_key_order_3: { + options = { + objects: true, + spread: true, + } + input: { + var o = { + a: 1, + b: 2, + ...{}, + a: 3, + }; + for (var k in o) + console.log(k, o[k]); + } + expect: { + var o = { + a: (1, 3), + b: 2, + }; + for (var k in o) + console.log(k, o[k]); + } + expect_stdout: [ + "a 3", + "b 2", + ] + node_version: ">=8" +} + +object_key_order_4: { + options = { + objects: true, + spread: true, + } + input: { + var o = { + a: 1, + b: 2, + a: 3, + ...{}, + }; + for (var k in o) + console.log(k, o[k]); + } + expect: { + var o = { + a: (1, 3), + b: 2, + }; + for (var k in o) + console.log(k, o[k]); + } + expect_stdout: [ + "a 3", + "b 2", + ] + node_version: ">=8" +} + +object_spread_array: { + options = { + objects: true, + spread: true, + } + input: { + var o = { + ...[ "foo", "bar" ], + }; + for (var k in o) + console.log(k, o[k]); + } + expect: { + var o = { + ...[ "foo", "bar" ], + }; + for (var k in o) + console.log(k, o[k]); + } + expect_stdout: [ + "0 foo", + "1 bar", + ] + node_version: ">=8" +} + +object_spread_string: { + options = { + objects: true, + spread: true, + } + input: { + var o = { + ..."foo", + }; + for (var k in o) + console.log(k, o[k]); + } + expect: { + var o = { + ..."foo", + }; + for (var k in o) + console.log(k, o[k]); + } + expect_stdout: [ + "0 f", + "1 o", + "2 o", + ] + node_version: ">=8" +} + unused_var_side_effects: { options = { unused: true, diff --git a/test/ufuzz/index.js b/test/ufuzz/index.js index 568b4822..b6970fb4 100644 --- a/test/ufuzz/index.js +++ b/test/ufuzz/index.js @@ -722,17 +722,17 @@ function createStatement(recurmax, canThrow, canBreak, canContinue, cannotReturn var label = createLabel(canBreak, canContinue); canBreak = label.break || enableLoopControl(canBreak, CAN_BREAK); canContinue = label.continue || enableLoopControl(canContinue, CAN_CONTINUE); - return "{var brake" + loop + " = 5; " + label.target + "do {" + createStatement(recurmax, canThrow, canBreak, canContinue, cannotReturn, stmtDepth) + "} while ((" + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + ") && --brake" + loop + " > 0);}"; + return "{var brake" + loop + " = 5; " + label.target + "do {" + createStatement(recurmax, canThrow, canBreak, canContinue, cannotReturn, stmtDepth) + "} while (" + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + " && --brake" + loop + " > 0);}"; case STMT_WHILE: var label = createLabel(canBreak, canContinue); canBreak = label.break || enableLoopControl(canBreak, CAN_BREAK); canContinue = label.continue || enableLoopControl(canContinue, CAN_CONTINUE); - return "{var brake" + loop + " = 5; " + label.target + "while ((" + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + ") && --brake" + loop + " > 0)" + createStatement(recurmax, canThrow, canBreak, canContinue, cannotReturn, stmtDepth) + "}"; + return "{var brake" + loop + " = 5; " + label.target + "while (" + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + " && --brake" + loop + " > 0)" + createStatement(recurmax, canThrow, canBreak, canContinue, cannotReturn, stmtDepth) + "}"; case STMT_FOR_LOOP: var label = createLabel(canBreak, canContinue); canBreak = label.break || enableLoopControl(canBreak, CAN_BREAK); canContinue = label.continue || enableLoopControl(canContinue, CAN_CONTINUE); - return label.target + "for (var brake" + loop + " = 5; (" + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + ") && brake" + loop + " > 0; --brake" + loop + ")" + createStatement(recurmax, canThrow, canBreak, canContinue, cannotReturn, stmtDepth); + return label.target + "for (var brake" + loop + " = 5; " + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + " && brake" + loop + " > 0; --brake" + loop + ")" + createStatement(recurmax, canThrow, canBreak, canContinue, cannotReturn, stmtDepth); case STMT_FOR_IN: var label = createLabel(canBreak, canContinue); canBreak = label.break || enableLoopControl(canBreak, CAN_BREAK); @@ -931,7 +931,7 @@ function _createExpression(recurmax, noComma, stmtDepth, canThrow) { case p++: return createExpression(recurmax, COMMA_OK, stmtDepth, canThrow); case p++: - return createExpression(recurmax, noComma, stmtDepth, canThrow) + "?" + createExpression(recurmax, NO_COMMA, stmtDepth, canThrow) + ":" + createExpression(recurmax, noComma, stmtDepth, canThrow); + return createExpression(recurmax, noComma, stmtDepth, canThrow) + " ? " + createExpression(recurmax, NO_COMMA, stmtDepth, canThrow) + " : " + createExpression(recurmax, noComma, stmtDepth, canThrow); case p++: case p++: var nameLenBefore = VAR_NAMES.length; @@ -1031,14 +1031,13 @@ function _createExpression(recurmax, noComma, stmtDepth, canThrow) { case p++: return createUnarySafePrefix() + "(" + createNestedBinaryExpr(recurmax, noComma, stmtDepth, canThrow) + ")"; case p++: - return " ((" + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + ") || a || 3).toString() "; + return " (" + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + " || a || 3).toString() "; case p++: - return " /[abc4]/.test(((" + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + ") || b || 5).toString()) "; + return " /[abc4]/.test((" + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + " || b || 5).toString()) "; case p++: - return " /[abc4]/g.exec(((" + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + ") || b || 5).toString()) "; + return " /[abc4]/g.exec((" + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + " || b || 5).toString()) "; case p++: - return " ((" + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + - ") || " + rng(10) + ").toString()[" + + return " (" + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + " || " + rng(10) + ").toString()[" + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + "] "; case p++: return createArrayLiteral(recurmax, stmtDepth, canThrow); @@ -1209,10 +1208,10 @@ function createObjectLiteral(recurmax, stmtDepth, canThrow) { obj.push("..." + getVarName() + ","); break; case 4: - obj.push("..." + createObjectLiteral(recurmax, stmtDepth, canThrow) + ","); + obj.push("..." + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + ","); break; default: - obj.push(createObjectKey(recurmax, stmtDepth, canThrow) + ":(" + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + "),"); + obj.push(createObjectKey(recurmax, stmtDepth, canThrow) + ": " + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + ","); break; } obj.push("})");