]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go: apply "go vet" to test files
authorIan Lance Taylor <iant@golang.org>
Fri, 12 Jan 2018 23:54:09 +0000 (15:54 -0800)
committerIan Lance Taylor <iant@golang.org>
Tue, 23 Jan 2018 02:09:05 +0000 (02:09 +0000)
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
src/cmd/go/internal/load/pkg.go
src/cmd/go/internal/test/test.go
src/cmd/go/internal/vet/vet.go

index 02c63de57f2b590c70b549e6c56936362ece9166..8662c81c93843f84e799aad70b4443d3e2e0b8be 100644 (file)
@@ -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)
index a0d052a26f41afe597e145aae6de9e4f8c6383c0..beccbb568971982f4a55b50adea689c61958d74d 100644 (file)
@@ -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
+}
index fa789a48b9bc41d762c169655a5c5e27c7574187..db874ff834f42c120406510f86d27c6e8595a406 100644 (file)
@@ -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
-                       return nil, nil, nil, err
-               }
-               if p1.ImportPath == p.ImportPath {
-                       pxtestNeedsPtest = true
-               } else {
-                       ximports = append(ximports, p1)
-               }
-               p.XTestImports[i] = p1.ImportPath
+       localCover := testCover && testCoverPaths == nil
+
+       ptest, pxtest, err = load.TestPackagesFor(p, localCover || p.Name == "main")
+       if err != nil {
+               return nil, nil, nil, err
        }
-       stk.Pop()
 
        // Use last element of import path, not package name.
        // They differ when package name is "main".
@@ -844,81 +800,12 @@ 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
-                       coverFiles = append(coverFiles, ptest.GoFiles...)
-                       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)
-               }
+       if localCover {
+               ptest.Internal.CoverMode = testCoverMode
+               var coverFiles []string
+               coverFiles = append(coverFiles, ptest.GoFiles...)
+               coverFiles = append(coverFiles, ptest.CgoFiles...)
+               ptest.Internal.CoverVars = declareCoverVars(ptest.ImportPath, coverFiles...)
        }
 
        testDir := b.NewObjdir()
@@ -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"
index 8b4f9264ac3ad062f16a0052c2b16585c44db5eb..3d095d450871d0784a200bdef4d2190309fff404 100644 (file)
@@ -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)
 }