diff --git a/lsp/cache/cache.go b/lsp/cache/cache.go index f63d606..4c71da1 100644 --- a/lsp/cache/cache.go +++ b/lsp/cache/cache.go @@ -1,18 +1,13 @@ package cache import ( - "reflect" "strconv" "sync/atomic" - - "github.com/karitham/thrift-ls/lsp/memoize" ) type Cache struct { id string - store *memoize.Store - IncludePaths []string *memoizedFS @@ -20,16 +15,11 @@ type Cache struct { var cacheIndex int64 -func New(store *memoize.Store, includePaths []string) *Cache { +func New(includePaths []string) *Cache { index := atomic.AddInt64(&cacheIndex, 1) - if store == nil { - store = &memoize.Store{} - } - c := &Cache{ id: strconv.FormatInt(index, 10), - store: store, IncludePaths: includePaths, memoizedFS: &memoizedFS{filesByID: map[FileID][]*DiskFile{}}, } @@ -37,5 +27,4 @@ func New(store *memoize.Store, includePaths []string) *Cache { return c } -func (c *Cache) ID() string { return c.id } -func (c *Cache) MemStats() map[reflect.Type]int { return c.store.Stats() } +func (c *Cache) ID() string { return c.id } diff --git a/lsp/cache/context.go b/lsp/cache/context.go new file mode 100644 index 0000000..68befb9 --- /dev/null +++ b/lsp/cache/context.go @@ -0,0 +1,129 @@ +package cache + +import ( + "slices" + "strings" + + "go.lsp.dev/uri" + + "github.com/karitham/thrift-ls/syntax" +) + +// Context tracks, per file, its transitive include dependencies. It owns the +// IncludeGraph; callers never touch the underlying graph directly. +type Context struct { + graph *IncludeGraph +} + +// NewContext returns an empty context. +func NewContext() *Context { + return &Context{graph: NewIncludeGraph()} +} + +// Register replaces file's include edges, resolving them via resolve the same +// way snapshot parsing does. It returns the URIs whose dependency set +// changed: the union of file's old and new transitive dependents. +// +// Duplicate includes resolve once; unknown include paths fall back to a +// relative path (see resolver.Resolve) and never crash. +func (c *Context) Register(file uri.URI, includes []*syntax.Include, resolve func(uri.URI, string) uri.URI) []uri.URI { + oldDeps := c.Dependents(file) + + c.graph.Set(file, dedupeIncludes(includes), resolve) + + return unionDependents(oldDeps, c.Dependents(file)) +} + +// Dependents returns every file that directly or transitively includes file, +// including file itself when it transitively includes itself. The result is +// sorted ascending by URI and cycle-safe. +func (c *Context) Dependents(file uri.URI) []uri.URI { + deps := make([]uri.URI, 0) + seen := make(map[uri.URI]struct{}) + + var walk func(f uri.URI) + + walk = func(f uri.URI) { + node := c.graph.Get(f) + if node == nil { + return + } + + for _, dependent := range node.InDegree() { + if _, ok := seen[dependent]; ok { + continue + } + + seen[dependent] = struct{}{} + deps = append(deps, dependent) + + walk(dependent) + } + } + + walk(file) + + slices.Sort(deps) + + return deps +} + +// Forget removes file's edges and returns its former dependents. +func (c *Context) Forget(file uri.URI) []uri.URI { + deps := c.Dependents(file) + + c.graph.Remove(file) + + return deps +} + +// Clone returns a deep copy, for snapshot copy-on-write. +func (c *Context) Clone() *Context { + return &Context{graph: c.graph.Clone()} +} + +// dedupeIncludes drops include statements with the same path text, keeping +// the first occurrence. +func dedupeIncludes(includes []*syntax.Include) []*syntax.Include { + deduped := make([]*syntax.Include, 0, len(includes)) + seen := make(map[string]struct{}) + + for _, inc := range includes { + if inc.Path == nil { + continue + } + + text := strings.Trim(inc.Path.Text, "\"'") + if _, ok := seen[text]; ok { + continue + } + + seen[text] = struct{}{} + + deduped = append(deduped, inc) + } + + return deduped +} + +// unionDependents merges two dependent sets, deduped and sorted ascending by +// URI. +func unionDependents(a, b []uri.URI) []uri.URI { + union := make([]uri.URI, 0, len(a)+len(b)) + seen := make(map[uri.URI]struct{}, len(a)+len(b)) + + for _, deps := range [][]uri.URI{a, b} { + for _, dep := range deps { + if _, ok := seen[dep]; ok { + continue + } + + seen[dep] = struct{}{} + union = append(union, dep) + } + } + + slices.Sort(union) + + return union +} diff --git a/lsp/cache/context_test.go b/lsp/cache/context_test.go new file mode 100644 index 0000000..1a5da94 --- /dev/null +++ b/lsp/cache/context_test.go @@ -0,0 +1,224 @@ +package cache + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "go.lsp.dev/uri" + + "github.com/karitham/thrift-ls/syntax" +) + +// gundam-themed URIs, fixed for deterministic sort order across tests: +// char < federation.gundam < mobile_suit.zeon < strike_rouge. +const ( + charURI = "file:///tmp/char.thrift" + federationURI = "file:///tmp/federation.gundam.thrift" + mobileSuitURI = "file:///tmp/mobile_suit.zeon.thrift" + strikeRougeURI = "file:///tmp/strike_rouge.thrift" +) + +// buildTestContext registers the given include edges, where the map value is +// the list of files the key file includes, and returns the context. +func buildTestContext(t *testing.T, edges map[string][]string) *Context { + t.Helper() + + c := NewContext() + + for file, includes := range edges { + inc := make([]*syntax.Include, 0, len(includes)) + for _, includePath := range includes { + inc = append(inc, &syntax.Include{Path: &syntax.Token{Text: includePath}}) + } + + c.Register(uri.URI(file), inc, resolveTestInclude) + } + + return c +} + +// resolveTestInclude resolves an include path relative to the including +// file's directory, mirroring the snapshot resolver's relative fallback. +func resolveTestInclude(cur uri.URI, includePath string) uri.URI { + return uri.File(filepath.Join(filepath.Dir(cur.Path()), includePath)) +} + +func Test_Context_Dependents(t *testing.T) { + for _, tt := range []struct { + name string + edges map[string][]string + file string + want []uri.URI + }{ + { + name: "linear chain", + edges: map[string][]string{ + strikeRougeURI: {"federation.gundam.thrift"}, + federationURI: {"mobile_suit.zeon.thrift"}, + }, + file: mobileSuitURI, + want: []uri.URI{federationURI, strikeRougeURI}, + }, + { + name: "diamond", + edges: map[string][]string{ + strikeRougeURI: {"federation.gundam.thrift", "mobile_suit.zeon.thrift"}, + federationURI: {"char.thrift"}, + mobileSuitURI: {"char.thrift"}, + }, + file: charURI, + want: []uri.URI{federationURI, mobileSuitURI, strikeRougeURI}, + }, + { + name: "three cycle terminates", + edges: map[string][]string{ + strikeRougeURI: {"federation.gundam.thrift"}, + federationURI: {"mobile_suit.zeon.thrift"}, + mobileSuitURI: {"strike_rouge.thrift"}, + }, + file: strikeRougeURI, + want: []uri.URI{federationURI, mobileSuitURI, strikeRougeURI}, + }, + { + name: "self include terminates", + edges: map[string][]string{ + "file:///tmp/side_effect.thrift": {"side_effect.thrift"}, + }, + file: "file:///tmp/side_effect.thrift", + want: []uri.URI{"file:///tmp/side_effect.thrift"}, + }, + { + name: "file with no includers", + edges: map[string][]string{ + strikeRougeURI: {"federation.gundam.thrift"}, + }, + file: strikeRougeURI, + want: []uri.URI{}, + }, + } { + t.Run(tt.name, func(t *testing.T) { + c := buildTestContext(t, tt.edges) + + got := c.Dependents(uri.URI(tt.file)) + assert.Equal(t, tt.want, got) + }) + } +} + +func Test_Context_RegisterChangedIncludes(t *testing.T) { + for _, tt := range []struct { + name string + edges map[string][]string + file string + registerWith []string + wantAffected []uri.URI + wantDependents []uri.URI + }{ + { + // A->B->A cycle: dropping A's edge to B also drops A from the + // dependent set (A only reached A through its own include). + // The return value is the union of old and new dependents. + name: "cycle re-register returns union of old and new dependents", + edges: map[string][]string{ + strikeRougeURI: {"federation.gundam.thrift"}, + federationURI: {"strike_rouge.thrift"}, + }, + file: strikeRougeURI, + registerWith: []string{}, + wantAffected: []uri.URI{federationURI, strikeRougeURI}, + wantDependents: []uri.URI{federationURI}, + }, + { + // Nothing includes A, so neither A's old nor new dependents are + // affected; the changed file itself is the caller's concern. + name: "re-register with no includers affects no dependents", + edges: map[string][]string{ + strikeRougeURI: {"federation.gundam.thrift"}, + }, + file: strikeRougeURI, + registerWith: []string{"mobile_suit.zeon.thrift"}, + wantAffected: []uri.URI{}, + wantDependents: []uri.URI{}, + }, + { + name: "re-register unchanged edges returns current dependents", + edges: map[string][]string{ + strikeRougeURI: {"federation.gundam.thrift"}, + }, + file: federationURI, + registerWith: []string{}, + wantAffected: []uri.URI{strikeRougeURI}, + wantDependents: []uri.URI{strikeRougeURI}, + }, + } { + t.Run(tt.name, func(t *testing.T) { + c := buildTestContext(t, tt.edges) + + inc := make([]*syntax.Include, 0, len(tt.registerWith)) + for _, includePath := range tt.registerWith { + inc = append(inc, &syntax.Include{Path: &syntax.Token{Text: includePath}}) + } + + gotAffected := c.Register(uri.URI(tt.file), inc, resolveTestInclude) + + assert.Equal(t, tt.wantAffected, gotAffected) + assert.Equal(t, tt.wantDependents, c.Dependents(uri.URI(tt.file))) + }) + } +} + +func Test_Context_Forget(t *testing.T) { + c := buildTestContext(t, map[string][]string{ + strikeRougeURI: {"federation.gundam.thrift"}, + federationURI: {"mobile_suit.zeon.thrift"}, + }) + + got := c.Forget(uri.URI(federationURI)) + + assert.Equal(t, []uri.URI{strikeRougeURI}, got) + + // B's edges are gone: nothing is reachable through it any more. + node := c.graph.Get(uri.URI(federationURI)) + assert.NotNil(t, node) + assert.Empty(t, node.OutDegree()) + + // A still includes B, so B keeps its dependents. + assert.Equal(t, []uri.URI{strikeRougeURI}, c.Dependents(uri.URI(federationURI))) + // C's includer chain is gone until B is re-parsed. + assert.Empty(t, c.Dependents(uri.URI(mobileSuitURI))) +} + +func Test_Context_DuplicateIncludes(t *testing.T) { + c := NewContext() + + duplicates := []*syntax.Include{ + {Path: &syntax.Token{Text: "federation.gundam.thrift"}}, + {Path: &syntax.Token{Text: "federation.gundam.thrift"}}, + } + + c.Register(uri.URI(strikeRougeURI), duplicates, resolveTestInclude) + + node := c.graph.Get(uri.URI(strikeRougeURI)) + assert.NotNil(t, node) + assert.Equal(t, []uri.URI{federationURI}, node.OutDegree()) + + // the include target records exactly one dependent + assert.Equal(t, []uri.URI{strikeRougeURI}, c.Dependents(uri.URI(federationURI))) +} + +func Test_Context_UnknownInclude(t *testing.T) { + c := NewContext() + + // A resolve func returning an unresolvable URI must not crash Register; + // the unknown target still records its dependent. + unknown := &syntax.Include{Path: &syntax.Token{Text: "missing/nonexistent.thrift"}} + unknownURI := uri.File(filepath.Join("/tmp", "missing", "nonexistent.thrift")) + + c.Register(uri.URI(strikeRougeURI), []*syntax.Include{unknown}, func(uri.URI, string) uri.URI { + return unknownURI + }) + + assert.Equal(t, []uri.URI{uri.URI(strikeRougeURI)}, c.Dependents(unknownURI)) + assert.Empty(t, c.Dependents(uri.URI(strikeRougeURI))) +} diff --git a/lsp/cache/cow_test.go b/lsp/cache/cow_test.go new file mode 100644 index 0000000..6b10f28 --- /dev/null +++ b/lsp/cache/cow_test.go @@ -0,0 +1,83 @@ +package cache + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "go.lsp.dev/uri" +) + +// gundamSnapshotFiles is a two-file corpus: strike_rouge includes +// federation.gundam. +func gundamSnapshotFiles() []*FileChange { + return []*FileChange{ + { + URI: uri.URI("file:///tmp/strike_rouge.thrift"), + Content: []byte(`include "federation.gundam.thrift" + +exception BayFull { + 1: string message +}`), + From: FileChangeTypeDidOpen, + }, + { + URI: uri.URI("file:///tmp/federation.gundam.thrift"), + Content: []byte(`struct Gundam { + 1: required string Name +}`), + From: FileChangeTypeDidOpen, + }, + } +} + +// TestSnapshotCloneIsolation: snapshot clones share maps copy-on-write, so +// mutating a clone must never leak into the snapshot it was cloned from. +func TestSnapshotCloneIsolation(t *testing.T) { + ss := BuildSnapshotForTest(gundamSnapshotFiles()) + + clone, release := ss.clone() + defer release() + + // Mutate the clone: replace the file's content and forget its caches. + clone.files.Set("file:///tmp/federation.gundam.thrift", NewOverlay( + "file:///tmp/federation.gundam.thrift", + []byte("struct Gundam {\n\t1: required string Name,\n\t2: optional i32 SerialNumber\n}"), + 2, + )) + clone.parsedCache.Forget("file:///tmp/federation.gundam.thrift") + clone.context.Forget("file:///tmp/federation.gundam.thrift") + + // The clone sees the new content; the original keeps the old. + clonePf, err := clone.Parse(t.Context(), "file:///tmp/federation.gundam.thrift") + assert.NoError(t, err) + assert.Len(t, clonePf.AST().Structs()[0].Fields, 2, "clone parses the new content") + + origPf := ss.parsedCache.Get("file:///tmp/federation.gundam.thrift") + assert.NotNil(t, origPf, "original snapshot keeps its parsed file") + assert.Len(t, origPf.AST().Structs()[0].Fields, 1, "original snapshot is unaffected by clone writes") + + // The original snapshot's graph is untouched by the clone's Forget. + assert.Equal(t, []uri.URI{"file:///tmp/strike_rouge.thrift"}, ss.Dependents("file:///tmp/federation.gundam.thrift")) +} + +// BenchmarkSnapshotClone shows that cloning a snapshot with many parsed +// files is O(1): the maps are shared copy-on-write. +func BenchmarkSnapshotClone(b *testing.B) { + files := make([]*FileChange, 0, 100) + for i := 0; i < 100; i++ { + files = append(files, &FileChange{ + URI: uri.URI(fmt.Sprintf("file:///tmp/bench%d.thrift", i)), + Content: []byte("struct Gundam {\n\t1: required string Name\n}"), + From: FileChangeTypeDidOpen, + }) + } + + ss := BuildSnapshotForTest(files) + + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, release := ss.clone() + release() + } +} diff --git a/lsp/cache/file.go b/lsp/cache/file.go index da9d2b8..99f752d 100644 --- a/lsp/cache/file.go +++ b/lsp/cache/file.go @@ -102,11 +102,14 @@ type FileSource interface { ReadFile(ctx context.Context, uri uri.URI) (FileHandle, error) } -// FilesMap holds files on disk and overlay files +// FilesMap holds files on disk and overlay files. Snapshots share the +// underlying maps (Clone is O(1)); the first write after a clone copies +// copy-on-write. type FilesMap struct { mu sync.RWMutex files map[uri.URI]FileHandle overlays map[uri.URI]*Overlay + shared bool } func (m *FilesMap) Get(key uri.URI) (FileHandle, bool) { @@ -122,6 +125,8 @@ func (m *FilesMap) Set(key uri.URI, file FileHandle) { m.mu.Lock() defer m.mu.Unlock() + m.copyOnWrite() + m.files[key] = file if o, ok := file.(*Overlay); ok { m.overlays[key] = o @@ -132,27 +137,47 @@ func (m *FilesMap) Forget(key uri.URI) { m.mu.Lock() defer m.mu.Unlock() + m.copyOnWrite() + delete(m.files, key) delete(m.overlays, key) } +// Clone returns a view sharing the same entries. The clone and the original +// both become copy-on-write. func (m *FilesMap) Clone() *FilesMap { - m.mu.RLock() - defer m.mu.RUnlock() + m.mu.Lock() + defer m.mu.Unlock() - newMap := &FilesMap{ - files: make(map[uri.URI]FileHandle), - overlays: make(map[uri.URI]*Overlay), + m.shared = true + + return &FilesMap{ + files: m.files, + overlays: m.overlays, + shared: true, } - for key := range m.files { - newMap.files[key] = m.files[key] +} + +// copyOnWrite detaches the maps from a shared parent before the first write. +// Callers must hold mu. +func (m *FilesMap) copyOnWrite() { + if !m.shared { + return + } + + files := make(map[uri.URI]FileHandle, len(m.files)+1) + for k, v := range m.files { + files[k] = v } - for key := range m.overlays { - newMap.overlays[key] = m.overlays[key] + overlays := make(map[uri.URI]*Overlay, len(m.overlays)+1) + for k, v := range m.overlays { + overlays[k] = v } - return newMap + m.files = files + m.overlays = overlays + m.shared = false } func (m *FilesMap) Destroy() { @@ -167,6 +192,7 @@ const ( FileChangeTypeDidOpen FileChangeType = "DidOpen" FileChangeTypeDidChange FileChangeType = "DidChange" FileChangeTypeDidSave FileChangeType = "DidSave" + FileChangeTypeDidClose FileChangeType = "DidClose" ) type FileChange struct { diff --git a/lsp/cache/fs_overlay.go b/lsp/cache/fs_overlay.go index 1c84a4e..fad216a 100644 --- a/lsp/cache/fs_overlay.go +++ b/lsp/cache/fs_overlay.go @@ -50,9 +50,16 @@ func (fs *overlayFS) ReadFile(ctx context.Context, uri uri.URI) (FileHandle, err return fs.delegate.ReadFile(ctx, uri) } -// Update only updates overlays +// Update applies changes to the overlay set. DidClose changes remove the +// overlay; all other types create or replace it. func (fs *overlayFS) Update(ctx context.Context, changes []*FileChange) error { for _, change := range changes { + if change.From == FileChangeTypeDidClose { + fs.Forget(change.URI) + + continue + } + var base []byte if change.From == FileChangeTypeDidChange { @@ -79,6 +86,24 @@ func (fs *overlayFS) Update(ctx context.Context, changes []*FileChange) error { return nil } +// HasOverlay reports whether uri has an open overlay. +func (fs *overlayFS) HasOverlay(uri uri.URI) bool { + fs.mu.Lock() + defer fs.mu.Unlock() + + _, ok := fs.overlays[uri] + + return ok +} + +// Forget drops the overlay for uri, falling back to disk content. +func (fs *overlayFS) Forget(uri uri.URI) { + fs.mu.Lock() + defer fs.mu.Unlock() + + delete(fs.overlays, uri) +} + // An Overlay is a file open in the editor. It may have unsaved edits. // It implements the source.FileHandle interface. type Overlay struct { diff --git a/lsp/cache/graph.go b/lsp/cache/graph.go index 9f6cea7..5a65038 100644 --- a/lsp/cache/graph.go +++ b/lsp/cache/graph.go @@ -40,9 +40,14 @@ func (n *IncludeNode) OutDegree() []uri.URI { return n.outdegree } +// IncludeGraph tracks include edges between files. Snapshots share the +// underlying mapper (Clone is O(1)); the first structural change after a +// clone deep-copies the graph, so edits that do not change include edges +// never pay for the copy. type IncludeGraph struct { mu sync.RWMutex mapper map[uri.URI]*IncludeNode + shared bool } func NewIncludeGraph() *IncludeGraph { @@ -78,36 +83,32 @@ func (g *IncludeGraph) Set(file uri.URI, includes []*syntax.Include, resolve fun return includeURIs[i] < includeURIs[j] }) - node, ok := g.mapper[file] - if ok { - if len(includeURIs) == len(node.outdegree) { - sort.SliceStable(node.outdegree, func(i, j int) bool { - return node.outdegree[i] < node.outdegree[j] - }) - - equal := true - - for i := range includeURIs { - if includeURIs[i] != node.outdegree[i] { - equal = false - - break - } - } + // Unchanged edges: nothing to write, so a shared snapshot graph is + // left untouched (no copy). + if node, ok := g.mapper[file]; ok && sameOutdegree(node, includeURIs) { + return + } - if equal { - return - } - } + g.detach() + g.removeWithoutLock(file) - g.removeWithoutLock(file) - } else { + node := g.mapper[file] + if node == nil { node = &IncludeNode{} } for _, inc := range includeURIs { node.outdegree = append(node.outdegree, inc) + if inc == file { + // Self-include: the target node is this node. Appending to a + // fresh node would be overwritten by g.mapper[file] below and + // the edge lost. + node.indegree = append(node.indegree, file) + + continue + } + outNode, exist := g.mapper[inc] if !exist { outNode = &IncludeNode{} @@ -120,23 +121,62 @@ func (g *IncludeGraph) Set(file uri.URI, includes []*syntax.Include, resolve fun g.mapper[file] = node } +// sameOutdegree reports whether the node's outdegree equals includeURIs. +// The node's slice is not modified. +func sameOutdegree(node *IncludeNode, includeURIs []uri.URI) bool { + if len(node.outdegree) != len(includeURIs) { + return false + } + + out := make([]uri.URI, len(node.outdegree)) + copy(out, node.outdegree) + sort.SliceStable(out, func(i, j int) bool { + return out[i] < out[j] + }) + + for i := range includeURIs { + if out[i] != includeURIs[i] { + return false + } + } + + return true +} + func (g *IncludeGraph) Remove(file uri.URI) { g.mu.Lock() defer g.mu.Unlock() + g.detach() g.removeWithoutLock(file) } +// Clone returns a view sharing the same mapper. The clone and the original +// both become copy-on-write: the next structural change deep-copies. func (g *IncludeGraph) Clone() *IncludeGraph { - g.mu.RLock() - defer g.mu.RUnlock() + g.mu.Lock() + defer g.mu.Unlock() + + g.shared = true + + return &IncludeGraph{mapper: g.mapper, shared: true} +} - newG := NewIncludeGraph() - for i := range g.mapper { - newG.mapper[i] = g.mapper[i].Clone() +// detach deep-copies the mapper before the first structural change after a +// clone, so the shared parent snapshot is never mutated. Callers must hold +// mu. +func (g *IncludeGraph) detach() { + if !g.shared { + return + } + + mapper := make(map[uri.URI]*IncludeNode, len(g.mapper)+1) + for file, node := range g.mapper { + mapper[file] = node.Clone() } - return newG + g.mapper = mapper + g.shared = false } func (g *IncludeGraph) removeWithoutLock(file uri.URI) { diff --git a/lsp/cache/graph_test.go b/lsp/cache/graph_test.go index 31ce238..6f5d1ea 100644 --- a/lsp/cache/graph_test.go +++ b/lsp/cache/graph_test.go @@ -149,3 +149,34 @@ func resolveWithPaths(includePaths []string) func(uri.URI, string) uri.URI { return uri.File(r.Resolve(cur.Path(), includePath)) } } + +// Test_SnapshotParseIncludeCycles exercises include cycles through the full +// snapshot parse path: parsing registers edges via Context.Register, and the +// graph must settle without infinite recursion. +func Test_SnapshotParseIncludeCycles(t *testing.T) { + dir := t.TempDir() + char := uri.File(filepath.Join(dir, "char.thrift")) + amuro := uri.File(filepath.Join(dir, "amuro.thrift")) + self := uri.File(filepath.Join(dir, "side_effect.thrift")) + + files := []*FileChange{ + {URI: char, Content: []byte(`include "amuro.thrift"`), From: FileChangeTypeDidOpen}, + {URI: amuro, Content: []byte(`include "char.thrift"`), From: FileChangeTypeDidOpen}, + {URI: self, Content: []byte(`include "side_effect.thrift"`), From: FileChangeTypeDidOpen}, + } + + ss := BuildSnapshotForTest(files) + + // both directions of the mutual cycle are recorded + node := ss.Graph().Get(char) + assert.NotNil(t, node) + assert.Equal(t, []uri.URI{amuro}, node.OutDegree()) + assert.Equal(t, []uri.URI{amuro}, node.InDegree()) + + // dependents terminate on the cycle and include both files + assert.Equal(t, []uri.URI{amuro, char}, ss.Dependents(char)) + assert.Equal(t, []uri.URI{amuro, char}, ss.Dependents(amuro)) + + // self-include: the file is its own dependent, and settles + assert.Equal(t, []uri.URI{self}, ss.Dependents(self)) +} diff --git a/lsp/cache/invalidation_test.go b/lsp/cache/invalidation_test.go new file mode 100644 index 0000000..ba76cb7 --- /dev/null +++ b/lsp/cache/invalidation_test.go @@ -0,0 +1,208 @@ +package cache + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "go.lsp.dev/uri" +) + +// The gundam test corpus: strike_rouge includes federation.gundam, which +// includes mobile_suit.zeon; char is standalone. +const ( + strikeRouge = "file:///tmp/strike_rouge.thrift" + federation = "file:///tmp/federation.gundam.thrift" + mobileSuit = "file:///tmp/mobile_suit.zeon.thrift" + char = "file:///tmp/char.thrift" +) + +func gundamFiles() []*FileChange { + return []*FileChange{ + { + URI: uri.URI(strikeRouge), + Content: []byte(`include "federation.gundam.thrift" + +exception BayFull { + 1: string message +}`), + From: FileChangeTypeDidOpen, + }, + { + URI: uri.URI(federation), + Content: []byte(`include "mobile_suit.zeon.thrift" + +struct Gundam { + 1: required string Name +}`), + From: FileChangeTypeDidOpen, + }, + { + URI: uri.URI(mobileSuit), + Content: []byte(`enum ZeonForces { + ZAKU_I, + ZAKU_II, + GELGOOG +}`), + From: FileChangeTypeDidOpen, + }, + } +} + +// viewHarness mirrors the server's open-then-change flow: overlays updated +// first, then routed through View.FileChange. +type viewHarness struct { + view *View + fs *overlayFS +} + +func newViewHarness(t *testing.T, files []*FileChange) *viewHarness { + t.Helper() + + c := New(nil) + fs := NewOverlayFS(c) + + if err := fs.Update(context.Background(), files); err != nil { + t.Fatal(err) + } + + view := NewView("test", "file:///tmp", fs, nil) + + ss, release := view.Snapshot() + defer release() + + for _, f := range files { + if _, err := ss.Parse(context.Background(), f.URI); err != nil { + t.Fatal(err) + } + } + + return &viewHarness{view: view, fs: fs} +} + +// change applies a change like the server's didChange: overlay first, then +// FileChange, returning the affected URIs passed to the postFn once the +// asynchronous postFn has run. +func (h *viewHarness) change(t *testing.T, change *FileChange) []uri.URI { + t.Helper() + + if err := h.fs.Update(context.Background(), []*FileChange{change}); err != nil { + t.Fatal(err) + } + + done := make(chan []uri.URI, 1) + + h.view.FileChange(context.Background(), []*FileChange{change}, func(a []uri.URI) { + done <- a + }) + + select { + case affected := <-done: + return affected + case <-time.After(5 * time.Second): + t.Fatal("postFn did not run") + + return nil + } +} + +func (h *viewHarness) snapshot(t *testing.T) *Snapshot { + t.Helper() + + ss, release := h.view.Snapshot() + t.Cleanup(release) + + return ss +} + +func Test_FileChangeInvalidatesDependents(t *testing.T) { + for _, tt := range []struct { + name string + files []*FileChange + change *FileChange + wantAffected []uri.URI + wantDropped []uri.URI + wantKept []uri.URI + }{ + { + name: "change mid-chain invalidates transitive dependents", + files: gundamFiles(), + change: &FileChange{ + URI: uri.URI(federation), + Version: 1, + Content: []byte(`include "mobile_suit.zeon.thrift" + +struct Gundam { + 1: required string Name, + 2: optional i32 SerialNumber +}`), + From: FileChangeTypeDidChange, + }, + wantAffected: []uri.URI{federation, strikeRouge}, + wantDropped: []uri.URI{strikeRouge}, + wantKept: []uri.URI{federation, mobileSuit}, + }, + { + name: "change leaf invalidates whole chain", + files: gundamFiles(), + change: &FileChange{ + URI: uri.URI(mobileSuit), + Version: 1, + Content: []byte(`enum ZeonForces { + ZAKU_I, + ZAKU_II, + GELGOOG, + CHARS_ZAKU +}`), + From: FileChangeTypeDidChange, + }, + wantAffected: []uri.URI{mobileSuit, federation, strikeRouge}, + wantDropped: []uri.URI{federation, strikeRouge}, + wantKept: []uri.URI{mobileSuit}, + }, + { + name: "change file with no dependents", + files: []*FileChange{ + { + URI: uri.URI(char), + Content: []byte(`struct Char { + 1: optional string Title +}`), + From: FileChangeTypeDidOpen, + }, + }, + change: &FileChange{ + URI: uri.URI(char), + Version: 1, + Content: []byte(`struct Char { + 1: optional string Title, + 2: optional bool Newtype +}`), + From: FileChangeTypeDidChange, + }, + wantAffected: []uri.URI{char}, + wantDropped: nil, + wantKept: []uri.URI{char}, + }, + } { + t.Run(tt.name, func(t *testing.T) { + h := newViewHarness(t, tt.files) + + gotAffected := h.change(t, tt.change) + assert.Equal(t, tt.wantAffected, gotAffected) + + ss := h.snapshot(t) + for _, file := range tt.wantDropped { + assert.Nil(t, ss.parsedCache.Get(file), "parse cache for %s should be dropped", file) + + _, ok := ss.files.Get(file) + assert.False(t, ok, "file handle for %s should be dropped", file) + } + + for _, file := range tt.wantKept { + assert.NotNil(t, ss.parsedCache.Get(file), "parse cache for %s should survive", file) + } + }) + } +} diff --git a/lsp/cache/parse.go b/lsp/cache/parse.go index 22e9224..25251ac 100644 --- a/lsp/cache/parse.go +++ b/lsp/cache/parse.go @@ -12,10 +12,14 @@ import ( "github.com/karitham/thrift-ls/syntax" ) +// ParseCaches maps URIs to parsed files. Snapshots share the underlying map +// (Clone is O(1)); the first write after a clone copies the map +// copy-on-write, so cloning per keystroke is cheap while old snapshots stay +// immutable. type ParseCaches struct { mu sync.RWMutex caches map[uri.URI]*ParsedFile - tokens map[string]struct{} + shared bool } func NewParseCaches() *ParseCaches { @@ -26,9 +30,11 @@ func NewParseCaches() *ParseCaches { func (c *ParseCaches) Set(filePath uri.URI, res *ParsedFile) { c.mu.Lock() + defer c.mu.Unlock() + + c.copyOnWrite() + c.caches[filePath] = res - c.tokens = nil - c.mu.Unlock() } func (c *ParseCaches) Get(filePath uri.URI) *ParsedFile { @@ -42,44 +48,41 @@ func (c *ParseCaches) Forget(filePath uri.URI) { c.mu.Lock() defer c.mu.Unlock() + c.copyOnWrite() + delete(c.caches, filePath) - c.tokens = nil } +// Clone returns a view sharing the same entries. The clone and the original +// both become copy-on-write. func (c *ParseCaches) Clone() *ParseCaches { - c.mu.RLock() - defer c.mu.RUnlock() + c.mu.Lock() + defer c.mu.Unlock() - clone := make(map[uri.URI]*ParsedFile) - for i := range c.caches { - clone[i] = c.caches[i] - } + c.shared = true - return &ParseCaches{caches: clone} + return &ParseCaches{caches: c.caches, shared: true} } -func (c *ParseCaches) Tokens() map[string]struct{} { - if len(c.tokens) > 0 { - return c.tokens +// copyOnWrite detaches caches from a shared parent before the first write. +// Callers must hold mu. +func (c *ParseCaches) copyOnWrite() { + if !c.shared { + return } - tokens := make(map[string]struct{}) - - for _, parsed := range c.caches { - if parsed.ast == nil { - continue - } - - collectTokens(parsed.ast, tokens) + caches := make(map[uri.URI]*ParsedFile, len(c.caches)+1) + for k, v := range c.caches { + caches[k] = v } - c.tokens = tokens - - return tokens + c.caches = caches + c.shared = false } // TokensForFile returns tokens for the given file and its transitively -// included files. +// included files. Each file's token set is computed once per parse and +// reused, so typing does not re-walk the include closure's ASTs. func (c *ParseCaches) TokensForFile(file uri.URI, getIncludes func(uri.URI) []uri.URI) map[string]struct{} { tokens := make(map[string]struct{}) visited := make(map[uri.URI]bool) @@ -93,9 +96,10 @@ func (c *ParseCaches) TokensForFile(file uri.URI, getIncludes func(uri.URI) []ur visited[f] = true - pf := c.Get(f) - if pf != nil && pf.ast != nil { - collectTokens(pf.ast, tokens) + if pf := c.Get(f); pf != nil { + for token := range pf.Tokens() { + tokens[token] = struct{}{} + } } for _, inc := range getIncludes(f) { @@ -209,6 +213,9 @@ type ParsedFile struct { // errs hold all ast parsing errors errs []syntax.Error + + // tokens is the identifier set of ast, computed lazily once per parse. + tokens map[string]struct{} } func (p *ParsedFile) Mapper() *mapper.Mapper { @@ -223,6 +230,24 @@ func (p *ParsedFile) Errors() []syntax.Error { return p.errs } +// Tokens returns the identifier tokens of the file, computed once and +// reused. A re-parse replaces the whole ParsedFile, so the cache never +// goes stale. +func (p *ParsedFile) Tokens() map[string]struct{} { + if p.tokens != nil { + return p.tokens + } + + tokens := make(map[string]struct{}) + if p.ast != nil { + collectTokens(p.ast, tokens) + } + + p.tokens = tokens + + return tokens +} + func (p *ParsedFile) AggregatedError() error { if len(p.errs) == 0 { return nil diff --git a/lsp/cache/resolver_test.go b/lsp/cache/resolver_test.go index 93ca719..364805a 100644 --- a/lsp/cache/resolver_test.go +++ b/lsp/cache/resolver_test.go @@ -8,7 +8,6 @@ import ( "github.com/stretchr/testify/assert" "go.lsp.dev/uri" - "github.com/karitham/thrift-ls/lsp/memoize" "github.com/karitham/thrift-ls/syntax" ) @@ -29,13 +28,12 @@ func TestResolver(t *testing.T) { err = os.WriteFile(sharedThrift, []byte(""), 0o644) assert.NoError(t, err) - store := &memoize.Store{} - c := New(store, nil) + c := New(nil) fs := NewOverlayFS(c) - view := NewView("test", uri.File(tmpDir), fs, store, nil) + view := NewView("test", uri.File(tmpDir), fs, nil) includePaths := []string{sharedDir} - ss := NewSnapshot(view, store, includePaths) + ss := NewSnapshot(view, includePaths) resolver := ss.Resolver() diff --git a/lsp/cache/session.go b/lsp/cache/session.go index cc9e414..c74e310 100644 --- a/lsp/cache/session.go +++ b/lsp/cache/session.go @@ -55,7 +55,7 @@ func (s *Session) Initialize(fn func()) { } func (s *Session) CreateView(folder uri.URI) { - view := NewView(folder.Path(), folder, s.overlayFS, s.cache.store, s.cache.IncludePaths) + view := NewView(folder.Path(), folder, s.overlayFS, s.cache.IncludePaths) s.views = append(s.views, view) } diff --git a/lsp/cache/snapshot.go b/lsp/cache/snapshot.go index b0215f6..c1809f6 100644 --- a/lsp/cache/snapshot.go +++ b/lsp/cache/snapshot.go @@ -13,7 +13,6 @@ import ( "go.lsp.dev/uri" "github.com/karitham/thrift-ls/lsp/lsputils" - "github.com/karitham/thrift-ls/lsp/memoize" "github.com/karitham/thrift-ls/resolver" "github.com/karitham/thrift-ls/syntax" ) @@ -158,22 +157,20 @@ type Snapshot struct { files *FilesMap - store *memoize.Store - - graph *IncludeGraph + context *Context parsedCache *ParseCaches includePaths []string } -func NewSnapshot(view *View, store *memoize.Store, includePaths []string) *Snapshot { +func NewSnapshot(view *View, includePaths []string) *Snapshot { snapshot := &Snapshot{ - id: rand.Int63(), - view: view, - store: store, + id: rand.Int63(), + view: view, + ctx: context.Background(), refCount: sync.WaitGroup{}, - graph: NewIncludeGraph(), + context: NewContext(), parsedCache: NewParseCaches(), files: &FilesMap{ files: make(map[uri.URI]FileHandle), @@ -195,7 +192,13 @@ func (s *Snapshot) Initialize(ctx context.Context) { } func (s *Snapshot) Graph() *IncludeGraph { - return s.graph + return s.context.graph +} + +// Dependents returns the transitive dependents of uri: every file that +// directly or transitively includes it, in this snapshot. +func (s *Snapshot) Dependents(uri uri.URI) []uri.URI { + return s.context.Dependents(uri) } // Resolver returns a new Resolver instance for this snapshot. @@ -224,12 +227,18 @@ func (s *Snapshot) ReadFile(ctx context.Context, uri uri.URI) (FileHandle, error return fh, nil } -// ForgetFile is called when file changed or removed -// it remove file cache and parsed cache +// ForgetFile is called when file changed or removed. It removes file's +// include edges and drops the parse and file caches for file and every +// transitive dependent of file: their derived data is rebuilt lazily on the +// next request, while their content survives on disk (or in the overlay). func (s *Snapshot) ForgetFile(uri uri.URI) { s.files.Forget(uri) - s.graph.Remove(uri) s.parsedCache.Forget(uri) + + for _, dependent := range s.context.Forget(uri) { + s.files.Forget(dependent) + s.parsedCache.Forget(dependent) + } } func (s *Snapshot) Parse(ctx context.Context, uri uri.URI) (*ParsedFile, error) { @@ -254,7 +263,7 @@ func (s *Snapshot) Parse(ctx context.Context, uri uri.URI) (*ParsedFile, error) } if pf.AST() != nil { - s.graph.Set(uri, pf.AST().Includes(), s.Resolver().ResolveInclude) + s.context.Register(uri, pf.AST().Includes(), s.Resolver().ResolveInclude) } s.parsedCache.Set(uri, pf) @@ -262,13 +271,11 @@ func (s *Snapshot) Parse(ctx context.Context, uri uri.URI) (*ParsedFile, error) return pf, nil } -func (s *Snapshot) Tokens() map[string]struct{} { - return s.parsedCache.Tokens() -} - +// TokensForFile returns the identifier tokens of file and its transitively +// included files. func (s *Snapshot) TokensForFile(file uri.URI) map[string]struct{} { return s.parsedCache.TokensForFile(file, func(f uri.URI) []uri.URI { - node := s.graph.Get(f) + node := s.context.graph.Get(f) if node == nil { return nil } @@ -288,7 +295,7 @@ func (s *Snapshot) clone() (*Snapshot, func()) { // files: make(map[uri.URI]FileHandle), // overlays: make(map[uri.URI]*Overlay), // }, - graph: s.graph.Clone(), + context: s.context.Clone(), parsedCache: s.parsedCache.Clone(), includePaths: s.includePaths, } @@ -303,13 +310,12 @@ func BuildSnapshotForTest(files []*FileChange) *Snapshot { // BuildSnapshotForTestWithPaths is BuildSnapshotForTest with configured // include paths, for cross-project include resolution tests. func BuildSnapshotForTestWithPaths(includePaths []string, files []*FileChange) *Snapshot { - store := &memoize.Store{} - c := New(store, includePaths) + c := New(includePaths) fs := NewOverlayFS(c) _ = fs.Update(context.TODO(), files) - view := NewView("test", "file:///tmp", fs, store, includePaths) - ss := NewSnapshot(view, store, includePaths) + view := NewView("test", "file:///tmp", fs, includePaths) + ss := NewSnapshot(view, includePaths) for _, f := range files { _, _ = ss.Parse(context.TODO(), f.URI) diff --git a/lsp/cache/view.go b/lsp/cache/view.go index 97fbc56..cc5d3b7 100644 --- a/lsp/cache/view.go +++ b/lsp/cache/view.go @@ -4,12 +4,11 @@ import ( "context" "log/slog" "math/rand" + "sort" "strings" "sync" "go.lsp.dev/uri" - - "github.com/karitham/thrift-ls/lsp/memoize" ) type View struct { @@ -42,7 +41,7 @@ type View struct { snapshotRelease func() } -func NewView(name string, folder uri.URI, fs FileSource, store *memoize.Store, includePaths []string) *View { +func NewView(name string, folder uri.URI, fs FileSource, includePaths []string) *View { view := &View{ id: rand.Int63(), name: name, @@ -52,7 +51,7 @@ func NewView(name string, folder uri.URI, fs FileSource, store *memoize.Store, i includePaths: includePaths, } - view.snapshot = NewSnapshot(view, store, includePaths) + view.snapshot = NewSnapshot(view, includePaths) view.snapshotRelease = view.snapshot.Acquire() @@ -102,14 +101,18 @@ func (v *View) FileKnown(uri uri.URI) bool { return v.knownFiles[uri] } -func (v *View) FileChange(ctx context.Context, changes []*FileChange, postFns ...func()) { +// FileChange applies changes to the view: it swaps in a new snapshot (an +// O(1) copy-on-write clone) and re-parses the changed files, then runs the +// postFns asynchronously with the affected URIs (changed files plus their +// transitive dependents). The request thread never blocks on postFns, so +// diagnostics-heavy work (semantic analysis) does not stall the editor. +func (v *View) FileChange(ctx context.Context, changes []*FileChange, postFns ...func(affected []uri.URI)) { for _, change := range changes { v.MarkFileKnown(change.URI) } - // snapshot clone + // Swap in the new snapshot. newSnapshot, release := v.snapshot.clone() - // release previous snapshot v.snapshotRelease() v.snapshotMu.Lock() @@ -121,30 +124,65 @@ func (v *View) FileChange(ctx context.Context, changes []*FileChange, postFns .. v.snapshotRelease = release asyncRelease := v.snapshot.Acquire() - // handle current snapshot - - // TODO(jpf): 异步 parse 和 completion 的顺序问题 - // go func() { - defer asyncRelease() - uris := make(map[uri.URI]struct{}) + // Re-parse the changed files so the snapshot's include edges and + // parsed caches reflect the change before any request observes it. + // Parse is lazy and cached, so requests racing ahead of this loop + // simply parse on demand. + uris := make([]uri.URI, 0, len(changes)) for _, change := range changes { - uris[change.URI] = struct{}{} + uris = append(uris, change.URI) } + sort.Slice(uris, func(i, j int) bool { return uris[i] < uris[j] }) + + for _, uri := range uris { + if _, err := v.snapshot.Parse(ctx, uri); err != nil { + slog.Error("parse error", "err", err) + } + } + + affected := v.affectedFiles(changes) + + go func() { + defer asyncRelease() + + for i := range postFns { + postFns[i](affected) + } + }() +} + +// affectedFiles returns the changed URIs plus the transitive dependents of +// each, deduped. Dependents are computed on the current snapshot, after the +// changes were applied and re-parsed, so the edges reflect the change. +// Changes come first, in order; dependents follow, sorted by URI. +func (v *View) affectedFiles(changes []*FileChange) []uri.URI { + affected := make([]uri.URI, 0, len(changes)) + seen := make(map[uri.URI]struct{}, len(changes)) + + for _, change := range changes { + if _, ok := seen[change.URI]; ok { + continue + } + + seen[change.URI] = struct{}{} + affected = append(affected, change.URI) - for uri := range uris { v.snapshotMu.Lock() - _, err := v.snapshot.Parse(ctx, uri) + deps := v.snapshot.Dependents(change.URI) v.snapshotMu.Unlock() - if err != nil { - slog.Error("parse error", "err", err) + for _, dep := range deps { + if _, ok := seen[dep]; ok { + continue + } + + seen[dep] = struct{}{} + affected = append(affected, dep) } } - for i := range postFns { - postFns[i]() - } + return affected } func (v *View) Snapshot() (*Snapshot, func()) { @@ -154,3 +192,12 @@ func (v *View) Snapshot() (*Snapshot, func()) { // The snapshot is created in NewView and only set to nil on shutdown. return v.snapshot, v.snapshot.Acquire() } + +// IsCurrent reports whether ss is the view's latest snapshot. Used by +// asynchronous work to drop results that a newer change superseded. +func (v *View) IsCurrent(ss *Snapshot) bool { + v.snapshotMu.Lock() + defer v.snapshotMu.Unlock() + + return v.snapshot == ss +} diff --git a/lsp/codejump/cross_reference_test.go b/lsp/codejump/cross_reference_test.go new file mode 100644 index 0000000..25f6a46 --- /dev/null +++ b/lsp/codejump/cross_reference_test.go @@ -0,0 +1,108 @@ +package codejump + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "go.lsp.dev/protocol" + "go.lsp.dev/uri" + + "github.com/karitham/thrift-ls/lsp/cache" +) + +// TestReferenceBareCrossFile covers find-references across an include when +// the reference uses the bare name: federation.gundam.thrift defines +// Gundam; main.thrift includes it and uses the bare Gundam. References +// found from the definition file must include the bare usage. +func TestReferenceBareCrossFile(t *testing.T) { + mainFile := `include "federation.gundam.thrift" + +struct StrikeRouge { + 1: required Gundam pack +}` + + gundamFile := `struct Gundam { + 1: required string Name +}` + + ss := cache.BuildSnapshotForTest([]*cache.FileChange{ + {URI: "file:///tmp/federation.gundam.thrift", Version: 0, Content: []byte(gundamFile), From: cache.FileChangeTypeDidOpen}, + {URI: "file:///tmp/main.thrift", Version: 0, Content: []byte(mainFile), From: cache.FileChangeTypeDidOpen}, + }) + + // Cursor on "Gundam" (the definition name) in federation.gundam.thrift. + locations, err := Reference(t.Context(), ss, "file:///tmp/federation.gundam.thrift", protocol.Position{Line: 0, Character: 7}) + assert.NoError(t, err) + + var uris []string + for _, loc := range locations { + uris = append(uris, string(loc.URI)) + } + + assert.Contains(t, uris, "file:///tmp/main.thrift", "bare reference in the including file must be found") +} + +// TestReferenceQualifiedCrossFile covers find-references when the +// reference uses the include-qualified name (includeName.Type). +func TestReferenceQualifiedCrossFile(t *testing.T) { + mainFile := `include "federation.gundam.thrift" + +struct StrikeRouge { + 1: required federation.Gundam pack +}` + + gundamFile := `struct Gundam { + 1: required string Name +}` + + ss := cache.BuildSnapshotForTest([]*cache.FileChange{ + {URI: "file:///tmp/federation.gundam.thrift", Version: 0, Content: []byte(gundamFile), From: cache.FileChangeTypeDidOpen}, + {URI: "file:///tmp/main.thrift", Version: 0, Content: []byte(mainFile), From: cache.FileChangeTypeDidOpen}, + }) + + locations, err := Reference(t.Context(), ss, "file:///tmp/federation.gundam.thrift", protocol.Position{Line: 0, Character: 7}) + assert.NoError(t, err) + + var uris []string + for _, loc := range locations { + uris = append(uris, string(loc.URI)) + } + + assert.Contains(t, uris, "file:///tmp/main.thrift", "qualified reference in the including file must be found") +} + +// TestDefinitionTransitiveInclude covers go-to-definition through a +// multi-hop include chain: main.thrift includes federation.gundam.thrift, +// which includes mobile_suit.zeon.thrift, which defines Zaku. A definition +// lookup in main.thrift must reach it. +func TestDefinitionTransitiveInclude(t *testing.T) { + mainFile := `include "federation.gundam.thrift" + +struct Char { + 1: optional Zaku ride +}` + + federationFile := `include "mobile_suit.zeon.thrift" + +struct Gundam { + 1: required string Name +}` + + zeonFile := `struct Zaku { + 1: required string Model +}` + + ss := cache.BuildSnapshotForTest([]*cache.FileChange{ + {URI: "file:///tmp/mobile_suit.zeon.thrift", Version: 0, Content: []byte(zeonFile), From: cache.FileChangeTypeDidOpen}, + {URI: "file:///tmp/federation.gundam.thrift", Version: 0, Content: []byte(federationFile), From: cache.FileChangeTypeDidOpen}, + {URI: "file:///tmp/main.thrift", Version: 0, Content: []byte(mainFile), From: cache.FileChangeTypeDidOpen}, + }) + + // Cursor on "Zaku" in main.thrift (line 3, character 13: the type of + // "1: optional Zaku ride"). + locations, err := Definition(t.Context(), ss, "file:///tmp/main.thrift", protocol.Position{Line: 3, Character: 13}) + assert.NoError(t, err) + + assert.Len(t, locations, 1) + assert.Equal(t, uri.URI("file:///tmp/mobile_suit.zeon.thrift"), locations[0].URI, "definition must resolve through the include chain") +} diff --git a/lsp/codejump/reference.go b/lsp/codejump/reference.go index 8a67c69..9c2685a 100644 --- a/lsp/codejump/reference.go +++ b/lsp/codejump/reference.go @@ -201,6 +201,8 @@ func searchServiceReferences(ctx context.Context, ss *cache.Snapshot, file uri.U res = append(res, locations...) for _, referenceFile := range referenceFiles(ss, file) { + // References in including files may use the bare name or the + // include-qualified name; the match function accepts both. locations, err := searchServiceDefinitionReferences(ctx, ss, referenceFile, svcName) if err != nil { return nil, err @@ -238,7 +240,12 @@ func searchServiceDefinitionReferences(ctx context.Context, ss *cache.Snapshot, } for _, svc := range pf.AST().Services() { - if svc.Extends == nil || svc.Extends.Text != svcName { + if svc.Extends == nil { + continue + } + + // Accept both the bare name and the include-qualified literal. + if bareName(svc.Extends.Text) != bareName(svcName) { continue } @@ -260,6 +267,8 @@ func searchIdentifierReferences(ctx context.Context, ss *cache.Snapshot, file ur res = append(res, locations...) for _, referenceFile := range referenceFiles(ss, file) { + // References in including files may use the bare name or the + // include-qualified name; the match function accepts both. locations, err := searchDefinitionIdentifierReferences(ctx, ss, referenceFile, typeName, definitionType) if err != nil { return nil, err @@ -285,7 +294,14 @@ func searchDefinitionIdentifierReferences(ctx context.Context, ss *cache.Snapsho } jumpFieldType := func(ft *syntax.FieldType) { - if ft == nil || typeReferenceName(ft) != typeName { + if ft == nil { + return + } + + // Accept every reference form of the same definition: bare + // ("Test"), include-name-qualified ("user.Test"), and + // file-base-qualified ("user.thrift.Test"). + if bareName(typeReferenceName(ft)) != bareName(typeName) { return } @@ -397,6 +413,8 @@ func searchConstValueIdentifierReferences(ctx context.Context, ss *cache.Snapsho res = append(res, locations...) for _, referenceFile := range referenceFiles(ss, file) { + // References in including files may use the bare name or the + // include-qualified name; the match function accepts both. locations, err := searchConstValueIdentifierReference(ctx, ss, referenceFile, valueName) if err != nil { return nil, err @@ -419,7 +437,7 @@ func searchConstValueIdentifierReference(ctx context.Context, ss *cache.Snapshot } jumpValue := func(v *syntax.ConstValue) { - if v != nil && v.Kind == syntax.ValueIdent && v.Text == valueName { + 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}) } } diff --git a/lsp/codejump/utils.go b/lsp/codejump/utils.go index c431b51..500a0a6 100644 --- a/lsp/codejump/utils.go +++ b/lsp/codejump/utils.go @@ -28,8 +28,9 @@ const ( // definitionFiles returns the files to search for a definition, in order. // A qualified name ("base.User") resolves to the include file; an -// unqualified name searches the current file first, then each included -// file. +// unqualified name searches the current file first, then every file +// transitively included, so a type visible through a multi-hop include +// chain (A includes B includes C) is found. func definitionFiles(ctx context.Context, ss *cache.Snapshot, file uri.URI, ast *syntax.Document, name string) []uri.URI { include, _ := lsputils.ParseIdent(file, ast.Includes(), name) if include != "" { @@ -45,14 +46,38 @@ func definitionFiles(ctx context.Context, ss *cache.Snapshot, file uri.URI, ast } files := []uri.URI{file} + seen := map[uri.URI]bool{file: true} resolver := ss.Resolver() - for _, inc := range ast.Includes() { - if path := lsputils.IncludePathText(inc); path != "" { - files = append(files, resolver.ResolveInclude(file, path)) + var visit func(f uri.URI) + + visit = func(f uri.URI) { + doc := ast + if f != file { + pf, err := ss.Parse(ctx, f) + if err != nil || pf.AST() == nil { + return + } + + doc = pf.AST() + } + + for _, inc := range doc.Includes() { + if path := lsputils.IncludePathText(inc); path != "" { + incFile := resolver.ResolveInclude(f, path) + if seen[incFile] { + continue + } + + seen[incFile] = true + files = append(files, incFile) + visit(incFile) + } } } + visit(file) + return files } @@ -286,3 +311,14 @@ func typeReferenceName(ft *syntax.FieldType) string { return "" } + +// bareName strips the include qualifier from a name: "base.User" becomes +// "User". References in files that include the definition file use the bare +// name, so qualified literals must match against it too. +func bareName(name string) string { + if i := strings.LastIndexByte(name, '.'); i >= 0 { + return name[i+1:] + } + + return name +} diff --git a/lsp/completion/completion_test.go b/lsp/completion/completion_test.go index b7a9961..c9af710 100644 --- a/lsp/completion/completion_test.go +++ b/lsp/completion/completion_test.go @@ -7,7 +7,6 @@ import ( "go.lsp.dev/uri" "github.com/karitham/thrift-ls/lsp/cache" - "github.com/karitham/thrift-ls/lsp/memoize" "github.com/karitham/thrift-ls/lsp/types" ) @@ -16,87 +15,12 @@ import ( func buildSnapshot(t *testing.T, includePaths []string, files ...*cache.FileChange) *cache.Snapshot { t.Helper() - store := &memoize.Store{} - c := cache.New(store, nil) + c := cache.New(nil) fs := cache.NewOverlayFS(c) _ = fs.Update(t.Context(), files) - view := cache.NewView("test", uri.File("/tmp"), fs, store, includePaths) + view := cache.NewView("test", uri.File("/tmp"), fs, includePaths) - return cache.NewSnapshot(view, store, includePaths) -} - -func TestSemanticCompletion(t *testing.T) { - userFile := `struct User { - 1: required i64 id -} - -enum Color { - RED = 1, - GREEN -} - -const i32 DEFAULT = 5` - - apiFile := `include "user.thrift" - -struct Profile { - 1: required User user - 2: optional string bio -} - -const i32 LIMIT = 10` - - ss := buildSnapshot(t, nil, - &cache.FileChange{URI: "file:///tmp/user.thrift", Version: 0, Content: []byte(userFile), From: cache.FileChangeTypeDidOpen}, - &cache.FileChange{URI: "file:///tmp/api.thrift", Version: 0, Content: []byte(apiFile), From: cache.FileChangeTypeDidOpen}, - ) - - tests := []struct { - name string - file string - pos types.Position - want []string - notWant []string - }{ - { - name: "type position completes type names from includes", - file: "file:///tmp/api.thrift", - pos: types.Position{Line: 3, Character: 15}, // on "User" in "1: required User user" - want: []string{"User", "Profile", "Color"}, - }, - { - name: "value position completes consts and enum values", - file: "file:///tmp/api.thrift", - pos: types.Position{Line: 7, Character: 19}, // on "10" in "const i32 LIMIT = 10" - want: []string{"DEFAULT", "RED", "Color.GREEN", "LIMIT"}, - notWant: []string{"User", "Profile"}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - pf, err := ss.Parse(t.Context(), uri.URI(tt.file)) - assert.NoError(t, err) - - pos, err := pf.Mapper().LSPPosToParserPosition(tt.pos) - assert.NoError(t, err) - - cands := semanticCandidates(t.Context(), ss, uri.URI(tt.file), pf, pos) - - got := make(map[string]bool) - for _, c := range cands { - got[c.showText] = true - } - - for _, w := range tt.want { - assert.True(t, got[w], "missing candidate %q in %v", w, got) - } - - for _, nw := range tt.notWant { - assert.False(t, got[nw], "unexpected candidate %q in %v", nw, got) - } - }) - } + return cache.NewSnapshot(view, includePaths) } func TestCompletionEndToEnd(t *testing.T) { @@ -112,7 +36,7 @@ func TestCompletionEndToEnd(t *testing.T) { Fh: fh, Pos: types.Position{Line: 5, Character: 16}, // after "Us" in "1: required Us" } - items, _, err := DefaultTokenCompletion.Completion(t.Context(), ss, cmp) + items, _, _, err := DefaultTokenCompletion.Completion(t.Context(), ss, cmp) assert.NoError(t, err) labels := make([]string, 0, len(items)) diff --git a/lsp/completion/context.go b/lsp/completion/context.go new file mode 100644 index 0000000..c96af85 --- /dev/null +++ b/lsp/completion/context.go @@ -0,0 +1,526 @@ +package completion + +import ( + "strings" + + "github.com/karitham/thrift-ls/syntax" +) + +// ContextKind classifies the grammar slot the cursor sits in. Providers are +// selected by slot, so a cursor on a field name never suggests value +// candidates and a cursor in an include literal never suggests keywords. +type ContextKind uint8 + +const ( + CtxNone ContextKind = iota + CtxIncludePath // inside an include/cpp_include string literal + CtxType // any type reference slot: field/return/arg/throws/typedef/const/map + CtxFieldName // struct/union/exception field, argument, or throws member name + CtxFieldID // field id slot (before ':') + CtxFieldValue // after '=' in a field or const + CtxEnumValueName // enum member name slot + CtxDefinitionName // top-level definition name (struct/enum/service/typedef/const) + CtxFunctionName // service function name slot + CtxServiceExtends // after 'extends' + CtxAnnotationKey // inside ( ... ) — annotation name position + CtxAnnotationValue // inside an annotation value string literal + CtxKeyword // no structural slot: keywords + identifiers +) + +// Context is the resolved grammar slot at the cursor. +type Context struct { + Kind ContextKind + + Path []syntax.Node // SearchNodePathByPosition result; annotations stay opaque + Offset int // byte offset of the cursor in the document + + // Prefix is the text to filter candidates against (the typed path + // inside quotes for include/annotation slots), and EditStart the byte + // offset where the completion edit range starts. + Prefix string + EditStart int + + Doc *syntax.Document +} + +// ResolveContext classifies the grammar slot at pos. Token-level checks come +// first (cheap and precise: strings, braces, parens, separators), then the +// node path, then a CtxKeyword fallback. +func ResolveContext(doc *syntax.Document, pos syntax.Position) Context { + c := Context{ + Kind: CtxKeyword, + Offset: pos.Offset, + Path: doc.SearchNodePathByPosition(pos), + Doc: doc, + } + + atIdx, at := tokenAt(doc, pos.Offset) + + // The token before the cursor: the token containing the cursor when the + // cursor sits at its end, the previous token when mid-token. + prevIdx := atIdx + if at != nil && pos.Offset < at.Offset+len(at.Text) { + prevIdx = atIdx - 1 + } + + c.Prefix, c.EditStart = prefixRange(doc, pos, atIdx, at) + + // 1. String literal slots: include paths and annotation values. Other + // strings (const defaults, values) are not completion slots. + if at != nil && at.Kind == syntax.TokenStringLiteral && insideString(at, pos.Offset) { + text := strings.Trim(stringPrefix(at, pos.Offset), "'\"") + + switch { + case hasInclude(c.Path): + c.Kind = CtxIncludePath + c.Prefix = text + c.EditStart = at.Offset + 1 + case deepestConstValue(c.Path): + // A default value string (e.g. an argument default): not a slot. + c.Kind = CtxNone + default: + if opener, ok := parenOpener(doc, atIdx); ok && !isThrowsGroup(doc, opener) { + c.Kind = CtxAnnotationValue + c.Prefix = text + c.EditStart = at.Offset + 1 + } else { + c.Kind = CtxNone + } + } + + return c + } + + // 2. Field id slot: cursor on an int immediately followed by ':' — + // before the struct member rule, so "{ |1:" is CtxFieldID, not a + // member name position. + if at != nil && at.Kind == syntax.TokenIntConstant && atIdx+1 < len(doc.Tokens) && doc.Tokens[atIdx+1].Kind == syntax.TokenColon { + c.Kind = CtxFieldID + + return c + } + + // 3. Token adjacency before the cursor. + if prevIdx >= 0 { + switch prev := doc.Tokens[prevIdx]; prev.Kind { + case syntax.TokenLParen: + c.Kind = afterParenKind(doc, prevIdx) + + return c + case syntax.TokenComma, syntax.TokenSemicolon: + if opener, ok := parenOpener(doc, prevIdx); ok { + c.Kind = insideParenKind(doc, opener) + } else if kw, ok := braceBodyKind(doc, prevIdx); ok { + c.Kind = memberKind(kw) + } + + return c + case syntax.TokenLBrace: + if kw, ok := braceBodyKind(doc, prevIdx); ok { + c.Kind = memberKind(kw) + } + + return c + case syntax.TokenExtends: + c.Kind = CtxServiceExtends + + return c + case syntax.TokenEqual: + c.Kind = CtxFieldValue + + return c + case syntax.TokenColon: + // Between the id and the type: a type position. + c.Kind = CtxType + + return c + case syntax.TokenRequired, syntax.TokenOptional: + c.Kind = CtxType + + return c + } + } + + // 4. Cursor on a token. + if at != nil { + switch at.Kind { + case syntax.TokenExtends: + c.Kind = CtxServiceExtends + + return c + case syntax.TokenRequired, syntax.TokenOptional: + c.Kind = CtxKeyword + + return c + case syntax.TokenIdentifier: + // "ZeonForces.|" — an identifier ending in a dot is a + // qualified value position; the lexer may split the dot off + // the token, so the cursor lands right at its end. + if strings.HasSuffix(at.Text, ".") && pos.Offset == at.Offset+len(at.Text) { + c.Kind = CtxFieldValue + + return c + } + } + } + + // 5. Node path: the deepest node decides. + switch n := deepestNode(c.Path).(type) { + case *syntax.FieldType: + c.Kind = CtxType + case *syntax.ConstValue: + if n.Kind == syntax.ValueIdent { + c.Kind = CtxFieldValue + } else { + c.Kind = CtxNone + } + case *syntax.Identifier: + c.Kind = identifierKind(c.Path, n) + case *syntax.Field: + switch { + case n.Name != nil && pos.Offset < tokenOffset(doc, n.Name): + c.Kind = CtxType + case n.Value == nil: + c.Kind = CtxFieldName + default: + c.Kind = CtxFieldValue + } + } + + return c +} + +// identifierKind classifies an identifier by its role, carried by its parent. +func identifierKind(path []syntax.Node, n *syntax.Identifier) ContextKind { + if len(path) < 2 { + return CtxKeyword + } + + switch parent := path[len(path)-2].(type) { + case *syntax.FieldType: + return CtxType + case *syntax.Field: + if parent.Value == nil { + return CtxFieldName + } + + return CtxFieldValue + case *syntax.EnumValue: + return CtxEnumValueName + case *syntax.Const: + return CtxDefinitionName + case *syntax.Struct, *syntax.Enum: + return CtxDefinitionName + case *syntax.Service: + if parent.Name == n { + return CtxDefinitionName + } + + return CtxServiceExtends + case *syntax.Function: + return CtxFunctionName + case *syntax.Typedef: + return CtxDefinitionName + } + + return CtxKeyword +} + +// afterParenKind classifies the cursor right after '(' (prev is the opener). +func afterParenKind(doc *syntax.Document, opener int) ContextKind { + switch prev := doc.Tokens[opener-1]; prev.Kind { + case syntax.TokenRParen: + // Function annotations after a closed args list. + return CtxAnnotationKey + case syntax.TokenThrows: + return CtxFieldName + case syntax.TokenIdentifier: + // A function name opens the args list; a member name opens its + // annotations. The enclosing brace body disambiguates. + if kw, ok := braceBodyKind(doc, opener-1); ok && kw == syntax.TokenService { + return CtxFieldName + } + + return CtxAnnotationKey + default: + // Type annotations (i32 ( ... ), map<...> ( ... )). + return CtxAnnotationKey + } +} + +// insideParenKind classifies the cursor after ','/';' inside the group +// opened at opener. +func insideParenKind(doc *syntax.Document, opener int) ContextKind { + switch prev := doc.Tokens[opener-1]; prev.Kind { + case syntax.TokenRParen: + return CtxAnnotationKey + case syntax.TokenThrows: + return CtxFieldName + case syntax.TokenIdentifier: + if kw, ok := braceBodyKind(doc, opener-1); ok && kw == syntax.TokenService { + return CtxFieldName + } + + return CtxAnnotationKey + default: + return CtxAnnotationKey + } +} + +// isThrowsGroup reports whether the group opened at opener is a throws +// clause (which contains fields, not annotations). +func isThrowsGroup(doc *syntax.Document, opener int) bool { + return opener > 0 && doc.Tokens[opener-1].Kind == syntax.TokenThrows +} + +// memberKind maps a container keyword to its member slot. +func memberKind(kw syntax.TokenKind) ContextKind { + switch kw { + case syntax.TokenEnum: + return CtxEnumValueName + case syntax.TokenService: + return CtxFunctionName + default: + return CtxFieldName + } +} + +// tokenAt returns the index and token containing offset (inclusive end), or +// (index of the token before offset, nil) when the cursor sits between +// tokens. -1 when offset precedes the first token. The zero-length EOF +// token never matches: a cursor at the end of the document resolves to the +// last real token. +func tokenAt(doc *syntax.Document, offset int) (int, *syntax.Token) { + toks := doc.Tokens + + lo, hi := 0, len(toks) + for lo < hi { + mid := (lo + hi) / 2 + if toks[mid].Offset <= offset { + lo = mid + 1 + } else { + hi = mid + } + } + + i := lo - 1 + for i >= 0 && toks[i].Kind == syntax.TokenEOF { + i-- + } + + if i < 0 { + return -1, nil + } + + t := &toks[i] + if offset >= t.Offset && offset <= t.Offset+len(t.Text) { + return i, t + } + + return i, nil +} + +// insideString reports whether offset is inside the string token: after the +// opening quote, or at the end of an unterminated literal. +func insideString(t *syntax.Token, offset int) bool { + if offset <= t.Offset || offset > t.Offset+len(t.Text) { + return false + } + + if offset < t.Offset+len(t.Text) { + return true + } + + // At the token end: still inside when the literal is unterminated. + return len(t.Text) < 2 || t.Text[len(t.Text)-1] != t.Text[0] +} + +// stringPrefix returns the string token text up to offset, including the +// opening quote. +func stringPrefix(t *syntax.Token, offset int) string { + end := min(offset-t.Offset, len(t.Text)) + + return t.Text[:end] +} + +// prefixRange returns the identifier-ish text typed before the cursor (same +// line, contiguous tokens) and the byte offset where it starts. +func prefixRange(doc *syntax.Document, pos syntax.Position, atIdx int, at *syntax.Token) (string, int) { + var parts []string + + start := pos.Offset + + // The cursor may sit inside a token (mid-word typing): take the partial + // text, then continue over byte-adjacent tokens only. + nextEnd := pos.Offset + if at != nil && pos.Offset > at.Offset && at.Kind != syntax.TokenStringLiteral { + partial := at.Text[:pos.Offset-at.Offset] + if identish(partial) { + parts = append(parts, partial) + start = at.Offset + nextEnd = at.Offset + } + } + + for i := atIdx - 1; i >= 0; i-- { + t := &doc.Tokens[i] + if t.Line != pos.Line || !identish(t.Text) { + break + } + + // Tokens must be byte-adjacent: whitespace ends the prefix. + if t.Offset+len(t.Text) != nextEnd { + break + } + + start = t.Offset + nextEnd = t.Offset + parts = append([]string{t.Text}, parts...) + } + + return strings.Join(parts, ""), start +} + +// identish reports whether s consists of identifier-ish characters only. +func identish(s string) bool { + if s == "" { + return false + } + + for _, r := range s { + switch { + case r == '_' || r == '.': + case r >= 'a' && r <= 'z': + case r >= 'A' && r <= 'Z': + case r >= '0' && r <= '9': + default: + return false + } + } + + return true +} + +// parenOpener returns the index of the '(' opening the nearest enclosing +// paren group at or before from. +func parenOpener(doc *syntax.Document, from int) (int, bool) { + if from < 0 { + return 0, false + } + + depth := 0 + + for i := from; i >= 0; i-- { + switch doc.Tokens[i].Kind { + case syntax.TokenRParen: + depth++ + case syntax.TokenLParen: + if depth == 0 { + return i, true + } + + depth-- + } + } + + return 0, false +} + +// braceBodyKind returns the container keyword (struct/union/exception/enum/ +// service) of the nearest enclosing brace body at or before from. +func braceBodyKind(doc *syntax.Document, from int) (syntax.TokenKind, bool) { + if from < 0 { + return 0, false + } + + depth := 0 + + for i := from; i >= 0; i-- { + switch doc.Tokens[i].Kind { + case syntax.TokenRBrace: + depth++ + case syntax.TokenLBrace: + if depth == 0 { + return containerKeywordBefore(doc, i) + } + + depth-- + } + } + + return 0, false +} + +// containerKeywordBefore returns the keyword of the container whose opening +// brace is at index brace, skipping annotations between the name and brace. +func containerKeywordBefore(doc *syntax.Document, brace int) (syntax.TokenKind, bool) { + j := brace - 1 + + // Skip a closing paren group (annotations on the container). + for j >= 0 && doc.Tokens[j].Kind == syntax.TokenRParen { + depth := 0 + + for j >= 0 { + switch doc.Tokens[j].Kind { + case syntax.TokenRParen: + depth++ + case syntax.TokenLParen: + depth-- + if depth == 0 { + j-- + + goto pastParens + } + } + + j-- + } + + pastParens: + } + + if j < 1 || doc.Tokens[j].Kind != syntax.TokenIdentifier { + return 0, false + } + + switch kw := doc.Tokens[j-1].Kind; kw { + case syntax.TokenStruct, syntax.TokenUnion, syntax.TokenException, + syntax.TokenEnum, syntax.TokenService: + return kw, true + } + + return 0, false +} + +// deepestNode returns the innermost node of the path, or nil. +func deepestNode(path []syntax.Node) syntax.Node { + if len(path) == 0 { + return nil + } + + return path[len(path)-1] +} + +// tokenOffset returns the byte offset of the first token of n. +func tokenOffset(doc *syntax.Document, n syntax.Node) int { + return doc.TokenPosition(n.TokStart()).Offset +} + +// deepestConstValue reports whether the deepest node is a ConstValue. +func deepestConstValue(path []syntax.Node) bool { + _, ok := deepestNode(path).(*syntax.ConstValue) + + return ok +} + +// hasInclude reports whether the path contains an include statement. +func hasInclude(path []syntax.Node) bool { + for _, n := range path { + switch n.(type) { + case *syntax.Include, *syntax.CPPInclude: + return true + } + } + + return false +} diff --git a/lsp/completion/context_test.go b/lsp/completion/context_test.go new file mode 100644 index 0000000..1f1edec --- /dev/null +++ b/lsp/completion/context_test.go @@ -0,0 +1,158 @@ +package completion + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/karitham/thrift-ls/syntax" +) + +// parseWithCursor parses src with a | cursor marker and returns the +// document and the byte offset of the cursor. +func parseWithCursor(t *testing.T, src string) (*syntax.Document, int) { + t.Helper() + + idx := strings.Index(src, "|") + assert.NotEqual(t, -1, idx, "missing cursor marker in %q", src) + + doc, _ := syntax.Parse([]byte(src[:idx] + src[idx+1:])) + + return doc, idx +} + +func TestResolveContext(t *testing.T) { + tests := []struct { + name string + src string + want ContextKind + }{ + // Top level and keywords. + {"empty file", `|`, CtxKeyword}, + {"whitespace only", " |\n", CtxKeyword}, + {"service name slot", `service |Federation {`, CtxDefinitionName}, + {"service name slot at end", `service Federatio|n`, CtxDefinitionName}, + + // Include paths. + {"include path mid-string", `include "fed|era"`, CtxIncludePath}, + {"include path unterminated at EOF", `include "fed|`, CtxIncludePath}, + {"include path after closing quote", `include "federation.gundam.thrift"|`, CtxKeyword}, + {"cpp include path", `cpp_include "zeon.thrift|"`, CtxIncludePath}, + + // Container member slots. + {"struct body after brace", `struct Gundam {|`, CtxFieldName}, + {"struct body after separator", `struct Gundam { 1: string Name; |}`, CtxFieldName}, + {"union body", `union MobileSuit { 1: string Name, |}`, CtxFieldName}, + {"exception body", `exception BayFull {|`, CtxFieldName}, + {"enum body after brace", `enum ZeonForces {|`, CtxEnumValueName}, + {"enum body after separator", `enum ZeonForces { ZAKU_I, |}`, CtxEnumValueName}, + {"service body after brace", `service Federation {|`, CtxFunctionName}, + {"service body after separator", `service Federation { void f(); |}`, CtxFunctionName}, + + // Function args and throws. + {"function args after paren", `service Federation { void f(|`, CtxFieldName}, + {"function args after comma", `service Federation { void f(1: i32 id, |`, CtxFieldName}, + {"throws after paren", `service Federation { void f() throws (|`, CtxFieldName}, + {"throws after comma", `service Federation { void f() throws (1: string m, |`, CtxFieldName}, + {"function annotations after args", `service Federation { void f() (|`, CtxAnnotationKey}, + + // Annotations. + {"field annotation after name", `struct Gundam { 1: string Name (|`, CtxAnnotationKey}, + {"field annotation after comma", `struct Gundam { 1: string Name (color = "blue", |)`, CtxAnnotationKey}, + {"field annotation value string", `struct Gundam { 1: string Name (color = "|")`, CtxAnnotationValue}, + {"type annotation after base type", `struct Gundam { 1: string (|) Name`, CtxAnnotationKey}, + + // Field structure. + {"field value after equal", `struct Gundam { 1: string Name = |}`, CtxFieldValue}, + {"const value after equal", `const i32 LIMIT = |`, CtxFieldValue}, + {"enum value after equal", `enum ZeonForces { ZAKU_I = |}`, CtxFieldValue}, + {"field id before colon", `struct Gundam { |1: string Name }`, CtxFieldID}, + {"field type after colon", `struct Gundam { 1: |}`, CtxType}, + {"field type after required", `struct Gundam { 1: required |}`, CtxType}, + {"field name on identifier", `struct Gundam { 1: required string Na|me }`, CtxFieldName}, + {"field type on identifier", `struct Gundam { 1: required Str|ing Name }`, CtxType}, + {"map key type", `struct Gundam { 1: map<|i32, string> fields }`, CtxType}, + {"field after annotations", `struct Gundam { 1: string Name (color = "blue") |}`, CtxKeyword}, + + // Values. + {"const ident value", `const i32 LIMIT = Color.GRE|EN`, CtxFieldValue}, + {"const string value", `const string s = "|`, CtxNone}, + {"const string value closed", `const string s = "x"|`, CtxNone}, + {"const int value", `const i32 LIMIT = 1|`, CtxNone}, + {"argument default string", `service Federation { void f(1: string s = "|") }`, CtxNone}, + + // Service extends. + {"extends after keyword", `service Federation extends |`, CtxServiceExtends}, + {"extends on identifier", `service Federation extends Zeo|n {`, CtxServiceExtends}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + doc, offset := parseWithCursor(t, tt.src) + + pos := positionAt(t, doc, offset) + got := ResolveContext(doc, pos) + + assert.Equal(t, tt.want, got.Kind) + }) + } +} + +// positionAt computes the document position for a byte offset. +func positionAt(t *testing.T, doc *syntax.Document, offset int) syntax.Position { + t.Helper() + + // Reconstruct line starts from the source offsets recorded on tokens. + for i := range doc.Tokens { + if doc.Tokens[i].Offset <= offset { + continue + } + } + + // The parser stores 1-based line/col per token; find the token at or + // before the offset and derive the position from it. + line, col := 1, 1 + + for i := range doc.Tokens { + tok := &doc.Tokens[i] + if tok.Offset > offset { + break + } + + line = tok.Line + col = tok.Col + (offset - tok.Offset) + } + + return syntax.Position{Line: line, Col: col, Offset: offset} +} + +// TestResolveContextNeverPanics exercises every byte offset of a document +// covering include, struct, enum, service, annotations, and values: the +// resolver must never panic on truncated or mid-token positions. +func TestResolveContextNeverPanics(t *testing.T) { + src := `include "federation.gundam.thrift" + +exception BayFull { + 1: string message (code = "BAY_FULL") +} + +enum ZeonForces { + ZAKU_I = 1, + GELGOOG +} + +service Federation extends ZeonForces { + void intercept(i32 count) throws (1: BayFull bay) +}` + + doc, errs := syntax.Parse([]byte(src)) + assert.Empty(t, errs) + + for offset := 0; offset <= len(src); offset++ { + pos := positionAt(t, doc, offset) + assert.NotPanics(t, func() { + _ = ResolveContext(doc, pos) + }, "offset %d", offset) + } +} diff --git a/lsp/completion/provider.go b/lsp/completion/provider.go new file mode 100644 index 0000000..088e5b0 --- /dev/null +++ b/lsp/completion/provider.go @@ -0,0 +1,239 @@ +package completion + +import ( + "context" + "path/filepath" + "sort" + + "go.lsp.dev/protocol" + "go.lsp.dev/uri" + + "github.com/karitham/thrift-ls/lsp/cache" + "github.com/karitham/thrift-ls/syntax" +) + +// Provider supplies completion candidates for one grammar slot. Prefix +// filtering, sorting, the edit range, and the item cap stay in the shared +// pipeline (TokenCompletion.Completion). +type Provider interface { + Kind() ContextKind + + // Candidates returns unfiltered candidates for the slot. The current + // context carries the prefix and the document. + Candidates(ctx context.Context, ss *cache.Snapshot, file uri.URI, c Context) []Candidate +} + +// providersFor returns the providers for a slot: the exact slot provider +// first, then supplementary providers (e.g. base type keywords on type +// positions). Unknown or intentionally empty slots (CtxNone, CtxFieldID, +// CtxDefinitionName, CtxFunctionName, CtxAnnotationValue) return nothing. +func providersFor(kind ContextKind) []Provider { + switch kind { + case CtxIncludePath: + return []Provider{includeProvider{}} + case CtxType: + return []Provider{typeProvider{}, keywordProvider{}} + case CtxFieldValue: + return []Provider{valueProvider{}} + case CtxFieldName: + return []Provider{fieldNameProvider{}} + case CtxEnumValueName: + return []Provider{valueProvider{}} + case CtxAnnotationKey: + return []Provider{annotationKeyProvider{}} + case CtxServiceExtends: + return []Provider{serviceExtendsProvider{}} + case CtxKeyword: + return []Provider{keywordProvider{}} + default: + return nil + } +} + +type includeProvider struct{} + +func (includeProvider) Kind() ContextKind { return CtxIncludePath } + +func (includeProvider) Candidates(_ context.Context, ss *cache.Snapshot, file uri.URI, c Context) []Candidate { + return ListDirAndFiles(filepath.Dir(file.Path()), ss.Resolver().IncludePaths(), c.Prefix) +} + +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) +} + +type valueProvider struct{} + +func (valueProvider) Kind() ContextKind { return CtxFieldValue } + +func (valueProvider) Candidates(ctx context.Context, ss *cache.Snapshot, file uri.URI, c Context) []Candidate { + return valueCandidates(ctx, ss, file, c.Doc) +} + +type keywordProvider struct{} + +func (keywordProvider) Kind() ContextKind { return CtxKeyword } + +// Candidates returns the keyword snippets and every identifier token known +// to the file (and its includes). +func (keywordProvider) Candidates(_ context.Context, ss *cache.Snapshot, file uri.URI, _ Context) []Candidate { + res := make([]Candidate, 0, len(keywords)+16) + + for text, format := range keywords { + res = append(res, Candidate{showText: text, insertText: text, format: format}) + } + + for text := range ss.TokensForFile(file) { + res = append(res, Candidate{showText: text, insertText: text, format: protocol.InsertTextFormatPlainText}) + } + + return res +} + +type fieldNameProvider struct{} + +func (fieldNameProvider) Kind() ContextKind { return CtxFieldName } + +// Candidates returns the field modifiers and every identifier token, so a +// field name position suggests required/optional and names from the +// codebase — never value candidates. +func (fieldNameProvider) Candidates(_ context.Context, ss *cache.Snapshot, file uri.URI, _ Context) []Candidate { + res := []Candidate{ + {showText: "required", insertText: "required", format: protocol.InsertTextFormatPlainText}, + {showText: "optional", insertText: "optional", format: protocol.InsertTextFormatPlainText}, + } + + for text := range ss.TokensForFile(file) { + res = append(res, Candidate{showText: text, insertText: text, format: protocol.InsertTextFormatPlainText}) + } + + return res +} + +type annotationKeyProvider struct{} + +func (annotationKeyProvider) Kind() ContextKind { return CtxAnnotationKey } + +// Candidates collects the annotation names used in the file and its +// transitively included files. +func (annotationKeyProvider) Candidates(ctx context.Context, ss *cache.Snapshot, file uri.URI, c Context) []Candidate { + keys := make(map[string]struct{}) + + collect := func(doc *syntax.Document) { + for key := range annotationKeys(doc) { + keys[key] = struct{}{} + } + } + + collect(c.Doc) + + for _, inc := range includedFiles(ss, file) { + if pf, err := ss.Parse(ctx, inc); err == nil && pf.AST() != nil { + collect(pf.AST()) + } + } + + var res []Candidate + for key := range keys { + res = append(res, Candidate{showText: key, insertText: key, format: protocol.InsertTextFormatPlainText}) + } + + sortCandidates(res) + + return res +} + +// annotationKeys collects the names of every annotation in the document: +// on definitions, fields, enum values, functions, namespaces, and typedefs. +func annotationKeys(doc *syntax.Document) map[string]struct{} { + keys := make(map[string]struct{}) + + add := func(annotations *syntax.Annotations) { + if annotations == nil { + return + } + + for _, a := range annotations.Items { + keys[a.Name.Text] = struct{}{} + } + } + + for _, ns := range doc.Namespaces() { + add(ns.Annotations) + } + + for _, td := range doc.Typedefs() { + add(td.Annotations) + } + + for _, cst := range doc.Consts() { + _ = cst // consts carry no annotations + } + + for _, st := range doc.Structs() { + add(st.Annotations) + + for _, f := range st.Fields { + add(f.Annotations) + } + } + + for _, enum := range doc.Enums() { + add(enum.Annotations) + + for _, v := range enum.Values { + add(v.Annotations) + } + } + + for _, svc := range doc.Services() { + add(svc.Annotations) + + for _, fn := range svc.Functions { + add(fn.Annotations) + } + } + + return keys +} + +type serviceExtendsProvider struct{} + +func (serviceExtendsProvider) Kind() ContextKind { return CtxServiceExtends } + +// Candidates returns the service names from the file and its includes. +func (serviceExtendsProvider) Candidates(ctx context.Context, ss *cache.Snapshot, file uri.URI, c Context) []Candidate { + names := make(map[string]struct{}) + + collect := func(doc *syntax.Document) { + for _, svc := range doc.Services() { + names[svc.Name.Text] = struct{}{} + } + } + + collect(c.Doc) + + for _, inc := range includedFiles(ss, file) { + if pf, err := ss.Parse(ctx, inc); err == nil && pf.AST() != nil { + collect(pf.AST()) + } + } + + var res []Candidate + for name := range names { + res = append(res, Candidate{showText: name, insertText: name, format: protocol.InsertTextFormatPlainText}) + } + + sortCandidates(res) + + return res +} + +// sortCandidates sorts candidates alphabetically by show text. +func sortCandidates(res []Candidate) { + sort.Slice(res, func(i, j int) bool { return res[i].showText < res[j].showText }) +} diff --git a/lsp/completion/semantic_based_completion.go b/lsp/completion/semantic_based_completion.go index 66c6ac2..a016716 100644 --- a/lsp/completion/semantic_based_completion.go +++ b/lsp/completion/semantic_based_completion.go @@ -9,13 +9,12 @@ import ( ) type Interface interface { - Completion(ctx context.Context, ss *cache.Snapshot, cmp *CompletionRequest) ([]*CompletionItem, protocol.Range, error) + // Completion returns the completion items for the request, the edit + // range, and whether the list was truncated by the item cap (the LSP + // isIncomplete flag). + Completion(ctx context.Context, ss *cache.Snapshot, cmp *CompletionRequest) ([]*CompletionItem, protocol.Range, bool, error) } -// SemanticBasedCompletion generates completion list based on semantic. It is more precisely than token based completion -// TODO(jpf) -type SemanticBasedCompletion struct{} - func BuildCompletionItem(candidate Candidate) *CompletionItem { return &CompletionItem{ Label: candidate.showText, diff --git a/lsp/completion/semantic_completion.go b/lsp/completion/semantic_completion.go index a611a39..7e532c3 100644 --- a/lsp/completion/semantic_completion.go +++ b/lsp/completion/semantic_completion.go @@ -11,75 +11,6 @@ import ( "github.com/karitham/thrift-ls/syntax" ) -// semanticCandidates returns context-aware completion candidates for the -// cursor position, or nil when the context is not a known completion -// position. Type positions complete with type names; constant value -// positions complete with const and enum value names. -func semanticCandidates(ctx context.Context, ss *cache.Snapshot, file uri.URI, parsedFile *cache.ParsedFile, pos syntax.Position) []Candidate { - path := parsedFile.AST().SearchNodePathByPosition(pos) - if len(path) == 0 { - return nil - } - - target := path[len(path)-1] - - switch n := target.(type) { - case *syntax.FieldType: - // Cursor on a type reference. - return typeCandidates(ctx, ss, file, parsedFile.AST()) - - case *syntax.Identifier: - // The role of an identifier is carried by its parent: inside a - // FieldType it is a type reference; as a field name the cursor is - // in the value position. - if len(path) < 2 { - return nil - } - - switch parent := path[len(path)-2].(type) { - case *syntax.FieldType: - return typeCandidates(ctx, ss, file, parsedFile.AST()) - case *syntax.Field: - if parent.Value == nil { - return valueCandidates(ctx, ss, file, parsedFile.AST()) - } - } - - case *syntax.ConstValue: - // Cursor on a constant value. - return valueCandidates(ctx, ss, file, parsedFile.AST()) - - case *syntax.Field: - // Cursor before the field name is a type position; after the name - // is a value position. - if n.Name != nil && pos.Offset < tokenOffset(parsedFile.AST(), n.Name) { - return typeCandidates(ctx, ss, file, parsedFile.AST()) - } - - if n.Value == nil { - return valueCandidates(ctx, ss, file, parsedFile.AST()) - } - - case *syntax.Const: - // Cursor on the const value. - if n.Value != nil && pos.Offset > tokenOffset(parsedFile.AST(), n.Name) { - return valueCandidates(ctx, ss, file, parsedFile.AST()) - } - - case *syntax.Typedef: - // Cursor on the typedef type. - if pos.Offset < tokenOffset(parsedFile.AST(), n.Name) { - return typeCandidates(ctx, ss, file, parsedFile.AST()) - } - } - - return nil -} - -func tokenOffset(doc *syntax.Document, n syntax.Node) int { - return doc.TokenPosition(n.TokStart()).Offset -} - // 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. diff --git a/lsp/completion/slot_completion_test.go b/lsp/completion/slot_completion_test.go new file mode 100644 index 0000000..58cbfb1 --- /dev/null +++ b/lsp/completion/slot_completion_test.go @@ -0,0 +1,358 @@ +package completion + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "go.lsp.dev/protocol" + "go.lsp.dev/uri" + + "github.com/karitham/thrift-ls/lsp/cache" + "github.com/karitham/thrift-ls/lsp/types" + "github.com/karitham/thrift-ls/syntax" +) + +// lspPosOf returns the LSP position (0-based line, UTF-16 character) +// immediately after the first occurrence of marker in content. +func lspPosOf(t *testing.T, content, marker string) types.Position { + t.Helper() + + idx := strings.Index(content, marker) + assert.NotEqual(t, -1, idx, "marker %q not found", marker) + + before := content[:idx] + line := strings.Count(before, "\n") + + lineStart := strings.LastIndex(before, "\n") + 1 + + return types.Position{ + Line: uint32(line), + Character: uint32(utf16Len([]byte(before[lineStart:])) + utf16Len([]byte(marker))), + } +} + +func utf16Len(b []byte) int { + n := 0 + + for _, r := range string(b) { + if r > 0xFFFF { + n += 2 + } else { + n++ + } + } + + return n +} + +// completionLabels runs the completion entry point at an LSP position and +// returns the item labels, the edit range, and the truncated flag. +func completionLabels(t *testing.T, ss *cache.Snapshot, file string, pos types.Position) ([]string, protocol.Range, bool) { + t.Helper() + + fh, err := ss.ReadFile(t.Context(), uri.URI(file)) + assert.NoError(t, err) + + items, rng, truncated, err := DefaultTokenCompletion.Completion(t.Context(), ss, &CompletionRequest{ + Pos: pos, + Fh: fh, + }) + assert.NoError(t, err) + + labels := make([]string, 0, len(items)) + for _, item := range items { + labels = append(labels, item.Label) + } + + return labels, rng, truncated +} + +// completionItems runs the entry point and returns raw items. +func completionItems(t *testing.T, ss *cache.Snapshot, file string, pos types.Position) ([]*CompletionItem, protocol.Range, bool) { + t.Helper() + + fh, err := ss.ReadFile(t.Context(), uri.URI(file)) + assert.NoError(t, err) + + items, rng, truncated, err := DefaultTokenCompletion.Completion(t.Context(), ss, &CompletionRequest{ + Pos: pos, + Fh: fh, + }) + assert.NoError(t, err) + + return items, rng, truncated +} + +// gundamSnapshot builds a snapshot with the gundam corpus: a main file with +// a struct, enum, const, and an included file defining another type. It +// returns the snapshot and the main file content. +func gundamSnapshot(t *testing.T, includePaths []string) (*cache.Snapshot, string) { + t.Helper() + + mainContent := `include "federation.gundam.thrift" + +struct Gundam { + 1: required string Name (color = "red") +} + +enum ZeonForces { + ZAKU_I = 1, + GELGOOG +} + +const i32 LIMIT = 10` + + incContent := `struct MobileSuit { + 1: required string ModelName +} + +exception BayFull { + 1: string message +}` + + ss := buildSnapshot(t, includePaths, + &cache.FileChange{URI: "file:///tmp/main.thrift", Version: 0, Content: []byte(mainContent), From: cache.FileChangeTypeDidOpen}, + &cache.FileChange{URI: "file:///tmp/federation.gundam.thrift", Version: 0, Content: []byte(incContent), From: cache.FileChangeTypeDidOpen}, + ) + + return ss, mainContent +} + +func TestCompletionSlots(t *testing.T) { + ss, mainContent := gundamSnapshot(t, nil) + + tests := []struct { + name string + marker string + want []string + notWant []string + }{ + { + name: "type slot suggests types and base keywords", + marker: "1: required ", + want: []string{"Gundam", "i32"}, + }, + { + name: "field name slot suggests the field name", + marker: "1: required string Na", + want: []string{"Name"}, + }, + { + name: "value slot suggests consts and enum values", + marker: "const i32 LIMIT = ", + want: []string{"LIMIT", "ZAKU_I", "ZeonForces.ZAKU_I"}, + }, + { + name: "annotation key slot suggests known keys", + marker: "1: required string Name (c", + want: []string{"color"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pos := lspPosOf(t, mainContent, tt.marker) + + labels, _, _ := completionLabels(t, ss, "file:///tmp/main.thrift", pos) + + for _, w := range tt.want { + assert.Contains(t, labels, w, "labels: %v", labels) + } + + for _, nw := range tt.notWant { + assert.NotContains(t, labels, nw, "labels: %v", labels) + } + }) + } +} + +// TestCompletionNoDuplicates: providers overlap (a type is both a type +// candidate and an identifier token), so the shared pipeline must dedupe. +func TestCompletionNoDuplicates(t *testing.T) { + ss, mainContent := gundamSnapshot(t, nil) + + for _, marker := range []string{"1: required ", "const i32 LIMIT = "} { + pos := lspPosOf(t, mainContent, marker) + + labels, _, _ := completionLabels(t, ss, "file:///tmp/main.thrift", pos) + + seen := make(map[string]struct{}, len(labels)) + for _, label := range labels { + assert.NotContains(t, seen, label, "duplicate candidate %q in %v", label, labels) + seen[label] = struct{}{} + } + } +} + +// TestCompletionSlotProviders asserts the full (uncapped) candidate sets per +// provider: cross-file types, modifiers on field names, and the absence of +// value candidates on field-name positions (regression: the old code +// suggested values while typing a field name). +func TestCompletionSlotProviders(t *testing.T) { + ss, _ := gundamSnapshot(t, nil) + + cc := Context{Doc: mustParse(t, ss, "file:///tmp/main.thrift")} + + ctx := t.Context() + + typeCands := typeProvider{}.Candidates(ctx, ss, "file:///tmp/main.thrift", cc) + typeLabels := labelsOf(typeCands) + assert.Contains(t, typeLabels, "Gundam") + assert.Contains(t, typeLabels, "MobileSuit", "types from included files") + assert.Contains(t, typeLabels, "BayFull", "types from included files") + + fieldCands := fieldNameProvider{}.Candidates(ctx, ss, "file:///tmp/main.thrift", cc) + fieldLabels := labelsOf(fieldCands) + assert.Contains(t, fieldLabels, "required") + assert.Contains(t, fieldLabels, "optional") + assert.NotContains(t, fieldLabels, "ZeonForces.ZAKU_I", "field name slot must not suggest qualified values") + + valueCands := valueProvider{}.Candidates(ctx, ss, "file:///tmp/main.thrift", cc) + valueLabels := labelsOf(valueCands) + assert.Contains(t, valueLabels, "ZeonForces.ZAKU_I", "value slot suggests qualified enum values") + + keyCands := annotationKeyProvider{}.Candidates(ctx, ss, "file:///tmp/main.thrift", cc) + keyLabels := labelsOf(keyCands) + assert.Contains(t, keyLabels, "color") +} + +func labelsOf(cands []Candidate) []string { + labels := make([]string, 0, len(cands)) + for _, c := range cands { + labels = append(labels, c.showText) + } + + return labels +} + +func mustParse(t *testing.T, ss *cache.Snapshot, file string) *syntax.Document { + t.Helper() + + pf, err := ss.Parse(t.Context(), uri.URI(file)) + assert.NoError(t, err) + assert.NotNil(t, pf.AST()) + + return pf.AST() +} + +// TestCompletionQualifiedValue covers "ZeonForces.|": the qualified name is +// filtered by the typed prefix, inserted without the qualifier, and the edit +// range starts at the cursor (the dot stays). +func TestCompletionQualifiedValue(t *testing.T) { + _, mainContent := gundamSnapshot(t, nil) + + content := strings.Replace(mainContent, "const i32 LIMIT = 10", "const i32 LIMIT = ZeonForces.", 1) + assert.NotEqual(t, mainContent, content) + + ss := buildSnapshot(t, nil, + &cache.FileChange{URI: "file:///tmp/main.thrift", Version: 0, Content: []byte(content), From: cache.FileChangeTypeDidOpen}, + &cache.FileChange{URI: "file:///tmp/federation.gundam.thrift", Version: 0, Content: []byte("struct MobileSuit {\n\t1: required string ModelName\n}"), From: cache.FileChangeTypeDidOpen}, + ) + + dotPos := lspPosOf(t, content, "const i32 LIMIT = ZeonForces.") + + items, rng, _ := completionItems(t, ss, "file:///tmp/main.thrift", dotPos) + + var labels []string + for _, item := range items { + labels = append(labels, item.Label) + } + + assert.Contains(t, labels, "ZeonForces.ZAKU_I") + + // The edit range starts at the cursor: the dot is not replaced. + assert.Equal(t, dotPos.Character, rng.Start.Character) + + for _, item := range items { + if item.Label == "ZeonForces.ZAKU_I" { + assert.Equal(t, "ZAKU_I", item.InsertText) + } + } +} + +func TestCompletionKeywordFallback(t *testing.T) { + ss := buildSnapshot(t, nil, + &cache.FileChange{URI: "file:///tmp/empty.thrift", Version: 0, Content: []byte(""), From: cache.FileChangeTypeDidOpen}, + ) + + labels, _, truncated := completionLabels(t, ss, "file:///tmp/empty.thrift", types.Position{Line: 0, Character: 0}) + assert.Contains(t, labels, "include") + assert.True(t, truncated, "keyword fallback exceeds the cap") +} + +// TestCompletionCapReportsIncomplete: a small result set reports the list as +// complete (isIncomplete false), the keyword fallback reports truncation. +func TestCompletionCapReportsIncomplete(t *testing.T) { + ss, mainContent := gundamSnapshot(t, nil) + + pos := lspPosOf(t, mainContent, "1: required ") + _, _, truncated := completionLabels(t, ss, "file:///tmp/main.thrift", pos) + assert.True(t, truncated, "type slot exceeds the cap (types + keywords)") + + pos = lspPosOf(t, mainContent, "1: required string Name (c") + _, _, truncated = completionLabels(t, ss, "file:///tmp/main.thrift", pos) + assert.False(t, truncated, "annotation key slot has one candidate") +} + +// TestCompletionIncludePath covers include-path completion: quotes are +// preserved (the edit range excludes them) and configured include path roots +// are searched in addition to the current directory. +func TestCompletionIncludePath(t *testing.T) { + dir := t.TempDir() + + assert.NoError(t, os.WriteFile(filepath.Join(dir, "federation.gundam.thrift"), []byte("struct Gundam {}"), 0o644)) + assert.NoError(t, os.MkdirAll(filepath.Join(dir, "zeon"), 0o755)) + assert.NoError(t, os.WriteFile(filepath.Join(dir, "zeon", "mobile_suit.thrift"), []byte("enum ZeonForces {}"), 0o644)) + + mainContent := "include \"fed|" + pos := lspPosOf(t, mainContent, "include \"fed") + + ss := buildSnapshot(t, []string{filepath.Join(dir, "zeon")}, + &cache.FileChange{URI: uri.File(filepath.Join(dir, "main.thrift")), Version: 0, Content: []byte(mainContent), From: cache.FileChangeTypeDidOpen}, + ) + + labels, rng, _ := completionLabels(t, ss, uri.File(filepath.Join(dir, "main.thrift")).String(), pos) + assert.Contains(t, labels, "federation.gundam.thrift") + + // The edit range starts after the opening quote: "include \"" is 9 + // UTF-16 units. + assert.Equal(t, uint32(9), rng.Start.Character, "range must exclude the opening quote") + + // includePaths root is searched too: typing "zeon/m" lists subdir files. + mainContent2 := "include \"zeon/m" + pos2 := lspPosOf(t, mainContent2, "include \"zeon/m") + ss2 := buildSnapshot(t, []string{filepath.Join(dir, "zeon")}, + &cache.FileChange{URI: uri.File(filepath.Join(dir, "main.thrift")), Version: 0, Content: []byte(mainContent2), From: cache.FileChangeTypeDidOpen}, + ) + + labels2, _, _ := completionLabels(t, ss2, uri.File(filepath.Join(dir, "main.thrift")).String(), pos2) + assert.Contains(t, labels2, "zeon/mobile_suit.thrift", "include path roots must be searched") +} + +// TestCompletionNoPrefixUnderflow: a line without spaces must not produce a +// wrapped edit range (the old whole-file prefix fallback bug). +func TestCompletionNoPrefixUnderflow(t *testing.T) { + ss := buildSnapshot(t, nil, + &cache.FileChange{URI: "file:///tmp/underflow.thrift", Version: 0, Content: []byte("const X=1"), From: cache.FileChangeTypeDidOpen}, + ) + + _, rng, _ := completionLabels(t, ss, "file:///tmp/underflow.thrift", types.Position{Line: 0, Character: 9}) + assert.LessOrEqual(t, rng.Start.Character, uint32(9), "edit range must not wrap") +} + +// TestCompletionNonASCIIPrefix: a non-ASCII line prefix must not corrupt the +// edit range character (UTF-16 vs byte counting). +func TestCompletionNonASCIIPrefix(t *testing.T) { + content := "// モビルスーツ\nconst X=1 😀" + pos := lspPosOf(t, content, "const X=1 😀") + + ss := buildSnapshot(t, nil, + &cache.FileChange{URI: "file:///tmp/emoji.thrift", Version: 0, Content: []byte(content), From: cache.FileChangeTypeDidOpen}, + ) + + _, rng, _ := completionLabels(t, ss, "file:///tmp/emoji.thrift", pos) + assert.Equal(t, pos.Character, rng.Start.Character, "empty prefix: range starts at the cursor") +} diff --git a/lsp/completion/token_completion.go b/lsp/completion/token_completion.go index 23ab72f..b6676d9 100644 --- a/lsp/completion/token_completion.go +++ b/lsp/completion/token_completion.go @@ -3,23 +3,19 @@ package completion import ( "context" "fmt" - "log/slog" - "path/filepath" "sort" "strings" - "unicode" "go.lsp.dev/protocol" - "go.lsp.dev/uri" "github.com/karitham/thrift-ls/lsp/cache" - "github.com/karitham/thrift-ls/syntax" ) var DefaultTokenCompletion Interface = &TokenCompletion{} -// TokenCompletion is token based completion. It generates completion list -// based on identifiers in the AST. +// TokenCompletion is the slot-based completion entry point. It resolves the +// grammar slot at the cursor, asks the providers for that slot, then +// filters, sorts, and caps the candidates. type TokenCompletion struct{} var keywords = map[string]protocol.InsertTextFormat{ @@ -48,175 +44,134 @@ var keywords = map[string]protocol.InsertTextFormat{ "typedef $1 $2": protocol.InsertTextFormatSnippet, } +// maxCandidates caps the completion list; the server reports the list as +// incomplete when the cap truncates. +const maxCandidates = 10 + +// Candidate is a single completion entry before LSP conversion. type Candidate struct { showText string insertText string format protocol.InsertTextFormat } -func (c *TokenCompletion) Completion(ctx context.Context, ss *cache.Snapshot, cmp *CompletionRequest) ([]*CompletionItem, protocol.Range, error) { - rng := protocol.Range{ - Start: protocol.Position{ - Line: cmp.Pos.Line, - Character: cmp.Pos.Character, - }, - End: protocol.Position{ - Line: cmp.Pos.Line, - Character: cmp.Pos.Character, - }, - } - +// Completion resolves the grammar slot at the cursor and returns the +// candidates for that slot, the edit range, and whether the list was +// truncated by the cap. +func (c *TokenCompletion) Completion(ctx context.Context, ss *cache.Snapshot, cmp *CompletionRequest) ([]*CompletionItem, protocol.Range, bool, error) { parsedFile, err := ss.Parse(ctx, cmp.Fh.URI()) if err != nil { - return nil, rng, err + return nil, protocol.Range{}, false, err } if parsedFile.AST() == nil { - return nil, rng, fmt.Errorf("parser ast failed") + return nil, protocol.Range{}, false, fmt.Errorf("parser ast failed") } pos, err := parsedFile.Mapper().LSPPosToParserPosition(cmp.Pos) if err != nil { - return nil, rng, err + return nil, protocol.Range{}, false, err } - tokens := ss.TokensForFile(cmp.Fh.URI()) - - slog.Debug("all tokens", "tokens", tokens) - - candidates := make([]Candidate, 0) - - slog.Debug("parser pos", "pos", pos) - - // Include completion: the cursor is inside an include path literal. - includePos := pos - includePos.Col-- - - includePath := parsedFile.AST().SearchNodePathByPosition(includePos) - if items, includeRng, err := c.includeCompletion(ss, cmp.Fh.URI(), parsedFile.AST(), includePath); err == nil { - candidates = append(candidates, items...) - if len(items) > 0 { - rng = includeRng - - slog.Debug("include completion candidates", "candidates", candidates) + cc := ResolveContext(parsedFile.AST(), pos) + + // A trailing dot (enum-qualified value position, e.g. "ZeonForces.|") + // means the user is about to type the member: filter on everything + // after the dots, insert after them, and strip the qualifier from + // inserted names so the result is "ZeonForces.ZAKU_I", not + // "ZeonForces.ZeonForces.ZAKU_I". The lexer drops a trailing dot, so + // detect it from the raw content, not the token stream. + qualified := strings.HasSuffix(cc.Prefix, ".") + + if !qualified && cc.Offset > 0 { + if content, err := cmp.Fh.Content(); err == nil && cc.Offset <= len(content) && content[cc.Offset-1] == '.' { + qualified = true + cc.Kind = CtxFieldValue + cc.Prefix = "" } } - if len(candidates) == 0 { - content, err := cmp.Fh.Content() - if err != nil { - return nil, rng, err - } + filterPrefix := cc.Prefix - var prefix []byte - // get prefix by pos - for i := pos.Offset - 1; i >= 0; i-- { - if unicode.IsSpace(rune(content[i])) || content[i] == '.' || content[i] == '\'' || content[i] == '"' { - prefix = content[i+1 : pos.Offset] - rng.Start.Character = rng.Start.Character - uint32(len(prefix)) + editStart := cc.EditStart + if qualified { + filterPrefix = strings.TrimRight(cc.Prefix, ".") + editStart = cc.Offset + } - break - } - } + var candidates []Candidate + for _, p := range providersFor(cc.Kind) { + candidates = append(candidates, p.Candidates(ctx, ss, cmp.Fh.URI(), cc)...) + } - if len(prefix) == 0 { - // prefix is empty, set prefix to content - prefix = content - rng.Start.Character = rng.Start.Character - uint32(len(prefix)) + // Shared pipeline: prefix filter, dedupe, sort, cap. + filtered := candidates[:0] + seen := make(map[string]struct{}, len(candidates)) + for _, cand := range candidates { + // Echo suppression: a candidate identical to the typed text adds + // nothing (the client already shows it). + if filterPrefix != "" && cand.showText == filterPrefix { + continue } - searchCandidate := func(token string, format protocol.InsertTextFormat) { - if len(token) > len(prefix) && strings.HasPrefix(token, string(prefix)) { - candidates = append(candidates, Candidate{ - showText: token, - insertText: token, - format: format, - }) - } + if !strings.HasPrefix(cand.showText, filterPrefix) { + continue } - // Semantic completion: context-aware candidates for type and - // constant value positions; fall back to keywords and all - // identifiers otherwise. - semantic := semanticCandidates(ctx, ss, cmp.Fh.URI(), parsedFile, pos) - if len(semantic) > 0 { - for _, cand := range semantic { - searchCandidate(cand.showText, cand.format) - } - } else { - for i := range keywords { - searchCandidate(i, keywords[i]) - } - - for i := range tokens { - searchCandidate(i, protocol.InsertTextFormatPlainText) - } + // Providers may yield the same name (e.g. a type defined in the + // file is both a type candidate and an identifier token). + if _, ok := seen[cand.showText]; ok { + continue } + seen[cand.showText] = struct{}{} - // Sort candidates: prefix matches first (by length, shorter first), then alphabetically - sort.Slice(candidates, func(i, j int) bool { - a, b := candidates[i].showText, candidates[j].showText - aStarts := strings.HasPrefix(a, string(prefix)) - - bStarts := strings.HasPrefix(b, string(prefix)) - if aStarts != bStarts { - return aStarts - } + filtered = append(filtered, cand) + } - if len(a) != len(b) { - return len(a) < len(b) + if qualified { + for i := range filtered { + if j := strings.LastIndex(filtered[i].showText, "."); j >= 0 { + filtered[i].insertText = filtered[i].showText[j+1:] } + } + } - return a < b - }) + sort.Slice(filtered, func(i, j int) bool { + a, b := filtered[i].showText, filtered[j].showText + aStarts := strings.HasPrefix(a, filterPrefix) - if len(candidates) > 10 { - candidates = candidates[:10] + bStarts := strings.HasPrefix(b, filterPrefix) + if aStarts != bStarts { + return aStarts } - slog.Debug("token prefix", "prefix", string(prefix), "candidates", candidates) - } + if len(a) != len(b) { + return len(a) < len(b) + } - res := make([]*CompletionItem, 0, len(candidates)) - for i := range candidates { - res = append(res, BuildCompletionItem(candidates[i])) - } + return a < b + }) - return res, rng, nil -} + truncated := false -// includeCompletion completes include path literals by listing the -// directory of the current file. -func (c *TokenCompletion) includeCompletion(ss *cache.Snapshot, file uri.URI, doc *syntax.Document, path []syntax.Node) (res []Candidate, rng protocol.Range, err error) { - if len(path) == 0 { - return res, rng, err + if len(filtered) > maxCandidates { + filtered = filtered[:maxCandidates] + truncated = true } - include, ok := path[len(path)-1].(*syntax.Include) - if !ok || include.Path == nil { - return res, rng, err - } + cursor := protocol.Position{Line: cmp.Pos.Line, Character: cmp.Pos.Character} - pathPrefix := include.Path.Text - start, end := doc.TokenRange(include.Path) - rng = 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), - }, + rng := protocol.Range{End: cursor} + if start, err := parsedFile.Mapper().OffsetToLSPPosition(editStart); err == nil { + rng.Start = protocol.Position{Line: start.Line, Character: start.Character} + } else { + rng.Start = cursor } - currentDir := filepath.Dir(file.Path()) - - slog.Debug("searching prefix in path", "prefix", pathPrefix, "dir", currentDir) - - res, err = ListDirAndFiles(currentDir, pathPrefix) - - slog.Debug("include completion", "res", res, "err", err) + res := make([]*CompletionItem, 0, len(filtered)) + for i := range filtered { + res = append(res, BuildCompletionItem(filtered[i])) + } - return res, rng, err + return res, rng, truncated, nil } diff --git a/lsp/completion/utils.go b/lsp/completion/utils.go index 0e2cbb9..4bd1c88 100644 --- a/lsp/completion/utils.go +++ b/lsp/completion/utils.go @@ -1,8 +1,7 @@ package completion import ( - "io/fs" - "log/slog" + "os" "path/filepath" "strings" @@ -11,61 +10,71 @@ import ( "github.com/karitham/thrift-ls/lsp/constants" ) -func ListDirAndFiles(dir, prefix string) (res []Candidate, err error) { - // handle prefix list ../../us - prefixClean := prefix - if len(prefix) > 0 { - prefixClean = filepath.Clean(prefix) - } +// ListDirAndFiles lists the entries matching the typed include path prefix, +// one level deep, under the current file's directory and every configured +// include path root. Directories are returned with a trailing slash; only +// .thrift files are returned. Results are deduplicated across roots. +func ListDirAndFiles(dir string, includePaths []string, prefix string) []Candidate { + prefix = strings.Trim(prefix, "'\"") - if prefix == "." { - prefix = prefix + "/" + roots := make([]string, 0, 1+len(includePaths)) + if dir != "" { + roots = append(roots, dir) } - up := strings.Count(prefixClean, "../") + for _, p := range includePaths { + if p != "" { + roots = append(roots, p) + } + } - pathItems := strings.Split(dir, "/") - if len(pathItems) < up { - return res, err + // Split the typed prefix into the directory part (resolved per root) + // and the file prefix. + dirPart, filePrefix := "", prefix + if i := strings.LastIndex(prefix, "/"); i >= 0 { + dirPart, filePrefix = prefix[:i+1], prefix[i+1:] } - pathItems = pathItems[0 : len(pathItems)-up] + seen := make(map[string]struct{}) - dir, filePrefix := filepath.Split(strings.TrimPrefix(prefixClean, "../")) - filePrefix = strings.TrimPrefix(filePrefix, "./") - baseDir := strings.Join(pathItems, "/") + "/" + dir - prefix = strings.TrimSuffix(prefix, filePrefix) + var res []Candidate - slog.Debug("include completion walk dir", "dir", baseDir, "prefix", prefix, "filePrefix", filePrefix) - _ = filepath.WalkDir(baseDir, func(path string, d fs.DirEntry, err error) error { - if err != nil || baseDir == path { - return nil + for _, root := range roots { + entries, err := os.ReadDir(filepath.Join(root, dirPart)) + if err != nil { + continue } - slog.Debug("include completion name", "name", d.Name(), "prefix", filePrefix) - - if strings.HasPrefix(d.Name(), filePrefix) { - if d.IsDir() { - res = append(res, Candidate{ - showText: prefix + d.Name() + "/", - insertText: prefix + d.Name() + "/", - format: protocol.InsertTextFormatPlainText, - }) - } else if strings.HasSuffix(d.Name(), constants.ThriftExtension) { - res = append(res, Candidate{ - showText: prefix + d.Name(), - insertText: prefix + d.Name(), - format: protocol.InsertTextFormatPlainText, - }) + for _, e := range entries { + name := e.Name() + if !strings.HasPrefix(name, filePrefix) { + continue } - } - if d.IsDir() { - return filepath.SkipDir - } + var text string + + switch { + case e.IsDir(): + text = filepath.Join(dirPart, name) + "/" + case strings.HasSuffix(name, constants.ThriftExtension): + text = filepath.Join(dirPart, name) + default: + continue + } + + if _, ok := seen[text]; ok { + continue + } - return nil - }) + seen[text] = struct{}{} + + res = append(res, Candidate{ + showText: text, + insertText: text, + format: protocol.InsertTextFormatPlainText, + }) + } + } - return res, err + return res } diff --git a/lsp/diagnostic.go b/lsp/diagnostic.go index 26b4b07..816982c 100644 --- a/lsp/diagnostic.go +++ b/lsp/diagnostic.go @@ -12,7 +12,7 @@ import ( "github.com/karitham/thrift-ls/lsp/diagnostic" ) -func (s *Server) diagnostic(ctx context.Context, ss *cache.Snapshot, changeFile *cache.FileChange) error { +func (s *Server) diagnostic(ctx context.Context, ss *cache.Snapshot, file uri.URI) error { if s.client == nil { return nil } @@ -22,7 +22,7 @@ func (s *Server) diagnostic(ctx context.Context, ss *cache.Snapshot, changeFile diag := diagnostic.NewDiagnostic() - diagRes, err := diag.Diagnostic(ctx, ss, []uri.URI{changeFile.URI}) + diagRes, err := diag.Diagnostic(ctx, ss, []uri.URI{file}) if err != nil { slog.Error("diagnostic failed", "err", err) } diff --git a/lsp/diagnostic/cycle_detect_test.go b/lsp/diagnostic/cycle_detect_test.go index 97e1edf..f2ab98e 100644 --- a/lsp/diagnostic/cycle_detect_test.go +++ b/lsp/diagnostic/cycle_detect_test.go @@ -9,7 +9,6 @@ import ( "go.lsp.dev/uri" "github.com/karitham/thrift-ls/lsp/cache" - "github.com/karitham/thrift-ls/lsp/memoize" "github.com/karitham/thrift-ls/syntax" ) @@ -175,13 +174,12 @@ include "./test/address.thrift"` } func buildSnapshotForTest(t *testing.T, files []*cache.FileChange) *cache.Snapshot { - store := &memoize.Store{} - c := cache.New(store, nil) + c := cache.New(nil) fs := cache.NewOverlayFS(c) _ = fs.Update(t.Context(), files) - view := cache.NewView("test", "file:///tmp", fs, store, nil) - ss := cache.NewSnapshot(view, store, nil) + view := cache.NewView("test", "file:///tmp", fs, nil) + ss := cache.NewSnapshot(view, nil) return ss } diff --git a/lsp/didchange_test.go b/lsp/didchange_test.go new file mode 100644 index 0000000..7280c46 --- /dev/null +++ b/lsp/didchange_test.go @@ -0,0 +1,292 @@ +package lsp + +import ( + "context" + "os" + "path/filepath" + "sync" + "testing" + "testing/synctest" + + "github.com/stretchr/testify/assert" + "go.lsp.dev/protocol" + "go.lsp.dev/uri" + + "github.com/karitham/thrift-ls/formatter" + "github.com/karitham/thrift-ls/lsp/cache" +) + +// recordingClient records PublishDiagnostics calls per URI; every other +// client method is a no-op. +type recordingClient struct { + protocol.Client + + mu sync.Mutex + published map[uri.URI]int +} + +func (c *recordingClient) PublishDiagnostics(ctx context.Context, params *protocol.PublishDiagnosticsParams) error { + c.mu.Lock() + defer c.mu.Unlock() + + if c.published == nil { + c.published = make(map[uri.URI]int) + } + + c.published[params.URI]++ + + return nil +} + +func (c *recordingClient) reset() { + c.mu.Lock() + defer c.mu.Unlock() + + c.published = nil +} + +func (c *recordingClient) count(file uri.URI) int { + c.mu.Lock() + defer c.mu.Unlock() + + return c.published[file] +} + +func newTestServer(client protocol.Client) *Server { + return NewServer(cache.New(nil), client, formatter.Options{}) +} + +func writeFile(t *testing.T, path, content string) { + t.Helper() + + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} + +func openDocument(t *testing.T, srv *Server, fileURI uri.URI, content string) { + t.Helper() + + err := srv.DidOpen(t.Context(), &protocol.DidOpenTextDocumentParams{ + TextDocument: protocol.TextDocumentItem{ + URI: fileURI, + LanguageID: LanguageIDThrift, + Version: 0, + Text: content, + }, + }) + assert.NoError(t, err) +} + +// Test_DidChangeRepublishesDependentsDiagnostics is the user-visible +// behavior: editing federation.gundam re-publishes diagnostics for +// strike_rouge, which includes it. Diagnostics publish asynchronously, so +// the test runs in a synctest bubble and waits for the goroutines. +func Test_DidChangeRepublishesDependentsDiagnostics(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "federation.gundam.thrift"), `struct Gundam { + 1: required string Name +}`) + writeFile(t, filepath.Join(dir, "strike_rouge.thrift"), `include "federation.gundam.thrift" + +exception BayFull { + 1: string message +}`) + + client := &recordingClient{} + srv := newTestServer(client) + + aURI := uri.File(filepath.Join(dir, "strike_rouge.thrift")) + bURI := uri.File(filepath.Join(dir, "federation.gundam.thrift")) + + openDocument(t, srv, aURI, `include "federation.gundam.thrift" + +exception BayFull { + 1: string message +}`) + openDocument(t, srv, bURI, `struct Gundam { + 1: required string Name +}`) + + client.reset() + + err := srv.DidChange(t.Context(), &protocol.DidChangeTextDocumentParams{ + TextDocument: protocol.VersionedTextDocumentIdentifier{ + TextDocumentIdentifier: protocol.TextDocumentIdentifier{URI: bURI}, + Version: 1, + }, + ContentChanges: []protocol.TextDocumentContentChangeEvent{ + &protocol.TextDocumentContentChangeWholeDocument{ + Text: `struct Gundam { + 1: required string Name, + 2: optional i32 SerialNumber +}`, + }, + }, + }) + assert.NoError(t, err) + + // Let the asynchronous diagnostics goroutine finish. + synctest.Wait() + + assert.GreaterOrEqual(t, client.count(bURI), 1, "changed file gets diagnostics") + assert.GreaterOrEqual(t, client.count(aURI), 1, "dependent of changed file gets diagnostics") + }) +} + +// Test_DidCloseDropsOverlay: closing a document removes its overlay; content +// falls back to disk and dependents keep their include edges. +func Test_DidCloseDropsOverlay(t *testing.T) { + dir := t.TempDir() + bDisk := "struct Gundam {\n\t1: required string Name\n}" + bOverlay := "struct Gundam {\n\t1: required string Name,\n\t2: optional i32 SerialNumber\n}" + + writeFile(t, filepath.Join(dir, "federation.gundam.thrift"), bDisk) + writeFile(t, filepath.Join(dir, "strike_rouge.thrift"), `include "federation.gundam.thrift" + +exception BayFull { + 1: string message +}`) + + srv := newTestServer(nil) + + aURI := uri.File(filepath.Join(dir, "strike_rouge.thrift")) + bURI := uri.File(filepath.Join(dir, "federation.gundam.thrift")) + + openDocument(t, srv, aURI, `include "federation.gundam.thrift" + +exception BayFull { + 1: string message +}`) + openDocument(t, srv, bURI, bOverlay) + + ctx := t.Context() + + fh, err := srv.session.ReadFile(ctx, bURI) + assert.NoError(t, err) + content, err := fh.Content() + assert.NoError(t, err) + assert.Equal(t, bOverlay, string(content)) + + err = srv.DidClose(ctx, &protocol.DidCloseTextDocumentParams{ + TextDocument: protocol.TextDocumentIdentifier{URI: bURI}, + }) + assert.NoError(t, err) + + // the overlay is gone: reads fall back to disk content + fh, err = srv.session.ReadFile(ctx, bURI) + assert.NoError(t, err) + content, err = fh.Content() + assert.NoError(t, err) + assert.Equal(t, bDisk, string(content)) + + // the dependent still parses and keeps its include edge + view, err := srv.session.ViewOf(aURI) + assert.NoError(t, err) + + ss, release := view.Snapshot() + defer release() + + _, err = ss.Parse(ctx, aURI) + assert.NoError(t, err) + assert.Equal(t, []uri.URI{aURI}, ss.Dependents(bURI)) +} + +// Test_DidChangeWatchedFilesRefreshesDiskContent: disk events outside the +// editor (git pull, other editors) refresh content and invalidate dependents. +func Test_DidChangeWatchedFilesRefreshesDiskContent(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "federation.gundam.thrift"), `struct Gundam { + 1: required string Name +}`) + writeFile(t, filepath.Join(dir, "strike_rouge.thrift"), `include "federation.gundam.thrift" + +exception BayFull { + 1: string message +}`) + + client := &recordingClient{} + srv := newTestServer(client) + + aURI := uri.File(filepath.Join(dir, "strike_rouge.thrift")) + bURI := uri.File(filepath.Join(dir, "federation.gundam.thrift")) + + openDocument(t, srv, aURI, `include "federation.gundam.thrift" + +exception BayFull { + 1: string message +}`) + + // external edit, e.g. git pull + writeFile(t, filepath.Join(dir, "federation.gundam.thrift"), `struct Gundam { + 1: required string Name, + 2: optional i32 SerialNumber +}`) + + client.reset() + + err := srv.DidChangeWatchedFiles(t.Context(), &protocol.DidChangeWatchedFilesParams{ + Changes: []protocol.FileEvent{ + {URI: bURI, Type: protocol.FileChangeTypeChanged}, + }, + }) + assert.NoError(t, err) + + // Let the asynchronous diagnostics goroutine finish. + synctest.Wait() + + assert.GreaterOrEqual(t, client.count(bURI), 1, "changed file gets diagnostics") + assert.GreaterOrEqual(t, client.count(aURI), 1, "dependent of changed file gets diagnostics") + + // the refreshed content is visible through the session + fh, err := srv.session.ReadFile(t.Context(), bURI) + assert.NoError(t, err) + content, err := fh.Content() + assert.NoError(t, err) + assert.Contains(t, string(content), "SerialNumber") + }) +} + +// Test_DidChangeWatchedFilesIgnoresOpenFiles: disk events for open documents +// are ignored, the overlay stays authoritative. +func Test_DidChangeWatchedFilesIgnoresOpenFiles(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "federation.gundam.thrift"), `struct Gundam { + 1: required string Name +}`) + writeFile(t, filepath.Join(dir, "strike_rouge.thrift"), `include "federation.gundam.thrift" + +exception BayFull { + 1: string message +}`) + + srv := newTestServer(nil) + + bURI := uri.File(filepath.Join(dir, "federation.gundam.thrift")) + + overlayContent := `struct Gundam { + 1: required string Name, + 2: optional i32 SerialNumber +}` + openDocument(t, srv, bURI, overlayContent) + + // disk differs from the overlay + writeFile(t, filepath.Join(dir, "federation.gundam.thrift"), `struct Gundam { + 1: required string Name +}`) + + err := srv.DidChangeWatchedFiles(t.Context(), &protocol.DidChangeWatchedFilesParams{ + Changes: []protocol.FileEvent{ + {URI: bURI, Type: protocol.FileChangeTypeChanged}, + }, + }) + assert.NoError(t, err) + + fh, err := srv.session.ReadFile(t.Context(), bURI) + assert.NoError(t, err) + content, err := fh.Content() + assert.NoError(t, err) + assert.Equal(t, overlayContent, string(content)) +} diff --git a/lsp/format_range_server_test.go b/lsp/format_range_server_test.go index d54515f..5c7c368 100644 --- a/lsp/format_range_server_test.go +++ b/lsp/format_range_server_test.go @@ -10,7 +10,6 @@ import ( "github.com/karitham/thrift-ls/formatter" "github.com/karitham/thrift-ls/lsp/cache" - "github.com/karitham/thrift-ls/lsp/memoize" ) func TestServerRangeFormatting(t *testing.T) { @@ -28,8 +27,7 @@ struct B { struct C { 3: i64 c } ` - store := &memoize.Store{} - srv := NewServer(cache.New(store, nil), nil, formatter.Options{}) + srv := NewServer(cache.New(nil), nil, formatter.Options{}) err = srv.DidOpen(ctx, &protocol.DidOpenTextDocumentParams{ TextDocument: protocol.TextDocumentItem{ diff --git a/lsp/impl.go b/lsp/impl.go index 202cf50..65d2ba3 100644 --- a/lsp/impl.go +++ b/lsp/impl.go @@ -59,15 +59,7 @@ func (s *Server) openFile(ctx context.Context, change *cache.FileChange) error { } view, _ := s.session.ViewOf(change.URI) - view.FileChange(ctx, []*cache.FileChange{change}, func() { - ss, release := view.Snapshot() - defer release() - - err := s.diagnostic(ctx, ss, change) - if err != nil { - slog.Error("diagnostic error", "err", err) - } - }) + view.FileChange(ctx, []*cache.FileChange{change}, s.postDiagnostics(ctx, view)) return nil } @@ -86,19 +78,115 @@ func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDo return err } - view.FileChange(ctx, changes, func() { + view.FileChange(ctx, changes, s.postDiagnostics(ctx, view)) + + return nil +} + +func (s *Server) didClose(ctx context.Context, params *protocol.DidCloseTextDocumentParams) error { + fileURI := params.TextDocument.URI + + view, err := s.session.ViewOf(fileURI) + if err != nil { + return err + } + + change := &cache.FileChange{URI: fileURI, From: cache.FileChangeTypeDidClose} + + if err := s.session.UpdateOverlayFS(ctx, []*cache.FileChange{change}); err != nil { + return err + } + + view.FileChange(ctx, []*cache.FileChange{change}, s.postDiagnostics(ctx, view)) + + return nil +} + +func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.DidChangeWatchedFilesParams) error { + byView := make(map[*cache.View][]*cache.FileChange) + + for _, event := range params.Changes { + if s.session.HasOverlay(event.URI) { + // The editor overlay is authoritative for open documents; disk + // events for them are ignored. + continue + } + + change, err := s.watchedFileChange(ctx, event) + if err != nil { + return err + } + + view, err := s.session.ViewOf(event.URI) + if err != nil { + continue + } + + byView[view] = append(byView[view], change) + } + + for view, changes := range byView { + view.FileChange(ctx, changes, s.postDiagnostics(ctx, view)) + } + + return nil +} + +// watchedFileChange builds a FileChange from a disk event, reading the +// current content through the memoized file source. Deleted files are +// reported as a close change. +func (s *Server) watchedFileChange(ctx context.Context, event protocol.FileEvent) (*cache.FileChange, error) { + if event.Type == protocol.FileChangeTypeDeleted { + return &cache.FileChange{URI: event.URI, From: cache.FileChangeTypeDidClose}, nil + } + + fh, err := s.session.ReadFile(ctx, event.URI) + if err != nil { + return nil, err + } + + content, err := fh.Content() + if err != nil { + return nil, err + } + + return &cache.FileChange{ + URI: event.URI, + Version: int(fh.Version()), + Content: content, + From: cache.FileChangeTypeDidChange, + }, nil +} + +// postDiagnostics returns a FileChange postFn that publishes diagnostics for +// every affected file (changed files plus their transitive dependents) on the +// view's current snapshot. Diagnostics run in the background (FileChange +// invokes postFns asynchronously); if a newer change lands while the analysis +// runs, the results are dropped — the newer change publishes its own. +func (s *Server) postDiagnostics(ctx context.Context, view *cache.View) func([]uri.URI) { + // The request context dies when the LSP request returns; the + // diagnostics goroutine outlives it. + ctx = context.WithoutCancel(ctx) + + return func(affected []uri.URI) { ss, release := view.Snapshot() defer release() - for i := range changes { - err := s.diagnostic(ctx, ss, changes[i]) - if err != nil { - slog.Error("diagnostic error", "err", err) - } + if !view.IsCurrent(ss) { + return } - }) - return nil + s.diagnose(ctx, ss, affected) + } +} + +// diagnose publishes diagnostics for every affected file. +func (s *Server) diagnose(ctx context.Context, ss *cache.Snapshot, affected []uri.URI) { + for i := range affected { + if err := s.diagnostic(ctx, ss, affected[i]); err != nil { + slog.Error("diagnostic error", "err", err) + } + } } func (s *Server) completion(ctx context.Context, params *protocol.CompletionParams) (*protocol.CompletionList, error) { @@ -108,7 +196,7 @@ func (s *Server) completion(ctx context.Context, params *protocol.CompletionPara } defer release() - items, rng, err := completion.DefaultTokenCompletion.Completion(ctx, snapshot, &completion.CompletionRequest{ + items, rng, truncated, err := completion.DefaultTokenCompletion.Completion(ctx, snapshot, &completion.CompletionRequest{ TriggerKind: 0, Pos: types.Position{ Line: params.Position.Line, @@ -120,12 +208,12 @@ func (s *Server) completion(ctx context.Context, params *protocol.CompletionPara return nil, err } - return toLspCompletionList(items, rng), nil + return toLspCompletionList(items, rng, truncated), nil } -func toLspCompletionList(items []*completion.CompletionItem, rng protocol.Range) *protocol.CompletionList { +func toLspCompletionList(items []*completion.CompletionItem, rng protocol.Range, truncated bool) *protocol.CompletionList { list := &protocol.CompletionList{ - IsIncomplete: true, + IsIncomplete: truncated, } for i := range items { diff --git a/lsp/impl_test.go b/lsp/impl_test.go index 75a5f44..15c609c 100644 --- a/lsp/impl_test.go +++ b/lsp/impl_test.go @@ -9,7 +9,6 @@ import ( "github.com/karitham/thrift-ls/formatter" "github.com/karitham/thrift-ls/lsp/cache" - "github.com/karitham/thrift-ls/lsp/memoize" ) func Test_DidOpen(t *testing.T) { @@ -33,8 +32,7 @@ struct Test { }, } - store := &memoize.Store{} - cache := cache.New(store, nil) + cache := cache.New(nil) srv := NewServer(cache, nil, formatter.Options{}) err = srv.DidOpen(ctx, params) assert.NoError(t, err) @@ -92,8 +90,7 @@ struct Test { }, } - store := &memoize.Store{} - cache := cache.New(store, nil) + cache := cache.New(nil) srv := NewServer(cache, nil, formatter.Options{}) err = srv.DidOpen(ctx, openParams) @@ -155,8 +152,7 @@ struct Test { }, } - store := &memoize.Store{} - cache := cache.New(store, nil) + cache := cache.New(nil) srv := NewServer(cache, nil, formatter.Options{}) err = srv.DidOpen(ctx, openParams) assert.NoError(t, err) @@ -253,8 +249,7 @@ struct Test { }, } - store := &memoize.Store{} - cache := cache.New(store, []string{"/tmp"}) + cache := cache.New([]string{"/tmp"}) srv := NewServer(cache, nil, formatter.Options{}) err = srv.DidOpen(ctx, baseParams) @@ -360,8 +355,7 @@ struct Other { }, } - store := &memoize.Store{} - cache := cache.New(store, nil) + cache := cache.New(nil) srv := NewServer(cache, nil, formatter.Options{}) err = srv.DidOpen(ctx, file1Params) diff --git a/lsp/include_paths_test.go b/lsp/include_paths_test.go index 39e4c5d..0bfa092 100644 --- a/lsp/include_paths_test.go +++ b/lsp/include_paths_test.go @@ -10,7 +10,6 @@ import ( "github.com/karitham/thrift-ls/formatter" "github.com/karitham/thrift-ls/lsp/cache" - "github.com/karitham/thrift-ls/lsp/memoize" ) // TestServerIncludePathsFlow verifies that include paths configured on the @@ -23,7 +22,7 @@ func TestServerIncludePathsFlow(t *testing.T) { shared := filepath.Join(includeDir, "shared.thrift") assert.NoError(t, os.WriteFile(shared, []byte("struct Shared {}"), 0o644)) - c := cache.New(&memoize.Store{}, []string{includeDir}) + c := cache.New([]string{includeDir}) srv := NewServer(c, nil, formatter.DefaultOptions()) // Views are created per workspace folder at initialization. diff --git a/lsp/initialize.go b/lsp/initialize.go index 0e5fb82..b27a1f4 100644 --- a/lsp/initialize.go +++ b/lsp/initialize.go @@ -119,7 +119,9 @@ func initializeResult() *protocol.InitializeResult { * present `console` besides others as a completion item. Characters that * make up identifiers don't need to be listed here. */ - TriggerCharacters: []string{"."}, + // "." for enum-qualified values (Color.|), "\"" for include + // path literals, "(" for annotation keys. + TriggerCharacters: []string{".", "\"", "("}, }, HoverProvider: &protocol.HoverOptions{ WorkDoneProgressOptions: protocol.WorkDoneProgressOptions{ diff --git a/lsp/mapper/mapper.go b/lsp/mapper/mapper.go index 6d7fa14..25dfd91 100644 --- a/lsp/mapper/mapper.go +++ b/lsp/mapper/mapper.go @@ -48,16 +48,17 @@ func (m *Mapper) initLineStart() { }) } +// GetLSPEndPosition returns the position immediately after the last +// character of the document: the last line (0-based) at the UTF-16 length +// of its content. A document ending with a newline has an empty last line. func (m *Mapper) GetLSPEndPosition() types.Position { m.initLineStart() lastLineStart := m.lineStart[len(m.lineStart)-1] lastLine := m.content[lastLineStart:] - utf16Len := utf16Count(lastLine) - return types.Position{ - Line: uint32(len(m.lineStart)), - Character: uint32(utf16Len) - 1, + Line: uint32(len(m.lineStart) - 1), + Character: uint32(utf16Count(lastLine)), } } diff --git a/lsp/mapper/mapper_test.go b/lsp/mapper/mapper_test.go index a23503f..9610553 100644 --- a/lsp/mapper/mapper_test.go +++ b/lsp/mapper/mapper_test.go @@ -213,3 +213,38 @@ func Test_utf16Count(t *testing.T) { }) } } + +func TestGetLSPEndPosition(t *testing.T) { + tests := []struct { + name string + content string + want types.Position + }{ + { + name: "single line", + content: "struct Gundam {}", + want: types.Position{Line: 0, Character: 16}, + }, + { + name: "multiline without trailing newline", + content: "enum ZeonForces {\n ZAKU_I\n}", + want: types.Position{Line: 2, Character: 1}, + }, + { + name: "trailing newline has an empty last line", + content: "struct Gundam {\n}\n", + want: types.Position{Line: 2, Character: 0}, + }, + { + name: "non-ascii on the last line", + content: `const string s = "モビルスーツ"`, + want: types.Position{Line: 0, Character: 25}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := NewMapper("file:///tmp/test.thrift", []byte(tt.content)) + assert.Equal(t, tt.want, m.GetLSPEndPosition()) + }) + } +} diff --git a/lsp/memoize/promise.go b/lsp/memoize/promise.go deleted file mode 100644 index 6da5fdb..0000000 --- a/lsp/memoize/promise.go +++ /dev/null @@ -1,229 +0,0 @@ -package memoize - -import ( - "context" - "fmt" - "runtime/trace" - "sync" - - "go.lsp.dev/pkg/xcontext" -) - -// Function is the type of a function that can be memoized. -// -// If the arg is a RefCounted, its Acquire/Release operations are called. -// -// The argument must not materially affect the result of the function -// in ways that are not captured by the promise's key, since if -// Promise.Get is called twice concurrently, with the same (implicit) -// key but different arguments, the Function is called only once but -// its result must be suitable for both callers. -// -// The main purpose of the argument is to avoid the Function closure -// needing to retain large objects (in practice: the snapshot) in -// memory that can be supplied at call time by any caller. -type Function func(ctx context.Context, arg any) any - -// A RefCounted is a value whose functional lifetime is determined by -// reference counting. -// -// Its Acquire method is called before the Function is invoked, and -// the corresponding release is called when the Function returns. -// Usually both events happen within a single call to Get, so Get -// would be fine with a "borrowed" reference, but if the context is -// cancelled, Get may return before the Function is complete, causing -// the argument to escape, and potential premature destruction of the -// value. For a reference-counted type, this requires a pair of -// increment/decrement operations to extend its life. -type RefCounted interface { - // Acquire prevents the value from being destroyed until the - // returned function is called. - Acquire() func() -} - -// A Promise represents the future result of a call to a function. -type Promise struct { - debug string // for observability - - // refcount is the reference count in the containing Store, used by - // Store.Promise. It is guarded by Store.promisesMu on the containing Store. - refcount int32 - - mu sync.Mutex - - // A Promise starts out IDLE, waiting for something to demand - // its evaluation. It then transitions into RUNNING state. - // - // While RUNNING, waiters tracks the number of Get calls - // waiting for a result, and the done channel is used to - // notify waiters of the next state transition. Once - // evaluation finishes, value is set, state changes to - // COMPLETED, and done is closed, unblocking waiters. - // - // Alternatively, as Get calls are cancelled, they decrement - // waiters. If it drops to zero, the inner context is - // cancelled, computation is abandoned, and state resets to - // IDLE to start the process over again. - state state - // done is set in running state, and closed when exiting it. - done chan struct{} - // cancel is set in running state. It cancels computation. - cancel context.CancelFunc - // waiters is the number of Gets outstanding. - waiters uint - // the function that will be used to populate the value - function Function - // value is set in completed state. - value any -} - -// NewPromise returns a promise for the future result of calling the -// specified function. -// -// The debug string is used to classify promises in logs and metrics. -// It should be drawn from a small set. -func NewPromise(debug string, function Function) *Promise { - if function == nil { - panic("nil function") - } - - return &Promise{ - debug: debug, - function: function, - } -} - -type state int - -const ( - stateIdle = iota // newly constructed, or last waiter was cancelled - stateRunning // start was called and not cancelled - stateCompleted // function call ran to completion -) - -// Cached returns the value associated with a promise. -// -// It will never cause the value to be generated. -// It will return the cached value, if present. -func (p *Promise) Cached() any { - p.mu.Lock() - defer p.mu.Unlock() - - if p.state == stateCompleted { - return p.value - } - - return nil -} - -// Get returns the value associated with a promise. -// -// All calls to Promise.Get on a given promise return the -// same result but the function is called (to completion) at most once. -// -// If the value is not yet ready, the underlying function will be invoked. -// -// If ctx is cancelled, Get returns (nil, Canceled). -// If all concurrent calls to Get are cancelled, the context provided -// to the function is cancelled. A later call to Get may attempt to -// call the function again. -func (p *Promise) Get(ctx context.Context, arg any) (any, error) { - if ctx.Err() != nil { - return nil, ctx.Err() - } - - p.mu.Lock() - switch p.state { - case stateIdle: - return p.run(ctx, arg) - case stateRunning: - return p.wait(ctx) - case stateCompleted: - defer p.mu.Unlock() - - return p.value, nil - default: - panic("unknown state") - } -} - -// run starts p.function and returns the result. p.mu must be locked. -func (p *Promise) run(ctx context.Context, arg any) (any, error) { - childCtx, cancel := context.WithCancel(xcontext.Detach(ctx)) - p.cancel = cancel - p.state = stateRunning - p.done = make(chan struct{}) - function := p.function // Read under the lock - - // Make sure that the argument isn't destroyed while we're running in it. - release := func() {} - if rc, ok := arg.(RefCounted); ok { - release = rc.Acquire() - } - - go func() { - trace.WithRegion(childCtx, fmt.Sprintf("Promise.run %s", p.debug), func() { - defer release() - // Just in case the function does something expensive without checking - // the context, double-check we're still alive. - if childCtx.Err() != nil { - return - } - - v := function(childCtx, arg) - if childCtx.Err() != nil { - return - } - - p.mu.Lock() - defer p.mu.Unlock() - // It's theoretically possible that the promise has been cancelled out - // of the run that started us, and then started running again since we - // checked childCtx above. Even so, that should be harmless, since each - // run should produce the same results. - if p.state != stateRunning { - return - } - - p.value = v - p.function = nil // aid GC - p.state = stateCompleted - close(p.done) - }) - }() - - return p.wait(ctx) -} - -// wait waits for the value to be computed, or ctx to be cancelled. p.mu must be locked. -func (p *Promise) wait(ctx context.Context) (any, error) { - p.waiters++ - done := p.done - p.mu.Unlock() - - select { - case <-done: - p.mu.Lock() - defer p.mu.Unlock() - - if p.state == stateCompleted { - return p.value, nil - } - - return nil, nil - case <-ctx.Done(): - p.mu.Lock() - defer p.mu.Unlock() - - p.waiters-- - if p.waiters == 0 && p.state == stateRunning { - p.cancel() - close(p.done) - p.state = stateIdle - p.done = nil - p.cancel = nil - } - - return nil, ctx.Err() - } -} diff --git a/lsp/memoize/store.go b/lsp/memoize/store.go deleted file mode 100644 index 5bb3cb0..0000000 --- a/lsp/memoize/store.go +++ /dev/null @@ -1,101 +0,0 @@ -package memoize - -import ( - "reflect" - "sync" - "sync/atomic" -) - -// An EvictionPolicy controls the eviction behavior of keys in a Store when -// they no longer have any references. -type EvictionPolicy int - -const ( - // ImmediatelyEvict evicts keys as soon as they no longer have references. - ImmediatelyEvict EvictionPolicy = iota - - // NeverEvict does not evict keys. - NeverEvict -) - -type Store struct { - evictionPolicy EvictionPolicy - - promisesMu sync.Mutex - promises map[any]*Promise -} - -// Promise returns a reference-counted promise for the future result of -// calling the specified function. -// -// Calls to Promise with the same key return the same promise, incrementing its -// reference count. The caller must call the returned function to decrement -// the promise's reference count when it is no longer needed. The returned -// function must not be called more than once. -// -// Once the last reference has been released, the promise is removed from the -// store. -func (store *Store) Promise(key any, function Function) (*Promise, func()) { - store.promisesMu.Lock() - - p, ok := store.promises[key] - if !ok { - p = NewPromise(reflect.TypeOf(key).String(), function) - - if store.promises == nil { - store.promises = map[any]*Promise{} - } - - store.promises[key] = p - } - - p.refcount++ - store.promisesMu.Unlock() - - var released atomic.Int32 - - release := func() { - if !released.CompareAndSwap(0, 1) { - panic("release called more than once") - } - - store.promisesMu.Lock() - - p.refcount-- - if p.refcount == 0 && store.evictionPolicy != NeverEvict { - // Inv: if p.refcount > 0, then store.promises[key] == p. - delete(store.promises, key) - } - store.promisesMu.Unlock() - } - - return p, release -} - -// Stats returns the number of each type of key in the store. -func (s *Store) Stats() map[reflect.Type]int { - result := map[reflect.Type]int{} - - s.promisesMu.Lock() - defer s.promisesMu.Unlock() - - for k := range s.promises { - result[reflect.TypeOf(k)]++ - } - - return result -} - -// DebugOnlyIterate iterates through the store and, for each completed -// promise, calls f(k, v) for the map key k and function result v. It -// should only be used for debugging purposes. -func (s *Store) DebugOnlyIterate(f func(k, v any)) { - s.promisesMu.Lock() - defer s.promisesMu.Unlock() - - for k, p := range s.promises { - if v := p.Cached(); v != nil { - f(k, v) - } - } -} diff --git a/lsp/server.go b/lsp/server.go index ffb4c17..cf8cbe5 100644 --- a/lsp/server.go +++ b/lsp/server.go @@ -110,7 +110,7 @@ func (s *Server) DidChangeConfiguration(ctx context.Context, params *protocol.Di } func (s *Server) DidChangeWatchedFiles(ctx context.Context, params *protocol.DidChangeWatchedFilesParams) (err error) { - return nil + return s.didChangeWatchedFiles(ctx, params) } func (s *Server) DidChangeWorkspaceFolders(ctx context.Context, params *protocol.DidChangeWorkspaceFoldersParams) (err error) { @@ -118,7 +118,7 @@ func (s *Server) DidChangeWorkspaceFolders(ctx context.Context, params *protocol } func (s *Server) DidClose(ctx context.Context, params *protocol.DidCloseTextDocumentParams) (err error) { - return nil + return s.didClose(ctx, params) } func (s *Server) DidOpen(ctx context.Context, params *protocol.DidOpenTextDocumentParams) (err error) { diff --git a/lsp/stream.go b/lsp/stream.go index 6e47253..c9d40af 100644 --- a/lsp/stream.go +++ b/lsp/stream.go @@ -8,7 +8,6 @@ import ( "github.com/karitham/thrift-ls/formatter" "github.com/karitham/thrift-ls/lsp/cache" - "github.com/karitham/thrift-ls/lsp/memoize" ) type StreamServer struct { @@ -22,10 +21,8 @@ type Options struct { } func NewStreamServer(opts *Options) *StreamServer { - store := &memoize.Store{} - return &StreamServer{ - cache: cache.New(store, opts.IncludePaths), + cache: cache.New(opts.IncludePaths), formatOpts: opts.Format, } }