fix(positions): report symbol and highlight columns in the negotiated encoding - #230
Merged
mmsaki merged 1 commit intoSep 15, 2026
Merged
Conversation
rifuki
force-pushed
the
fix/utf16-symbol-highlight-positions
branch
from
August 25, 2026 16:38
e80180e to
2e64b38
Compare
… encoding `symbols.rs` and `highlight.rs` built their LSP ranges straight from `Node::start_position()`, whose columns tree-sitter reports in bytes. LSP columns are in the negotiated encoding — UTF-16 unless the client opts into UTF-8 — so any non-ASCII text earlier on a line shifted every position after it. `selection.rs` converts its output columns, so its ranges were correct. documentHighlight was broken on both sides: `find_identifier_at` also fed `position.character` into a `tree_sitter::Point` as if it were a byte column, so a cursor placed inside an identifier on a non-ASCII line resolved to the wrong node — or to none at all, returning zero highlights. Measured against a contract containing `unicode"héllo 🚀"` before a declaration: documentSymbol reported column 66 where UTF-16 is 63, and documentHighlight returned no results at all. Both are correct now. Conversion goes through a new `utils::byte_column_to_position()` rather than the existing `byte_offset_to_position()`, because that one walks the source from byte 0 on every call. Fine once per request, quadratic when a whole syntax tree is converted node by node: an early version of this patch that used it took documentSymbol on forge-std `Vm.sol` from 5.0ms to 67ms, and on `PoolManager.t.sol` — the file behind the README's p95 table — from 4.6ms to 8.5ms. Tree-sitter already reports a column measured from the start of its row, so the line begins at `byte_offset - byte_column` and only that prefix needs measuring. Re-measured on the same fixtures, release build, best of 5: Vm.sol (152 KB) main 5.0ms branch 4.9ms safeconsole.sol (399 KB) main 21.2ms branch 20.1ms PoolManager.t.sol main 4.6ms branch 4.8ms Taking the row from tree-sitter rather than deriving it also keeps positions in the same line model as the tree they describe. `TextIndex` treats U+2028 and U+2029 as line terminators and tree-sitter does not, so an offset-derived row put symbols on the following line; there is a regression test for that. Affects documentSymbol, workspace/symbol and documentHighlight. The same defect is still present in `folding.rs`, `semantic_tokens.rs` (which encodes byte *lengths* as well as columns), `inlay_hints.rs`, and the cursor-resolution side of `selection.rs` — its output converts, but it builds its tree-sitter `Point` from `position.character` the way `highlight.rs` did. Those are left for a follow-up that can reuse `byte_column_to_position`, and the outstanding work is recorded in docs/pages/reference/symbols.md.
rifuki
force-pushed
the
fix/utf16-symbol-highlight-positions
branch
from
August 25, 2026 17:08
2e64b38 to
0feef61
Compare
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
2 participants
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.
Closes #234.
symbols.rsandhighlight.rsbuilt their ranges fromNode::start_position(), whose columns tree-sitter reports in bytes. LSP wants them in the negotiated encoding, so anything non-ASCII earlier on the line shifted every position after it.selection.rsconverts its output columns, so its ranges were correct.documentHighlightwas wrong on the input side too —find_identifier_atfedposition.characterinto atree_sitter::Pointas a byte column, so a cursor on a non-ASCII line resolved to the wrong node or to none.Both now go through a new
utils::byte_column_to_position(). I did not use the existingbyte_offset_to_position(), because it walks the source from byte 0 on every call — fine once per request, quadratic when you convert a whole tree node by node. My first attempt did use it and tookdocumentSymbolon forge-stdVm.solfrom 5.0ms to 67ms, and onPoolManager.t.solfrom 4.6ms to 8.5ms. Tree-sitter already gives a column measured from the start of its row, so the line starts atbyte_offset - byte_columnand only that prefix needs measuring. Re-measured, release build, best of 5: 4.9ms, 20.1ms and 4.8ms against 5.0ms, 21.2ms and 4.6ms onmain.Keeping tree-sitter's row also avoids a line-model mismatch —
TextIndextreats U+2028 as a line terminator and tree-sitter does not, so deriving the row put symbols on the following line. There is a test for that.6 new tests,
cargo test666 passed, clippy and fmt clean.While writing this I found the same defect is wider than I first thought:
folding.rs,semantic_tokens.rs(which encodes byte lengths as well as columns, so highlighting paints the wrong span),inlay_hints.rs, and the cursor-resolution side ofselection.rs— its output converts, but it builds its tree-sitterPointfromposition.characterthe wayhighlight.rsdid, so expand-selection grabs the wrong token. I left all of them out to keep this reviewable and recorded the outstanding work indocs/pages/reference/symbols.md. They can all reusebyte_column_to_positionfrom this PR; happy to do them next.