Skip to content

Commit c812816

Browse files
LMMilewskidsnet
authored andcommitted
Nudge people to use custom comparers rather than Ignore/Allow Unexported 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
1 parent 2e500c5 commit c812816

File tree

2 files changed

+6
-2
lines changed

2 files changed

+6
-2
lines changed

cmp/cmpopts/ignore.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,10 @@ func (tf ifaceFilter) filter(p cmp.Path) bool {
112112
// In particular, unexported fields within the struct's exported fields
113113
// of struct types, including anonymous fields, will not be ignored unless the
114114
// type of the field itself is also passed to IgnoreUnexported.
115+
//
116+
// Avoid ignoring unexported fields of a type which you do not control (i.e. a
117+
// type from another repository), as changes to the implementation of such types
118+
// may change how the comparison behaves. Prefer a custom Comparer instead.
115119
func IgnoreUnexported(typs ...interface{}) cmp.Option {
116120
ux := newUnexportedFilter(typs...)
117121
return cmp.FilterPath(ux.filter, cmp.Ignore())

cmp/options.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ func (validator) apply(s *state, vx, vy reflect.Value) {
225225

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

@@ -371,7 +371,7 @@ func (cm comparer) String() string {
371371
// defined in an internal package where the semantic meaning of an unexported
372372
// field is in the control of the user.
373373
//
374-
// For some cases, a custom Comparer should be used instead that defines
374+
// In many cases, a custom Comparer should be used instead that defines
375375
// equality as a function of the public API of a type rather than the underlying
376376
// unexported implementation.
377377
//

0 commit comments

Comments
 (0)