fixup for #1585 (#1589)

As patched on `harmony`, `statement()` is the only user of `embed_tokens()` with a missing error branch.

Updated test case and match up with `harmony` to facilitate future merging.

.github/ISSUE_TEMPLATE.md

@@ -0,0 +1,7 @@
+- Bug report or feature request?
+- `uglify-js` version (`uglifyjs -V`)
+- JavaScript input - ideally as small as possible.
+- The `uglifyjs` CLI command executed or `minify()` options used.
+- An example of JavaScript output produced and/or the error or warning.
+
+Note: the release version of `uglify-js` only supports ES5. Those wishing to minify ES6 should use the experimental [`harmony`](https://github.com/mishoo/UglifyJS2#harmony) branch.

.travis.yml

@@ -1,10 +1,13 @@
 language: node_js
 before_install: "npm install -g npm"
 node_js:
-    - "0.12"
     - "0.10"
+    - "0.12"
     - "4"
     - "6"
+    - "7"
+env:
+    - UGLIFYJS_TEST_ALL=1
 matrix:
   fast_finish: true
 sudo: false

README.md

@@ -391,11 +391,11 @@ to set `true`; it's effectively a shortcut for `foo=true`).
 - `cascade` -- small optimization for sequences, transform `x, x` into `x`
   and `x = something(), x` into `x = something()`
 
-- `collapse_vars` -- default `false`. Collapse single-use `var` and `const`
-  definitions when possible.
+- `collapse_vars` -- Collapse single-use `var` and `const` definitions
+  when possible.
 
-- `reduce_vars` -- default `false`. Improve optimization on variables assigned
-  with and used as constant values.
+- `reduce_vars` -- Improve optimization on variables assigned with and
+  used as constant values.
 
 - `warnings` -- display warnings when dropping unreachable code or unused
   declarations etc.

lib/compress.js

@@ -281,6 +281,9 @@ merge(Compressor.prototype, {
                 if (node instanceof AST_Function
                     && (iife = tw.parent()) instanceof AST_Call
                     && iife.expression === node) {
+                    // Virtually turn IIFE parameters into variable definitions:
+                    //   (function(a,b) {...})(c,d) => (function() {var a=c,b=d; ...})()
+                    // So existing transformation rules can work on them.
                     node.argnames.forEach(function(arg, i) {
                         var d = arg.definition();
                         d.fixed = iife.args[i] || make_node(AST_Undefined, iife);
@@ -1810,10 +1813,12 @@ merge(Compressor.prototype, {
                         node.name = null;
                     }
                     if (node instanceof AST_Lambda && !(node instanceof AST_Accessor)) {
-                        if (!compressor.option("keep_fargs")) {
-                            for (var a = node.argnames, i = a.length; --i >= 0;) {
-                                var sym = a[i];
-                                if (!(sym.definition().id in in_use_ids)) {
+                        var trim = !compressor.option("keep_fargs");
+                        for (var a = node.argnames, i = a.length; --i >= 0;) {
+                            var sym = a[i];
+                            if (!(sym.definition().id in in_use_ids)) {
+                                sym.__unused = true;
+                                if (trim) {
                                     a.pop();
                                     compressor.warn("Dropping unused function argument {name} [{file}:{line},{col}]", {
                                         name : sym.name,
@@ -1822,7 +1827,9 @@ merge(Compressor.prototype, {
                                         col  : sym.start.col
                                     });
                                 }
-                                else break;
+                            }
+                            else {
+                                trim = false;
                             }
                         }
                     }
@@ -1840,6 +1847,7 @@ merge(Compressor.prototype, {
                     }
                     if (drop_vars && node instanceof AST_Definitions && !(tt.parent() instanceof AST_ForIn)) {
                         var def = node.definitions.filter(function(def){
+                            if (def.value) def.value = def.value.transform(tt);
                             if (def.name.definition().id in in_use_ids) return true;
                             var w = {
                                 name : def.name.name,
@@ -1900,17 +1908,15 @@ merge(Compressor.prototype, {
                         }
                         return node;
                     }
-                    if (drop_vars && assign_as_unused) {
-                        var n = node;
-                        while (n instanceof AST_Assign
-                            && n.operator == "="
-                            && n.left instanceof AST_SymbolRef) {
-                            var def = n.left.definition();
-                            if (def.id in in_use_ids
-                                || self.variables.get(def.name) !== def) break;
-                            n = n.right;
+                    if (drop_vars && assign_as_unused
+                        && node instanceof AST_Assign
+                        && node.operator == "="
+                        && node.left instanceof AST_SymbolRef) {
+                        var def = node.left.definition();
+                        if (!(def.id in in_use_ids)
+                            && self.variables.get(def.name) === def) {
+                            return maintain_this_binding(tt.parent(), node, node.right.transform(tt));
                         }
-                        if (n !== node) return n;
                     }
                     if (node instanceof AST_For) {
                         descend(node, this);
@@ -2100,7 +2106,8 @@ merge(Compressor.prototype, {
         def(AST_This, return_null);
         def(AST_Call, function(compressor, first_in_statement){
             if (!this.has_pure_annotation(compressor) && compressor.pure_funcs(this)) {
-                if (this.expression instanceof AST_Function) {
+                if (this.expression instanceof AST_Function
+                    && (!this.expression.name || !this.expression.name.definition().references.length)) {
                     var node = this.clone();
                     node.expression = node.expression.process_expression(false);
                     return node;
@@ -2609,10 +2616,10 @@ merge(Compressor.prototype, {
                 exp = def.fixed;
                 if (compressor.option("unused")
                     && def.references.length == 1
+                    && !(def.scope.uses_arguments
+                        && def.orig[0] instanceof AST_SymbolFunarg)
+                    && !def.scope.uses_eval
                     && compressor.find_parent(AST_Scope) === def.scope) {
-                    if (!compressor.option("keep_fnames")) {
-                        exp.name = null;
-                    }
                     self.expression = exp;
                 }
             }
@@ -2620,16 +2627,26 @@ merge(Compressor.prototype, {
         if (compressor.option("unused")
             && exp instanceof AST_Function
             && !exp.uses_arguments
-            && !exp.uses_eval
-            && self.args.length > exp.argnames.length) {
-            var end = exp.argnames.length;
-            for (var i = end, len = self.args.length; i < len; i++) {
-                var node = self.args[i].drop_side_effect_free(compressor);
-                if (node) {
-                    self.args[end++] = node;
+            && !exp.uses_eval) {
+            var pos = 0, last = 0;
+            for (var i = 0, len = self.args.length; i < len; i++) {
+                var trim = i >= exp.argnames.length;
+                if (trim || exp.argnames[i].__unused) {
+                    var node = self.args[i].drop_side_effect_free(compressor);
+                    if (node) {
+                        self.args[pos++] = node;
+                    } else if (!trim) {
+                        self.args[pos++] = make_node(AST_Number, self.args[i], {
+                            value: 0
+                        });
+                        continue;
+                    }
+                } else {
+                    self.args[pos++] = self.args[i];
                 }
+                last = pos;
             }
-            self.args.length = end;
+            self.args.length = last;
         }
         if (compressor.option("unsafe")) {
             if (exp instanceof AST_SymbolRef && exp.undeclared()) {

lib/output.js

@@ -777,16 +777,14 @@ function OutputStream(options) {
     DEFPRINT(AST_Do, function(self, output){
         output.print("do");
         output.space();
-        self._do_print_body(output);
+        make_block(self.body, output);
         output.space();
         output.print("while");
         output.space();
         output.with_parens(function(){
             self.condition.print(output);
         });
-        if (output.option("beautify") && output.option("screw_ie8")) {
-            output.semicolon();
-        }
+        output.semicolon();
     });
     DEFPRINT(AST_While, function(self, output){
         output.print("while");
@@ -906,10 +904,10 @@ function OutputStream(options) {
 
     /* -----[ if ]----- */
     function make_then(self, output) {
-        if (output.option("bracketize")) {
-            make_block(self.body, output);
-            return;
-        }
+        var b = self.body;
+        if (output.option("bracketize")
+            || !output.option("screw_ie8") && b instanceof AST_Do)
+            return make_block(b, output);
         // The squeezer replaces "block"-s that contain only a single
         // statement with the statement itself; technically, the AST
         // is correct, but this can create problems when we output an
@@ -917,9 +915,7 @@ function OutputStream(options) {
         // IF *without* an ELSE block (then the outer ELSE would refer
         // to the inner IF).  This function checks for this case and
         // adds the block brackets if needed.
-        if (!self.body)
-            return output.force_semicolon();
-        var b = self.body;
+        if (!b) return output.force_semicolon();
         while (true) {
             if (b instanceof AST_If) {
                 if (!b.alternative) {
@@ -1328,15 +1324,7 @@ function OutputStream(options) {
 
     function force_statement(stat, output) {
         if (output.option("bracketize")) {
-            if (!stat || stat instanceof AST_EmptyStatement)
-                output.print("{}");
-            else if (stat instanceof AST_BlockStatement)
-                stat.print(output);
-            else output.with_block(function(){
-                output.indent();
-                stat.print(output);
-                output.newline();
-            });
+            make_block(stat, output);
         } else {
             if (!stat || stat instanceof AST_EmptyStatement)
                 output.force_semicolon();
@@ -1385,11 +1373,11 @@ function OutputStream(options) {
     };
 
     function make_block(stmt, output) {
-        if (stmt instanceof AST_BlockStatement) {
+        if (!stmt || stmt instanceof AST_EmptyStatement)
+            output.print("{}");
+        else if (stmt instanceof AST_BlockStatement)
             stmt.print(output);
-            return;
-        }
-        output.with_block(function(){
+        else output.with_block(function(){
             output.indent();
             stmt.print(output);
             output.newline();

lib/parse.js

@@ -928,11 +928,9 @@ function parse($TEXT, options) {
                     expression : parenthesised(),
                     body       : statement()
                 });
-
-              default:
-                unexpected();
             }
         }
+        unexpected();
     });
 
     function labeled_statement() {

lib/scope.js

@@ -212,9 +212,10 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options){
         self.walk(new TreeWalker(function(node, descend) {
             if (node instanceof AST_SymbolCatch) {
                 var name = node.name;
+                var refs = node.thedef.references;
                 var scope = node.thedef.scope.parent_scope;
                 var def = scope.find_variable(name) || self.globals.get(name) || scope.def_variable(node);
-                node.thedef.references.forEach(function(ref) {
+                refs.forEach(function(ref) {
                     ref.thedef = def;
                     ref.reference(options);
                 });

package.json

@@ -4,7 +4,7 @@
   "homepage": "http://lisperator.net/uglifyjs",
   "author": "Mihai Bazon <mihai.bazon@gmail.com> (http://lisperator.net/)",
   "license": "BSD-2-Clause",
-  "version": "2.8.8",
+  "version": "2.8.11",
   "engines": {
     "node": ">=0.8.0"
   },

test/compress/drop-unused.js

@@ -700,3 +700,94 @@ issue_1539: {
         }
     }
 }
+
+vardef_value: {
+    options = {
+        keep_fnames: false,
+        reduce_vars: true,
+        unused: true,
+    }
+    input: {
+        function f() {
+            function g(){
+                return x();
+            }
+            var a = g();
+            return a(42);
+        }
+    }
+    expect: {
+        function f() {
+            var a = function(){
+                return x();
+            }();
+            return a(42);
+        }
+    }
+}
+
+assign_binding: {
+    options = {
+        cascade: true,
+        side_effects: true,
+        unused: true,
+    }
+    input: {
+        function f() {
+            var a;
+            a = f.g, a();
+        }
+    }
+    expect: {
+        function f() {
+            (0, f.g)();
+        }
+    }
+}
+
+assign_chain: {
+    options = {
+        unused: true,
+    }
+    input: {
+        function f() {
+            var a, b;
+            x = a = y = b = 42;
+        }
+    }
+    expect: {
+        function f() {
+            x = y = 42;
+        }
+    }
+}
+
+issue_1583: {
+    options = {
+        keep_fargs: true,
+        reduce_vars: true,
+        unused: true,
+    }
+    input: {
+        function m(t) {
+            (function(e) {
+                t = e();
+            })(function() {
+                return (function(a) {
+                    return a;
+                })(function(a) {});
+            });
+        }
+    }
+    expect: {
+        function m(t) {
+            (function(e) {
+                t = (function() {
+                    return (function(a) {
+                        return a;
+                    })(function(a) {});
+                })();
+            })();
+        }
+    }
+}

test/compress/issue-1569.js

@@ -0,0 +1,19 @@
+inner_reference: {
+    options = {
+        side_effects: true,
+    }
+    input: {
+        !function f(a) {
+            return a && f(a - 1) + a;
+        }(42);
+        !function g(a) {
+            return a;
+        }(42);
+    }
+    expect: {
+        !function f(a) {
+            return a && f(a - 1) + a;
+        }(42);
+        !void 0;
+    }
+}

test/compress/loops.js

@@ -257,7 +257,7 @@ issue_186: {
         else
             bar();
     }
-    expect_exact: 'var x=3;if(foo())do do alert(x);while(--x)while(x)else bar();'
+    expect_exact: 'var x=3;if(foo())do{do{alert(x)}while(--x)}while(x);else bar();'
 }
 
 issue_186_ie8: {
@@ -276,7 +276,7 @@ issue_186_ie8: {
         else
             bar();
     }
-    expect_exact: 'var x=3;if(foo())do do alert(x);while(--x)while(x)else bar();'
+    expect_exact: 'var x=3;if(foo()){do{do{alert(x)}while(--x)}while(x)}else bar();'
 }
 
 issue_186_beautify: {
@@ -295,7 +295,7 @@ issue_186_beautify: {
         else
             bar();
     }
-    expect_exact: 'var x = 3;\n\nif (foo()) do do alert(x); while (--x); while (x); else bar();'
+    expect_exact: 'var x = 3;\n\nif (foo()) do {\n    do {\n        alert(x);\n    } while (--x);\n} while (x); else bar();'
 }
 
 issue_186_beautify_ie8: {
@@ -314,7 +314,7 @@ issue_186_beautify_ie8: {
         else
             bar();
     }
-    expect_exact: 'var x = 3;\n\nif (foo()) do do alert(x); while (--x) while (x) else bar();'
+    expect_exact: 'var x = 3;\n\nif (foo()) {\n    do {\n        do {\n            alert(x);\n        } while (--x);\n    } while (x);\n} else bar();'
 }
 
 issue_186_bracketize: {
@@ -394,5 +394,5 @@ issue_186_beautify_bracketize_ie8: {
         else
             bar();
     }
-    expect_exact: 'var x = 3;\n\nif (foo()) {\n    do {\n        do {\n            alert(x);\n        } while (--x)\n    } while (x)\n} else {\n    bar();\n}'
+    expect_exact: 'var x = 3;\n\nif (foo()) {\n    do {\n        do {\n            alert(x);\n        } while (--x);\n    } while (x);\n} else {\n    bar();\n}'
 }

test/compress/reduce_vars.js

@@ -1122,3 +1122,133 @@ defun_label: {
         }();
     }
 }
+
+double_reference: {
+    options = {
+        reduce_vars: true,
+        unused: true,
+    }
+    input: {
+        function f() {
+            var g = function g() {
+                g();
+            };
+            g();
+        }
+    }
+    expect: {
+        function f() {
+            (function g() {
+                g();
+            })();
+        }
+    }
+}
+
+iife_arguments_1: {
+    options = {
+        reduce_vars: true,
+        unused: true,
+    }
+    input: {
+        (function(x) {
+            console.log(x() === arguments[0]);
+        })(function f() {
+            return f;
+        });
+    }
+    expect: {
+        (function(x) {
+            console.log(x() === arguments[0]);
+        })(function f() {
+            return f;
+        });
+    }
+}
+
+iife_arguments_2: {
+    options = {
+        reduce_vars: true,
+        unused: true,
+    }
+    input: {
+        (function() {
+            var x = function f() {
+                return f;
+            };
+            console.log(x() === arguments[0]);
+        })();
+    }
+    expect: {
+        (function() {
+            console.log(function f() {
+                return f;
+            }() === arguments[0]);
+        })();
+    }
+}
+
+iife_eval_1: {
+    options = {
+        reduce_vars: true,
+        unused: true,
+    }
+    input: {
+        (function(x) {
+            console.log(x() === eval("x"));
+        })(function f() {
+            return f;
+        });
+    }
+    expect: {
+        (function(x) {
+            console.log(x() === eval("x"));
+        })(function f() {
+            return f;
+        });
+    }
+}
+
+iife_eval_2: {
+    options = {
+        reduce_vars: true,
+        unused: true,
+    }
+    input: {
+        (function() {
+            var x = function f() {
+                return f;
+            };
+            console.log(x() === eval("x"));
+        })();
+    }
+    expect: {
+        (function() {
+            var x = function f() {
+                return f;
+            };
+            console.log(x() === eval("x"));
+        })();
+    }
+}
+
+iife_func_side_effects: {
+    options = {
+        reduce_vars: true,
+        unused: true,
+    }
+    input: {
+        (function(a, b, c) {
+            return b();
+        })(x(), function() {
+            return y();
+        }, z());
+    }
+    expect: {
+        (function(a, b, c) {
+            return function() {
+                return y();
+            }();
+        })(x(), 0, z());
+    }
+}

test/compress/screw-ie8.js

@@ -182,3 +182,39 @@ reduce_vars: {
         }
     }
 }
+
+issue_1586_1: {
+    options = {
+        screw_ie8: false,
+    }
+    mangle = {
+        screw_ie8: false,
+    }
+    input: {
+        function f() {
+            try {
+            } catch (err) {
+                console.log(err.message);
+            }
+        }
+    }
+    expect_exact: "function f(){try{}catch(c){console.log(c.message)}}"
+}
+
+issue_1586_2: {
+    options = {
+        screw_ie8: true,
+    }
+    mangle = {
+        screw_ie8: true,
+    }
+    input: {
+        function f() {
+            try {
+            } catch (err) {
+                console.log(err.message);
+            }
+        }
+    }
+    expect_exact: "function f(){try{}catch(c){console.log(c.message)}}"
+}

test/input/invalid/loop-no-body.js

@@ -0,0 +1 @@
+for (var i = 0; i < 1; i++) 

test/mocha/benchmark.js

@@ -1,24 +0,0 @@
-var assert = require("assert");
-var exec = require("child_process").exec;
-
-describe("test/benchmark.js", function() {
-    this.timeout(120000);
-    var command = '"' + process.argv[0] + '" test/benchmark.js ';
-    [
-        "-b",
-        "-b bracketize",
-        "-m",
-        "-mc passes=3",
-        "-mc passes=3,toplevel",
-        "-mc passes=3,unsafe",
-        "-mc keep_fargs=false,passes=3",
-        "-mc keep_fargs=false,passes=3,pure_getters,unsafe,unsafe_comps,unsafe_math,unsafe_proto",
-    ].forEach(function(args) {
-        it("Should pass with options " + args, function(done) {
-            exec(command + args, function(err) {
-                if (err) throw err;
-                done();
-            });
-        });
-    });
-});

test/mocha/cli.js

@@ -238,4 +238,17 @@ describe("bin/uglifyjs", function () {
             done();
         });
     });
+    it("Should fail with a missing loop body", function(done) {
+        var command = uglifyjscmd + ' test/input/invalid/loop-no-body.js';
+
+        exec(command, function (err, stdout, stderr) {
+            assert.ok(err);
+            var lines = stderr.split(/\n/);
+            assert.strictEqual(lines[0], "Parse error at test/input/invalid/loop-no-body.js:2,0");
+            assert.strictEqual(lines[1], "for (var i = 0; i < 1; i++) ");
+            assert.strictEqual(lines[2], "                            ^");
+            assert.strictEqual(lines[3], "SyntaxError: Unexpected token: eof (undefined)");
+            done();
+        });
+    });
 });

test/mocha/release.js

@@ -0,0 +1,54 @@
+var assert = require("assert");
+var spawn = require("child_process").spawn;
+
+if (!process.env.UGLIFYJS_TEST_ALL) return;
+
+function run(command, args, done) {
+    var id = setInterval(function() {
+        process.stdout.write("\0");
+    }, 5 * 60 * 1000);
+    spawn(command, args, {
+        stdio: "ignore"
+    }).on("exit", function(code) {
+        clearInterval(id);
+        assert.strictEqual(code, 0);
+        done();
+    });
+}
+
+describe("test/benchmark.js", function() {
+    this.timeout(5 * 60 * 1000);
+    [
+        "-b",
+        "-b bracketize",
+        "-m",
+        "-mc passes=3",
+        "-mc passes=3,toplevel",
+        "-mc passes=3,unsafe",
+        "-mc keep_fargs=false,passes=3",
+        "-mc keep_fargs=false,passes=3,pure_getters,unsafe,unsafe_comps,unsafe_math,unsafe_proto",
+    ].forEach(function(options) {
+        it("Should pass with options " + options, function(done) {
+            var args = options.split(/ /);
+            args.unshift("test/benchmark.js");
+            run(process.argv[0], args, done);
+        });
+    });
+});
+
+describe("test/jetstream.js", function() {
+    this.timeout(20 * 60 * 1000);
+    it("Should install phantomjs-prebuilt", function(done) {
+        run("npm", ["install", "phantomjs-prebuilt@2.1.14"], done);
+    });
+    [
+        "-mc warnings=false",
+        "-mc keep_fargs=false,passes=3,pure_getters,unsafe,unsafe_comps,unsafe_math,unsafe_proto,warnings=false",
+    ].forEach(function(options) {
+        it("Should pass with options " + options, function(done) {
+            var args = options.split(/ /);
+            args.unshift("test/jetstream.js");
+            run(process.argv[0], args, done);
+        });
+    });
+});