internal/lsp: show multiple modules error message with GO111MODULE=auto

When GO111MODULE=auto and the workspace is outside of $GOPATH/src, we
should still show diagnostics about how to correctly configure
multi-module workspaces. We should not show these warnings for
workspaces outside of a module within GOPATH.

This adds an extra piece to the WorkspacePackages error logic--we may
still need to show the errors even if WorkspacePackages returned
results. We should eventually consolidate all of this logic to be more
cohesive, but for now I think it's more important to cover all of the
different cases and add tests.

Updates golang/go#42109

Change-Id: I673a03c9840cdaaf7f058de1cda3bf36b96fa7d3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/275553
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>

gopls/internal/regtest/diagnostics_test.go

@@ -1651,7 +1651,7 @@ import (
 	})
 }
 
-func TestMultipleModules_GO111MODULE_on(t *testing.T) {
+func TestMultipleModules_Warning(t *testing.T) {
 	const modules = `
 -- a/go.mod --
 module a.com
@@ -1666,21 +1666,47 @@ go 1.12
 -- b/b.go --
 package b
 `
-	withOptions(
-		WithModes(Singleton),
-		EditorConfig{
-			Env: map[string]string{
-				"GO111MODULE": "on",
+	for _, go111module := range []string{"on", "auto"} {
+		t.Run("GO111MODULE="+go111module, func(t *testing.T) {
+			withOptions(
+				WithModes(Singleton),
+				EditorConfig{
+					Env: map[string]string{
+						"GO111MODULE": go111module,
+					},
+				},
+			).run(t, modules, func(t *testing.T, env *Env) {
+				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("Error loading workspace", "gopls requires a module at the root of your workspace."),
+				)
+			})
+		})
+	}
+
+	// Expect no warning if GO111MODULE=auto in a directory in GOPATH.
+	t.Run("GOPATH_GO111MODULE_auto", func(t *testing.T) {
+		withOptions(
+			WithModes(Singleton),
+			EditorConfig{
+				Env: map[string]string{
+					"GO111MODULE": "auto",
+				},
 			},
-		},
-	).run(t, modules, func(t *testing.T, env *Env) {
-		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("Error loading workspace", "gopls requires a module at the root of your workspace."),
-		)
+			InGOPATH(),
+		).run(t, modules, func(t *testing.T, env *Env) {
+			env.OpenFile("a/a.go")
+			env.Await(
+				OnceMet(
+					CompletedWork(lsp.DiagnosticWorkTitle(lsp.FromDidOpen), 1),
+					NoDiagnostics("a/a.go"),
+				),
+				NoOutstandingWork(),
+			)
+		})
 	})
 }
 

internal/lsp/cache/load.go

@@ -175,10 +175,8 @@ func (s *snapshot) load(ctx context.Context, allowNetwork bool, scopes ...interf
 }
 
 func (s *snapshot) parseLoadError(ctx context.Context, loadErr error) *source.CriticalError {
-	// The error may be a result of the user's workspace layout. Check for
-	// a valid workspace configuration first.
-	if criticalErr := s.workspaceLayoutErrors(ctx, loadErr); criticalErr != nil {
-		return criticalErr
+	if strings.Contains(loadErr.Error(), "cannot find main module") {
+		return s.WorkspaceLayoutError(ctx)
 	}
 	criticalErr := &source.CriticalError{
 		MainError: loadErr,
@@ -200,7 +198,7 @@ func (s *snapshot) parseLoadError(ctx context.Context, loadErr error) *source.Cr
 
 // workspaceLayoutErrors returns a diagnostic for every open file, as well as
 // an error message if there are no open files.
-func (s *snapshot) workspaceLayoutErrors(ctx context.Context, err error) *source.CriticalError {
+func (s *snapshot) WorkspaceLayoutError(ctx context.Context) *source.CriticalError {
 	// Assume the workspace is misconfigured only if we've detected an invalid
 	// build configuration. Currently, a valid build configuration is either a
 	// module at the root of the view or a GOPATH workspace.
@@ -210,8 +208,7 @@ func (s *snapshot) workspaceLayoutErrors(ctx context.Context, err error) *source
 	if len(s.workspace.getKnownModFiles()) == 0 {
 		return nil
 	}
-	// TODO(rstambler): Handle GO111MODULE=auto.
-	if s.view.userGo111Module != on {
+	if s.view.userGo111Module == off {
 		return nil
 	}
 	if s.workspace.moduleSource != legacyWorkspace {

internal/lsp/diagnostics.go

@@ -182,6 +182,16 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, forceAn
 		return false
 	}
 
+	// Even if workspace packages were returned, there still may be an error
+	// 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.
+	if onlyCommandLineArguments(wsPkgs) {
+		if criticalErr := snapshot.WorkspaceLayoutError(ctx); criticalErr != nil {
+			err = criticalErr
+		}
+	}
+
 	// Show the error as a progress error report so that it appears in the
 	// status bar. If a client doesn't support progress reports, the error
 	// will still be shown as a ShowMessage. If there is no error, any running
@@ -577,3 +587,12 @@ func (s *Server) shouldIgnoreError(ctx context.Context, snapshot source.Snapshot
 	})
 	return !hasGo
 }
+
+func onlyCommandLineArguments(pkgs []source.Package) bool {
+	for _, pkg := range pkgs {
+		if pkg.ID() != "command-line-arguments" {
+			return false
+		}
+	}
+	return true
+}

internal/lsp/source/view.go

@@ -146,6 +146,11 @@ type Snapshot interface {
 
 	// WorkspacePackages returns the snapshot's top-level packages.
 	WorkspacePackages(ctx context.Context) ([]Package, error)
+
+	// WorkspaceLayoutError reports whether there might be any problems with
+	// the user's workspace configuration, which would cause bad or incorrect
+	// diagnostics.
+	WorkspaceLayoutError(ctx context.Context) *CriticalError
 }
 
 // PackageFilter sets how a package is filtered out from a set of packages