go/analysis/unitchecker: add test of go vet on std

This change adds a new test that runs go vet on std, using
a unitchecker-based tool with (hopefully) the same set of
analyzers as the real cmd/vet in GOROOT. This should serve
as an early warning when a change to an analyzer causes
it to become stricter on std packages.

Change-Id: I87503f40ad2d54b928d876216a951ed88b216e58
Reviewed-on: https://go-review.googlesource.com/c/tools/+/493619
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
diff --git a/go/analysis/unitchecker/main.go b/go/analysis/unitchecker/main.go
index a054a2d..6e08ce9 100644
--- a/go/analysis/unitchecker/main.go
+++ b/go/analysis/unitchecker/main.go
@@ -27,16 +27,23 @@
 	"golang.org/x/tools/go/analysis/passes/cgocall"
 	"golang.org/x/tools/go/analysis/passes/composite"
 	"golang.org/x/tools/go/analysis/passes/copylock"
+	"golang.org/x/tools/go/analysis/passes/directive"
 	"golang.org/x/tools/go/analysis/passes/errorsas"
+	"golang.org/x/tools/go/analysis/passes/framepointer"
 	"golang.org/x/tools/go/analysis/passes/httpresponse"
+	"golang.org/x/tools/go/analysis/passes/ifaceassert"
 	"golang.org/x/tools/go/analysis/passes/loopclosure"
 	"golang.org/x/tools/go/analysis/passes/lostcancel"
 	"golang.org/x/tools/go/analysis/passes/nilfunc"
 	"golang.org/x/tools/go/analysis/passes/printf"
 	"golang.org/x/tools/go/analysis/passes/shift"
+	"golang.org/x/tools/go/analysis/passes/sigchanyzer"
 	"golang.org/x/tools/go/analysis/passes/stdmethods"
+	"golang.org/x/tools/go/analysis/passes/stringintconv"
 	"golang.org/x/tools/go/analysis/passes/structtag"
+	"golang.org/x/tools/go/analysis/passes/testinggoroutine"
 	"golang.org/x/tools/go/analysis/passes/tests"
+	"golang.org/x/tools/go/analysis/passes/timeformat"
 	"golang.org/x/tools/go/analysis/passes/unmarshal"
 	"golang.org/x/tools/go/analysis/passes/unreachable"
 	"golang.org/x/tools/go/analysis/passes/unsafeptr"
@@ -53,16 +60,23 @@
 		cgocall.Analyzer,
 		composite.Analyzer,
 		copylock.Analyzer,
+		directive.Analyzer,
 		errorsas.Analyzer,
+		framepointer.Analyzer,
 		httpresponse.Analyzer,
+		ifaceassert.Analyzer,
 		loopclosure.Analyzer,
 		lostcancel.Analyzer,
 		nilfunc.Analyzer,
 		printf.Analyzer,
 		shift.Analyzer,
+		sigchanyzer.Analyzer,
 		stdmethods.Analyzer,
+		stringintconv.Analyzer,
 		structtag.Analyzer,
 		tests.Analyzer,
+		testinggoroutine.Analyzer,
+		timeformat.Analyzer,
 		unmarshal.Analyzer,
 		unreachable.Analyzer,
 		unsafeptr.Analyzer,
diff --git a/go/analysis/unitchecker/unitchecker.go b/go/analysis/unitchecker/unitchecker.go
index 3c6fbe4..ff22d23 100644
--- a/go/analysis/unitchecker/unitchecker.go
+++ b/go/analysis/unitchecker/unitchecker.go
@@ -183,11 +183,6 @@
 	return cfg, nil
 }
 
-var importerForCompiler = func(_ *token.FileSet, compiler string, lookup importer.Lookup) types.Importer {
-	// broken legacy implementation (https://golang.org/issue/28995)
-	return importer.For(compiler, lookup)
-}
-
 func run(fset *token.FileSet, cfg *Config, analyzers []*analysis.Analyzer) ([]result, error) {
 	// Load, parse, typecheck.
 	var files []*ast.File
@@ -203,7 +198,7 @@
 		}
 		files = append(files, f)
 	}
