Enable methods to sort map keys and spew itself as last resort

If enabled by flags, try to use methods to stringify map keys and sort on that.

If we can't use primitive sorting and we can't use methods, we can still fall
back on spew itself.  If SpewKeys is enabled, use Sprintf("%#v") to generate a
string and sort by that.

README.md

@@ -132,9 +132,16 @@ options. See the ConfigState documentation for more details.
 	Specifies map keys should be sorted before being printed. Use
 	this to have a more deterministic, diffable output.  Note that
 	only native types (bool, int, uint, floats, uintptr and string)
-	are supported with other types sorted according to the
+	and types which implement error or Stringer interfaces are supported,
-	reflect.Value.String() output which guarantees display stability.
+	with other types sorted according to the reflect.Value.String() output
-	Natural map order is used by default.
+	which guarantees display stability.  Natural map order is used by
+	default.
+
+* SpewKeys
+	SpewKeys specifies that, as a last resort attempt, map keys should be
+	spewed to strings and sorted by those strings.  This is only considered
+	if SortKeys is true.
+
 ```
 
 ## License

spew/common.go

@@ -17,6 +17,7 @@
 package spew
 
 import (
+	"bytes"
 	"fmt"
 	"io"
 	"reflect"
@@ -325,7 +326,61 @@ func printHexPtr(w io.Writer, p uintptr) {
 // valuesSorter implements sort.Interface to allow a slice of reflect.Value
 // elements to be sorted.
 type valuesSorter struct {
-	values []reflect.Value
+	values  []reflect.Value
+	strings []string // either nil or same len and values
+	cs      *ConfigState
+}
+
+// newValuesSorter initializes a valuesSorter instance, which holds a set of
+// surrogate keys on which the data should be sorted.  It uses flags in
+// ConfigState to decide if and how to populate those surrogate keys.
+func newValuesSorter(values []reflect.Value, cs *ConfigState) sort.Interface {
+	vs := &valuesSorter{values: values, cs: cs}
+	if canSortSimply(vs.values[0].Kind()) {
+		return vs
+	}
+	if !cs.DisableMethods {
+		vs.strings = make([]string, len(values))
+		for i := range vs.values {
+			b := bytes.Buffer{}
+			if !handleMethods(cs, &b, vs.values[i]) {
+				vs.strings = nil
+				break
+			}
+			vs.strings[i] = b.String()
+		}
+	}
+	if vs.strings == nil && cs.SpewKeys {
+		vs.strings = make([]string, len(values))
+		for i := range vs.values {
+			vs.strings[i] = Sprintf("%#v", vs.values[i].Interface())
+		}
+	}
+	return vs
+}
+
+// canSortSimply tests whether a reflect.Kind is a primitive that can be sorted
+// directly, or whether it should be considered for sorting by surrogate keys
+// (if the ConfigState allows it).
+func canSortSimply(kind reflect.Kind) bool {
+	// This switch parallels valueSortLess, except for the default case.
+	switch kind {
+	case reflect.Bool:
+		return true
+	case reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Int:
+		return true
+	case reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uint:
+		return true
+	case reflect.Float32, reflect.Float64:
+		return true
+	case reflect.String:
+		return true
+	case reflect.Uintptr:
+		return true
+	case reflect.Array:
+		return true
+	}
+	return false
 }
 
 // Len returns the number of values in the slice.  It is part of the
@@ -338,6 +393,9 @@ func (s *valuesSorter) Len() int {
 // sort.Interface implementation.
 func (s *valuesSorter) Swap(i, j int) {
 	s.values[i], s.values[j] = s.values[j], s.values[i]
+	if s.strings != nil {
+		s.strings[i], s.strings[j] = s.strings[j], s.strings[i]
+	}
 }
 
 // valueSortLess returns whether the first value should sort before the second
@@ -375,15 +433,18 @@ func valueSortLess(a, b reflect.Value) bool {
 // Less returns whether the value at index i should sort before the
 // value at index j.  It is part of the sort.Interface implementation.
 func (s *valuesSorter) Less(i, j int) bool {
-	return valueSortLess(s.values[i], s.values[j])
+	if s.strings == nil {
+		return valueSortLess(s.values[i], s.values[j])
+	}
+	return s.strings[i] < s.strings[j]
 }
 
-// sortValues is a generic sort function for native types: int, uint, bool,
+// sortValues is a sort function that handles both native types and any type that
-// string and uintptr.  Other inputs are sorted according to their
+// can be converted to error or Stringer.  Other inputs are sorted according to
-// Value.String() value to ensure display stability.
+// their Value.String() value to ensure display stability.
-func sortValues(values []reflect.Value) {
+func sortValues(values []reflect.Value, cs *ConfigState) {
 	if len(values) == 0 {
 		return
 	}
-	sort.Sort(&valuesSorter{values})
+	sort.Sort(newValuesSorter(values, cs))
 }

spew/common_test.go

@@ -18,9 +18,10 @@ package spew_test
 
 import (
 	"fmt"
-	"github.com/davecgh/go-spew/spew"
 	"reflect"
 	"testing"
+
+	"github.com/davecgh/go-spew/spew"
 )
 
 // custom type to test Stinger interface on non-pointer receiver.
@@ -113,9 +114,24 @@ func testFailed(result string, wants []string) bool {
 	return true
 }
 
-// TestSortValues ensures the sort functionality for relect.Value based sorting
+type sortableStruct struct {
-// works as intended.
+	x int
-func TestSortValues(t *testing.T) {
+}
+
+func (ss sortableStruct) String() string {
+	return fmt.Sprintf("ss.%d", ss.x)
+}
+
+type unsortableStruct struct {
+	x int
+}
+
+type sortTestCase struct {
+	input    []reflect.Value
+	expected []reflect.Value
+}
+
+func helpTestSortValues(tests []sortTestCase, cs *spew.ConfigState, t *testing.T) {
 	getInterfaces := func(values []reflect.Value) []interface{} {
 		interfaces := []interface{}{}
 		for _, v := range values {
@@ -124,6 +140,23 @@ func TestSortValues(t *testing.T) {
 		return interfaces
 	}
 
+	for _, test := range tests {
+		spew.SortValues(test.input, cs)
+		// reflect.DeepEqual cannot really make sense of reflect.Value,
+		// probably because of all the pointer tricks. For instance,
+		// v(2.0) != v(2.0) on a 32-bits system. Turn them into interface{}
+		// instead.
+		input := getInterfaces(test.input)
+		expected := getInterfaces(test.expected)
+		if !reflect.DeepEqual(input, expected) {
+			t.Errorf("Sort mismatch:\n %v != %v", input, expected)
+		}
+	}
+}
+
+// TestSortValues ensures the sort functionality for relect.Value based sorting
+// works as intended.
+func TestSortValues(t *testing.T) {
 	v := reflect.ValueOf
 
 	a := v("a")
@@ -132,10 +165,7 @@ func TestSortValues(t *testing.T) {
 	embedA := v(embed{"a"})
 	embedB := v(embed{"b"})
 	embedC := v(embed{"c"})
-	tests := []struct {
+	tests := []sortTestCase{
-		input    []reflect.Value
-		expected []reflect.Value
-	}{
 		// No values.
 		{
 			[]reflect.Value{},
@@ -176,22 +206,93 @@ func TestSortValues(t *testing.T) {
 			[]reflect.Value{v(uintptr(2)), v(uintptr(1)), v(uintptr(3))},
 			[]reflect.Value{v(uintptr(1)), v(uintptr(2)), v(uintptr(3))},
 		},
+		// SortableStructs.
+		{
+			// Note: not sorted - DisableMethods is set.
+			[]reflect.Value{v(sortableStruct{2}), v(sortableStruct{1}), v(sortableStruct{3})},
+			[]reflect.Value{v(sortableStruct{2}), v(sortableStruct{1}), v(sortableStruct{3})},
+		},
+		// UnsortableStructs.
+		{
+			// Note: not sorted - SpewKeys is false.
+			[]reflect.Value{v(unsortableStruct{2}), v(unsortableStruct{1}), v(unsortableStruct{3})},
+			[]reflect.Value{v(unsortableStruct{2}), v(unsortableStruct{1}), v(unsortableStruct{3})},
+		},
 		// Invalid.
 		{
 			[]reflect.Value{embedB, embedA, embedC},
 			[]reflect.Value{embedB, embedA, embedC},
 		},
 	}
-	for _, test := range tests {
+	cs := spew.ConfigState{DisableMethods: true, SpewKeys: false}
-		spew.SortValues(test.input)
+	helpTestSortValues(tests, &cs, t)
-		// reflect.DeepEqual cannot really make sense of reflect.Value,
+}
-		// probably because of all the pointer tricks. For instance,
+
-		// v(2.0) != v(2.0) on a 32-bits system. Turn them into interface{}
+// TestSortValuesWithMethods ensures the sort functionality for relect.Value
-		// instead.
+// based sorting works as intended when using string methods.
-		input := getInterfaces(test.input)
+func TestSortValuesWithMethods(t *testing.T) {
-		expected := getInterfaces(test.expected)
+	v := reflect.ValueOf
-		if !reflect.DeepEqual(input, expected) {
+
-			t.Errorf("Sort mismatch:\n %v != %v", input, expected)
+	a := v("a")
-		}
+	b := v("b")
-	}
+	c := v("c")
+	tests := []sortTestCase{
+		// Ints.
+		{
+			[]reflect.Value{v(2), v(1), v(3)},
+			[]reflect.Value{v(1), v(2), v(3)},
+		},
+		// Strings.
+		{
+			[]reflect.Value{b, a, c},
+			[]reflect.Value{a, b, c},
+		},
+		// SortableStructs.
+		{
+			[]reflect.Value{v(sortableStruct{2}), v(sortableStruct{1}), v(sortableStruct{3})},
+			[]reflect.Value{v(sortableStruct{1}), v(sortableStruct{2}), v(sortableStruct{3})},
+		},
+		// UnsortableStructs.
+		{
+			// Note: not sorted - SpewKeys is false.
+			[]reflect.Value{v(unsortableStruct{2}), v(unsortableStruct{1}), v(unsortableStruct{3})},
+			[]reflect.Value{v(unsortableStruct{2}), v(unsortableStruct{1}), v(unsortableStruct{3})},
+		},
+	}
+	cs := spew.ConfigState{DisableMethods: false, SpewKeys: false}
+	helpTestSortValues(tests, &cs, t)
+}
+
+// TestSortValuesWithSpew ensures the sort functionality for relect.Value
+// based sorting works as intended when using spew to stringify keys.
+func TestSortValuesWithSpew(t *testing.T) {
+	v := reflect.ValueOf
+
+	a := v("a")
+	b := v("b")
+	c := v("c")
+	tests := []sortTestCase{
+		// Ints.
+		{
+			[]reflect.Value{v(2), v(1), v(3)},
+			[]reflect.Value{v(1), v(2), v(3)},
+		},
+		// Strings.
+		{
+			[]reflect.Value{b, a, c},
+			[]reflect.Value{a, b, c},
+		},
+		// SortableStructs.
+		{
+			[]reflect.Value{v(sortableStruct{2}), v(sortableStruct{1}), v(sortableStruct{3})},
+			[]reflect.Value{v(sortableStruct{1}), v(sortableStruct{2}), v(sortableStruct{3})},
+		},
+		// UnsortableStructs.
+		{
+			[]reflect.Value{v(unsortableStruct{2}), v(unsortableStruct{1}), v(unsortableStruct{3})},
+			[]reflect.Value{v(unsortableStruct{1}), v(unsortableStruct{2}), v(unsortableStruct{3})},
+		},
+	}
+	cs := spew.ConfigState{DisableMethods: true, SpewKeys: true}
+	helpTestSortValues(tests, &cs, t)
 }

spew/config.go

@@ -76,10 +76,16 @@ type ConfigState struct {
 
 	// SortKeys specifies map keys should be sorted before being printed. Use
 	// this to have a more deterministic, diffable output.  Note that only
-	// native types (bool, int, uint, floats, uintptr and string) are supported
+	// native types (bool, int, uint, floats, uintptr and string) and types
-	// with other types sorted according to the reflect.Value.String() output
+	// that support the error or Stringer interfaces (if methods are
-	// which guarantees display stability.
+	// enabled) are supported, with other types sorted according to the
+	// reflect.Value.String() output which guarantees display stability.
 	SortKeys bool
+
+	// SpewKeys specifies that, as a last resort attempt, map keys should
+	// be spewed to strings and sorted by those strings.  This is only
+	// considered if SortKeys is true.
+	SpewKeys bool
 }
 
 // Config is the active configuration of the top-level functions.

spew/doc.go

@@ -99,9 +99,15 @@ The following configuration options are available:
 		Specifies map keys should be sorted before being printed. Use
 		this to have a more deterministic, diffable output.  Note that
 		only native types (bool, int, uint, floats, uintptr and string)
-		are supported with other types sorted according to the
+		and types which implement error or Stringer interfaces are
-		reflect.Value.String() output which guarantees display stability.
+		supported with other types sorted according to the
-		Natural map order is used by default.
+		reflect.Value.String() output which guarantees display
+		stability.  Natural map order is used by default.
+
+	* SpewKeys
+		Specifies that, as a last resort attempt, map keys should be
+		spewed to strings and sorted by those strings.  This is only
+		considered if SortKeys is true.
 
 Dump Usage
 

spew/dump.go

@@ -382,7 +382,7 @@ func (d *dumpState) dump(v reflect.Value) {
 			numEntries := v.Len()
 			keys := v.MapKeys()
 			if d.cs.SortKeys {
-				sortValues(keys)
+				sortValues(keys, d.cs)
 			}
 			for i, key := range keys {
 				d.dump(d.unpackValue(key))

spew/dump_test.go

@@ -64,9 +64,10 @@ package spew_test
 import (
 	"bytes"
 	"fmt"
-	"github.com/davecgh/go-spew/spew"
 	"testing"
 	"unsafe"
+
+	"github.com/davecgh/go-spew/spew"
 )
 
 // dumpTest is used to describe a test to be perfomed against the Dump method.
@@ -983,4 +984,38 @@ func TestDumpSortedKeys(t *testing.T) {
 	if s != expected {
 		t.Errorf("Sorted keys mismatch:\n  %v %v", s, expected)
 	}
+
+	s = cfg.Sdump(map[stringer]int{"1": 1, "3": 3, "2": 2})
+	expected = `(map[spew_test.stringer]int) (len=3) {
+(spew_test.stringer) (len=1) stringer 1: (int) 1,
+(spew_test.stringer) (len=1) stringer 2: (int) 2,
+(spew_test.stringer) (len=1) stringer 3: (int) 3
+}
+`
+	if s != expected {
+		t.Errorf("Sorted keys mismatch:\n  %v %v", s, expected)
+	}
+
+	s = cfg.Sdump(map[pstringer]int{pstringer("1"): 1, pstringer("3"): 3, pstringer("2"): 2})
+	expected = `(map[spew_test.pstringer]int) (len=3) {
+(spew_test.pstringer) (len=1) stringer 1: (int) 1,
+(spew_test.pstringer) (len=1) stringer 2: (int) 2,
+(spew_test.pstringer) (len=1) stringer 3: (int) 3
+}
+`
+	if s != expected {
+		t.Errorf("Sorted keys mismatch:\n  %v %v", s, expected)
+	}
+
+	s = cfg.Sdump(map[customError]int{customError(1): 1, customError(3): 3, customError(2): 2})
+	expected = `(map[spew_test.customError]int) (len=3) {
+(spew_test.customError) error: 1: (int) 1,
+(spew_test.customError) error: 2: (int) 2,
+(spew_test.customError) error: 3: (int) 3
+}
+`
+	if s != expected {
+		t.Errorf("Sorted keys mismatch:\n  %v %v", s, expected)
+	}
+
 }

spew/format.go

@@ -309,7 +309,7 @@ func (f *formatState) format(v reflect.Value) {
 		} else {
 			keys := v.MapKeys()
 			if f.cs.SortKeys {
-				sortValues(keys)
+				sortValues(keys, f.cs)
 			}
 			for i, key := range keys {
 				if i > 0 {

spew/format_test.go

@@ -69,9 +69,10 @@ package spew_test
 import (
 	"bytes"
 	"fmt"
-	"github.com/davecgh/go-spew/spew"
 	"testing"
 	"unsafe"
+
+	"github.com/davecgh/go-spew/spew"
 )
 
 // formatterTest is used to describe a test to be perfomed against NewFormatter.
@@ -1478,6 +1479,22 @@ func TestFormatter(t *testing.T) {
 	}
 }
 
+type testStruct struct {
+	x int
+}
+
+func (ts testStruct) String() string {
+	return fmt.Sprintf("ts.%d", ts.x)
+}
+
+type testStructP struct {
+	x int
+}
+
+func (ts *testStructP) String() string {
+	return fmt.Sprintf("ts.%d", ts.x)
+}
+
 func TestPrintSortedKeys(t *testing.T) {
 	cfg := spew.ConfigState{SortKeys: true}
 	s := cfg.Sprint(map[int]string{1: "1", 3: "3", 2: "2"})
@@ -1485,4 +1502,34 @@ func TestPrintSortedKeys(t *testing.T) {
 	if s != expected {
 		t.Errorf("Sorted keys mismatch:\n  %v %v", s, expected)
 	}
+
+	s = cfg.Sprint(map[stringer]int{"1": 1, "3": 3, "2": 2})
+	expected = "map[stringer 1:1 stringer 2:2 stringer 3:3]"
+	if s != expected {
+		t.Errorf("Sorted keys mismatch:\n  %v %v", s, expected)
+	}
+
+	s = cfg.Sprint(map[pstringer]int{pstringer("1"): 1, pstringer("3"): 3, pstringer("2"): 2})
+	expected = "map[stringer 1:1 stringer 2:2 stringer 3:3]"
+	if s != expected {
+		t.Errorf("Sorted keys mismatch:\n  %v %v", s, expected)
+	}
+
+	s = cfg.Sprint(map[testStruct]int{testStruct{1}: 1, testStruct{3}: 3, testStruct{2}: 2})
+	expected = "map[ts.1:1 ts.2:2 ts.3:3]"
+	if s != expected {
+		t.Errorf("Sorted keys mismatch:\n  %v %v", s, expected)
+	}
+
+	s = cfg.Sprint(map[testStructP]int{testStructP{1}: 1, testStructP{3}: 3, testStructP{2}: 2})
+	expected = "map[ts.1:1 ts.2:2 ts.3:3]"
+	if s != expected {
+		t.Errorf("Sorted keys mismatch:\n  %v %v", s, expected)
+	}
+
+	s = cfg.Sprint(map[customError]int{customError(1): 1, customError(3): 3, customError(2): 2})
+	expected = "map[error: 1:1 error: 2:2 error: 3:3]"
+	if s != expected {
+		t.Errorf("Sorted keys mismatch:\n  %v %v", s, expected)
+	}
 }

spew/internal_test.go

@@ -151,6 +151,6 @@ func TestAddedReflectValue(t *testing.T) {
 
 // SortValues makes the internal sortValues function available to the test
 // package.
-func SortValues(values []reflect.Value) {
+func SortValues(values []reflect.Value, cs *ConfigState) {
-	sortValues(values)
+	sortValues(values, cs)
 }