support trailing commas in function parameter lists and calls (#2156)

fixes #2155

lib/parse.js

@@ -856,6 +856,7 @@ function parse($TEXT, options) {
 
     options = defaults(options, {
         bare_returns   : false,
+        ecma           : 8,
         expression     : false,
         filename       : null,
         html5_comments : true,
@@ -1386,22 +1387,20 @@ function parse($TEXT, options) {
 
     function parameters() {
         var start = S.token;
-        var first = true;
         var params = [];
         var used_parameters = track_used_binding_identifiers(true, S.input.has_directive("use strict"));
 
         expect("(");
 
         while (!is("punc", ")")) {
-            if (first) {
-                first = false;
-            } else {
-                expect(",");
-            }
-
             var param = parameter(used_parameters);
             params.push(param);
 
+            if (!is("punc", ")")) {
+                expect(",");
+                if (is("punc", ")") && options.ecma < 8) unexpected();
+            }
+
             if (param instanceof AST_Expansion) {
                 break;
             }
@@ -1617,25 +1616,40 @@ function parse($TEXT, options) {
         }
     }
 
-    function params_or_seq_() {
+    function params_or_seq_(allow_arrows, maybe_sequence) {
-        var first = true;
+        var spread_token;
+        var invalid_sequence;
+        var trailing_comma;
         var a = [];
+        expect("(");
         while (!is("punc", ")")) {
-            if (first) first = false; else expect(",");
+            if (spread_token) unexpected(spread_token);
             if (is("expand", "...")) {
-                var spread_token = S.token;
+                spread_token = S.token;
+                if (maybe_sequence) invalid_sequence = S.token;
                 next();
                 a.push(new AST_Expansion({
                     start: prev(),
                     expression: expression(),
                     end: S.token,
                 }));
-                if (!is("punc", ")")) {
-                    unexpected(spread_token);
-                }
             } else {
                 a.push(expression());
             }
+            if (!is("punc", ")")) {
+                expect(",");
+                if (is("punc", ")")) {
+                    if (options.ecma < 8) unexpected();
+                    trailing_comma = prev();
+                    if (maybe_sequence) invalid_sequence = trailing_comma;
+                }
+            }
+        }
+        expect(")");
+        if (allow_arrows && is("arrow", "=>")) {
+            if (spread_token && trailing_comma) unexpected(trailing_comma);
+        } else if (invalid_sequence) {
+            unexpected(invalid_sequence);
         }
         return a;
     }
@@ -1883,7 +1897,7 @@ function parse($TEXT, options) {
         var newexp = expr_atom(false), args;
         if (is("punc", "(")) {
             next();
-            args = expr_list(")");
+            args = expr_list(")", options.ecma >= 8);
         } else {
             args = [];
         }
@@ -2000,9 +2014,7 @@ function parse($TEXT, options) {
             switch (S.token.value) {
               case "(":
                 if (async && !allow_calls) break;
-                next();
+                var exprs = params_or_seq_(allow_arrows, !async);
-                var exprs = params_or_seq_();
-                expect(")");
                 if (allow_arrows && is("arrow", "=>")) {
                     return arrow_function(start, exprs.map(to_fun_args), !!async);
                 }
@@ -2608,11 +2620,9 @@ function parse($TEXT, options) {
         return expr;
     };
 
-    var call_args = embed_tokens(function call_args() {
+    var call_args = embed_tokens(function _call_args() {
-        var first = true;
         var args = [];
         while (!is("punc", ")")) {
-            if (first) first = false; else expect(",");
             if (is("expand", "...")) {
                 next();
                 args.push(new AST_Expansion({
@@ -2622,6 +2632,10 @@ function parse($TEXT, options) {
             } else {
                 args.push(expression(false));
             }
+            if (!is("punc", ")")) {
+                expect(",");
+                if (is("punc", ")") && options.ecma < 8) unexpected();
+            }
         }
         next();
         return args;

test/input/invalid/sequence.js

@@ -0,0 +1 @@
+(a, ...b);

test/mocha/cli.js

@@ -596,6 +596,21 @@ describe("bin/uglifyjs", function () {
             done();
         });
     });
+    it("Should throw syntax error (spread in sequence)", function(done) {
+        var command = uglifyjscmd + ' test/input/invalid/sequence.js';
+
+        exec(command, function (err, stdout, stderr) {
+            assert.ok(err);
+            assert.strictEqual(stdout, "");
+            assert.strictEqual(stderr.split(/\n/).slice(0, 4).join("\n"), [
+                "Parse error at test/input/invalid/sequence.js:1,4",
+                "(a, ...b);",
+                "    ^",
+                "ERROR: Unexpected token: expand (...)"
+            ].join("\n"));
+            done();
+        });
+    });
     it("Should handle literal string as source map input", function(done) {
         var command = [
             uglifyjscmd,

test/mocha/function.js

@@ -191,15 +191,51 @@ describe("Function", function() {
         ];
         var test = function(code) {
             return function() {
-                uglify.parse(code);
+                uglify.parse(code, { ecma: 5 });
             }
         }
         var error = function(e) {
-            return e instanceof uglify.JS_Parse_Error &&
+            return e instanceof uglify.JS_Parse_Error;
-                e.message === "Invalid function parameter";
         }
         for (var i = 0; i < tests.length; i++) {
-            assert.throws(test(tests[i]), error);
+            assert.throws(test(tests[i]), error, tests[i]);
+        }
+    });
+    it("Should accept trailing commas only for ES8", function() {
+        [
+            "new Foo(a, );",
+            "async(...[1, 2], );",
+            "console.log(...[1, 2], );",
+            "!function(a, b, ){ console.log(a + b); }(3, 4, );",
+        ].forEach(function(code) {
+            uglify.parse(code, { ecma: 8 });
+            assert.throws(function() {
+                uglify.parse(code, { ecma: 6 });
+            }, function(e) {
+                return e instanceof uglify.JS_Parse_Error;
+            }, code);
+        });
+    });
+    it("Should not accept invalid trailing commas", function() {
+        var tests = [
+            "f(, );",
+            "(, ) => {};",
+            "(...p, ) => {};",
+            "function f(, ) {}",
+            "function f(...p, ) {}",
+            "function foo(a, b, , ) {}",
+            'console.log("hello", , );',
+        ];
+        var test = function(code) {
+            return function() {
+                uglify.parse(code, { ecma: 8 });
+            }
+        }
+        var error = function(e) {
+            return e instanceof uglify.JS_Parse_Error;
+        }
+        for (var i = 0; i < tests.length; i++) {
+            assert.throws(test(tests[i]), error, tests[i]);
         }
     });
     it("Should not accept an initializer when parameter is a rest parameter", function() {