Skip to content

Commit

Permalink
Report with primitive types in diffs
Browse files Browse the repository at this point in the history
When the formatted strings are identical, the reporter tries to fallback
on more exact formatting. This fallback should include the types for
primitive types since the difference can simply be between
an int and an uint.

Fixes #64
  • Loading branch information
dsnet committed Jan 3, 2018
1 parent ab3beb0 commit bef75ba
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 19 deletions.
11 changes: 11 additions & 0 deletions cmp/compare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,17 @@ root:
return x == nil && y == nil
}, cmp.Ignore()),
},
}, {
label: label,
x: []interface{}{map[string]interface{}{"avg": 0.278, "hr": 65, "name": "Mark McGwire"}, map[string]interface{}{"avg": 0.288, "hr": 63, "name": "Sammy Sosa"}},
y: []interface{}{map[string]interface{}{"avg": 0.278, "hr": 65.0, "name": "Mark McGwire"}, map[string]interface{}{"avg": 0.288, "hr": 63.0, "name": "Sammy Sosa"}},
wantDiff: `
root[0]["hr"]:
-: int(65)
+: float64(65)
root[1]["hr"]:
-: int(63)
+: float64(63)`,
}}
}

Expand Down
30 changes: 17 additions & 13 deletions cmp/internal/value/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,24 +22,28 @@ var stringerIface = reflect.TypeOf((*fmt.Stringer)(nil)).Elem()
// * Avoids printing struct fields that are zero
// * Prints a nil-slice as being nil, not empty
// * Prints map entries in deterministic order
func Format(v reflect.Value, useStringer bool) string {
return formatAny(v, formatConfig{useStringer, true, true, true}, nil)
func Format(v reflect.Value, conf FormatConfig) string {
conf.printType = true
conf.followPointers = true
conf.realPointers = true
return formatAny(v, conf, nil)
}

type formatConfig struct {
useStringer bool // Should the String method be used if available?
printType bool // Should we print the type before the value?
followPointers bool // Should we recursively follow pointers?
realPointers bool // Should we print the real address of pointers?
type FormatConfig struct {
UseStringer bool // Should the String method be used if available?
printType bool // Should we print the type before the value?
PrintPrimitiveType bool // Should we print the type of primitives?
followPointers bool // Should we recursively follow pointers?
realPointers bool // Should we print the real address of pointers?
}

func formatAny(v reflect.Value, conf formatConfig, visited map[uintptr]bool) string {
func formatAny(v reflect.Value, conf FormatConfig, visited map[uintptr]bool) string {
// TODO: Should this be a multi-line printout in certain situations?

if !v.IsValid() {
return "<non-existent>"
}
if conf.useStringer && v.Type().Implements(stringerIface) && v.CanInterface() {
if conf.UseStringer && v.Type().Implements(stringerIface) && v.CanInterface() {
if (v.Kind() == reflect.Ptr || v.Kind() == reflect.Interface) && v.IsNil() {
return "<nil>"
}
Expand Down Expand Up @@ -150,7 +154,7 @@ func formatAny(v reflect.Value, conf formatConfig, visited map[uintptr]bool) str
continue // Elide zero value fields
}
name := v.Type().Field(i).Name
subConf.useStringer = conf.useStringer
subConf.UseStringer = conf.UseStringer
s := formatAny(vv, subConf, visited)
ss = append(ss, fmt.Sprintf("%s: %s", name, s))
}
Expand Down Expand Up @@ -183,14 +187,14 @@ func formatString(s string) string {
return qs
}

func formatPrimitive(t reflect.Type, v interface{}, conf formatConfig) string {
if conf.printType && t.PkgPath() != "" {
func formatPrimitive(t reflect.Type, v interface{}, conf FormatConfig) string {
if conf.printType && (conf.PrintPrimitiveType || t.PkgPath() != "") {
return fmt.Sprintf("%v(%v)", t, v)
}
return fmt.Sprintf("%v", v)
}

func formatPointer(v reflect.Value, conf formatConfig) string {
func formatPointer(v reflect.Value, conf FormatConfig) string {
p := v.Pointer()
if !conf.realPointers {
p = 0 // For deterministic printing purposes
Expand Down
2 changes: 1 addition & 1 deletion cmp/internal/value/format_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func TestFormat(t *testing.T) {
// ensure the format logic does not depend on read-write access
// to the reflect.Value.
v := reflect.ValueOf(struct{ x interface{} }{tt.in}).Field(0)
got := formatAny(v, formatConfig{useStringer: true, printType: true, followPointers: true}, nil)
got := formatAny(v, FormatConfig{UseStringer: true, printType: true, followPointers: true}, nil)
if got != tt.want {
t.Errorf("test %d, Format():\ngot %q\nwant %q", i, got, tt.want)
}
Expand Down
10 changes: 5 additions & 5 deletions cmp/reporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ func (r *defaultReporter) Report(x, y reflect.Value, eq bool, p Path) {
const maxLines = 256
r.ndiffs++
if r.nbytes < maxBytes && r.nlines < maxLines {
sx := value.Format(x, true)
sy := value.Format(y, true)
sx := value.Format(x, value.FormatConfig{UseStringer: true})
sy := value.Format(y, value.FormatConfig{UseStringer: true})
if sx == sy {
// Stringer is not helpful, so rely on more exact formatting.
sx = value.Format(x, false)
sy = value.Format(y, false)
// Unhelpful output, so use more exact formatting.
sx = value.Format(x, value.FormatConfig{PrintPrimitiveType: true})
sy = value.Format(y, value.FormatConfig{PrintPrimitiveType: true})
}
s := fmt.Sprintf("%#v:\n\t-: %s\n\t+: %s\n", p, sx, sy)
r.diffs = append(r.diffs, s)
Expand Down

0 comments on commit bef75ba

Please sign in to comment.