Version 3.8.0 comma-dangle breaks Flowtype annotations when a object rest parameter is used

[mormahr]

Tell us about your environment

• ESLint Version:
  v3.8.0
• Node Version:
  v6.7.0
• npm Version:
  3.10.3

What parser (default, Babel-ESLint, etc.) are you using?
Babel-ESLint

Please show your full configuration:

.eslintrc.js

module.exports = {
    "parser": "babel-eslint",
    "parserOptions": {
        "ecmaVersion": 6,
        "sourceType": "module"
    },
    "env": {
        "es6": true,
        "node": true
    },
    "plugins": [
        "flowtype",
        "flow-vars"
    ],
    "extends": "eslint:recommended",
    "rules": {
        "comma-dangle": [
            "error",
            "always-multiline"
        ],
    }
};

.babelrc

{
  "plugins": [
    "transform-flow-comments",
    "transform-function-bind",
    "transform-async-to-generator",
    "transform-exponentiation-operator",
    "transform-class-properties",
    "transform-es2015-destructuring",
    "transform-es2015-spread",
    "transform-es2015-parameters",
    "transform-object-rest-spread",
    "transform-es2015-classes",
    "transform-es2015-modules-commonjs"
  ]
}

What did you do? Please include the actual source code causing the issue.
I created a "minimal" demo to reproduce the problem. My impression is that since 3.8.0 (it works on 3.7.1) comma-dangle (set to always-multiline) reports a comma in a flow annotation when a object rest parameter is used before.

Complete demo code:
https://gist.github.com/mormahr/e6af53438a9bae9b9fe8905cc938b77d

export default class AntragsEvent {
    id: ?string
    rest: any

    constructor({
        id,
        ...rest, // <- this (the object rest spread) is causing
    }: {
        id?: string, // <- this comma to be reported by comma-dangle
    }) {
        this.id = id
        this.rest = rest
    }
}

What did you expect to happen?
No report of an unexpected comma

What actually happened? Please include the actual, raw output from ESLint.

[PRIVATE]/CommaDangleBug/test.js
9:14 error Unexpected trailing comma comma-dangle

✖ 1 problem (1 error, 0 warnings)

[not-an-aardvark]

Thanks for the report. This is probably caused by c8796e9, or maybe 52da71e.

[not-an-aardvark]

For what it's worth, I think your syntax is invalid based on the current version of the rest/spread spec; trailing commas aren't allowed after rest properties, since no property could actually be added after it. (See #7297 for more info.)

class Foo {
  constructor ({
    bar,
    ...baz, // <-- trailing comma is not allowed here according to rest/spread spec
  }) {}
}

Does the problem go away if you remove that comma?

[mormahr]

You're right about the comma after spread, but the comma dangle error persists even after removing it.

[mormahr]

Thanks for the fast response and fix 👍🏻

[sstur]

Yes, I can confirm this is an issue when upgrading to v3.8.0. Here is a much simpler repro example:

let {
  foo,
  ...otherStuff,
} = object;

Let me know if you need my eslint config for this.

[not-an-aardvark]

@sstur That part is actually intended; your code's syntax is invalid, so it wouldn't make sense for ESLint to require a trailing comma in that case. The error reported there is a result of the bugfix in #7297.

[sstur]

Oh, I see. I believe my syntax is valid for Stage 3 of ECMAScript and is handled for us by Babel. We use ESLint with some newer ECMAScript proposed features such as this (specifically we use the same ES feature set as ReactNative.) It has been working as long as I can remember but broke when upgrading from 3.7.1 to 3.8.0 (presumably because of the bugfix you referenced).

I am sticking to 3.7.1 now but probably need a way forward with proposed, not-yet-standardized features.

[not-an-aardvark]

I understand that you're using babel, but a trailing comma after a rest property is not valid syntax even according to the current proposal spec, because there is nothing that could follow a rest property.

The same thing applies with the already-standardized rest elements:

const [
  foo,
  ...bar, // <-- This is a syntax error in ES6
] = baz;

In order to be more compliant with the spec, babel's parser temporarily reported this as a syntax error, but it was later reverted due to breakage.

The fix in this case is to simply remove the comma, so that it becomes valid syntax according to the Stage 3 proposal.

[sstur]

Oh, I see! This I did not know!

So Babel is being more liberal than the standards in what it accepts. Interesting.

Thank you for taking the time to explain this. I wonder how to get the word out there so people don't continue opening issues..