internal/typeparams: work around LookupFieldOrMethod inconsistency

This change adds to x/tools a workaround for a bug in go/types
that causes LookupFieldOrMethod and NewTypeSet to be inconsistent
wrt an ill-typed method (*T).f where T itself is a pointer.

The workaround is that, if Lookup fails, we walk the MethodSet.

Updates golang/go#60634
Fixes golang/go#60628

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

internal/typeparams/common.go

@@ -105,6 +105,26 @@ func OriginMethod(fn *types.Func) *types.Func {
 	}
 	orig := NamedTypeOrigin(named)
 	gfn, _, _ := types.LookupFieldOrMethod(orig, true, fn.Pkg(), fn.Name())
+
+	// This is a fix for a gopls crash (#60628) due to a go/types bug (#60634). In:
+	// 	package p
+	//      type T *int
+	//      func (*T) f() {}
+	// LookupFieldOrMethod(T, true, p, f)=nil, but NewMethodSet(*T)={(*T).f}.
+	// Here we make them consistent by force.
+	// (The go/types bug is general, but this workaround is reached only
+	// for generic T thanks to the early return above.)
+	if gfn == nil {
+		mset := types.NewMethodSet(types.NewPointer(orig))
+		for i := 0; i < mset.Len(); i++ {
+			m := mset.At(i)
+			if m.Obj().Id() == fn.Id() {
+				gfn = m.Obj()
+				break
+			}
+		}
+	}
+
 	return gfn.(*types.Func)
 }
 

internal/typeparams/common_test.go

@@ -140,10 +140,12 @@ func TestOriginMethodUses(t *testing.T) {
 			t.Fatal(err)
 		}
 
+		// Look up func T.m.
 		T := pkg.Scope().Lookup("T").Type()
 		obj, _, _ := types.LookupFieldOrMethod(T, true, pkg, "m")
 		m := obj.(*types.Func)
 
+		// Assert that the origin of each t.m() call is p.T.m.
 		ast.Inspect(f, func(n ast.Node) bool {
 			if call, ok := n.(*ast.CallExpr); ok {
 				sel := call.Fun.(*ast.SelectorExpr)
@@ -158,6 +160,54 @@ func TestOriginMethodUses(t *testing.T) {
 	}
 }
 
+// Issue #60628 was a crash in gopls caused by inconsistency (#60634) between
+// LookupFieldOrMethod and NewFileSet for methods with an illegal
+// *T receiver type, where T itself is a pointer.
+// This is a regression test for the workaround in OriginMethod.
+func TestOriginMethod60628(t *testing.T) {
+	const src = `package p; type T[P any] *int; func (r *T[A]) f() {}`
+	fset := token.NewFileSet()
+	f, err := parser.ParseFile(fset, "p.go", src, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Expect type error: "invalid receiver type T[A] (pointer or interface type)".
+	info := types.Info{
+		Uses: make(map[*ast.Ident]types.Object),
+	}
+	var conf types.Config
+	pkg, _ := conf.Check("p", fset, []*ast.File{f}, &info) // error expected
+	if pkg == nil {
+		t.Fatal("no package")
+	}
+
+	// Look up methodset of *T.
+	T := pkg.Scope().Lookup("T").Type()
+	mset := types.NewMethodSet(types.NewPointer(T))
+	if mset.Len() == 0 {
+		t.Errorf("NewMethodSet(*T) is empty")
+	}
+	for i := 0; i < mset.Len(); i++ {
+		sel := mset.At(i)
+		m := sel.Obj().(*types.Func)
+
+		// TODO(adonovan): check the consistency property required to fix #60634.
+		if false {
+			m2, _, _ := types.LookupFieldOrMethod(T, true, m.Pkg(), m.Name())
+			if m2 != m {
+				t.Errorf("LookupFieldOrMethod(%v, indirect=true, %v) = %v, want %v",
+					T, m, m2, m)
+			}
+		}
+
+		// Check the workaround.
+		if OriginMethod(m) == nil {
+			t.Errorf("OriginMethod(%v) = nil", m)
+		}
+	}
+}
+
 func TestGenericAssignableTo(t *testing.T) {
 	testenv.NeedsGo1Point(t, 18)