Use raw literal for string formatting

When formatting strings, avoid using strconv.Quote if the string looks
like it has already been escaped. Instead, use a raw string literal
by wrapping the string with '`' characters if possible.
For now, we still use strconv.Quote if the input string contains newlines
to maintain the property that Format outputs a single line.

Also, prefix strings obtained by the Stringer.String method with a 's'.
This allows users to more easily distinguish when an output really is
of type string or if the String method was used.

cmp/compare_test.go

@@ -206,7 +206,7 @@ func comparerTests() []test {
 		label:    label,
 		x:        &struct{ R *bytes.Buffer }{new(bytes.Buffer)},
 		y:        &struct{ R *bytes.Buffer }{},
-		wantDiff: "root.R:\n\t-: \"\"\n\t+: <nil>\n",
+		wantDiff: "root.R:\n\t-: s\"\"\n\t+: <nil>\n",
 	}, {
 		label: label,
 		x:     &struct{ R *bytes.Buffer }{new(bytes.Buffer)},
@@ -262,8 +262,8 @@ func comparerTests() []test {
 		})},
 		wantDiff: `
 {[]*regexp.Regexp}[1]:
-	-: "a*b*c*"
-	+: "a*b*d*"`,
+	-: s"a*b*c*"
+	+: s"a*b*d*"`,
 	}, {
 		label: label,
 		x: func() ***int {
@@ -312,8 +312,8 @@ func comparerTests() []test {
 		opts:  []cmp.Option{cmp.Comparer(func(x, y fmt.Stringer) bool { return x.String() == y.String() })},
 		wantDiff: `
 root:
