go/analysis/analysistest: stricter errors and GOWORK setting

This change causes analysistest to report complete failure to
load one or more of the packages named by the load patterns.
(This is indicated by a missing Package.Name.)
Previously, these errors were silently ignored, meaning
that tests were providing less coverage than they seemed.

In particular, the stdversion test was unable to load the
specified modules because there was no GOWORK file to
locate them. (Worse: the user's GOWORK environment was
affecting the test.) So, we now allow tests to specify a
go.work file in the root of the test tree. If present,
we honor it, and, crucially, if absent, we set GOWORK=off.

Also, two other tests in gopls/internal/analysis were
silently doing nothing (!).

Change-Id: I4ee7ae2a636497a64f1e43eb05a4a414ceaa5f4a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/582595
Auto-Submit: Alan Donovan <[email protected]>
Reviewed-by: Tim King <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: adonovan

go/analysis/analysistest/analysistest.go

@@ -369,11 +369,16 @@ type Result = checker.TestAnalyzerResult
 // loadPackages returns an error if any package had an error, or the pattern
 // matched no packages.
 func loadPackages(a *analysis.Analyzer, dir string, patterns ...string) ([]*packages.Package, error) {
-	env := []string{"GOPATH=" + dir, "GO111MODULE=off"} // GOPATH mode
+	env := []string{"GOPATH=" + dir, "GO111MODULE=off", "GOWORK=off"} // GOPATH mode
 
 	// Undocumented module mode. Will be replaced by something better.
 	if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil {
-		env = []string{"GO111MODULE=on", "GOPROXY=off"} // module mode
+		gowork := filepath.Join(dir, "go.work")
+		if _, err := os.Stat(gowork); err != nil {
+			gowork = "off"
+		}
+
+		env = []string{"GO111MODULE=on", "GOPROXY=off", "GOWORK=" + gowork} // module mode
 	}
 
 	// packages.Load loads the real standard library, not a minimal
@@ -397,12 +402,23 @@ func loadPackages(a *analysis.Analyzer, dir string, patterns ...string) ([]*pack
 		return nil, err
 	}
 
+	// If any named package couldn't be loaded at all
+	// (e.g. the Name field is unset), fail fast.
+	for _, pkg := range pkgs {
+		if pkg.Name == "" {
+			return nil, fmt.Errorf("failed to load %q: Errors=%v",
+				pkg.PkgPath, pkg.Errors)
+		}
+	}
+
 	// Do NOT print errors if the analyzer will continue running.
 	// It is incredibly confusing for tests to be printing to stderr
 	// willy-nilly instead of their test logs, especially when the
 	// errors are expected and are going to be fixed.
 	if !a.RunDespiteErrors {
-		packages.PrintErrors(pkgs)
+		if packages.PrintErrors(pkgs) > 0 {
+			return nil, fmt.Errorf("there were package loading errors (and RunDespiteErrors is false)")
+		}
 	}
 
 	if len(pkgs) == 0 {

go/analysis/passes/stdversion/testdata/test.txtar

@@ -8,6 +8,13 @@ See also gopls/internal/test/marker/testdata/diagnostics/stdversion.txt
 which runs the same test within the gopls analysis driver, to ensure
 coverage of per-file Go version support.
 
+-- go.work --
+go 1.21
+
+use .
+use sub
+use old
+
 -- go.mod --
 module example.com
 

gopls/internal/analysis/fillreturns/testdata/src/a/typeparams/a.go renamed to gopls/internal/analysis/fillreturns/testdata/src/typeparams/a.go

@@ -13,5 +13,5 @@ import (
 
 func Test(t *testing.T) {
 	testdata := analysistest.TestData()
-	analysistest.Run(t, testdata, stubmethods.Analyzer, "a")
+	analysistest.Run(t, testdata, stubmethods.Analyzer, "typeparams")
 }

gopls/internal/analysis/fillreturns/testdata/src/a/typeparams/a.go.golden renamed to gopls/internal/analysis/fillreturns/testdata/src/typeparams/a.go.golden

@@ -4,7 +4,7 @@
 
 package stubmethods
 
-var _ I = Y{} // want "Implement I"
+var _ I = Y{} // want "does not implement I"
 
 type I interface{ F() }