Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Multiline trailing comma rule flags type parameters #1769

Closed
wmrowan opened this issue Nov 22, 2016 · 8 comments
Closed

Multiline trailing comma rule flags type parameters #1769

wmrowan opened this issue Nov 22, 2016 · 8 comments

Comments

@wmrowan
Copy link
Contributor

wmrowan commented Nov 22, 2016

Bug Report

  • TSLint version: 4.0.1
  • TypeScript version: 2.1.1
  • Running TSLint via: CLI

TypeScript code being linted

somefunction<
  TypeArg1,
  TypeArg2
>(
  arg1,
  arg2,
);

with tslint.json configuration:

{
    "trailing-comma": [true, {"multiline": "always", "singleline": "never"}]
}

Actual behavior

TSLint flags line 3 in the sample code as a lint error. TSLint says that this line should have a trailing comma.

Expected behavior

TSLint should not flag this as a lint failure as a trailing comma is not allowed here by Typescript. Adding a comma to satisfy the linter will result in a Typescript compile error. While I think I personally agree with TSLint that a comma should be allowed here to be consistent with non-type function arguments, Typescript nevertheless disallows this syntax. While I think it would be reasonable to ask Typescript to fix this inconsistency, TSLint should probably be consistent with Typescript in the meantime.

@jmlopez-rod
Copy link
Contributor

Perhaps more options need to be available, see eslint comma-dangle. With TSLint we need an option for generics.

@adidahiya
Copy link
Contributor

yeah, makes sense, if it is not valid TypeScript then TSLint should not flag it as a violation. accepting PRs to fix this in the default rule behavior

wmrowan added a commit to plaid/tslint that referenced this issue Nov 22, 2016
Typescript disallows trailing commas in type parameter lists, both on declaration and invocation. Linting should not flag this case as fixing the lint error would introduce a compiler error.

See palantir#1769
@wmrowan wmrowan closed this as completed Dec 2, 2016
@mizunashi-mana
Copy link

mizunashi-mana commented Dec 17, 2016

Missing such case:

var foo = <
    A,
    B
     ~ [Missing trailing comma] // expecting no warnings
>(a: A, b: B) => 42;

This issue should be reopened.
However, this may be caused by syntax parser.

@mizunashi-mana
Copy link

I found another case:

function foo(a: [
  number,
  string
  ~ [Missing trailing comma] // expecting no warnings
]): void {
  return 42; 
}

@hitendramalviya
Copy link

Is there any workaround to get this fixed or we just need to sicked with // tslint:disable-next-line:trailing-comma ? But due to this my ts file looks ugly.

@adidahiya
Copy link
Contributor

this was fixed by a277873, available in v4.1+

@nchen63 nchen63 reopened this Jan 22, 2017
@nchen63
Copy link
Contributor

nchen63 commented Jan 22, 2017

I tested @mizunashi-mana's 2 test cases above and they're not fixed in 4.3.1

@nchen63
Copy link
Contributor

nchen63 commented Jan 22, 2017

actually, #1905 was opened to cover those issues

@nchen63 nchen63 closed this as completed Jan 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants