diff --git a/gopls/doc/settings.md b/gopls/doc/settings.md index 44ab0e7214c..92d9008601a 100644 --- a/gopls/doc/settings.md +++ b/gopls/doc/settings.md @@ -463,6 +463,19 @@ or configuration change will still trigger diagnostics. Default: `"Edit"`. + +### `eagerDiagnosticsClear bool` + +**This is an advanced setting and should not be configured by most `gopls` users.** + +eagerDiagnosticsClear controls whether gopls immediately publishes empty +diagnostics for a file when it receives a textDocument/didChange +notification, before reanalysis completes. This prevents stale +diagnostics from persisting in the editor between an edit and the next +diagnostic pass. + +Default: `false`. + ### `analysisProgressReporting bool` diff --git a/gopls/internal/server/text_synchronization.go b/gopls/internal/server/text_synchronization.go index 0775ed33925..6760d638d89 100644 --- a/gopls/internal/server/text_synchronization.go +++ b/gopls/internal/server/text_synchronization.go @@ -266,6 +266,36 @@ func (s *server) didModifyFiles(ctx context.Context, modifications []file.Modifi s.mustPublishDiagnostics(mod.URI) } + // Eagerly publish empty diagnostics for changed files so that stale + // diagnostics are not shown to the client between the edit and the + // completion of reanalysis. Only applies to direct in-editor edits + // (FromDidChange), not on-disk changes or other modification sources. + // + // publishFileDiagnosticsLocked filters byView entries by file version. + // Because the file version just changed, no existing view diagnostics + // will match the new version, so the published set will be empty. + // The real diagnostics will be published once reanalysis completes. + if cause == FromDidChange && s.Options().EagerDiagnosticsClear { + viewMap := make(viewSet) + for _, v := range s.session.Views() { + viewMap[v] = unit{} + } + s.diagnosticsMu.Lock() + for _, mod := range modifications { + uri := mod.URI + f, ok := s.diagnostics[uri] + if !ok { + continue // no prior diagnostics for this file; nothing to clear + } + fh, err := s.session.ReadFile(ctx, uri) + if err != nil { + continue + } + _ = s.publishFileDiagnosticsLocked(ctx, viewMap, uri, fh.Version(), f) + } + s.diagnosticsMu.Unlock() + } + modCtx, modID := s.needsDiagnosis(ctx, viewsToDiagnose) wg.Go(func() { diff --git a/gopls/internal/settings/default.go b/gopls/internal/settings/default.go index f197ad26ae9..857c2bfac67 100644 --- a/gopls/internal/settings/default.go +++ b/gopls/internal/settings/default.go @@ -104,6 +104,7 @@ func DefaultOptions(overrides ...func(*Options)) *Options { DiagnosticsDelay: 1 * time.Second, DiagnosticsTrigger: DiagnosticsOnEdit, AnalysisProgressReporting: true, + EagerDiagnosticsClear: false, }, InlayHintOptions: InlayHintOptions{ Hints: map[InlayHint]bool{}, diff --git a/gopls/internal/settings/settings.go b/gopls/internal/settings/settings.go index 401965b4b9a..b1e44a99016 100644 --- a/gopls/internal/settings/settings.go +++ b/gopls/internal/settings/settings.go @@ -532,6 +532,13 @@ type DiagnosticOptions struct { // DiagnosticsTrigger controls when to run diagnostics. DiagnosticsTrigger DiagnosticsTrigger `status:"experimental"` + // EagerDiagnosticsClear controls whether gopls immediately publishes empty + // diagnostics for a file when it receives a textDocument/didChange + // notification, before reanalysis completes. This prevents stale + // diagnostics from persisting in the editor between an edit and the next + // diagnostic pass. + EagerDiagnosticsClear bool `status:"advanced"` + // AnalysisProgressReporting controls whether gopls sends progress // notifications when construction of its index of analysis facts is taking a // long time. Cancelling these notifications will cancel the indexing task, @@ -1369,6 +1376,9 @@ func (o *Options) setOne(name string, value any) (applied []CounterPath, _ error DiagnosticsOnEdit, DiagnosticsOnSave) + case "eagerDiagnosticsClear": + return setBool(&o.EagerDiagnosticsClear, value) + case "analysisProgressReporting": return setBool(&o.AnalysisProgressReporting, value) diff --git a/gopls/internal/test/integration/diagnostics/invalidation_test.go b/gopls/internal/test/integration/diagnostics/invalidation_test.go index 0ee23eda003..2ddf63c5dc7 100644 --- a/gopls/internal/test/integration/diagnostics/invalidation_test.go +++ b/gopls/internal/test/integration/diagnostics/invalidation_test.go @@ -105,6 +105,79 @@ func _() { }) } +// TestEagerDiagnosticInvalidation verifies that when eagerDiagnosticsClear is +// enabled, the first publishDiagnostics notification after an edit is an empty +// clear of stale diagnostics, sent before reanalysis completes. +func TestEagerDiagnosticInvalidation(t *testing.T) { + const files = ` +-- go.mod -- +module mod.com + +go 1.16 +-- main.go -- +package main + +func main() { + x := 2 +} +` + // With eagerDiagnosticsClear enabled: editing a file that has diagnostics + // should first publish an empty clear, then the real diagnostics. + WithOptions( + Settings{"eagerDiagnosticsClear": true}, + ).Run(t, files, func(t *testing.T, env *Env) { + env.OpenFile("main.go") + env.AfterChange( + Diagnostics(env.AtRegexp("main.go", "x")), + ) + + // Start collecting all diagnostic notifications before editing. + getDiagHistory := env.Awaiter.ListenToDiagnostics("main.go") + + // Fix the error by using the variable. + env.RegexpReplace("main.go", "x := 2", "_ = 2") + env.AfterChange( + NoDiagnostics(ForFile("main.go")), + ) + + history := getDiagHistory() + if len(history) == 0 { + t.Fatal("expected at least one diagnostic notification after edit") + } + if len(history[0].Diagnostics) != 0 { + t.Errorf("first notification after edit should be empty (eager clear), got %d diagnostics", len(history[0].Diagnostics)) + } + }) + + // With eagerDiagnosticsClear disabled: no eager empty clear before + // reanalysis. Use a no-op edit (comment change) so the error persists + // and we can verify the first notification still carries diagnostics. + WithOptions( + Settings{"eagerDiagnosticsClear": false}, + ).Run(t, files, func(t *testing.T, env *Env) { + env.OpenFile("main.go") + env.AfterChange( + Diagnostics(env.AtRegexp("main.go", "x")), + ) + + getDiagHistory := env.Awaiter.ListenToDiagnostics("main.go") + + // Add a comment - the unused-variable error should persist. + env.RegexpReplace("main.go", "x := 2", "x := 2 // edited") + env.AfterChange( + Diagnostics(env.AtRegexp("main.go", "x")), + ) + + history := getDiagHistory() + if len(history) == 0 { + t.Fatal("expected at least one diagnostic notification after edit") + } + if len(history[0].Diagnostics) == 0 { + t.Errorf("without eagerDiagnosticsClear, first notification should have diagnostics, got empty") + } + }) +} + func TestCreatingPackageInvalidatesDiagnostics_Issue66384(t *testing.T) { const files = ` -- go.mod -- diff --git a/gopls/internal/test/integration/env.go b/gopls/internal/test/integration/env.go index 48a31c94666..64f6fefd086 100644 --- a/gopls/internal/test/integration/env.go +++ b/gopls/internal/test/integration/env.go @@ -68,6 +68,13 @@ type Awaiter struct { // received since the registration was created. docCollectors map[uint64][]*protocol.ShowDocumentParams messageCollectors map[uint64][]*protocol.ShowMessageParams + diagCollectors map[uint64]diagCollector +} + +// diagCollector records all diagnostic notifications for a specific file path. +type diagCollector struct { + path string + params []*protocol.PublishDiagnosticsParams } func NewAwaiter(workdir *fake.Workdir) *Awaiter { @@ -142,6 +149,15 @@ func (a *Awaiter) onDiagnostics(_ context.Context, d *protocol.PublishDiagnostic pth := a.workdir.URIToPath(d.URI) a.state.diagnostics[pth] = d + + // Update any outstanding diagnostic listeners for this path. + for id, dc := range a.diagCollectors { + if dc.path == pth { + dc.params = append(dc.params, d) + a.diagCollectors[id] = dc + } + } + a.checkConditionsLocked() return nil } @@ -226,6 +242,32 @@ func (a *Awaiter) ListenToShownMessages() func() []*protocol.ShowMessageParams { } } +// ListenToDiagnostics registers a listener that records all diagnostic +// notifications for the given file path. Unlike the normal diagnostics +// tracking which overwrites per-file state, this records the full history +// including intermediate empty notifications (e.g. from eager clearing). +// Call the resulting func to deregister the listener and receive all +// notifications in order. +func (a *Awaiter) ListenToDiagnostics(path string) func() []*protocol.PublishDiagnosticsParams { + id := nextAwaiterRegistration.Add(1) + + a.mu.Lock() + defer a.mu.Unlock() + + if a.diagCollectors == nil { + a.diagCollectors = make(map[uint64]diagCollector) + } + a.diagCollectors[id] = diagCollector{path: path} + + return func() []*protocol.PublishDiagnosticsParams { + a.mu.Lock() + defer a.mu.Unlock() + dc := a.diagCollectors[id] + delete(a.diagCollectors, id) + return dc.params + } +} + func (a *Awaiter) onShowMessageRequest(_ context.Context, m *protocol.ShowMessageRequestParams) error { a.mu.Lock() defer a.mu.Unlock()