feat: improve the way time.Time is stringified in spew

Now we vendored go-spew we can modify how time.Time values are handled.

We can remove the imperfect fix we had in place before.

Author: ccoVeille

assert/assertions.go

@@ -1951,9 +1951,6 @@ func diff(expected interface{}, actual interface{}) string {
 	case reflect.TypeOf(""):
 		e = reflect.ValueOf(expected).String()
 		a = reflect.ValueOf(actual).String()
-	case reflect.TypeOf(time.Time{}):
-		e = spewConfigStringerEnabled.Sdump(expected)
-		a = spewConfigStringerEnabled.Sdump(actual)
 	default:
 		e = spewConfig.Sdump(expected)
 		a = spewConfig.Sdump(actual)
@@ -1985,14 +1982,7 @@ var spewConfig = spew.ConfigState{
 	DisableCapacities:       true,
 	SortKeys:                true,
 	DisableMethods:          true,
-	MaxDepth:                10,
-}
-
-var spewConfigStringerEnabled = spew.ConfigState{
-	Indent:                  " ",
-	DisablePointerAddresses: true,
-	DisableCapacities:       true,
-	SortKeys:                true,
+	EnableTimeStringer:      true,
 	MaxDepth:                10,
 }
 

assert/assertions_test.go

@@ -3020,6 +3020,9 @@ Diff:
 	)
 	Equal(t, expected, actual)
 
+	timeA := time.Date(2020, 9, 24, 0, 0, 0, 0, time.UTC)
+	timeB := time.Date(2020, 9, 25, 0, 0, 0, 0, time.UTC)
+
 	expected = `
 
 Diff:
@@ -3028,14 +3031,117 @@ Diff:
 @@ -1,2 +1,2 @@
 -(time.Time) 2020-09-24 00:00:00 +0000 UTC
 +(time.Time) 2020-09-25 00:00:00 +0000 UTC
- 
+` + " \n"
+
+	actual = diff(timeA, timeB)
+	Equal(t, expected, actual)
+
+	expected = `
+
+Diff:
+--- Expected
++++ Actual
+@@ -1,2 +1,2 @@
+-(*time.Time)(2020-09-24 00:00:00 +0000 UTC)
++(*time.Time)(2020-09-25 00:00:00 +0000 UTC)
+` + " \n"
+
+	actual = diff(&timeA, &timeB)
+	Equal(t, expected, actual)
+
+	expected = `
+
+Diff:
+--- Expected
++++ Actual
+@@ -1,3 +1,3 @@
+ (assert.someStruct) {
+- t: (time.Time) 2020-09-24 00:00:00 +0000 UTC
++ t: (time.Time) 2020-09-25 00:00:00 +0000 UTC
+ }
 `
 
+	type someStruct struct {
+		t time.Time
+	}
+
 	actual = diff(
-		time.Date(2020, 9, 24, 0, 0, 0, 0, time.UTC),
-		time.Date(2020, 9, 25, 0, 0, 0, 0, time.UTC),
+		someStruct{t: timeA},
+		someStruct{t: timeB},
 	)
+
+	Equal(t, expected, actual)
+
+	// here we test the diff is stable even if the order of map keys is not
+	expected = `
+
+Diff:
+--- Expected
++++ Actual
+@@ -1,4 +1,4 @@
+ (map[time.Time]int) (len=3) {
+- (time.Time) 2020-09-24 00:00:00 +0000 UTC: (int) 1,
+- (time.Time) 2020-09-25 00:00:00 +0000 UTC: (int) 42,
++ (time.Time) 2020-09-24 00:00:00 +0000 UTC: (int) 2,
++ (time.Time) 2020-09-26 00:00:00 +0000 UTC: (int) 42,
+  (time.Time) 2020-09-27 00:00:00 +0000 UTC: (int) 100
+`
+
+	timeC := time.Date(2020, 9, 26, 0, 0, 0, 0, time.UTC)
+	timeD := time.Date(2020, 9, 27, 0, 0, 0, 0, time.UTC)
+
+	mapTimeA := map[time.Time]int{
+		timeA: 1,
+		timeB: 42,
+		timeD: 100,
+	}
+
+	mapTimeB := map[time.Time]int{
+		timeA: 2,
+		timeC: 42,
+		timeD: 100,
+	}
+
+	actual = diff(mapTimeA, mapTimeB)
 	Equal(t, expected, actual)
