cmd/go: apply "go vet" to test files

In earlier versions of Go the "go vet" command would run on regular
source files and test files. That was lost in CL74750.  Bring it back.

This required moving a chunk of code from internal/test to
internal/load. The diff looks big but the code is unchanged.

Fixes #23395

Change-Id: Ie9ec183337e8db81c5fc421d118a22b351b5409e
Reviewed-on: https://go-review.googlesource.com/87636
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/cmd/go/go_test.go

@@ -3206,6 +3206,16 @@ func TestGoVetWithFlagsOff(t *testing.T) {
 	tg.run("vet", "-printf=false", "vetpkg")
 }
 
+// Issue 23395.
+func TestGoVetWithOnlyTestFiles(t *testing.T) {
+	tg := testgo(t)
+	defer tg.cleanup()
+	tg.parallel()
+	tg.tempFile("src/p/p_test.go", "package p; import \"testing\"; func TestMe(*testing.T) {}")
+	tg.setenv("GOPATH", tg.path("."))
+	tg.run("vet", "p")
+}
+
 // Issue 9767, 19769.
 func TestGoGetDotSlashDownload(t *testing.T) {
 	testenv.MustHaveExternalNetwork(t)

src/cmd/go/internal/load/pkg.go

@@ -1508,3 +1508,147 @@ func GoFilesPackage(gofiles []string) *Package {
 
 	return pkg
 }
+
+// TestPackagesFor returns package structs ptest, the package p plus
+// its test files, and pxtest, the external tests of package p.
+// pxtest may be nil. If there are no test files, forceTest decides
+// whether this returns a new package struct or just returns p.
+func TestPackagesFor(p *Package, forceTest bool) (ptest, pxtest *Package, err error) {
+	var imports, ximports []*Package
+	var stk ImportStack
+	stk.Push(p.ImportPath + " (test)")
+	rawTestImports := str.StringList(p.TestImports)
+	for i, path := range p.TestImports {
+		p1 := LoadImport(path, p.Dir, p, &stk, p.Internal.Build.TestImportPos[path], UseVendor)
+		if p1.Error != nil {
+			return nil, nil, p1.Error
+		}
+		if len(p1.DepsErrors) > 0 {
+			err := p1.DepsErrors[0]
+			err.Pos = "" // show full import stack
+			return nil, nil, err
+		}
+		if str.Contains(p1.Deps, p.ImportPath) || p1.ImportPath == p.ImportPath {
+			// Same error that loadPackage returns (via reusePackage) in pkg.go.
+			// Can't change that code, because that code is only for loading the
+			// non-test copy of a package.
+			err := &PackageError{
+				ImportStack:   testImportStack(stk[0], p1, p.ImportPath),
+				Err:           "import cycle not allowed in test",
+				IsImportCycle: true,
+			}
+			return nil, nil, err
+		}
+		p.TestImports[i] = p1.ImportPath
+		imports = append(imports, p1)
+	}
+	stk.Pop()
+	stk.Push(p.ImportPath + "_test")
+	pxtestNeedsPtest := false
+	rawXTestImports := str.StringList(p.XTestImports)
+	for i, path := range p.XTestImports {
+		p1 := LoadImport(path, p.Dir, p, &stk, p.Internal.Build.XTestImportPos[path], UseVendor)
+		if p1.Error != nil {
+			return nil, nil, p1.Error
+		}
+		if len(p1.DepsErrors) > 0 {
+			err := p1.DepsErrors[0]
+			err.Pos = "" // show full import stack
+			return nil, nil, err
+		}
+		if p1.ImportPath == p.ImportPath {
+			pxtestNeedsPtest = true
+		} else {
+			ximports = append(ximports, p1)
+		}
+		p.XTestImports[i] = p1.ImportPath
+	}
+	stk.Pop()
+
+	// Test package.
+	if len(p.TestGoFiles) > 0 || forceTest {
+		ptest = new(Package)
+		*ptest = *p
+		ptest.GoFiles = nil
+		ptest.GoFiles = append(ptest.GoFiles, p.GoFiles...)
+		ptest.GoFiles = append(ptest.GoFiles, p.TestGoFiles...)
+		ptest.Target = ""
+		// Note: The preparation of the vet config requires that common
+		// indexes in ptest.Imports, ptest.Internal.Imports, and ptest.Internal.RawImports
+		// all line up (but RawImports can be shorter than the others).
+		// That is, for 0 ≤ i < len(RawImports),
+		// RawImports[i] is the import string in the program text,
+		// Imports[i] is the expanded import string (vendoring applied or relative path expanded away),
+		// and Internal.Imports[i] is the corresponding *Package.
+		// Any implicitly added imports appear in Imports and Internal.Imports
+		// but not RawImports (because they were not in the source code).
+		// We insert TestImports, imports, and rawTestImports at the start of
+		// these lists to preserve the alignment.
+		ptest.Imports = str.StringList(p.TestImports, p.Imports)
+		ptest.Internal.Imports = append(imports, p.Internal.Imports...)
+		ptest.Internal.RawImports = str.StringList(rawTestImports, p.Internal.RawImports)
+		ptest.Internal.ForceLibrary = true
+		ptest.Internal.Build = new(build.Package)
+		*ptest.Internal.Build = *p.Internal.Build
+		m := map[string][]token.Position{}
+		for k, v := range p.Internal.Build.ImportPos {
+			m[k] = append(m[k], v...)
+		}
+		for k, v := range p.Internal.Build.TestImportPos {
+			m[k] = append(m[k], v...)
+		}
+		ptest.Internal.Build.ImportPos = m
+	} else {
+		ptest = p
+	}
+
+	// External test package.
+	if len(p.XTestGoFiles) > 0 {
+		pxtest = &Package{
+			PackagePublic: PackagePublic{
+				Name:       p.Name + "_test",
+				ImportPath: p.ImportPath + "_test",
+				Root:       p.Root,
+				Dir:        p.Dir,
+				GoFiles:    p.XTestGoFiles,
+				Imports:    p.XTestImports,
+			},
+			Internal: PackageInternal{
+				LocalPrefix: p.Internal.LocalPrefix,
+				Build: &build.Package{
+					ImportPos: p.Internal.Build.XTestImportPos,
+				},
+				Imports:    ximports,
+				RawImports: rawXTestImports,
+
+				Asmflags:   p.Internal.Asmflags,
+				Gcflags:    p.Internal.Gcflags,
+				Ldflags:    p.Internal.Ldflags,
+				Gccgoflags: p.Internal.Gccgoflags,
+			},
+		}
+		if pxtestNeedsPtest {
+			pxtest.Internal.Imports = append(pxtest.Internal.Imports, ptest)
+		}
+	}
+
+	return ptest, pxtest, nil
+}
+
+func testImportStack(top string, p *Package, target string) []string {
+	stk := []string{top, p.ImportPath}
+Search:
+	for p.ImportPath != target {
+		for _, p1 := range p.Internal.Imports {
+			if p1.ImportPath == target || str.Contains(p1.Deps, target) {
+				stk = append(stk, p1.ImportPath)
+				p = p1
+				continue Search
+			}
+		}
+		// Can't happen, but in case it does...
+		stk = append(stk, "<lost path to cycle>")
+		break
+	}
+	return stk
+}

src/cmd/go/internal/test/test.go

@@ -777,56 +777,12 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
 	//	pmain - pkg.test binary
 	var ptest, pxtest, pmain *load.Package
 
-	var imports, ximports []*load.Package
-	var stk load.ImportStack
-	stk.Push(p.ImportPath + " (test)")
-	rawTestImports := str.StringList(p.TestImports)
-	for i, path := range p.TestImports {
-		p1 := load.LoadImport(path, p.Dir, p, &stk, p.Internal.Build.TestImportPos[path], load.UseVendor)
-		if p1.Error != nil {
-			return nil, nil, nil, p1.Error
-		}
-		if len(p1.DepsErrors) > 0 {
-			err := p1.DepsErrors[0]
-			err.Pos = "" // show full import stack
-			return nil, nil, nil, err
-		}
-		if str.Contains(p1.Deps, p.ImportPath) || p1.ImportPath == p.ImportPath {
-			// Same error that loadPackage returns (via reusePackage) in pkg.go.
-			// Can't change that code, because that code is only for loading the
-			// non-test copy of a package.
-			err := &load.PackageError{
-				ImportStack:   testImportStack(stk[0], p1, p.ImportPath),
-				Err:           "import cycle not allowed in test",
-				IsImportCycle: true,
-			}
-			return nil, nil, nil, err
-		}
-		p.TestImports[i] = p1.ImportPath
-		imports = append(imports, p1)
-	}
-	stk.Pop()
-	stk.Push(p.ImportPath + "_test")
-	pxtestNeedsPtest := false
-	rawXTestImports := str.StringList(p.XTestImports)
-	for i, path := range p.XTestImports {
-		p1 := load.LoadImport(path, p.Dir, p, &stk, p.Internal.Build.XTestImportPos[path], load.UseVendor)
-		if p1.Error != nil {
-			return nil, nil, nil, p1.Error
-		}
-		if len(p1.DepsErrors) > 0 {
-			err := p1.DepsErrors[0]
-			err.Pos = "" // show full import stack
+	localCover := testCover && testCoverPaths == nil
+
+	ptest, pxtest, err = load.TestPackagesFor(p, localCover || p.Name == "main")
+	if err != nil {
 		return nil, nil, nil, err
 	}
-		if p1.ImportPath == p.ImportPath {
-			pxtestNeedsPtest = true
-		} else {
-			ximports = append(ximports, p1)
-		}
-		p.XTestImports[i] = p1.ImportPath
-	}
-	stk.Pop()
 
 	// Use last element of import path, not package name.
 	// They differ when package name is "main".
@@ -844,42 +800,6 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
 	// only for this package and only for this test?
 	// Yes, if -cover is on but -coverpkg has not specified
 	// a list of packages for global coverage.
-	localCover := testCover && testCoverPaths == nil
-
-	// Test package.
-	if len(p.TestGoFiles) > 0 || localCover || p.Name == "main" {
-		ptest = new(load.Package)
-		*ptest = *p
-		ptest.GoFiles = nil
-		ptest.GoFiles = append(ptest.GoFiles, p.GoFiles...)
-		ptest.GoFiles = append(ptest.GoFiles, p.TestGoFiles...)
-		ptest.Target = ""
-		// Note: The preparation of the vet config requires that common
-		// indexes in ptest.Imports, ptest.Internal.Imports, and ptest.Internal.RawImports
-		// all line up (but RawImports can be shorter than the others).
-		// That is, for 0 ≤ i < len(RawImports),
-		// RawImports[i] is the import string in the program text,
-		// Imports[i] is the expanded import string (vendoring applied or relative path expanded away),
-		// and Internal.Imports[i] is the corresponding *Package.
-		// Any implicitly added imports appear in Imports and Internal.Imports
-		// but not RawImports (because they were not in the source code).
-		// We insert TestImports, imports, and rawTestImports at the start of
-		// these lists to preserve the alignment.
-		ptest.Imports = str.StringList(p.TestImports, p.Imports)
-		ptest.Internal.Imports = append(imports, p.Internal.Imports...)
-		ptest.Internal.RawImports = str.StringList(rawTestImports, p.Internal.RawImports)
-		ptest.Internal.ForceLibrary = true
-		ptest.Internal.Build = new(build.Package)
-		*ptest.Internal.Build = *p.Internal.Build
-		m := map[string][]token.Position{}
-		for k, v := range p.Internal.Build.ImportPos {
-			m[k] = append(m[k], v...)
-		}
-		for k, v := range p.Internal.Build.TestImportPos {
-			m[k] = append(m[k], v...)
-		}
-		ptest.Internal.Build.ImportPos = m
-
 	if localCover {
 		ptest.Internal.CoverMode = testCoverMode
 		var coverFiles []string
@@ -887,39 +807,6 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
 		coverFiles = append(coverFiles, ptest.CgoFiles...)
 		ptest.Internal.CoverVars = declareCoverVars(ptest.ImportPath, coverFiles...)
 	}
-	} else {
-		ptest = p
-	}
-
-	// External test package.
-	if len(p.XTestGoFiles) > 0 {
-		pxtest = &load.Package{
-			PackagePublic: load.PackagePublic{
-				Name:       p.Name + "_test",
-				ImportPath: p.ImportPath + "_test",
-				Root:       p.Root,
-				Dir:        p.Dir,
-				GoFiles:    p.XTestGoFiles,
-				Imports:    p.XTestImports,
-			},
-			Internal: load.PackageInternal{
-				LocalPrefix: p.Internal.LocalPrefix,
-				Build: &build.Package{
-					ImportPos: p.Internal.Build.XTestImportPos,
-				},
-				Imports:    ximports,
-				RawImports: rawXTestImports,
-
-				Asmflags:   p.Internal.Asmflags,
-				Gcflags:    p.Internal.Gcflags,
-				Ldflags:    p.Internal.Ldflags,
-				Gccgoflags: p.Internal.Gccgoflags,
-			},
-		}
-		if pxtestNeedsPtest {
-			pxtest.Internal.Imports = append(pxtest.Internal.Imports, ptest)
-		}
-	}
 
 	testDir := b.NewObjdir()
 	if err := b.Mkdir(testDir); err != nil {
@@ -948,6 +835,7 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
 
 	// The generated main also imports testing, regexp, and os.
 	// Also the linker introduces implicit dependencies reported by LinkerDeps.
+	var stk load.ImportStack
 	stk.Push("testmain")
 	deps := testMainDeps // cap==len, so safe for append
 	for _, d := range load.LinkerDeps(p) {
@@ -1150,24 +1038,6 @@ func addTestVet(b *work.Builder, p *load.Package, runAction, installAction *work
 	}
 }
 
-func testImportStack(top string, p *load.Package, target string) []string {
-	stk := []string{top, p.ImportPath}
-Search:
-	for p.ImportPath != target {
-		for _, p1 := range p.Internal.Imports {
-			if p1.ImportPath == target || str.Contains(p1.Deps, target) {
-				stk = append(stk, p1.ImportPath)
-				p = p1
-				continue Search
-			}
-		}
-		// Can't happen, but in case it does...
-		stk = append(stk, "<lost path to cycle>")
-		break
-	}
-	return stk
-}
-
 func recompileForTest(pmain, preal, ptest, pxtest *load.Package) {
 	// The "test copy" of preal is ptest.
 	// For each package that depends on preal, make a "test copy"

src/cmd/go/internal/vet/vet.go

@@ -57,7 +57,21 @@ func runVet(cmd *base.Command, args []string) {
 
 	root := &work.Action{Mode: "go vet"}
 	for _, p := range pkgs {
-		root.Deps = append(root.Deps, b.VetAction(work.ModeBuild, work.ModeBuild, p))
+		ptest, pxtest, err := load.TestPackagesFor(p, false)
+		if err != nil {
+			base.Errorf("%v", err)
+			continue
+		}
+		if len(ptest.GoFiles) == 0 && pxtest == nil {
+			base.Errorf("go vet %s: no Go files in %s", p.ImportPath, p.Dir)
+			continue
+		}
+		if len(ptest.GoFiles) > 0 {
+			root.Deps = append(root.Deps, b.VetAction(work.ModeBuild, work.ModeBuild, ptest))
+		}
+		if pxtest != nil {
+			root.Deps = append(root.Deps, b.VetAction(work.ModeBuild, work.ModeBuild, pxtest))
+		}
 	}
 	b.Do(root)
 }