microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.29k stars 12.39k forks source link

Shorten error spans for missing key errors reported on object literals #58062

Open ssalbdivad opened 5 months ago

ssalbdivad commented 5 months ago

🔍 Search Terms

object literal, error span, squigglies, highlight, decoration, editor, range

✅ Viability Checklist

⭐ Suggestion

When defining an object literal with a missing key, e.g. as a function input, highlight only the first line of the object literal as opposed to the entire node.

📃 Motivating Example

Current behavior (one parameter with missing key):

image

Suggested behavior (one parameter with missing key): image

Current behavior (two parameters with missing keys): image

Suggested behavior(two parameters with missing keys): image

Here's a TS playground link with the original source:

https://tsplay.dev/N9a3qN

💻 Use Cases

This would make errors clearer and less jarring when defining large objects as function inputs.

RyanCavanaugh commented 5 months ago

I like the motivating example here, though it's definitely not the case that an object literal has a meaningfully-defined "start of line".

One implementation issue is that right now errors get an associated node which provides their error span, so we don't have any architectural way to do a span like

const foo = someFunc({
~~~~~~~~~~~~~~~~~~~~

where the span is a partial covering of part of the tree, nor do we have anything that is like "start at the start of the line" to help make the span not just be a single character that might be in the middle of some other punctuation soup. But I think that'd be fixable with some flags on the error object to say "highlighting the entire thing is not necessary".

Would be possibly open to PRs if we could have some better definition of what a better spanning rule is, showing the difficult cases as well as the easier ones. Super-long error spans aren't great but single-character spans can also be very hard to spot, so ensuring that we're getting a net benefit would be needed.

ssalbdivad commented 5 months ago

@RyanCavanaugh That's very helpful thanks for the additional input.

I was talking about this a bit with @Andarist as perhaps you inferred from the parallel https://github.com/microsoft/TypeScript/pull/58061.

I had also suggested potentially just right padding the start of an object literal if the contents before the first newline were less than e.g. 4 chars. I realize there might be some nuance to "if the contents before the first newline...", so we'd need to determine the best way to represent something like that that integrates reasonably well with the current error span model. Depending on what these flags might enable, this might be more intuitive visually:

image

Although perhaps this would be much less straightforward since I assume VSCode's model for decoration assumes a correlation with existing characters. Just thought I'd throw it out there since IMO, given no restrictions, I think it is the most intuitive and makes the best use of visual space to convey the error location.

RyanCavanaugh commented 5 months ago

Yeah, there's currently no way to make an error squiggle appear in the virtual space beyond the end of a line. Error spans are just start/end.