gopls/internal/golang: Implementations: support generics

This CL adds support for generic types to the local (go/types-based)
implementation of Implementations. Instead of using
types.AssignableTo(C, I), which doesn't consider generics,
we check that C has all the methods of I and that the
signatures of each corresponding pair match when type
parameters are allowed to stand for any type at all.

A more precise implementation would use true unification,
ensuring that only consistent substitutions are considered
to match, but this is good enough for now. Perhaps when
go/types adds a Unify API (#63982) this will be easier.

See CL 634197 for the corresponding change to the global
(fingerprint-based) implementation.

Fixes golang/go#59224
Updates golang/go#63982

Change-Id: I6959f855b69436d52cafbe4de07bcf24b1e0f280
Reviewed-on: https://go-review.googlesource.com/c/tools/+/634596
Reviewed-by: Robert Findley <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: adonovan

gopls/internal/golang/completion/completion.go

@@ -266,9 +266,10 @@ type completer struct {
 	// matcher matches the candidates against the surrounding prefix.
 	matcher matcher
 
-	// methodSetCache caches the types.NewMethodSet call, which is relatively
+	// methodSetCache caches the [types.NewMethodSet] call, which is relatively
 	// expensive and can be called many times for the same type while searching
 	// for deep completions.
+	// TODO(adonovan): use [typeutil.MethodSetCache], which exists for this purpose.
 	methodSetCache map[methodSetKey]*types.MethodSet
 
 	// tooNewSymbolsCache is a cache of

gopls/internal/golang/implementation.go

@@ -17,6 +17,7 @@ import (
 	"sync"
 
 	"golang.org/x/sync/errgroup"
+	"golang.org/x/tools/go/types/typeutil"
 	"golang.org/x/tools/gopls/internal/cache"
 	"golang.org/x/tools/gopls/internal/cache/metadata"
 	"golang.org/x/tools/gopls/internal/cache/methodsets"
@@ -345,6 +346,8 @@ func implementsObj(ctx context.Context, snapshot *cache.Snapshot, uri protocol.D
 func localImplementations(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.Package, queryType types.Type, method *types.Func) ([]protocol.Location, error) {
 	queryType = methodsets.EnsurePointer(queryType)
 
+	var msets typeutil.MethodSetCache
+
 	// Scan through all type declarations in the syntax.
 	var locs []protocol.Location
 	var methodLocs []methodsets.Location
@@ -366,16 +369,16 @@ func localImplementations(ctx context.Context, snapshot *cache.Snapshot, pkg *ca
 			// The historical behavior enshrined by this
 			// function rejects cases where both are
 			// (nontrivial) interface types?
-			// That seems like useful information.
+			// That seems like useful information; see #68641.
 			// TODO(adonovan): UX: report I/I pairs too?
 			// The same question appears in the global algorithm (methodsets).
-			if !concreteImplementsIntf(candidateType, queryType) {
+			if !concreteImplementsIntf(&msets, candidateType, queryType) {
 				return true // not assignable
 			}
 
 			// Ignore types with empty method sets.
 			// (No point reporting that every type satisfies 'any'.)
-			mset := types.NewMethodSet(candidateType)
+			mset := msets.MethodSet(candidateType)
 			if mset.Len() == 0 {
 				return true
 			}
@@ -449,28 +452,173 @@ func errorLocation(ctx context.Context, snapshot *cache.Snapshot) (protocol.Loca
 	return protocol.Location{}, fmt.Errorf("built-in error type not found")
 }
 
-// concreteImplementsIntf returns true if a is an interface type implemented by
-// concrete type b, or vice versa.
-func concreteImplementsIntf(a, b types.Type) bool {
-	aIsIntf, bIsIntf := types.IsInterface(a), types.IsInterface(b)
+// concreteImplementsIntf reports whether x is an interface type
+// implemented by concrete type y, or vice versa.
+//
+// If one or both types are generic, the result indicates whether the
+// interface may be implemented under some instantiation.
+func concreteImplementsIntf(msets *typeutil.MethodSetCache, x, y types.Type) bool {
+	xiface := types.IsInterface(x)
+	yiface := types.IsInterface(y)
 
 	// Make sure exactly one is an interface type.
-	if aIsIntf == bIsIntf {
+	// TODO(adonovan): rescind this policy choice and report
+	// I/I relationships. See CL 619719 + issue #68641.
+	if xiface == yiface {
 		return false
 	}
 
-	// Rearrange if needed so "a" is the concrete type.
-	if aIsIntf {
-		a, b = b, a
+	// Rearrange if needed so x is the concrete type.
+	if xiface {
+		x, y = y, x
 	}
+	// Inv: y is an interface type.
 
-	// TODO(adonovan): this should really use GenericAssignableTo
-	// to report (e.g.) "ArrayList[T] implements List[T]", but
-	// GenericAssignableTo doesn't work correctly on pointers to
-	// generic named types. Thus the legacy implementation and the
-	// "local" part of implementations fail to report generics.
-	// The global algorithm based on subsets does the right thing.
-	return types.AssignableTo(a, b)
+	// For each interface method of y, check that x has it too.
+	// It is not necessary to compute x's complete method set.
+	//
+	// If y is a constraint interface (!y.IsMethodSet()), we
+	// ignore non-interface terms, leading to occasional spurious
+	// matches. We could in future filter based on them, but it
+	// would lead to divergence with the global (fingerprint-based)
+	// algorithm, which operates only on methodsets.
+	ymset := msets.MethodSet(y)
+	for i := range ymset.Len() {
+		ym := ymset.At(i).Obj().(*types.Func)
+
+		xobj, _, _ := types.LookupFieldOrMethod(x, false, ym.Pkg(), ym.Name())
+		xm, ok := xobj.(*types.Func)
+		if !ok {
+			return false // x lacks a method of y
+		}
+		if !unify(xm.Signature(), ym.Signature()) {
+			return false // signatures do not match
+		}
+	}
+	return true // all methods found
+}
+
+// unify reports whether the types of x and y match, allowing free
+// type parameters to stand for anything at all, without regard to
+// consistency of substitutions.
+//
+// TODO(adonovan): implement proper unification (#63982), finding the
+// most general unifier across all the interface methods.
+//
+// See also: unify in cache/methodsets/fingerprint, which uses a
+// similar ersatz unification approach on type fingerprints, for
+// the global index.
+func unify(x, y types.Type) bool {
+	x = types.Unalias(x)
+	y = types.Unalias(y)
+
+	// For now, allow a type parameter to match anything,
+	// without regard to consistency of substitutions.
+	if is[*types.TypeParam](x) || is[*types.TypeParam](y) {
+		return true
+	}
+
+	if reflect.TypeOf(x) != reflect.TypeOf(y) {
+		return false // mismatched types
+	}
+
+	switch x := x.(type) {
+	case *types.Array:
+		y := y.(*types.Array)
+		return x.Len() == y.Len() &&
+			unify(x.Elem(), y.Elem())
+
+	case *types.Basic:
+		y := y.(*types.Basic)
+		return x.Kind() == y.Kind()
+
+	case *types.Chan:
+		y := y.(*types.Chan)
+		return x.Dir() == y.Dir() &&
+			unify(x.Elem(), y.Elem())
+
+	case *types.Interface:
+		y := y.(*types.Interface)
+		// TODO(adonovan): fix: for correctness, we must check
+		// that both interfaces have the same set of methods
+		// modulo type parameters, while avoiding the risk of
+		// unbounded interface recursion.
+		//
+		// Since non-empty interface literals are vanishingly
+		// rare in methods signatures, we ignore this for now.
+		// If more precision is needed we could compare method
+		// names and arities, still without full recursion.
+		return x.NumMethods() == y.NumMethods()
+
+	case *types.Map:
+		y := y.(*types.Map)
+		return unify(x.Key(), y.Key()) &&
+			unify(x.Elem(), y.Elem())
+
+	case *types.Named:
+		y := y.(*types.Named)
+		if x.Origin() != y.Origin() {
+			return false // different named types
+		}
+		xtargs := x.TypeArgs()
+		ytargs := y.TypeArgs()
+		if xtargs.Len() != ytargs.Len() {
+			return false // arity error (ill-typed)
+		}
+		for i := range xtargs.Len() {
+			if !unify(xtargs.At(i), ytargs.At(i)) {
+				return false // mismatched type args
+			}
+		}
+		return true
+
+	case *types.Pointer:
+		y := y.(*types.Pointer)
+		return unify(x.Elem(), y.Elem())
+
+	case *types.Signature:
+		y := y.(*types.Signature)
+		return x.Variadic() == y.Variadic() &&
+			unify(x.Params(), y.Params()) &&
+			unify(x.Results(), y.Results())
+
+	case *types.Slice:
+		y := y.(*types.Slice)
+		return unify(x.Elem(), y.Elem())
+
+	case *types.Struct:
+		y := y.(*types.Struct)
+		if x.NumFields() != y.NumFields() {
+			return false
+		}
+		for i := range x.NumFields() {
+			xf := x.Field(i)
+			yf := y.Field(i)
+			if xf.Embedded() != yf.Embedded() ||
+				xf.Name() != yf.Name() ||
+				x.Tag(i) != y.Tag(i) ||
+				!xf.Exported() && xf.Pkg() != yf.Pkg() ||
+				!unify(xf.Type(), yf.Type()) {
+				return false
+			}
+		}
+		return true
+
+	case *types.Tuple:
+		y := y.(*types.Tuple)
+		if x.Len() != y.Len() {
+			return false
+		}
+		for i := range x.Len() {
+			if !unify(x.At(i).Type(), y.At(i).Type()) {
+				return false
+			}
+		}
+		return true
+
+	default: // incl. *Union, *TypeParam
+		panic(fmt.Sprintf("unexpected Type %#v", x))
+	}
 }
 
 var (

gopls/internal/golang/references.go

@@ -25,6 +25,7 @@ import (
 
 	"golang.org/x/sync/errgroup"
 	"golang.org/x/tools/go/types/objectpath"
+	"golang.org/x/tools/go/types/typeutil"
 	"golang.org/x/tools/gopls/internal/cache"
 	"golang.org/x/tools/gopls/internal/cache/metadata"
 	"golang.org/x/tools/gopls/internal/cache/methodsets"
@@ -579,6 +580,8 @@ func localReferences(pkg *cache.Package, targets map[types.Object]bool, correspo
 		}
 	}
 
+	var msets typeutil.MethodSetCache
+
 	// matches reports whether obj either is or corresponds to a target.
 	// (Correspondence is defined as usual for interface methods.)
 	matches := func(obj types.Object) bool {
@@ -588,7 +591,7 @@ func localReferences(pkg *cache.Package, targets map[types.Object]bool, correspo
 		if methodRecvs != nil && obj.Name() == methodName {
 			if orecv := effectiveReceiver(obj); orecv != nil {
 				for _, mrecv := range methodRecvs {
-					if concreteImplementsIntf(orecv, mrecv) {
+					if concreteImplementsIntf(&msets, orecv, mrecv) {
 						return true
 					}
 				}

gopls/internal/test/marker/testdata/implementation/generics-basicalias.txt

@@ -0,0 +1,26 @@
+Test of special case of 'implementation' query: aliases of basic types
+(rune vs int32) in the "tricky" (=generic) algorithm for unifying
+method signatures.
+
+We test both the local (intra-package) and global (cross-package)
+algorithms.
+
+-- go.mod --
+module example.com
+go 1.18
+
+-- a/a.go --
+package a
+
+type C[T any] struct{}
+func (C[T]) F(rune, T) {} //@ loc(aCF, "F"), implementation("F", aIF, bIF)
+
+type I[T any] interface{ F(int32, T) } //@ loc(aIF, "F"), implementation("F", aCF, bCF)
+
+-- b/b.go --
+package b
+
+type C[T any] struct{}
+func (C[T]) F(rune, T) {} //@ loc(bCF, "F"), implementation("F", aIF, bIF)
+
+type I[T any] interface{ F(int32, T) } //@ loc(bIF, "F"), implementation("F", aCF, bCF)

gopls/internal/test/marker/testdata/implementation/generics.txt

@@ -7,25 +7,25 @@ go 1.18
 -- implementation/implementation.go --
 package implementation
 
-type GenIface[T any] interface { //@loc(GenIface, "GenIface"),implementation("GenIface", GC)
-	F(int, string, T) //@loc(GenIfaceF, "F"),implementation("F", GCF)
+type GenIface[T any] interface { //@loc(GenIface, "GenIface"),implementation("GenIface", GC, GenConc, GenConcString)
+	F(int, string, T) //@loc(GenIfaceF, "F"),implementation("F", GCF, GenConcF)
 }
 
-type GenConc[U any] int //@loc(GenConc, "GenConc"),implementation("GenConc", GI, GIString)
+type GenConc[U any] int //@loc(GenConc, "GenConc"),implementation("GenConc", GI, GIString, GenIface)
 
-func (GenConc[V]) F(int, string, V) {} //@loc(GenConcF, "F"),implementation("F", GIF)
+func (GenConc[V]) F(int, string, V) {} //@loc(GenConcF, "F"),implementation("F", GIF, GenIfaceF)
 
-type GenConcString struct{ GenConc[string] } //@loc(GenConcString, "GenConcString"),implementation(GenConcString, GIString, GI)
+type GenConcString struct{ GenConc[string] } //@loc(GenConcString, "GenConcString"),implementation(GenConcString, GIString, GI, GenIface)
 
 -- other/other.go --
 package other
 
-type GI[T any] interface { //@loc(GI, "GI"),implementation("GI", GenConc, GenConcString)
-	F(int, string, T) //@loc(GIF, "F"),implementation("F", GenConcF)
+type GI[T any] interface { //@loc(GI, "GI"),implementation("GI", GenConc, GenConcString, GC)
+	F(int, string, T) //@loc(GIF, "F"),implementation("F", GenConcF, GCF)
 }
 
-type GIString GI[string] //@loc(GIString, "GIString"),implementation("GIString", GenConcString, GenConc)
+type GIString GI[string] //@loc(GIString, "GIString"),implementation("GIString", GenConcString, GenConc, GC)
 
-type GC[U any] int //@loc(GC, "GC"),implementation("GC", GenIface)
+type GC[U any] int //@loc(GC, "GC"),implementation("GC", GenIface, GI, GIString)
 
-func (GC[V]) F(int, string, V) {} //@loc(GCF, "F"),implementation("F", GenIfaceF)
+func (GC[V]) F(int, string, V) {} //@loc(GCF, "F"),implementation("F", GenIfaceF, GIF)