cmd/compile: package-annotate structs when error would be ambiguous

Before emitting a "wanted Foo but got Bar" message for an interface
type match failure, check that Foo and Bar are different.  If they
are not, add package paths to first unexported struct field seen,
because that is the cause (a cause, there could be more than one).

Replicated in go/types.

Added tests to go/types and cmd/compile/internal/types2

Fixes #54258.

Change-Id: Ifc2b2067d62fe2138996972cdf3b6cb7ca0ed456
Reviewed-on: https://go-review.googlesource.com/c/go/+/422914
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: David Chase <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>

src/cmd/compile/internal/typecheck/expr.go

@@ -516,6 +516,18 @@ func tcDot(n *ir.SelectorExpr, top int) ir.Node {
 	return n
 }
 
+func wrongTypeFor(haveSym *types.Sym, haveType *types.Type, wantSym *types.Sym, wantType *types.Type) string {
+	haveT := fmt.Sprintf("%S", haveType)
+	wantT := fmt.Sprintf("%S", wantType)
+	if haveT == wantT {
+		// Add packages instead of reporting "got Foo but wanted Foo", see #54258.
+		haveT = haveType.Pkg().Path + "." + haveT
+		wantT = wantType.Pkg().Path + "." + wantT
+	}
+	return fmt.Sprintf("(wrong type for %v method)\n"+
+		"\t\thave %v%s\n\t\twant %v%s", wantSym, haveSym, haveT, wantSym, wantT)
+}
+
 // tcDotType typechecks an ODOTTYPE node.
 func tcDotType(n *ir.TypeAssertExpr) ir.Node {
 	n.X = Expr(n.X)
@@ -539,8 +551,8 @@ func tcDotType(n *ir.TypeAssertExpr) ir.Node {
 		var ptr int
 		if !implements(n.Type(), t, &missing, &have, &ptr) {
 			if have != nil && have.Sym == missing.Sym {
-				base.Errorf("impossible type assertion:\n\t%v does not implement %v (wrong type for %v method)\n"+
-					"\t\thave %v%S\n\t\twant %v%S", n.Type(), t, missing.Sym, have.Sym, have.Type, missing.Sym, missing.Type)
+				base.Errorf("impossible type assertion:\n\t%v does not implement %v %s", n.Type(), t,
+					wrongTypeFor(have.Sym, have.Type, missing.Sym, missing.Type))
 			} else if ptr != 0 {
 				base.Errorf("impossible type assertion:\n\t%v does not implement %v (%v method has pointer receiver)", n.Type(), t, missing.Sym)
 			} else if have != nil {

src/cmd/compile/internal/typecheck/stmt.go

@@ -604,8 +604,8 @@ func tcSwitchType(n *ir.SwitchStmt) {
 			}
 			if !n1.Type().IsInterface() && !implements(n1.Type(), t, &missing, &have, &ptr) {
 				if have != nil {
-					base.ErrorfAt(ncase.Pos(), "impossible type switch case: %L cannot have dynamic type %v"+
-						" (wrong type for %v method)\n\thave %v%S\n\twant %v%S", guard.X, n1.Type(), missing.Sym, have.Sym, have.Type, missing.Sym, missing.Type)
+					base.ErrorfAt(ncase.Pos(), "impossible type switch case: %L cannot have dynamic type %v %s", guard.X, n1.Type(),
+						wrongTypeFor(have.Sym, have.Type, missing.Sym, missing.Type))
 				} else if ptr != 0 {
 					base.ErrorfAt(ncase.Pos(), "impossible type switch case: %L cannot have dynamic type %v"+
 						" (%v method has pointer receiver)", guard.X, n1.Type(), missing.Sym)

src/cmd/compile/internal/typecheck/subr.go

@@ -397,8 +397,7 @@ func Assignop1(src, dst *types.Type) (ir.Op, string) {
 		} else if have != nil && have.Sym == missing.Sym && have.Nointerface() {
 			why = fmt.Sprintf(":\n\t%v does not implement %v (%v method is marked 'nointerface')", src, dst, missing.Sym)
 		} else if have != nil && have.Sym == missing.Sym {
-			why = fmt.Sprintf(":\n\t%v does not implement %v (wrong type for %v method)\n"+
-				"\t\thave %v%S\n\t\twant %v%S", src, dst, missing.Sym, have.Sym, have.Type, missing.Sym, missing.Type)
+			why = fmt.Sprintf(":\n\t%v does not implement %v %s", src, dst, wrongTypeFor(have.Sym, have.Type, missing.Sym, missing.Type))
 		} else if ptr != 0 {
 			why = fmt.Sprintf(":\n\t%v does not implement %v (%v method has pointer receiver)", src, dst, missing.Sym)
 		} else if have != nil {

src/cmd/compile/internal/types2/issues_test.go

@@ -10,6 +10,7 @@ import (
 	"cmd/compile/internal/syntax"
 	"fmt"
 	"internal/testenv"
+	"regexp"
 	"sort"
 	"strings"
 	"testing"
@@ -703,3 +704,123 @@ func TestIssue51093(t *testing.T) {
 		}
 	}
 }
+
+func TestIssue54258(t *testing.T) {
+	tests := []struct{ main, b, want string }{
+		{ //---------------------------------------------------------------
+			`package main
+import "b"
+type I0 interface {
+	M0(w struct{ f string })
+}
+var _ I0 = b.S{}
+`,
+			`package b
+type S struct{}
+func (S) M0(struct{ f string }) {}
+`,
+			`6:12: cannot use b[.]S{} [(]value of type b[.]S[)] as I0 value in variable declaration: b[.]S does not implement I0 [(]wrong type for method M0[)]
+.*have M0[(]struct{f string /[*] package b [*]/ }[)]
+.*want M0[(]struct{f string /[*] package main [*]/ }[)]`},
+
+		{ //---------------------------------------------------------------
+			`package main
+import "b"
+type I1 interface {
+	M1(struct{ string })
+}
+var _ I1 = b.S{}
+`,
+			`package b
+type S struct{}
+func (S) M1(struct{ string }) {}
+`,
+			`6:12: cannot use b[.]S{} [(]value of type b[.]S[)] as I1 value in variable declaration: b[.]S does not implement I1 [(]wrong type for method M1[)]
+.*have M1[(]struct{string /[*] package b [*]/ }[)]
+.*want M1[(]struct{string /[*] package main [*]/ }[)]`},
+
+		{ //---------------------------------------------------------------
+			`package main
+import "b"
+type I2 interface {
+	M2(y struct{ f struct{ f string } })
+}
+var _ I2 = b.S{}
+`,
+			`package b
+type S struct{}
+func (S) M2(struct{ f struct{ f string } }) {}
+`,
+			`6:12: cannot use b[.]S{} [(]value of type b[.]S[)] as I2 value in variable declaration: b[.]S does not implement I2 [(]wrong type for method M2[)]
+.*have M2[(]struct{f struct{f string} /[*] package b [*]/ }[)]
+.*want M2[(]struct{f struct{f string} /[*] package main [*]/ }[)]`},
+
+		{ //---------------------------------------------------------------
+			`package main
+import "b"
+type I3 interface {
+	M3(z struct{ F struct{ f string } })
+}
+var _ I3 = b.S{}
+`,
+			`package b
+type S struct{}
+func (S) M3(struct{ F struct{ f string } }) {}
+`,
+			`6:12: cannot use b[.]S{} [(]value of type b[.]S[)] as I3 value in variable declaration: b[.]S does not implement I3 [(]wrong type for method M3[)]
+.*have M3[(]struct{F struct{f string /[*] package b [*]/ }}[)]
+.*want M3[(]struct{F struct{f string /[*] package main [*]/ }}[)]`},
+
+		{ //---------------------------------------------------------------
+			`package main
+import "b"
+type I4 interface {
+	M4(_ struct { *string })
+}
+var _ I4 = b.S{}
+`,
+			`package b
+type S struct{}
+func (S) M4(struct { *string }) {}
+`,
+			`6:12: cannot use b[.]S{} [(]value of type b[.]S[)] as I4 value in variable declaration: b[.]S does not implement I4 [(]wrong type for method M4[)]
+.*have M4[(]struct{[*]string /[*] package b [*]/ }[)]
+.*want M4[(]struct{[*]string /[*] package main [*]/ }[)]`},
+
+		{ //---------------------------------------------------------------
+			`package main
+import "b"
+type t struct{ A int }
+type I5 interface {
+	M5(_ struct {b.S;t})
+}
+var _ I5 = b.S{}
+`,
+			`package b
+type S struct{}
+type t struct{ A int }
+func (S) M5(struct {S;t}) {}
+`,
+			`7:12: cannot use b[.]S{} [(]value of type b[.]S[)] as I5 value in variable declaration: b[.]S does not implement I5 [(]wrong type for method M5[)]
+.*have M5[(]struct{b[.]S; b[.]t}[)]
+.*want M5[(]struct{b[.]S; t}[)]`},
+	}
+
+	test := func(main, imported, want string) {
+		re := regexp.MustCompile(want)
+		a := mustTypecheck("b", imported, nil)
+		bast := mustParse("", main)
+		conf := Config{Importer: importHelper{pkg: a}}
+		_, err := conf.Check(bast.PkgName.Value, []*syntax.File{bast}, nil)
+		if err == nil {
+			t.Errorf("Expected failure, but it did not")
+		} else if got := err.Error(); !re.MatchString(got) {
+			t.Errorf("Wanted match for\n%s\n but got \n%s", want, got)
+		} else if testing.Verbose() {
+			t.Logf("Saw expected\n%s", err.Error())
+		}
+	}
+	for _, t := range tests {
+		test(t.main, t.b, t.want)
+	}
+}

src/cmd/compile/internal/types2/lookup.go

@@ -390,15 +390,21 @@ func (check *Checker) missingMethodCause(V, T Type, m, alt *Func) string {
 	if alt != nil {
 		if m.Name() != alt.Name() {
 			return check.sprintf("(missing %s)\n\t\thave %s\n\t\twant %s",
-				mname, check.funcString(alt), check.funcString(m))
+				mname, check.funcString(alt, false), check.funcString(m, false))
 		}
 
 		if Identical(m.typ, alt.typ) {
 			return check.sprintf("(%s has pointer receiver)", mname)
 		}
 
+		altS, mS := check.funcString(alt, false), check.funcString(m, false)
+		if altS == mS {
+			// Would tell the user that Foo isn't a Foo, add package information to disambiguate.  See #54258.
+			altS, mS = check.funcString(alt, true), check.funcString(m, true)
+		}
+
 		return check.sprintf("(wrong type for %s)\n\t\thave %s\n\t\twant %s",
-			mname, check.funcString(alt), check.funcString(m))
+			mname, altS, mS)
 	}
 
 	if isInterfacePtr(V) {
@@ -433,13 +439,14 @@ func (check *Checker) interfacePtrError(T Type) string {
 
 // funcString returns a string of the form name + signature for f.
 // check may be nil.
-func (check *Checker) funcString(f *Func) string {
+func (check *Checker) funcString(f *Func, pkgInfo bool) string {
 	buf := bytes.NewBufferString(f.name)
 	var qf Qualifier
-	if check != nil {
+	if check != nil && !pkgInfo {
 		qf = check.qualifier
 	}
 	w := newTypeWriter(buf, qf)
+	w.pkgInfo = pkgInfo
 	w.paramNames = false
 	w.signature(f.typ.(*Signature))
 	return buf.String()

src/cmd/compile/internal/types2/typestring.go

@@ -58,9 +58,8 @@ func WriteType(buf *bytes.Buffer, typ Type, qf Qualifier) {
 }
 
 // WriteSignature writes the representation of the signature sig to buf,
-// without a leading "func" keyword.
-// The Qualifier controls the printing of
-// package-level objects, and may be nil.
+// without a leading "func" keyword. The Qualifier controls the printing
+// of package-level objects, and may be nil.
 func WriteSignature(buf *bytes.Buffer, sig *Signature, qf Qualifier) {
 	newTypeWriter(buf, qf).signature(sig)
 }
@@ -73,15 +72,16 @@ type typeWriter struct {
 	tparams      *TypeParamList // local type parameters
 	paramNames   bool           // if set, write function parameter names, otherwise, write types only
 	tpSubscripts bool           // if set, write type parameter indices as subscripts
+	pkgInfo      bool           // package-annotate first unexported-type field to avoid confusing type description
 }
 
 func newTypeWriter(buf *bytes.Buffer, qf Qualifier) *typeWriter {
-	return &typeWriter{buf, make(map[Type]bool), qf, nil, nil, true, false}
+	return &typeWriter{buf, make(map[Type]bool), qf, nil, nil, true, false, false}
 }
 
 func newTypeHasher(buf *bytes.Buffer, ctxt *Context) *typeWriter {
 	assert(ctxt != nil)
-	return &typeWriter{buf, make(map[Type]bool), nil, ctxt, nil, false, false}
+	return &typeWriter{buf, make(map[Type]bool), nil, ctxt, nil, false, false, false}
 }
 
 func (w *typeWriter) byte(b byte) {
@@ -148,6 +148,16 @@ func (w *typeWriter) typ(typ Type) {
 			if i > 0 {
 				w.byte(';')
 			}
+
+			// If disambiguating one struct for another, look for the first unexported field.
+			// Do this first in case of nested structs; tag the first-outermost field.
+			pkgAnnotate := false
+			if w.qf == nil && w.pkgInfo && !isExported(f.name) {
+				// note for embedded types, type name is field name, and "string" etc are lower case hence unexported.
+				pkgAnnotate = true
+				w.pkgInfo = false // only tag once
+			}
+
 			// This doesn't do the right thing for embedded type
 			// aliases where we should print the alias name, not
 			// the aliased type (see issue #44410).
@@ -156,6 +166,11 @@ func (w *typeWriter) typ(typ Type) {
 				w.byte(' ')
 			}
 			w.typ(f.typ)
+			if pkgAnnotate {
+				w.string(" /* package ")
+				w.string(f.pkg.Path())
+				w.string(" */ ")
+			}
 			if tag := t.Tag(i); tag != "" {
 				w.byte(' ')
 				// TODO(gri) If tag contains blanks, replacing them with '#'