Skip to content

Commit

Permalink
gopls/internal/golang/completion: fix the isEmptyInterface predicate
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
findleyr committed Apr 29, 2024
1 parent 77f691b commit 5ef4fc9
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 3 deletions.
7 changes: 7 additions & 0 deletions gopls/internal/golang/completion/completion.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
13 changes: 10 additions & 3 deletions gopls/internal/golang/completion/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 5ef4fc9

Please sign in to comment.