Skip to content

Commit

Permalink
Support PackagePrefix in AllowUnexported
Browse files Browse the repository at this point in the history
For sufficiently large codebases with many struct types defined,
it is untenable to specify a large list of all types in the repository
that AllowUnexported permits visibility for.

In Go, we observe that code with a shared owner often lives in the same
repository, and all sub-packages within a repository often share a common
package path prefix. Thus, permit expressing visibility allowances in terms
of package path prefixes instead of individual types.

While not explicitly documented, this feature allows the universal comparing
of unexported fields on all types:
	cmp.Equal(x, y, cmp.AllowUnexported(cmp.PackagePrefix("")))

This "feature" is intentionally undocumented (but a natural side-effect of the
expressed behavior of PackagePrefix) since it considered to be bad practice.
  • Loading branch information
dsnet committed Feb 25, 2019
1 parent 77b690b commit 8cc4425
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 27 deletions.
16 changes: 8 additions & 8 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 p := range opt.pkgs {
s.exporters.insertPrefix(p)
}
for t := range opt {
s.exporters[t] = true
for t := range opt.typs {
s.exporters.insertType(t)
}
case reporter:
if s.reporter != nil {
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
91 changes: 72 additions & 19 deletions cmp/options.go
Expand Up @@ -338,17 +338,19 @@ 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 or by passing in a
// PackagePrefix value, which represents all struct types defined in a package
// for which the prefix is match of.
//
// 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.
// Users must understand that comparing unexported fields from external packages
// that are not owned by the user is unsafe since changes in the internal
// implementation of external packages may cause the result of Equal to
// unexpectedly change. However, it may be valid to use this option on types
// owned by the user where the semantic meaning of an unexported field 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 +365,75 @@ 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(templates ...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)
if t.Kind() != reflect.Struct {
panic(fmt.Sprintf("invalid struct type: %T", typ))
var x fieldExporter
for _, v := range templates {
if p, ok := v.(PackagePrefix); ok {
x.insertPrefix(p)
} else {
t := reflect.TypeOf(v)
if t.Kind() != reflect.Struct {
panic(fmt.Sprintf("invalid struct type: %T", v))
}
x.insertType(t)
}
m[t] = true
}
return visibleStructs(m)
return x
}

// PackagePrefix the prefix for a Go package path.
//
// A prefix match is either an identical string match to the package path or
// a string prefix match where the next character is a forward slash.
//
// 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"
type PackagePrefix string

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

type visibleStructs map[reflect.Type]bool
func (x *fieldExporter) insertPrefix(p PackagePrefix) {
if x.pkgs == nil {
x.pkgs = make(map[PackagePrefix]struct{})
}
x.pkgs[p] = 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) mayExport(t reflect.Type, sf reflect.StructField) bool {
for pkgPrefix := range x.pkgs {
if !strings.HasPrefix(sf.PkgPath, string(pkgPrefix)) {
continue
}
if len(sf.PkgPath) == len(pkgPrefix) || sf.PkgPath[len(pkgPrefix)] == '/' {
return true
}
}
if _, ok := x.typs[t]; ok {
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

0 comments on commit 8cc4425

Please sign in to comment.