Skip to content

Commit

Permalink
Nudge people to use custom comparers rather than Ignore/Allow Unexpor…
Browse files Browse the repository at this point in the history
…ted options (#115)

A common scenario is:
  1. someone uses cmp.Diff on big.Int (transitively)
  2. they get a message that says uses Ignore/Allow Unexported options
  3. they see that AllowUnexported is hard to use correctly
  4. they use IgnoreUnexported
They end up with:
  cmpopts.IgnoreUnexported(big.Int{})
Which definitely doesn't do what's intended.

If we point out that a custom comparer is what they most likely need, then they are more likely to use cmp correctly
  • Loading branch information
LMMilewski authored and dsnet committed Feb 28, 2019
1 parent 2e500c5 commit c812816
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 2 deletions.
4 changes: 4 additions & 0 deletions cmp/cmpopts/ignore.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ func (tf ifaceFilter) filter(p cmp.Path) bool {
// In particular, unexported fields within the struct's exported fields
// of struct types, including anonymous fields, will not be ignored unless the
// type of the field itself is also passed to IgnoreUnexported.
//
// Avoid ignoring unexported fields of a type which you do not control (i.e. a
// type from another repository), as changes to the implementation of such types
// may change how the comparison behaves. Prefer a custom Comparer instead.
func IgnoreUnexported(typs ...interface{}) cmp.Option {
ux := newUnexportedFilter(typs...)
return cmp.FilterPath(ux.filter, cmp.Ignore())
Expand Down
4 changes: 2 additions & 2 deletions cmp/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ func (validator) apply(s *state, vx, vy reflect.Value) {

// Unable to Interface implies unexported field without visibility access.
if !vx.CanInterface() || !vy.CanInterface() {
const help = "consider using AllowUnexported or cmpopts.IgnoreUnexported"
const help = "consider using a custom Comparer; if you control the implementation of type, you can also consider AllowUnexported or cmpopts.IgnoreUnexported"
panic(fmt.Sprintf("cannot handle unexported field: %#v\n%s", s.curPath, help))
}

Expand Down Expand Up @@ -371,7 +371,7 @@ func (cm comparer) String() string {
// defined in an internal package where the semantic meaning of an unexported
// field is in the control of the user.
//
// For some cases, a custom Comparer should be used instead that defines
// In many 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 Down

0 comments on commit c812816

Please sign in to comment.