Repository navigation
[string_scanner] Fix SpanScanner.within() and LineScanner edge cases - #2633
Conversation
* Fix `SpanScanner.within()`'s `state.line` and `state.column` to return span-relative coordinates rather than file-relative offsets. * Fix `SpanScanner.within()`'s `error()` to default `length` to `0` rather than `1` when `lastMatch` is `null`, preventing a `RangeError` when the span ends at the end of the underlying `SourceFile`. * Fix `SpanScanner.within()`'s `lastSpan` to be cleared when `lastMatch` is lazily invalidated after a position change. * Fix `SpanScanner.within()`'s `spanFromPosition()` to validate ranges against `string.length` rather than `_sourceFile.length - _startLocation.offset`. * Fix `LineScanner.position=` and `SpanScanner.eager().position=` to clear `lastMatch` and `lastSpan` when assigned the current `position`. * Deduplicate forward and backward CR/LF newline detection in `_LineScanner` via `_isNewlineAt()` and `_advancePosition()`, avoiding substring allocations in `LineScanner.scan()`.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors and fixes several issues in the string_scanner package. Specifically, it updates SpanScanner.within() to return span-relative coordinates for state.line and state.column, defaults the error length to 0 when there is no lastMatch to prevent RangeError at EOF, resets lastSpan when the position changes without a match, and validates ranges against the span's length in spanFromPosition(). Additionally, assigning the current position to LineScanner.position= or SpanScanner.eager().position= now correctly clears lastMatch and lastSpan. Comprehensive tests have been added to verify these behaviors. I have no feedback to provide as there are no review comments.
PR HealthCoverage ✔️
This check for test coverage is informational (issues shown here will not fail the PR). This check can be disabled by tagging the PR with License Headers ✔️
All source files should start with a license header. Unrelated files missing license headers
This check can be disabled by tagging the PR with Breaking changes ✔️
This check can be disabled by tagging the PR with Unused Dependencies ✔️
For details on how to fix these, see dependency_validator. This check can be disabled by tagging the PR with API leaks ✔️The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.
This check can be disabled by tagging the PR with Changelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. This check can be disabled by tagging the PR with |
SpanScanner.within()'sstate.lineandstate.columnto return span-relative coordinates rather than file-relative offsets.SpanScanner.within()'serror()to defaultlengthto0rather than1whenlastMatchisnull, preventing aRangeErrorwhen the span ends at the end of the underlyingSourceFile.SpanScanner.within()'slastSpanto be cleared whenlastMatchis lazily invalidated after a position change.SpanScanner.within()'sspanFromPosition()to validate ranges againststring.lengthrather than_sourceFile.length - _startLocation.offset.LineScanner.position=andSpanScanner.eager().position=to clearlastMatchandlastSpanwhen assigned the currentposition._LineScannervia_isNewlineAt()and_advancePosition(), avoiding substring allocations inLineScanner.scan().