Sort map keys. Expand maps unconditionally when len() > 2

.github/workflows/build-test.yml

@@ -0,0 +1,17 @@
+name: build-test
+on:
+  push:
+  pull_request:
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/setup-go@v1
+        with:
+          go-version: 1.13.x
+      - uses: actions/checkout@v2
+      - name: Build
+        run: go build .
+      - name: Test
+        run: go test -v .

License

@@ -1,5 +1,3 @@
-The MIT License (MIT)
-
 Copyright 2012 Keith Rarick
 
 Permission is hereby granted, free of charge, to any person obtaining a copy

diff.go

@@ -41,7 +41,12 @@ type Printfer interface {
 // It calls Printf once for each difference, with no trailing newline.
 // The standard library log.Logger is a Printfer.
 func Pdiff(p Printfer, a, b interface{}) {
-	diffPrinter{w: p}.diff(reflect.ValueOf(a), reflect.ValueOf(b))
+	d := diffPrinter{
+		w:        p,
+		aVisited: make(map[visit]visit),
+		bVisited: make(map[visit]visit),
+	}
+	d.diff(reflect.ValueOf(a), reflect.ValueOf(b))
 }
 
 type Logfer interface {
@@ -66,6 +71,9 @@ func Ldiff(l Logfer, a, b interface{}) {
 type diffPrinter struct {
 	w Printfer
 	l string // label
+
+	aVisited map[visit]visit
+	bVisited map[visit]visit
 }
 
 func (w diffPrinter) printf(f string, a ...interface{}) {
@@ -96,6 +104,28 @@ func (w diffPrinter) diff(av, bv reflect.Value) {
 		return
 	}
 
+	if av.CanAddr() && bv.CanAddr() {
+		avis := visit{av.UnsafeAddr(), at}
+		bvis := visit{bv.UnsafeAddr(), bt}
+		var cycle bool
+
+		// Have we seen this value before?
+		if vis, ok := w.aVisited[avis]; ok {
+			cycle = true
+			if vis != bvis {
+				w.printf("%# v (previously visited) != %# v", formatter{v: av, quote: true}, formatter{v: bv, quote: true})
+			}
+		} else if _, ok := w.bVisited[bvis]; ok {
+			cycle = true
+			w.printf("%# v != %# v (previously visited)", formatter{v: av, quote: true}, formatter{v: bv, quote: true})
+		}
+		w.aVisited[avis] = bvis
+		w.bVisited[bvis] = avis
+		if cycle {
+			return
+		}
+	}
+
 	switch kind := at.Kind(); kind {
 	case reflect.Bool:
 		if a, b := av.Bool(), bv.Bool(); a != b {

diff_test.go

@@ -130,20 +130,23 @@ var diffs = []difftest{
 
 func TestDiff(t *testing.T) {
 	for _, tt := range diffs {
-		got := Diff(tt.a, tt.b)
-		eq := len(got) == len(tt.exp)
-		if eq {
-			for i := range got {
-				eq = eq && got[i] == tt.exp[i]
-			}
-		}
-		if !eq {
-			t.Errorf("diffing % #v", tt.a)
-			t.Errorf("with    % #v", tt.b)
-			diffdiff(t, got, tt.exp)
-			continue
+		expectDiffOutput(t, tt.a, tt.b, tt.exp)
+	}
+}
+
+func expectDiffOutput(t *testing.T, a, b interface{}, exp []string) {
+	got := Diff(a, b)
+	eq := len(got) == len(exp)
+	if eq {
+		for i := range got {
+			eq = eq && got[i] == exp[i]
 		}
 	}
+	if !eq {
+		t.Errorf("diffing % #v", a)
+		t.Errorf("with    % #v", b)
+		diffdiff(t, got, exp)
+	}
 }
 
 func TestKeyEqual(t *testing.T) {
@@ -193,6 +196,47 @@ func TestFdiff(t *testing.T) {
 	}
 }
 
+func TestDiffCycle(t *testing.T) {
+	// Diff two cyclic structs
+	a := &I{i: 1, R: nil}
+	a.R = a
+	b := &I{i: 2, R: nil}
+	b.R = b
+	expectDiffOutput(t, a, b, []string{
+		`i: 1 != 2`,
+	})
+
+	// Diff two equal cyclic structs
+	b.i = 1
+	expectDiffOutput(t, a, b, []string{})
+
+	// Diff two structs with different cycles
+	b2 := &I{i: 1, R: b}
+	b.R = b2
+	expectDiffOutput(t, a, b, []string{`R: pretty.I{
+    i:  1,
+    R:  &pretty.I{(CYCLIC REFERENCE)},
+} (previously visited) != pretty.I{
+    i:  1,
+    R:  &pretty.I{
+        i:  1,
+        R:  &pretty.I{(CYCLIC REFERENCE)},
+    },
+}`})
+
+	// ... and the same in the other direction
+	expectDiffOutput(t, b, a, []string{`R: pretty.I{
+    i:  1,
+    R:  &pretty.I{
+        i:  1,
+        R:  &pretty.I{(CYCLIC REFERENCE)},
+    },
+} != pretty.I{
+    i:  1,
+    R:  &pretty.I{(CYCLIC REFERENCE)},
+} (previously visited)`})
+}
+
 func diffdiff(t *testing.T, got, exp []string) {
 	minus(t, "unexpected:", got, exp)
 	minus(t, "missing:", exp, got)

formatter.go

@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"io"
 	"reflect"
+	"runtime"
+	"sort"
 	"strconv"
 	"text/tabwriter"
 
@@ -37,7 +39,7 @@ func (fo formatter) passThrough(f fmt.State, c rune) {
 	s := "%"
 	for i := 0; i < 128; i++ {
 		if f.Flag(i) {
-			s += string(i)
+			s += string(rune(i))
 		}
 	}
 	if w, ok := f.Width(); ok {
@@ -117,23 +119,22 @@ func (p *printer) printValue(v reflect.Value, showType, quote bool) {
 		}
 		writeByte(p, '{')
 		if nonzero(v) {
-			expand := !canInline(v.Type())
+			expand := !canInline(v.Type()) || v.Len() > 2
 			pp := p
 			if expand {
 				writeByte(p, '\n')
 				pp = p.indent()
 			}
-			keys := v.MapKeys()
-			for i := 0; i < v.Len(); i++ {
-				showTypeInStruct := true
-				k := keys[i]
+			keys := sortableValues(v.MapKeys())
+			sort.Sort(keys)
+			for i, k := range keys {
 				mv := v.MapIndex(k)
 				pp.printValue(k, false, true)
 				writeByte(pp, ':')
 				if expand {
 					writeByte(pp, '\t')
 				}
-				showTypeInStruct = t.Elem().Kind() == reflect.Interface
+				showTypeInStruct := t.Elem().Kind() == reflect.Interface
 				pp.printValue(mv, showTypeInStruct, true)
 				if expand {
 					io.WriteString(pp, ",\n")
@@ -267,6 +268,84 @@ func (p *printer) printValue(v reflect.Value, showType, quote bool) {
 	}
 }
 
+// sortableValues implement sort.Interface for reflect.Value to sort map keys.
+type sortableValues []reflect.Value
+
+func (s sortableValues) Len() int           { return len(s) }
+func (s sortableValues) Less(i, j int) bool { return less(s[i], s[j]) }
+func (s sortableValues) Swap(i, j int)      { s[i], s[j] = s[j], s[i] }
+
+func less(i, j reflect.Value) bool {
+	kind := i.Kind()
+	if kind != j.Kind() {
+		panic("different kinds in same slice, unexpected")
+	}
+	switch i.Kind() {
+	case reflect.Bool:
+		return !i.Bool() && j.Bool()
+	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
+		return i.Int() < j.Int()
+	case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
+		return i.Uint() < j.Uint()
+	case reflect.Float32, reflect.Float64:
+		return i.Float() < j.Float()
+	case reflect.Complex64, reflect.Complex128:
+		ii := i.Complex()
+		jj := j.Complex()
+		if real(ii) < real(jj) {
+			return true
+		}
+		if real(ii) > real(jj) {
+			return false
+		}
+		return imag(ii) < imag(jj)
+	case reflect.String:
+		return i.String() < j.String()
+	case reflect.Array, reflect.Slice:
+		// TODO(maruel): Use Less(i, j int) bool when available.
+		l := i.Len()
+		if ll := j.Len(); ll < l {
+			l = ll
+		}
+		for index := 0; index < l; index++ {
+			ii := i.Index(index)
+			jj := j.Index(index)
+			if less(ii, jj) {
+				return true
+			}
+			if less(jj, ii) {
+				return false
+			}
+		}
+		return i.Len() < j.Len()
+	case reflect.Chan, reflect.Map:
+		return i.Len() < j.Len()
+	case reflect.Interface, reflect.Ptr:
+		ii := nonzero(i)
+		jj := nonzero(j)
+		if !ii {
+			return jj
+		}
+		if !jj {
+			return false
+		}
+		return less(i.Elem(), j.Elem())
+	case reflect.Func:
+		return getFuncName(i) < getFuncName(j)
+	case reflect.Struct:
+		// TODO(maruel): Compare members one by one.
+		return false
+	case reflect.UnsafePointer:
+		fallthrough
+	default:
+		return i.Pointer() < j.Pointer()
+	}
+}
+
+func getFuncName(i reflect.Value) string {
+	return runtime.FuncForPC(i.Pointer()).Name()
+}
+
 func canInline(t reflect.Type) bool {
 	switch t.Kind() {
 	case reflect.Map:

formatter_test.go

@@ -3,6 +3,7 @@ package pretty
 import (
 	"fmt"
 	"io"
+	"reflect"
 	"strings"
 	"testing"
 	"unsafe"
@@ -38,7 +39,7 @@ func (f F) Format(s fmt.State, c rune) {
 	fmt.Fprintf(s, "F(%d)", int(f))
 }
 
-type Stringer struct { i int }
+type Stringer struct{ i int }
 
 func (s *Stringer) String() string { return "foo" }
 
@@ -91,15 +92,36 @@ var gosyntax = []test{
 }`,
 	},
 	{
-		map[int][]byte{1: {}},
+		map[int]string{1: "a", 2: "b"},
+		`map[int]string{1:"a", 2:"b"}`,
+	},
+	{
+		map[int]string{1: "a", 2: "b", 3: "c"},
+		`map[int]string{
+    1:  "a",
+    2:  "b",
+    3:  "c",
+}`,
+	},
+	{
+		map[int][]byte{1: {}, 2: {}},
 		`map[int][]uint8{
     1:  {},
+    2:  {},
 }`,
 	},
 	{
-		map[int]T{1: {}},
+		map[int]T{1: {}, 2: {}},
 		`map[int]pretty.T{
     1:  {},
+    2:  {},
+}`,
+	},
+	{
+		map[string]T{"a": {}, "b": {}},
+		`map[string]pretty.T{
+    "a": {},
+    "b": {},
 }`,
 	},
 	{
@@ -286,3 +308,42 @@ func TestCycle(t *testing.T) {
 	*iv = *i
 	t.Logf("Example long interface cycle:\n%# v", Formatter(i))
 }
+
+func TestLess(t *testing.T) {
+	zero := new(int)
+	one := new(int)
+	*one = 1
+	data := []struct {
+		i, j interface{}
+		less bool
+	}{
+		{true, true, false},
+		{false, true, true},
+		{uint(0), uint(0), false},
+		{uint(0), uint(1), true},
+		{0., 0., false},
+		{0., 1., true},
+		{complex(0., 0.), complex(0., 0.), false},
+		{complex(0., 0.), complex(1., 0.), true},
+		{complex(1., 0.), complex(0., 0.), false},
+		{[]int{0, 0}, []int{0}, false},
+		{[]int{0, 0}, []int{1}, true},
+		{[]int{1}, []int{0}, false},
+		{map[int]int{}, map[int]int{0: 0}, true},
+		//{interface{}(0), interface{}(0), false},
+		{TestLess, TestCycle, false},
+		{TestCycle, TestLess, true},
+		{zero, zero, false},
+		{zero, one, true},
+		{zero, (*int)(nil), false},
+		{(*int)(nil), (*int)(nil), false},
+		{(*int)(nil), zero, true},
+	}
+	for _, line := range data {
+		if less(reflect.ValueOf(line.i), reflect.ValueOf(line.j)) != line.less {
+			t.Errorf("less(%q, %q) != %t", line.i, line.j, line.less)
+		}
+	}
+	// Return value is non-deterministic. Just ensure it doesn't crash.
+	less(reflect.ValueOf(struct{}{}), reflect.ValueOf(struct{}{}))
+}

go.mod

@@ -1,3 +1,5 @@
-module "github.com/kr/pretty"
+module github.com/kr/pretty
 
-require "github.com/kr/text" v0.1.0
+go 1.12
+
+require github.com/kr/text v0.1.0