Fix: no-sequences is reporting incorrect locations

[mdjermanovic]

What is the purpose of this pull request? (put an "X" next to item)

[X] Bug fix

Tell us about your environment

• ESLint Version: 6.3.0
• Node Version: 10.16.0
• npm Version: 6.9.0

What parser (default, Babel-ESLint, etc.) are you using?

default

Please show your full configuration:

Configuration

module.exports = {
  parserOptions: {
    ecmaVersion: 2015,
  }
};

What did you do? Please include the actual source code causing the issue.

Demo link

/* eslint no-sequences:error */

(1), 2;
((1)) , (2);
while((1) , 2);

What did you expect to happen?

3 errors with correct locations of the comma operator.

What actually happened? Please include the actual, raw output from ESLint.

It's reporting the location of the first closing paren after the first expression in the sequence.

  3:3  error  Unexpected use of comma operator  no-sequences
  4:4  error  Unexpected use of comma operator  no-sequences
  5:9  error  Unexpected use of comma operator  no-sequences

What changes did you make? (Give an overview)

Find the first comma token instead of just first token (which could be a closing paren token).

Additionally, the rule will now report comma operator's loc instead of just loc.start.

Is there anything you'd like reviewers to focus on?

I have a question. This rule, and most of the rules that report single-char tokens are reporting locations as token.loc.start instead of the full token.loc (which has the start and the end).

Why is that? This has a strange effects in editors, e.g. Demo does not highlight anything (at least in my browser), VSCode usually highlights the first character before the token (if it isn't an empty space) instead of the token character.

[platinumazure]

Why is that?

I would guess historical reasons more than anything else. We didn't always support reporting of end locations, so some of these rules' logic could simply have been written before we supported that. I think we should always require ranges of length 1 or higher personally, but that would be a core breaking change.

For this case specifically, I would be in support of highlighting the comma (with start/end), or the whole sequence.

[mdjermanovic]

It's changed now to report firstCommaToken.loc instead of just firstCommaToken.loc.start.

[mysticatea]

I have a question. This rule, and most of the rules that report single-char tokens are reporting locations as token.loc.start instead of the full token.loc (which has the start and the end).

It's a historical reason. After I added that feature, I had planned to update core rules (#3307 (comment)), but I had failed to make time.

[mysticatea]

LGTM, thank you!