Files excluded by build constraints make gobco panic with "redeclared in this block"

[jmrplens]

Since 1.3.4, gobco type-checks every .go file of the package directory as one package, whatever its build constraints say. A package that implements a function once per platform, which is the usual Go idiom for platform-specific code, therefore makes gobco panic before anything is instrumented, while go test passes on the same package.

Minimal reproduction

Four files in an empty directory:

go.mod

module example.com/repro

go 1.22

sep_unix.go

//go:build !windows

package repro

func separator(c byte) bool {
	return c == '/'
}

sep_windows.go (constrained by its file name alone)

package repro

func separator(c byte) bool {
	return c == '/' || c == '\\'
}

repro_test.go

package repro

import "testing"

func TestSeparator(t *testing.T) {
	if !separator('/') {
		t.Error("a slash is a separator")
	}
}

On Linux:

$ go test ./... -count=1
ok  	example.com/repro	0.001s
$ go run github.com/rillig/gobco@v1.3.4
panic: sep_windows.go:3:6: separator redeclared in this block

goroutine 1 [running]:
main.ok(...)
	/go/pkg/mod/github.com/rillig/gobco@v1.3.4/util.go:100
main.(*instrumenter).resolveTypes(0xc623d6de070, 0xc623d6f8150)
	/go/pkg/mod/github.com/rillig/gobco@v1.3.4/instrumenter.go:143 +0x445
main.(*instrumenter).instrument(0xc623d6de070, {0x6c3000, 0x1}, {0x0, 0x0}, {0xc623d6f6140, 0x33})
	/go/pkg/mod/github.com/rillig/gobco@v1.3.4/instrumenter.go:95 +0x105
main.(*gobco).instrument(0xc623d754120)
	/go/pkg/mod/github.com/rillig/gobco@v1.3.4/main.go:275 +0x2b6
main.gobcoMain({0x96fc10?, 0xc623d706050?}, {0x96fc10?, 0xc623d706058?}, {0xc623d7161f0, 0x1, 0x1})
	/go/pkg/mod/github.com/rillig/gobco@v1.3.4/main.go:27 +0x67
main.main()
	/go/pkg/mod/github.com/rillig/gobco@v1.3.4/main.go:20 +0x45
exit status 2

1.3.3 reports Condition coverage: 1/2 for this package, as it did not resolve types yet. The master branch at 7a09995 panics like 1.3.4. All measured with Go 1.27.1 on linux/amd64.

Related cases with the same cause

1. A //go:build ignore file with package main next to the package, as used for go generate, makes gobco write a gobco_bridge_test.go containing import "", and go test fails with invalid import path (1.3.3, 1.3.4 and master).
2. A TestMain in a test file that is not built on the current platform, such as main_windows_test.go on Linux, keeps gobco from adding its own TestMain. The result is open .../gobco-counts.json: no such file or directory followed by Condition coverage: 0/0, with exit status 0 (1.3.3, 1.3.4 and master).
3. Files that are only built with a build tag, as in go test -tags integration, are never instrumented, not even with gobco -test -tags=integration. The report says Condition coverage: 0/0 although go test compiled and ran the code (1.3.3, 1.3.4 and master). The same happens to a file constrained on a release tag such as //go:build go1.21 (1.3.4).
4. If such a package has a black box test, gobco panics with could not import example.com/blackbox (no buildable Go source files in ...), since the package under test is imported without the build tags (1.3.4 and master).

Where

• instrumenter.instrument passes every .go file of the directory to parser.ParseDir, whose filter only narrows the files down to a single file given on the command line, and resolveTypes type-checks all of them together. ParseDir also reads files whose names start with _ or ., which the go command ignores.
• shouldBuild matches each file against build.Context{GOOS: runtime.GOOS, GOARCH: runtime.GOARCH}, which has no build tags and no release tags. Files constrained on a custom tag, or on go1.21, are therefore not instrumented even though go test builds them.
• The source importer from importer.ForCompiler(fset, "source", nil) resolves the imported packages in build.Default, which knows nothing about the tags that gobco passes to go test.

Possible fix

Select the files with build.Default.MatchFile before parsing them, which makes shouldBuild unnecessary, and take the build tags from GOFLAGS and from the -test options, the same way as the go command. Since importer.ForCompiler offers no way to pass another build context, the tags have to be set in build.Default.BuildTags while the package is instrumented.

I have a patch for this with tests on https://github.com/jmrplens/gobco/tree/build-constraints, and I'll open a pull request from it that refers to this issue. Feel free to take only what you like from it.