go/internal/gcimporter: fix bug in struct iexport

The iexport function was emitting the wrong implicit
qualifying package name for unexported struct fields.

The logic was deceptively similar to that in the compiler,
and that was the bug: the compiler records which package
a struct{...} syntax node appears in, and uses that (correctly)
as the qualifier for unexported fields; but go/types only
records a package in each Object, such as the field Vars,
which led the author of the previous code to use the
current package, not the struct's package, as the implicit
qualifier. The solution is to use the package of any
field; they should all be the same.

(Unlike interfaces, where embedding is flattened out,
leading to interface types in which method Func objects
may belong to different packages, struct embedding is
not flattened out, so all field Vars belong to the
same package in which the struct was declared.)

Also, a regression test.

Thanks to Rob Findley for identifying the cause.

I don't understand how this hasn't shown up sooner,
since the test case is far from obscure.

Change-Id: I0a6c58a566b87a148827fb0ab4655a020806c31a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/445097
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>

go/internal/gcimporter/iexport.go

@@ -602,14 +602,17 @@ func (w *exportWriter) doTyp(t types.Type, pkg *types.Package) {
 
 	case *types.Struct:
 		w.startType(structType)
-		w.setPkg(pkg, true)
-
 		n := t.NumFields()
+		if n > 0 {
+			w.setPkg(t.Field(0).Pkg(), true) // qualifying package for field objects
+		} else {
+			w.setPkg(pkg, true)
+		}
 		w.uint64(uint64(n))
 		for i := 0; i < n; i++ {
 			f := t.Field(i)
 			w.pos(f.Pos())
-			w.string(f.Name())
+			w.string(f.Name()) // unexported fields implicitly qualified by prior setPkg
 			w.typ(f.Type(), pkg)
 			w.bool(f.Anonymous())
 			w.string(t.Tag(i)) // note (or tag)

go/internal/gcimporter/iexport_test.go

@@ -30,6 +30,7 @@ import (
 
 	"golang.org/x/tools/go/ast/inspector"
 	"golang.org/x/tools/go/buildutil"
+	"golang.org/x/tools/go/gcexportdata"
 	"golang.org/x/tools/go/internal/gcimporter"
 	"golang.org/x/tools/go/loader"
 	"golang.org/x/tools/internal/typeparams/genericfeatures"
@@ -403,3 +404,50 @@ func valueToRat(x constant.Value) *big.Rat {
 	}
 	return new(big.Rat).SetInt(new(big.Int).SetBytes(bytes))
 }
+
+// This is a regression test for a bug in iexport of types.Struct:
+// unexported fields were losing their implicit package qualifier.
+func TestUnexportedStructFields(t *testing.T) {
+	fset := token.NewFileSet()
+	export := make(map[string][]byte)
+
+	// process parses and type-checks a single-file
+	// package and saves its export data.
+	process := func(path, content string) {
+		syntax, err := parser.ParseFile(fset, path+"/x.go", content, 0)
+		if err != nil {
+			t.Fatal(err)
+		}
+		packages := make(map[string]*types.Package) // keys are package paths
+		cfg := &types.Config{
+			Importer: importerFunc(func(path string) (*types.Package, error) {
+				data, ok := export[path]
+				if !ok {
+					return nil, fmt.Errorf("missing export data for %s", path)
+				}
+				return gcexportdata.Read(bytes.NewReader(data), fset, packages, path)
+			}),
+		}
+		pkg := types.NewPackage(path, syntax.Name.Name)
+		check := types.NewChecker(cfg, fset, pkg, nil)
+		if err := check.Files([]*ast.File{syntax}); err != nil {
+			t.Fatal(err)
+		}
+		var out bytes.Buffer
+		if err := gcexportdata.Write(&out, fset, pkg); err != nil {
+			t.Fatal(err)
+		}
+		export[path] = out.Bytes()
+	}
+
+	// Historically this led to a spurious error:
+	// "cannot convert a.M (variable of type a.MyTime) to type time.Time"
+	// because the private fields of Time and MyTime were not identical.
+	process("time", `package time; type Time struct { x, y int }`)
+	process("a", `package a; import "time"; type MyTime time.Time; var M MyTime`)
+	process("b", `package b; import ("a"; "time"); var _ = time.Time(a.M)`)
+}
+
+type importerFunc func(path string) (*types.Package, error)
+
+func (f importerFunc) Import(path string) (*types.Package, error) { return f(path) }