Skip to content

Commit 3a319f6

Browse files
Clear stale LSP diagnostics when a file is closed
When the user closed a file, the LSP server never published empty diagnostics to clear the Problems panel. The root cause is that the workspace's indexFiles path calls fileManager.Track for every file in the module, incrementing the reference count beyond the editor's own reference. When DidClose decrements the count, it rarely drops to zero, so fileManager.Close never calls Reset and diagnostics were never cleared. Fix this by handling the clear explicitly in DidClose: cancel in-flight checks, publish empty diagnostics (while IsOpenInEditor is still true), then set version=-1 to mark the file as no longer open in the editor so subsequent RunChecks skip re-publishing lint diagnostics. Also update Reset to be defensive: publish empty diagnostics if IsOpenInEditor is still true at Reset time (e.g. for workspace-external files whose ref count drops to zero without going through DidClose). Extract the "buf lint" diagnostic source string into a named constant lintSourceName alongside the existing serverName constant. Add a regression test that opens a file with a known lint violation, waits for diagnostics, sends DidClose, then asserts the Problems panel is cleared. Fixes bufbuild/vscode-buf#626.
1 parent 7984e01 commit 3a319f6

4 files changed

Lines changed: 67 additions & 3 deletions

File tree

private/buf/buflsp/diagnostics_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,47 @@ func TestDiagnosticsNoTransitiveLeak(t *testing.T) {
511511
})
512512
}
513513

514+
// TestDiagnosticsClearedOnClose verifies that stale diagnostics are cleared from the
515+
// client when a file is closed, reproducing the scenario from
516+
// https://github.com/bufbuild/vscode-buf/issues/626 where lint errors on a deleted file
517+
// continued to show in the editor's Problems panel after the file was removed.
518+
func TestDiagnosticsClearedOnClose(t *testing.T) {
519+
t.Parallel()
520+
521+
t.Run("lint_diagnostics_cleared_after_close", func(t *testing.T) {
522+
t.Parallel()
523+
524+
synctest.Test(t, func(t *testing.T) {
525+
// Use an existing testdata file with a known lint violation (IMPORT_USED)
526+
// so the workspace is fully configured and RunChecks produces diagnostics.
527+
protoPath, err := filepath.Abs("testdata/diagnostics/unused_import.proto")
528+
require.NoError(t, err)
529+
530+
clientJSONConn, testURI, capture := setupLSPServerWithDiagnostics(t, protoPath)
531+
532+
ctx := t.Context()
533+
534+
// Wait for lint diagnostics to appear.
535+
initial := capture.wait(t, testURI, 10*time.Second, func(p *protocol.PublishDiagnosticsParams) bool {
536+
return len(p.Diagnostics) > 0
537+
})
538+
require.NotEmpty(t, initial.Diagnostics, "expected lint diagnostics before close")
539+
540+
// Close the file, simulating what happens when the user closes or deletes it.
541+
err = clientJSONConn.Notify(ctx, protocol.MethodTextDocumentDidClose, &protocol.DidCloseTextDocumentParams{
542+
TextDocument: protocol.TextDocumentIdentifier{URI: testURI},
543+
})
544+
require.NoError(t, err)
545+
546+
// The server must publish empty diagnostics so the Problems panel clears.
547+
cleared := capture.wait(t, testURI, 5*time.Second, func(p *protocol.PublishDiagnosticsParams) bool {
548+
return len(p.Diagnostics) == 0
549+
})
550+
assert.Empty(t, cleared.Diagnostics, "stale diagnostics should be cleared after file close")
551+
})
552+
})
553+
}
554+
514555
// diagnosticsCapture captures publishDiagnostics notifications from the LSP server.
515556
type diagnosticsCapture struct {
516557
mu sync.Mutex

private/buf/buflsp/file.go

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ func (f *file) Reset(ctx context.Context) {
101101
// Evict the query key if there is a query cached on the file. We cache the [queries.File]
102102
// query since this allows the executor to evict all dependent queries, e.g. AST and IR.
103103
f.lsp.queryExecutor.Evict(f.queryFileKeys()...)
104+
f.clearEditorState(ctx)
104105
// Clear the file as nothing should use it after reset. This asserts that.
105106
*f = file{}
106107
}
@@ -110,6 +111,7 @@ func (f *file) Reset(ctx context.Context) {
110111
// This will not necessarily evict the file, since there may be more than one user
111112
// for this file.
112113
func (f *file) Close(ctx context.Context) {
114+
f.clearEditorState(ctx)
113115
f.Manager().Close(ctx, f.uri)
114116
}
115117

@@ -1080,6 +1082,26 @@ func (f *file) CancelChecks(ctx context.Context) {
10801082
}
10811083
}
10821084

1085+
// clearEditorState cancels in-flight checks, publishes empty diagnostics to clear
1086+
// the client's Problems panel (only when diagnostics are non-empty, to avoid a
1087+
// no-op network write), and marks the file as no longer open in the editor by
1088+
// setting version=-1. Called while the LSP lock is held, before the editor ref
1089+
// is decremented.
1090+
//
1091+
// The publish must happen before version is set to -1 because PublishDiagnostics
1092+
// checks IsOpenInEditor (which returns f.version != -1) and returns early if false.
1093+
func (f *file) clearEditorState(ctx context.Context) {
1094+
if !f.IsOpenInEditor() {
1095+
return
1096+
}
1097+
f.CancelChecks(ctx)
1098+
if len(f.diagnostics) > 0 {
1099+
f.diagnostics = nil
1100+
f.PublishDiagnostics(ctx)
1101+
}
1102+
f.version = -1
1103+
}
1104+
10831105
// RunChecks triggers the run of checks for this file. Diagnostics are published asynchronously.
10841106
func (f *file) RunChecks(ctx context.Context) {
10851107
if f.IsWKT() || !f.IsOpenInEditor() {
@@ -1174,7 +1196,7 @@ func (f *file) RunChecks(ctx context.Context) {
11741196
// TODO: prefer diagnostics from the old compiler to the new compiler to remove duplicates from both.
11751197
f.diagnostics = diagnostics
11761198
}
1177-
f.appendAnnotations("buf lint", annotations)
1199+
f.appendAnnotations(lintSourceName, annotations)
11781200
f.PublishDiagnostics(ctx)
11791201
}()
11801202
}

private/buf/buflsp/lint_ignore.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func (s *server) getLintIgnoreCodeActions(
4646
// Filter diagnostics to find lint errors that overlap with the requested range
4747
for _, diagnostic := range file.diagnostics {
4848
// Only handle lint diagnostics (not IR diagnostics)
49-
if diagnostic.Source != "buf lint" {
49+
if diagnostic.Source != lintSourceName {
5050
continue
5151
}
5252

private/buf/buflsp/server.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ import (
3333
)
3434

3535
const (
36-
serverName = "buf-lsp"
36+
serverName = "buf-lsp"
37+
lintSourceName = "buf lint"
3738

3839
maxSymbolResults = 1000
3940
)

0 commit comments

Comments
 (0)