x/tools/go/packages: overlay does not cause ExportFile to be cleared

[mateusz834]

In the x/tools/go/pacakges there is a comment, mentioning that overlay might invalidate export data, see:

https://github.com/golang/tools/blob/b93274bf6492f1e1ab72ee5def8da5dfbc9e0e43/go/packages/packages.go#L800-L802

It was introduced in CL 151999.

If that is still true, then we should probably also clear the ExportFile field, this is currently not being done. But looking at the exported file, it seems to include the Test2 symbol defined in the overlay, so it seems to work. The comment mentions that it can so maybe this might happen only in some other corner-cases (that i am not aware of), if so we should probably clear that ExportFile.

Reproducer (using the x/tools/go/packages testing API):

func TestReproducer(t *testing.T) {
	log.SetFlags(log.Lshortfile)
	exported := packagestest.Export(t, packagestest.Modules, []packagestest.Module{
		{
			Name: "test",
			Files: map[string]any{
				"test.go":       `package test; import "test/other"; func test() { other.Test2() }`,
				"other/file.go": `package other; func Test() {}`,
			},
			Overlay: map[string][]byte{
				`other/file.go`: []byte(`package other; func Test2() {}`),
			},
		},
	})
	t.Cleanup(exported.Cleanup)

	exported.Config.Mode = packages.LoadSyntax | packages.NeedExportFile
	pkgs, err := packages.Load(exported.Config, "test")
	if err != nil {
		t.Error(err)
	}
	packages.PrintErrors(pkgs)

	ef := pkgs[0].Imports["test/other"].ExportFile
	if ef != "" {
		t.Error("ExportFile for package text/other is not empty") // fails
	}

	f, err := os.Open(ef)
	if err != nil {
		t.Fatal(err)
	}

	r, err := gcexportdata.NewReader(f)
	if err != nil {
		t.Fatal(err)
	}

	fset := token.NewFileSet()
	pkg, err := gcexportdata.Read(r, fset, make(map[string]*types.Package), "test/other")
	if err != nil {
		t.Fatal(err)
	}

	t.Log(pkg.Scope().Lookup("Test2") != nil) // true
}

CC @matloob @adonovan

[gabyhelp]

Related Issues

• x/tools/go/packages: overlay does not work properly with external module files #71075

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[matloob]

As I understand it, the comment isn't true anymore. We now use the go command's overlay support (which was added after the referenced code was written) to handle overlays, so the export data should take overlays into account.