Permit use of IgnoreFields with unexported fields (#203)
Modify IgnoreFields to permit the specification of unexported fields.
To avoid a bug with reflect.Type.FieldByName, we disallow forwarding
on unexported fields. See https://golang.org/issue/4876 for details.
Fixes #196
diff --git a/cmp/cmpopts/struct_filter.go b/cmp/cmpopts/struct_filter.go
index 97f7079..dae7ced 100644
--- a/cmp/cmpopts/struct_filter.go
+++ b/cmp/cmpopts/struct_filter.go
@@ -160,14 +160,19 @@
// Find the canonical name for this current field name.
// If the field exists in an embedded struct, then it will be expanded.
+ sf, _ := t.FieldByName(name)
if !isExported(name) {
- // Disallow unexported fields:
- // * To discourage people from actually touching unexported fields
- // * FieldByName is buggy (https://golang.org/issue/4876)
- return []string{name}, fmt.Errorf("name must be exported")
+ // Avoid using reflect.Type.FieldByName for unexported fields due to
+ // buggy behavior with regard to embeddeding and unexported fields.
+ // See https://golang.org/issue/4876 for details.
+ sf = reflect.StructField{}
+ for i := 0; i < t.NumField() && sf.Name == ""; i++ {
+ if t.Field(i).Name == name {
+ sf = t.Field(i)
+ }
+ }
}
- sf, ok := t.FieldByName(name)
- if !ok {
+ if sf.Name == "" {
return []string{name}, fmt.Errorf("does not exist")
}
var ss []string
diff --git a/cmp/cmpopts/util_test.go b/cmp/cmpopts/util_test.go
index d0fd888..37704c8 100644
--- a/cmp/cmpopts/util_test.go
+++ b/cmp/cmpopts/util_test.go
@@ -772,6 +772,39 @@
wantEqual: false,
reason: "not equal because highest-level field is not ignored: Foo3",
}, {
+ label: "IgnoreFields",
+ x: ParentStruct{
+ privateStruct: &privateStruct{private: 1},
+ PublicStruct: &PublicStruct{private: 2},
+ private: 3,
+ },
+ y: ParentStruct{
+ privateStruct: &privateStruct{private: 10},
+ PublicStruct: &PublicStruct{private: 20},
+ private: 30,
+ },
+ opts: []cmp.Option{cmp.AllowUnexported(ParentStruct{}, PublicStruct{}, privateStruct{})},
+ wantEqual: false,
+ reason: "not equal because unexported fields mismatch",
+ }, {
+ label: "IgnoreFields",
+ x: ParentStruct{
+ privateStruct: &privateStruct{private: 1},
+ PublicStruct: &PublicStruct{private: 2},
+ private: 3,
+ },
+ y: ParentStruct{
+ privateStruct: &privateStruct{private: 10},
+ PublicStruct: &PublicStruct{private: 20},
+ private: 30,
+ },
+ opts: []cmp.Option{
+ cmp.AllowUnexported(ParentStruct{}, PublicStruct{}, privateStruct{}),
+ IgnoreFields(ParentStruct{}, "PublicStruct.private", "privateStruct.private", "private"),
+ },
+ wantEqual: true,
+ reason: "equal because mismatching unexported fields are ignored",
+ }, {
label: "IgnoreTypes",
x: []interface{}{5, "same"},
y: []interface{}{6, "same"},
@@ -1193,11 +1226,21 @@
wantPanic: "must be a struct",
reason: "the type must be a struct (not pointer to a struct)",
}, {
+ label: "IgnoreFields",
+ fnc: IgnoreFields,
+ args: args(struct{ privateStruct }{}, "privateStruct"),
+ reason: "privateStruct field permitted since it is the default name of the embedded type",
+ }, {
+ label: "IgnoreFields",
+ fnc: IgnoreFields,
+ args: args(struct{ privateStruct }{}, "Public"),
+ reason: "Public field permitted since it is a forwarded field that is exported",
+ }, {
label: "IgnoreFields",
fnc: IgnoreFields,
- args: args(Foo1{}, "unexported"),
- wantPanic: "name must be exported",
- reason: "unexported fields must not be specified",
+ args: args(struct{ privateStruct }{}, "private"),
+ wantPanic: "does not exist",
+ reason: "private field not permitted since it is a forwarded field that is unexported",
}, {
label: "IgnoreTypes",
fnc: IgnoreTypes,
