From d5fcb386864232c4b315ea7f9838e455a2ce3cda Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Sun, 18 Jul 2021 17:32:48 -0700 Subject: [PATCH] Fix textual printing of byte slices There are two bugs being fixed: 1. The hueristic for whether a slice of byte looks like text should check whether a rune IsPrint OR IsSpace, and not both. Only a single rune (i.e., U+0020) ever satisfies both conditions. Previously, it would print as: MyBytes{0x68, 0x65, 0x6c, 0x6c, 0x6f} and now it would now print as: MyBytes(MyBytes("hello")) 2. If we're printing as string, then we should set skipType=true since we already explicitly format the value with the type. Previously, it would print as: MyBytes(MyBytes("hello")) and now it would now print as: MyBytes("hello") --- cmp/compare_test.go | 20 ++++++++++++++++++++ cmp/report_reflect.go | 3 ++- cmp/testdata/diffs | 24 ++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/cmp/compare_test.go b/cmp/compare_test.go index dc0bfe0..a435209 100644 --- a/cmp/compare_test.go +++ b/cmp/compare_test.go @@ -1317,6 +1317,26 @@ using the AllowUnexported option.`, "\n"), x: "d5c14bdf6bac81c27afc5429500ed750\n25483503b557c606dad4f144d27ae10b\n90bdbcdbb6ea7156068e3dcfb7459244\n978f480a6e3cced51e297fbff9a506b7\n", y: "Xd5c14bdf6bac81c27afc5429500ed750\nX25483503b557c606dad4f144d27ae10b\nX90bdbcdbb6ea7156068e3dcfb7459244\nX978f480a6e3cced51e297fbff9a506b7\n", reason: "all lines are different, so diffing based on lines is pointless", + }, { + label: label + "/StringifiedBytes", + x: struct{ X []byte }{[]byte("hello, world!")}, + y: struct{ X []byte }{}, + reason: "[]byte should be printed as text since it is printable text", + }, { + label: label + "/NonStringifiedBytes", + x: struct{ X []byte }{[]byte("\xde\xad\xbe\xef")}, + y: struct{ X []byte }{}, + reason: "[]byte should not be printed as text since it is binary data", + }, { + label: label + "/StringifiedNamedBytes", + x: struct{ X MyBytes }{MyBytes("hello, world!")}, + y: struct{ X MyBytes }{}, + reason: "MyBytes should be printed as text since it is printable text", + }, { + label: label + "/NonStringifiedNamedBytes", + x: struct{ X MyBytes }{MyBytes("\xde\xad\xbe\xef")}, + y: struct{ X MyBytes }{}, + reason: "MyBytes should not be printed as text since it is binary data", }} } diff --git a/cmp/report_reflect.go b/cmp/report_reflect.go index 33f0357..76c04fd 100644 --- a/cmp/report_reflect.go +++ b/cmp/report_reflect.go @@ -207,9 +207,10 @@ func (opts formatOptions) FormatValue(v reflect.Value, parentKind reflect.Kind, // Check whether this is a []byte of text data. if t.Elem() == reflect.TypeOf(byte(0)) { b := v.Bytes() - isPrintSpace := func(r rune) bool { return unicode.IsPrint(r) && unicode.IsSpace(r) } + isPrintSpace := func(r rune) bool { return unicode.IsPrint(r) || unicode.IsSpace(r) } if len(b) > 0 && utf8.Valid(b) && len(bytes.TrimFunc(b, isPrintSpace)) == 0 { out = opts.formatString("", string(b)) + skipType = true return opts.WithTypeMode(emitType).FormatType(t, out) } } diff --git a/cmp/testdata/diffs b/cmp/testdata/diffs index a3d5909..96ea191 100644 --- a/cmp/testdata/diffs +++ b/cmp/testdata/diffs @@ -1096,6 +1096,30 @@ "978f480a6e3cced51e297fbff9a506b7\n", }, "") >>> TestDiff/Reporter/AllLinesDiffer +<<< TestDiff/Reporter/StringifiedBytes + struct{ X []uint8 }{ +- X: []uint8("hello, world!"), ++ X: nil, + } +>>> TestDiff/Reporter/StringifiedBytes +<<< TestDiff/Reporter/NonStringifiedBytes + struct{ X []uint8 }{ +- X: []uint8{0xde, 0xad, 0xbe, 0xef}, ++ X: nil, + } +>>> TestDiff/Reporter/NonStringifiedBytes +<<< TestDiff/Reporter/StringifiedNamedBytes + struct{ X cmp_test.MyBytes }{ +- X: cmp_test.MyBytes("hello, world!"), ++ X: nil, + } +>>> TestDiff/Reporter/StringifiedNamedBytes +<<< TestDiff/Reporter/NonStringifiedNamedBytes + struct{ X cmp_test.MyBytes }{ +- X: cmp_test.MyBytes{0xde, 0xad, 0xbe, 0xef}, ++ X: nil, + } +>>> TestDiff/Reporter/NonStringifiedNamedBytes <<< TestDiff/EmbeddedStruct/ParentStructA/Inequal teststructs.ParentStructA{ privateStruct: teststructs.privateStruct{