From adff941f05b6ab71c99281d3190e10329454ebee Mon Sep 17 00:00:00 2001 From: karitham Date: Fri, 7 Aug 2026 22:04:58 +0200 Subject: [PATCH] lsp: qualified type completion, no services in type slots --- lsp/completion/provider.go | 7 +- lsp/completion/semantic_completion.go | 102 ++++++++--- lsp/format.go | 244 +++++++++++++------------- lsp/format_range_fuzz_test.go | 107 +++++------ lsp/format_range_test.go | 203 +++++++++------------ lsp/impl_test.go | 123 +++++++++++++ 6 files changed, 472 insertions(+), 314 deletions(-) diff --git a/lsp/completion/provider.go b/lsp/completion/provider.go index 088e5b0..c65df1a 100644 --- a/lsp/completion/provider.go +++ b/lsp/completion/provider.go @@ -32,7 +32,10 @@ func providersFor(kind ContextKind) []Provider { case CtxIncludePath: return []Provider{includeProvider{}} case CtxType: - return []Provider{typeProvider{}, keywordProvider{}} + // The type provider covers base and container keywords itself, so + // the identifier-dumping keyword provider cannot leak non-type + // names (services) into a type position. + return []Provider{typeProvider{}} case CtxFieldValue: return []Provider{valueProvider{}} case CtxFieldName: @@ -63,7 +66,7 @@ type typeProvider struct{} func (typeProvider) Kind() ContextKind { return CtxType } func (typeProvider) Candidates(ctx context.Context, ss *cache.Snapshot, file uri.URI, c Context) []Candidate { - return typeCandidates(ctx, ss, file, c.Doc) + return typeCandidates(ctx, ss, file, c) } type valueProvider struct{} diff --git a/lsp/completion/semantic_completion.go b/lsp/completion/semantic_completion.go index 7e532c3..5c50a89 100644 --- a/lsp/completion/semantic_completion.go +++ b/lsp/completion/semantic_completion.go @@ -3,6 +3,7 @@ package completion import ( "context" "sort" + "strings" "go.lsp.dev/protocol" "go.lsp.dev/uri" @@ -14,39 +15,45 @@ import ( // typeCandidates collects the names of all type definitions (structs, // unions, exceptions, enums, typedefs, services) from the file and its // transitively included files, plus the base type keywords. -func typeCandidates(ctx context.Context, ss *cache.Snapshot, file uri.URI, doc *syntax.Document) []Candidate { - names := make(map[string]struct{}) - collectTypeNames := func(ast *syntax.Document) { - for _, st := range ast.Structs() { - names[st.Name.Text] = struct{}{} +func typeCandidates(ctx context.Context, ss *cache.Snapshot, file uri.URI, c Context) []Candidate { + // A dotted prefix scopes the completion to the include: suggest the + // include's type names, qualified with the include name. + if i := strings.LastIndexByte(c.Prefix, '.'); i >= 0 { + includeName := c.Prefix[:i] + + incURI := ss.Resolver().GetIncludeURI(file, c.Doc, includeName) + if incURI == "" { + return nil } - for _, st := range ast.Unions() { - names[st.Name.Text] = struct{}{} + pf, err := ss.Parse(ctx, incURI) + if err != nil || pf.AST() == nil { + return nil } - for _, st := range ast.Exceptions() { - names[st.Name.Text] = struct{}{} - } + names := make(map[string]struct{}) + collectTypeNames(pf.AST(), names) - for _, enum := range ast.Enums() { - names[enum.Name.Text] = struct{}{} + res := make([]Candidate, 0, len(names)) + for name := range names { + res = append(res, Candidate{ + showText: includeName + "." + name, + insertText: includeName + "." + name, + format: protocol.InsertTextFormatPlainText, + }) } - for _, td := range ast.Typedefs() { - names[td.Name.Text] = struct{}{} - } + sort.Slice(res, func(i, j int) bool { return res[i].showText < res[j].showText }) - for _, svc := range ast.Services() { - names[svc.Name.Text] = struct{}{} - } + return res } - collectTypeNames(doc) + names := make(map[string]struct{}) + collectTypeNames(c.Doc, names) for _, inc := range includedFiles(ss, file) { if pf, err := ss.Parse(ctx, inc); err == nil && pf.AST() != nil { - collectTypeNames(pf.AST()) + collectTypeNames(pf.AST(), names) } } @@ -59,11 +66,66 @@ func typeCandidates(ctx context.Context, ss *cache.Snapshot, file uri.URI, doc * }) } + for _, kw := range typeKeywords { + res = append(res, Candidate{ + showText: kw.text, + insertText: kw.text, + format: kw.format, + }) + } + sort.Slice(res, func(i, j int) bool { return res[i].showText < res[j].showText }) return res } +// collectTypeNames adds the type definition names of a document to names: +// structs, unions, exceptions, enums, and typedefs. Services are not +// types and never complete in a type position. +func collectTypeNames(ast *syntax.Document, names map[string]struct{}) { + for _, st := range ast.Structs() { + names[st.Name.Text] = struct{}{} + } + + for _, st := range ast.Unions() { + names[st.Name.Text] = struct{}{} + } + + for _, st := range ast.Exceptions() { + names[st.Name.Text] = struct{}{} + } + + for _, enum := range ast.Enums() { + names[enum.Name.Text] = struct{}{} + } + + for _, td := range ast.Typedefs() { + names[td.Name.Text] = struct{}{} + } +} + +// typeKeywords are the base and container type keywords offered in a type +// position. +var typeKeywords = []struct { + text string + format protocol.InsertTextFormat +}{ + {"bool", protocol.InsertTextFormatPlainText}, + {"byte", protocol.InsertTextFormatPlainText}, + {"i8", protocol.InsertTextFormatPlainText}, + {"i16", protocol.InsertTextFormatPlainText}, + {"i32", protocol.InsertTextFormatPlainText}, + {"i64", protocol.InsertTextFormatPlainText}, + {"double", protocol.InsertTextFormatPlainText}, + {"string", protocol.InsertTextFormatPlainText}, + {"binary", protocol.InsertTextFormatPlainText}, + {"slist", protocol.InsertTextFormatPlainText}, + {"uuid", protocol.InsertTextFormatPlainText}, + {"list<$1>", protocol.InsertTextFormatSnippet}, + {"set<$1>", protocol.InsertTextFormatSnippet}, + {"map<$1, $2>", protocol.InsertTextFormatSnippet}, +} + // valueCandidates collects const names and enum names and values from the // file and its transitively included files, both bare and enum-qualified. func valueCandidates(ctx context.Context, ss *cache.Snapshot, file uri.URI, doc *syntax.Document) []Candidate { diff --git a/lsp/format.go b/lsp/format.go index a2817ec..287bc3b 100644 --- a/lsp/format.go +++ b/lsp/format.go @@ -3,14 +3,12 @@ package lsp import ( "bytes" "context" - "strings" "go.lsp.dev/protocol" "github.com/karitham/thrift-ls/formatter" "github.com/karitham/thrift-ls/lsp/mapper" "github.com/karitham/thrift-ls/lsp/types" - "github.com/karitham/thrift-ls/syntax" ) func (s *Server) formatting(ctx context.Context, params *protocol.DocumentFormattingParams) (result []protocol.TextEdit, err error) { @@ -75,11 +73,11 @@ func (s *Server) formatting(ctx context.Context, params *protocol.DocumentFormat // rangeFormatting implements textDocument/rangeFormatting. // // The formatter only knows how to print whole documents, so a range is -// formatted by extracting the selected slice, formatting it as a standalone -// document, and splicing the result back into the file. Following Prettier's -// range-formatting contract, the range must be bounded by blank lines (or -// file edges) after expansion to line boundaries; otherwise no edits are -// produced and the request is a no-op. +// formatted by formatting the whole document and diffing it against the +// original at the granularity of blank-line-separated blocks. Blank lines +// are preserved exactly by the formatter, so the blocks align one-to-one; +// every edit is bounded by blank lines or file edges, and any subset +// splices safely. Only the edits overlapping the selection are returned. func (s *Server) rangeFormatting(ctx context.Context, params *protocol.DocumentRangeFormattingParams) (result []protocol.TextEdit, err error) { fileURI := params.TextDocument.URI @@ -101,50 +99,66 @@ func (s *Server) rangeFormatting(ctx context.Context, params *protocol.DocumentR return nil, err } - mp := mapper.NewMapper(fileURI, content) - - start, err := mp.LSPPosToParserPosition(lspPosition(params.Range.Start)) + pf, err := ss.Parse(ctx, fileURI) if err != nil { - return nil, nil + return nil, err } - end, err := mp.LSPPosToParserPosition(lspPosition(params.Range.End)) + if len(pf.Errors()) > 0 || pf.AST() == nil { + return nil, pf.AggregatedError() + } + + formatted, err := formatter.Format(pf.AST(), s.formatOpts) if err != nil { - return nil, nil + return nil, err } - rs, re := start.Offset, end.Offset - if rs >= re { + if string(content) == formatted { return nil, nil } - // A selection covering the whole document delegates to full formatting. - if rs == 0 && re == len(content) { - return s.formatting(ctx, &protocol.DocumentFormattingParams{TextDocument: params.TextDocument}) - } + mp := mapper.NewMapper(fileURI, content) - newText, rs, re, ok := formatRangeText(content, rs, re, s.formatOpts) - if !ok { + start, err := mp.LSPPosToParserPosition(lspPosition(params.Range.Start)) + if err != nil { return nil, nil } - startPos, err := mp.OffsetToLSPPosition(rs) + end, err := mp.LSPPosToParserPosition(lspPosition(params.Range.End)) if err != nil { return nil, nil } - endPos, err := mp.OffsetToLSPPosition(re) - if err != nil { - return nil, nil + // The selection expanded to whole lines. + selStart := lineStart(content, start.Offset) + selEnd := nextLineStart(content, lineStart(content, end.Offset)) + + for _, be := range blockDiff(content, []byte(formatted)) { + // Overlap test on byte offsets; adjacent edits touch at most. + if be.end <= selStart || be.start >= selEnd { + continue + } + + startPos, err := mp.OffsetToLSPPosition(be.start) + if err != nil { + return nil, nil + } + + endPos, err := mp.OffsetToLSPPosition(be.end) + if err != nil { + return nil, nil + } + + result = append(result, protocol.TextEdit{ + Range: protocol.Range{ + Start: protocolPosition(startPos), + End: protocolPosition(endPos), + }, + NewText: be.text, + }) } - return []protocol.TextEdit{{ - Range: protocol.Range{ - Start: protocolPosition(startPos), - End: protocolPosition(endPos), - }, - NewText: newText, - }}, nil + return result, nil } // lspPosition converts a protocol position to the internal position type. @@ -163,50 +177,98 @@ func protocolPosition(p types.Position) protocol.Position { } } -// formatRangeText formats content[rs:re] (half-open byte offsets) and returns -// the replacement text. ok is false when the range is not safely bounded by -// blank lines or file edges, or the slice does not parse cleanly. -func formatRangeText(content []byte, rs, re int, opts formatter.Options) (newText string, outRS, outRE int, ok bool) { - // An empty or inverted selection produces no edits. - if rs >= re { - return "", rs, re, false +// blockEdit replaces content[start:end] with text. Every block edit is +// bounded by blank lines or file edges, so it splices safely. +type blockEdit struct { + start, end int + text string +} + +// blockDiff returns the edits turning old into new, one per changed block +// of non-blank lines, plus the leading and trailing blank regions when +// they differ. Blank lines are preserved exactly by the formatter, so old +// and new split into the same number of aligned blocks. +func blockDiff(old, new []byte) []blockEdit { + oldBlocks := blocks(old) + newBlocks := blocks(new) + + if len(oldBlocks) != len(newBlocks) { + // Should not happen: the formatter preserves the blank-line + // structure. Fall back to a single whole-document edit. + return []blockEdit{{0, len(old), string(new)}} } - // Expand to line boundaries, then trim leading and trailing blank lines - // from the selection. - rs = lineStart(content, rs) - re = lineEnd(content, re) - rs = skipBlankLinesForward(content, rs, re) + var edits []blockEdit - re = skipBlankLinesBackward(content, rs, re) - if rs >= re { - return "", rs, re, false + // Leading blank region. + oldLead := old[:oldBlocks[0].start] + newLead := new[:newBlocks[0].start] + if string(oldLead) != string(newLead) { + edits = append(edits, blockEdit{0, oldBlocks[0].start, string(newLead)}) } - // The lines immediately outside the range must be blank, or the range - // must touch a file edge. - if !blankLineBefore(content, rs) || !blankLineAfter(content, re) { - return "", rs, re, false + for i := range oldBlocks { + if oldBlocks[i].text != newBlocks[i].text { + edits = append(edits, blockEdit{ + start: oldBlocks[i].start, + end: oldBlocks[i].end, + text: newBlocks[i].text, + }) + } } - slice := content[rs:re] - - doc, errs := syntax.Parse(slice) - if len(errs) > 0 { - return "", rs, re, false + // Trailing blank region. + oldTail := old[oldBlocks[len(oldBlocks)-1].end:] + newTail := new[newBlocks[len(newBlocks)-1].end:] + if string(oldTail) != string(newTail) { + last := oldBlocks[len(oldBlocks)-1] + edits = append(edits, blockEdit{last.end, len(old), string(newTail)}) } - formatted, err := formatter.Format(doc, opts) - if err != nil { - return "", rs, re, false + return edits +} + +// block is a maximal run of non-blank lines: the byte range from the first +// line's start to just after the last line's newline, with the exact text. +type block struct { + start, end int + text string +} + +// blocks splits content into runs of non-blank lines. +func blocks(content []byte) []block { + var out []block + + i := 0 + for i < len(content) { + // Skip blank lines. + for i < len(content) && len(bytes.TrimSpace(content[i:lineEnd(content, i)])) == 0 { + i = nextLineStart(content, i) + } + + if i >= len(content) { + break + } + + start := i + for i < len(content) && len(bytes.TrimSpace(content[i:lineEnd(content, i)])) > 0 { + i = nextLineStart(content, i) + } + + out = append(out, block{start: start, end: i, text: string(content[start:i])}) } - // The splice must not add or drop newlines at the boundaries: the slice - // starts at a line start and ends just before its last line's newline. - formatted = strings.TrimLeft(formatted, "\n") - formatted = strings.TrimRight(formatted, "\r\n") + return out +} + +// nextLineStart returns the offset just after the newline ending the line +// containing offset, or len(content) for the last line. +func nextLineStart(content []byte, offset int) int { + if i := bytes.IndexByte(content[offset:], '\n'); i != -1 { + return offset + i + 1 + } - return formatted, rs, re, true + return len(content) } // lineStart returns the byte offset of the start of the line containing offset. @@ -230,57 +292,3 @@ func lineEnd(content []byte, offset int) int { // blankLineBefore reports whether the line before the line starting at offset // is blank (whitespace only) or offset is at the start of the file. -func blankLineBefore(content []byte, offset int) bool { - if offset == 0 { - return true - } - - start := lineStart(content, offset-1) - - return len(bytes.TrimSpace(content[start:offset])) == 0 -} - -// blankLineAfter reports whether the line after the one ending at offset is -// blank (whitespace only) or offset is at the end of the file. -func blankLineAfter(content []byte, offset int) bool { - if offset == len(content) { - return true - } - - end := lineEnd(content, offset+1) - - return len(bytes.TrimSpace(content[offset+1:end])) == 0 -} - -// skipBlankLinesForward advances offset past blank lines, stopping at limit. -func skipBlankLinesForward(content []byte, offset, limit int) int { - for offset < limit { - end := lineEnd(content, offset) - if end >= limit { - break - } - - if len(bytes.TrimSpace(content[offset:end])) > 0 { - break - } - - offset = end + 1 - } - - return offset -} - -// skipBlankLinesBackward retreats offset (a newline position or len(content)) -// past blank lines, stopping at start. -func skipBlankLinesBackward(content []byte, start, offset int) int { - for offset > start { - lineStart := lineStart(content, offset-1) - if len(bytes.TrimSpace(content[lineStart:offset])) > 0 { - break - } - - offset = max(lineStart-1, 0) - } - - return offset -} diff --git a/lsp/format_range_fuzz_test.go b/lsp/format_range_fuzz_test.go index 789804d..a874ec0 100644 --- a/lsp/format_range_fuzz_test.go +++ b/lsp/format_range_fuzz_test.go @@ -1,17 +1,18 @@ package lsp import ( + "strings" "testing" "github.com/karitham/thrift-ls/formatter" "github.com/karitham/thrift-ls/syntax" ) -// FuzzFormatRangeText checks the invariants of range formatting: -// - a refused range must produce no output -// - an accepted range must be structurally sane (line-aligned, in bounds) -// - splicing the formatted slice back into the document must keep the -// document parseable +// FuzzFormatRangeText checks the invariants of block-diff range formatting: +// - a document the formatter refuses yields no edits +// - an unchanged document yields no edits +// - the edits are ordered, non-overlapping, line-aligned, and in bounds +// - applying every edit reproduces the whole-document formatting func FuzzFormatRangeText(f *testing.F) { seeds := []string{ "struct A {\n1: string a\n}\n\nstruct B { 1: i32 b }", @@ -26,73 +27,73 @@ func FuzzFormatRangeText(f *testing.F) { "\n\n\n", } for _, s := range seeds { - f.Add([]byte(s), 0, 0) + f.Add([]byte(s)) } - f.Fuzz(func(t *testing.T, content []byte, rs, re int) { + f.Fuzz(func(t *testing.T, content []byte) { opts := formatter.Options{} - // Clamp the fuzzed offsets into the content. - limit := len(content) + 1 - rs = rs % limit - if rs < 0 { - rs += limit - } + doc, errs := syntax.Parse(content) + if len(errs) > 0 { + // The formatter refuses documents with errors: no edits. + if edits := blockDiff(content, content); len(edits) != 0 { + t.Fatalf("unparseable document produced edits") + } - re = re % limit - if re < 0 { - re += limit + return } - newText, outRS, outRE, ok := formatRangeText(content, rs, re, opts) - if !ok { + formatted, err := formatter.Format(doc, opts) + if err != nil { return } - // The accepted range must be line-aligned and in bounds. - if outRS < 0 || outRE > len(content) || outRS >= outRE { - t.Fatalf("invalid accepted range [%d, %d) for content %q", outRS, outRE, content) - } + if string(content) == formatted { + if edits := blockDiff(content, []byte(formatted)); len(edits) != 0 { + t.Fatalf("no-op format produced edits for %q", content) + } - if outRS != 0 && content[outRS-1] != '\n' { - t.Fatalf("accepted range starts mid-line at %d in %q", outRS, content) + return } - if outRE != len(content) && content[outRE] != '\n' { - t.Fatalf("accepted range ends mid-line at %d in %q", outRE, content) + edits := blockDiff(content, []byte(formatted)) + if len(edits) == 0 { + t.Fatalf("changed format produced no edits for %q", content) } - // Splicing the formatted slice back must not introduce new parse - // errors: errors outside the range (the original may not parse - // cleanly) must be preserved, and the formatted slice itself is - // known to parse. - spliced := make([]byte, 0, len(content)-outRE+outRS+len(newText)) - spliced = append(spliced, content[:outRS]...) - spliced = append(spliced, newText...) - spliced = append(spliced, content[outRE:]...) - - origErrs := errorMessages(syntax.Parse(content)) - - splicedErrs := errorMessages(syntax.Parse(spliced)) - for msg, splicedCount := range splicedErrs { - if splicedCount > origErrs[msg] { - t.Fatalf("splice introduced new parse error %q\ncontent: %q\nrange: [%d, %d)\nnewText: %q", - msg, content, outRS, outRE, newText) + // Edits are ordered, non-overlapping, line-aligned, and in bounds. + prev := 0 + + for _, e := range edits { + if e.start < prev || e.end > len(content) || e.start >= e.end { + t.Fatalf("invalid edit [%d, %d) in %q", e.start, e.end, content) + } + + if e.start != 0 && content[e.start-1] != '\n' { + t.Fatalf("edit starts mid-line at %d in %q", e.start, content) } + + if e.end != len(content) && content[e.end] != '\n' { + t.Fatalf("edit ends mid-line at %d in %q", e.end, content) + } + + prev = e.end } - }) -} -// errorMessages counts error-severity messages, ignoring warnings. -func errorMessages(doc *syntax.Document, errs []syntax.Error) map[string]int { - _ = doc - counts := make(map[string]int) + // Applying every edit reproduces the whole-document formatting. + var sb strings.Builder + prev = 0 - for _, err := range errs { - if err.Severity == syntax.SeverityError { - counts[err.Message]++ + for _, e := range edits { + sb.Write(content[prev:e.start]) + sb.WriteString(e.text) + prev = e.end } - } - return counts + sb.Write(content[prev:]) + + if sb.String() != formatted { + t.Fatalf("applying all edits != whole-document format\ncontent: %q\nformatted: %q\nedits: %+v", content, formatted, edits) + } + }) } diff --git a/lsp/format_range_test.go b/lsp/format_range_test.go index 4c919d8..a18db69 100644 --- a/lsp/format_range_test.go +++ b/lsp/format_range_test.go @@ -5,162 +5,123 @@ import ( "testing" "github.com/stretchr/testify/assert" - - "github.com/karitham/thrift-ls/formatter" - "github.com/karitham/thrift-ls/syntax" + "github.com/stretchr/testify/require" ) -func TestFormatRangeText(t *testing.T) { - opts := formatter.Options{} - +func TestBlockDiff(t *testing.T) { tests := []struct { - name string - content string - sel func(content string) (rs, re int) - want string - wantOK bool + name string + old string + new string + want []blockEdit }{ { - // selection inside the struct B line: expands to the whole line, - // bounded by the blank lines around struct B. - name: "formats struct bounded by blank lines", - content: "struct A {\n1: string a\n}\n\nstruct B { 1: i32 b }", - sel: func(c string) (int, int) { - return strings.Index(c, " 1:") + 1, strings.Index(c, "i32") + 1 - }, - want: "struct B { 1: i32 b }", - wantOK: true, - }, - { - // selection covering both declarations, bounded by blank lines. - name: "selection of two structs formats both", - content: "\nstruct A {\n1: string a\n}\n\nstruct B {\n2: i32 b\n}\n\nstruct C {}", - sel: func(c string) (int, int) { - return strings.Index(c, "struct A"), strings.Index(c, "struct C") - 2 - }, - want: "struct A { 1: string a }\n\nstruct B { 2: i32 b }", - wantOK: true, - }, - { - // selection starts on a blank line before struct B. - name: "trims leading blank lines from selection", - content: "struct A {}\n\n\n\nstruct B {\n1: string b\n}", - sel: func(c string) (int, int) { - return strings.Index(c, "struct B"), len(c) - }, - want: "struct B { 1: string b }", - wantOK: true, - }, - { - // selection ends on a blank line after struct B. - name: "trims trailing blank lines from selection", - content: "struct A {}\n\nstruct B {\n1: string b\n}\n\n\nstruct C {}", - sel: func(c string) (int, int) { - return strings.Index(c, "struct B"), strings.Index(c, "struct C") - 2 - }, - want: "struct B { 1: string b }", - wantOK: true, + name: "unchanged documents yield no edits", + old: "struct A {}\n\nstruct B {}\n", + new: "struct A {}\n\nstruct B {}\n", + want: nil, }, { - // selection cuts struct B in half: the expanded slice does not - // parse, so formatting is refused. - name: "refuses when selection cuts a declaration", - content: "struct A {}\n\nstruct B {\n1: string b\n}\n\nstruct C {}", - sel: func(c string) (int, int) { - return strings.Index(c, "struct B") + 6, strings.Index(c, "1: string b") + 4 - }, - wantOK: false, + name: "one changed block", + old: "struct A {}\n\nstruct B {\n2: i32 b\n}\n", + new: "struct A {}\n\nstruct B { 2: i32 b }\n", + want: []blockEdit{{ + start: strings.Index("struct A {}\n\nstruct B {\n2: i32 b\n}\n", "struct B"), + end: len("struct A {}\n\nstruct B {\n2: i32 b\n}\n"), + text: "struct B { 2: i32 b }\n", + }}, }, { - // the line right after the range is not blank. - name: "refuses when next line is not blank", - content: "struct A {\n1: string a\n}\nstruct B {}", - sel: func(c string) (int, int) { - return 0, strings.Index(c, "struct B") - 1 + name: "two changed blocks get separate edits", + old: "struct A {\n1: string a\n}\n\nstruct B {\n2: i32 b\n}\n", + new: "struct A { 1: string a }\n\nstruct B { 2: i32 b }\n", + want: []blockEdit{ + {0, len("struct A {\n1: string a\n}\n"), "struct A { 1: string a }\n"}, + {len("struct A {\n1: string a\n}\n\n"), len("struct A {\n1: string a\n}\n\nstruct B {\n2: i32 b\n}\n"), "struct B { 2: i32 b }\n"}, }, - wantOK: false, }, { - // slice is "struct B {\n1: string b" — unclosed, does not parse. - name: "refuses when slice does not parse", - content: "\nstruct B {\n1: string b\n\nstruct C {}", - sel: func(c string) (int, int) { - return strings.Index(c, "struct B"), strings.Index(c, "1: string b") + 3 - }, - wantOK: false, + name: "leading blank lines are dropped", + old: "\n\nstruct A {}\n", + new: "struct A {}\n", + want: []blockEdit{{ + start: 0, + end: strings.Index("\n\nstruct A {}\n", "struct"), + text: "", + }}, }, { - name: "refuses whitespace-only selection", - content: "struct A {}\n\n\nstruct B {}", - sel: func(c string) (int, int) { - return strings.Index(c, "struct B") - 2, strings.Index(c, "struct B") - 1 - }, - wantOK: false, + name: "trailing blank lines are dropped", + old: "struct A {}\n\n\n", + new: "struct A {}\n", + want: []blockEdit{{ + start: len("struct A {}"), + end: len("struct A {}\n\n\n"), + text: "", + }}, }, { - name: "refuses empty selection", - content: "struct A {}", - sel: func(c string) (int, int) { return 5, 5 }, - wantOK: false, + name: "crlf line endings are preserved", + old: "struct A {}\r\n\r\nstruct B {\r\n2: i32 b\r\n}\r\n", + new: "struct A {}\r\n\r\nstruct B { 2: i32 b }\r\n", + want: []blockEdit{{ + start: strings.Index("struct A {}\r\n\r\nstruct B {\r\n2: i32 b\r\n}\r\n", "struct B"), + end: len("struct A {}\r\n\r\nstruct B {\r\n2: i32 b\r\n}\r\n"), + text: "struct B { 2: i32 b }\r\n", + }}, }, { - name: "handles CRLF line endings", - content: "struct A {}\r\n\r\nstruct B {\r\n1: string b\r\n}\r\n\r\nstruct C {}", - sel: func(c string) (int, int) { - return strings.Index(c, "struct B") + 3, strings.Index(c, "struct C") - 4 - }, - want: "struct B { 1: string b }", - wantOK: true, + name: "block structure change falls back to one edit", + old: "struct A {}\n\n\nstruct B {}\n", + new: "struct A {}\n\nstruct B {}\n", + want: []blockEdit{{0, len("struct A {}\n\n\nstruct B {}\n"), "struct A {}\n\nstruct B {}\n"}}, }, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - rs, re := tt.sel(tt.content) - got, _, _, ok := formatRangeText([]byte(tt.content), rs, re, opts) - assert.Equal(t, tt.wantOK, ok) + got := blockDiff([]byte(tt.old), []byte(tt.new)) assert.Equal(t, tt.want, got) }) } } -// TestFormatRangeTextSplice verifies that applying the formatted slice to the -// original content reproduces a whole-document formatting. -func TestFormatRangeTextSplice(t *testing.T) { - opts := formatter.Options{} - content := "struct A { 1: string a }\n\nstruct B { 1: i32 b }\n\nstruct C { 3: i64 c }\n" +// TestBlockDiffApplyAll pins the core invariant: applying every edit +// reproduces the new document. +func TestBlockDiffApplyAll(t *testing.T) { + old := "struct A {\n1: string a\n}\n\nstruct B {\n2: i32 b\n}\n\nstruct C {\n3: i64 c\n}\n" + new := "struct A { 1: string a }\n\nstruct B { 2: i32 b }\n\nstruct C { 3: i64 c }\n" - rs := strings.Index(content, "struct B") - re := rs + len("struct B { ") // inside struct B's line + edits := blockDiff([]byte(old), []byte(new)) - newText, gotRS, gotRE, ok := formatRangeText([]byte(content), rs, re, opts) - if !ok { - t.Fatalf("formatRangeText refused a valid range") + var sb strings.Builder + prev := 0 + for _, e := range edits { + require.GreaterOrEqual(t, e.start, prev, "edits must be ordered and non-overlapping") + sb.WriteString(old[prev:e.start]) + sb.WriteString(e.text) + prev = e.end } + sb.WriteString(old[prev:]) - // The returned range must cover the full struct B line, not the raw - // mid-line selection: it starts at the line start and ends at the - // line's newline. - if gotRS != strings.Index(content, "struct B") { - t.Errorf("effective start = %d, want %d", gotRS, strings.Index(content, "struct B")) - } - - if got := content[gotRE]; got != '\n' { - t.Errorf("effective end = %d, want a newline position (got %q)", gotRE, got) - } + assert.Equal(t, new, sb.String()) +} - spliced := content[:gotRS] + newText + content[gotRE:] +// TestBlockDiffSpliceSafety pins that every edit is bounded by blank lines +// or file edges: splicing it alone never merges with neighboring content. +func TestBlockDiffSpliceSafety(t *testing.T) { + old := "struct A {}\n\nstruct B {\n2: i32 b\n}\n\nstruct C {}\n" + new := "struct A {}\n\nstruct B { 2: i32 b }\n\nstruct C {}\n" - doc, errs := syntax.Parse([]byte(content)) - if len(errs) > 0 { - t.Fatalf("parse errors: %v", errs) - } + edits := blockDiff([]byte(old), []byte(new)) + require.Len(t, edits, 1) - want, err := formatter.Format(doc, opts) - if err != nil { - t.Fatalf("whole-doc format: %v", err) + e := edits[0] + if e.start != 0 { + assert.Equal(t, byte('\n'), old[e.start-1], "edit must start after a blank line") } - if spliced != want { - t.Errorf("splice mismatch\n got: %q\nwant: %q", spliced, want) + if e.end != len(old) { + assert.Equal(t, byte('\n'), old[e.end], "edit must end before a blank line") } } diff --git a/lsp/impl_test.go b/lsp/impl_test.go index b08f540..4ded1a2 100644 --- a/lsp/impl_test.go +++ b/lsp/impl_test.go @@ -569,3 +569,126 @@ func Test_CodeActionFormatDocument(t *testing.T) { }) } } + +// Test_CompletionQualifiedType pins qualified type completion: in a type +// position, typing an include name followed by a dot suggests the +// include's types, qualified. +func Test_CompletionQualifiedType(t *testing.T) { + ctx := t.Context() + + baseParams := &protocol.DidOpenTextDocumentParams{ + TextDocument: protocol.TextDocumentItem{ + URI: "file:///tmp/federation.thrift", + LanguageID: "thrift", + Version: 0, + Text: `struct MobileSuit { + 1: required string Name +} + +struct Guntank { + 1: required i32 Treads +}`, + }, + } + + testContent := `include "federation.thrift" + +struct StrikeRouge { + 1: required federation.MobileSuit pack, + 2: required federation.Guntank support, +}` + testURI := uri.URI("file:///tmp/test.thrift") + + srv := NewServer(cache.New([]string{"/tmp"}), nil, formatter.Options{}) + require.NoError(t, srv.DidOpen(ctx, baseParams)) + require.NoError(t, srv.DidOpen(ctx, &protocol.DidOpenTextDocumentParams{ + TextDocument: protocol.TextDocumentItem{ + URI: testURI, + LanguageID: "thrift", + Version: 0, + Text: testContent, + }, + })) + + completion := func(line, character uint32) []string { + result, err := srv.Completion(ctx, &protocol.CompletionParams{ + TextDocumentPositionParams: protocol.TextDocumentPositionParams{ + TextDocument: protocol.TextDocumentIdentifier{URI: testURI}, + Position: protocol.Position{Line: line, Character: character}, + }, + Context: protocol.CompletionContext{TriggerKind: protocol.CompletionTriggerKindInvoked}, + }) + require.NoError(t, err) + + list, ok := result.(*protocol.CompletionList) + require.True(t, ok) + + labels := make([]string, len(list.Items)) + for i, item := range list.Items { + labels[i] = item.Label + } + + return labels + } + + t.Run("mid-word qualified prefix suggests matching include types", func(t *testing.T) { + // Cursor at "federation.Mo|bileSuit" in the first field type. + labels := completion(3, 27) + + assert.Contains(t, labels, "federation.MobileSuit") + // The prefix filter excludes include types that do not match. + assert.NotContains(t, labels, "federation.Guntank") + // Bare names from other files are not suggested behind a qualifier. + assert.NotContains(t, labels, "MobileSuit") + }) + + t.Run("cursor right after the dot suggests include types", func(t *testing.T) { + // Cursor at "federation.|" in the second field type. + labels := completion(4, 24) + + assert.Contains(t, labels, "federation.Guntank") + assert.Contains(t, labels, "federation.MobileSuit") + }) + + t.Run("no services in a type slot", func(t *testing.T) { + // Cursor in the first field type position, bare prefix: services + // are not valid field types and must not be suggested. + testContent := `include "federation.thrift" + +struct StrikeRouge { + 1: required | +}` + testURI := uri.URI("file:///tmp/test.thrift") + + srv := NewServer(cache.New([]string{"/tmp"}), nil, formatter.Options{}) + require.NoError(t, srv.DidOpen(ctx, baseParams)) + require.NoError(t, srv.DidOpen(ctx, &protocol.DidOpenTextDocumentParams{ + TextDocument: protocol.TextDocumentItem{ + URI: testURI, + LanguageID: "thrift", + Version: 0, + Text: testContent, + }, + })) + + result, err := srv.Completion(ctx, &protocol.CompletionParams{ + TextDocumentPositionParams: protocol.TextDocumentPositionParams{ + TextDocument: protocol.TextDocumentIdentifier{URI: testURI}, + Position: protocol.Position{Line: 3, Character: 15}, + }, + Context: protocol.CompletionContext{TriggerKind: protocol.CompletionTriggerKindInvoked}, + }) + require.NoError(t, err) + + list, ok := result.(*protocol.CompletionList) + require.True(t, ok) + + labels := make([]string, len(list.Items)) + for i, item := range list.Items { + labels[i] = item.Label + } + + assert.Contains(t, labels, "i32") + assert.NotContains(t, labels, "Federation") + }) +} -- 2.51.2