Add a better error message for disallowed trailing commas after rest elements in function params

[simonbuchan]

Update

Let's make a nice "A trailing comma is not permitted after the rest element" error for all commas after function params. Currently we throw a nice error for arrow functions in this case, but apparently not for non-arrow functions.

Original Issue below

This is a bug report.

Input Code

// error-input.js
function foo(
    first,
    ...rest
) {
}

// trailing-input.js
function foo(
    first,
    rest,
) {
}

// error-input.js
function foo(
    first,
    ...rest,
) {
}

Babel/Babylon Configuration (.babelrc, package.json, cli command)

// package.json
{
  "dependencies": {
    "@babel/cli": "7.0.0-beta.40",
    "@babel/core": "7.0.0-beta.40"
  }
}

Expected Behavior

> yarn -s babel rest-input.js
function foo(first, ...rest) {}

> yarn -s babel trailing-input.js
function foo(first, rest) {}

> yarn -s babel error-input.js
function foo(first, ...rest) {}

Current Behavior

> yarn -s babel rest-input.js
function foo(first, ...rest) {}

> yarn -s babel trailing-input.js
function foo(first, rest) {}

> yarn -s babel error-input.js
{ SyntaxError: error-input.js: Unexpected token, expected ")" (3:11)

  1 | function foo(
  2 |     first,
> 3 |     ...rest,
    |            ^
  4 | ) {
  5 | }
  6 |
    at Parser.raise (C:\code\personal\bugs\babel-ts-traling-comma\node_modules\babylon\lib\index.js:772:15)
    at Parser.unexpected (C:\code\personal\bugs\babel-ts-traling-comma\node_modules\babylon\lib\index.js:2047:16)
    at Parser.expect (C:\code\personal\bugs\babel-ts-traling-comma\node_modules\babylon\lib\index.js:2035:28)
    at Parser.parseBindingList (C:\code\personal\bugs\babel-ts-traling-comma\node_modules\babylon\lib\index.js:2324:14)
    at Parser.parseFunctionParams (C:\code\personal\bugs\babel-ts-traling-comma\node_modules\babylon\lib\index.js:4560:24)
    at Parser.parseFunction (C:\code\personal\bugs\babel-ts-traling-comma\node_modules\babylon\lib\index.js:4548:10)
    at Parser.parseFunctionStatement (C:\code\personal\bugs\babel-ts-traling-comma\node_modules\babylon\lib\index.js:4206:17)
    at Parser.parseStatementContent (C:\code\personal\bugs\babel-ts-traling-comma\node_modules\babylon\lib\index.js:3923:21)
    at Parser.parseStatement (C:\code\personal\bugs\babel-ts-traling-comma\node_modules\babylon\lib\index.js:3899:17)
    at Parser.parseBlockOrModuleBlockBody (C:\code\personal\bugs\babel-ts-traling-comma\node_modules\babylon\lib\index.js:4426:23)
  pos: 38,
  loc: Position { line: 3, column: 11 },
  code: 'BABEL_PARSE_ERROR' }
error An unexpected error occurred: "Command failed.
Exit code: 1
Command: C:\\windows\\system32\\cmd.exe
Arguments: /d /s /c C:\\code\\personal\\bugs\\babel-ts-traling-comma\
ode_modules\\.bin\\babel error-input.js
Directory: C:\\code\\personal\\bugs\\babel-ts-traling-comma
Output:
".
info If you think this is a bug, please open a bug report with the information provided in "C:\\code\\personal\\bugs\\babel-ts-traling-comma\\yarn-error.log".
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Your Environment

software	version(s)
Babel	7.0.0-beta.40
node	v9.6.1
npm	yarn 1.5.1
Operating System	Win 10

[babel-bot]

Hey @simonbuchan! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

[simonbuchan]

Checking, MDN says this isn't legal: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Trailing_commas#Trailing_commas_in_functions

Note for arrow functions, this gives an explicit error, so it should do the same for function:

