From ed676982f6a5cbc04cc6e53f2f215e47fbe1cb87 Mon Sep 17 00:00:00 2001 From: karitham Date: Fri, 7 Aug 2026 23:50:40 +0200 Subject: [PATCH] lsp: emit UTF-16 columns in every LSP range LSP character columns count UTF-16 code units; the parser's positions count runes. They diverge on astral-plane characters (emoji, non-BMP CJK), which encode as surrogate pairs, so any such character before a token on its line shifted every outgoing range one column short: go-to-definition underlines, rename edits, highlights, document symbols, folding, links, diagnostics, and semantic token spans (whose lengths were byte counts). Every range builder now converts through the file's mapper (toLSPPosition/toLSPRange on the ParsedFile): nodeRange, tokenRange, nameRange, spanRange, syntaxErrorToDiagnostic, and the semantic token encoder. The mapper already handled incoming positions (resolveTarget); outgoing ranges went through bare Col-1 math. Tests are red-first (utf16_test.go, K-On themed): each fixture puts an emoji before the token of interest so rune and UTF-16 columns diverge, and pins definition, document symbols, semantic tokens (comment length and keyword column), parse-error diagnostics, links, folding, and rename edits to the UTF-16 column. --- lsp/source/cycle_detect.go | 6 +- lsp/source/cycle_detect_test.go | 31 ++++--- lsp/source/diagnostic.go | 35 ++------ lsp/source/document.go | 81 ++++++++---------- lsp/source/fieldid_check.go | 4 +- lsp/source/folding.go | 52 +++++++----- lsp/source/links.go | 2 +- lsp/source/parse.go | 16 ++-- lsp/source/reference.go | 8 +- lsp/source/rename.go | 4 +- lsp/source/semantic.go | 20 ++++- lsp/source/semantic_analysis.go | 22 ++--- lsp/source/target.go | 41 +++++---- lsp/source/utf16_test.go | 144 ++++++++++++++++++++++++++++++++ 14 files changed, 299 insertions(+), 167 deletions(-) create mode 100644 lsp/source/utf16_test.go diff --git a/lsp/source/cycle_detect.go b/lsp/source/cycle_detect.go index 721d0c8..f5dae96 100644 --- a/lsp/source/cycle_detect.go +++ b/lsp/source/cycle_detect.go @@ -40,7 +40,7 @@ func cycleToDiagnosticItems(pairs []CyclePair) DiagnosticResult { func cyclePairToDiagnostic(pair CyclePair) protocol.Diagnostic { res := protocol.Diagnostic{ - Range: nodeRange(pair.include.doc, pair.include.include), + Range: nodeRange(pair.include.pf, pair.include.include), Severity: protocol.DiagnosticSeverityWarning, Source: protocol.NewOptional("thrift-ls"), Message: protocol.String(fmt.Sprintf("cycle dependency in %s", pair.include.file)), @@ -52,7 +52,7 @@ func cyclePairToDiagnostic(pair CyclePair) protocol.Diagnostic { type Include struct { file uri.URI include *syntax.Include - doc *syntax.Document + pf *cache.ParsedFile } type CyclePair struct { @@ -105,7 +105,7 @@ func getIncludes(ctx context.Context, ss *cache.Snapshot, file uri.URI, includes (*includesMap)[file] = append((*includesMap)[file], Include{ file: includeURI, include: includes[i], - doc: pf.AST(), + pf: pf, }) if _, ok := (*includesMap)[includeURI]; ok { diff --git a/lsp/source/cycle_detect_test.go b/lsp/source/cycle_detect_test.go index 42cb82e..f259680 100644 --- a/lsp/source/cycle_detect_test.go +++ b/lsp/source/cycle_detect_test.go @@ -6,10 +6,10 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.lsp.dev/uri" "github.com/karitham/thrift-ls/lsp/cache" - "github.com/karitham/thrift-ls/syntax" ) func Test_cycleDetect(t *testing.T) { @@ -231,31 +231,28 @@ include "./test/address.thrift"` }, }) - // Expected includes, built by parsing the same sources. - parseFor := func(uriStr, src string) *syntax.Document { - doc, errs := syntax.Parse([]byte(src)) - for _, e := range errs { - if e.Severity == syntax.SeverityError { - t.Fatal(e) - } - } + // Expected includes, parsed through the same snapshot so the ParsedFile + // pointers match the ones getIncludes stores. + pfFor := func(uriStr string) *cache.ParsedFile { + pf, err := ss.Parse(t.Context(), uri.URI(uriStr)) + require.NoError(t, err) - return doc + return pf } - userDoc := parseFor("file:///tmp/user.thrift", file1) - goodsDoc := parseFor("file:///tmp/test/goods.thrift", file2) - addressDoc := parseFor("file:///tmp/test/address.thrift", file3) + userPf := pfFor("file:///tmp/user.thrift") + goodsPf := pfFor("file:///tmp/test/goods.thrift") + addressPf := pfFor("file:///tmp/test/address.thrift") expectIncludeMap := map[uri.URI][]Include{ "file:///tmp/user.thrift": { - Include{file: "file:///tmp/test/goods.thrift", include: userDoc.Includes()[0], doc: userDoc}, - Include{file: "file:///tmp/test/address.thrift", include: userDoc.Includes()[1], doc: userDoc}, + Include{file: "file:///tmp/test/goods.thrift", include: userPf.AST().Includes()[0], pf: userPf}, + Include{file: "file:///tmp/test/address.thrift", include: userPf.AST().Includes()[1], pf: userPf}, }, "file:///tmp/test/goods.thrift": { - Include{file: "file:///tmp/user.thrift", include: goodsDoc.Includes()[0], doc: goodsDoc}, + Include{file: "file:///tmp/user.thrift", include: goodsPf.AST().Includes()[0], pf: goodsPf}, }, "file:///tmp/test/address.thrift": { - Include{file: "file:///tmp/user.thrift", include: addressDoc.Includes()[0], doc: addressDoc}, + Include{file: "file:///tmp/user.thrift", include: addressPf.AST().Includes()[0], pf: addressPf}, }, } diff --git a/lsp/source/diagnostic.go b/lsp/source/diagnostic.go index f3c91a0..293c4df 100644 --- a/lsp/source/diagnostic.go +++ b/lsp/source/diagnostic.go @@ -4,6 +4,7 @@ import ( "context" "errors" "log/slog" + "unicode/utf8" "go.lsp.dev/protocol" "go.lsp.dev/uri" @@ -66,39 +67,13 @@ func (d *Diagnostic) Name() string { type DiagnosticResult map[uri.URI][]protocol.Diagnostic // tokenRange converts a token's span to an LSP range. -func tokenRange(doc *syntax.Document, tok *syntax.Token) protocol.Range { +func tokenRange(pf *cache.ParsedFile, tok *syntax.Token) protocol.Range { if tok == nil { return protocol.Range{} } - start := doc.TokenPosition(tokIndex(doc, tok)) - end := doc.TokenEndPosition(tokIndex(doc, tok)) - - return protocol.Range{ - Start: protocol.Position{ - Line: uint32(start.Line - 1), - Character: uint32(start.Col - 1), - }, - End: protocol.Position{ - Line: uint32(end.Line - 1), - Character: uint32(end.Col - 1), - }, - } -} - -// tokIndex finds the index of a token pointer in the document's token -// stream by its offset. -func tokIndex(doc *syntax.Document, tok *syntax.Token) int { - if tok == nil { - return 0 - } - // Token pointers are stable: the first token with a matching offset is - // the one. - for i, t := range doc.Tokens { - if t.Offset == tok.Offset { - return i - } - } + start := syntax.Position{Line: tok.Line, Col: tok.Col, Offset: tok.Offset} + end := syntax.Position{Line: tok.Line, Col: tok.Col + utf8.RuneCountInString(tok.Text), Offset: tok.Offset + len(tok.Text)} - return 0 + return toLSPRange(pf, start, end) } diff --git a/lsp/source/document.go b/lsp/source/document.go index 1a6c83e..dcb4853 100644 --- a/lsp/source/document.go +++ b/lsp/source/document.go @@ -25,7 +25,7 @@ func DocumentSymbols(ctx context.Context, ss *cache.Snapshot, file uri.URI) []*p doc := pf.AST() for _, node := range doc.Nodes { - child := nodeSymbol(doc, node) + child := nodeSymbol(pf, node) if child != nil { res = append(res, child) } @@ -37,60 +37,51 @@ func DocumentSymbols(ctx context.Context, ss *cache.Snapshot, file uri.URI) []*p //go:fix inline // nameRange returns the LSP range of an identifier. -func nameRange(doc *syntax.Document, id *syntax.Identifier) protocol.Range { +func nameRange(pf *cache.ParsedFile, id *syntax.Identifier) protocol.Range { if id == nil { return protocol.Range{} } - start, end := doc.Range(id) - - return protocol.Range{ - Start: protocol.Position{ - Line: uint32(start.Line - 1), - Character: uint32(start.Col - 1), - }, - End: protocol.Position{ - Line: uint32(end.Line - 1), - Character: uint32(end.Col - 1), - }, - } + start, end := pf.AST().Range(id) + + return toLSPRange(pf, start, end) } // nodeSymbol builds the symbol for a top-level definition. -func nodeSymbol(doc *syntax.Document, node syntax.Node) *protocol.DocumentSymbol { +func nodeSymbol(pf *cache.ParsedFile, node syntax.Node) *protocol.DocumentSymbol { switch v := node.(type) { case *syntax.Typedef: - return typedefSymbol(doc, v) + return typedefSymbol(pf, v) case *syntax.Const: - return constSymbol(doc, v) + return constSymbol(pf, v) case *syntax.Struct: switch v.Kind { case syntax.StructDecl: - return structSymbol(doc, v, "Struct", protocol.SymbolKindStruct) + return structSymbol(pf, v, "Struct", protocol.SymbolKindStruct) case syntax.UnionDecl: - return structSymbol(doc, v, "Union", protocol.SymbolKindInterface) + return structSymbol(pf, v, "Union", protocol.SymbolKindInterface) case syntax.ExceptionDecl: - return structSymbol(doc, v, "Exception", protocol.SymbolKindClass) + return structSymbol(pf, v, "Exception", protocol.SymbolKindClass) } case *syntax.Enum: - return enumSymbol(doc, v) + return enumSymbol(pf, v) case *syntax.Service: - return serviceSymbol(doc, v) + return serviceSymbol(pf, v) } return nil } -func structSymbol(doc *syntax.Document, st *syntax.Struct, detail string, kind protocol.SymbolKind) *protocol.DocumentSymbol { +func structSymbol(pf *cache.ParsedFile, st *syntax.Struct, detail string, kind protocol.SymbolKind) *protocol.DocumentSymbol { res := &protocol.DocumentSymbol{ Name: st.Name.Text, Detail: &detail, Kind: kind, - Range: nameRange(doc, st.Name), - SelectionRange: nameRange(doc, st.Name), + Range: nameRange(pf, st.Name), + SelectionRange: nameRange(pf, st.Name), } for _, field := range st.Fields { - child := fieldSymbol(doc, field) + child := fieldSymbol(pf, field) if child != nil { res.Children = append(res.Children, *child) } @@ -99,20 +90,20 @@ func structSymbol(doc *syntax.Document, st *syntax.Struct, detail string, kind p return res } -func enumSymbol(doc *syntax.Document, enum *syntax.Enum) *protocol.DocumentSymbol { +func enumSymbol(pf *cache.ParsedFile, enum *syntax.Enum) *protocol.DocumentSymbol { res := &protocol.DocumentSymbol{ Name: enum.Name.Text, Detail: new("Enum"), Kind: protocol.SymbolKindEnum, - Range: nameRange(doc, enum.Name), - SelectionRange: nameRange(doc, enum.Name), + Range: nameRange(pf, enum.Name), + SelectionRange: nameRange(pf, enum.Name), } for _, value := range enum.Values { child := &protocol.DocumentSymbol{ Name: value.Name.Text, Kind: protocol.SymbolKindEnumMember, - Range: nameRange(doc, value.Name), - SelectionRange: nameRange(doc, value.Name), + Range: nameRange(pf, value.Name), + SelectionRange: nameRange(pf, value.Name), } res.Children = append(res.Children, *child) } @@ -120,19 +111,19 @@ func enumSymbol(doc *syntax.Document, enum *syntax.Enum) *protocol.DocumentSymbo return res } -func serviceSymbol(doc *syntax.Document, svc *syntax.Service) *protocol.DocumentSymbol { +func serviceSymbol(pf *cache.ParsedFile, svc *syntax.Service) *protocol.DocumentSymbol { res := &protocol.DocumentSymbol{ Name: svc.Name.Text, Kind: protocol.SymbolKindInterface, - Range: nameRange(doc, svc.Name), - SelectionRange: nameRange(doc, svc.Name), + Range: nameRange(pf, svc.Name), + SelectionRange: nameRange(pf, svc.Name), } for _, fn := range svc.Functions { child := &protocol.DocumentSymbol{ Name: fn.Name.Text, Kind: protocol.SymbolKindFunction, - Range: nameRange(doc, fn.Name), - SelectionRange: nameRange(doc, fn.Name), + Range: nameRange(pf, fn.Name), + SelectionRange: nameRange(pf, fn.Name), } res.Children = append(res.Children, *child) } @@ -140,31 +131,31 @@ func serviceSymbol(doc *syntax.Document, svc *syntax.Service) *protocol.Document return res } -func fieldSymbol(doc *syntax.Document, field *syntax.Field) *protocol.DocumentSymbol { +func fieldSymbol(pf *cache.ParsedFile, field *syntax.Field) *protocol.DocumentSymbol { return &protocol.DocumentSymbol{ Name: field.Name.Text, Kind: protocol.SymbolKindField, - Range: nameRange(doc, field.Name), - SelectionRange: nameRange(doc, field.Name), + Range: nameRange(pf, field.Name), + SelectionRange: nameRange(pf, field.Name), } } -func typedefSymbol(doc *syntax.Document, td *syntax.Typedef) *protocol.DocumentSymbol { +func typedefSymbol(pf *cache.ParsedFile, td *syntax.Typedef) *protocol.DocumentSymbol { return &protocol.DocumentSymbol{ Name: td.Name.Text, Detail: new("Typedef"), Kind: protocol.SymbolKindTypeParameter, - Range: nameRange(doc, td.Name), - SelectionRange: nameRange(doc, td.Name), + Range: nameRange(pf, td.Name), + SelectionRange: nameRange(pf, td.Name), } } -func constSymbol(doc *syntax.Document, cst *syntax.Const) *protocol.DocumentSymbol { +func constSymbol(pf *cache.ParsedFile, cst *syntax.Const) *protocol.DocumentSymbol { return &protocol.DocumentSymbol{ Name: cst.Name.Text, Detail: new("Const"), Kind: protocol.SymbolKindConstant, - Range: nameRange(doc, cst.Name), - SelectionRange: nameRange(doc, cst.Name), + Range: nameRange(pf, cst.Name), + SelectionRange: nameRange(pf, cst.Name), } } diff --git a/lsp/source/fieldid_check.go b/lsp/source/fieldid_check.go index cc25abb..e1862e2 100644 --- a/lsp/source/fieldid_check.go +++ b/lsp/source/fieldid_check.go @@ -73,7 +73,7 @@ func (c *FieldIDCheck) diagnostic(ctx context.Context, ss *cache.Snapshot, file if fieldID < 1 || fieldID > 32767 { for _, field := range set { ret = append(ret, protocol.Diagnostic{ - Range: tokenRange(pf.AST(), field.FieldID), + Range: tokenRange(pf, field.FieldID), Severity: protocol.DiagnosticSeverityError, Source: protocol.NewOptional("thrift-ls"), Message: protocol.String("field id should be a positive integer in [1, 32767]"), @@ -87,7 +87,7 @@ func (c *FieldIDCheck) diagnostic(ctx context.Context, ss *cache.Snapshot, file for _, field := range set { ret = append(ret, protocol.Diagnostic{ - Range: tokenRange(pf.AST(), field.FieldID), + Range: tokenRange(pf, field.FieldID), Severity: protocol.DiagnosticSeverityError, Source: protocol.NewOptional("thrift-ls"), Message: protocol.String("field id conflict"), diff --git a/lsp/source/folding.go b/lsp/source/folding.go index 53564f2..04af479 100644 --- a/lsp/source/folding.go +++ b/lsp/source/folding.go @@ -30,14 +30,14 @@ func Ranges(ctx context.Context, ss *cache.Snapshot, file uri.URI) []protocol.Fo for _, node := range doc.Nodes { switch v := node.(type) { case *syntax.Struct, *syntax.Enum, *syntax.Service: - if r, ok := bracedRange(doc, node); ok { + if r, ok := bracedRange(pf, node); ok { ranges = append(ranges, r) } case *syntax.Const: if v.Value != nil { switch v.Value.Kind { case syntax.ValueList, syntax.ValueMap: - if r, ok := spanRange(doc, v.Value.TokStart(), v.Value.TokEnd()); ok { + if r, ok := spanRange(pf, v.Value.TokStart(), v.Value.TokEnd()); ok { ranges = append(ranges, r) } } @@ -47,13 +47,13 @@ func Ranges(ctx context.Context, ss *cache.Snapshot, file uri.URI) []protocol.Fo if ann := nodeAnnotations(doc, doc.Nodes); len(ann) > 0 { for _, a := range ann { - if r, ok := spanRange(doc, a.TokStart(), a.TokEnd()); ok { + if r, ok := spanRange(pf, a.TokStart(), a.TokEnd()); ok { ranges = append(ranges, r) } } } - ranges = append(ranges, commentBlocks(doc)...) + ranges = append(ranges, commentBlocks(pf)...) sort.Slice(ranges, func(i, j int) bool { if ranges[i].StartLine != ranges[j].StartLine { @@ -77,11 +77,11 @@ func startChar(r protocol.FoldingRange) uint32 { // bracedRange returns the fold range of a brace-delimited body: from the // opening brace to the closing one. -func bracedRange(doc *syntax.Document, n syntax.Node) (protocol.FoldingRange, bool) { +func bracedRange(pf *cache.ParsedFile, n syntax.Node) (protocol.FoldingRange, bool) { open := -1 for i := n.TokStart(); i <= n.TokEnd(); i++ { - if doc.Tokens[i].Kind == syntax.TokenLBrace { + if pf.AST().Tokens[i].Kind == syntax.TokenLBrace { open = i break @@ -94,14 +94,14 @@ func bracedRange(doc *syntax.Document, n syntax.Node) (protocol.FoldingRange, bo close := open for i := n.TokEnd(); i > open; i-- { - if doc.Tokens[i].Kind == syntax.TokenRBrace { + if pf.AST().Tokens[i].Kind == syntax.TokenRBrace { close = i break } } - return spanRange(doc, open, close) + return spanRange(pf, open, close) } // nodeAnnotations collects the annotations of every top-level node, in @@ -139,7 +139,8 @@ func nodeAnnotation(n syntax.Node) *syntax.Annotations { // spanRange converts the token span [start, end] into a folding range. // Degenerate single-line spans yield no range. -func spanRange(doc *syntax.Document, start, end int) (protocol.FoldingRange, bool) { +func spanRange(pf *cache.ParsedFile, start, end int) (protocol.FoldingRange, bool) { + doc := pf.AST() s := doc.TokenPosition(start) e := doc.TokenEndPosition(end) @@ -147,17 +148,20 @@ func spanRange(doc *syntax.Document, start, end int) (protocol.FoldingRange, boo return protocol.FoldingRange{}, false } + startPos := toLSPPosition(pf, s) + endPos := toLSPPosition(pf, e) + return protocol.FoldingRange{ - StartLine: uint32(s.Line - 1), - StartCharacter: new(uint32(s.Col - 1)), - EndLine: uint32(e.Line - 1), - EndCharacter: new(uint32(e.Col - 1)), + StartLine: startPos.Line, + StartCharacter: new(uint32(startPos.Character)), + EndLine: endPos.Line, + EndCharacter: new(uint32(endPos.Character)), }, true } // commentSpanRange is spanRange for comment folds. -func commentSpanRange(doc *syntax.Document, start, end int) (protocol.FoldingRange, bool) { - r, ok := spanRange(doc, start, end) +func commentSpanRange(pf *cache.ParsedFile, start, end int) (protocol.FoldingRange, bool) { + r, ok := spanRange(pf, start, end) if ok { r.Kind = protocol.FoldingRangeKindComment } @@ -167,9 +171,10 @@ func commentSpanRange(doc *syntax.Document, start, end int) (protocol.FoldingRan // blockCommentSpan folds a multi-line block comment. The token records // only its start position, so the end line is derived from the text. -func blockCommentSpan(doc *syntax.Document, idx int) (protocol.FoldingRange, bool) { +func blockCommentSpan(pf *cache.ParsedFile, idx int) (protocol.FoldingRange, bool) { + doc := pf.AST() tok := doc.Tokens[idx] - start := doc.TokenPosition(idx) + startPos := toLSPPosition(pf, doc.TokenPosition(idx)) lines := strings.Count(tok.Text, "\n") if lines == 0 { @@ -179,9 +184,9 @@ func blockCommentSpan(doc *syntax.Document, idx int) (protocol.FoldingRange, boo last := tok.Text[strings.LastIndex(tok.Text, "\n")+1:] return protocol.FoldingRange{ - StartLine: uint32(start.Line - 1), - StartCharacter: new(uint32(start.Col - 1)), - EndLine: uint32(start.Line - 1 + lines), + StartLine: startPos.Line, + StartCharacter: new(uint32(startPos.Character)), + EndLine: startPos.Line + uint32(lines), EndCharacter: new(uint32(len(last))), Kind: protocol.FoldingRangeKindComment, }, true @@ -189,14 +194,15 @@ func blockCommentSpan(doc *syntax.Document, idx int) (protocol.FoldingRange, boo // commentBlocks folds consecutive same-line comments on consecutive // source lines, and multi-line block comments, into comment fold ranges. -func commentBlocks(doc *syntax.Document) []protocol.FoldingRange { +func commentBlocks(pf *cache.ParsedFile) []protocol.FoldingRange { var ranges []protocol.FoldingRange + doc := pf.AST() for i := 0; i < len(doc.Tokens); i++ { tok := doc.Tokens[i] if !isLineComment(tok.Kind) { if tok.Kind == syntax.TokenBlockComment || tok.Kind == syntax.TokenDocComment { - if r, ok := blockCommentSpan(doc, i); ok { + if r, ok := blockCommentSpan(pf, i); ok { ranges = append(ranges, r) } } @@ -211,7 +217,7 @@ func commentBlocks(doc *syntax.Document) []protocol.FoldingRange { } if i > start { - if r, ok := commentSpanRange(doc, start, i); ok { + if r, ok := commentSpanRange(pf, start, i); ok { ranges = append(ranges, r) } } diff --git a/lsp/source/links.go b/lsp/source/links.go index 6d76870..2096dfd 100644 --- a/lsp/source/links.go +++ b/lsp/source/links.go @@ -39,7 +39,7 @@ func Links(ctx context.Context, ss *cache.Snapshot, file uri.URI) []protocol.Doc target := resolver.ResolveInclude(file, text) out = append(out, protocol.DocumentLink{ - Range: tokenRange(doc, path), + Range: tokenRange(pf, path), Target: &target, }) } diff --git a/lsp/source/parse.go b/lsp/source/parse.go index 7407653..b838dd5 100644 --- a/lsp/source/parse.go +++ b/lsp/source/parse.go @@ -29,7 +29,7 @@ func (p *Parse) Diagnostic(ctx context.Context, ss *cache.Snapshot, changeFiles for _, err := range parseRes.Errors() { slog.Debug("diagnostic parse failed", "err", err) - res[uri] = append(res[uri], syntaxErrorToDiagnostic(err)) + res[uri] = append(res[uri], syntaxErrorToDiagnostic(parseRes, err)) } } @@ -46,22 +46,18 @@ func (p *Parse) Name() string { // syntaxErrorToDiagnostic converts a syntax error or warning to an LSP // diagnostic. -func syntaxErrorToDiagnostic(err syntax.Error) protocol.Diagnostic { +func syntaxErrorToDiagnostic(pf *cache.ParsedFile, err syntax.Error) protocol.Diagnostic { severity := protocol.DiagnosticSeverityError if err.Severity == syntax.SeverityWarning { severity = protocol.DiagnosticSeverityWarning } + pos := toLSPPosition(pf, syntax.Position{Line: err.Line, Col: err.Col, Offset: err.Offset}) + return protocol.Diagnostic{ Range: protocol.Range{ - Start: protocol.Position{ - Line: uint32(err.Line - 1), - Character: uint32(err.Col - 1), - }, - End: protocol.Position{ - Line: uint32(err.Line - 1), - Character: uint32(err.Col - 1), - }, + Start: pos, + End: pos, }, Severity: severity, Source: protocol.NewOptional("thrift-ls"), diff --git a/lsp/source/reference.go b/lsp/source/reference.go index 57844c1..23feb39 100644 --- a/lsp/source/reference.go +++ b/lsp/source/reference.go @@ -68,7 +68,7 @@ func Highlight(ctx context.Context, ss *cache.Snapshot, file uri.URI, pos protoc // search already includes it when the cursor sits on a usage, so the // set dedups. if id := target.identifier(); id != nil { - add(nodeRange(pf.AST(), id)) + add(nodeRange(pf, id)) } for _, r := range refs { @@ -288,7 +288,7 @@ func searchServiceDefinitionReferences(ctx context.Context, ss *cache.Snapshot, continue } - res = append(res, referenceHit{loc: jump(file, pf.AST(), svc.Extends), text: svc.Extends.Text}) + res = append(res, referenceHit{loc: jump(file, pf, svc.Extends), text: svc.Extends.Text}) } return res, err @@ -344,7 +344,7 @@ func searchDefinitionIdentifierReferences(ctx context.Context, ss *cache.Snapsho return } - res = append(res, referenceHit{loc: jump(file, pf.AST(), ft.Ident), text: ft.Ident.Text}) + res = append(res, referenceHit{loc: jump(file, pf, ft.Ident), text: ft.Ident.Text}) } var searchFieldType func(ft *syntax.FieldType) @@ -477,7 +477,7 @@ func searchConstValueIdentifierReference(ctx context.Context, ss *cache.Snapshot jumpValue := func(v *syntax.ConstValue) { if v != nil && v.Kind == syntax.ValueIdent && bareName(v.Text) == bareName(valueName) { - res = append(res, referenceHit{loc: jump(file, pf.AST(), v), text: v.Text}) + res = append(res, referenceHit{loc: jump(file, pf, v), text: v.Text}) } } processStructLike := func(fields []*syntax.Field) { diff --git a/lsp/source/rename.go b/lsp/source/rename.go index 4e5bfa3..c183d69 100644 --- a/lsp/source/rename.go +++ b/lsp/source/rename.go @@ -22,7 +22,7 @@ func PrepareRename(ctx context.Context, ss *cache.Snapshot, file uri.URI, pos pr switch target.kind { case TargetDefinition, TargetConstValue, TargetService: - rg := nodeRange(pf.AST(), target.node) + rg := nodeRange(pf, target.node) return &rg, nil } @@ -98,7 +98,7 @@ func Rename(ctx context.Context, ss *cache.Snapshot, file uri.URI, pos protocol. refs = append(refs, referenceHit{ loc: protocol.Location{ URI: file, - Range: nodeRange(pf.AST(), target.node), + Range: nodeRange(pf, target.node), }, text: "", }) diff --git a/lsp/source/semantic.go b/lsp/source/semantic.go index 64216c9..9a1f235 100644 --- a/lsp/source/semantic.go +++ b/lsp/source/semantic.go @@ -66,8 +66,22 @@ func Tokens(ctx context.Context, ss *cache.Snapshot, file uri.URI) ([]uint32, er continue } - line := tok.Line - 1 - char := tok.Col - 1 + // The token span in UTF-16 code units: lengths and columns are + // byte- and rune-based in the lexer, so non-ASCII content (e.g. + // astral chars in comments or string literals) shifts them. + start, err := pf.Mapper().OffsetToLSPPosition(tok.Offset) + if err != nil { + continue + } + + end, err := pf.Mapper().OffsetToLSPPosition(tok.Offset + len(tok.Text)) + if err != nil { + continue + } + + line := int(start.Line) + char := int(start.Character) + length := int(end.Character - start.Character) deltaChar := char if line == prevLine { @@ -77,7 +91,7 @@ func Tokens(ctx context.Context, ss *cache.Snapshot, file uri.URI) ([]uint32, er data = append(data, uint32(line-prevLine), uint32(deltaChar), - uint32(len(tok.Text)), + uint32(length), uint32(typ), 0, // no token modifiers ) diff --git a/lsp/source/semantic_analysis.go b/lsp/source/semantic_analysis.go index 9b0ca93..ae8d07e 100644 --- a/lsp/source/semantic_analysis.go +++ b/lsp/source/semantic_analysis.go @@ -67,7 +67,7 @@ func (s *SemanticAnalysis) checkDefineConflict(ctx context.Context, pf *cache.Pa field := fields[i] if _, exist := fieldMap[field.Name.Text]; exist { ret = append(ret, protocol.Diagnostic{ - Range: nodeRange(pf.AST(), field.Name), + Range: nodeRange(pf, field.Name), Severity: protocol.DiagnosticSeverityError, Source: protocol.NewOptional("thrift-ls"), Message: protocol.String("field name conflict with other field"), @@ -83,7 +83,7 @@ func (s *SemanticAnalysis) checkDefineConflict(ctx context.Context, pf *cache.Pa processDefinition := func(name string, node syntax.Node, kind string) { if previous, exist := definitionNameMap[name]; exist { ret = append(ret, protocol.Diagnostic{ - Range: nodeRange(pf.AST(), node), + Range: nodeRange(pf, node), Severity: protocol.DiagnosticSeverityError, Source: protocol.NewOptional("thrift-ls"), Message: protocol.String(fmt.Sprintf("%s name conflict with other %s", kind, previous)), @@ -127,7 +127,7 @@ func (s *SemanticAnalysis) checkDefineConflict(ctx context.Context, pf *cache.Pa for _, fn := range svc.Functions { if _, exist := fnMap[fn.Name.Text]; exist { ret = append(ret, protocol.Diagnostic{ - Range: nodeRange(pf.AST(), fn.Name), + Range: nodeRange(pf, fn.Name), Severity: protocol.DiagnosticSeverityWarning, Source: protocol.NewOptional("thrift-ls"), Message: protocol.String("function name conflict with other function"), @@ -216,7 +216,7 @@ func (s *SemanticAnalysis) checkConstValueExist(ctx context.Context, ss *cache.S _, id, err := FindConstValueDefinition(ctx, ss, file, pf.AST(), cst) if err != nil || id == nil { res = append(res, protocol.Diagnostic{ - Range: nodeRange(pf.AST(), cst), + Range: nodeRange(pf, cst), Severity: protocol.DiagnosticSeverityError, Source: protocol.NewOptional("thrift-ls"), Message: protocol.String("default value doesn't exist"), @@ -238,13 +238,13 @@ func (s *SemanticAnalysis) checkConstValueMatchType(pf *cache.ParsedFile, field switch valueKind { case syntax.ValueList, syntax.ValueMap, syntax.ValueString, syntax.ValueDouble: if !sameKind(expect, valueKind) { - return mismatchDiagnostic(pf.AST(), field, expect, kindName(valueKind)) + return mismatchDiagnostic(pf, field, expect, kindName(valueKind)) } case syntax.ValueInt: // true/false lex as int constants but are bools. if value.Text == "true" || value.Text == "false" { if expect != "bool" { - return mismatchDiagnostic(pf.AST(), field, expect, "bool") + return mismatchDiagnostic(pf, field, expect, "bool") } return nil @@ -253,11 +253,11 @@ func (s *SemanticAnalysis) checkConstValueMatchType(pf *cache.ParsedFile, field switch expect { case "i8", "i16", "i32", "i64": default: - return mismatchDiagnostic(pf.AST(), field, expect, "i64") + return mismatchDiagnostic(pf, field, expect, "i64") } case syntax.ValueIdent: if expect == "bool" { - return mismatchDiagnostic(pf.AST(), field, expect, "identifier") + return mismatchDiagnostic(pf, field, expect, "identifier") } } @@ -298,9 +298,9 @@ func kindName(kind syntax.ConstValueKind) string { return "unknown" } -func mismatchDiagnostic(doc *syntax.Document, field *syntax.Field, expect, got string) *protocol.Diagnostic { +func mismatchDiagnostic(pf *cache.ParsedFile, field *syntax.Field, expect, got string) *protocol.Diagnostic { return &protocol.Diagnostic{ - Range: nodeRange(doc, field.Value), + Range: nodeRange(pf, field.Value), Severity: protocol.DiagnosticSeverityError, Source: protocol.NewOptional("thrift-ls"), Message: protocol.String(fmt.Sprintf("expect %s but got %s", expect, got)), @@ -346,7 +346,7 @@ func (s *SemanticAnalysis) checkTypeExist(ctx context.Context, ss *cache.Snapsho _, id, _, err := FindTypeDefinition(ctx, ss, file, pf.AST(), ft) if err != nil || id == nil { res = append(res, protocol.Diagnostic{ - Range: nodeRange(pf.AST(), ft.Ident), + Range: nodeRange(pf, ft.Ident), Severity: protocol.DiagnosticSeverityError, Source: protocol.NewOptional("thrift-ls"), Message: protocol.String("field type doesn't exist"), diff --git a/lsp/source/target.go b/lsp/source/target.go index 44978a8..6416c3b 100644 --- a/lsp/source/target.go +++ b/lsp/source/target.go @@ -109,9 +109,9 @@ func (t *target) identifier() *syntax.Identifier { } // jump builds an LSP location for a node. -func jump(file uri.URI, doc *syntax.Document, node syntax.Node) protocol.Location { +func jump(file uri.URI, pf *cache.ParsedFile, node syntax.Node) protocol.Location { return protocol.Location{ - Range: nodeRange(doc, node), + Range: nodeRange(pf, node), URI: file, } } @@ -130,21 +130,30 @@ func jumpInFile(ctx context.Context, ss *cache.Snapshot, file uri.URI, node synt return protocol.Location{}, errNoAST } - return jump(file, pf.AST(), node), nil + return jump(file, pf, node), nil } -// nodeRange converts a node span to an LSP range. -func nodeRange(doc *syntax.Document, node syntax.Node) protocol.Range { - start, end := doc.Range(node) - - return protocol.Range{ - Start: protocol.Position{ - Line: uint32(start.Line - 1), - Character: uint32(start.Col - 1), - }, - End: protocol.Position{ - Line: uint32(end.Line - 1), - Character: uint32(end.Col - 1), - }, +// toLSPPosition converts a parser position to a protocol position via the +// file mapper, so character columns are UTF-16 code units as the protocol +// requires. When the offset does not map (defensive), the rune column is +// the fallback. +func toLSPPosition(pf *cache.ParsedFile, pos syntax.Position) protocol.Position { + p, err := pf.Mapper().OffsetToLSPPosition(pos.Offset) + if err != nil { + return protocol.Position{Line: uint32(pos.Line - 1), Character: uint32(pos.Col - 1)} } + + return protocolPosition(p) +} + +// toLSPRange converts a parser span to an LSP range with UTF-16 columns. +func toLSPRange(pf *cache.ParsedFile, start, end syntax.Position) protocol.Range { + return protocol.Range{Start: toLSPPosition(pf, start), End: toLSPPosition(pf, end)} +} + +// nodeRange converts a node span to an LSP range. +func nodeRange(pf *cache.ParsedFile, node syntax.Node) protocol.Range { + start, end := pf.AST().Range(node) + + return toLSPRange(pf, start, end) } diff --git a/lsp/source/utf16_test.go b/lsp/source/utf16_test.go new file mode 100644 index 0000000..90dfe14 --- /dev/null +++ b/lsp/source/utf16_test.go @@ -0,0 +1,144 @@ +package source + +// UTF-16 regression tests. +// +// LSP character columns count UTF-16 code units; the parser's columns +// count runes. The two diverge on astral-plane characters (e.g. emoji, +// which encode as surrogate pairs), so any emoji before a token on its +// line shifts the correct LSP column by one. These tests pin every range +// builder to UTF-16 columns. +// +// The fixtures follow the light music club at Sakuragaoka High: HTT +// (Houkago Tea Time) is the band, Gitah the guitar, fuwa fuwa time the +// song. + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.lsp.dev/protocol" + "go.lsp.dev/uri" + + "github.com/karitham/thrift-ls/lsp/cache" +) + +// utf16Snapshot parses src as htt.thrift. +func utf16Snapshot(t *testing.T, src string) *cache.Snapshot { + t.Helper() + + return cache.BuildSnapshotForTest([]*cache.FileChange{ + {URI: "file:///tmp/htt.thrift", Version: 0, Content: []byte(src), From: cache.FileChangeTypeDidOpen}, + }) +} + +// In "/* 😀 */ struct HTT {}", the emoji is one rune but two UTF-16 units, +// so every token after it on the line sits one column further than the +// rune-based column. HTT starts at UTF-16 column 16. + +func TestDefinitionUTF16(t *testing.T) { + // htt.thrift declares HTT after an emoji comment; sakuragaoka.thrift + // references it. The returned definition range must use UTF-16 columns. + ss := cache.BuildSnapshotForTest([]*cache.FileChange{ + {URI: "file:///tmp/htt.thrift", Version: 0, Content: []byte("/* 😀 */ struct HTT {}"), From: cache.FileChangeTypeDidOpen}, + {URI: "file:///tmp/sakuragaoka.thrift", Version: 0, Content: []byte("include \"htt.thrift\"\nstruct LightMusicClub {\n 1: required HTT band\n}"), From: cache.FileChangeTypeDidOpen}, + }) + + locs, err := Definition(t.Context(), ss, "file:///tmp/sakuragaoka.thrift", protocol.Position{ + Line: 2, + Character: 14, // 'H' of HTT, in UTF-16 units + }) + require.NoError(t, err) + require.Len(t, locs, 1) + assert.Equal(t, uri.URI("file:///tmp/htt.thrift"), locs[0].URI) + + // 'H' of HTT starts at UTF-16 column 16: 15 runes before it, one of + // them the emoji pair. + assert.Equal(t, uint32(16), locs[0].Range.Start.Character) +} + +func TestDocumentSymbolsUTF16(t *testing.T) { + src := "/* 😀 */ struct HTT {\n 1: required string yui\n}" + + ss := utf16Snapshot(t, src) + + syms := DocumentSymbols(t.Context(), ss, "file:///tmp/htt.thrift") + require.Len(t, syms, 1) + + assert.Equal(t, uint32(16), syms[0].SelectionRange.Start.Character) +} + +func TestSemanticTokensUTF16(t *testing.T) { + tokens := semanticTokens(t, "/* 😀 */ struct HTT {}") + + // The comment is 8 UTF-16 units long: 7 ASCII chars + the emoji pair. + comment := tokens[0] + require.Equal(t, tokComment, comment.typ) + assert.Equal(t, uint32(8), comment.length) + + // The struct keyword starts after the comment at UTF-16 column 9. + kw := tokens[1] + require.Equal(t, tokKeyword, kw.typ) + assert.Equal(t, uint32(9), kw.char) +} + +func TestParseErrorDiagnosticUTF16(t *testing.T) { + src := `/* 😀 */ const string song = "fuwa fuwa time` + + ss := utf16Snapshot(t, src) + + res, err := (&Parse{}).Diagnostic(t.Context(), ss, []uri.URI{"file:///tmp/htt.thrift"}) + require.NoError(t, err) + + diags := res["file:///tmp/htt.thrift"] + require.NotEmpty(t, diags) + + // The unterminated string error points at the opening quote, at UTF-16 + // column 29: 28 runes before it, one of them the emoji pair. + assert.Equal(t, uint32(29), diags[0].Range.Start.Character) +} + +func TestLinksUTF16(t *testing.T) { + src := `include /* 😀 */ "htt.thrift"` + + file := "file:///tmp/sakuragaoka.thrift" + ss := buildLinksSnapshot(t, uri.URI(file), src) + + links := Links(t.Context(), ss, uri.URI(file)) + require.Len(t, links, 1) + + // The path literal starts at UTF-16 column 17: 16 runes before it, one + // of them the emoji pair. + assert.Equal(t, uint32(17), links[0].Range.Start.Character) +} + +func TestFoldingUTF16(t *testing.T) { + ranges := foldingRanges(t, "/* 😀 */ struct HTT {\n}\n") + require.Len(t, ranges, 1) + + // The fold spans the braces; the opening brace starts at UTF-16 column + // 27: 26 runes before it, one of them the emoji pair. + require.NotNil(t, ranges[0].StartCharacter) + assert.Equal(t, uint32(20), *ranges[0].StartCharacter) +} + +// TestRenameUTF16 exercises the same path as Definition through rename: +// the workspace edit for the definition file must use UTF-16 columns. +func TestRenameUTF16(t *testing.T) { + ss := cache.BuildSnapshotForTest([]*cache.FileChange{ + {URI: "file:///tmp/htt.thrift", Version: 0, Content: []byte("/* 😀 */ struct HTT {}"), From: cache.FileChangeTypeDidOpen}, + {URI: "file:///tmp/sakuragaoka.thrift", Version: 0, Content: []byte("include \"htt.thrift\"\nstruct LightMusicClub {\n 1: required HTT band\n}"), From: cache.FileChangeTypeDidOpen}, + }) + + edit, err := Rename(t.Context(), ss, "file:///tmp/sakuragaoka.thrift", protocol.Position{ + Line: 2, + Character: 14, + }, "HoukagoTeaTime") + require.NoError(t, err) + + edits := edit.Changes["file:///tmp/htt.thrift"] + require.NotEmpty(t, edits) + + // The renamed identifier starts at UTF-16 column 16. + assert.Equal(t, uint32(16), edits[0].Range.Start.Character) +} -- 2.51.2