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

What does ban-comma-operator do? #3380

Closed
ChrisMBarr opened this issue Oct 23, 2017 · 4 comments
Closed

What does ban-comma-operator do? #3380

ChrisMBarr opened this issue Oct 23, 2017 · 4 comments

Comments

@ChrisMBarr
Copy link
Contributor

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?

@ajafff
Copy link
Contributor

ajafff commented Oct 23, 2017

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

@ChrisMBarr
Copy link
Contributor Author

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)

@ajafff
Copy link
Contributor

ajafff commented Oct 23, 2017

I like it. Thanks for the suggestions

@ajafff
Copy link
Contributor

ajafff commented Oct 24, 2017

Fixed by #3384

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

2 participants