Tslint: trailingComma is required after rest parameter

[thgreasi]

Bug Report

• __TSLint version__: 5.6.0
• __TypeScript version__: 2.4.0
• __Running TSLint via__: CLI

TypeScript code being linted

export const foo = (
    a: any,
    ...rest: any[]
) => {};

with tslint.json configuration:

{
  "extends": "tslint:recommended",
  "rules": {
    "indent": [ true, "tabs" ],
    "jsdoc-format": true,
    "whitespace": [
      false,
      "check-type"
    ],
    "object-literal-sort-keys": false,
    "only-arrow-functions": [false],
    "quotemark": [true, "single", "avoid-escape"],
    "max-classes-per-file": [false],
    "max-line-length": [180],
    "member-access": false,
    "member-ordering": [false],
    "no-consecutive-blank-lines": false,
    "no-shadowed-variable": false,
    "no-var-requires": true,
    "arrow-parens": false,
    "no-angle-bracket-type-assertion": false,
    "no-console": [false],
    "no-empty-interface": false,
    "no-string-literal": false,
    "object-literal-key-quotes": [true, "as-needed"],
    "interface-name": [false],
    "interface-over-type-literal": false,
    "variable-name": [
      true,
      "ban-keywords",
      "check-format",
      "allow-leading-underscore",
      "allow-pascal-case"
    ],
    "semicolon": [true, "always", "ignore-bound-class-methods"],
    "trailing-comma": [
      true,
      {
        "multiline": {
          "objects": "always",
          "arrays": "always",
          "functions": "always",
          "typeLiterals": "ignore"
        }
      }
    ]
  }
}

Actual behavior

Error: x:y trailing-comma Missing trailing comma on the line of ...rest: any[]

Expected behavior

It shouldn't error since trailing commas are not allowed after spreaded parameters.
See: https://github.com/prettier/prettier/issues/1079#issuecomment-288693961

[ajafff]

trailing commas are not allowed after spreaded parameters.

Is there an official spec that states this?
TypeScript accepts trailing commas after rest parameters and spread arguments. See http://www.typescriptlang.org/play/#src=function%20foo(%0A%20%20%20%20...baz%3A%20string%5B%5D%2C%0A)%20%7B%0A%7D%0A%0Alet%20arr%20%3D%20%5B'a'%2C%20'b'%5D%3B%0Afoo(%0A%20%20%20%20...arr%2C%0A)%3B

[thgreasi]

Maybe we need a new tslint option for that.

[IllusionMH]

@ajafff https://github.com/tc39/proposal-object-rest-spread/issues/47#issuecomment-305799545

But there is a chance that TS Spec is not aligned with ES spec in this case.

UPD.
Edge

const [a,b,...c,] = [1,2,3] // Destructuring rest variables must be in the last position of the expression
function err(a, ...rest,) {} // The rest parameter must be the last parameter in a formals list.

Chrome:

const [a,b,...c,] = [1,2,3] // Uncaught SyntaxError: Rest element must be last element
function err(a, ...rest,) {} // Uncaught SyntaxError: Rest parameter must be last formal parameter

Option to skip trailing coma for rest looks like good idea, probably turned on by default.
Main reason for trailing comas - clean diffs when elements/props/params added, but it isn't possible to add something after rest param.
Trailing commas after spread should follow rules for objects/arrays/functions as previously.

[ajafff]

Ah, now I see. The issue title was misleading, so I updated it.
Thank you for the link and detailed explanation @IllusionMH

I agree that there should be an option that overrides the value specified for the options "arrays", "object" and "functions" to disallow (rather than ignoring) trailing commas on rest.

To whoever implements this: If the object or array is the target of the assignment, the trailing comma is not allowed, but on the right side it is.

({a, b, ...c,} = {a, b, ...c,}); // error only on c before the equals token

By just looking at the AST, both object literals are identical. You can use isReassignmentTarget of tsutils to conveniently check if it's on the left side of the assignment.

[adidahiya]

For TS, it doesn't feel right to fix this problem with a lint rule -- the compiler should instead be fixed to emit ES spec compliant JS.

However, it does make sense to fix this for use cases where TSLint is run on JS files.

[ajafff]

@adidahiya are you suggesting that this should not be enabled by an option but rather depend on the file name / type?

Regarding compiler output: I tested this with target: es6. The emit is spec compliant. So TypeScript just allows traling commas where ES forbids them.
This is not about fixing a bug in TypeScript, it's about consistency between TS and JS source files.

[thgreasi]

Linting a ts file has nothing to do with the compilation output.
Just like we have the option to require or prohibit trailing commas after the last array item and the compiler is happy with either choice; in the same manner we need the aforementioned option to enforce a coding style among devs.

[adidahiya]

@ajafff no, sorry, I don't mean to suggest that we should change the behavior based on file name / type. I do agree that consistency between TS / JS files is good, so the new option should apply to both kinds of files.

@thgreasi your initial post doesn't sound like a stylistic concern to me since you mention the spec. TS is lenient and allows the trailing comma, so I don't quite agree with this statement __for TS files__: "It shouldn't error since trailing commas are not allowed after spreaded parameters."

[myspivey]

It seems this should be re-opened as Typescript 2.9 no longer allows the comma on the end of spread operators.

[thgreasi]

Sound so, thankfully all that has to be done is to just set the default value for the new option to also prevent trailing commas and ignore the user preference.
That's if we can reason about the TS version used in the current project, otherwise this might better be a major release (or even a minor, since even TS included that breaking change in a minor release...).

[mgol]

Since TypeScript now disallows commas after an object rest param, I submitted a followup: #4172.