internal/gcimporter: updates how imports are handled in unified IR
The imports of the main package are now all of the packages that
were referenced during reading the unified IR. This is similar to
the non-bundled option for iimport.
Without this imports of some packages are not the transitive closure.
Fixes golang/go#58296
Change-Id: I4c58c99a5538a9f990ca7325baa1637e7141a97e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/465936
Run-TryBot: Tim King <taking@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
diff --git a/internal/gcimporter/gcimporter_test.go b/internal/gcimporter/gcimporter_test.go
index 6ef704c..0a247f7 100644
--- a/internal/gcimporter/gcimporter_test.go
+++ b/internal/gcimporter/gcimporter_test.go
@@ -23,6 +23,7 @@
"path"
"path/filepath"
"runtime"
+ "sort"
"strings"
"testing"
"time"
@@ -847,6 +848,60 @@
}
}
+func TestIssue58296(t *testing.T) {
+ // Compiles packages c, b, and a where c imports b and b imports a,
+ // then imports c with stub *types.Packages for b and a, and checks that
+ // both a and b are in the Imports() of c.
+ //
+ // This is how go/packages can read the exportdata when NeedDeps is off.
+
+ // This package only handles gc export data.
+ needsCompiler(t, "gc")
+ testenv.NeedsGoBuild(t) // to find stdlib export data in the build cache
+
+ // On windows, we have to set the -D option for the compiler to avoid having a drive
+ // letter and an illegal ':' in the import path - just skip it (see also issue #3483).
+ if runtime.GOOS == "windows" {
+ t.Skip("avoid dealing with relative paths/drive letters on windows")
+ }
+
+ tmpdir := mktmpdir(t)
+ defer os.RemoveAll(tmpdir)
+ testoutdir := filepath.Join(tmpdir, "testdata")
+
+ apkg := filepath.Join(testoutdir, "a")
+ bpkg := filepath.Join(testoutdir, "b")
+ cpkg := filepath.Join(testoutdir, "c")
+
+ srcdir := filepath.Join("testdata", "issue58296")
+ compilePkg(t, filepath.Join(srcdir, "a"), "a.go", testoutdir, nil, apkg)
+ compilePkg(t, filepath.Join(srcdir, "b"), "b.go", testoutdir, map[string]string{apkg: filepath.Join(testoutdir, "a.o")}, bpkg)
+ compilePkg(t, filepath.Join(srcdir, "c"), "c.go", testoutdir, map[string]string{bpkg: filepath.Join(testoutdir, "b.o")}, cpkg)
+
+ // The export data reader for c cannot rely on Package.Imports
+ // being populated for a or b. (For the imports {a,b} it is unset.)
+ imports := map[string]*types.Package{
+ apkg: types.NewPackage(apkg, "a"),
+ bpkg: types.NewPackage(bpkg, "b"),
+ }
+
+ // make sure a and b are both imported by c.
+ pkg, err := Import(imports, "./c", testoutdir, nil)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ var names []string
+ for _, imp := range pkg.Imports() {
+ names = append(names, imp.Name())
+ }
+ sort.Strings(names)
+
+ if got, want := strings.Join(names, ","), "a,b"; got != want {
+ t.Errorf("got imports %v for package c. wanted %v", names, want)
+ }
+}
+
// apkg returns the package "a" prefixed by (as a package) testoutdir
func apkg(testoutdir string) string {
apkg := testoutdir + "/a"
diff --git a/internal/gcimporter/testdata/issue58296/a/a.go b/internal/gcimporter/testdata/issue58296/a/a.go
new file mode 100644
index 0000000..236978a
--- /dev/null
+++ b/internal/gcimporter/testdata/issue58296/a/a.go
@@ -0,0 +1,9 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package a
+
+type A int
+
+func (A) f() {}
diff --git a/internal/gcimporter/testdata/issue58296/b/b.go b/internal/gcimporter/testdata/issue58296/b/b.go
new file mode 100644
index 0000000..8886ca5
--- /dev/null
+++ b/internal/gcimporter/testdata/issue58296/b/b.go
@@ -0,0 +1,11 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package b
+
+import "./a"
+
+type B struct {
+ a a.A
+}
diff --git a/internal/gcimporter/testdata/issue58296/c/c.go b/internal/gcimporter/testdata/issue58296/c/c.go
new file mode 100644
index 0000000..bad8be8
--- /dev/null
+++ b/internal/gcimporter/testdata/issue58296/c/c.go
@@ -0,0 +1,11 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package c
+
+import "./b"
+
+type C struct {
+ b b.B
+}
diff --git a/internal/gcimporter/ureader_yes.go b/internal/gcimporter/ureader_yes.go
index b285a11..34fc783 100644
--- a/internal/gcimporter/ureader_yes.go
+++ b/internal/gcimporter/ureader_yes.go
@@ -12,6 +12,7 @@
import (
"go/token"
"go/types"
+ "sort"
"strings"
"golang.org/x/tools/internal/pkgbits"
@@ -121,6 +122,16 @@
iface.Complete()
}
+ // Imports() of pkg are all of the transitive packages that were loaded.
+ var imps []*types.Package
+ for _, imp := range pr.pkgs {
+ if imp != nil && imp != pkg {
+ imps = append(imps, imp)
+ }
+ }
+ sort.Sort(byPath(imps))
+ pkg.SetImports(imps)
+
pkg.MarkComplete()
return pkg
}
@@ -260,39 +271,9 @@
pkg := types.NewPackage(path, name)
r.p.imports[path] = pkg
- imports := make([]*types.Package, r.Len())
- for i := range imports {
- imports[i] = r.pkg()
- }
- pkg.SetImports(flattenImports(imports))
-
return pkg
}
-// flattenImports returns the transitive closure of all imported
-// packages rooted from pkgs.
-func flattenImports(pkgs []*types.Package) []*types.Package {
- var res []*types.Package
- seen := make(map[*types.Package]struct{})
- for _, pkg := range pkgs {
- if _, ok := seen[pkg]; ok {
- continue
- }
- seen[pkg] = struct{}{}
- res = append(res, pkg)
-
- // pkg.Imports() is already flattened.
- for _, pkg := range pkg.Imports() {
- if _, ok := seen[pkg]; ok {
- continue
- }
- seen[pkg] = struct{}{}
- res = append(res, pkg)
- }
- }
- return res
-}
-
// @@@ Types
func (r *reader) typ() types.Type {
