gopls/internal/golang/completion: fix the isEmptyInterface predicate

The isEmptyInterface predicate had a TODO noting that it should probably
be considering the underlying type. After looking at usage, I agree that
this was probably simple any oversight, but didn't matter because most
uses of the empty interface were uses of any or interface{}, both of
which were an interface.

But with CL 580355, as the 'any' type is becoming a types.Alias, and
this logic is caused inference scoring to change and gopls tests to
fail. Fix isEmptyInterface to check underlying, and add a bit of
commentary for the future.

For golang/go#66921

Change-Id: I7ba3db1b04d83dda0372d9c39b943965f4d8c955
Reviewed-on: https://go-review.googlesource.com/c/tools/+/582335
Reviewed-by: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: findleyr

gopls/internal/golang/completion/completion.go

@@ -2936,6 +2936,13 @@ func (ci *candidateInference) candTypeMatches(cand *candidate) bool {
 
 		for _, expType := range expTypes {
 			if isEmptyInterface(expType) {
+				// If any type matches the expected type, fall back to other
+				// considerations below.
+				//
+				// TODO(rfindley): can this be expressed via scoring, rather than a boolean?
+				// Why is it the case that we break ties for the empty interface, but
+				// not for other expected types that may be satisfied by a lot of
+				// types, such as fmt.Stringer?
 				continue
 			}
 

gopls/internal/golang/completion/util.go

@@ -142,10 +142,17 @@ func isPkgName(obj types.Object) bool  { return is[*types.PkgName](obj) }
 // TODO(adonovan): shouldn't this use CoreType(T)?
 func isPointer(T types.Type) bool { return is[*types.Pointer](aliases.Unalias(T)) }
 
+// isEmptyInterface whether T is a (possibly Named or Alias) empty interface
+// type, such that every type is assignable to T.
+//
+// isEmptyInterface returns false for type parameters, since they have
+// different assignability rules.
 func isEmptyInterface(T types.Type) bool {
-	// TODO(adonovan): shouldn't this use Underlying?
-	intf, _ := T.(*types.Interface)
-	return intf != nil && intf.NumMethods() == 0 && intf.IsMethodSet()
+	if _, ok := T.(*types.TypeParam); ok {
+		return false
+	}
+	intf, _ := T.Underlying().(*types.Interface)
+	return intf != nil && intf.Empty()
 }
 
 func isUntyped(T types.Type) bool {