internal/lsp: update the broken workspace message to mention go.work

For users of Go >= 1.18, update the error message presented to users
with broken workspaces to suggest using a go.work. For older Go
versions, suggest updating to 1.18+.

Also:
- add support for changing workspace folders from the fake editor
- update the test to confirm that changing workspace folders resolves
  the error message
- inline validBuildConfiguration, and make a few TODOs for how the code
  could be further cleaned up

Fixes golang/go#53882

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

Author: findleyr

gopls/internal/regtest/workspace/broken_test.go

@@ -169,6 +169,14 @@ const F = named.D - 3
 }
 
 func TestMultipleModules_Warning(t *testing.T) {
+	msgForVersion := func(ver int) string {
+		if ver >= 18 {
+			return `gopls was not able to find modules in your workspace.`
+		} else {
+			return `gopls requires a module at the root of your workspace.`
+		}
+	}
+
 	const modules = `
 -- a/go.mod --
 module a.com
@@ -189,13 +197,34 @@ package b
 				Modes(Default),
 				EnvVars{"GO111MODULE": go111module},
 			).Run(t, modules, func(t *testing.T, env *Env) {
+				ver := env.GoVersion()
+				msg := msgForVersion(ver)
 				env.OpenFile("a/a.go")
 				env.OpenFile("b/go.mod")
 				env.Await(
 					env.DiagnosticAtRegexp("a/a.go", "package a"),
 					env.DiagnosticAtRegexp("b/go.mod", "module b.com"),
-					OutstandingWork(lsp.WorkspaceLoadFailure, "gopls requires a module at the root of your workspace."),
+					OutstandingWork(lsp.WorkspaceLoadFailure, msg),
 				)
+
+				// Changing the workspace folders to the valid modules should resolve
+				// the workspace error.
+				env.ChangeWorkspaceFolders("a", "b")
+				env.Await(NoOutstandingWork())
+
+				env.ChangeWorkspaceFolders(".")
+
+				// TODO(rfindley): when GO111MODULE=auto, we need to open or change a
+				// file here in order to detect a critical error. This is because gopls
+				// has forgotten about a/a.go, and therefor doesn't hit the heuristic
+				// "all packages are command-line-arguments".
+				//
+				// This is broken, and could be fixed by adjusting the heuristic to
+				// account for the scenario where there are *no* workspace packages, or
+				// (better) trying to get workspace packages for each open file. See
+				// also golang/go#54261.
+				env.OpenFile("b/b.go")
+				env.Await(OutstandingWork(lsp.WorkspaceLoadFailure, msg))
 			})
 		})
 	}

internal/lsp/cache/load.go

@@ -283,15 +283,20 @@ func (m *moduleErrorMap) Error() string {
 // workspaceLayoutErrors returns a diagnostic for every open file, as well as
 // an error message if there are no open files.
 func (s *snapshot) workspaceLayoutError(ctx context.Context) *source.CriticalError {
+	// 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
 	}
+
+	// TODO(rfindley): both of the checks below should be delegated to the workspace.
 	if s.view.userGo111Module == off {
 		return nil
 	}
 	if s.workspace.moduleSource != legacyWorkspace {
 		return nil
 	}
+
 	// If the user has one module per view, there is nothing to warn about.
 	if s.ValidBuildConfiguration() && len(s.workspace.getKnownModFiles()) == 1 {
 		return nil
@@ -305,10 +310,21 @@ func (s *snapshot) workspaceLayoutError(ctx context.Context) *source.CriticalErr
 	// that the user has opened a directory that contains multiple modules.
 	// Check for that an warn about it.
 	if !s.ValidBuildConfiguration() {
-		msg := `gopls requires a module at the root of your workspace.
-You can work with multiple modules by opening each one as a workspace folder.
-Improvements to this workflow will be coming soon, and you can learn more here:
+		var msg string
+		if s.view.goversion >= 18 {
+			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.`
+		} else {
+			msg = `gopls requires a module at the root of your workspace.
+You can work with multiple modules by upgrading to Go 1.18 or later, and using
+go workspaces (go.work files).
+See the documentation for more information on setting up your workspace:
+https://github.com/golang/tools/blob/master/gopls/doc/workspace.md.`
+		}
 		return &source.CriticalError{
 			MainError:   fmt.Errorf(msg),
 			Diagnostics: s.applyCriticalErrorToFiles(ctx, msg, openFiles),

internal/lsp/cache/snapshot.go

@@ -274,7 +274,24 @@ func (s *snapshot) Templates() map[span.URI]source.VersionedFileHandle {
 }
 
 func (s *snapshot) ValidBuildConfiguration() bool {
-	return validBuildConfiguration(s.view.rootURI, &s.view.workspaceInformation, s.workspace.getActiveModFiles())
+	// Since we only really understand the `go` command, if the user has a
+	// different GOPACKAGESDRIVER, assume that their configuration is valid.
+	if s.view.hasGopackagesDriver {
+		return true
+	}
+	// 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 {
+		return true
+	}
+	// The user may have a multiple directories in their GOPATH.
+	// Check if the workspace is within any of them.
+	for _, gp := range filepath.SplitList(s.view.gopath) {
+		if source.InDir(filepath.Join(gp, "src"), s.view.rootURI.Filename()) {
+			return true
+		}
+	}
+	return false
 }
 
 // workspaceMode describes the way in which the snapshot's workspace should
@@ -1358,6 +1375,8 @@ func (s *snapshot) GetCriticalError(ctx context.Context) *source.CriticalError {
 		// with the user's workspace layout. Workspace packages that only have the
 		// ID "command-line-arguments" are usually a symptom of a bad workspace
 		// configuration.
+		//
+		// TODO(rfindley): re-evaluate this heuristic.
 		if containsCommandLineArguments(wsPkgs) {
 			return s.workspaceLayoutError(ctx)
 		}

internal/lsp/cache/view.go

@@ -926,27 +926,6 @@ func defaultCheckPathCase(path string) error {
 	return nil
 }
 
-func validBuildConfiguration(folder span.URI, ws *workspaceInformation, modFiles map[span.URI]struct{}) bool {
-	// Since we only really understand the `go` command, if the user has a
-	// different GOPACKAGESDRIVER, assume that their configuration is valid.
-	if ws.hasGopackagesDriver {
-		return true
-	}
-	// Check if the user is working within a module or if we have found
-	// multiple modules in the workspace.
-	if len(modFiles) > 0 {
-		return true
-	}
-	// The user may have a multiple directories in their GOPATH.
-	// Check if the workspace is within any of them.
-	for _, gp := range filepath.SplitList(ws.gopath) {
-		if source.InDir(filepath.Join(gp, "src"), folder.Filename()) {
-			return true
-		}
-	}
-	return false
-}
-
 // getGoEnv gets the view's various GO* values.
 func (s *Session) getGoEnv(ctx context.Context, folder string, goversion int, go111module string, configEnv []string) (environmentVariables, map[string]string, error) {
 	envVars := environmentVariables{}

internal/lsp/fake/client.go

@@ -74,10 +74,11 @@ func (c *Client) WorkspaceFolders(context.Context) ([]protocol.WorkspaceFolder,
 func (c *Client) Configuration(_ context.Context, p *protocol.ParamConfiguration) ([]interface{}, error) {
 	results := make([]interface{}, len(p.Items))
 	for i, item := range p.Items {
-		if item.Section != "gopls" {
-			continue
+		if item.Section == "gopls" {
+			c.editor.mu.Lock()
+			results[i] = c.editor.settingsLocked()
+			c.editor.mu.Unlock()
 		}
-		results[i] = c.editor.settings()
 	}
 	return results, nil
 }

internal/lsp/fake/editor.go

@@ -121,7 +121,7 @@ func NewEditor(sandbox *Sandbox, config EditorConfig) *Editor {
 //
 // It returns the editor, so that it may be called as follows:
 //
-//	editor, err := NewEditor(s).Connect(ctx, conn)
+//	editor, err := NewEditor(s).Connect(ctx, conn, hooks)
 func (e *Editor) Connect(ctx context.Context, connector servertest.Connector, hooks ClientHooks) (*Editor, error) {
 	bgCtx, cancelConn := context.WithCancel(xcontext.Detach(ctx))
 	conn := connector.Connect(bgCtx)
@@ -135,7 +135,7 @@ func (e *Editor) Connect(ctx context.Context, connector servertest.Connector, ho
 			protocol.ClientHandler(e.client,
 				jsonrpc2.MethodNotFound)))
 
-	if err := e.initialize(ctx, e.config.WorkspaceFolders); err != nil {
+	if err := e.initialize(ctx); err != nil {
 		return nil, err
 	}
 	e.sandbox.Workdir.AddWatcher(e.onFileChanges)
@@ -197,11 +197,10 @@ func (e *Editor) Client() *Client {
 	return e.client
 }
 
-// settings builds the settings map for use in LSP settings
-// RPCs.
-func (e *Editor) settings() map[string]interface{} {
-	e.mu.Lock()
-	defer e.mu.Unlock()
+// settingsLocked builds the settings map for use in LSP settings RPCs.
+//
+// e.mu must be held while calling this function.
+func (e *Editor) settingsLocked() map[string]interface{} {
 	env := make(map[string]string)
 	for k, v := range e.defaultEnv {
 		env[k] = v
@@ -240,26 +239,19 @@ func (e *Editor) settings() map[string]interface{} {
 	return settings
 }
 
-func (e *Editor) initialize(ctx context.Context, workspaceFolders []string) error {
+func (e *Editor) initialize(ctx context.Context) error {
 	params := &protocol.ParamInitialize{}
 	params.ClientInfo.Name = "fakeclient"
 	params.ClientInfo.Version = "v1.0.0"
-
-	if workspaceFolders == nil {
-		workspaceFolders = []string{string(e.sandbox.Workdir.RelativeTo)}
-	}
-	for _, folder := range workspaceFolders {
-		params.WorkspaceFolders = append(params.WorkspaceFolders, protocol.WorkspaceFolder{
-			URI:  string(e.sandbox.Workdir.URI(folder)),
-			Name: filepath.Base(folder),
-		})
-	}
-
+	e.mu.Lock()
+	params.WorkspaceFolders = e.makeWorkspaceFoldersLocked()
+	params.InitializationOptions = e.settingsLocked()
+	e.mu.Unlock()
 	params.Capabilities.Workspace.Configuration = true
 	params.Capabilities.Window.WorkDoneProgress = true
+
 	// TODO: set client capabilities
 	params.Capabilities.TextDocument.Completion.CompletionItem.TagSupport.ValueSet = []protocol.CompletionItemTag{protocol.ComplDeprecated}
-	params.InitializationOptions = e.settings()
 
 	params.Capabilities.TextDocument.Completion.CompletionItem.SnippetSupport = true
 	params.Capabilities.TextDocument.SemanticTokens.Requests.Full = true
@@ -296,6 +288,27 @@ func (e *Editor) initialize(ctx context.Context, workspaceFolders []string) erro
 	return nil
 }
 
+// makeWorkspaceFoldersLocked creates a slice of workspace folders to use for
+// this editing session, based on the editor configuration.
+//
+// e.mu must be held while calling this function.
+func (e *Editor) makeWorkspaceFoldersLocked() (folders []protocol.WorkspaceFolder) {
+	paths := e.config.WorkspaceFolders
+	if len(paths) == 0 {
+		paths = append(paths, string(e.sandbox.Workdir.RelativeTo))
+	}
+
+	for _, path := range paths {
+		uri := string(e.sandbox.Workdir.URI(path))
+		folders = append(folders, protocol.WorkspaceFolder{
+			URI:  uri,
+			Name: filepath.Base(uri),
+		})
+	}
+
+	return folders
+}
+
 // onFileChanges is registered to be called by the Workdir on any writes that
 // go through the Workdir API. It is called synchronously by the Workdir.
 func (e *Editor) onFileChanges(ctx context.Context, evts []FileEvent) {
@@ -1195,6 +1208,54 @@ func (e *Editor) ChangeConfiguration(ctx context.Context, newConfig EditorConfig
 	return nil
 }
 
+// ChangeWorkspaceFolders sets the new workspace folders, and sends a
+// didChangeWorkspaceFolders notification to the server.
+//
+// The given folders must all be unique.
+func (e *Editor) ChangeWorkspaceFolders(ctx context.Context, folders []string) error {
+	// capture existing folders so that we can compute the change.
+	e.mu.Lock()
+	oldFolders := e.makeWorkspaceFoldersLocked()
+	e.config.WorkspaceFolders = folders
+	newFolders := e.makeWorkspaceFoldersLocked()
+	e.mu.Unlock()
+
+	if e.Server == nil {
+		return nil
+	}
+
+	var params protocol.DidChangeWorkspaceFoldersParams
+
+	// Keep track of old workspace folders that must be removed.
+	toRemove := make(map[protocol.URI]protocol.WorkspaceFolder)
+	for _, folder := range oldFolders {
+		toRemove[folder.URI] = folder
+	}
+
+	// Sanity check: if we see a folder twice the algorithm below doesn't work,
+	// so track seen folders to ensure that we panic in that case.
+	seen := make(map[protocol.URI]protocol.WorkspaceFolder)
+	for _, folder := range newFolders {
+		if _, ok := seen[folder.URI]; ok {
+			panic(fmt.Sprintf("folder %s seen twice", folder.URI))
+		}
+
+		// If this folder already exists, we don't want to remove it.
+		// Otherwise, we need to add it.
+		if _, ok := toRemove[folder.URI]; ok {
+			delete(toRemove, folder.URI)
+		} else {
+			params.Event.Added = append(params.Event.Added, folder)
+		}
+	}
+
+	for _, v := range toRemove {
+		params.Event.Removed = append(params.Event.Removed, v)
+	}
+
+	return e.Server.DidChangeWorkspaceFolders(ctx, &params)
+}
+
 // CodeAction executes a codeAction request on the server.
 func (e *Editor) CodeAction(ctx context.Context, path string, rng *protocol.Range, diagnostics []protocol.Diagnostic) ([]protocol.CodeAction, error) {
 	if e.Server == nil {