[release-branch.go1.11] go/types: use correct receiver types for embedded interface methods

Interface methods don't declare a receiver (it's implicit), but after
type-checking the respective *types.Func objects are marked as methods
by having a receiver. For interface methods, the receiver base type used
to be the interface that declared the method in the first place, even if
the method also appeared in other interfaces via embedding. A change in
the computation of method sets for interfaces for Go1.10 changed that
inadvertently, with the consequence that sometimes a method's receiver
type ended up being an interface into which the method was embedded.
The exact behavior also depended on file type-checking order, and because
files are sometimes sorted by name, the behavior depended on file names.

This didn't matter for type-checking (the typechecker doesn't need the
receiver), but it matters for clients, and for printing of methods.

This change fixes interface method receivers at the end of type-checking
when we have all relevant information.

Fixes #28249
Updates #28005

Change-Id: I96c120fb0e517d7f8a14b8530f0273674569d5ea
Reviewed-on: https://go-review.googlesource.com/c/141358
Reviewed-by: Alan Donovan <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/146660
Run-TryBot: Brad Fitzpatrick <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>

Author: griesemer

src/go/types/issues_test.go

@@ -355,3 +355,70 @@ func TestIssue25627(t *testing.T) {
 		})
 	}
 }
+
+func TestIssue28005(t *testing.T) {
+	// method names must match defining interface name for this test
+	// (see last comment in this function)
+	sources := [...]string{
+		"package p; type A interface{ A() }",
+		"package p; type B interface{ B() }",
+		"package p; type X interface{ A; B }",
+	}
+
+	// compute original file ASTs
+	var orig [len(sources)]*ast.File
+	for i, src := range sources {
+		f, err := parser.ParseFile(fset, "", src, 0)
+		if err != nil {
+			t.Fatal(err)
+		}
+		orig[i] = f
+	}
+
+	// run the test for all order permutations of the incoming files
+	for _, perm := range [][len(sources)]int{
+		{0, 1, 2},
+		{0, 2, 1},
+		{1, 0, 2},
+		{1, 2, 0},
+		{2, 0, 1},
+		{2, 1, 0},
+	} {
+		// create file order permutation
+		files := make([]*ast.File, len(sources))
+		for i := range perm {
+			files[i] = orig[perm[i]]
+		}
+
+		// type-check package with given file order permutation
+		var conf Config
+		info := &Info{Defs: make(map[*ast.Ident]Object)}
+		_, err := conf.Check("", fset, files, info)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		// look for interface object X
+		var obj Object
+		for name, def := range info.Defs {
+			if name.Name == "X" {
+				obj = def
+				break
+			}
+		}
+		if obj == nil {
+			t.Fatal("interface not found")
+		}
+		iface := obj.Type().Underlying().(*Interface) // I must be an interface
+
+		// Each iface method m is embedded; and m's receiver base type name
+		// must match the method's name per the choice in the source file.
+		for i := 0; i < iface.NumMethods(); i++ {
+			m := iface.Method(i)
+			recvName := m.Type().(*Signature).Recv().Type().(*Named).Obj().Name()
+			if recvName != m.Name() {
+				t.Errorf("perm %v: got recv %s; want %s", perm, recvName, m.Name())
+			}
+		}
+	}
+}

src/go/types/typexpr.go

@@ -571,6 +571,15 @@ func (check *Checker) interfaceType(ityp *Interface, iface *ast.InterfaceType, d
 		recvTyp = def
 	}
 
+	// Correct receiver type for all methods explicitly declared
+	// by this interface after we're done with type-checking at
+	// this level. See comment below for details.
+	check.later(func() {
+		for _, m := range ityp.methods {
+			m.typ.(*Signature).recv.typ = recvTyp
+		}
+	})
+
 	// collect methods
 	var sigfix []*methodInfo
 	for i, minfo := range info.methods {
@@ -580,9 +589,27 @@ func (check *Checker) interfaceType(ityp *Interface, iface *ast.InterfaceType, d
 			pos := name.Pos()
 			// Don't type-check signature yet - use an
 			// empty signature now and update it later.
-			// Since we know the receiver, set it up now
-			// (required to avoid crash in ptrRecv; see
-			// e.g. test case for issue 6638).
+			// But set up receiver since we know it and
+			// its position, and because interface method
+			// signatures don't get a receiver via regular
+			// type-checking (there isn't a receiver in the
+			// method's AST). Setting the receiver type is
+			// also important for ptrRecv() (see methodset.go).
+			//
+			// Note: For embedded methods, the receiver type
+			// should be the type of the interface that declared
+			// the methods in the first place. Since we get the
+			// methods here via methodInfo, which may be computed
+			// before we have all relevant interface types, we use
+			// the current interface's type (recvType). This may be
+			// the type of the interface embedding the interface that
+			// declared the methods. This doesn't matter for type-
+			// checking (we only care about the receiver type for
+			// the ptrRecv predicate, and it's never a pointer recv
+			// for interfaces), but it matters for go/types clients
+			// and for printing. We correct the receiver after type-
+			// checking.
+			//
 			// TODO(gri) Consider marking methods signatures
 			// as incomplete, for better error messages. See
 			// also the T4 and T5 tests in testdata/cycles2.src.