-	-: "hello"
-	+: "hello2"`,
+	-: s"hello"
+	+: s"hello2"`,
 	}, {
 		label: label,
 		x:     md5.Sum([]byte{'a'}),
@@ -405,6 +405,22 @@ root:
 {[]int}:
 	-: []int{0, 0, 0, 0, 0, 0, 0, 0, 0, 0}
 	+: []int{0, 0, 0, 0, 0, 0, 0, 0, 0, 0}`,
+	}, {
+		// Ensure reasonable Stringer formatting of map keys.
+		label: label,
+		x: map[*pb.Stringer]*pb.Stringer{
+			&pb.Stringer{"hello"}: &pb.Stringer{"world"},
+		},
+		y: map[*pb.Stringer]*pb.Stringer(nil),
+		wantDiff: `
+{map[*testprotos.Stringer]*testprotos.Stringer}:
+	-: map[*testprotos.Stringer]*testprotos.Stringer{s"hello": s"world"}
+	+: map[*testprotos.Stringer]*testprotos.Stringer(nil)`,
+	}, {
+		// Ensure Stringer avoids double-quote escaping if possible.
+		label:    label,
+		x:        []*pb.Stringer{&pb.Stringer{`multi\nline\nline\nline`}},
+		wantDiff: ":\n\t-: []*testprotos.Stringer{s`multi\\nline\\nline\\nline`}\n\t+: <non-existent>",
 	}}
 }
 
@@ -1535,7 +1551,7 @@ func project1Tests() []test {
 			Args: &pb.MetaData{Stringer: pb.Stringer{"metadata2"}},
 		}}},
 		opts:     []cmp.Option{cmp.Comparer(pb.Equal)},
-		wantDiff: "{teststructs.Eagle}.Slaps[4].Args:\n\t-: \"metadata\"\n\t+: \"metadata2\"\n",
+		wantDiff: "{teststructs.Eagle}.Slaps[4].Args:\n\t-: s\"metadata\"\n\t+: s\"metadata2\"\n",
 	}, {
 		label: label,
 		x:     createEagle(),
@@ -1657,11 +1673,11 @@ func project2Tests() []test {
 		opts: []cmp.Option{cmp.Comparer(pb.Equal), equalDish},
 		wantDiff: `
 {teststructs.GermBatch}.DirtyGerms[18][0->?]:
-	-: "germ2"
+	-: s"germ2"
 	+: <non-existent>
 {teststructs.GermBatch}.DirtyGerms[18][?->2]:
 	-: <non-existent>
-	+: "germ2"`,
+	+: s"germ2"`,
 	}, {
 		label: label,
 		x:     createBatch(),
@@ -1690,9 +1706,9 @@ func project2Tests() []test {
 		wantDiff: `
 {teststructs.GermBatch}.DirtyGerms[17]:
 	-: <non-existent>
-	+: []*testprotos.Germ{"germ1"}
+	+: []*testprotos.Germ{s"germ1"}
 {teststructs.GermBatch}.DirtyGerms[18][2->?]:
-	-: "germ4"
+	-: s"germ4"
 	+: <non-existent>
 {teststructs.GermBatch}.DishMap[1]:
 	-: (*teststructs.Dish)(nil)
@@ -1777,14 +1793,14 @@ func project3Tests() []test {
 	-: teststructs.DiscordState(554)
 	+: teststructs.DiscordState(500)
 λ({teststructs.Dirt}.Proto):
-	-: "blah"
-	+: "proto"
+	-: s"blah"
+	+: s"proto"
 {teststructs.Dirt}.wizard["albus"]:
-	-: "dumbledore"
+	-: s"dumbledore"
 	+: <non-existent>
 {teststructs.Dirt}.wizard["harry"]:
-	-: "potter"
-	+: "otter"`,
+	-: s"potter"
+	+: s"otter"`,
 	}}
 }
 

cmp/internal/value/format.go

@@ -8,9 +8,9 @@ package value
 import (
 	"fmt"
 	"reflect"
+	"strconv"
 	"strings"
 	"unicode"
-	"unicode/utf8"
 )
 
 var stringerIface = reflect.TypeOf((*fmt.Stringer)(nil)).Elem()
@@ -43,7 +43,10 @@ func formatAny(v reflect.Value, conf formatConfig, visited map[uintptr]bool) str
 		if (v.Kind() == reflect.Ptr || v.Kind() == reflect.Interface) && v.IsNil() {
 			return "<nil>"
 		}
-		return fmt.Sprintf("%q", v.Interface().(fmt.Stringer).String())
+
+		const stringerPrefix = "s" // Indicates that the String method was used
+		s := v.Interface().(fmt.Stringer).String()
+		return stringerPrefix + formatString(s)
 	}
 
 	switch v.Kind() {
@@ -62,7 +65,7 @@ func formatAny(v reflect.Value, conf formatConfig, visited map[uintptr]bool) str
 	case reflect.Complex64, reflect.Complex128:
 		return formatPrimitive(v.Type(), v.Complex(), conf)
 	case reflect.String:
-		return formatPrimitive(v.Type(), fmt.Sprintf("%q", v), conf)
+		return formatPrimitive(v.Type(), formatString(v.String()), conf)
 	case reflect.UnsafePointer, reflect.Chan, reflect.Func:
 		return formatPointer(v, conf)
 	case reflect.Ptr:
@@ -126,7 +129,7 @@ func formatAny(v reflect.Value, conf formatConfig, visited map[uintptr]bool) str
 		subConf := conf
 		subConf.printType = v.Type().Elem().Kind() == reflect.Interface
 		for _, k := range SortKeys(v.MapKeys()) {
-			sk := formatAny(k, formatConfig{realPointers: conf.realPointers}, visited)
+			sk := formatAny(k, subConf, visited)
 			sv := formatAny(v.MapIndex(k), subConf, visited)
 			ss = append(ss, fmt.Sprintf("%s: %s", sk, sv))
 		}
@@ -145,7 +148,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 && isExported(name)
+			subConf.useStringer = conf.useStringer
 			s := formatAny(vv, subConf, visited)
 			ss = append(ss, fmt.Sprintf("%s: %s", name, s))
 		}
@@ -159,6 +162,17 @@ func formatAny(v reflect.Value, conf formatConfig, visited map[uintptr]bool) str
 	}
 }
 
+func formatString(s string) string {
+	// Avoid performing quote-escaping if the string is already escaped.
+	hasEscapes := strings.ContainsAny(s, `\`)
+	allPrintable := strings.IndexFunc(s, unicode.IsPrint) >= 0
+	rawAllowed := !strings.ContainsAny(s, "`\n")
+	if hasEscapes && allPrintable && rawAllowed {
+		return "`" + s + "`"
+	}
+	return strconv.Quote(s)
+}
+
 func formatPrimitive(t reflect.Type, v interface{}, conf formatConfig) string {
 	if conf.printType && t.PkgPath() != "" {
 		return fmt.Sprintf("%v(%v)", t, v)
@@ -247,9 +261,3 @@ func isZero(v reflect.Value) bool {
 	}
 	return false
 }
-
-// isExported reports whether the identifier is exported.
-func isExported(id string) bool {
-	r, _ := utf8.DecodeRuneInString(id)
-	return unicode.IsUpper(r)
-}