perf(linter/plugins): use start and end instead of range in tokens methods#16808
Conversation
github-actions
bot
added
A-linter
github-actions
bot
added
the
C-performance
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. |
overlookmotel
force-pushed
the
12-13-perf_linter_plugins_use_start_and_end_instead_of_range_in_tokens_methods
branch
from
485d2a0 to
12d543f
Compare
overlookmotel
force-pushed
the
12-13-refactor_linter_plugins_shorten_nodeortoken_comment_type
branch
from
b1f2d7a to
6274182
Compare
There was a problem hiding this comment.
Pull request overview
This PR optimizes token-related methods in the linting plugin by using direct start and end properties instead of array indexing (range[0] and range[1]) for system-produced tokens and comments. This is a performance improvement that makes property access faster and more direct. The PR correctly maintains the use of range array indexing for user-provided parameters where these properties may not exist.
Key changes:
- Removed TODO comment about replacing
range[0]withstart(now addressed) - Updated all token/comment property accesses in binary search and iteration logic to use
startandend - Preserved
range[0]andrange[1]usage for user-provided function parameters
💡 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.
Most changes correctly replace range[0/1] with start/end for tokens/comments and keep range for user-supplied nodes. Two potential improvements remain: strengthen debugCheckTokensAndComments() to validate end/non-overlap invariants, and consider a token/comment fast-path in getTokenAfter() to use end when applicable.
Additional notes (2)
-
Maintainability |
apps/oxlint/src-js/plugins/tokens.ts:259-263
expected.sort()is now comparing onlya.start/b.startand throws on ties. Previously it usedrange[0]which is equivalent for start comparisons, but the debug check still doesn't validate the end ordering invariant (i.e., that the merged list is strictly non-overlapping and monotonically increasing byend). Since this debug function is explicitly meant to validate ordering correctness, it's a good opportunity to strengthen it now thatendis available on tokens/comments. -
Performance |
apps/oxlint/src-js/plugins/tokens.ts:1011-1019
getTokenAfter()still readsrangeEndfromnodeOrToken.range[1], which is fine per the instruction to keep usingrangefor user-provided params. However, this function searches intokenListwhich may include comments whenincludeCommentsis true; the binary search comparestokenList[mid].startagainstrangeEnd(good). The potential issue is correctness whennodeOrTokenis itself a token/comment: usingrange[1]is correct, but if we knownodeOrTokenis a token/comment in this code path, we could useendand avoid array indexing. As written, it unconditionally usesrange, missing the perf win for the token/comment case.
Summary of changes
What changed
- Updated token/comment position access throughout
apps/oxlint/src-js/plugins/tokens.tsto use the newtoken.start/token.endproperties instead oftoken.range[0]/token.range[1]. - Removed an obsolete TODO in
initTokensAndComments()about migrating fromrangetostart. - Adjusted ordering/check logic in
debugCheckTokensAndComments()and whitespace detection (isSpaceBetween*) to rely onstart/end. - Updated all binary searches in token query helpers (
getTokens*,getFirstToken*,getLastToken*,getToken*) to compare againststart.
Overall this is a targeted perf-oriented refactor that keeps range for user-provided node inputs while using start/end for internally-produced tokens/comments, matching the PR context.
graphite-app
bot
force-pushed
the
12-13-refactor_linter_plugins_shorten_nodeortoken_comment_type
branch
from
6274182 to
f567ee7
Compare
graphite-app
bot
force-pushed
the
12-13-perf_linter_plugins_use_start_and_end_instead_of_range_in_tokens_methods
branch
from
12d543f to
ad06b3f
Compare
overlookmotel
force-pushed
the
12-13-perf_linter_plugins_use_start_and_end_instead_of_range_in_tokens_methods
branch
from
ad06b3f to
32ddb16
Compare
overlookmotel
force-pushed
the
12-13-refactor_linter_plugins_shorten_nodeortoken_comment_type
branch
from
f567ee7 to
2ddff0c
Compare
overlookmotel
force-pushed
the
12-13-perf_linter_plugins_use_start_and_end_instead_of_range_in_tokens_methods
branch
from
32ddb16 to
9b6667a
Compare
overlookmotel
force-pushed
the
12-13-refactor_linter_plugins_shorten_nodeortoken_comment_type
branch
2 times, most recently
from
395c082 to
ae67bee
Compare
overlookmotel
force-pushed
the
12-13-perf_linter_plugins_use_start_and_end_instead_of_range_in_tokens_methods
branch
from
9b6667a to
cf8e0a1
Compare
Merge activity
|
…ens methods (#16808) #16805 added `start` and `end` properties to tokens. Use them instead of the slower and more verbose `range[0]` and `range[1]` in all tokens methods. Note: Only use `start` and `end` for tokens and comments, as we produce them and know for sure that these properties are present. Continue to use `range` for objects passed to methods by user.
graphite-app
bot
force-pushed
the
12-13-refactor_linter_plugins_shorten_nodeortoken_comment_type
branch
from
ae67bee to
c4fb462
Compare
graphite-app
bot
force-pushed
the
12-13-perf_linter_plugins_use_start_and_end_instead_of_range_in_tokens_methods
branch
from
cf8e0a1 to
3b7d345
Compare
graphite-app
bot
deleted the
12-13-perf_linter_plugins_use_start_and_end_instead_of_range_in_tokens_methods
branch
#16805 added
startandendproperties to tokens. Use them instead of the slower and more verboserange[0]andrange[1]in all tokens methods.Note: Only use
startandendfor tokens and comments, as we produce them and know for sure that these properties are present. Continue to userangefor objects passed to methods by user.