refactor(linter/plugins): stronger debug checks for tokens#16810
Conversation
github-actions
bot
added
A-linter
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
Pull request overview
This PR refactors token validation in the oxlint JavaScript plugin system by introducing a dedicated debugCheckValidRanges function that performs stronger debug-time validation. The refactoring consolidates duplicate checking logic and adds validation immediately after parsing to catch issues earlier in the pipeline.
Key Changes:
- Introduces
debugCheckValidRangesfunction that validates tokens have valid ranges (end > start) and are in ascending non-overlapping order - Adds immediate validation of parsed tokens and comments in
initTokens()before they're used elsewhere - Simplifies
debugCheckTokensAndComments()by removing redundant duplicate-start checking from the sort comparator, since this is now handled by the new validation function
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
The new debug range validation is a good direction, but the removal of the explicit “same start” invariant plus sorting only by range[0] can make tie cases non-deterministic and potentially less diagnosable. Consider making the ordering check and comparator total (e.g., by [start,end]) and/or explicitly rejecting duplicate starts in debugCheckValidRanges() so the debug assertions remain strict and deterministic.
Additional notes (1)
- Performance |
apps/oxlint/src-js/plugins/tokens.ts:177-185
initTokens()now doestokensAndComments.sort(...)on a freshly merged array, but the existingdebugCheckTokensAndComments()later validates thattokensAndComments(the module-level variable) is correctly ordered.
This is fine, but the current debug check in initTokens() is doing two sorts (here and later) and validates ordering of a merged list that is not otherwise used. If the intent is to catch issues earlier, you can avoid the extra sort by validating tokens and comments individually (already done) and leave merged-order validation to debugCheckTokensAndComments() (which checks the actual stored tokensAndComments).
Summary of changes
Summary
This change strengthens debug-time validation around token ranges produced by tsEslintParse.
- Imports
Rangefrom./location.tsto enable range validation typing. - Adds a debug-only validation pass in
initTokens():- Validates
tokensandcommentsindividually. - Merges
tokens+comments, sorts byrange[0], and validates the combined list.
- Validates
- Introduces
debugCheckValidRanges()to ensure each item has a validrange(end > start) and that items are in ascending (non-overlapping) order. - Simplifies
debugCheckTokensAndComments()sorting logic by removing the explicit “same start” check and relying on ordering validation elsewhere.
Merge activity
|
Add more debug checks for tokens - that they all have valid ranges, and are in ascending order.
graphite-app
bot
force-pushed
the
12-13-refactor_linter_plugins_stronger_debug_checks_for_tokens
branch
from
d608f7c to
c06d243
Compare
graphite-app
bot
deleted the
12-13-refactor_linter_plugins_stronger_debug_checks_for_tokens
branch
Add more debug checks for tokens - that they all have valid ranges, and are in ascending order.