exit-code of 'go list' is ignored

[yves-tutti]

Actual behavior when "go list" is invoked the exit-code is discarded in createPackageMap().

Expected behavior The exit code should be checked and if non-zero the error could be printed.

To Reproduce Add/replace in createPackageMap:

	var stderr bytes.Buffer
	cmd.Stderr = &stderr
	err := cmd.Run()
	if err != nil {
		log.Printf("failed to decode 'go list' output: %s: %s", err, stderr.String())
	}

Executing the test import_embedded_interface will output: exit status 1: package syscall/js: build constraints exclude all Go files in [..]/go/src/syscall/js

This happens because mockgen parses all files in the package, this way it parses files which are normally excluded on the current platform, in these files it finds imports to other packages (here syscall/js) which can't be read by "go list" since that checks the build-constraints.

The fix for that could be to change parsePackage(path) to only process files that meet the build constraints. That means changing:

	if imp, err := build.Import(path, newP.srcDir, 0); err != nil {
		return nil, err
	} else if pkgs, err = parser.ParseDir(newP.fileSet, imp.Dir, func(info fs.FileInfo) bool {
		res := slices.Contains(imp.GoFiles, info.Name())
		// if 'p' parser is for the same package then the one we are importing then
		// this is because of an embedded interface that might potentially be defined in a _test.go file.
		// For this case, include the _test.go files in the parser.
		if p.srcDir == imp.Dir {
			res = res || slices.Contains(imp.TestGoFiles, info.Name())
		}
		return res
	}, 0); err != nil {
		return nil, err
	}

This change would also reduce the number of source files that have to be parsed over all and this way makes mockgen a bit faster in my tests.

[JacobOaks]

Hey @yves-tutti! Thanks for reporting this. If you'd like to submit a PR to fix this, we'd be happy to review. Otherwise we should try to fix it soon.

[yuri-potatoq]

Hey everybody. I was testing that code suggestion to open a PR.

But i face some problems with the build.ImportMode as 0. That way we will also search for vendors import paths.
And then go list will receive some import filters which will never find because are dependencies from std packages (in this example, from net/http package).

With import_embedded_interface test, go list will receive something like that:

go list -json=ImportPath,Name context crypto/tls encoding/base64 errors fmt io log net/http/internal/ascii net/url reflect sort strings sync sync/atomic time mime/multipart net/textproto net strconv io/fs internal/safefilepath mime os path path/filepath bufio bytes compress/gzip crypto/rand encoding/binary math math/bits net/http/httptrace runtime golang.org/x/net/http/httpguts golang.org/x/net/http2/hpack golang.org/x/net/idna unicode/utf8 unicode internal/godebug net/http/internal container/list golang.org/x/net/http/httpproxy

And then raise that error:

2024/08/10 10:19:47 failed to decode 'go list' output: exit status 1: missing go.sum entry for module providing package golang.org/x/net/http/httpguts; to add:
        go mod download golang.org/x/net
missing go.sum entry for module providing package golang.org/x/net/http2/hpack; to add:
        go mod download golang.org/x/net
missing go.sum entry for module providing package golang.org/x/net/idna; to add:
        go mod download golang.org/x/net
missing go.sum entry for module providing package golang.org/x/net/http/httpproxy; to add:
        go mod download golang.org/x/net

These dependencies also comes from here.

Can somebody help me with a tip to avoid that? I tried to use build.IgnoreVendor but didn't worked. Maybe we can filter that into ParseDir filter callback, but i'm not sure how hard can be to mantain the line with p.srcDir == imp.Dir is already doing that, but we need a more complex logic to handle imports like go.uber.org/mock/mockgen/internal/tests/import_embedded_interface/ersatz in that test case.