gopls/internal/lsp/cache: simplify critical errors

The critical error logic was hard to follow, and ill defined because it
was constructed in two places: once during initialization, and another
time in Snapshot.CriticalError.

CriticalError is now rebranded as an InitializationError, and
constructed only during snapshot initialization. It covers a load error,
and an unparsable go.work or go.mod file. Critical errors are applied
to orphaned files.

For golang/go#57979

Change-Id: Ib3cdf602954202be0c87594c26dbbd0ff7e6458a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/553097
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Alan Donovan <adonovan@google.com>

Author: findleyr

gopls/internal/lsp/cache/check.go

@@ -203,7 +203,7 @@ func (s *Snapshot) resolveImportGraph() (*importGraph, error) {
 	s.mu.Unlock()
 
 	openPackages := make(map[PackageID]bool)
-	for _, fh := range s.overlays() {
+	for _, fh := range s.Overlays() {
 		mps, err := s.MetadataForFile(ctx, fh.URI())
 		if err != nil {
 			return nil, err

gopls/internal/lsp/cache/diagnostics.go

@@ -12,14 +12,18 @@ import (
 	"golang.org/x/tools/gopls/internal/util/bug"
 )
 
-// A CriticalError is a workspace-wide error that generally prevents gopls from
-// functioning correctly. In the presence of critical errors, other diagnostics
-// in the workspace may not make sense.
-type CriticalError struct {
+// A InitializationError is an error that causes snapshot initialization to fail.
+// It is either the error returned from go/packages.Load, or an error parsing a
+// workspace go.work or go.mod file.
+//
+// Such an error generally indicates that the View is malformed, and will never
+// be usable.
+type InitializationError struct {
 	// MainError is the primary error. Must be non-nil.
 	MainError error
 
-	// Diagnostics contains any supplemental (structured) diagnostics.
+	// Diagnostics contains any supplemental (structured) diagnostics extracted
+	// from the load error.
 	Diagnostics map[protocol.DocumentURI][]*Diagnostic
 }
 

gopls/internal/lsp/cache/load.go

@@ -297,82 +297,6 @@ func (m *moduleErrorMap) Error() string {
 	return buf.String()
 }
 
-// workspaceLayoutError returns an error describing a misconfiguration of the
-// workspace, along with related diagnostic.
-//
-// The unusual argument ordering of results is intentional: if the resulting
-// error is nil, so must be the resulting diagnostics.
-//
-// If ctx is cancelled, it may return ctx.Err(), nil.
-//
-// TODO(rfindley): separate workspace diagnostics from critical workspace
-// errors.
-func (s *Snapshot) workspaceLayoutError(ctx context.Context) (error, []*Diagnostic) {
-	// TODO(rfindley): both of the checks below should be delegated to the workspace.
-
-	if s.view.adjustedGO111MODULE() == "off" {
-		return nil, nil
-	}
-
-	// If the user is using a go.work file, assume that they know what they are
-	// doing.
-	//
-	// TODO(golang/go#53880): improve orphaned file diagnostics when using go.work.
-	if s.view.typ == GoWorkView {
-		return nil, nil
-	}
-
-	// Apply diagnostics about the workspace configuration to relevant open
-	// files.
-	openFiles := s.overlays()
-
-	// If the snapshot does not have a valid build configuration, it may be
-	// that the user has opened a directory that contains multiple modules.
-	// Check for that an warn about it.
-	if s.view.typ == AdHocView {
-		msg := `gopls was not able to find modules in your workspace.
-When outside of GOPATH, gopls needs to know which modules you are working on.
-You can fix this by opening your workspace to a folder inside a Go module, or
-by using a go.work file to specify multiple modules.
-See the documentation for more information on setting up your workspace:
-https://github.com/golang/tools/blob/master/gopls/doc/workspace.md.`
-		return fmt.Errorf(msg), s.applyCriticalErrorToFiles(ctx, msg, openFiles)
-	}
-
-	return nil, nil
-}
-
-func (s *Snapshot) applyCriticalErrorToFiles(ctx context.Context, msg string, files []*Overlay) []*Diagnostic {
-	var srcDiags []*Diagnostic
-	for _, fh := range files {
-		// Place the diagnostics on the package or module declarations.
-		var rng protocol.Range
-		switch s.FileKind(fh) {
-		case file.Go:
-			if pgf, err := s.ParseGo(ctx, fh, ParseHeader); err == nil {
-				// Check that we have a valid `package foo` range to use for positioning the error.
-				if pgf.File.Package.IsValid() && pgf.File.Name != nil && pgf.File.Name.End().IsValid() {
-					rng, _ = pgf.PosRange(pgf.File.Package, pgf.File.Name.End())
-				}
-			}
-		case file.Mod:
-			if pmf, err := s.ParseMod(ctx, fh); err == nil {
-				if mod := pmf.File.Module; mod != nil && mod.Syntax != nil {
-					rng, _ = pmf.Mapper.OffsetRange(mod.Syntax.Start.Byte, mod.Syntax.End.Byte)
-				}
-			}
-		}
-		srcDiags = append(srcDiags, &Diagnostic{
-			URI:      fh.URI(),
-			Range:    rng,
-			Severity: protocol.SeverityError,
-			Source:   ListError,
-			Message:  msg,
-		})
-	}
-	return srcDiags
-}
-
 // buildMetadata populates the updates map with metadata updates to
 // apply, based on the given pkg. It recurs through pkg.Imports to ensure that
 // metadata exists for all dependencies.

gopls/internal/lsp/cache/mod_tidy.go

@@ -72,15 +72,6 @@ func (s *Snapshot) ModTidy(ctx context.Context, pm *ParsedModule) (*TidiedModule
 			}
 		}
 
-		if criticalErr := s.CriticalError(ctx); criticalErr != nil {
-			return &TidiedModule{
-				Diagnostics: criticalErr.Diagnostics[fh.URI()],
-			}, nil
-		}
-		if ctx.Err() != nil { // must check ctx after GetCriticalError
-			return nil, ctx.Err()
-		}
-
 		if err := s.awaitLoaded(ctx); err != nil {
 			return nil, err
 		}