Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support AllowUnexportedWithinModule #116

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 10 additions & 10 deletions cmp/compare.go
Expand Up @@ -116,8 +116,8 @@ type state struct {
dynChecker dynChecker

// These fields, once set by processOption, will not change.
exporters map[reflect.Type]bool // Set of structs with unexported field visibility
opts Options // List of all fundamental and filter options
exporters fieldExporter // Set of structs with unexported field visibility
opts Options // List of all fundamental and filter options
}

func newState(opts []Option) *state {
Expand All @@ -143,12 +143,12 @@ func (s *state) processOption(opt Option) {
panic(fmt.Sprintf("cannot use an unfiltered option: %v", opt))
}
s.opts = append(s.opts, opt)
case visibleStructs:
if s.exporters == nil {
s.exporters = make(map[reflect.Type]bool)
case fieldExporter:
for t := range opt.typs {
s.exporters.insertType(t)
}
for t := range opt {
s.exporters[t] = true
for p := range opt.pkgs {
s.exporters.insertPrefix(p)
}
case reporter:
if s.reporter != nil {
Expand Down Expand Up @@ -379,8 +379,8 @@ func detectRaces(c chan<- reflect.Value, f reflect.Value, vs ...reflect.Value) {
// Otherwise, it returns the input value as is.
func sanitizeValue(v reflect.Value, t reflect.Type) reflect.Value {
// TODO(dsnet): Workaround for reflect bug (https://golang.org/issue/22143).
// The upstream fix landed in Go1.10, so we can remove this when drop support
// for Go1.9 and below.
// The upstream fix landed in Go1.10, so we can remove this when dropping
// support for Go1.9 and below.
if v.Kind() == reflect.Interface && v.IsNil() && v.Type() != t {
return reflect.New(t).Elem()
}
Expand Down Expand Up @@ -413,7 +413,7 @@ func (s *state) compareStruct(vx, vy reflect.Value, t reflect.Type) {
vax = makeAddressable(vx)
vay = makeAddressable(vy)
}
step.force = s.exporters[t]
step.force = s.exporters.mayExport(t, t.Field(i))
step.pvx = vax
step.pvy = vay
step.field = t.Field(i)
Expand Down
26 changes: 26 additions & 0 deletions cmp/compare_test.go
Expand Up @@ -12,6 +12,7 @@ import (
"io"
"math"
"math/rand"
"path"
"reflect"
"regexp"
"sort"
Expand Down Expand Up @@ -1220,6 +1221,31 @@ func embeddedTests() []test {
opts: []cmp.Option{
cmp.AllowUnexported(ts.ParentStructJ{}, ts.PublicStruct{}, privateStruct),
},
}, {
label: label + "ParentStructJ",
x: createStructJ(0),
y: createStructJ(0),
opts: []cmp.Option{
cmp.AllowUnexportedWithinModule(""),
},
}, {
label: label + "ParentStructJ",
x: createStructJ(0),
y: createStructJ(0),
opts: []cmp.Option{
cmp.AllowUnexportedWithinModule("wrong/package/prefix"),
},
wantPanic: "cannot handle unexported field",
}, {
label: label + "ParentStructJ",
x: createStructJ(0),
y: createStructJ(0),
opts: []cmp.Option{
cmp.AllowUnexportedWithinModule(func() string {
type X struct{}
return path.Dir(reflect.TypeOf(X{}).PkgPath())
}()),
},
}, {
label: label + "ParentStructJ",
x: createStructJ(0),
Expand Down
100 changes: 82 additions & 18 deletions cmp/options.go
Expand Up @@ -338,17 +338,15 @@ func (cm comparer) String() string {
}

// AllowUnexported returns an Option that forcibly allows operations on
// unexported fields in certain structs, which are specified by passing in a
// value of each struct type.
// unexported fields in certain structs. Struct types with permitted visibility
// are specified by passing in a value of the struct type.
//
// Users of this option must understand that comparing on unexported fields
// from external packages is not safe since changes in the internal
// implementation of some external package may cause the result of Equal
// to unexpectedly change. However, it may be valid to use this option on types
// defined in an internal package where the semantic meaning of an unexported
// field is in the control of the user.
// Comparing unexported fields from packages that are not owned by the user
// is unsafe since changes in the internal implementation may cause the result
// of Equal to unexpectedly change. This option should only be used on types
// where the semantic meaning of unexported fields is in full control of the user.
//
// For some cases, a custom Comparer should be used instead that defines
// For most cases, a custom Comparer should be used instead that defines
// equality as a function of the public API of a type rather than the underlying
// unexported implementation.
//
Expand All @@ -363,24 +361,90 @@ func (cm comparer) String() string {
//
// In other cases, the cmpopts.IgnoreUnexported option can be used to ignore
// all unexported fields on specified struct types.
func AllowUnexported(types ...interface{}) Option {
func AllowUnexported(typs ...interface{}) Option {
if !supportAllowUnexported {
panic("AllowUnexported is not supported on purego builds, Google App Engine Standard, or GopherJS")
}
m := make(map[reflect.Type]bool)
for _, typ := range types {
t := reflect.TypeOf(typ)
var x fieldExporter
for _, v := range typs {
t := reflect.TypeOf(v)
if t.Kind() != reflect.Struct {
panic(fmt.Sprintf("invalid struct type: %T", typ))
panic(fmt.Sprintf("invalid struct type: %T", v))
}
m[t] = true
x.insertType(t)
}
return visibleStructs(m)
return x
}

type visibleStructs map[reflect.Type]bool
// AllowUnexportedWithinModule returns an Option that forcibly allows
// operations on unexported fields in certain structs.
// See AllowUnexported for proper guidance on comparing unexported fields.
//
// Unexported visibility is permitted for any struct type declared within a
// module where the pkgPrefix is a path prefix match of the full package path.
// A path prefix match is defined as a string prefix match where the next
// character is either the first character, a forward slash,
// or the end of the string.
//
// For example, the package path "example.com/foo/bar" is matched by:
// • "example.com/foo/bar"
// • "example.com/foo"
// • "example.com"
// and is not matched by:
// • "example.com/foo/ba"
// • "example.com/fizz"
// • "example.org"
func AllowUnexportedWithinModule(pkgPrefix string) Option {
if !supportAllowUnexported {
panic("AllowUnexported is not supported on purego builds, Google App Engine Standard, or GopherJS")
}
var x fieldExporter
x.insertPrefix(pkgPrefix)
return x
}

type fieldExporter struct {
typs map[reflect.Type]struct{}
pkgs map[string]struct{}
}

func (x *fieldExporter) insertType(t reflect.Type) {
if x.typs == nil {
x.typs = make(map[reflect.Type]struct{})
}
x.typs[t] = struct{}{}
}

func (x *fieldExporter) insertPrefix(p string) {
if x.pkgs == nil {
x.pkgs = make(map[string]struct{})
}
x.pkgs[p] = struct{}{}
}

func (x fieldExporter) mayExport(t reflect.Type, sf reflect.StructField) bool {
// TODO(dsnet): Workaround for reflect bug (https://golang.org/issue/21122).
// The upstream fix landed in Go1.10, so we can remove this when dropping
// support for Go1.9 and below.
if len(x.pkgs) > 0 && sf.PkgPath == "" {
return true // Liberally allow exporting since we lack path information
}

if _, ok := x.typs[t]; ok {
return true
}
for pkgPrefix := range x.pkgs {
if !strings.HasPrefix(sf.PkgPath, string(pkgPrefix)) {
continue
}
if len(sf.PkgPath) == len(pkgPrefix) || sf.PkgPath[len(pkgPrefix)] == '/' || len(pkgPrefix) == 0 {
return true
}
}
return false
}

func (visibleStructs) filter(_ *state, _, _ reflect.Value, _ reflect.Type) applicableOption {
func (fieldExporter) filter(_ *state, _, _ reflect.Value, _ reflect.Type) applicableOption {
panic("not implemented")
}

Expand Down
4 changes: 4 additions & 0 deletions cmp/options_test.go
Expand Up @@ -44,6 +44,10 @@ func TestOptionPanic(t *testing.T) {
fnc: AllowUnexported,
args: []interface{}{ts.StructA{}, &ts.StructB{}, ts.StructA{}},
wantPanic: "invalid struct type",
}, {
label: "AllowUnexportedWithinModule",
fnc: AllowUnexportedWithinModule,
args: []interface{}{""},
}, {
label: "Comparer",
fnc: Comparer,
Expand Down