From 5ef4fc9014e76bcc56324b24218c0216a7146306 Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Mon, 29 Apr 2024 13:25:56 +0000 Subject: [PATCH] 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 LUCI-TryBot-Result: Go LUCI --- gopls/internal/golang/completion/completion.go | 7 +++++++ gopls/internal/golang/completion/util.go | 13 ++++++++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/gopls/internal/golang/completion/completion.go b/gopls/internal/golang/completion/completion.go index 36d382dfc78..f02f7e26f1d 100644 --- a/gopls/internal/golang/completion/completion.go +++ b/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 } diff --git a/gopls/internal/golang/completion/util.go b/gopls/internal/golang/completion/util.go index 1261d417080..ad4ee5e09fc 100644 --- a/gopls/internal/golang/completion/util.go +++ b/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 {