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 <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/gopls/internal/golang/completion/completion.go b/gopls/internal/golang/completion/completion.go
index 36d382d..f02f7e2 100644
--- a/gopls/internal/golang/completion/completion.go
+++ b/gopls/internal/golang/completion/completion.go
@@ -2936,6 +2936,13 @@
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
}
diff --git a/gopls/internal/golang/completion/util.go b/gopls/internal/golang/completion/util.go
index 1261d41..ad4ee5e 100644
--- a/gopls/internal/golang/completion/util.go
+++ b/gopls/internal/golang/completion/util.go
@@ -142,10 +142,17 @@
// 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 {
