Repository navigation
fix(65078): keep empty spans for missing nodes when skipping trivia - #65081
Open
Timur Vural (timurzvural) wants to merge 1 commit into
Open
Timur Vural (timurzvural) wants to merge 1 commit into
Timur Vural (timurzvural) wants to merge 1 commit into
Conversation
SkipTrivia on an empty (missing) type annotation could move start past end, inverting the TS8010 range and panicking --pretty via strings.Repeat. Match Strada: do not skip trivia for empty ranges, and clamp negative snippet lengths as a backstop. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
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
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.