Skip to content

Commit cebc706

Browse files
ianlancetaylorianlancetaylor
committed
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 <[email protected]> Reviewed-by: Rob Pike <[email protected]> Reviewed-by: Russ Cox <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
1 parent dbd8f3d commit cebc706

File tree

4 files changed

+181
-143
lines changed

4 files changed

+181
-143
lines changed

‎src/cmd/go/go_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3206,6 +3206,16 @@ func TestGoVetWithFlagsOff(t *testing.T) {
32063206
tg.run("vet", "-printf=false", "vetpkg")
32073207
}
32083208

3209+
// Issue 23395.
3210+
func TestGoVetWithOnlyTestFiles(t *testing.T) {
3211+
tg := testgo(t)
3212+
defer tg.cleanup()
3213+
tg.parallel()
3214+
tg.tempFile("src/p/p_test.go", "package p; import \"testing\"; func TestMe(*testing.T) {}")
3215+
tg.setenv("GOPATH", tg.path("."))
3216+
tg.run("vet", "p")
3217+
}
3218+
32093219
// Issue 9767, 19769.
32103220
func TestGoGetDotSlashDownload(t *testing.T) {
32113221
testenv.MustHaveExternalNetwork(t)

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

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1508,3 +1508,147 @@ func GoFilesPackage(gofiles []string) *Package {
15081508

15091509
return pkg
15101510
}
1511+
1512+
// TestPackagesFor returns package structs ptest, the package p plus
1513+
// its test files, and pxtest, the external tests of package p.
1514+
// pxtest may be nil. If there are no test files, forceTest decides
1515+
// whether this returns a new package struct or just returns p.
1516+
func TestPackagesFor(p *Package, forceTest bool) (ptest, pxtest *Package, err error) {
1517+
var imports, ximports []*Package
1518+
var stk ImportStack
1519+
stk.Push(p.ImportPath + " (test)")
1520+
rawTestImports := str.StringList(p.TestImports)
1521+
for i, path := range p.TestImports {
1522+
p1 := LoadImport(path, p.Dir, p, &stk, p.Internal.Build.TestImportPos[path], UseVendor)
1523+
if p1.Error != nil {
1524+
return nil, nil, p1.Error
1525+
}
1526+
if len(p1.DepsErrors) > 0 {
1527+
err := p1.DepsErrors[0]
1528+
err.Pos = "" // show full import stack
1529+
return nil, nil, err
1530+
}
1531+
if str.Contains(p1.Deps, p.ImportPath) || p1.ImportPath == p.ImportPath {
1532+
// Same error that loadPackage returns (via reusePackage) in pkg.go.
1533+
// Can't change that code, because that code is only for loading the
1534+
// non-test copy of a package.
1535+
err := &PackageError{
1536+
ImportStack: testImportStack(stk[0], p1, p.ImportPath),
1537+
Err: "import cycle not allowed in test",
1538+
IsImportCycle: true,
1539+
}
1540+
return nil, nil, err
1541+
}
1542+
p.TestImports[i] = p1.ImportPath
1543+
imports = append(imports, p1)
1544+
}
1545+
stk.Pop()
1546+
stk.Push(p.ImportPath + "_test")
1547+
pxtestNeedsPtest := false
1548+
rawXTestImports := str.StringList(p.XTestImports)
1549+
for i, path := range p.XTestImports {
1550+
p1 := LoadImport(path, p.Dir, p, &stk, p.Internal.Build.XTestImportPos[path], UseVendor)
1551+
if p1.Error != nil {
1552+
return nil, nil, p1.Error
1553+
}
1554+
if len(p1.DepsErrors) > 0 {
1555+
err := p1.DepsErrors[0]
1556+
err.Pos = "" // show full import stack
1557+
return nil, nil, err
1558+
}
1559+
if p1.ImportPath == p.ImportPath {
1560+
pxtestNeedsPtest = true
1561+
} else {
1562+
ximports = append(ximports, p1)
1563+
}
1564+
p.XTestImports[i] = p1.ImportPath
1565+
}
1566+
stk.Pop()
1567+
1568+
// Test package.
1569+
if len(p.TestGoFiles) > 0 || forceTest {
1570+
ptest = new(Package)
1571+
*ptest = *p
1572+
ptest.GoFiles = nil
1573+
ptest.GoFiles = append(ptest.GoFiles, p.GoFiles...)
1574+
ptest.GoFiles = append(ptest.GoFiles, p.TestGoFiles...)
1575+
ptest.Target = ""
1576+
// Note: The preparation of the vet config requires that common
1577+
// indexes in ptest.Imports, ptest.Internal.Imports, and ptest.Internal.RawImports
1578+
// all line up (but RawImports can be shorter than the others).
1579+
// That is, for 0 ≤ i < len(RawImports),
1580+
// RawImports[i] is the import string in the program text,
1581+
// Imports[i] is the expanded import string (vendoring applied or relative path expanded away),
1582+
// and Internal.Imports[i] is the corresponding *Package.
1583+
// Any implicitly added imports appear in Imports and Internal.Imports
1584+
// but not RawImports (because they were not in the source code).
1585+
// We insert TestImports, imports, and rawTestImports at the start of
1586+
// these lists to preserve the alignment.
1587+
ptest.Imports = str.StringList(p.TestImports, p.Imports)
1588+
ptest.Internal.Imports = append(imports, p.Internal.Imports...)
1589+
ptest.Internal.RawImports = str.StringList(rawTestImports, p.Internal.RawImports)
1590+
ptest.Internal.ForceLibrary = true
1591+
ptest.Internal.Build = new(build.Package)
1592+
*ptest.Internal.Build = *p.Internal.Build
1593+
m := map[string][]token.Position{}
1594+
for k, v := range p.Internal.Build.ImportPos {
1595+
m[k] = append(m[k], v...)
1596+
}
1597+
for k, v := range p.Internal.Build.TestImportPos {
1598+
m[k] = append(m[k], v...)
1599+
}
1600+
ptest.Internal.Build.ImportPos = m
1601+
} else {
1602+
ptest = p
1603+
}
1604+
1605+
// External test package.
1606+
if len(p.XTestGoFiles) > 0 {
1607+
pxtest = &Package{
1608+
PackagePublic: PackagePublic{
1609+
Name: p.Name + "_test",
1610+
ImportPath: p.ImportPath + "_test",
1611+
Root: p.Root,
1612+
Dir: p.Dir,
1613+
GoFiles: p.XTestGoFiles,
1614+
Imports: p.XTestImports,
1615+
},
1616+
Internal: PackageInternal{
1617+
LocalPrefix: p.Internal.LocalPrefix,
1618+
Build: &build.Package{
1619+
ImportPos: p.Internal.Build.XTestImportPos,
1620+
},
1621+
Imports: ximports,
1622+
RawImports: rawXTestImports,
1623+
1624+
Asmflags: p.Internal.Asmflags,
1625+
Gcflags: p.Internal.Gcflags,
1626+
Ldflags: p.Internal.Ldflags,
1627+
Gccgoflags: p.Internal.Gccgoflags,
1628+
},
1629+
}
1630+
if pxtestNeedsPtest {
1631+
pxtest.Internal.Imports = append(pxtest.Internal.Imports, ptest)
1632+
}
1633+
}
1634+
1635+
return ptest, pxtest, nil
1636+
}
1637+
1638+
func testImportStack(top string, p *Package, target string) []string {
1639+
stk := []string{top, p.ImportPath}
1640+
Search:
1641+
for p.ImportPath != target {
1642+
for _, p1 := range p.Internal.Imports {
1643+
if p1.ImportPath == target || str.Contains(p1.Deps, target) {
1644+
stk = append(stk, p1.ImportPath)
1645+
p = p1
1646+
continue Search
1647+
}
1648+
}
1649+
// Can't happen, but in case it does...
1650+
stk = append(stk, "<lost path to cycle>")
1651+
break
1652+
}
1653+
return stk
1654+
}

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

Lines changed: 12 additions & 142 deletions
Original file line numberDiff line numberDiff line change
@@ -777,56 +777,12 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
777777
// pmain - pkg.test binary
778778
var ptest, pxtest, pmain *load.Package
779779

780-
var imports, ximports []*load.Package
781-
var stk load.ImportStack
782-
stk.Push(p.ImportPath + " (test)")
783-
rawTestImports := str.StringList(p.TestImports)
784-
for i, path := range p.TestImports {
785-
p1 := load.LoadImport(path, p.Dir, p, &stk, p.Internal.Build.TestImportPos[path], load.UseVendor)
786-
if p1.Error != nil {
787-
return nil, nil, nil, p1.Error
788-
}
789-
if len(p1.DepsErrors) > 0 {
790-
err := p1.DepsErrors[0]
791-
err.Pos = "" // show full import stack
792-
return nil, nil, nil, err
793-
}
794-
if str.Contains(p1.Deps, p.ImportPath) || p1.ImportPath == p.ImportPath {
795-
// Same error that loadPackage returns (via reusePackage) in pkg.go.
796-
// Can't change that code, because that code is only for loading the
797-
// non-test copy of a package.
798-
err := &load.PackageError{
799-
ImportStack: testImportStack(stk[0], p1, p.ImportPath),
800-
Err: "import cycle not allowed in test",
801-
IsImportCycle: true,
802-
}
803-
return nil, nil, nil, err
804-
}
805-
p.TestImports[i] = p1.ImportPath
806-
imports = append(imports, p1)
807-
}
808-
stk.Pop()
809-
stk.Push(p.ImportPath + "_test")
810-
pxtestNeedsPtest := false
811-
rawXTestImports := str.StringList(p.XTestImports)
812-
for i, path := range p.XTestImports {
813-
p1 := load.LoadImport(path, p.Dir, p, &stk, p.Internal.Build.XTestImportPos[path], load.UseVendor)
814-
if p1.Error != nil {
815-
return nil, nil, nil, p1.Error
816-
}
817-
if len(p1.DepsErrors) > 0 {
818-
err := p1.DepsErrors[0]
819-
err.Pos = "" // show full import stack
820-
return nil, nil, nil, err
821-
}
822-
if p1.ImportPath == p.ImportPath {
823-
pxtestNeedsPtest = true
824-
} else {
825-
ximports = append(ximports, p1)
826-
}
827-
p.XTestImports[i] = p1.ImportPath
780+
localCover := testCover && testCoverPaths == nil
781+
782+
ptest, pxtest, err = load.TestPackagesFor(p, localCover || p.Name == "main")
783+
if err != nil {
784+
return nil, nil, nil, err
828785
}
829-
stk.Pop()
830786

831787
// Use last element of import path, not package name.
832788
// They differ when package name is "main".
@@ -844,81 +800,12 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
844800
// only for this package and only for this test?
845801
// Yes, if -cover is on but -coverpkg has not specified
846802
// a list of packages for global coverage.
847-
localCover := testCover && testCoverPaths == nil
848-
849-
// Test package.
850-
if len(p.TestGoFiles) > 0 || localCover || p.Name == "main" {
851-
ptest = new(load.Package)
852-
*ptest = *p
853-
ptest.GoFiles = nil
854-
ptest.GoFiles = append(ptest.GoFiles, p.GoFiles...)
855-
ptest.GoFiles = append(ptest.GoFiles, p.TestGoFiles...)
856-
ptest.Target = ""
857-
// Note: The preparation of the vet config requires that common
858-
// indexes in ptest.Imports, ptest.Internal.Imports, and ptest.Internal.RawImports
859-
// all line up (but RawImports can be shorter than the others).
860-
// That is, for 0 ≤ i < len(RawImports),
861-
// RawImports[i] is the import string in the program text,
862-
// Imports[i] is the expanded import string (vendoring applied or relative path expanded away),
863-
// and Internal.Imports[i] is the corresponding *Package.
864-
// Any implicitly added imports appear in Imports and Internal.Imports
865-
// but not RawImports (because they were not in the source code).
866-
// We insert TestImports, imports, and rawTestImports at the start of
867-
// these lists to preserve the alignment.
868-
ptest.Imports = str.StringList(p.TestImports, p.Imports)
869-
ptest.Internal.Imports = append(imports, p.Internal.Imports...)
870-
ptest.Internal.RawImports = str.StringList(rawTestImports, p.Internal.RawImports)
871-
ptest.Internal.ForceLibrary = true
872-
ptest.Internal.Build = new(build.Package)
873-
*ptest.Internal.Build = *p.Internal.Build
874-
m := map[string][]token.Position{}
875-
for k, v := range p.Internal.Build.ImportPos {
876-
m[k] = append(m[k], v...)
877-
}
878-
for k, v := range p.Internal.Build.TestImportPos {
879-
m[k] = append(m[k], v...)
880-
}
881-
ptest.Internal.Build.ImportPos = m
882-
883-
if localCover {
884-
ptest.Internal.CoverMode = testCoverMode
885-
var coverFiles []string
886-
coverFiles = append(coverFiles, ptest.GoFiles...)
887-
coverFiles = append(coverFiles, ptest.CgoFiles...)
888-
ptest.Internal.CoverVars = declareCoverVars(ptest.ImportPath, coverFiles...)
889-
}
890-
} else {
891-
ptest = p
892-
}
893-
894-
// External test package.
895-
if len(p.XTestGoFiles) > 0 {
896-
pxtest = &load.Package{
897-
PackagePublic: load.PackagePublic{
898-
Name: p.Name + "_test",
899-
ImportPath: p.ImportPath + "_test",
900-
Root: p.Root,
901-
Dir: p.Dir,
902-
GoFiles: p.XTestGoFiles,
903-
Imports: p.XTestImports,
904-
},
905-
Internal: load.PackageInternal{
906-
LocalPrefix: p.Internal.LocalPrefix,
907-
Build: &build.Package{
908-
ImportPos: p.Internal.Build.XTestImportPos,
909-
},
910-
Imports: ximports,
911-
RawImports: rawXTestImports,
912-
913-
Asmflags: p.Internal.Asmflags,
914-
Gcflags: p.Internal.Gcflags,
915-
Ldflags: p.Internal.Ldflags,
916-
Gccgoflags: p.Internal.Gccgoflags,
917-
},
918-
}
919-
if pxtestNeedsPtest {
920-
pxtest.Internal.Imports = append(pxtest.Internal.Imports, ptest)
921-
}
803+
if localCover {
804+
ptest.Internal.CoverMode = testCoverMode
805+
var coverFiles []string
806+
coverFiles = append(coverFiles, ptest.GoFiles...)
807+
coverFiles = append(coverFiles, ptest.CgoFiles...)
808+
ptest.Internal.CoverVars = declareCoverVars(ptest.ImportPath, coverFiles...)
922809
}
923810

924811
testDir := b.NewObjdir()
@@ -948,6 +835,7 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
948835

949836
// The generated main also imports testing, regexp, and os.
950837
// Also the linker introduces implicit dependencies reported by LinkerDeps.
838+
var stk load.ImportStack
951839
stk.Push("testmain")
952840
deps := testMainDeps // cap==len, so safe for append
953841
for _, d := range load.LinkerDeps(p) {
@@ -1150,24 +1038,6 @@ func addTestVet(b *work.Builder, p *load.Package, runAction, installAction *work
11501038
}
11511039
}
11521040

1153-
func testImportStack(top string, p *load.Package, target string) []string {
1154-
stk := []string{top, p.ImportPath}
1155-
Search:
1156-
for p.ImportPath != target {
1157-
for _, p1 := range p.Internal.Imports {
1158-
if p1.ImportPath == target || str.Contains(p1.Deps, target) {
1159-
stk = append(stk, p1.ImportPath)
1160-
p = p1
1161-
continue Search
1162-
}
1163-
}
1164-
// Can't happen, but in case it does...
1165-
stk = append(stk, "<lost path to cycle>")
1166-
break
1167-
}
1168-
return stk
1169-
}
1170-
11711041
func recompileForTest(pmain, preal, ptest, pxtest *load.Package) {
11721042
// The "test copy" of preal is ptest.
11731043
// For each package that depends on preal, make a "test copy"

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,21 @@ func runVet(cmd *base.Command, args []string) {
5757

5858
root := &work.Action{Mode: "go vet"}
5959
for _, p := range pkgs {
60-
root.Deps = append(root.Deps, b.VetAction(work.ModeBuild, work.ModeBuild, p))
60+
ptest, pxtest, err := load.TestPackagesFor(p, false)
61+
if err != nil {
62+
base.Errorf("%v", err)
63+
continue
64+
}
65+
if len(ptest.GoFiles) == 0 && pxtest == nil {
66+
base.Errorf("go vet %s: no Go files in %s", p.ImportPath, p.Dir)
67+
continue
68+
}
69+
if len(ptest.GoFiles) > 0 {
70+
root.Deps = append(root.Deps, b.VetAction(work.ModeBuild, work.ModeBuild, ptest))
71+
}
72+
if pxtest != nil {
73+
root.Deps = append(root.Deps, b.VetAction(work.ModeBuild, work.ModeBuild, pxtest))
74+
}
6175
}
6276
b.Do(root)
6377
}

0 commit comments

Comments
 (0)