internal/lsp/cache: simplify importsState modfile hashing logic

While checking for changes to go.mod files in the importsState, there is
no reason for special handling based on workspace mode: we can simply
hash all active modfiles. This moves us towards an improved API for the
workspace: it should simply be responsible for tracking active modfiles.

This also incidentally avoids the panic reported in golang/go#55837.

Fixes golang/go#55837

Change-Id: I8cb345d1689be12382683186afe3f9addb19d467
Reviewed-on: https://go-review.googlesource.com/c/tools/+/447956
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

gopls/internal/lsp/cache/imports.go

@@ -13,11 +13,11 @@ import (
 	"sync"
 	"time"
 
+	"golang.org/x/tools/gopls/internal/lsp/source"
 	"golang.org/x/tools/internal/event"
 	"golang.org/x/tools/internal/event/keys"
 	"golang.org/x/tools/internal/gocommand"
 	"golang.org/x/tools/internal/imports"
-	"golang.org/x/tools/gopls/internal/lsp/source"
 )
 
 type importsState struct {
@@ -37,33 +37,18 @@ func (s *importsState) runProcessEnvFunc(ctx context.Context, snapshot *snapshot
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
-	// Find the hash of the active mod file, if any. Using the unsaved content
+	// Find the hash of active mod files, if any. Using the unsaved content
 	// is slightly wasteful, since we'll drop caches a little too often, but
 	// the mod file shouldn't be changing while people are autocompleting.
-	var modFileHash source.Hash
-	// If we are using 'legacyWorkspace' mode, we can just read the modfile from
-	// the snapshot. Otherwise, we need to get the synthetic workspace mod file.
 	//
-	// TODO(rfindley): we should be able to just always use the synthetic
-	// workspace module, or alternatively use the go.work file.
-	if snapshot.workspace.moduleSource == legacyWorkspace {
-		for m := range snapshot.workspace.getActiveModFiles() { // range to access the only element
-			modFH, err := snapshot.GetFile(ctx, m)
-			if err != nil {
-				return err
-			}
-			modFileHash = modFH.FileIdentity().Hash
-		}
-	} else {
-		modFile, err := snapshot.workspace.modFile(ctx, snapshot)
+	// TODO(rfindley): consider instead hashing on-disk modfiles here.
+	var modFileHash source.Hash
+	for m := range snapshot.workspace.ActiveModFiles() {
+		fh, err := snapshot.GetFile(ctx, m)
 		if err != nil {
 			return err
 		}
-		modBytes, err := modFile.Format()
-		if err != nil {
-			return err
-		}
-		modFileHash = source.HashOf(modBytes)
+		modFileHash.XORWith(fh.FileIdentity().Hash)
 	}
 
 	// view.goEnv is immutable -- changes make a new view. Options can change.

gopls/internal/lsp/cache/load.go

@@ -352,10 +352,10 @@ https://github.com/golang/tools/blob/master/gopls/doc/workspace.md.`
 	// If the user has one active go.mod file, they may still be editing files
 	// in nested modules. Check the module of each open file and add warnings
 	// that the nested module must be opened as a workspace folder.
-	if len(s.workspace.getActiveModFiles()) == 1 {
+	if len(s.workspace.ActiveModFiles()) == 1 {
 		// Get the active root go.mod file to compare against.
 		var rootModURI span.URI
-		for uri := range s.workspace.getActiveModFiles() {
+		for uri := range s.workspace.ActiveModFiles() {
 			rootModURI = uri
 		}
 		nestedModules := map[string][]source.VersionedFileHandle{}

gopls/internal/lsp/cache/snapshot.go

@@ -250,7 +250,7 @@ func (s *snapshot) FileSet() *token.FileSet {
 
 func (s *snapshot) ModFiles() []span.URI {
 	var uris []span.URI
-	for modURI := range s.workspace.getActiveModFiles() {
+	for modURI := range s.workspace.ActiveModFiles() {
 		uris = append(uris, modURI)
 	}
 	return uris
@@ -281,7 +281,7 @@ func (s *snapshot) ValidBuildConfiguration() bool {
 	}
 	// Check if the user is working within a module or if we have found
 	// multiple modules in the workspace.
-	if len(s.workspace.getActiveModFiles()) > 0 {
+	if len(s.workspace.ActiveModFiles()) > 0 {
 		return true
 	}
 	// The user may have a multiple directories in their GOPATH.
@@ -308,7 +308,7 @@ func (s *snapshot) workspaceMode() workspaceMode {
 	// If the view is not in a module and contains no modules, but still has a
 	// valid workspace configuration, do not create the workspace module.
 	// It could be using GOPATH or a different build system entirely.
-	if len(s.workspace.getActiveModFiles()) == 0 && validBuildConfiguration {
+	if len(s.workspace.ActiveModFiles()) == 0 && validBuildConfiguration {
 		return mode
 	}
 	mode |= moduleMode
@@ -480,7 +480,7 @@ func (s *snapshot) goCommandInvocation(ctx context.Context, flags source.Invocat
 	if mode == source.LoadWorkspace {
 		switch s.workspace.moduleSource {
 		case legacyWorkspace:
-			for m := range s.workspace.getActiveModFiles() { // range to access the only element
+			for m := range s.workspace.ActiveModFiles() { // range to access the only element
 				modURI = m
 			}
 		case goWorkWorkspace:

gopls/internal/lsp/cache/view.go

@@ -581,13 +581,13 @@ func (v *View) Session() *Session {
 func (s *snapshot) IgnoredFile(uri span.URI) bool {
 	filename := uri.Filename()
 	var prefixes []string
-	if len(s.workspace.getActiveModFiles()) == 0 {
+	if len(s.workspace.ActiveModFiles()) == 0 {
 		for _, entry := range filepath.SplitList(s.view.gopath) {
 			prefixes = append(prefixes, filepath.Join(entry, "src"))
 		}
 	} else {
 		prefixes = append(prefixes, s.view.gomodcache)
-		for m := range s.workspace.getActiveModFiles() {
+		for m := range s.workspace.ActiveModFiles() {
 			prefixes = append(prefixes, dirURI(m).Filename())
 		}
 	}
@@ -679,8 +679,8 @@ func (s *snapshot) loadWorkspace(ctx context.Context, firstAttempt bool) {
 		})
 	}
 
-	if len(s.workspace.getActiveModFiles()) > 0 {
-		for modURI := range s.workspace.getActiveModFiles() {
+	if len(s.workspace.ActiveModFiles()) > 0 {
+		for modURI := range s.workspace.ActiveModFiles() {
 			// Be careful not to add context cancellation errors as critical module
 			// errors.
 			fh, err := s.GetFile(ctx, modURI)

gopls/internal/lsp/cache/workspace.go

@@ -73,6 +73,7 @@ type workspaceCommon struct {
 type workspace struct {
 	workspaceCommon
 
+	// The source of modules in this workspace.
 	moduleSource workspaceSource
 
 	// activeModFiles holds the active go.mod files.
@@ -192,11 +193,13 @@ func (ws *workspace) loadExplicitWorkspaceFile(ctx context.Context, fs source.Fi
 
 var noHardcodedWorkspace = errors.New("no hardcoded workspace")
 
+// TODO(rfindley): eliminate getKnownModFiles.
 func (w *workspace) getKnownModFiles() map[span.URI]struct{} {
 	return w.knownModFiles
 }
 
-func (w *workspace) getActiveModFiles() map[span.URI]struct{} {
+// ActiveModFiles returns the set of active mod files for the current workspace.
+func (w *workspace) ActiveModFiles() map[span.URI]struct{} {
 	return w.activeModFiles
 }
 

gopls/internal/lsp/cache/workspace_test.go

@@ -386,7 +386,7 @@ func checkState(ctx context.Context, t *testing.T, fs source.FileSource, rel fak
 		t.Errorf("module source = %v, want %v", got.moduleSource, want.source)
 	}
 	modules := make(map[span.URI]struct{})
-	for k := range got.getActiveModFiles() {
+	for k := range got.ActiveModFiles() {
 		modules[k] = struct{}{}
 	}
 	for _, modPath := range want.modules {