gopls/internal/lsp/cache: simplify named error values

Simplify error values that were defined in the source package, but never used in the source package. These should have reduced to ~nothing: the only remaining use-case for named errors was to implement support for legacy Go command behavior. Additional TODOs are left to clean this up. Change-Id: I4047974c5accc9fcbc4be921ba50d567bc364c2e Reviewed-on: https://go-review.googlesource.com/c/tools/+/543917 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Alan Donovan <adonovan@google.com>
golang · Nov 20, 2023 · e7d61d9 · e7d61d9
1 parent 3a915e4
commit e7d61d9
Show file tree

Hide file tree

Showing 6 changed files with 15 additions and 18 deletions.
diff --git a/gopls/internal/lsp/cache/mod_tidy.go b/gopls/internal/lsp/cache/mod_tidy.go
@@ -6,6 +6,7 @@ package cache
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"go/ast"
 	"go/token"
@@ -25,6 +26,9 @@ import (
 	"golang.org/x/tools/internal/memoize"
 )
 
+// This error is sought by mod diagnostics.
+var ErrNoModOnDisk = errors.New("go.mod file is not on disk")
+
 // ModTidy returns the go.mod file that would be obtained by running
 // "go mod tidy". Concurrent requests are combined into a single command.
 func (s *Snapshot) ModTidy(ctx context.Context, pm *ParsedModule) (*TidiedModule, error) {

diff --git a/gopls/internal/lsp/cache/pkg.go b/gopls/internal/lsp/cache/pkg.go
@@ -66,10 +66,6 @@ var (
 	AllowNetwork          = source.AllowNetwork
 	LoadWorkspace         = source.LoadWorkspace
 	WriteTemporaryModFile = source.WriteTemporaryModFile
-
-	// Errors
-	ErrNoModOnDisk = source.ErrNoModOnDisk
-	ErrViewExists  = source.ErrViewExists
 )
 
 // Functions

diff --git a/gopls/internal/lsp/cache/session.go b/gopls/internal/lsp/cache/session.go
@@ -6,6 +6,7 @@ package cache
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -73,6 +74,9 @@ func (s *Session) Cache() *Cache {
 	return s.cache
 }
 
+// TODO(rfindley): is the logic surrounding this error actually necessary?
+var ErrViewExists = errors.New("view already exists for session")
+
 // NewView creates a new View, returning it and its first snapshot. If a
 // non-empty tempWorkspace directory is provided, the View will record a copy
 // of its gopls workspace module in that directory, so that client tooling

diff --git a/gopls/internal/lsp/general.go b/gopls/internal/lsp/general.go
@@ -23,7 +23,6 @@ import (
 	"golang.org/x/tools/gopls/internal/lsp/cache"
 	"golang.org/x/tools/gopls/internal/lsp/debug"
 	"golang.org/x/tools/gopls/internal/lsp/protocol"
-	"golang.org/x/tools/gopls/internal/lsp/source"
 	"golang.org/x/tools/gopls/internal/settings"
 	"golang.org/x/tools/gopls/internal/telemetry"
 	"golang.org/x/tools/internal/event"
@@ -315,7 +314,7 @@ func (s *server) addFolders(ctx context.Context, folders []protocol.WorkspaceFol
 		work := s.progress.Start(ctx, "Setting up workspace", "Loading packages...", nil, nil)
 		snapshot, release, err := s.addView(ctx, folder.Name, uri)
 		if err != nil {
-			if err == source.ErrViewExists {
+			if err == cache.ErrViewExists {
 				continue
 			}
 			viewErrors[uri] = err

diff --git a/gopls/internal/lsp/mod/diagnostics.go b/gopls/internal/lsp/mod/diagnostics.go
@@ -115,7 +115,12 @@ func ModTidyDiagnostics(ctx context.Context, snapshot *cache.Snapshot, fh file.H
 	}
 
 	tidied, err := snapshot.ModTidy(ctx, pm)
-	if err != nil && !source.IsNonFatalGoModError(err) {
+	if err != nil && err != cache.ErrNoModOnDisk {
+		// TODO(rfindley): the check for ErrNoModOnDisk was historically determined
+		// to be benign, but may date back to the time when the Go command did not
+		// have overlay support.
+		//
+		// See if we can pass the overlay to the Go command, and eliminate this guard..
 		event.Error(ctx, fmt.Sprintf("tidy: diagnosing %s", pm.URI), err)
 	}
 	if err == nil {

diff --git a/gopls/internal/lsp/source/snapshot.go b/gopls/internal/lsp/source/snapshot.go
@@ -8,7 +8,6 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"go/ast"
 	"go/parser"
@@ -571,16 +570,6 @@ func RemoveIntermediateTestVariants(pmetas *[]*Metadata) {
 	*pmetas = res
 }
 
-var (
-	ErrViewExists            = errors.New("view already exists for session")
-	ErrTmpModfileUnsupported = errors.New("-modfile is unsupported for this Go version")
-	ErrNoModOnDisk           = errors.New("go.mod file is not on disk")
-)
-
-func IsNonFatalGoModError(err error) bool {
-	return err == ErrTmpModfileUnsupported || err == ErrNoModOnDisk
-}
-
 // Common parse modes; these should be reused wherever possible to increase
 // cache hits.
 const (