fix corner cases with spread syntax (#4358)

diff --git a/README.md b/README.md
index 5718014..f1c51a8 100644 (file)
--- 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 fbf6036..d4b911e 100644 (file)
--- 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 5d3653d..5e59726 100644 (file)
--- 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 aaa8a7f..32ed0b8 100644 (file)
--- 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 568b482..b6970fb 100644 (file)
--- 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("})");