Tslint: What does ban-comma-operator do?

Created on 23 Oct 2017  路  4Comments  路  Source: palantir/tslint

I saw the new rule ban-comma-operator was added, but the documentation literally just repeats the name of the rule and add the word "the" which is not helpful.

"Bans the comma operator."

It's like a dictionary definition referencing the word you are trying to understand.

Looking at the source wasn't helpful since the failure message just says "don't do this" without any reasoning as to why. The test file sheds a little bit of light on this, but not enough for me to understand what problem this rule solves.

I believe @calebegg is the author for this rule - could you possibly shed some light on this and add a more descriptive rule description, and the rationale behind it?

Documentation Accepting PRs good first issue

Most helpful comment

Gotcha, ok I was not even aware some of that was even possible, good to know! This seems like a good rule that just needs some "PR help". Below are my suggestions based on my current understanding, please feel free to improve upon them.

Rule failure message

Before: "Don't use the comma operator."
After: "Do not use comma operator here because it can be easily misunderstood and lead to bugs"

Rule Description

Before: "Bans the comma operator"
After: "Disallows the comma operator to be used. Read more about the comma operator here."

Rule Rational

"Using the comma operator can create a potential for many non-obvious bugs or lead to misunderstanding of code."
_(and then 2 or 3 code samples like what you provided above would be included here)_

All 4 comments

It bans this thing: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Comma_Operator

It catches not so obvious bugs like:

switch (foo) {
    case 1, 2: // equals `case 2` - probably intended `case 1: case2:`
        return true;
    case 3:
        return false;
}

foo((bar, baz)); // evaluates to `foo(baz)` because of the extra parens

Gotcha, ok I was not even aware some of that was even possible, good to know! This seems like a good rule that just needs some "PR help". Below are my suggestions based on my current understanding, please feel free to improve upon them.

Rule failure message

Before: "Don't use the comma operator."
After: "Do not use comma operator here because it can be easily misunderstood and lead to bugs"

Rule Description

Before: "Bans the comma operator"
After: "Disallows the comma operator to be used. Read more about the comma operator here."

Rule Rational

"Using the comma operator can create a potential for many non-obvious bugs or lead to misunderstanding of code."
_(and then 2 or 3 code samples like what you provided above would be included here)_

I like it. Thanks for the suggestions

Fixed by #3384

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ghost picture ghost  路  3Comments

ghost picture ghost  路  3Comments

allbto picture allbto  路  3Comments

denkomanceski picture denkomanceski  路  3Comments

DanielKucal picture DanielKucal  路  3Comments