internal/lsp/cache: track explicit go.work files outside the workspace

In order to correctly process changes to the go.work file, the workspace
must know about GOWORK settings configured in the users environment.
Compute this when initializing the view, and thread this through to the
workspace.

At this point, workspace information is spread around in a few places.
Add some TODOs to clean this up.

Also remove some module data that was not used in
TestBrokenWorkspace_DuplicateModules.

Updates golang/go#53631

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

Author: findleyr

gopls/internal/regtest/workspace/broken_test.go

@@ -16,6 +16,10 @@ import (
 // This file holds various tests for UX with respect to broken workspaces.
 //
 // TODO: consolidate other tests here.
+//
+// TODO: write more tests:
+//  - an explicit GOWORK value that doesn't exist
+//  - using modules and/or GOWORK inside of GOPATH?
 
 // Test for golang/go#53933
 func TestBrokenWorkspace_DuplicateModules(t *testing.T) {
@@ -28,8 +32,6 @@ func TestBrokenWorkspace_DuplicateModules(t *testing.T) {
 module example.com/foo
 
 go 1.12
--- example.com/[email protected]/foo.go --
-package foo
 `
 
 	const src = `

gopls/internal/regtest/workspace/fromenv_test.go

@@ -0,0 +1,54 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package workspace
+
+import (
+	"testing"
+
+	. "golang.org/x/tools/internal/lsp/regtest"
+)
+
+// Test that setting go.work via environment variables or settings works.
+func TestUseGoWorkOutsideTheWorkspace(t *testing.T) {
+	const files = `
+-- work/a/go.mod --
+module a.com
+
+go 1.12
+-- work/a/a.go --
+package a
+-- work/b/go.mod --
+module b.com
+
+go 1.12
+-- work/b/b.go --
+package b
+
+func _() {
+	x := 1 // unused
+}
+-- config/go.work --
+go 1.18
+
+use (
+	$SANDBOX_WORKDIR/work/a
+	$SANDBOX_WORKDIR/work/b
+)
+`
+
+	WithOptions(
+		EnvVars{"GOWORK": "$SANDBOX_WORKDIR/config/go.work"},
+	).Run(t, files, func(t *testing.T, env *Env) {
+		// Even though work/b is not open, we should get its diagnostics as it is
+		// included in the workspace.
+		env.OpenFile("work/a/a.go")
+		env.Await(
+			OnceMet(
+				env.DoneWithOpen(),
+				env.DiagnosticAtRegexpWithMessage("work/b/b.go", "x := 1", "not used"),
+			),
+		)
+	})
+}

internal/lsp/cache/session.go

@@ -7,6 +7,7 @@ package cache
 import (
 	"context"
 	"fmt"
+	"os"
 	"strconv"
 	"strings"
 	"sync"
@@ -199,8 +200,14 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI,
 		}
 	}
 
+	explicitGowork := os.Getenv("GOWORK")
+	if v, ok := options.Env["GOWORK"]; ok {
+		explicitGowork = v
+	}
+	goworkURI := span.URIFromPath(explicitGowork)
+
 	// Build the gopls workspace, collecting active modules in the view.
-	workspace, err := newWorkspace(ctx, root, s, pathExcludedByFilterFunc(root.Filename(), wsInfo.gomodcache, options), wsInfo.userGo111Module == off, options.ExperimentalWorkspaceModule)
+	workspace, err := newWorkspace(ctx, root, goworkURI, s, pathExcludedByFilterFunc(root.Filename(), wsInfo.gomodcache, options), wsInfo.userGo111Module == off, options.ExperimentalWorkspaceModule)
 	if err != nil {
 		return nil, nil, func() {}, err
 	}
@@ -223,6 +230,7 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI,
 		filesByURI:           map[span.URI]*fileBase{},
 		filesByBase:          map[string][]*fileBase{},
 		rootURI:              root,
+		explicitGowork:       goworkURI,
 		workspaceInformation: *wsInfo,
 	}
 	v.importsState = &importsState{

internal/lsp/cache/snapshot.go

@@ -436,6 +436,12 @@ func (s *snapshot) goCommandInvocation(ctx context.Context, flags source.Invocat
 	s.view.optionsMu.Lock()
 	allowModfileModificationOption := s.view.options.AllowModfileModifications
 	allowNetworkOption := s.view.options.AllowImplicitNetworkAccess
+
+	// TODO(rfindley): this is very hard to follow, and may not even be doing the
+	// right thing: should inv.Env really trample view.options? Do we ever invoke
+	// this with a non-empty inv.Env?
+	//
+	// We should refactor to make it clearer that the correct env is being used.
 	inv.Env = append(append(append(os.Environ(), s.view.options.EnvSlice()...), inv.Env...), "GO111MODULE="+s.view.effectiveGo111Module)
 	inv.BuildFlags = append([]string{}, s.view.options.BuildFlags...)
 	s.view.optionsMu.Unlock()

internal/lsp/cache/view.go

@@ -27,6 +27,7 @@ import (
 	"golang.org/x/tools/internal/event"
 	"golang.org/x/tools/internal/gocommand"
 	"golang.org/x/tools/internal/imports"
+	"golang.org/x/tools/internal/lsp/bug"
 	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/lsp/source"
 	"golang.org/x/tools/internal/span"
@@ -98,6 +99,13 @@ type View struct {
 	// is just the folder. If we are in module mode, this is the module rootURI.
 	rootURI span.URI
 
+	// explicitGowork is, if non-empty, the URI for the explicit go.work file
+	// provided via the users environment.
+	//
+	// TODO(rfindley): this is duplicated in the workspace type. Refactor to
+	// eliminate this duplication.
+	explicitGowork span.URI
+
 	// workspaceInformation tracks various details about this view's
 	// environment variables, go version, and use of modules.
 	workspaceInformation
@@ -469,7 +477,7 @@ func (v *View) relevantChange(c source.FileModification) bool {
 	// TODO(rstambler): Make sure the go.work/gopls.mod files are always known
 	// to the view.
 	for _, src := range []workspaceSource{goWorkWorkspace, goplsModWorkspace} {
-		if c.URI == uriForSource(v.rootURI, src) {
+		if c.URI == uriForSource(v.rootURI, v.explicitGowork, src) {
 			return true
 		}
 	}
@@ -813,9 +821,13 @@ func (s *Session) getWorkspaceInformation(ctx context.Context, folder span.URI,
 	}
 	// The value of GOPACKAGESDRIVER is not returned through the go command.
 	gopackagesdriver := os.Getenv("GOPACKAGESDRIVER")
+	// TODO(rfindley): this looks wrong, or at least overly defensive. If the
+	// value of GOPACKAGESDRIVER is not returned from the go command... why do we
+	// look it up here?
 	for _, s := range env {
 		split := strings.SplitN(s, "=", 2)
 		if split[0] == "GOPACKAGESDRIVER" {
+			bug.Reportf("found GOPACKAGESDRIVER from the go command") // see note above
 			gopackagesdriver = split[1]
 		}
 	}

internal/lsp/cache/workspace.go

@@ -46,6 +46,19 @@ func (s workspaceSource) String() string {
 	}
 }
 
+// workspaceCommon holds immutable information about the workspace setup.
+//
+// TODO(rfindley): there is some redundancy here with workspaceInformation.
+// Reconcile these two types.
+type workspaceCommon struct {
+	root        span.URI
+	excludePath func(string) bool
+
+	// explicitGowork is, if non-empty, the URI for the explicit go.work file
+	// provided via the user's environment.
+	explicitGowork span.URI
+}
+
 // workspace tracks go.mod files in the workspace, along with the
 // gopls.mod file, to provide support for multi-module workspaces.
 //
@@ -58,8 +71,8 @@ func (s workspaceSource) String() string {
 // This type is immutable (or rather, idempotent), so that it may be shared
 // across multiple snapshots.
 type workspace struct {
-	root         span.URI
-	excludePath  func(string) bool
+	workspaceCommon
+
 	moduleSource workspaceSource
 
 	// activeModFiles holds the active go.mod files.
@@ -98,17 +111,21 @@ type workspace struct {
 //
 // TODO(rfindley): newWorkspace should perhaps never fail, relying instead on
 // the criticalError method to surface problems in the workspace.
-// TODO(rfindley): this function should accept the GOWORK value, if specified
-// by the user.
-func newWorkspace(ctx context.Context, root span.URI, fs source.FileSource, excludePath func(string) bool, go111moduleOff, useWsModule bool) (*workspace, error) {
+func newWorkspace(ctx context.Context, root, explicitGowork span.URI, fs source.FileSource, excludePath func(string) bool, go111moduleOff, useWsModule bool) (*workspace, error) {
 	ws := &workspace{
-		root:        root,
-		excludePath: excludePath,
+		workspaceCommon: workspaceCommon{
+			root:           root,
+			explicitGowork: explicitGowork,
+			excludePath:    excludePath,
+		},
 	}
 
 	// The user may have a gopls.mod or go.work file that defines their
 	// workspace.
-	if err := loadExplicitWorkspaceFile(ctx, ws, fs); err == nil {
+	//
+	// TODO(rfindley): if GO111MODULE=off, this looks wrong, though there are
+	// probably other problems.
+	if err := ws.loadExplicitWorkspaceFile(ctx, fs); err == nil {
 		return ws, nil
 	}
 
@@ -140,15 +157,15 @@ func newWorkspace(ctx context.Context, root span.URI, fs source.FileSource, excl
 // loadExplicitWorkspaceFile loads workspace information from go.work or
 // gopls.mod files, setting the active modules, mod file, and module source
 // accordingly.
-func loadExplicitWorkspaceFile(ctx context.Context, ws *workspace, fs source.FileSource) error {
+func (ws *workspace) loadExplicitWorkspaceFile(ctx context.Context, fs source.FileSource) error {
 	for _, src := range []workspaceSource{goWorkWorkspace, goplsModWorkspace} {
-		fh, err := fs.GetFile(ctx, uriForSource(ws.root, src))
+		fh, err := fs.GetFile(ctx, uriForSource(ws.root, ws.explicitGowork, src))
 		if err != nil {
 			return err
 		}
 		contents, err := fh.Read()
 		if err != nil {
-			continue
+			continue // TODO(rfindley): is it correct to proceed here?
 		}
 		var file *modfile.File
 		var activeModFiles map[span.URI]struct{}
@@ -313,15 +330,14 @@ func (w *workspace) Clone(ctx context.Context, changes map[span.URI]*fileChange,
 	// Clone the workspace. This may be discarded if nothing changed.
 	changed := false
 	result := &workspace{
-		root:           w.root,
-		moduleSource:   w.moduleSource,
-		knownModFiles:  make(map[span.URI]struct{}),
-		activeModFiles: make(map[span.URI]struct{}),
-		workFile:       w.workFile,
-		mod:            w.mod,
-		sum:            w.sum,
-		wsDirs:         w.wsDirs,
-		excludePath:    w.excludePath,
+		workspaceCommon: w.workspaceCommon,
+		moduleSource:    w.moduleSource,
+		knownModFiles:   make(map[span.URI]struct{}),
+		activeModFiles:  make(map[span.URI]struct{}),
+		workFile:        w.workFile,
+		mod:             w.mod,
+		sum:             w.sum,
+		wsDirs:          w.wsDirs,
 	}
 	for k, v := range w.knownModFiles {
 		result.knownModFiles[k] = v
@@ -391,7 +407,7 @@ func handleWorkspaceFileChanges(ctx context.Context, ws *workspace, changes map[
 	// exists or walk the filesystem if it has been deleted.
 	// go.work should override the gopls.mod if both exist.
 	for _, src := range []workspaceSource{goWorkWorkspace, goplsModWorkspace} {
-		uri := uriForSource(ws.root, src)
+		uri := uriForSource(ws.root, ws.explicitGowork, src)
 		// File opens/closes are just no-ops.
 		change, ok := changes[uri]
 		if !ok {
@@ -460,12 +476,15 @@ func handleWorkspaceFileChanges(ctx context.Context, ws *workspace, changes map[
 }
 
 // goplsModURI returns the URI for the gopls.mod file contained in root.
-func uriForSource(root span.URI, src workspaceSource) span.URI {
+func uriForSource(root, explicitGowork span.URI, src workspaceSource) span.URI {
 	var basename string
 	switch src {
 	case goplsModWorkspace:
 		basename = "gopls.mod"
 	case goWorkWorkspace:
+		if explicitGowork != "" {
+			return explicitGowork
+		}
 		basename = "go.work"
 	default:
 		return ""

internal/lsp/cache/workspace_test.go

@@ -280,7 +280,7 @@ replace gopls.test => ../../gopls.test2`, false},
 
 			fs := &osFileSource{}
 			excludeNothing := func(string) bool { return false }
-			w, err := newWorkspace(ctx, root, fs, excludeNothing, false, !test.legacyMode)
+			w, err := newWorkspace(ctx, root, "", fs, excludeNothing, false, !test.legacyMode)
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -325,7 +325,7 @@ func workspaceFromTxtar(t *testing.T, files string) (*workspace, func(), error)
 
 	fs := &osFileSource{}
 	excludeNothing := func(string) bool { return false }
-	workspace, err := newWorkspace(ctx, root, fs, excludeNothing, false, false)
+	workspace, err := newWorkspace(ctx, root, "", fs, excludeNothing, false, false)
 	return workspace, cleanup, err
 }