Skip to content

Fix UTF-16 positions in diagnostic ranges - #397

Open
dajiaohuang wants to merge 1 commit into
dalance:masterfrom
dajiaohuang:fix/utf16-diagnostic-ranges-20261016
Open

dajiaohuang wants to merge 1 commit into
dalance:masterfrom
dajiaohuang:fix/utf16-diagnostic-ranges-20261016

Conversation

@dajiaohuang

Copy link
Copy Markdown

Problem

Diagnostics currently use UTF-8 byte counts for LSP character positions. With the default UTF-16 position encoding, this puts underlines on the wrong characters after non-ASCII comments and can extend ranges past the end of a line.

For example, enable keyword_forbidden_wire_reg in .svlint.toml, initialize svls with the fixture directory as rootUri and working directory, then open:

module test;
  /* 中文 */ wire a;
endmodule

The stdio textDocument/publishDiagnostics response reports line 1, characters 15..19 for wire; the correct UTF-16 range is 11..15. A comment containing a non-BMP character has the same byte/code-unit mismatch.

Change

Convert parser/linter byte offsets to UTF-16 columns. Convert both endpoints rather than adding byte lengths to a UTF-16 start column, including the parser-error range ending at the end of a line. Add regression tests for BMP/non-BMP comments, line transitions and Unicode range endpoints.

Validation

  • cargo install --path . --locked --root <isolated-install>
  • cargo test --locked and cargo test --release --locked: 2 tests passed
  • Own isolated stdio client: initialize/initialized, didOpen/full didChange/didClose, shutdown/exit; ASCII/BMP/non-BMP diagnostics, Unicode text-rule spans, parser errors, disabled linter, empty/default configuration, excluded paths, include paths and defines (11 scenarios)
  • The Chinese-comment stdio range assertion fails on the base and passes with this change; the non-BMP case and both Unicode endpoint scenarios also pass
  • git diff --check

Tested on Windows with rustc 1.98.1. Linux/macOS, GUI editor integrations and dynamic plugins were not tested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant