gopls: remove the experimentalWorkspaceModule mode

Remove the experimentalWorkspaceModule setting, and all of the roots it
has planted. Specifically, this removes:
- the gopls.mod and filesystem workspace modes
- the command to generate a gopls.mod
- the need to track separate workspace modes entirely, as we align with
  the Go command and don't need any additional logic
- the need to maintain a workspace directory
- the 'cache.workspace' abstraction entirely; now we can just track
  workspace modules

Along the way, further simplify the treatment of view workspace
information. In particular, just use the value of GOWORK returned by the
go command, rather than computing it ourselves. This means that we may
need to re-run `go env` while processing a change to go.mod or go.work
files. If that proves to be problematic, we can improve it in the future.

Many workspace tests had to be restricted to just Go 1.18+, because we
no longer fake go.work support at earlier Go versions.

Fixes golang/go#55331

Change-Id: I15ad58f548295727a51b99f43c7572b066f9df07
Reviewed-on: https://go-review.googlesource.com/c/tools/+/458116
gopls-CI: kokoro <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Robert Findley <[email protected]>

Author: findleyr

gopls/doc/commands.md

@@ -147,20 +147,6 @@ Args:
 }
 ```
 
-### **Generate gopls.mod**
-Identifier: `gopls.generate_gopls_mod`
-
-(Re)generate the gopls.mod file for a workspace.
-
-Args:
-
-```
-{
-	// The file URI.
-	"URI": string,
-}
-```
-
 ### **go get a package**
 Identifier: `gopls.go_get_package`
 

gopls/doc/settings.md

@@ -116,18 +116,6 @@ a go.mod file, narrowing the scope to that directory if it exists.
 
 Default: `true`.
 
-#### **experimentalWorkspaceModule** *bool*
-
-**This setting is experimental and may be deleted.**
-
-experimentalWorkspaceModule opts a user into the experimental support
-for multi-module workspaces.
-
-Deprecated: this feature is deprecated and will be removed in a future
-version of gopls (https://go.dev/issue/55331).
-
-Default: `false`.
-
 #### **experimentalPackageCacheKey** *bool*
 
 **This setting is experimental and may be deleted.**

gopls/internal/lsp/cache/check.go

@@ -357,13 +357,9 @@ func typeCheckImpl(ctx context.Context, snapshot *snapshot, goFiles, compiledGoF
 	// If this is a replaced module in the workspace, the version is
 	// meaningless, and we don't want clients to access it.
 	if m.Module != nil {
-		version := m.Module.Version
-		if source.IsWorkspaceModuleVersion(version) {
-			version = ""
-		}
 		pkg.version = &module.Version{
 			Path:    m.Module.Path,
-			Version: version,
+			Version: m.Module.Version,
 		}
 	}
 

gopls/internal/lsp/cache/imports.go

@@ -7,7 +7,6 @@ package cache
 import (
 	"context"
 	"fmt"
-	"os"
 	"reflect"
 	"strings"
 	"sync"
@@ -25,12 +24,18 @@ type importsState struct {
 
 	mu                     sync.Mutex
 	processEnv             *imports.ProcessEnv
-	cleanupProcessEnv      func()
 	cacheRefreshDuration   time.Duration
 	cacheRefreshTimer      *time.Timer
 	cachedModFileHash      source.Hash
 	cachedBuildFlags       []string
 	cachedDirectoryFilters []string
+
+	// runOnce records whether runProcessEnvFunc has been called at least once.
+	// This is necessary to avoid resetting state before the process env is
+	// populated.
+	//
+	// TODO(rfindley): this shouldn't be necessary.
+	runOnce bool
 }
 
 func (s *importsState) runProcessEnvFunc(ctx context.Context, snapshot *snapshot, fn func(*imports.Options) error) error {
@@ -43,7 +48,7 @@ func (s *importsState) runProcessEnvFunc(ctx context.Context, snapshot *snapshot
 	//
 	// TODO(rfindley): consider instead hashing on-disk modfiles here.
 	var modFileHash source.Hash
-	for m := range snapshot.workspace.ActiveModFiles() {
+	for m := range snapshot.workspaceModFiles {
 		fh, err := snapshot.GetFile(ctx, m)
 		if err != nil {
 			return err
@@ -70,22 +75,21 @@ func (s *importsState) runProcessEnvFunc(ctx context.Context, snapshot *snapshot
 		// As a special case, skip cleanup the first time -- we haven't fully
 		// initialized the environment yet and calling GetResolver will do
 		// unnecessary work and potentially mess up the go.mod file.
-		if s.cleanupProcessEnv != nil {
+		if s.runOnce {
 			if resolver, err := s.processEnv.GetResolver(); err == nil {
 				if modResolver, ok := resolver.(*imports.ModuleResolver); ok {
 					modResolver.ClearForNewMod()
 				}
 			}
-			s.cleanupProcessEnv()
 		}
+
 		s.cachedModFileHash = modFileHash
 		s.cachedBuildFlags = currentBuildFlags
 		s.cachedDirectoryFilters = currentDirectoryFilters
-		var err error
-		s.cleanupProcessEnv, err = s.populateProcessEnv(ctx, snapshot)
-		if err != nil {
+		if err := s.populateProcessEnv(ctx, snapshot); err != nil {
 			return err
 		}
+		s.runOnce = true
 	}
 
 	// Run the user function.
@@ -120,7 +124,7 @@ func (s *importsState) runProcessEnvFunc(ctx context.Context, snapshot *snapshot
 
 // populateProcessEnv sets the dynamically configurable fields for the view's
 // process environment. Assumes that the caller is holding the s.view.importsMu.
-func (s *importsState) populateProcessEnv(ctx context.Context, snapshot *snapshot) (cleanup func(), err error) {
+func (s *importsState) populateProcessEnv(ctx context.Context, snapshot *snapshot) error {
 	pe := s.processEnv
 
 	if snapshot.view.Options().VerboseOutput {
@@ -141,7 +145,7 @@ func (s *importsState) populateProcessEnv(ctx context.Context, snapshot *snapsho
 		WorkingDir: snapshot.view.workingDir().Filename(),
 	})
 	if err != nil {
-		return nil, err
+		return err
 	}
 
 	pe.BuildFlags = inv.BuildFlags
@@ -156,29 +160,9 @@ func (s *importsState) populateProcessEnv(ctx context.Context, snapshot *snapsho
 	}
 	// We don't actually use the invocation, so clean it up now.
 	cleanupInvocation()
-
-	// If the snapshot uses a synthetic workspace directory, create a copy for
-	// the lifecycle of the importsState.
-	//
-	// Notably, we cannot use the snapshot invocation working directory, as that
-	// is tied to the lifecycle of the snapshot.
-	//
-	// Otherwise return a no-op cleanup function.
-	cleanup = func() {}
-	if snapshot.usesWorkspaceDir() {
-		tmpDir, err := makeWorkspaceDir(ctx, snapshot.workspace, snapshot)
-		if err != nil {
-			return nil, err
-		}
-		pe.WorkingDir = tmpDir
-		cleanup = func() {
-			os.RemoveAll(tmpDir) // ignore error
-		}
-	} else {
-		pe.WorkingDir = snapshot.view.workingDir().Filename()
-	}
-
-	return cleanup, nil
+	// TODO(rfindley): should this simply be inv.WorkingDir?
+	pe.WorkingDir = snapshot.view.workingDir().Filename()
+	return nil
 }
 
 func (s *importsState) refreshProcessEnv() {
@@ -202,11 +186,3 @@ func (s *importsState) refreshProcessEnv() {
 	s.cacheRefreshTimer = nil
 	s.mu.Unlock()
 }
-
-func (s *importsState) destroy() {
-	s.mu.Lock()
-	if s.cleanupProcessEnv != nil {
-		s.cleanupProcessEnv()
-	}
-	s.mu.Unlock()
-}

gopls/internal/lsp/cache/load.go

@@ -9,8 +9,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"io/ioutil"
-	"os"
 	"path/filepath"
 	"sort"
 	"strings"
@@ -287,22 +285,17 @@ func (m *moduleErrorMap) Error() string {
 // TODO(rfindley): separate workspace diagnostics from critical workspace
 // errors.
 func (s *snapshot) workspaceLayoutError(ctx context.Context) (error, []*source.Diagnostic) {
-	// TODO(rfindley): do we really not want to show a critical error if the user
-	// has no go.mod files?
-	if len(s.workspace.getKnownModFiles()) == 0 {
-		return nil, nil
-	}
-
 	// TODO(rfindley): both of the checks below should be delegated to the workspace.
+
 	if s.view.effectiveGO111MODULE() == off {
 		return nil, nil
 	}
-	if s.workspace.moduleSource != legacyWorkspace {
-		return nil, nil
-	}
 
-	// If the user has one module per view, there is nothing to warn about.
-	if s.ValidBuildConfiguration() && len(s.workspace.getKnownModFiles()) == 1 {
+	// If the user is using a go.work file, we assume that they know what they
+	// are doing.
+	//
+	// TODO(golang/go#53880): improve orphaned file diagnostics when using go.work.
+	if s.view.gowork != "" {
 		return nil, nil
 	}
 
@@ -335,10 +328,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.ActiveModFiles()) == 1 {
+	if len(s.workspaceModFiles) == 1 {
 		// Get the active root go.mod file to compare against.
 		var rootMod string
-		for uri := range s.workspace.ActiveModFiles() {
+		for uri := range s.workspaceModFiles {
 			rootMod = uri.Filename()
 		}
 		rootDir := filepath.Dir(rootMod)
@@ -413,55 +406,6 @@ func (s *snapshot) applyCriticalErrorToFiles(ctx context.Context, msg string, fi
 	return srcDiags
 }
 
-// getWorkspaceDir returns the URI for the workspace directory
-// associated with this snapshot. The workspace directory is a
-// temporary directory containing the go.mod file computed from all
-// active modules.
-func (s *snapshot) getWorkspaceDir(ctx context.Context) (span.URI, error) {
-	s.mu.Lock()
-	dir, err := s.workspaceDir, s.workspaceDirErr
-	s.mu.Unlock()
-	if dir == "" && err == nil { // cache miss
-		dir, err = makeWorkspaceDir(ctx, s.workspace, s)
-		s.mu.Lock()
-		s.workspaceDir, s.workspaceDirErr = dir, err
-		s.mu.Unlock()
-	}
-	return span.URIFromPath(dir), err
-}
-
-// makeWorkspaceDir creates a temporary directory containing a go.mod
-// and go.sum file for each module in the workspace.
-// Note: snapshot's mutex must be unlocked for it to satisfy FileSource.
-func makeWorkspaceDir(ctx context.Context, workspace *workspace, fs source.FileSource) (string, error) {
-	file, err := workspace.modFile(ctx, fs)
-	if err != nil {
-		return "", err
-	}
-	modContent, err := file.Format()
-	if err != nil {
-		return "", err
-	}
-	sumContent, err := workspace.sumFile(ctx, fs)
-	if err != nil {
-		return "", err
-	}
-	tmpdir, err := ioutil.TempDir("", "gopls-workspace-mod")
-	if err != nil {
-		return "", err
-	}
-	for name, content := range map[string][]byte{
-		"go.mod": modContent,
-		"go.sum": sumContent,
-	} {
-		if err := ioutil.WriteFile(filepath.Join(tmpdir, name), content, 0644); err != nil {
-			os.RemoveAll(tmpdir) // ignore error
-			return "", err
-		}
-	}
-	return tmpdir, nil
-}
-
 // 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.
@@ -628,9 +572,13 @@ func containsPackageLocked(s *snapshot, m *source.Metadata) bool {
 	// Otherwise if the package has a module it must be an active module (as
 	// defined by the module root or go.work file) and at least one file must not
 	// be filtered out by directoryFilters.
-	if m.Module != nil && s.workspace.moduleSource != legacyWorkspace {
+	//
+	// TODO(rfindley): revisit this function. We should not need to predicate on
+	// gowork != "". It should suffice to consider workspace mod files (also, we
+	// will hopefully eliminate the concept of a workspace package soon).
+	if m.Module != nil && s.view.gowork != "" {
 		modURI := span.URIFromPath(m.Module.GoMod)
-		_, ok := s.workspace.activeModFiles[modURI]
+		_, ok := s.workspaceModFiles[modURI]
 		if !ok {
 			return false
 		}

gopls/internal/lsp/cache/mod.go

@@ -383,11 +383,6 @@ func (s *snapshot) matchErrorToModule(ctx context.Context, pm *source.ParsedModu
 
 	for i := len(matches) - 1; i >= 0; i-- {
 		ver := module.Version{Path: matches[i][1], Version: matches[i][2]}
-		// Any module versions that come from the workspace module should not
-		// be shown to the user.
-		if source.IsWorkspaceModuleVersion(ver.Version) {
-			continue
-		}
 		if err := module.Check(ver.Path, ver.Version); err != nil {
 			continue
 		}
@@ -424,11 +419,6 @@ func (s *snapshot) goCommandDiagnostic(pm *source.ParsedModule, spn span.Span, g
 	var innermost *module.Version
 	for i := len(matches) - 1; i >= 0; i-- {
 		ver := module.Version{Path: matches[i][1], Version: matches[i][2]}
-		// Any module versions that come from the workspace module should not
-		// be shown to the user.
-		if source.IsWorkspaceModuleVersion(ver.Version) {
-			continue
-		}
 		if err := module.Check(ver.Path, ver.Version); err != nil {
 			continue
 		}