[tools] gopls/internal/lsp/source: make "chatty" diagnostics the default

[Robert Findley (Gerrit)]

Robert Findley submitted this change.

View Change

Approvals: Gopher Robot: TryBots succeeded Robert Findley: Run TryBots kokoro: gopls CI succeeded Alan Donovan: Looks good to me, approved

gopls/internal/lsp/source: make "chatty" diagnostics the default

This CL changes the default behavior of gopls to always publish
diagnostics for new file versions. In practice, this avoids stale
diagnostics in multiple LSP clients (golang/go#54983 has more details).

After this change, TestDownloadDeps was failing because it asserted on
the non-existence of published diagnostics. Update the test to treat an
empty diagnostic set the same as an unreceived diagnostic set.

Fixes golang/go#54983

Change-Id: I41ed2f859b748e14585e7feb53702d3f38dcd599
Reviewed-on: https://go-review.googlesource.com/c/tools/+/429935
Run-TryBot: Robert Findley <rfin...@google.com>
gopls-CI: kokoro <noreply...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Reviewed-by: Alan Donovan <adon...@google.com>
---
M gopls/internal/lsp/source/options.go
M gopls/internal/regtest/modfile/modfile_test.go
2 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/gopls/internal/lsp/source/options.go b/gopls/internal/lsp/source/options.go
index 45aeca9..b15d714 100644
--- a/gopls/internal/lsp/source/options.go
+++ b/gopls/internal/lsp/source/options.go
@@ -164,6 +164,7 @@
 				CompleteUnimported:      true,
 				CompletionDocumentation: true,
 				DeepCompletion:          true,
+				ChattyDiagnostics:       true,
 			},
 			Hooks: Hooks{
 				ComputeEdits:         myers.ComputeEdits,
@@ -587,11 +588,6 @@
 	// ChattyDiagnostics controls whether to report file diagnostics for each
 	// file change. If unset, gopls only reports diagnostics when they change, or
 	// when a file is opened or closed.
-	//
-	// TODO(rfindley): is seems that for many clients this should be true by
-	// default. For example, coc.nvim seems to get confused if diagnostics are
-	// not re-published. Switch the default to true after some period of internal
-	// testing.
 	ChattyDiagnostics bool
 }
 
@@ -824,7 +820,6 @@
 	o.ExperimentalUseInvalidMetadata = true
 	o.ExperimentalWatchedFileDelay = 50 * time.Millisecond
 	o.NewDiff = "checked"
-	o.ChattyDiagnostics = true
 }
 
 func (o *Options) enableAllExperimentMaps() {
diff --git a/gopls/internal/regtest/modfile/modfile_test.go b/gopls/internal/regtest/modfile/modfile_test.go
index 7da8830..eb3f966 100644
--- a/gopls/internal/regtest/modfile/modfile_test.go
+++ b/gopls/internal/regtest/modfile/modfile_test.go
@@ -1189,8 +1189,8 @@
 		)
 		env.ApplyQuickFixes("main.go", d.Diagnostics)
 		env.Await(
-			EmptyDiagnostics("main.go"),
-			NoDiagnostics("go.mod"),
+			EmptyOrNoDiagnostics("main.go"),
+			EmptyOrNoDiagnostics("go.mod"),
 		)
 	})
 }

To view, visit change 429935. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: tools

Gerrit-Branch: master

Gerrit-Change-Id: I41ed2f859b748e14585e7feb53702d3f38dcd599

Gerrit-Change-Number: 429935

Gerrit-PatchSet: 4

Gerrit-Owner: Robert Findley <rfin...@google.com>

Gerrit-Reviewer: Alan Donovan <adon...@google.com>

Gerrit-Reviewer: Gopher Robot <go...@golang.org>

Gerrit-Reviewer: Robert Findley <rfin...@google.com>

Gerrit-Reviewer: kokoro <noreply...@google.com>

Gerrit-MessageType: merged