Repository navigation
fix(65078): keep empty spans for missing nodes when skipping trivia - #65081
Timur Vural (timurzvural) wants to merge 1 commit into
Conversation
typescript-automation
Bot
added
the
For Uncommitted Bug
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are recorded for the diagnostic-span handling changes.
No open issues 🎉
What changed in this PR
Fixes #65078 by preserving empty diagnostic spans for missing nodes, correcting TS8010’s reported position and preventing the associated --pretty crash.
Changes:
- Guard trivia skipping against empty and inverted ranges, and reuse it for JavaScript and decorator diagnostics.
- Treat negative diagnostic lengths as empty markers when formatting source snippets.
- Add a regression test asserting TS8010’s position and zero-length span for
var R: ;.
| File | Description |
|---|---|
tsc/internal/parser/parser.go |
Preserve valid diagnostic spans when skipping trivia. |
tsc/internal/parser/parser_test.go |
Verify the missing-type diagnostic retains its empty span. |
tsc/internal/diagnosticwriter/diagnosticwriter.go |
Clamp negative lengths before rendering diagnostic markers. |
Review effort: Balanced
|
|
||
| func (p *Parser) skipRangeTrivia(textRange core.TextRange) core.TextRange { | ||
| return core.NewTextRange(scanner.SkipTrivia(p.sourceText, textRange.Pos()), textRange.End()) | ||
| // Missing nodes use an empty range (pos == end). Skipping trivia from that |
There was a problem hiding this comment.
We have in general resisted doing this, as it usually means a bug elsewhere.
There was a problem hiding this comment.
Agreed — special-casing skipRangeTrivia was the wrong layer. The inverted span came from jsErrorAtRange always running SkipTrivia on a missing (empty) type annotation range. Updated to match Strada’s getErrorSpanForNode: skip trivia only when the range is non-empty. skipRangeTrivia is back to the plain helper.
| if length < 0 { | ||
| length = 0 |
There was a problem hiding this comment.
Okay, but why is length negative in the first place? That is surely a bug (and also this code, if we wanted it, would need to be written length = min(length, 0).
There was a problem hiding this comment.
Agreed — negative length shouldn’t reach the formatter. Removed the clamp; with the jsErrorAtRange fix the span is empty at the missing location (pos == end), so --pretty no longer sees a negative length. (Also: if we ever did clamp, it would be max(length, 0), not min.)
jsErrorAtRange always skipped trivia when building JS diagnostics. For a missing (empty) type annotation such as `var R: ;`, that moved start past end, inverted the TS8010 span, reported the wrong column, and panicked --pretty via strings.Repeat. Match Strada getErrorSpanForNode: skip trivia only when the range is non-empty. Keep skipRangeTrivia as a plain helper and do not clamp negative lengths in the diagnostic writer. Fixes microsoft#65078
276f0cf to
2bd966e
Compare
typescript-automation
Bot
added
For Backlog Bug
Fixes #65078.
Bug
jsErrorAtRange/skipRangeTriviaalways didTextRange(SkipTrivia(pos), end). For a missing type annotation (var R: ;in a JS file undercheckJs), the type node is an empty range. Skipping the space after:moves start past end, so the diagnostic length is −1.That reports TS8010 at column 8 instead of 7, and
--prettycrashes inwriteCodeSnippetwithpanic: strings: negative Repeat count.TypeScript 6.0.3 avoids this via
getErrorSpanForNode, which does not skip trivia for missing nodes.Fix
skipRangeTrivia: ifpos == end, return the range unchanged; also refuse to advance start past end.jsErrorAtRangeand the related decorator diagnostics now useskipRangeTrivia.writeCodeSnippet: treat negative lengths as empty markers so inverted spans cannot panic.Tests
TestJSErrorAtMissingTypeAnnotationKeepsEmptySpan— asserts TS8010 at pos 6 with length 0 forvar R: ;.Local verification
Built
tscand ran the issue repro:--pretty true: no panic; TS8010 at1:7with a single~--pretty false:a.js(1,7): error TS8010(matches 6.0.3)AI assistance disclosure
This PR was authored with assistance from Cursor Agent. I have read and understand the contents of this PR.