Actually let's move away those monsters from the evaluate function

ev() should do a single thing — evaluate constant expressions.  if that's
not possible, just return the original node.  it's not the best place for
partial evaluation there, instead doing it in the compress functions.

diff --git a/lib/compress.js b/lib/compress.js
index f71ba4a..dafed5d 100644 (file)
--- a/lib/compress.js
+++ b/lib/compress.js
@@ -637,12 +637,6 @@ merge(Compressor.prototype, {
             if (!compressor.option("evaluate")) return [ this ];
             try {
                 var val = this._eval(compressor);
-                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;
@@ -704,22 +698,7 @@ merge(Compressor.prototype, {
               case "|"          : return ev(left, c) |          ev(right, c);
               case "&"          : return ev(left, c) &          ev(right, c);
               case "^"          : return ev(left, c) ^          ev(right, c);
-              case "+"          :
-                // handle concatenating strings even if the left part cannot
-                // be evaluated, e.g. (variable + "str") + "str"
-                if (left instanceof AST_Binary && left.operator == "+" && left.is_string(c)) {
-                    return make_node(AST_Binary, this, {
-                        operator: "+",
-                        left: left.left,
-                        right: make_node(AST_String, null, {
-                            value : "" + ev(left.right, c) + ev(right, c),
-                            start : left.right.start,
-                            end   : right.end
-                        })
-                    });
-                } else {
-                    return ev(left, c) + ev(right, c);
-                }
+              case "+"          : return ev(left, c) +          ev(right, c);
               case "*"          : return ev(left, c) *          ev(right, c);
               case "/"          : return ev(left, c) /          ev(right, c);
               case "%"          : return ev(left, c) %          ev(right, c);
@@ -750,65 +729,6 @@ merge(Compressor.prototype, {
             if (d && d.constant && d.init) return ev(d.init, compressor);
             throw def;
         });
-        def(AST_Call, function(compressor){
-            if (compressor.option("unsafe")) {
-                if (this.expression instanceof AST_Dot
-                    && this.expression.expression instanceof AST_Array
-                    && this.expression.property == "join") {
-                    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 node;
-                }
-            }
-            throw def;
-        });
     })(function(node, func){
         node.DEFMETHOD("_eval", func);
     });
@@ -1759,6 +1679,53 @@ merge(Compressor.prototype, {
                     right: exp.expression
                 }).transform(compressor);
             }
+            else if (exp instanceof AST_Dot && exp.expression instanceof AST_Array && exp.property == "join") EXIT: {
+                var separator = self.args.length == 0 ? "," : self.args[0].evaluate(compressor)[1];
+                if (separator == null) break EXIT; // not a constant
+                var elements = exp.expression.elements.reduce(function(a, el){
+                    el = el.evaluate(compressor);
+                    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 make_node(AST_String, self, { value: "" });
+                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, self, { value: "" });
+                    }
+                    return elements.reduce(function(prev, el){
+                        return make_node(AST_Binary, el[0], {
+                            operator : "+",
+                            left     : prev,
+                            right    : el[0],
+                        });
+                    }, first).transform(compressor);
+                }
+                // need this awkward cloning to not affect original element
+                // best_of will decide which one to get through.
+                var node = self.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 best_of(self, node);
+            }
         }
         if (compressor.option("side_effects")) {
             if (self.expression instanceof AST_Function
@@ -1988,6 +1955,40 @@ merge(Compressor.prototype, {
             && self.left.operator == "+" && self.left.is_string(compressor)) {
             return self.left;
         }
+        if (compressor.option("evaluate")) {
+            if (self.operator == "+") {
+                if (self.left instanceof AST_Constant
+                    && self.right instanceof AST_Binary
+                    && self.right.operator == "+"
+                    && self.right.left instanceof AST_Constant
+                    && self.right.is_string(compressor)) {
+                    self = make_node(AST_Binary, self, {
+                        operator: "+",
+                        left: make_node(AST_String, null, {
+                            value: self.left.getValue() + self.right.left.getValue(),
+                            start: self.left.start,
+                            end: self.right.left.end
+                        }),
+                        right: self.right.right
+                    });
+                }
+                if (self.right instanceof AST_Constant
+                    && self.left instanceof AST_Binary
+                    && self.left.operator == "+"
+                    && self.left.right instanceof AST_Constant
+                    && self.left.is_string(compressor)) {
+                    self = make_node(AST_Binary, self, {
+                        operator: "+",
+                        left: self.left.left,
+                        right: make_node(AST_String, null, {
+                            value: self.left.right.getValue() + self.right.getValue(),
+                            start: self.left.right.start,
+                            end: self.right.end
+                        })
+                    });
+                }
+            }
+        }
         return self.evaluate(compressor)[0];
     });
 

diff --git a/test/compress/arrays.js b/test/compress/arrays.js
index 5fb21a8..766ec48 100644 (file)
--- a/test/compress/arrays.js
+++ b/test/compress/arrays.js
@@ -69,6 +69,6 @@ constant_join_2: {
         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);
+        var f = "strstr" + variable + "foobar" + ("moo" + foo);
     }
 }