From e7e35f602cb672792459ffaccd934e923913bff8 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Thu, 20 Aug 2026 17:59:08 +0100 Subject: [PATCH 1/3] Fix LSP workspaces leases --- CHANGELOG.md | 2 + private/buf/buflsp/buf_gen_yaml_lsp_test.go | 2 +- private/buf/buflsp/buf_yaml_lsp_test.go | 2 +- private/buf/buflsp/file.go | 102 ++++-- private/buf/buflsp/hover_cel_test.go | 2 +- private/buf/buflsp/server.go | 4 + private/buf/buflsp/workspace.go | 32 +- private/buf/buflsp/workspace_symbol_test.go | 136 -------- private/buf/buflsp/workspace_test.go | 346 ++++++++++++++++++++ 9 files changed, 451 insertions(+), 177 deletions(-) delete mode 100644 private/buf/buflsp/workspace_symbol_test.go create mode 100644 private/buf/buflsp/workspace_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index d12f5bb901..f0eb90dc48 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 workspace file associations which degraded completion and renames within those + files, and release workspaces to free resources per session. ## [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..c0c93126ff 100644 --- a/private/buf/buflsp/file.go +++ b/private/buf/buflsp/file.go @@ -60,6 +60,11 @@ type file struct { hasText bool // Whether this file has ever had text read into it. workspace *workspace // May be nil. objectInfo storage.ObjectInfo // Info in the context of the workspace. + // hasWorkspaceLease reports whether this file holds a lease on workspace. + // A lease is counted in the workspace's reference count. Only files opened + // in the editor take a lease. Dependency and WKT files have a workspace + // without a lease. + hasWorkspaceLease bool ir *ir.File referenceableSymbols map[ir.FullName]*symbol @@ -94,10 +99,8 @@ 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.releaseWorkspaceLease() + f.workspace = nil // 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,13 +109,29 @@ 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 lease, 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) { + // Manager().Close may evict the file and zero *f, so capture locals first. + fileManager := f.Manager() + uri := f.uri f.clearEditorState(ctx) - f.Manager().Close(ctx, f.uri) + f.releaseWorkspaceLease() + fileManager.Close(ctx, uri) +} + +// releaseWorkspaceLease releases this file's lease on its workspace, if any. +// Only the first call releases. +func (f *file) releaseWorkspaceLease() { + if !f.hasWorkspaceLease { + return + } + f.hasWorkspaceLease = false + if f.workspace != nil { + f.workspace.Release() + } } // IsOpenInEditor returns whether this file was opened in the LSP client's @@ -180,24 +199,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]. 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)), ) - if f.workspace != nil { - if err := f.workspace.Refresh(ctx); err != nil { - f.lsp.logger.Error( - "could not refresh workspace", - slog.String("uri", string(f.uri)), - xslog.ErrorAttr(err), - ) - } - } else { + defer f.lsp.workspaceManager.Cleanup(ctx) + if f.workspace == nil { workspace, err := f.lsp.workspaceManager.LeaseWorkspace(ctx, f.uri) if err != nil { f.lsp.logger.Error( @@ -208,6 +223,22 @@ func (f *file) RefreshWorkspace(ctx context.Context) { return } f.workspace = workspace + f.hasWorkspaceLease = true + return + } + if !f.hasWorkspaceLease { + // The workspace associated this file when indexing it. This happens for + // dependency and WKT files the editor opens directly. Take a lease so + // the workspace outlives the open document. + f.workspace.Lease() + f.hasWorkspaceLease = true + } + if err := f.workspace.Refresh(ctx); err != nil { + f.lsp.logger.Error( + "could not refresh workspace", + slog.String("uri", string(f.uri)), + xslog.ErrorAttr(err), + ) } } @@ -1155,11 +1186,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 +1208,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 +1226,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/workspace.go b/private/buf/buflsp/workspace.go index e693854cd6..1fef73a57a 100644 --- a/private/buf/buflsp/workspace.go +++ b/private/buf/buflsp/workspace.go @@ -78,6 +78,16 @@ func (w *workspaceManager) Cleanup(ctx context.Context) { } w.lsp.logger.Debug("workspace: cleanup removing workspace", slog.String("parent", workspace.workspaceURI.Filename())) for _, file := range workspace.pathToFile { + if file.IsOpenInEditor() { + // A file open in the editor leases its workspace, so it should be + // unreachable here. Drop the association rather than closing the + // file, which would silence its diagnostics. + w.lsp.logger.Error("workspace: cleanup reached an open file", slog.String("path", file.uri.Filename())) + if file.workspace == workspace { + file.workspace = nil + } + continue + } file.Close(ctx) } workspace.pathToFile = nil @@ -144,10 +154,13 @@ 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 } // Refresh rebuilds the workspace and required context. @@ -252,13 +265,16 @@ func (w *workspace) indexFiles(ctx context.Context) { w.lsp.logger.Debug("workspace: index track file", slog.String("path", file.uri.Filename())) } - // Currently we only associate a file with one workspace. This assumption isn't accurate - // for shared dependencies. Here we update to the latest, most recently used, workspace. - // This will make goto definition and find references only work in that workspace. - if oldWorkspace := file.workspace; oldWorkspace != nil && oldWorkspace != w { - oldWorkspace.Release() - w.Lease() + // Associate every indexed file with this workspace. A file belongs to + // one workspace at a time, and for shared dependencies the latest + // workspace wins. Only files opened in the editor hold a lease, so a + // lease moves only when such a file changes workspace. + if oldWorkspace := file.workspace; oldWorkspace != w { file.workspace = w + if file.hasWorkspaceLease && oldWorkspace != nil { + w.Lease() + oldWorkspace.Release() + } } file.objectInfo = fileInfo 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..5a580e4991 --- /dev/null +++ b/private/buf/buflsp/workspace_test.go @@ -0,0 +1,346 @@ +// 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) + + // Completion inside the dependency file works before it is even opened. + items := requestCompletion(ctx, t, clientJSONConn, dependencyURI, dependencyPosition) + assert.NotEmpty(t, items, "expected completions inside the dependency file") + + // Opening the dependency file in the editor keeps it working. + openFileFromDisk(ctx, t, clientJSONConn, dependencyURI) + items = requestCompletion(ctx, t, clientJSONConn, dependencyURI, dependencyPosition) + assert.NotEmpty(t, items, "expected completions in the opened dependency file") +} + +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 workspace must survive on the dependency + // file's lease. + closeFile(ctx, t, clientJSONConn, testURI) + items := requestCompletion(ctx, t, clientJSONConn, dependencyURI, dependencyPosition) + assert.NotEmpty(t, items, "expected completions while the dependency file is still open") + + // Closing the dependency file drops the last lease and evicts everything. + closeFile(ctx, t, clientJSONConn, dependencyURI) + items = requestCompletion(ctx, t, clientJSONConn, dependencyURI, dependencyPosition) + assert.Empty(t, items, "expected no completions 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}, + })) +} + +// requestCompletion requests completion items at the given position. +func requestCompletion( + ctx context.Context, + t *testing.T, + clientJSONConn jsonrpc2.Conn, + uri protocol.URI, + position protocol.Position, +) []protocol.CompletionItem { + t.Helper() + + var completionList *protocol.CompletionList + _, err := clientJSONConn.Call(ctx, protocol.MethodTextDocumentCompletion, protocol.CompletionParams{ + TextDocumentPositionParams: protocol.TextDocumentPositionParams{ + TextDocument: protocol.TextDocumentIdentifier{URI: uri}, + Position: position, + }, + }, &completionList) + require.NoError(t, err) + if completionList == nil { + return nil + } + return completionList.Items +} + +// 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 +} From 7d75c7da0ef97f7e458499d0e96a8e74cf69377f Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Thu, 20 Aug 2026 18:52:51 +0100 Subject: [PATCH 2/3] Remove workspace from deps --- CHANGELOG.md | 4 +- private/buf/buflsp/file.go | 79 ++++++++++++++-------------- private/buf/buflsp/symbol.go | 18 +++++++ private/buf/buflsp/workspace.go | 42 +++++++-------- private/buf/buflsp/workspace_test.go | 45 ++++++++-------- 5 files changed, 104 insertions(+), 84 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f0eb90dc48..f8791c43bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,8 +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 workspace file associations which degraded completion and renames within those - files, and release workspaces to free resources per session. +- 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/file.go b/private/buf/buflsp/file.go index c0c93126ff..118146b509 100644 --- a/private/buf/buflsp/file.go +++ b/private/buf/buflsp/file.go @@ -60,11 +60,6 @@ type file struct { hasText bool // Whether this file has ever had text read into it. workspace *workspace // May be nil. objectInfo storage.ObjectInfo // Info in the context of the workspace. - // hasWorkspaceLease reports whether this file holds a lease on workspace. - // A lease is counted in the workspace's reference count. Only files opened - // in the editor take a lease. Dependency and WKT files have a workspace - // without a lease. - hasWorkspaceLease bool ir *ir.File referenceableSymbols map[ir.FullName]*symbol @@ -99,8 +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())) - f.releaseWorkspaceLease() - 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()...) @@ -110,27 +104,25 @@ func (f *file) Reset(ctx context.Context) { } // Close marks a file as closed by the editor. It clears the editor state, -// releases the workspace lease, and decrements the ref count. The file is -// only evicted when the ref count reaches zero. The workspace may hold -// additional references. +// 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) { // Manager().Close may evict the file and zero *f, so capture locals first. fileManager := f.Manager() uri := f.uri f.clearEditorState(ctx) - f.releaseWorkspaceLease() + f.releaseWorkspace() fileManager.Close(ctx, uri) } -// releaseWorkspaceLease releases this file's lease on its workspace, if any. -// Only the first call releases. -func (f *file) releaseWorkspaceLease() { - if !f.hasWorkspaceLease { - return - } - f.hasWorkspaceLease = false +// 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 } } @@ -204,7 +196,8 @@ func (f *file) Update(ctx context.Context, version int32, text string) { // // 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]. +// 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", @@ -212,34 +205,32 @@ func (f *file) RefreshWorkspace(ctx context.Context) { slog.Int("version", int(f.version)), ) defer f.lsp.workspaceManager.Cleanup(ctx) - if f.workspace == nil { - workspace, err := f.lsp.workspaceManager.LeaseWorkspace(ctx, f.uri) - if err != nil { + if f.workspace != nil { + if err := f.workspace.Refresh(ctx); err != nil { f.lsp.logger.Error( - "could not lease workspace", + "could not refresh workspace", slog.String("uri", string(f.uri)), xslog.ErrorAttr(err), ) - return } - f.workspace = workspace - f.hasWorkspaceLease = true return } - if !f.hasWorkspaceLease { - // The workspace associated this file when indexing it. This happens for - // dependency and WKT files the editor opens directly. Take a lease so - // the workspace outlives the open document. - f.workspace.Lease() - f.hasWorkspaceLease = true - } - if err := f.workspace.Refresh(ctx); err != nil { + 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.lsp.logger.Error( - "could not refresh workspace", + "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. @@ -255,6 +246,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)), @@ -1068,15 +1066,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 } @@ -1106,6 +1106,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", 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 1fef73a57a..1f3c7b7ed8 100644 --- a/private/buf/buflsp/workspace.go +++ b/private/buf/buflsp/workspace.go @@ -71,23 +71,13 @@ 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 { - if file.IsOpenInEditor() { - // A file open in the editor leases its workspace, so it should be - // unreachable here. Drop the association rather than closing the - // file, which would silence its diagnostics. - w.lsp.logger.Error("workspace: cleanup reached an open file", slog.String("path", file.uri.Filename())) - if file.workspace == workspace { - file.workspace = nil - } - continue - } file.Close(ctx) } workspace.pathToFile = nil @@ -163,6 +153,19 @@ func (w *workspace) Release() { w.refCount-- } +// hasOpenFile reports whether any file in this workspace is open in the editor. +// Dependency files hold no lease, so an open one keeps the workspace alive +// through this check instead. Tearing the workspace down under an open file +// would evict files its symbols still point into. +func (w *workspace) hasOpenFile() bool { + for _, file := range w.pathToFile { + if file.IsOpenInEditor() { + return true + } + } + return false +} + // Refresh rebuilds the workspace and required context. func (w *workspace) Refresh(ctx context.Context) error { if w == nil { @@ -265,16 +268,13 @@ func (w *workspace) indexFiles(ctx context.Context) { w.lsp.logger.Debug("workspace: index track file", slog.String("path", file.uri.Filename())) } - // Associate every indexed file with this workspace. A file belongs to - // one workspace at a time, and for shared dependencies the latest - // workspace wins. Only files opened in the editor hold a lease, so a - // lease moves only when such a file changes workspace. - if oldWorkspace := file.workspace; oldWorkspace != w { + // Currently we only associate a file with one workspace. This assumption isn't accurate + // for shared dependencies. Here we update to the latest, most recently used, workspace. + // This will make goto definition and find references only work in that workspace. + if oldWorkspace := file.workspace; oldWorkspace != nil && oldWorkspace != w { + oldWorkspace.Release() + w.Lease() file.workspace = w - if file.hasWorkspaceLease && oldWorkspace != nil { - w.Lease() - oldWorkspace.Release() - } } file.objectInfo = fileInfo diff --git a/private/buf/buflsp/workspace_test.go b/private/buf/buflsp/workspace_test.go index 5a580e4991..2955973dbc 100644 --- a/private/buf/buflsp/workspace_test.go +++ b/private/buf/buflsp/workspace_test.go @@ -151,14 +151,18 @@ func TestWorkspaceDependencyFile(t *testing.T) { dependencyURI := resolveDependencyURI(ctx, t, clientJSONConn, testURI) dependencyPosition := dependencyTypePosition(t, dependencyURI) - // Completion inside the dependency file works before it is even opened. - items := requestCompletion(ctx, t, clientJSONConn, dependencyURI, dependencyPosition) - assert.NotEmpty(t, items, "expected completions inside the dependency file") + // 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) - items = requestCompletion(ctx, t, clientJSONConn, dependencyURI, dependencyPosition) - assert.NotEmpty(t, items, "expected completions in the opened dependency file") + 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) { @@ -204,16 +208,16 @@ func TestWorkspaceSurvivesCloseOrder(t *testing.T) { dependencyPosition := dependencyTypePosition(t, dependencyURI) openFileFromDisk(ctx, t, clientJSONConn, dependencyURI) - // Close the main file first. The workspace must survive on the dependency - // file's lease. + // 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) - items := requestCompletion(ctx, t, clientJSONConn, dependencyURI, dependencyPosition) - assert.NotEmpty(t, items, "expected completions while the dependency file is still open") + 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 last lease and evicts everything. + // Closing the dependency file drops the workspace and evicts everything. closeFile(ctx, t, clientJSONConn, dependencyURI) - items = requestCompletion(ctx, t, clientJSONConn, dependencyURI, dependencyPosition) - assert.Empty(t, items, "expected no completions after the last open document closed") + 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 @@ -300,28 +304,25 @@ func closeFile( })) } -// requestCompletion requests completion items at the given position. -func requestCompletion( +// 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.CompletionItem { +) []protocol.Location { t.Helper() - var completionList *protocol.CompletionList - _, err := clientJSONConn.Call(ctx, protocol.MethodTextDocumentCompletion, protocol.CompletionParams{ + var locations []protocol.Location + _, err := clientJSONConn.Call(ctx, protocol.MethodTextDocumentDefinition, protocol.DefinitionParams{ TextDocumentPositionParams: protocol.TextDocumentPositionParams{ TextDocument: protocol.TextDocumentIdentifier{URI: uri}, Position: position, }, - }, &completionList) + }, &locations) require.NoError(t, err) - if completionList == nil { - return nil - } - return completionList.Items + return locations } // requestHover requests hover contents at the given position. From 511f72222eb1f17d5a9a9b3480bd425ba046c2cb Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Thu, 20 Aug 2026 19:30:54 +0100 Subject: [PATCH 3/3] Cleanup --- private/buf/buflsp/file.go | 25 +++++++++++-------------- private/buf/buflsp/workspace.go | 3 --- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/private/buf/buflsp/file.go b/private/buf/buflsp/file.go index 118146b509..9d41d2a550 100644 --- a/private/buf/buflsp/file.go +++ b/private/buf/buflsp/file.go @@ -108,22 +108,9 @@ func (f *file) Reset(ctx context.Context) { // evicted when the ref count reaches zero. The workspace may hold additional // references. func (f *file) Close(ctx context.Context) { - // Manager().Close may evict the file and zero *f, so capture locals first. - fileManager := f.Manager() - uri := f.uri f.clearEditorState(ctx) f.releaseWorkspace() - fileManager.Close(ctx, uri) -} - -// 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 - } + f.Manager().Close(ctx, f.uri) } // IsOpenInEditor returns whether this file was opened in the LSP client's @@ -335,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 { diff --git a/private/buf/buflsp/workspace.go b/private/buf/buflsp/workspace.go index 1f3c7b7ed8..fb0b251f8a 100644 --- a/private/buf/buflsp/workspace.go +++ b/private/buf/buflsp/workspace.go @@ -154,9 +154,6 @@ func (w *workspace) Release() { } // hasOpenFile reports whether any file in this workspace is open in the editor. -// Dependency files hold no lease, so an open one keeps the workspace alive -// through this check instead. Tearing the workspace down under an open file -// would evict files its symbols still point into. func (w *workspace) hasOpenFile() bool { for _, file := range w.pathToFile { if file.IsOpenInEditor() {