From ab46b8bd0abd4c4557cc4709ad7ae12d47570603 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Tue, 20 Oct 2020 14:23:13 -0700 Subject: [PATCH] Adjust for reflect.Type.NumMethod change in Go1.16 (#240) In Go1.16, the reflect.Type.NumMethod method will no longer report unexported fields, matching the documented behavior on the method. This means that t.NumMethod() == 0 is no longer a reliable means to detect whether an interface type is the empty interface or not. Fix the code to check whether the empty interface itself implements the target type. --- cmp/cmpopts/ignore.go | 3 ++- cmp/internal/value/iface.go | 14 +++++++++++++ cmp/internal/value/iface_test.go | 35 ++++++++++++++++++++++++++++++++ cmp/options.go | 7 ++++--- 4 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 cmp/internal/value/iface.go create mode 100644 cmp/internal/value/iface_test.go diff --git a/cmp/cmpopts/ignore.go b/cmp/cmpopts/ignore.go index 80c6061..3cdc211 100644 --- a/cmp/cmpopts/ignore.go +++ b/cmp/cmpopts/ignore.go @@ -12,6 +12,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/internal/function" + "github.com/google/go-cmp/cmp/internal/value" ) // IgnoreFields returns an Option that ignores fields of the @@ -82,7 +83,7 @@ func newIfaceFilter(ifaces interface{}) (tf ifaceFilter) { panic("struct cannot have named fields") case fi.Type.Kind() != reflect.Interface: panic("embedded field must be an interface type") - case fi.Type.NumMethod() == 0: + case value.IsEmptyInterface(fi.Type): // This matches everything; why would you ever want this? panic("cannot ignore empty interface") default: diff --git a/cmp/internal/value/iface.go b/cmp/internal/value/iface.go new file mode 100644 index 0000000..d8d07a9 --- /dev/null +++ b/cmp/internal/value/iface.go @@ -0,0 +1,14 @@ +// Copyright 2020, The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE.md file. + +package value + +import "reflect" + +var emptyIfaceType = reflect.TypeOf((*interface{})(nil)).Elem() + +// IsEmptyInterface reports whether t is an interface type with no methods. +func IsEmptyInterface(t reflect.Type) bool { + return t.Kind() == reflect.Interface && emptyIfaceType.Implements(t) +} diff --git a/cmp/internal/value/iface_test.go b/cmp/internal/value/iface_test.go new file mode 100644 index 0000000..817e1d6 --- /dev/null +++ b/cmp/internal/value/iface_test.go @@ -0,0 +1,35 @@ +// Copyright 2020, The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE.md file. + +package value + +import ( + "reflect" + "testing" +) + +func TestIsEmptyInterface(t *testing.T) { + type ( + Empty interface{} + Exported interface{ X() } + Unexported interface{ x() } + ) + tests := []struct { + in reflect.Type + want bool + }{ + {reflect.TypeOf((*interface{})(nil)).Elem(), true}, + {reflect.TypeOf((*Empty)(nil)).Elem(), true}, + {reflect.TypeOf((*Exported)(nil)).Elem(), false}, + {reflect.TypeOf((*Unexported)(nil)).Elem(), false}, + {reflect.TypeOf(5), false}, + {reflect.TypeOf(struct{}{}), false}, + } + for _, tt := range tests { + got := IsEmptyInterface(tt.in) + if got != tt.want { + t.Errorf("IsEmptyInterface(%v) = %v, want %v", tt.in, got, tt.want) + } + } +} diff --git a/cmp/options.go b/cmp/options.go index e57b9eb..3b71f54 100644 --- a/cmp/options.go +++ b/cmp/options.go @@ -11,6 +11,7 @@ import ( "strings" "github.com/google/go-cmp/cmp/internal/function" + "github.com/google/go-cmp/cmp/internal/value" ) // Option configures for specific behavior of Equal and Diff. In particular, @@ -161,7 +162,7 @@ func FilterValues(f interface{}, opt Option) Option { } if opt := normalizeOption(opt); opt != nil { vf := &valuesFilter{fnc: v, opt: opt} - if ti := v.Type().In(0); ti.Kind() != reflect.Interface || ti.NumMethod() > 0 { + if ti := v.Type().In(0); !value.IsEmptyInterface(ti) { vf.typ = ti } return vf @@ -286,7 +287,7 @@ func Transformer(name string, f interface{}) Option { panic(fmt.Sprintf("invalid name: %q", name)) } tr := &transformer{name: name, fnc: reflect.ValueOf(f)} - if ti := v.Type().In(0); ti.Kind() != reflect.Interface || ti.NumMethod() > 0 { + if ti := v.Type().In(0); !value.IsEmptyInterface(ti) { tr.typ = ti } return tr @@ -345,7 +346,7 @@ func Comparer(f interface{}) Option { panic(fmt.Sprintf("invalid comparer function: %T", f)) } cm := &comparer{fnc: v} - if ti := v.Type().In(0); ti.Kind() != reflect.Interface || ti.NumMethod() > 0 { + if ti := v.Type().In(0); !value.IsEmptyInterface(ti) { cm.typ = ti } return cm