diff --git a/CHANGELOG.md b/CHANGELOG.md index d12f5bb901..f8791c43bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ - Fix `buf format` dropping comments next to commas or semicolons in message literals. - Fix compilation failing to resolve symbols re-exported through `import public` when the re-exporting file also reaches those symbols through a non-public import. +- Fix LSP go to definition not resolving across files inside dependency and well-known-type + files, and release workspaces when their last open file closes to free resources. ## [v1.72.0] - 2026-07-17 diff --git a/private/buf/buflsp/buf_gen_yaml_lsp_test.go b/private/buf/buflsp/buf_gen_yaml_lsp_test.go index 7c6db7ef8c..fd86e6290f 100644 --- a/private/buf/buflsp/buf_gen_yaml_lsp_test.go +++ b/private/buf/buflsp/buf_gen_yaml_lsp_test.go @@ -736,7 +736,7 @@ func TestBufGenYAMLCheckPluginUpdates(t *testing.T) { ctx := t.Context() var result any - _, err = clientJSONConn.Call(ctx, protocol.MethodWorkspaceExecuteCommand, &protocol.ExecuteCommandParams{ + _, err := clientJSONConn.Call(ctx, protocol.MethodWorkspaceExecuteCommand, &protocol.ExecuteCommandParams{ Command: buflsp.CommandCheckPluginUpdates, Arguments: []any{string(bufGenYAMLURI)}, }, &result) diff --git a/private/buf/buflsp/buf_yaml_lsp_test.go b/private/buf/buflsp/buf_yaml_lsp_test.go index 6e72ddba53..8a4d3ca595 100644 --- a/private/buf/buflsp/buf_yaml_lsp_test.go +++ b/private/buf/buflsp/buf_yaml_lsp_test.go @@ -352,7 +352,7 @@ func TestBufYAMLCheckUpdates(t *testing.T) { ctx := t.Context() var result any - _, err = clientJSONConn.Call(ctx, protocol.MethodWorkspaceExecuteCommand, &protocol.ExecuteCommandParams{ + _, err := clientJSONConn.Call(ctx, protocol.MethodWorkspaceExecuteCommand, &protocol.ExecuteCommandParams{ Command: "buf.dep.checkUpdates", Arguments: []any{string(bufYAMLURI)}, }, &result) diff --git a/private/buf/buflsp/file.go b/private/buf/buflsp/file.go index 3ee402f47f..9d41d2a550 100644 --- a/private/buf/buflsp/file.go +++ b/private/buf/buflsp/file.go @@ -94,10 +94,7 @@ func (f *file) Manager() *fileManager { // Reset clears all bookkeeping information on this file and resets it. func (f *file) Reset(ctx context.Context) { f.lsp.logger.DebugContext(ctx, "resetting file", slog.String("uri", f.uri.Filename())) - if f.workspace != nil { - f.workspace.Release() - f.workspace = nil - } + f.releaseWorkspace() // Evict the query key if there is a query cached on the file. We cache the [queries.File] // query since this allows the executor to evict all dependent queries, e.g. AST and IR. f.lsp.queryExecutor.Evict(f.queryFileKeys()...) @@ -106,12 +103,13 @@ func (f *file) Reset(ctx context.Context) { *f = file{} } -// Close marks a file as closed by the editor. It clears the editor state -// (cancels in-flight checks and publishes empty diagnostics) then decrements -// the ref count. The file is only evicted when the ref count reaches zero, -// since the workspace may hold additional references. +// Close marks a file as closed by the editor. It clears the editor state, +// releases the workspace, and decrements the ref count. The file is only +// evicted when the ref count reaches zero. The workspace may hold additional +// references. func (f *file) Close(ctx context.Context) { f.clearEditorState(ctx) + f.releaseWorkspace() f.Manager().Close(ctx, f.uri) } @@ -180,15 +178,20 @@ func (f *file) Update(ctx context.Context, version int32, text string) { f.PublishDiagnostics(ctx) } -// RefreshWorkspace rebuilds the workspace for the current file and sets the workspace. +// RefreshWorkspace rebuilds the workspace for the current file. It takes a +// lease on the workspace if this file does not hold one yet. // -// The Buf workspace provides the sources for the compiler to work with. +// The Buf workspace provides the sources for the compiler to work with. Only +// files the editor interacts with directly are refreshed, so the lease taken +// here pairs with the release in [file.Close]. Dependency and WKT files have +// no resolvable workspace and stay workspace-less. func (f *file) RefreshWorkspace(ctx context.Context) { f.lsp.logger.Debug( "refresh workspace", slog.String("file", f.uri.Filename()), slog.Int("version", int(f.version)), ) + defer f.lsp.workspaceManager.Cleanup(ctx) if f.workspace != nil { if err := f.workspace.Refresh(ctx); err != nil { f.lsp.logger.Error( @@ -197,18 +200,24 @@ func (f *file) RefreshWorkspace(ctx context.Context) { xslog.ErrorAttr(err), ) } - } else { - workspace, err := f.lsp.workspaceManager.LeaseWorkspace(ctx, f.uri) - if err != nil { - f.lsp.logger.Error( - "could not lease workspace", - slog.String("uri", string(f.uri)), - xslog.ErrorAttr(err), - ) + return + } + workspace, err := f.lsp.workspaceManager.LeaseWorkspace(ctx, f.uri) + if err != nil { + var unresolvable errUnresolvableWorkspace + if errors.As(err, &unresolvable) { + // Expected for dependency and WKT files opened from the cache. + f.lsp.logger.Debug("no workspace for file", slog.String("uri", string(f.uri))) return } - f.workspace = workspace + f.lsp.logger.Error( + "could not lease workspace", + slog.String("uri", string(f.uri)), + xslog.ErrorAttr(err), + ) + return } + f.workspace = workspace } // RefreshIR queries for the IR of the file and the IR of each import file. @@ -224,6 +233,13 @@ func (f *file) RefreshIR(ctx context.Context) { return } + if f.workspace == nil { + // Dependency and WKT files have no workspace to compile against. Their + // IR and symbols are populated when a workspace member that imports + // them refreshes, so keep that state instead of wiping it. + return + } + f.lsp.logger.Info( "parsing IR for file", slog.String("uri", string(f.uri)), @@ -306,6 +322,16 @@ func (f *file) RefreshIR(ctx context.Context) { ) } +// releaseWorkspace releases this file's lease on its workspace, if any. A +// non-nil workspace is the lease. Only files the editor opened directly have +// one, taken in [file.RefreshWorkspace]. +func (f *file) releaseWorkspace() { + if f.workspace != nil { + f.workspace.Release() + f.workspace = nil + } +} + // queryIR returns the [queries.IR] for the current file. func (f *file) queryIR() incremental.Query[*ir.File] { if f.objectInfo == nil { @@ -1037,15 +1063,17 @@ func (f *file) resolveASTDefinition(def ast.DeclDef, defName ir.FullName) *symbo if def.Span().Path() == f.file.Path() { return f.referenceableSymbols[defName] } - // No workspace, we cannot resolve the AST definition from outside of the file. - if f.workspace == nil { - return nil - } for _, file := range f.workspace.PathToFile() { if file.file.Path() == def.Span().Path() { return file.referenceableSymbols[defName] } } + // Fall back to the file manager. Dependency and WKT files have no + // workspace, and span paths are absolute local paths, so the lookup by + // path is exact. + if file := f.lsp.fileManager.Get(FilePathToURI(def.Span().Path())); file != nil { + return file.referenceableSymbols[defName] + } return nil } @@ -1075,6 +1103,7 @@ func (f *file) SymbolAt(ctx context.Context, cursor protocol.Position) *symbol { symbol = before } if symbol != nil { + symbol.resolveDefinition() f.lsp.logger.DebugContext( ctx, "symbol at", @@ -1155,11 +1184,18 @@ func (f *file) RunChecks(ctx context.Context) { ctx, cancel := context.WithTimeout(f.lsp.connCtx, checkTimeout) f.cancelChecks = cancel + // Capture values used by the goroutine below. Eviction zeroes *f while + // checks run, so the goroutine must re-resolve the file under the lock + // before reading f. + lsp := f.lsp + uri := f.uri + uriFilename := f.uri.Filename() + go func() { var annotations []bufanalysis.FileAnnotation - image, diagnostics := buildImage(ctx, path, f.lsp.logger, opener) + image, diagnostics := buildImage(ctx, path, lsp.logger, opener) if image != nil { - f.lsp.logger.DebugContext(ctx, "checks running lint", slog.String("uri", f.uri.Filename()), slog.String("module", module.OpaqueID())) + lsp.logger.DebugContext(ctx, "checks running lint", slog.String("uri", uriFilename), slog.String("module", module.OpaqueID())) if err := checkClient.Lint( ctx, workspace.GetLintConfigForOpaqueID(module.OpaqueID()), @@ -1170,16 +1206,16 @@ func (f *file) RunChecks(ctx context.Context) { var fileAnnotationSet bufanalysis.FileAnnotationSet if !errors.As(err, &fileAnnotationSet) { if errors.Is(err, context.Canceled) || ctx.Err() != nil { - f.lsp.logger.DebugContext(ctx, "checks cancelled", slog.String("uri", f.uri.Filename()), xslog.ErrorAttr(err)) + lsp.logger.DebugContext(ctx, "checks cancelled", slog.String("uri", uriFilename), xslog.ErrorAttr(err)) } else if errors.Is(err, context.DeadlineExceeded) { - f.lsp.logger.WarnContext(ctx, "checks deadline exceeded", slog.String("uri", f.uri.Filename()), xslog.ErrorAttr(err)) + lsp.logger.WarnContext(ctx, "checks deadline exceeded", slog.String("uri", uriFilename), xslog.ErrorAttr(err)) } else { - f.lsp.logger.WarnContext(ctx, "checks failed", slog.String("uri", f.uri.Filename()), xslog.ErrorAttr(err)) + lsp.logger.WarnContext(ctx, "checks failed", slog.String("uri", uriFilename), xslog.ErrorAttr(err)) } return } if len(fileAnnotationSet.FileAnnotations()) == 0 { - f.lsp.logger.DebugContext(ctx, "checks lint passed", slog.String("uri", f.uri.Filename())) + lsp.logger.DebugContext(ctx, "checks lint passed", slog.String("uri", uriFilename)) } else { annotations = append(annotations, fileAnnotationSet.FileAnnotations()...) } @@ -1188,17 +1224,21 @@ func (f *file) RunChecks(ctx context.Context) { select { case <-ctx.Done(): - f.lsp.logger.DebugContext(ctx, "checks cancelled", slog.String("uri", f.uri.Filename()), xslog.ErrorAttr(ctx.Err())) + lsp.logger.DebugContext(ctx, "checks cancelled", slog.String("uri", uriFilename), xslog.ErrorAttr(ctx.Err())) return default: } - f.lsp.lock.Lock() - defer f.lsp.lock.Unlock() + lsp.lock.Lock() + defer lsp.lock.Unlock() + if lsp.fileManager.Get(uri) != f { + lsp.logger.DebugContext(ctx, "checks: file evicted while checks ran", slog.String("uri", uriFilename)) + return // The file was evicted, and possibly re-tracked, while checks ran. + } select { case <-ctx.Done(): - f.lsp.logger.DebugContext(ctx, "checks: cancelled after waiting for file lock", slog.String("uri", f.uri.Filename()), xslog.ErrorAttr(ctx.Err())) + lsp.logger.DebugContext(ctx, "checks: cancelled after waiting for file lock", slog.String("uri", uriFilename), xslog.ErrorAttr(ctx.Err())) return // Context cancelled whilst waiting to publishing diagnostics. default: } diff --git a/private/buf/buflsp/hover_cel_test.go b/private/buf/buflsp/hover_cel_test.go index 8a616b67ec..847d47cbbc 100644 --- a/private/buf/buflsp/hover_cel_test.go +++ b/private/buf/buflsp/hover_cel_test.go @@ -632,7 +632,7 @@ func TestCELHover(t *testing.T) { t.Parallel() var hoverResult *protocol.Hover - _, err = clientJSONConn.Call(ctx, protocol.MethodTextDocumentHover, protocol.HoverParams{ + _, err := clientJSONConn.Call(ctx, protocol.MethodTextDocumentHover, protocol.HoverParams{ TextDocumentPositionParams: protocol.TextDocumentPositionParams{ TextDocument: protocol.TextDocumentIdentifier{URI: testURI}, Position: protocol.Position{Line: tc.line, Character: tc.char}, diff --git a/private/buf/buflsp/server.go b/private/buf/buflsp/server.go index b17599c22f..46c1e921ef 100644 --- a/private/buf/buflsp/server.go +++ b/private/buf/buflsp/server.go @@ -388,6 +388,8 @@ func (s *server) DidClose( } if file := s.fileManager.Get(params.TextDocument.URI); file != nil { file.Close(ctx) + // Drop workspaces that no longer have any open files. + s.lsp.workspaceManager.Cleanup(ctx) } return nil } @@ -416,6 +418,8 @@ func (s *server) DidDeleteFiles( } } } + // Drop workspaces that no longer have any open files. + s.lsp.workspaceManager.Cleanup(ctx) return nil } diff --git a/private/buf/buflsp/symbol.go b/private/buf/buflsp/symbol.go index c4e02eeb0d..46dc1e16a9 100644 --- a/private/buf/buflsp/symbol.go +++ b/private/buf/buflsp/symbol.go @@ -107,6 +107,24 @@ func (*builtin) isSymbolKind() {} func (*tag) isSymbolKind() {} func (*keywordBuiltin) isSymbolKind() {} +// resolveDefinition resolves the symbol's definition if it has none yet. A +// definition can be missing when this symbol's file was indexed before the +// file declaring the definition. Resolution is cheap, so retry at query time. +func (s *symbol) resolveDefinition() { + if s.def != nil { + return + } + switch kind := s.kind.(type) { + case *reference: + s.def = s.file.resolveASTDefinition(kind.def, kind.fullName) + case *option: + s.def = s.file.resolveASTDefinition(kind.def, kind.defFullName) + if s.typeDef == nil { + s.typeDef = s.file.resolveASTDefinition(kind.typeDef, kind.typeDefFullName) + } + } +} + // Range constructs an LSP protocol code range for this symbol. func (s *symbol) Range() protocol.Range { return reportSpanToProtocolRange(s.span) diff --git a/private/buf/buflsp/workspace.go b/private/buf/buflsp/workspace.go index e693854cd6..fb0b251f8a 100644 --- a/private/buf/buflsp/workspace.go +++ b/private/buf/buflsp/workspace.go @@ -71,10 +71,10 @@ func (w *workspaceManager) Cleanup(ctx context.Context) { // Delete in-place. index := 0 for _, workspace := range w.workspaces { - if workspace.refCount > 0 { + if workspace.refCount > 0 || workspace.hasOpenFile() { w.workspaces[index] = workspace index++ - continue // workspace leased + continue // workspace in use } w.lsp.logger.Debug("workspace: cleanup removing workspace", slog.String("parent", workspace.workspaceURI.Filename())) for _, file := range workspace.pathToFile { @@ -144,10 +144,23 @@ func (w *workspace) Lease() { } // Release decrements the reference count. -func (w *workspace) Release() int { +func (w *workspace) Release() { w.lsp.logger.Debug("workspace: release", slog.String("path", w.workspaceURI.Filename())) + if w.refCount <= 0 { + w.lsp.logger.Error("workspace: refcount released below zero", slog.String("path", w.workspaceURI.Filename())) + return + } w.refCount-- - return w.refCount +} + +// hasOpenFile reports whether any file in this workspace is open in the editor. +func (w *workspace) hasOpenFile() bool { + for _, file := range w.pathToFile { + if file.IsOpenInEditor() { + return true + } + } + return false } // Refresh rebuilds the workspace and required context. diff --git a/private/buf/buflsp/workspace_symbol_test.go b/private/buf/buflsp/workspace_symbol_test.go deleted file mode 100644 index 47eaa0c598..0000000000 --- a/private/buf/buflsp/workspace_symbol_test.go +++ /dev/null @@ -1,136 +0,0 @@ -// Copyright 2020-2026 Buf Technologies, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package buflsp_test - -import ( - "path/filepath" - "slices" - "testing" - - "github.com/bufbuild/buf/private/buf/buflsp" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "go.lsp.dev/protocol" -) - -func TestWorkspaceSymbol(t *testing.T) { - t.Parallel() - - ctx := t.Context() - - testProtoPath, err := filepath.Abs("testdata/workspace_symbols/workspace_symbols.proto") - require.NoError(t, err) - - typesProtoPath, err := filepath.Abs("testdata/workspace_symbols/types.proto") - require.NoError(t, err) - - clientJSONConn, testURI := setupLSPServer(t, testProtoPath) - typesURI := buflsp.FilePathToURI(typesProtoPath) - - type symbolInfo struct { - name string - kind protocol.SymbolKind - line uint32 - deprecated bool - uri protocol.URI - } - - tests := []struct { - name string - query string - expectedSymbols []symbolInfo // Symbols that should be found with their details - minResults int // Minimum number of results expected - }{ - { - name: "search_for_item", - query: "Item", - expectedSymbols: []symbolInfo{ - {name: "workspace_symbols.v1.Item", kind: protocol.SymbolKindClass, line: 6, uri: testURI}, - {name: "workspace_symbols.v1.GetItemRequest", kind: protocol.SymbolKindClass, line: 24, uri: testURI}, - {name: "workspace_symbols.v1.GetItemResponse", kind: protocol.SymbolKindClass, line: 28, uri: testURI}, - {name: "workspace_symbols.v1.ListItemsRequest", kind: protocol.SymbolKindClass, line: 32, uri: testURI}, - {name: "workspace_symbols.v1.ListItemsResponse", kind: protocol.SymbolKindClass, line: 36, uri: testURI}, - {name: "workspace_symbols.v1.ItemService", kind: protocol.SymbolKindInterface, line: 19, uri: testURI}, - }, - minResults: 6, - }, - { - name: "search_for_color", - query: "Color", - expectedSymbols: []symbolInfo{ - {name: "workspace_symbols.v1.Color", kind: protocol.SymbolKindEnum, line: 4, uri: typesURI}, - {name: "workspace_symbols.v1.COLOR_UNSPECIFIED", kind: protocol.SymbolKindEnumMember, line: 5, uri: typesURI}, - {name: "workspace_symbols.v1.COLOR_RED", kind: protocol.SymbolKindEnumMember, line: 6, uri: typesURI}, - {name: "workspace_symbols.v1.COLOR_BLUE", kind: protocol.SymbolKindEnumMember, line: 7, uri: typesURI}, - }, - minResults: 4, - }, - { - name: "search_for_label", - query: "Label", - expectedSymbols: []symbolInfo{ - {name: "workspace_symbols.v1.Label", kind: protocol.SymbolKindClass, line: 10, uri: typesURI}, - }, - minResults: 1, - }, - { - name: "search_for_container", - query: "Container", - expectedSymbols: []symbolInfo{ - {name: "workspace_symbols.v1.Container", kind: protocol.SymbolKindClass, line: 13, uri: testURI}, - }, - minResults: 1, - }, - { - name: "search_for_deprecated", - query: "Legacy", - expectedSymbols: []symbolInfo{ - {name: "workspace_symbols.v1.LegacyItem", kind: protocol.SymbolKindClass, line: 40, deprecated: true, uri: testURI}, - }, - minResults: 1, - }, - { - name: "empty_query_returns_all_symbols", - query: "", - minResults: 20, // Should return many symbols from both files - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - var symbols []protocol.SymbolInformation - _, symErr := clientJSONConn.Call(ctx, protocol.MethodWorkspaceSymbol, protocol.WorkspaceSymbolParams{ - Query: tt.query, - }, &symbols) - require.NoError(t, symErr) - - assert.GreaterOrEqual(t, len(symbols), tt.minResults) - - for _, expectedSymbol := range tt.expectedSymbols { - idx := slices.IndexFunc(symbols, func(s protocol.SymbolInformation) bool { - return s.Name == expectedSymbol.name - }) - require.NotEqual(t, -1, idx, "expected to find symbol %s", expectedSymbol.name) - found := symbols[idx] - assert.Equal(t, expectedSymbol.kind, found.Kind, "symbol %s has wrong kind", expectedSymbol.name) - assert.Equal(t, expectedSymbol.uri, found.Location.URI, "symbol %s has wrong URI", expectedSymbol.name) - assert.Equal(t, expectedSymbol.line, found.Location.Range.Start.Line, "symbol %s has wrong line number", expectedSymbol.name) - assert.Equal(t, expectedSymbol.deprecated, found.Deprecated, "symbol %s has wrong deprecated status", expectedSymbol.name) - } - }) - } -} diff --git a/private/buf/buflsp/workspace_test.go b/private/buf/buflsp/workspace_test.go new file mode 100644 index 0000000000..2955973dbc --- /dev/null +++ b/private/buf/buflsp/workspace_test.go @@ -0,0 +1,347 @@ +// Copyright 2020-2026 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package buflsp_test + +import ( + "context" + "os" + "path/filepath" + "slices" + "strings" + "testing" + + "github.com/bufbuild/buf/private/buf/buflsp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.lsp.dev/jsonrpc2" + "go.lsp.dev/protocol" +) + +func TestWorkspaceSymbol(t *testing.T) { + t.Parallel() + + ctx := t.Context() + + testProtoPath, err := filepath.Abs("testdata/workspace_symbols/workspace_symbols.proto") + require.NoError(t, err) + + typesProtoPath, err := filepath.Abs("testdata/workspace_symbols/types.proto") + require.NoError(t, err) + + clientJSONConn, testURI := setupLSPServer(t, testProtoPath) + typesURI := buflsp.FilePathToURI(typesProtoPath) + + type symbolInfo struct { + name string + kind protocol.SymbolKind + line uint32 + deprecated bool + uri protocol.URI + } + + tests := []struct { + name string + query string + expectedSymbols []symbolInfo // Symbols that should be found with their details + minResults int // Minimum number of results expected + }{ + { + name: "search_for_item", + query: "Item", + expectedSymbols: []symbolInfo{ + {name: "workspace_symbols.v1.Item", kind: protocol.SymbolKindClass, line: 6, uri: testURI}, + {name: "workspace_symbols.v1.GetItemRequest", kind: protocol.SymbolKindClass, line: 24, uri: testURI}, + {name: "workspace_symbols.v1.GetItemResponse", kind: protocol.SymbolKindClass, line: 28, uri: testURI}, + {name: "workspace_symbols.v1.ListItemsRequest", kind: protocol.SymbolKindClass, line: 32, uri: testURI}, + {name: "workspace_symbols.v1.ListItemsResponse", kind: protocol.SymbolKindClass, line: 36, uri: testURI}, + {name: "workspace_symbols.v1.ItemService", kind: protocol.SymbolKindInterface, line: 19, uri: testURI}, + }, + minResults: 6, + }, + { + name: "search_for_color", + query: "Color", + expectedSymbols: []symbolInfo{ + {name: "workspace_symbols.v1.Color", kind: protocol.SymbolKindEnum, line: 4, uri: typesURI}, + {name: "workspace_symbols.v1.COLOR_UNSPECIFIED", kind: protocol.SymbolKindEnumMember, line: 5, uri: typesURI}, + {name: "workspace_symbols.v1.COLOR_RED", kind: protocol.SymbolKindEnumMember, line: 6, uri: typesURI}, + {name: "workspace_symbols.v1.COLOR_BLUE", kind: protocol.SymbolKindEnumMember, line: 7, uri: typesURI}, + }, + minResults: 4, + }, + { + name: "search_for_label", + query: "Label", + expectedSymbols: []symbolInfo{ + {name: "workspace_symbols.v1.Label", kind: protocol.SymbolKindClass, line: 10, uri: typesURI}, + }, + minResults: 1, + }, + { + name: "search_for_container", + query: "Container", + expectedSymbols: []symbolInfo{ + {name: "workspace_symbols.v1.Container", kind: protocol.SymbolKindClass, line: 13, uri: testURI}, + }, + minResults: 1, + }, + { + name: "search_for_deprecated", + query: "Legacy", + expectedSymbols: []symbolInfo{ + {name: "workspace_symbols.v1.LegacyItem", kind: protocol.SymbolKindClass, line: 40, deprecated: true, uri: testURI}, + }, + minResults: 1, + }, + { + name: "empty_query_returns_all_symbols", + query: "", + minResults: 20, // Should return many symbols from both files + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var symbols []protocol.SymbolInformation + _, symErr := clientJSONConn.Call(ctx, protocol.MethodWorkspaceSymbol, protocol.WorkspaceSymbolParams{ + Query: tt.query, + }, &symbols) + require.NoError(t, symErr) + + assert.GreaterOrEqual(t, len(symbols), tt.minResults) + + for _, expectedSymbol := range tt.expectedSymbols { + idx := slices.IndexFunc(symbols, func(s protocol.SymbolInformation) bool { + return s.Name == expectedSymbol.name + }) + require.NotEqual(t, -1, idx, "expected to find symbol %s", expectedSymbol.name) + found := symbols[idx] + assert.Equal(t, expectedSymbol.kind, found.Kind, "symbol %s has wrong kind", expectedSymbol.name) + assert.Equal(t, expectedSymbol.uri, found.Location.URI, "symbol %s has wrong URI", expectedSymbol.name) + assert.Equal(t, expectedSymbol.line, found.Location.Range.Start.Line, "symbol %s has wrong line number", expectedSymbol.name) + assert.Equal(t, expectedSymbol.deprecated, found.Deprecated, "symbol %s has wrong deprecated status", expectedSymbol.name) + } + }) + } +} + +func TestWorkspaceDependencyFile(t *testing.T) { + t.Parallel() + + ctx := t.Context() + + protoPath, err := filepath.Abs("testdata/hover_dependency/main.proto") + require.NoError(t, err) + clientJSONConn, testURI := setupLSPServer(t, protoPath) + + dependencyURI := resolveDependencyURI(ctx, t, clientJSONConn, testURI) + dependencyPosition := dependencyTypePosition(t, dependencyURI) + + // Definition inside the dependency file resolves to another dependency + // file before it is even opened. Dependency files have no workspace, so + // this exercises the file manager fallback. + locations := requestDefinition(ctx, t, clientJSONConn, dependencyURI, dependencyPosition) + require.Len(t, locations, 1) + assert.Contains(t, string(locations[0].URI), "source_context.proto") + + // Opening the dependency file in the editor keeps it working. + openFileFromDisk(ctx, t, clientJSONConn, dependencyURI) + locations = requestDefinition(ctx, t, clientJSONConn, dependencyURI, dependencyPosition) + require.Len(t, locations, 1) + assert.Contains(t, string(locations[0].URI), "source_context.proto") +} + +func TestWorkspaceReleasedOnClose(t *testing.T) { + t.Parallel() + + ctx := t.Context() + + protoPath, err := filepath.Abs("testdata/hover_dependency/main.proto") + require.NoError(t, err) + clientJSONConn, testURI := setupLSPServer(t, protoPath) + + // Position of the Api type reference in main.proto. + position := protocol.Position{Line: 9, Character: 20} + hover := requestHover(ctx, t, clientJSONConn, testURI, position) + require.NotNil(t, hover, "expected hover while the file is open") + + // Close and reopen the only open document twice. The workspace must be + // dropped and recreated each time. + for range 2 { + closeFile(ctx, t, clientJSONConn, testURI) + + // The file is evicted with its workspace, so hover has nothing to + // answer from. + hover = requestHover(ctx, t, clientJSONConn, testURI, position) + assert.Nil(t, hover, "expected no hover after the last open document closed") + + openFileFromDisk(ctx, t, clientJSONConn, testURI) + hover = requestHover(ctx, t, clientJSONConn, testURI, position) + assert.NotNil(t, hover, "expected hover after reopening") + } +} + +func TestWorkspaceSurvivesCloseOrder(t *testing.T) { + t.Parallel() + + ctx := t.Context() + + protoPath, err := filepath.Abs("testdata/hover_dependency/main.proto") + require.NoError(t, err) + clientJSONConn, testURI := setupLSPServer(t, protoPath) + + dependencyURI := resolveDependencyURI(ctx, t, clientJSONConn, testURI) + dependencyPosition := dependencyTypePosition(t, dependencyURI) + openFileFromDisk(ctx, t, clientJSONConn, dependencyURI) + + // Close the main file first. The open dependency file holds no lease, so + // the workspace must be kept alive by the open file check instead. + closeFile(ctx, t, clientJSONConn, testURI) + locations := requestDefinition(ctx, t, clientJSONConn, dependencyURI, dependencyPosition) + require.Len(t, locations, 1, "expected definitions while the dependency file is still open") + + // Closing the dependency file drops the workspace and evicts everything. + closeFile(ctx, t, clientJSONConn, dependencyURI) + hover := requestHover(ctx, t, clientJSONConn, dependencyURI, dependencyPosition) + assert.Nil(t, hover, "expected no hover after the last open document closed") +} + +// resolveDependencyURI follows the import in main.proto to the well-known +// type, as an editor would before opening it. +func resolveDependencyURI( + ctx context.Context, + t *testing.T, + clientJSONConn jsonrpc2.Conn, + testURI protocol.URI, +) protocol.URI { + t.Helper() + + var locations []protocol.Location + _, err := clientJSONConn.Call(ctx, protocol.MethodTextDocumentDefinition, protocol.DefinitionParams{ + TextDocumentPositionParams: protocol.TextDocumentPositionParams{ + TextDocument: protocol.TextDocumentIdentifier{URI: testURI}, + Position: protocol.Position{Line: 6, Character: 12}, // import "google/protobuf/api.proto"; + }, + }, &locations) + require.NoError(t, err) + require.Len(t, locations, 1) + return locations[0].URI +} + +// dependencyTypePosition finds a position on a type reference in the +// dependency file, so completion there suggests types. The line is found +// rather than hardcoded so the test survives a well-known type update. +func dependencyTypePosition(t *testing.T, dependencyURI protocol.URI) protocol.Position { + t.Helper() + + text, err := os.ReadFile(dependencyURI.Filename()) + require.NoError(t, err) + var line uint32 + for fileLine := range strings.SplitSeq(string(text), "\n") { + if strings.HasPrefix(strings.TrimSpace(fileLine), "SourceContext source_context") { + // Position within the SourceContext type name, past the indentation. + var indent uint32 + for _, r := range fileLine { + if r != ' ' && r != '\t' { + break + } + indent++ + } + return protocol.Position{Line: line, Character: indent + 5} + } + line++ + } + t.Fatalf("no type reference found in %s", dependencyURI.Filename()) + return protocol.Position{} +} + +// openFileFromDisk opens the file in the editor with its on-disk contents. +func openFileFromDisk( + ctx context.Context, + t *testing.T, + clientJSONConn jsonrpc2.Conn, + uri protocol.URI, +) { + t.Helper() + + text, err := os.ReadFile(uri.Filename()) + require.NoError(t, err) + require.NoError(t, clientJSONConn.Notify(ctx, protocol.MethodTextDocumentDidOpen, &protocol.DidOpenTextDocumentParams{ + TextDocument: protocol.TextDocumentItem{ + URI: uri, + LanguageID: "protobuf", + Version: 1, + Text: string(text), + }, + })) +} + +// closeFile closes the file in the editor. +func closeFile( + ctx context.Context, + t *testing.T, + clientJSONConn jsonrpc2.Conn, + uri protocol.URI, +) { + t.Helper() + + require.NoError(t, clientJSONConn.Notify(ctx, protocol.MethodTextDocumentDidClose, &protocol.DidCloseTextDocumentParams{ + TextDocument: protocol.TextDocumentIdentifier{URI: uri}, + })) +} + +// requestDefinition requests the definition locations at the given position. +func requestDefinition( + ctx context.Context, + t *testing.T, + clientJSONConn jsonrpc2.Conn, + uri protocol.URI, + position protocol.Position, +) []protocol.Location { + t.Helper() + + var locations []protocol.Location + _, err := clientJSONConn.Call(ctx, protocol.MethodTextDocumentDefinition, protocol.DefinitionParams{ + TextDocumentPositionParams: protocol.TextDocumentPositionParams{ + TextDocument: protocol.TextDocumentIdentifier{URI: uri}, + Position: position, + }, + }, &locations) + require.NoError(t, err) + return locations +} + +// requestHover requests hover contents at the given position. +func requestHover( + ctx context.Context, + t *testing.T, + clientJSONConn jsonrpc2.Conn, + uri protocol.URI, + position protocol.Position, +) *protocol.Hover { + t.Helper() + + var hover *protocol.Hover + _, err := clientJSONConn.Call(ctx, protocol.MethodTextDocumentHover, protocol.HoverParams{ + TextDocumentPositionParams: protocol.TextDocumentPositionParams{ + TextDocument: protocol.TextDocumentIdentifier{URI: uri}, + Position: position, + }, + }, &hover) + require.NoError(t, err) + return hover +}