Skip to content

Commit 4664e24

Browse files
authored
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.
1 parent 79433ac commit 4664e24

File tree

4 files changed

+66
-2
lines changed

4 files changed

+66
-2
lines changed

cmp/compare.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ import (
4040
"github.com/google/go-cmp/cmp/internal/value"
4141
)
4242

43+
// TODO(≥go1.18): Use any instead of interface{}.
44+
4345
// Equal reports whether x and y are equal by recursively applying the
4446
// following rules in the given order to x and y and all of their sub-values:
4547
//

cmp/compare_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ func mustFormatGolden(path string, in []struct{ Name, Data string }) {
104104

105105
var now = time.Date(2009, time.November, 10, 23, 00, 00, 00, time.UTC)
106106

107+
// TODO(≥go1.18): Define a generic function that boxes a value on the heap.
107108
func newInt(n int) *int { return &n }
108109

109110
type Stringer string
@@ -885,6 +886,7 @@ func reporterTests() []test {
885886
FloatsB []MyFloat
886887
FloatsC MyFloats
887888
}
889+
PointerString *string
888890
)
889891

890892
return []test{{
@@ -1351,6 +1353,38 @@ using the AllowUnexported option.`, "\n"),
13511353
"bar": true,
13521354
}`,
13531355
reason: "short multiline JSON should prefer triple-quoted string diff as it is more readable",
1356+
}, {
1357+
label: label + "/PointerToStringOrAny",
1358+
x: func() *string {
1359+
var v string = "hello"
1360+
return &v
1361+
}(),
1362+
y: func() *interface{} {
1363+
var v interface{} = "hello"
1364+
return &v
1365+
}(),
1366+
reason: "mismatched types between any and *any should print differently",
1367+
}, {
1368+
label: label + "/NamedPointer",
1369+
x: func() *string {
1370+
v := "hello"
1371+
return &v
1372+
}(),
1373+
y: func() PointerString {
1374+
v := "hello"
1375+
return &v
1376+
}(),
1377+
reason: "mismatched pointer types should print differently",
1378+
}, {
1379+
label: label + "/MapStringAny",
1380+
x: map[string]interface{}{"key": int(0)},
1381+
y: map[string]interface{}{"key": uint(0)},
1382+
reason: "mismatched underlying value within interface",
1383+
}, {
1384+
label: label + "/StructFieldAny",
1385+
x: struct{ X interface{} }{int(0)},
1386+
y: struct{ X interface{} }{uint(0)},
1387+
reason: "mismatched underlying value within interface",
13541388
}}
13551389
}
13561390

cmp/report_reflect.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,12 @@ func (opts formatOptions) FormatValue(v reflect.Value, parentKind reflect.Kind,
282282
}
283283
defer ptrs.Pop()
284284

285-
skipType = true // Let the underlying value print the type instead
285+
// Skip the name only if this is an unnamed pointer type.
286+
// Otherwise taking the address of a value does not reproduce
287+
// the named pointer type.
288+
if v.Type().Name() == "" {
289+
skipType = true // Let the underlying value print the type instead
290+
}
286291
out = opts.FormatValue(v.Elem(), t.Kind(), ptrs)
287292
out = wrapTrunkReference(ptrRef, opts.PrintAddresses, out)
288293
out = &textWrap{Prefix: "&", Value: out}
@@ -293,7 +298,6 @@ func (opts formatOptions) FormatValue(v reflect.Value, parentKind reflect.Kind,
293298
}
294299
// Interfaces accept different concrete types,
295300
// so configure the underlying value to explicitly print the type.
296-
skipType = true // Print the concrete type instead
297301
return opts.WithTypeMode(emitType).FormatValue(v.Elem(), t.Kind(), ptrs)
298302
default:
299303
panic(fmt.Sprintf("%v kind not handled", v.Kind()))

cmp/testdata/diffs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1134,6 +1134,30 @@
11341134
"""
11351135
)
11361136
>>> TestDiff/Reporter/ShortJSON
1137+
<<< TestDiff/Reporter/PointerToStringOrAny
1138+
any(
1139+
- &string("hello"),
1140+
+ &any(string("hello")),
1141+
)
1142+
>>> TestDiff/Reporter/PointerToStringOrAny
1143+
<<< TestDiff/Reporter/NamedPointer
1144+
any(
1145+
- &string("hello"),
1146+
+ cmp_test.PointerString(&string("hello")),
1147+
)
1148+
>>> TestDiff/Reporter/NamedPointer
1149+
<<< TestDiff/Reporter/MapStringAny
1150+
map[string]any{
1151+
- "key": int(0),
1152+
+ "key": uint(0),
1153+
}
1154+
>>> TestDiff/Reporter/MapStringAny
1155+
<<< TestDiff/Reporter/StructFieldAny
1156+
struct{ X any }{
1157+
- X: int(0),
1158+
+ X: uint(0),
1159+
}
1160+
>>> TestDiff/Reporter/StructFieldAny
11371161
<<< TestDiff/EmbeddedStruct/ParentStructA/Inequal
11381162
teststructs.ParentStructA{
11391163
privateStruct: teststructs.privateStruct{

0 commit comments

Comments
 (0)