// arrow-input.js
const foo = (
    first,
    ...rest,
) => {
};

{ SyntaxError: arrow-input.js: A trailing comma is not permitted after the rest element (3:11)

  1 | const foo = (
  2 |     first,
> 3 |     ...rest,
    |            ^
  4 | ) => {
  5 | };
    at Parser.raise (C:\code\personal\bugs\babel-ts-traling-comma\node_modules\babylon\lib\index.js:772:15)
    at Parser.parseParenAndDistinguishExpression (C:\code\personal\bugs\babel-ts-traling-comma\node_modules\babylon\lib\index.js:3240:16)
    at Parser.parseExprAtom (C:\code\personal\bugs\babel-ts-traling-comma\node_modules\babylon\lib\index.js:3059:21)
    at Parser.parseExprSubscripts (C:\code\personal\bugs\babel-ts-traling-comma\node_modules\babylon\lib\index.js:2705:21)
    at Parser.parseMaybeUnary (C:\code\personal\bugs\babel-ts-traling-comma\node_modules\babylon\lib\index.js:2684:21)
    at Parser.parseExprOps (C:\code\personal\bugs\babel-ts-traling-comma\node_modules\babylon\lib\index.js:2596:21)
    at Parser.parseMaybeConditional (C:\code\personal\bugs\babel-ts-traling-comma\node_modules\babylon\lib\index.js:2568:21)
    at Parser.parseMaybeAssign (C:\code\personal\bugs\babel-ts-traling-comma\node_modules\babylon\lib\index.js:2526:21)
    at Parser.parseVar (C:\code\personal\bugs\babel-ts-traling-comma\node_modules\babylon\lib\index.js:4495:26)
    at Parser.parseVarStatement (C:\code\personal\bugs\babel-ts-traling-comma\node_modules\babylon\lib\index.js:4325:10)
  pos: 38,
  loc: Position { line: 3, column: 11 },
  code: 'BABEL_PARSE_ERROR' }
error An unexpected error occurred: "Command failed.
Exit code: 1
Command: C:\\windows\\system32\\cmd.exe
Arguments: /d /s /c C:\\code\\personal\\bugs\\babel-ts-traling-comma\
ode_modules\\.bin\\babel arrow-input.js
Directory: C:\\code\\personal\\bugs\\babel-ts-traling-comma
Output:
".

[simonbuchan]

I actually ran into this in typescript code, where the editor wasn't complaining, checking, tsc accepts this, so perhaps preset-typescript should allow it as an extension?

> yarn -s tsc --allowJs --outFile tsc-out.js error-input.js
> type tsc-out.js
function foo(first) {
    var rest = [];
    for (var _i = 1; _i < arguments.length; _i++) {
        rest[_i - 1] = arguments[_i];
    }
}

[loganfsmyth]

Yep, as you discovered, trailing commas are not allowed in function params after rest elements. The fact that TS allows it may or may not be a bug, @andy-ms could you comment?

I'm leaning toward it being an accidental bug in the way it parses function arguments, because TS also appears to allow

function fn(arg, ...other, third) {
    console.log(other);
}

but then transforms it into

function fn(arg, third) {
    console.log(other);
}

which is broken.

[ghost]

@loganfsmyth Made a pull request for trailing comma rest parameters. We already show an error for a non-last rest parameter.

[loganfsmyth]

Made a pull request for trailing comma rest parameters.

Perfect, thanks! I'll close this then.

We already show an error for a non-last rest parameter.

Ahh you're right. I looked at the /play example and saw that the code on the right looked bad, but didn't notice that it also errored out in the typechecker.

[simonbuchan]

We can't get the nicer arrow function message? That's a lot clearer that it's deliberate.

[loganfsmyth]

Oh sure, I'd be happy to leave this open for that.

[debugpai]

I would like to work on this if no one else is already.

[pburdette]

@loganfsmyth I can handle this. What package is currently handling that error output for the trailing comma ?

[morozRed]

@loganfsmyth fixed in #9046