internal/lsp: mitigate possiibility of a slow code action

Related to microsoft/vscode-go#3063.
A small collection of fixes: (1) Don't reload packages with missing
dependencies unless imports change. (2) Show the error message from the
initial workspace load to the user.

Also, a small fix for deletion suggested fixes - upgrading to the latest
version of the VS Code language client revealed that I had made a
mistake there.

Change-Id: I7ab944ed27dc3da24a79d5d311531a1eb2b73102
Reviewed-on: https://go-review.googlesource.com/c/tools/+/221217
Run-TryBot: Rebecca Stambler <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Heschi Kreinick <[email protected]>

Author: stamblerre

internal/lsp/cache/snapshot.go

@@ -145,10 +145,13 @@ func (s *snapshot) PackageHandles(ctx context.Context, fh source.FileHandle) ([]
 		// Reload package metadata if any of the metadata has missing
 		// dependencies, in case something has changed since the last time we
 		// reloaded it.
-		if m := s.getMetadata(id); m == nil || len(m.missingDeps) > 0 {
+		if m := s.getMetadata(id); m == nil {
 			reload = true
 			break
 		}
+		// TODO(golang/go#36918): Previously, we would reload any package with
+		// missing dependencies. This is expensive and results in too many
+		// calls to packages.Load. Determine what we should do instead.
 	}
 	if reload {
 		if err := s.load(ctx, fileURI(fh.Identity().URI)); err != nil {

internal/lsp/diagnostics.go

@@ -6,6 +6,7 @@ package lsp
 
 import (
 	"context"
+	"fmt"
 	"strings"
 	"sync"
 
@@ -86,6 +87,18 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, alwaysA
 		return nil
 	}
 	if err != nil {
+		// If we encounter a genuine error when getting workspace packages,
+		// notify the user.
+		s.showedInitialErrorMu.Lock()
+		if !s.showedInitialError {
+			err := s.client.ShowMessage(ctx, &protocol.ShowMessageParams{
+				Type:    protocol.Error,
+				Message: fmt.Sprintf("Your workspace is misconfigured: %s. Please see https://github.com/golang/tools/blob/master/gopls/doc/troubleshooting.md for more information or file an issue (https://github.com/golang/go/issues/new) if you believe this is a mistake.", err.Error()),
+			})
+			s.showedInitialError = err == nil
+		}
+		s.showedInitialErrorMu.Unlock()
+
 		log.Error(ctx, "diagnose: no workspace packages", err, telemetry.Snapshot.Of(snapshot.ID()), telemetry.Directory.Of(snapshot.View().Folder))
 		return nil
 	}

internal/lsp/server.go

@@ -58,6 +58,9 @@ type Server struct {
 	deliveredMu sync.Mutex
 	delivered   map[span.URI]sentDiagnostics
 
+	showedInitialError   bool
+	showedInitialErrorMu sync.Mutex
+
 	// diagnosticsSema limits the concurrency of diagnostics runs, which can be expensive.
 	diagnosticsSema chan struct{}
 }

internal/lsp/source/diagnostics.go

@@ -355,7 +355,7 @@ func onlyDeletions(fixes []SuggestedFix) bool {
 			}
 		}
 	}
-	return true
+	return len(fixes) > 0
 }
 
 // hasUndeclaredErrors returns true if a package has a type error