-	compilerImporter := importerForCompiler(fset, cfg.Compiler, func(path string) (io.ReadCloser, error) {
+	compilerImporter := importer.ForCompiler(fset, cfg.Compiler, func(path string) (io.ReadCloser, error) {
 		// path is a resolved package path, not an import path.
 		file, ok := cfg.PackageFile[path]
 		if !ok {
diff --git a/go/analysis/unitchecker/unitchecker112.go b/go/analysis/unitchecker/unitchecker112.go
deleted file mode 100644
index 3180f4a..0000000
--- a/go/analysis/unitchecker/unitchecker112.go
+++ /dev/null
@@ -1,14 +0,0 @@
-// Copyright 2018 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.
-
-//go:build go1.12
-// +build go1.12
-
-package unitchecker
-
-import "go/importer"
-
-func init() {
-	importerForCompiler = importer.ForCompiler
-}
diff --git a/go/analysis/unitchecker/unitchecker_test.go b/go/analysis/unitchecker/unitchecker_test.go
index 197abd9..1ed0012 100644
--- a/go/analysis/unitchecker/unitchecker_test.go
+++ b/go/analysis/unitchecker/unitchecker_test.go
@@ -2,15 +2,8 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-//go:build go1.12
-// +build go1.12
-
 package unitchecker_test
 
-// This test depends on features such as
-// go vet's support for vetx files (1.11) and
-// the (*os.ProcessState).ExitCode method (1.12).
-
 import (
 	"flag"
 	"os"
@@ -28,17 +21,23 @@
 )
 
 func TestMain(m *testing.M) {
-	if os.Getenv("UNITCHECKER_CHILD") == "1" {
-		// child process
-		main()
+	// child process?
+	switch os.Getenv("ENTRYPOINT") {
+	case "vet":
+		vet()
+		panic("unreachable")
+	case "minivet":
+		minivet()
 		panic("unreachable")
 	}
 
+	// test process
 	flag.Parse()
 	os.Exit(m.Run())
 }
 
-func main() {
+// minivet is a vet-like tool with a few analyzers, for testing.
+func minivet() {
 	unitchecker.Main(
 		findcall.Analyzer,
 		printf.Analyzer,
@@ -162,7 +161,7 @@
 	} {
 		cmd := exec.Command("go", "vet", "-vettool="+os.Args[0], "-findcall.name=MyFunc123")
 		cmd.Args = append(cmd.Args, strings.Fields(test.args)...)
-		cmd.Env = append(exported.Config.Env, "UNITCHECKER_CHILD=1")
+		cmd.Env = append(exported.Config.Env, "ENTRYPOINT=minivet")
 		cmd.Dir = exported.Config.Dir
 
 		out, err := cmd.CombinedOutput()
diff --git a/go/analysis/unitchecker/vet_std_test.go b/go/analysis/unitchecker/vet_std_test.go
new file mode 100644
index 0000000..feea1a2
--- /dev/null
+++ b/go/analysis/unitchecker/vet_std_test.go
@@ -0,0 +1,97 @@
+// Copyright 2023 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 unitchecker_test
+
+import (
+	"os"
+	"os/exec"
+	"runtime"
+	"strings"
+	"testing"
+
+	"golang.org/x/tools/go/analysis/passes/asmdecl"
+	"golang.org/x/tools/go/analysis/passes/assign"
+	"golang.org/x/tools/go/analysis/passes/atomic"
+	"golang.org/x/tools/go/analysis/passes/bools"
+	"golang.org/x/tools/go/analysis/passes/buildtag"
+	"golang.org/x/tools/go/analysis/passes/cgocall"
+	"golang.org/x/tools/go/analysis/passes/composite"
+	"golang.org/x/tools/go/analysis/passes/copylock"
+	"golang.org/x/tools/go/analysis/passes/directive"
+	"golang.org/x/tools/go/analysis/passes/errorsas"
+	"golang.org/x/tools/go/analysis/passes/framepointer"
+	"golang.org/x/tools/go/analysis/passes/httpresponse"
+	"golang.org/x/tools/go/analysis/passes/ifaceassert"
+	"golang.org/x/tools/go/analysis/passes/loopclosure"
+	"golang.org/x/tools/go/analysis/passes/lostcancel"
+	"golang.org/x/tools/go/analysis/passes/nilfunc"
+	"golang.org/x/tools/go/analysis/passes/printf"
+	"golang.org/x/tools/go/analysis/passes/shift"
+	"golang.org/x/tools/go/analysis/passes/sigchanyzer"
+	"golang.org/x/tools/go/analysis/passes/stdmethods"
+	"golang.org/x/tools/go/analysis/passes/stringintconv"
+	"golang.org/x/tools/go/analysis/passes/structtag"
+	"golang.org/x/tools/go/analysis/passes/testinggoroutine"
+	"golang.org/x/tools/go/analysis/passes/tests"
+	"golang.org/x/tools/go/analysis/passes/timeformat"
+	"golang.org/x/tools/go/analysis/passes/unmarshal"
+	"golang.org/x/tools/go/analysis/passes/unreachable"
+	"golang.org/x/tools/go/analysis/passes/unusedresult"
+	"golang.org/x/tools/go/analysis/unitchecker"
+)
+
+// vet is the entrypoint of this executable when ENTRYPOINT=vet.
+// Keep consistent with the actual vet in GOROOT/src/cmd/vet/main.go.
+func vet() {
+	unitchecker.Main(
+		asmdecl.Analyzer,
+		assign.Analyzer,
+		atomic.Analyzer,
+		bools.Analyzer,
+		buildtag.Analyzer,
+		cgocall.Analyzer,
+		composite.Analyzer,
+		copylock.Analyzer,
+		directive.Analyzer,
+		errorsas.Analyzer,
+		framepointer.Analyzer,
+		httpresponse.Analyzer,
+		ifaceassert.Analyzer,
+		loopclosure.Analyzer,
+		lostcancel.Analyzer,
+		nilfunc.Analyzer,
+		printf.Analyzer,
+		shift.Analyzer,
+		sigchanyzer.Analyzer,
+		stdmethods.Analyzer,
+		stringintconv.Analyzer,
+		structtag.Analyzer,
+		tests.Analyzer,
+		testinggoroutine.Analyzer,
+		timeformat.Analyzer,
+		unmarshal.Analyzer,
+		unreachable.Analyzer,
+		// unsafeptr.Analyzer, // currently reports findings in runtime
+		unusedresult.Analyzer,
+	)
+}
+
+// TestVetStdlib runs the same analyzers as the actual vet over the
+// standard library, using go vet and unitchecker, to ensure that
+// there are no findings.
+func TestVetStdlib(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping in -short mode")
+	}
+	if version := runtime.Version(); !strings.HasPrefix(version, "devel") {
+		t.Skipf("This test is only wanted on development branches where code can be easily fixed. Skipping because runtime.Version=%q.", version)
+	}
+
+	cmd := exec.Command("go", "vet", "-vettool="+os.Args[0], "std")
+	cmd.Env = append(os.Environ(), "ENTRYPOINT=vet")
+	if out, err := cmd.CombinedOutput(); err != nil {
+		t.Errorf("go vet std failed (%v):\n%s", err, out)
+	}
+}