More dirty handling of [ ... ].join() in unsafe mode

Close #300

diff --git a/lib/compress.js b/lib/compress.js
index abf2001..f71ba4a 100644 (file)
--- a/lib/compress.js
+++ b/lib/compress.js
@@ -630,15 +630,20 @@ merge(Compressor.prototype, {
         // elements.  If the node has been successfully reduced to a
         // constant, then the second element tells us the value;
         // otherwise the second element is missing.  The first element
-        // of the array is always an AST_Node descendant; when
+        // of the array is always an AST_Node descendant; if
         // evaluation was successful it's a node that represents the
-        // constant; otherwise it's the original node.
+        // constant; otherwise it's the original or a replacement node.
         AST_Node.DEFMETHOD("evaluate", function(compressor){
             if (!compressor.option("evaluate")) return [ this ];
             try {
                 var val = this._eval(compressor);
-                var ast = val instanceof AST_Binary ? val : make_node_from_constant(compressor, val, this);
-                return [ best_of(ast, this), val ];
+                if (val instanceof AST_Node) {
+                    // we didn't really reduce it to a constant, so
+                    // the return array should contain only one
+                    // node; but perhaps it's a better one.
+                    return [ best_of(val, this) ];
+                }
+                return [ best_of(make_node_from_constant(compressor, val, this), this), val ];
             } catch(ex) {
                 if (ex !== def) throw ex;
                 return [ this ];
@@ -750,10 +755,56 @@ merge(Compressor.prototype, {
                 if (this.expression instanceof AST_Dot
                     && this.expression.expression instanceof AST_Array
                     && this.expression.property == "join") {
-                    var x =  this.expression.expression.elements.map(function(el){
-                        return ev(el, compressor);
+                    var separator = this.args.length == 0 ? "," : ev(this.args[0], compressor);
+                    if (separator instanceof AST_Node) throw def; // not a constant
+                    var elements = this.expression.expression.elements.map(function(el){
+                        return el.evaluate(compressor); // doesn't throw.
+                    });
+                    elements = elements.reduce(function(a, el){
+                        if (a.length == 0 || el.length == 1) {
+                            a.push(el);
+                        } else {
+                            var last = a[a.length - 1];
+                            if (last.length == 2) {
+                                // it's a constant
+                                var val = "" + last[1] + separator + el[1];
+                                a[a.length - 1] = [
+                                    make_node_from_constant(compressor, val, last[0]),
+                                    val
+                                ];
+                            } else {
+                                a.push(el);
+                            }
+                        }
+                        return a;
+                    }, []);
+                    if (elements.length == 0) return "";
+                    if (elements.length == 1) return elements[0][0];
+                    if (separator == "") {
+                        var first;
+                        if (elements[0][0] instanceof AST_String
+                            || elements[1][0] instanceof AST_String) {
+                            first = elements.shift()[0];
+                        } else {
+                            first = make_node(AST_String, this, { value: "" });
+                        }
+                        return elements.reduce(function(prev, el){
+                            return make_node(AST_Binary, el[0], {
+                                operator : "+",
+                                left     : prev,
+                                right    : el[0],
+                            });
+                        }, first);
+                    }
+                    // need this awkward cloning to not affect original element
+                    // best_of will decide which one to get through.
+                    var node = this.clone();
+                    node.expression = node.expression.clone();
+                    node.expression.expression = node.expression.expression.clone();
+                    node.expression.expression.elements = elements.map(function(el){
+                        return el[0];
                     });
-                    return x.join(ev(this.args[0], compressor));
+                    return node;
                 }
             }
             throw def;
@@ -1918,11 +1969,6 @@ merge(Compressor.prototype, {
             }
             break;
         }
-        var exp = self.evaluate(compressor);
-        if (exp.length > 1) {
-            if (best_of(exp[0], self) !== self)
-                return exp[0];
-        }
         if (compressor.option("comparisons")) {
             if (!(compressor.parent() instanceof AST_Binary)
                 || compressor.parent() instanceof AST_Assign) {
@@ -1942,7 +1988,7 @@ merge(Compressor.prototype, {
             && self.left.operator == "+" && self.left.is_string(compressor)) {
             return self.left;
         }
-        return self;
+        return self.evaluate(compressor)[0];
     });
 
     OPT(AST_SymbolRef, function(self, compressor){

diff --git a/test/compress/arrays.js b/test/compress/arrays.js
index 06e03ae..5fb21a8 100644 (file)
--- a/test/compress/arrays.js
+++ b/test/compress/arrays.js
@@ -20,16 +20,55 @@ constant_join: {
     };
     input: {
         var a = [ "foo", "bar", "baz" ].join("");
+        var a1 = [ "foo", "bar", "baz" ].join();
         var b = [ "foo", 1, 2, 3, "bar" ].join("");
         var c = [ boo(), "foo", 1, 2, 3, "bar", bar() ].join("");
+        var c1 = [ boo(), bar(), "foo", 1, 2, 3, "bar", bar() ].join("");
+        var c2 = [ 1, 2, "foo", "bar", baz() ].join("");
         var d = [ "foo", 1 + 2 + "bar", "baz" ].join("-");
         var e = [].join(foo + bar);
+        var f = [].join("");
+        var g = [].join("foo");
     }
     expect: {
         var a = "foobarbaz";
+        var a1 = "foo,bar,baz";
         var b = "foo123bar";
-        var c = [ boo(), "foo", 1, 2, 3, "bar", bar() ].join(""); // we could still shorten this one, but oh well.
+        var c = boo() + "foo123bar" + bar();
+        var c1 = "" + boo() + bar() + "foo123bar" + bar();
+        var c2 = "12foobar" + baz();
         var d = "foo-3bar-baz";
         var e = [].join(foo + bar);
+        var f = "";
+        var g = "";
+    }
+}
+
+constant_join_2: {
+    options = {
+        unsafe   : true,
+        evaluate : true
+    };
+    input: {
+        var a = [ "foo", "bar", boo(), "baz", "x", "y" ].join("");
+        var b = [ "foo", "bar", boo(), "baz", "x", "y" ].join("-");
+        var c = [ "foo", "bar", boo(), "baz", "x", "y" ].join("really-long-separator");
+        var d = [ "foo", "bar", boo(),
+                  [ "foo", 1, 2, 3, "bar" ].join("+"),
+                  "baz", "x", "y" ].join("-");
+        var e = [ "foo", "bar", boo(),
+                  [ "foo", 1, 2, 3, "bar" ].join("+"),
+                  "baz", "x", "y" ].join("really-long-separator");
+        var f = [ "str", "str" + variable, "foo", "bar", "moo" + foo ].join("");
+    }
+    expect: {
+        var a = "foobar" + boo() + "bazxy";
+        var b = [ "foo-bar", boo(), "baz-x-y" ].join("-");
+        var c = [ "foo", "bar", boo(), "baz", "x", "y" ].join("really-long-separator");
+        var d = [ "foo-bar", boo(), "foo+1+2+3+bar-baz-x-y" ].join("-");
+        var e = [ "foo", "bar", boo(),
+                  "foo+1+2+3+bar",
+                  "baz", "x", "y" ].join("really-long-separator");
+        var f = "str" + ("str" + variable) + "foobar" + ("moo" + foo);
     }
 }