Linter: Fix byte-offset/UTF-16 index mismatch corrupting autofix and offense locations#1866
Open
drjayvee wants to merge 1 commit into
Open
Linter: Fix byte-offset/UTF-16 index mismatch corrupting autofix and offense locations#1866drjayvee wants to merge 1 commit into
drjayvee wants to merge 1 commit into
Conversation
…offense locations Prism reports node locations as UTF-8 byte offsets, but several rules and the erb-prefer-direct-output autofix used them directly as UTF-16 string indices via `source.substring()`. Any multi-byte character earlier in the file (even in an unrelated tag) shifted every subsequent offset, corrupting the generated output and misreporting offense locations/messages. Adds `stringIndexFromByteOffset` to `@herb-tools/core` and uses it wherever Prism byte offsets are converted to string positions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Heads up: this commit and PR are LLM-generated and only superficially reviewed by me. The bug is real though! End of human output :D
Summary
Prism reports node locations as UTF-8 byte offsets, but several linter rules and the
erb-prefer-direct-outputautofix used those offsets directly as UTF-16 string-index offsets when callingsource.substring(). This is only correct when the file is pure ASCII — any multi-byte character earlier in the file (even inside an unrelated tag or comment) shifts every subsequent byte offset, corrupting the result.This is what caused reports like:
when running
herb-lint --fixon a file that has any non-ASCII character (accented letters, emoji, etc.) earlier in the source.stringIndexFromByteOffsetto@herb-tools/coreto correctly convert a Prism UTF-8 byte offset to a UTF-16 string index.erb-prefer-direct-outputautofix (ERBStringToDirectOutputRewriter.extractStringContent/extractExpressionSource), which was corrupting the rewritten source.locationFromOffset(shared byerb-no-instance-variables-in-partials,erb-no-debug-output,erb-no-unused-literals,erb-no-unused-expressions,erb-prefer-direct-output), which was reporting the wrong offense line/column whenever non-ASCII characters preceded the offense.substringFromByteOffsethelper and uses it in the 3 rules that extracted a source snippet for their message text (erb-no-debug-output,erb-no-unused-literals,erb-no-unused-expressions), which were showing a corrupted snippet in the same scenario.Test plan
erb-prefer-direct-output.autofix.test.tsreproducing the reported bug (preceded by an accented character / emoji)stringIndexFromByteOffsetincore/test/prism/byte-offset.test.tserb-no-debug-output.test.ts,erb-no-unused-literals.test.ts,erb-no-unused-expressions.test.ts,erb-no-instance-variables-in-partials.test.ts0.10.2packages, patched with this fix) that all of the above scenarios produce corrupted output before the fix and correct output after