Fix Node#rangeBy() ignoring index 0#2091
Open
sarathfrancis90 wants to merge 1 commit into
Open
Conversation
When calling Node#rangeBy() (and therefore Node#error() and Node#warn()) with index 0, the resulting range covered the whole node instead of a single character at offset 0. The end-of-range branch used a truthy check on opts.index, so an index of 0 fell through and left the end position at the node's end. This is the same off-by-zero issue that was previously fixed for endIndex; the index branches were missed. Switch both index checks to a numeric type check, matching the existing endIndex handling. index 0 now produces a single-character range, consistent with every other index value.
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.
What
Node#rangeBy()ignored anindexof0, returning a range that spanned the entire node instead of a single character at offset0. This also affectedNode#error()andNode#warn(), which build their positions throughrangeBy().Root cause
In
lib/node.js, the end-of-range branch used a truthy check onopts.index:With
index === 0the condition is falsy, so the branch is skipped andendkeeps the node's end position, producing a whole-node range. The start branch had the same truthy check, though for the start positionpositionInside(0)happens to equal the node start, so only the end was observably wrong.This is the same off-by-zero that was previously fixed for
endIndexin #1932 (else if (opts.endIndex)→typeof opts.endIndex === 'number'); the twoopts.indexbranches were left unchanged at that time.Fix
Switch both
opts.indexchecks to a numeric type check, matching the existingendIndexhandling:index: 0now yields a single-character range, consistent with every other index value.Tests
Added regression tests (both confirmed failing before the fix, passing after):
test/node.test.ts—rangeBy()returns a single-character range forindex: 0.test/warning.test.ts— a warning created withindex: 0reportsendColumnone past the start rather than the node end.Full suite locally:
pnpm unit— 654/654 passingpnpm test:lint— cleanpnpm test:types— clean