Vscode and atom highlight locations are unexpected when end location is not provided

[golopot]

Summary
Many rules reports only start location and no end location. For these rules, editors often renders tildes in unexpected places.

Examples
In these examples, the rules only reports start location but no end locations. Instead of highlighting tokens like \n, (, or {, the token before the said token is highlighted.

/* eslint semi: [2, 'always'] */
const a = Math.random
//             ~~~~~~    actual highlight in vscode
//                   ~   expected highlight
//                   ^   reported start location


/* eslint object-curly-spacing: [2, 'always'] */
const b = {b: Math.random}
//                 ~~~~~~    actual highlight in vscode
//                       ~   expected highlight
//                       ^   reported start location


/* eslint space-in-parens: [2, 'never']  */
function ffff( a ) {}
//       ~~~~    actual highlight in vscode
//           ~   expected highlight
//           ^   reported start location

There are many other rules suffering from this problem, most of them are named *space* or *spacing*.

How to fix
The problem disappear if the both start location and end location are reported and start location is different to end location. That is, the location range is not zero-width-ed.

There are several places this can be done:

1. Modify each rules to report end location. Pros: explicit. Cons: lots of place needs change.

2. Modify ESLint core, automatically add end location when end location is missing. Cons: not explicit.

3. Modify vscode-eslint plugin to automatically add end location when end location is missing.

[g-plane]

Maybe we can modify rules, because this can benefit formatters as well. However, I'm not sure how formatters behave when reporting those rules.

[kaicataldo]

I think one of the tricky things with some rules like semi is that there might not be a character to highlight if it's at the end of the line. Example from above:

/* eslint semi: [2, 'always'] */
const a = Math.random
//                   ^ If there's no whitespace after the last character (and many editors can be configured
//                     to do this automatically), will VS Code be able to highlight it?

[golopot]

I have tested that case, report the location of the linefeed (with the end location at the next line) will do well in vscode.

[eslint-deprecated]

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that issues failing to reach accepted status after 21 days tend to
never be accepted, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.

[kaicataldo]

Reopening this because this is actively being worked on.

[kaicataldo]

Leaving the "evaluating" label on here because we haven't officially accepted this, but have been evaluating and accepting smaller individual PRs that solve this.

[aladdin-add]

@eslint/eslint-team, it seems an useful enhancement, can we get one more 👍 to accept this issue?

[btmills]

Is the decision here to accept this issue as blanket acceptance for PRs to individual rules that add end locations to reports (option 1 in the original issue description) so we don't have to evaluate them individually? I've added my 👍 under that understanding, and if that is indeed the case, then this can be marked as accepted.

[kaicataldo]

@btmills I think that makes the most sense? Ideally, it would be nice to audit the rules and make a list we can keep track of here.

[golopot]

... Ideally, it would be nice to audit the rules and make a list we can keep track of here.

This is a list of rules that reports start column but misses end column. The list is acquired by patching rule tester in a dirty way.

• array-callback-return (Update: Improve report location for array-callback-return (refs #12334) #13109)
• arrow-body-style (Update: improve report location in arrow-body-style (refs #12334) #13424)
• block-spacing (Update: block-spacing changed loc for extra (refs #12334) #13314)
• consistent-return (Update: add end location to report in consistent-return (refs #12334) #14798)
• comma-style (Update: Improve report location for comma-style #13111)
• curly (Update: change reporting location for curly rule (refs #12334) #14766)
• eol-last (Update: reporting loc for never option in eol-last (refs #12334) #14840 )
• func-call-spacing (Update: reporter loc of func-call-spacing (refs #12334) #13311)
• getter-return (Update: Improve report location for getter-return (refs #12334) #13164)
• indent-legacy (deprecated)
• key-spacing (Update: key-spacing loc changes for extra space (refs #12334) #13362)
• keyword-spacing ( Update: keyword-spacing unexpected space loc improve (refs #12334) #13377) (Fix: add end location to reports in keyword-spacing (refs #12334) #13461)
• linebreak-style (Update: Improve report location for linebreak-style (refs #12334) #13317)
• max-len (Update: Improve report location for max-len (refs #12334) #13458)
• max-lines (Update: max-lines reporting loc improvement (refs #12334) #13318)
• new-cap (Update: Improve report location for new-cap (refs #12334) #13136)
• newline-per-chained-call (Update: Improve report location newline-per-chained-call (refs #12334) #13116)
• no-empty-function (Update: Improve report location for no-empty-function (refs #12334) #13121)
• no-extra-bind (Fix: add end location to report in no-extra-bind (refs #12334) #13083)
• no-irregular-whitespace (Update: Improve report location no-irregular-whitespace (refs #12334) #13462)
• no-mixed-spaces-and-tabs (Update: Improve location for no-mixed-spaces-and-tabs (refs #12334) #13365)
• no-prototype-builtins (Fix: add end location to report in no-prototype-builtins (refs #12334) #13087)
• no-redeclare (Fix: getNameLocationInGlobalDirectiveComment end location (refs #12334) #13086)
• no-spaced-func (deprecated)
• no-unexpected-multiline (Update: report backtick loc in no-unexpected-multiline (refs #12334) #13142)
• no-unneeded-ternary (Update: Improve report location for no-unneeded-ternary (refs #12334) #13456)
• no-unused-vars (Fix: getNameLocationInGlobalDirectiveComment end location (refs #12334) #13086)
• no-useless-concat (Fix: add end location to report in no-useless-concat (refs #12334) #13110)
• object-curly-newline (Fix: add end location to reports in object-curly-newline (refs #12334) #13460)
• object-property-newline (Update: object-property-newline end location (refs #12334) #13399)
• one-var-declaration-per-line (Fix: report end loc in one-var-declaration-per-line (refs #12334) #13326)
• operator-linebreak (Update: report operator location in operator-linebreak (refs #12334) #13102)
• padded-blocks (Update: padded-blocks loc position changes (refs #12334) #13328)
• rest-spread-spacing (Update: Improve report location for rest-spread-spacing (refs #12334) #13313)
• semi
  (Update: improve location for semi and comma-dangle #12380)
  (Chore: add test assertions on reporting location in semi (refs #12334) #14899)
• semi-spacing (Update: reporting location for semi-spacing (refs #12334) #13285)
• space-before-function-paren (Update: Improve report location for space-before-function-paren #13128)
• template-curly-spacing (Update: Improve report location for template-curly-spacing #12813)
• template-tag-spacing (Update: Improve report location for template-tag-spacing (refs #12334) #13203)
• unicode-bom (Chore: assertions on reporting loc in unicode-bom (refs #12334) #14809)

[kaicataldo]

I will champion this. Given @btmills's comment here, I'm going to mark this as accepted. @golopot Thanks for spearheading this!

[fisker]

Any interest to implement a visualize tester like this to test locations? Or even better add to RuleTester?

[anikethsaha]

Do you mean snapshot testing ?

[fisker]

I mean this code frame

    `␊
      1 | function foo() {␊
    > 2 | 	function bar() {}␊
        | 	^^^^^^^^^^^^ Move function 'bar' to the outer scope.␊
      3 | }␊
    `

Easier to test.

[anikethsaha]

I meant, how would we assert that ? I can think of snapshot testing only other than any hacky approach.
Or you meant to do the assertion like it is currently setup and just visually display the frame ?

[fisker]

Yes, snapshot should be the easiest way.

[nzakas]

@fisker I’d suggest opening a separate issue to discuss your idea.

[snitin315]

Taking this up.

[snitin315]

This is all done 🎉 .

[nzakas]

Awesome! Thanks to everyone who helped push through this. 🙏