[no-sequences] does not trigger if comma is used in parentheses as part of a condition

[danielrentz]

Tell us about your environment
Node version: v12.16.2
npm version: v6.14.4
Local ESLint version: v7.21.0 (Currently used)

Operating System:
Win10

What parser (default, @babel/eslint-parser, @typescript-eslint/parser, etc.) are you using?
default

Please show your full configuration:

rules:
  no-sequences: error

What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.

Actual code:

        if ((x1 === x2) && (y1, y2)) {

Intended was

        if ((x1 === x2) && (y1 === y2)) {

What did you expect to happen?

When a comma-sequence in parentheses is part of a longer expression (arithmetic, logical, ...), this rule should not let it pass.

[aladdin-add]

it was by design, per the docs:

This rule forbids the use of the comma operator, with the following exceptions:

• In the initialization or update portions of a for statement.
• If the expression sequence is explicitly wrapped in parentheses.

you can entirely disallow comma operator with the no-restricted-syntax:

{
    "rules": {
        "no-restricted-syntax": ["error", "SequenceExpression"]
    }
}

[danielrentz]

Hi @aladdin-add
thank you for the response. I know about this "by design" rule limitation, maybe this report should be labeled as "enhancement" instead of "bug".

The code example above is from our real code base, and it took me over an hour to track down the bug, because the faulty behavior is hidden in an short-circuiting AND expression. The improvement I am suggesting would have helped a lot here.

OTOH, the mentioned code base makes use of for-loops with multiple initializations, so completely disabling the comma operator is also not desirable...

[aladdin-add]

agreed it make sense to add an option to support it!

[mdjermanovic]

When a comma-sequence in parentheses is part of a longer expression (arithmetic, logical, ...), this rule should not let it pass.

It seems difficult to define this. What would be examples of a code with comma sequences that should pass?

[aladdin-add]

just an option to report comma-sequence in parentheses (eslint ignore it atm). IMHO, the option could be enabled by default in next major release.

[mdjermanovic]

just an option to report comma-sequence in parentheses (eslint ignore it atm).

Then, if the option is enabled, the rule would report all comma sequences except for those in for-loops? In other words, the option would just skip this check?

[aladdin-add]

yes!

[mdjermanovic]

Makes sense to me 👍

@danielrentz I marked this issue as "works as intended" since it's indeed the intended behavior. If you agree with the solution to add an option described above, then could you please open a rule change request issue? That way, it would be much easier for the team to evaluate a concrete proposal.

[danielrentz]

@mdjermanovic Yes, an option to skip (or enable) the "in-parentheses" exception would be fine for me.

[mdjermanovic]

Thanks! Then, I'm closing this in favor of #14197