bugfix(parser): Skipped-token diagnostic spans the token, not its leading comment. - #10088
Conversation
|
This change is |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
2a5cc7b to
8a7a46a
Compare
c61ce65 to
5051f92
Compare
PR SummaryLow Risk Overview A diagnostic test covers Reviewed by Cursor Bugbot for commit b8f2bd0. Bugbot is set up for automated code reviews on this repo. Configure here. |
…ding comment. skip_until computed the "Skipped tokens" span from self.offset, which after take_raw() points at the start of the skipped token's leading trivia. A skipped token preceded by a `//` line comment therefore had its diagnostic underline the comment instead of the token. Advance past the leading trivia (text_start = offset + leading_trivia_width) for both the span start and end, mirroring append_skipped_token_to_pending_trivia. Adds regression tests to diagnostics/skipped_tokens.
8a7a46a to
b8f2bd0
Compare
TomerStarkware
left a comment
There was a problem hiding this comment.
@TomerStarkware reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on eytan-starkware).
eytan-starkware
left a comment
There was a problem hiding this comment.
@eytan-starkware reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status:
Summary
Fix the diagnostic span for skipped tokens so that it points to the token itself rather than to any leading trivia (e.g., a comment) that precedes it. Previously,
diag_startwas set toself.offsetbefore accounting for leading trivia width, causing the reported error span to begin at the start of the leading trivia. Now,text_startis computed by advancing past the leading trivia, anddiag_start/diag_endare derived from that position.Type of change
Please check one:
Why is this change needed?
When a skipped token was preceded by a leading comment or other trivia, the diagnostic span incorrectly pointed at the beginning of that trivia rather than at the actual skipped token. This made error messages misleading, as the caret would appear under the comment rather than the offending token.
What was the behavior or documentation before?
The error span for a skipped token started at
self.offset, which is the position before leading trivia is consumed. For example, a)preceded by// a leading commentwould have its error caret pointing at the comment line rather than at).What is the behavior or documentation after?
The error span now starts at
text_start, which isself.offsetadvanced by the total width of the leading trivia. The caret in the diagnostic correctly points at the skipped token itself, not at any preceding comment or whitespace.Related issue or discussion (if any)
Additional context
Two new test cases are added to
skipped_tokensto cover both theskip_untilpath and the single-token skip path, verifying that the diagnostic span targets the token rather than its leading comment.