From 4664e24d52beca933d6f5378d36e1d405099115d Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Mon, 25 Apr 2022 09:26:47 -0700 Subject: [PATCH] Fix printing of types in reporter output (#293) When printing a pointer, only elide the type for unnamed pointers. Otherwise, we can run into situations where named and unnamed pointers format the same way in indistinguishable ways. When printing an interview, never skip the interface type. Whether we skip printing the type should be determined by the parent containers, and not locally determined. For examples, interface values within a struct, slice, or map will always be elided since they can be inferred. --- cmp/compare.go | 2 ++ cmp/compare_test.go | 34 ++++++++++++++++++++++++++++++++++ cmp/report_reflect.go | 8 ++++++-- cmp/testdata/diffs | 24 ++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 2 deletions(-) diff --git a/cmp/compare.go b/cmp/compare.go index 2a54467..fd2b3a4 100644 --- a/cmp/compare.go +++ b/cmp/compare.go @@ -40,6 +40,8 @@ import ( "github.com/google/go-cmp/cmp/internal/value" ) +// TODO(≥go1.18): Use any instead of interface{}. + // Equal reports whether x and y are equal by recursively applying the // following rules in the given order to x and y and all of their sub-values: // diff --git a/cmp/compare_test.go b/cmp/compare_test.go index 9ad9456..7278485 100644 --- a/cmp/compare_test.go +++ b/cmp/compare_test.go @@ -104,6 +104,7 @@ func mustFormatGolden(path string, in []struct{ Name, Data string }) { var now = time.Date(2009, time.November, 10, 23, 00, 00, 00, time.UTC) +// TODO(≥go1.18): Define a generic function that boxes a value on the heap. func newInt(n int) *int { return &n } type Stringer string @@ -885,6 +886,7 @@ func reporterTests() []test { FloatsB []MyFloat FloatsC MyFloats } + PointerString *string ) return []test{{ @@ -1351,6 +1353,38 @@ using the AllowUnexported option.`, "\n"), "bar": true, }`, reason: "short multiline JSON should prefer triple-quoted string diff as it is more readable", + }, { + label: label + "/PointerToStringOrAny", + x: func() *string { + var v string = "hello" + return &v + }(), + y: func() *interface{} { + var v interface{} = "hello" + return &v + }(), + reason: "mismatched types between any and *any should print differently", + }, { + label: label + "/NamedPointer", + x: func() *string { + v := "hello" + return &v + }(), + y: func() PointerString { + v := "hello" + return &v + }(), + reason: "mismatched pointer types should print differently", + }, { + label: label + "/MapStringAny", + x: map[string]interface{}{"key": int(0)}, + y: map[string]interface{}{"key": uint(0)}, + reason: "mismatched underlying value within interface", + }, { + label: label + "/StructFieldAny", + x: struct{ X interface{} }{int(0)}, + y: struct{ X interface{} }{uint(0)}, + reason: "mismatched underlying value within interface", }} } diff --git a/cmp/report_reflect.go b/cmp/report_reflect.go index 76c04fd..d9d7323 100644 --- a/cmp/report_reflect.go +++ b/cmp/report_reflect.go @@ -282,7 +282,12 @@ func (opts formatOptions) FormatValue(v reflect.Value, parentKind reflect.Kind, } defer ptrs.Pop() - skipType = true // Let the underlying value print the type instead + // Skip the name only if this is an unnamed pointer type. + // Otherwise taking the address of a value does not reproduce + // the named pointer type. + if v.Type().Name() == "" { + skipType = true // Let the underlying value print the type instead + } out = opts.FormatValue(v.Elem(), t.Kind(), ptrs) out = wrapTrunkReference(ptrRef, opts.PrintAddresses, out) out = &textWrap{Prefix: "&", Value: out} @@ -293,7 +298,6 @@ func (opts formatOptions) FormatValue(v reflect.Value, parentKind reflect.Kind, } // Interfaces accept different concrete types, // so configure the underlying value to explicitly print the type. - skipType = true // Print the concrete type instead return opts.WithTypeMode(emitType).FormatValue(v.Elem(), t.Kind(), ptrs) default: panic(fmt.Sprintf("%v kind not handled", v.Kind())) diff --git a/cmp/testdata/diffs b/cmp/testdata/diffs index d207803..23781f7 100644 --- a/cmp/testdata/diffs +++ b/cmp/testdata/diffs @@ -1134,6 +1134,30 @@ """ ) >>> TestDiff/Reporter/ShortJSON +<<< TestDiff/Reporter/PointerToStringOrAny + any( +- &string("hello"), ++ &any(string("hello")), + ) +>>> TestDiff/Reporter/PointerToStringOrAny +<<< TestDiff/Reporter/NamedPointer + any( +- &string("hello"), ++ cmp_test.PointerString(&string("hello")), + ) +>>> TestDiff/Reporter/NamedPointer +<<< TestDiff/Reporter/MapStringAny + map[string]any{ +- "key": int(0), ++ "key": uint(0), + } +>>> TestDiff/Reporter/MapStringAny +<<< TestDiff/Reporter/StructFieldAny + struct{ X any }{ +- X: int(0), ++ X: uint(0), + } +>>> TestDiff/Reporter/StructFieldAny <<< TestDiff/EmbeddedStruct/ParentStructA/Inequal teststructs.ParentStructA{ privateStruct: teststructs.privateStruct{