+
+	// here we test the time are ordered against the time.Time.Before() and not the time.Time.String()
+	expected = `
+
+Diff:
+--- Expected
++++ Actual
+@@ -1,5 +1,5 @@
+ (map[time.Time]int) (len=3) {
+- (time.Time) 2020-09-24 00:00:00 +0000 UTC: (int) 1,
+- (time.Time) 2020-09-25 00:00:00 +0900 JST: (int) 100,
+- (time.Time) 2020-09-25 00:00:00 +0000 UTC: (int) 42
++ (time.Time) 2020-09-24 00:00:00 +0900 JST: (int) 42,
++ (time.Time) 2020-09-24 00:00:00 +0000 UTC: (int) 2,
++ (time.Time) 2020-09-25 00:00:00 +0900 JST: (int) 100
+ }
+`
+
+	loc := time.FixedZone("JST", 9*60*60)
+
+	timeE := time.Date(2020, 9, 24, 0, 0, 0, 0, loc)
+	timeF := time.Date(2020, 9, 25, 0, 0, 0, 0, loc)
+
+	mapTimeLocA := map[time.Time]int{
+		timeA: 1,
+		timeB: 42,
+		timeF: 100,
+	}
+
+	mapTimeLocB := map[time.Time]int{
+		timeA: 2,
+		timeE: 42,
+		timeF: 100,
+	}
+
+	actual = diff(mapTimeLocA, mapTimeLocB)
+	Equal(t, expected, actual)
+
 }
 
 func TestTimeEqualityErrorFormatting(t *testing.T) {

internal/spew/common.go

@@ -23,6 +23,7 @@ import (
 	"reflect"
 	"sort"
 	"strconv"
+	"time"
 )
 
 // Some constants in the form of bytes to avoid string overhead.  This mirrors
@@ -227,7 +228,7 @@ type valuesSorter struct {
 // 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()) {
+	if canSortSimply(vs.values[0]) {
 		return vs
 	}
 	if !cs.DisableMethods {
@@ -253,9 +254,9 @@ func newValuesSorter(values []reflect.Value, cs *ConfigState) sort.Interface {
 // 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 {
+func canSortSimply(v reflect.Value) bool {
 	// This switch parallels valueSortLess, except for the default case.
-	switch kind {
+	switch v.Kind() {
 	case reflect.Bool:
 		return true
 	case reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Int:
@@ -271,7 +272,8 @@ func canSortSimply(kind reflect.Kind) bool {
 	case reflect.Array:
 		return true
 	}
-	return false
+
+	return isTime(v)
 }
 
 // Len returns the number of values in the slice.  It is part of the
@@ -318,6 +320,13 @@ func valueSortLess(a, b reflect.Value) bool {
 			return valueSortLess(av, bv)
 		}
 	}
+
+	if isTime(a) && a.CanInterface() && b.CanInterface() {
+		timeA, okA := a.Interface().(time.Time)
+		timeB, okB := b.Interface().(time.Time)
+		return okA && okB && timeA.Before(timeB)
+	}
+
 	return a.String() < b.String()
 }
 
@@ -339,3 +348,8 @@ func sortValues(values []reflect.Value, cs *ConfigState) {
 	}
 	sort.Sort(newValuesSorter(values, cs))
 }
+
+// isTime returns whether the passed reflect.Value is a [time.Time] struct.
+func isTime(v reflect.Value) bool {
+	return v.Kind() == reflect.Struct && v.Type().PkgPath() == "time" && v.Type().Name() == "Time"
+}

internal/spew/config.go

@@ -53,6 +53,12 @@ type ConfigState struct {
 	// invoked for types that implement them.
 	DisableMethods bool
 
+	// EnableTimeStringer specifies whether to invoke the Stringer interface on
+	// time.Time values even when DisableMethods is true. This is useful to get
+	// human-readable output for time.Time values while keeping method calls
+	// disabled for other types.
+	EnableTimeStringer bool
+
 	// DisablePointerMethods specifies whether or not to check for and invoke
 	// error and Stringer interfaces on types which only accept a pointer
 	// receiver when the current type is not a pointer.

internal/spew/dump.go

@@ -301,7 +301,7 @@ func (d *dumpState) dump(v reflect.Value) {
 
 	// Call Stringer/error interfaces if they exist and the handle methods flag
 	// is enabled
-	if !d.cs.DisableMethods {
+	if !d.cs.DisableMethods || (d.cs.EnableTimeStringer && isTime(v)) {
 		if (kind != reflect.Invalid) && (kind != reflect.Interface) {
 			if handled := handleMethods(d.cs, d.w, v); handled {
 				return

internal/spew/format.go

@@ -222,7 +222,7 @@ func (f *formatState) format(v reflect.Value) {
 
 	// Call Stringer/error interfaces if they exist and the handle methods
 	// flag is enabled.
-	if !f.cs.DisableMethods {
+	if !f.cs.DisableMethods || (f.cs.EnableTimeStringer && isTime(v)) {
 		if (kind != reflect.Invalid) && (kind != reflect.Interface) {
 			if handled := handleMethods(f.cs, f.fs, v); handled {
 				return

internal/spew/spew_test.go

@@ -22,6 +22,7 @@ import (
 	"io/ioutil"
 	"os"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/internal/spew"
 )
@@ -127,6 +128,7 @@ func initSpewTests() {
 	// Config states with various settings.
 	scsDefault := spew.NewDefaultConfig()
 	scsNoMethods := &spew.ConfigState{Indent: " ", DisableMethods: true}
+	scsNoMethodsButTimeStringer := &spew.ConfigState{Indent: " ", DisableMethods: true, EnableTimeStringer: true}
 	scsNoPmethods := &spew.ConfigState{Indent: " ", DisablePointerMethods: true}
 	scsMaxDepth := &spew.ConfigState{Indent: " ", MaxDepth: 1}
 	scsContinue := &spew.ConfigState{Indent: " ", ContinueOnMethod: true}
@@ -138,6 +140,8 @@ func initSpewTests() {
 	ts := stringer("test")
 	tps := pstringer("test")
 
+	tm := time.Date(2006, time.January, 2, 15, 4, 5, 999999999, time.UTC)
+
 	type ptrTester struct {
 		s *struct{}
 	}
@@ -203,6 +207,16 @@ func initSpewTests() {
 		{scsNoPtrAddr, fCSSdump, "", tptr, "(*spew_test.ptrTester)({\ns: (*struct {})({\n})\n})\n"},
 		{scsNoCap, fCSSdump, "", make([]string, 0, 10), "([]string) {\n}\n"},
 		{scsNoCap, fCSSdump, "", make([]string, 1, 10), "([]string) (len=1) {\n(string) \"\"\n}\n"},
+
+		// time.Time formatting:
+		{scsDefault, fCSFprint, "", tm, "2006-01-02 15:04:05.999999999 +0000 UTC"},
+		{scsNoMethods, fCSFprint, "", tm, scsNoMethods.Sprint(tm)},
+		{scsNoMethodsButTimeStringer, fCSFprint, "", tm, "2006-01-02 15:04:05.999999999 +0000 UTC"},
+
+		// *time.Time formatting:
+		{scsDefault, fCSFprint, "", &tm, "<*>2006-01-02 15:04:05.999999999 +0000 UTC"},
+		{scsNoMethods, fCSFprint, "", &tm, scsNoMethods.Sprint(&tm)},
+		{scsNoMethodsButTimeStringer, fCSFprint, "", &tm, "<*>2006-01-02 15:04:05.999999999 +0000 UTC"},
 	}
 }