fix corner case in string concatenations (#3692)

- migrate de-facto compression to `conditionals` & `strings` fixes #3689
2020-01-28 07:33:11 +08:00
parent 0dcedad2d5
commit e9e76dcf04
6 changed files with 164 additions and 98 deletions
--- a/README.md
+++ b/README.md
@@ -736,6 +736,8 @@ If you're using the `X-SourceMap` header instead, you can just omit `sourceMap.u
  annotation `/*@__PURE__*/` or `/*#__PURE__*/` immediately precedes the call. For
  example: `/*@__PURE__*/foo();`

+- `strings` (default: `true`) -- compact string concatenations.
+
 - `switches` (default: `true`) -- de-duplicate and remove unreachable `switch` branches

 - `toplevel` (default: `false`) -- drop unreferenced functions (`"funcs"`) and/or
--- a/lib/compress.js
+++ b/lib/compress.js
@@ -83,6 +83,7 @@ function Compressor(options, false_by_default) {
        reduce_vars     : !false_by_default,
        sequences       : !false_by_default,
        side_effects    : !false_by_default,
+        strings         : !false_by_default,
        switches        : !false_by_default,
        top_retain      : null,
        toplevel        : !!(options && options["top_retain"]),
@@ -6188,6 +6189,18 @@ merge(Compressor.prototype, {
                self.right = tmp;
            }
        }
+        function swap_chain() {
+            var rhs = self.right;
+            self.left = make_node(AST_Binary, self, {
+                operator: self.operator,
+                left: self.left,
+                right: rhs.left,
+                start: self.left.start,
+                end: rhs.left.end
+            });
+            self.right = rhs.right;
+            self.left = self.left.transform(compressor);
+        }
        if (commutativeOperators[self.operator]
            && self.right.is_constant()
            && !self.left.is_constant()
@@ -6338,17 +6351,28 @@ merge(Compressor.prototype, {
              case ">=": reverse("<="); break;
            }
        }
-        if (self.operator == "+") {
+        // x && (y && z) => x && y && z
+        // x || (y || z) => x || y || z
+        if (compressor.option("conditionals")
+            && lazy_op[self.operator]
+            && self.right instanceof AST_Binary
+            && self.operator == self.right.operator) {
+            swap_chain();
+        }
+        if (compressor.option("strings") && self.operator == "+") {
+            // "foo" + 42 + "" => "foo" + 42
            if (self.right instanceof AST_String
                && self.right.value == ""
                && self.left.is_string(compressor)) {
                return self.left.optimize(compressor);
            }
+            // "" + ("foo" + 42) => "foo" + 42
            if (self.left instanceof AST_String
                && self.left.value == ""
                && self.right.is_string(compressor)) {
                return self.right.optimize(compressor);
            }
+            // "" + 42 + "foo" => 42 + "foo"
            if (self.left instanceof AST_Binary
                && self.left.operator == "+"
                && self.left.left instanceof AST_String
@@ -6357,6 +6381,14 @@ merge(Compressor.prototype, {
                self.left = self.left.right;
                return self.optimize(compressor);
            }
+            // "x" + (y + "z") => "x" + y + "z"
+            // x + ("y" + z) => x + "y" + z
+            if (self.right instanceof AST_Binary
+                && self.operator == self.right.operator
+                && (self.left.is_string(compressor) && self.right.is_string(compressor)
+                    || self.right.left.is_string(compressor) && !self.right.right.has_side_effects(compressor))) {
+                swap_chain();
+            }
        }
        if (compressor.option("evaluate")) {
            var associative = true;
@@ -6727,26 +6759,6 @@ merge(Compressor.prototype, {
                return node.optimize(compressor);
            }
        }
-        // x && (y && z) => x && y && z
-        // x || (y || z) => x || y || z
-        // x + ("y" + z) => x + "y" + z
-        // "x" + (y + "z") => "x" + y + "z"
-        if (self.right instanceof AST_Binary
-            && self.right.operator == self.operator
-            && (lazy_op[self.operator]
-                || (self.operator == "+"
-                    && (self.right.left.is_string(compressor)
-                        || (self.left.is_string(compressor)
-                            && self.right.right.is_string(compressor))))))
-        {
-            self.left = make_node(AST_Binary, self.left, {
-                operator : self.operator,
-                left     : self.left,
-                right    : self.right.left
-            });
-            self.right = self.right.right;
-            return self.transform(compressor);
-        }
        return try_evaluate(compressor, self);

        function align(ref, op) {
--- a/test/compress/arrays.js
+++ b/test/compress/arrays.js
@@ -16,6 +16,7 @@ holes_and_undefined: {
 constant_join: {
    options = {
        evaluate: true,
+        strings: true,
        unsafe: true,
    }
    input: {
@@ -65,6 +66,7 @@ constant_join: {
 constant_join_2: {
    options = {
        evaluate: true,
+        strings: true,
        unsafe: true,
    }
    input: {
@@ -94,9 +96,11 @@ constant_join_2: {
 constant_join_3: {
    options = {
        evaluate: true,
+        strings: true,
        unsafe: true,
    }
    input: {
+        var foo, bar, baz;
        var a = [ null ].join();
        var b = [ , ].join();
        var c = [ , 1, , 3 ].join();
@@ -111,6 +115,7 @@ constant_join_3: {
        var l = [ foo, bar + "baz" ].join("");
    }
    expect: {
+        var foo, bar, baz;
        var a = "";
        var b = "";
        var c = ",1,,3";
--- a/test/compress/concat-strings.js
+++ b/test/compress/concat-strings.js
@@ -26,7 +26,9 @@ concat_1: {
 }

 concat_2: {
-    options = {}
+    options = {
+        strings: true,
+    }
    input: {
        console.log(
            1 + (2 + 3),
@@ -55,7 +57,9 @@ concat_2: {
 }

 concat_3: {
-    options = {}
+    options = {
+        strings: true,
+    }
    input: {
        console.log(
            1 + 2 + (3 + 4 + 5),
@@ -84,7 +88,9 @@ concat_3: {
 }

 concat_4: {
-    options = {}
+    options = {
+        strings: true,
+    }
    input: {
        console.log(
            1 + "2" + (3 + 4 + 5),
@@ -113,7 +119,9 @@ concat_4: {
 }

 concat_5: {
-    options = {}
+    options = {
+        strings: true,
+    }
    input: {
        console.log(
            "1" + 2 + (3 + 4 + 5),
@@ -142,7 +150,9 @@ concat_5: {
 }

 concat_6: {
-    options = {}
+    options = {
+        strings: true,
+    }
    input: {
        console.log(
            "1" + "2" + (3 + 4 + 5),
@@ -171,6 +181,9 @@ concat_6: {
 }

 concat_7: {
+    options = {
+        strings: true,
+    }
    input: {
        console.log(
            "" + 1,
@@ -197,6 +210,9 @@ concat_7: {
 }

 concat_8: {
+    options = {
+        strings: true,
+    }
    input: {
        console.log(
            1 + "",
@@ -221,3 +237,20 @@ concat_8: {
    }
    expect_stdout: true
 }
+
+issue_3689: {
+    options = {
+        strings: true,
+    }
+    input: {
+        console.log(function(a) {
+            return a + ("" + (a[0] = 0));
+        }([]));
+    }
+    expect: {
+        console.log(function(a) {
+            return a + ("" + (a[0] = 0));
+        }([]));
+    }
+    expect_stdout: "00"
+}
--- a/test/compress/dead-code.js
+++ b/test/compress/dead-code.js
@@ -830,6 +830,7 @@ issue_3552: {
 unreachable_assign: {
    options = {
        dead_code: true,
+        strings: true,
    }
    input: {
        console.log(A = "P" + (A = "A" + (B = "S" + (A = B = "S"))), A, B);
--- a/test/compress/issue-269.js
+++ b/test/compress/issue-269.js
@@ -3,7 +3,8 @@ issue_269_1: {
        unsafe: true,
    }
    input: {
-		f(
+        var x = {};
+        console.log(
            String(x),
            Number(x),
            Boolean(x),
@@ -14,11 +15,13 @@ issue_269_1: {
        );
    }
    expect: {
-		f(
-			x + '', +x, !!x,
-			'', 0, false
+        var x = {};
+        console.log(
+            x + "", +x, !!x,
+            "", 0, false
        );
    }
+    expect_stdout: true
 }

 issue_269_dangers: {
@@ -26,15 +29,18 @@ issue_269_dangers: {
        unsafe: true,
    }
    input: {
-		f(
+        var x = {};
+        console.log(
            String(x, x),
            Number(x, x),
            Boolean(x, x)
        );
    }
    expect: {
-		f(String(x, x), Number(x, x), Boolean(x, x));
+        var x = {};
+        console.log(String(x, x), Number(x, x), Boolean(x, x));
    }
+    expect_stdout: true
 }

 issue_269_in_scope: {
@@ -43,7 +49,8 @@ issue_269_in_scope: {
    }
    input: {
        var String, Number, Boolean;
-		f(
+        var x = {};
+        console.log(
            String(x),
            Number(x, x),
            Boolean(x)
@@ -51,26 +58,32 @@ issue_269_in_scope: {
    }
    expect: {
        var String, Number, Boolean;
-		f(String(x), Number(x, x), Boolean(x));
+        var x = {};
+        console.log(String(x), Number(x, x), Boolean(x));
    }
+    expect_stdout: true
 }

 strings_concat: {
    options = {
+        strings: true,
        unsafe: true,
    }
    input: {
-		f(
-			String(x + 'str'),
-			String('str' + x)
+        var x = {};
+        console.log(
+            String(x + "str"),
+            String("str" + x)
        );
    }
    expect: {
-		f(
-			x + 'str',
-			'str' + x
+        var x = {};
+        console.log(
+            x + "str",
+            "str" + x
        );
    }
+    expect_stdout: true
 }

 regexp: {
@@ -93,6 +106,6 @@ regexp: {
        RegExp("should", "fail");
    }
    expect_warnings: [
-		'WARN: Error converting RegExp("should","fail") [test/compress/issue-269.js:5,2]',
+        'WARN: Error converting RegExp("should","fail") [test/compress/issue-269.js:5,8]',
    ]
 }