Skip to content

Commit 7e8da94

Browse files
smowtonsmowton
authored
Merge pull request #17216 from smowton/smowton/feature/golang-test-extraction
Go: support extracting test code
2 parents 1cd8af5 + 209f9ec commit 7e8da94

34 files changed

+450
-14
lines changed

‎go/codeql-extractor.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,11 @@ file_types:
1919
extensions:
2020
- .go
2121
legacy_qltest_extraction: true
22+
options:
23+
extract_tests:
24+
title: Whether to include Go test files in the CodeQL database.
25+
description: >
26+
A value indicating whether Go test files should be included in the CodeQL database.
27+
The default is 'false'.
28+
type: string
29+
pattern: "^(false|true)$"

‎go/extractor/cli/go-extractor/go-extractor.go

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ func usage() {
2121
fmt.Fprintf(os.Stderr, "--help Print this help.\n")
2222
}
2323

24-
func parseFlags(args []string, mimic bool) ([]string, []string) {
24+
// extractTests is set (a) if we were manually commanded to extract tests via the relevant
25+
// environment variable / extractor option, or (b) we're mimicking a `go test` command.
26+
func parseFlags(args []string, mimic bool, extractTests bool) ([]string, []string, bool) {
2527
i := 0
2628
buildFlags := []string{}
2729
for ; i < len(args) && strings.HasPrefix(args[i], "-"); i++ {
@@ -44,9 +46,9 @@ func parseFlags(args []string, mimic bool) ([]string, []string) {
4446
if i+1 < len(args) {
4547
i++
4648
command := args[i]
47-
if command == "build" || command == "install" || command == "run" {
48-
log.Printf("Intercepting build")
49-
return parseFlags(args[i+1:], true)
49+
if command == "build" || command == "install" || command == "run" || command == "test" {
50+
log.Printf("Intercepting build for %s command", command)
51+
return parseFlags(args[i+1:], true, command == "test")
5052
} else {
5153
log.Printf("Non-build command '%s'; skipping", strings.Join(args[1:], " "))
5254
os.Exit(0)
@@ -63,12 +65,12 @@ func parseFlags(args []string, mimic bool) ([]string, []string) {
6365

6466
// parse go build flags
6567
switch args[i] {
66-
// skip `-o output` and `-i`, if applicable
68+
// skip `-o output`, `-i` and `-c`, if applicable
6769
case "-o":
6870
if i+1 < len(args) {
6971
i++
7072
}
71-
case "-i":
73+
case "-i", "-c":
7274
case "-p", "-asmflags", "-buildmode", "-compiler", "-gccgoflags", "-gcflags", "-installsuffix",
7375
"-ldflags", "-mod", "-modfile", "-pkgdir", "-tags", "-toolexec", "-overlay":
7476
if i+1 < len(args) {
@@ -90,11 +92,12 @@ func parseFlags(args []string, mimic bool) ([]string, []string) {
9092
cpuprofile = os.Getenv("CODEQL_EXTRACTOR_GO_CPU_PROFILE")
9193
memprofile = os.Getenv("CODEQL_EXTRACTOR_GO_MEM_PROFILE")
9294

93-
return buildFlags, args[i:]
95+
return buildFlags, args[i:], extractTests
9496
}
9597

9698
func main() {
97-
buildFlags, patterns := parseFlags(os.Args[1:], false)
99+
extractTestsDefault := os.Getenv("CODEQL_EXTRACTOR_GO_OPTION_EXTRACT_TESTS") == "true"
100+
buildFlags, patterns, extractTests := parseFlags(os.Args[1:], false, extractTestsDefault)
98101

99102
if cpuprofile != "" {
100103
f, err := os.Create(cpuprofile)
@@ -114,7 +117,7 @@ func main() {
114117
}
115118

116119
log.Printf("Build flags: '%s'; patterns: '%s'\n", strings.Join(buildFlags, " "), strings.Join(patterns, " "))
117-
err := extractor.ExtractWithFlags(buildFlags, patterns)
120+
err := extractor.ExtractWithFlags(buildFlags, patterns, extractTests)
118121
if err != nil {
119122
errString := err.Error()
120123
if strings.Contains(errString, "unexpected directory layout:") {

‎go/extractor/extractor.go

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,11 @@ func init() {
5959

6060
// Extract extracts the packages specified by the given patterns
6161
func Extract(patterns []string) error {
62-
return ExtractWithFlags(nil, patterns)
62+
return ExtractWithFlags(nil, patterns, false)
6363
}
6464

6565
// ExtractWithFlags extracts the packages specified by the given patterns and build flags
66-
func ExtractWithFlags(buildFlags []string, patterns []string) error {
66+
func ExtractWithFlags(buildFlags []string, patterns []string, extractTests bool) error {
6767
startTime := time.Now()
6868

6969
extraction := NewExtraction(buildFlags, patterns)
@@ -81,14 +81,22 @@ func ExtractWithFlags(buildFlags []string, patterns []string) error {
8181
}
8282
}
8383

84-
log.Println("Running packages.Load.")
84+
testMessage := ""
85+
if extractTests {
86+
testMessage = " (test extraction enabled)"
87+
}
88+
log.Printf("Running packages.Load%s.", testMessage)
89+
90+
// This includes test packages if either we're tracing a `go test` command,
91+
// or if CODEQL_EXTRACTOR_GO_OPTION_EXTRACT_TESTS is set to "true".
8592
cfg := &packages.Config{
8693
Mode: packages.NeedName | packages.NeedFiles |
8794
packages.NeedCompiledGoFiles |
8895
packages.NeedImports | packages.NeedDeps |
8996
packages.NeedTypes | packages.NeedTypesSizes |
9097
packages.NeedTypesInfo | packages.NeedSyntax,
9198
BuildFlags: buildFlags,
99+
Tests: extractTests,
92100
}
93101
pkgs, err := packages.Load(cfg, patterns...)
94102
if err != nil {
@@ -123,7 +131,7 @@ func ExtractWithFlags(buildFlags []string, patterns []string) error {
123131
if os.Getenv("CODEQL_EXTRACTOR_GO_FAST_PACKAGE_INFO") != "false" {
124132
log.Printf("Running go list to resolve package and module directories.")
125133
// get all packages information
126-
pkgInfos, err = toolchain.GetPkgsInfo(patterns, true, modFlags...)
134+
pkgInfos, err = toolchain.GetPkgsInfo(patterns, true, extractTests, modFlags...)
127135
if err != nil {
128136
log.Fatalf("Error getting dependency package or module directories: %v.", err)
129137
}
@@ -132,8 +140,36 @@ func ExtractWithFlags(buildFlags []string, patterns []string) error {
132140

133141
pkgsNotFound := make([]string, 0, len(pkgs))
134142

143+
// Build a map from package paths to their longest IDs--
144+
// in the context of a `go test -c` compilation, we will see the same package more than
145+
// once, with IDs like "abc.com/pkgname [abc.com/pkgname.test]" to distinguish the version
146+
// that contains and is used by test code.
147+
// For our purposes it is simplest to just ignore the non-test version, since the test
148+
// version seems to be a superset of it.
149+
longestPackageIds := make(map[string]string)
150+
packages.Visit(pkgs, nil, func(pkg *packages.Package) {
151+
if longestIDSoFar, present := longestPackageIds[pkg.PkgPath]; present {
152+
if len(pkg.ID) > len(longestIDSoFar) {
153+
longestPackageIds[pkg.PkgPath] = pkg.ID
154+
}
155+
} else {
156+
longestPackageIds[pkg.PkgPath] = pkg.ID
157+
}
158+
})
159+
135160
// Do a post-order traversal and extract the package scope of each package
136161
packages.Visit(pkgs, nil, func(pkg *packages.Package) {
162+
// Note that if test extraction is enabled, we will encounter a package twice here:
163+
// once as the main package, and once as the test package (with a package ID like
164+
// "abc.com/pkgname [abc.com/pkgname.test]").
165+
//
166+
// We will extract it both times however, because we need to visit the packages
167+
// in the right order in order to visit used types before their users, and the
168+
// ordering determined by packages.Visit for the main and the test package may differ.
169+
//
170+
// This should only cause some wasted time and not inconsistency because the names for
171+
// objects seen in this process should be the same each time.
172+
137173
log.Printf("Processing package %s.", pkg.PkgPath)
138174

139175
if _, ok := pkgInfos[pkg.PkgPath]; !ok {
@@ -210,6 +246,19 @@ func ExtractWithFlags(buildFlags []string, patterns []string) error {
210246

211247
// extract AST information for all packages
212248
packages.Visit(pkgs, nil, func(pkg *packages.Package) {
249+
250+
// If this is a variant of a package that also occurs with a longer ID, skip it;
251+
// otherwise we would extract the same file more than once including extracting the
252+
// body of methods twice, causing database inconsistencies.
253+
//
254+
// We prefer the version with the longest ID because that is (so far as I know) always
255+
// the version that defines more entities -- the only case I'm aware of being a test
256+
// variant of a package, which includes test-only functions in addition to the complete
257+
// contents of the main variant.
258+
if pkg.ID != longestPackageIds[pkg.PkgPath] {
259+
return
260+
}
261+
213262
for root := range wantedRoots {
214263
pkgInfo := pkgInfos[pkg.PkgPath]
215264
relDir, err := filepath.Rel(root, pkgInfo.PkgDir)

‎go/extractor/toolchain/toolchain.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,14 +223,19 @@ type PkgInfo struct {
223223
// GetPkgsInfo gets the absolute module and package root directories for the packages matched by the
224224
// patterns `patterns`. It passes to `go list` the flags specified by `flags`. If `includingDeps`
225225
// is true, all dependencies will also be included.
226-
func GetPkgsInfo(patterns []string, includingDeps bool, flags ...string) (map[string]PkgInfo, error) {
226+
func GetPkgsInfo(patterns []string, includingDeps bool, extractTests bool, flags ...string) (map[string]PkgInfo, error) {
227227
// enable module mode so that we can find a module root if it exists, even if go module support is
228228
// disabled by a build
229229
if includingDeps {
230230
// the flag `-deps` causes all dependencies to be retrieved
231231
flags = append(flags, "-deps")
232232
}
233233

234+
if extractTests {
235+
// Without the `-test` flag, test packages would be omitted from the `go list` output.
236+
flags = append(flags, "-test")
237+
}
238+
234239
// using -json overrides -f format
235240
output, err := RunList("", patterns, append(flags, "-json")...)
236241
if err != nil {
@@ -272,6 +277,12 @@ func GetPkgsInfo(patterns []string, includingDeps bool, flags ...string) (map[st
272277
PkgDir: pkgAbsDir,
273278
ModDir: modAbsDir,
274279
}
280+
281+
if extractTests && strings.Contains(pkgInfo.ImportPath, " [") {
282+
// Assume " [" is the start of a qualifier, and index the package by its base name
283+
baseImportPath := strings.Split(pkgInfo.ImportPath, " [")[0]
284+
pkgInfoMapping[baseImportPath] = pkgInfoMapping[pkgInfo.ImportPath]
285+
}
275286
}
276287
return pkgInfoMapping, nil
277288
}

‎go/ql/integration-tests/go-mod-sample/src/blackbox_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package makesample_test
2+
3+
import (
4+
"makesample"
5+
"testing"
6+
)
7+
8+
// Note because this is a test we do NOT expect this to be extracted.
9+
func TestTestMe(t *testing.T) {
10+
11+
publicResult := makesample.PublicFunction()
12+
if publicResult != 1 {
13+
t.Errorf("Expected 1, got %d", publicResult)
14+
}
15+
16+
}

‎go/ql/integration-tests/go-mod-sample/src/test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,7 @@ func test() {
1010
header.Version = 4
1111

1212
}
13+
14+
func PublicFunction() int {
15+
return 1
16+
}

‎go/ql/integration-tests/go-mod-sample/src/test_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package makesample
2+
3+
import (
4+
"testing"
5+
)
6+
7+
func TestTestMe(t *testing.T) {
8+
9+
// Note because this is a test we do NOT expect this to be extracted.
10+
publicResult := PublicFunction()
11+
if publicResult != 1 {
12+
t.Errorf("Expected 1, got %d", publicResult)
13+
}
14+
15+
}

‎go/ql/integration-tests/test-extraction-autobuild/src/Makefile

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
all:
2+
go get

‎go/ql/integration-tests/test-extraction-autobuild/src/go.mod

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
go 1.14
2+
3+
module testsample

‎go/ql/integration-tests/test-extraction-autobuild/src/go.sum

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
2+
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
3+
golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
4+
golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU=
5+
golang.org/x/crypto v0.21.0/go.mod h1:0BP7YvVV9gBbVKyeTG0Gyn+gZm94bibOW5BjDEYAOMs=
6+
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
7+
golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
8+
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
9+
golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
10+
golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
11+
golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs=
12+
golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg=
13+
golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44=
14+
golang.org/x/net v0.23.0 h1:7EYJ93RZ9vYSZAIb2x3lnuvqO5zneoD6IvWjuhfxjTs=
15+
golang.org/x/net v0.23.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg=
16+
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
17+
golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
18+
golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
19+
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
20+
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
21+
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
22+
golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
23+
golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
24+
golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
25+
golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
26+
golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
27+
golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4=
28+
golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
29+
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
30+
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
31+
golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k=
32+
golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo=
33+
golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk=
34+
golang.org/x/term v0.18.0/go.mod h1:ILwASektA3OnRv7amZ1xhE/KTR+u50pbXfZ03+6Nx58=
35+
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
36+
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
37+
golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
38+
golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
39+
golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8=
40+
golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU=
41+
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
42+
golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
43+
golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
44+
golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU=
45+
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=

0 commit comments

Comments
 (0)