Merge pull request #74 from cdr/more-assert

assert: Add Error and remove google/go-cmp

Author: nhooyr

go.mod

@@ -6,7 +6,6 @@ require (
 	cloud.google.com/go v0.43.0
 	github.com/alecthomas/chroma v0.6.6
 	github.com/fatih/color v1.7.0
-	github.com/google/go-cmp v0.3.2-0.20190829225427-b1c9c4891a65
 	github.com/mattn/go-colorable v0.1.2 // indirect
 	github.com/mattn/go-isatty v0.0.9 // indirect
 	go.opencensus.io v0.22.1

go.sum

@@ -48,8 +48,6 @@ github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ
 github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M=
 github.com/google/go-cmp v0.3.0 h1:crn/baboCvb5fXaQ0IJ1SGTsTVrWpDsCWC8EGETZijY=
 github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
-github.com/google/go-cmp v0.3.2-0.20190829225427-b1c9c4891a65 h1:B3yqxlLHBEoav+FDQM8ph7IIRA6AhQ70w119k3hoT2Y=
-github.com/google/go-cmp v0.3.2-0.20190829225427-b1c9c4891a65/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
 github.com/google/martian v2.1.0+incompatible h1:/CP5g8u/VJHijgedC/Legn3BAbAaWPgecwXBIDzw5no=
 github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs=
 github.com/google/pprof v0.0.0-20181206194817-3ea8567a2e57/go.mod h1:zfwlbNMJ+OItoe0UupaVj+oy1omPYYDuagoSzA8v9mc=

internal/assert/assert.go

@@ -10,17 +10,16 @@ import (
 // Equal asserts exp == act.
 func Equal(t testing.TB, exp, act interface{}, name string) {
 	t.Helper()
-	diff := CmpDiff(exp, act)
-	if diff != "" {
-		t.Fatalf("unexpected %v: %v", name, diff)
+	if !reflect.DeepEqual(exp, act) {
+		t.Fatalf("unexpected %v: exp: %q but got %q", name, exp, act)
 	}
 }
 
 // NotEqual asserts exp != act.
 func NotEqual(t testing.TB, exp, act interface{}, name string) {
 	t.Helper()
-	if CmpDiff(exp, act) == "" {
-		t.Fatalf("expected different %v: %+v", name, act)
+	if reflect.DeepEqual(exp, act) {
+		t.Fatalf("expected different %v: %q", name, act)
 	}
 }
 

internal/assert/cmp.go

@@ -47,6 +47,18 @@ func TestEntry(t *testing.T) {
 line2`)
 	})
 
+	t.Run("multilineField", func(t *testing.T) {
+		t.Parallel()
+
+		test(t, slog.SinkEntry{
+			Message: "msg",
+			Level:   slog.LevelInfo,
+			Fields:  slog.M(slog.F("field", "line1\nline2")),
+		}, `0001-01-01 00:00:00.000 [INFO]	<.:0>	msg ...
+"field": line1
+line2`)
+	})
+
 	t.Run("named", func(t *testing.T) {
 		t.Parallel()
 

internal/entryhuman/entry_test.go

@@ -27,17 +27,23 @@ var _ json.Marshaler = Map(nil)
 //
 // Every field value is encoded with the following process:
 //
-// 1. slog.Value is handled to allow any type to replace its representation for logging.
+// 1. slog.Value is checked to allow any type to replace its representation for logging.
+//
+// 2. json.Marshaller is handled.
 //
 // 2. xerrors.Formatter is handled.
 //
-// 3. Protobufs are handled with json.Marshal.
+// 3. Protobufs are encoded with json.Marshal.
+//
+// 4. error and fmt.Stringer are used if possible.
 //
-// 4. error and fmt.Stringer are handled.
+// 5. slices and arrays go through the encode function for every element.
 //
-// 5. slices and arrays are handled to go through the encode function for every value.
+// 6. If the value is a struct without exported fields or a type that
+//    cannot be encoded with json.Marshal (like channels) then
+//    fmt.Sprintf("%+v") is used.
 //
-// 6. json.Marshal is invoked as the default case.
+// 8. json.Marshal(v) is used for all other values.
 func (m Map) MarshalJSON() ([]byte, error) {
 	b := &bytes.Buffer{}
 	b.WriteByte('{')
@@ -56,8 +62,11 @@ func (m Map) MarshalJSON() ([]byte, error) {
 	return b.Bytes(), nil
 }
 
-// ForceJSON ensures the value is logged via json.Marshal even
-// if it implements fmt.Stringer or error.
+// ForceJSON ensures the value is logged via json.Marshal.
+//
+// Use it when implementing SlogValue to ensure a structured
+// representation of a struct if you know it's capable even
+// when it implements fmt.Stringer or error.
 func ForceJSON(v interface{}) interface{} {
 	return jsonVal{v: v}
 }
@@ -66,13 +75,6 @@ type jsonVal struct {
 	v interface{}
 }
 
-var _ json.Marshaler = jsonVal{}
-
-// MarshalJSON implements json.Marshaler.
-func (v jsonVal) MarshalJSON() ([]byte, error) {
-	return json.Marshal(v.v)
-}
-
 func marshalList(rv reflect.Value) []byte {
 	b := &bytes.Buffer{}
 	b.WriteByte('[')
@@ -93,6 +95,10 @@ func encode(v interface{}) []byte {
 	switch v := v.(type) {
 	case Value:
 		return encode(v.SlogValue())
+	case json.Marshaler:
+		return encodeJSON(v)
+	case jsonVal:
+		return encodeJSON(v.v)
 	case xerrors.Formatter:
 		return encode(errorChain(v))
 	case interface {
@@ -103,28 +109,47 @@ func encode(v interface{}) []byte {
 		return encode(fmt.Sprint(v))
 	default:
 		rv := reflect.Indirect(reflect.ValueOf(v))
-		if rv.IsValid() {
-			switch rv.Type().Kind() {
-			case reflect.Slice:
-				if rv.IsNil() {
-					break
-				}
-				fallthrough
-			case reflect.Array:
+		if !rv.IsValid() {
+			return encodeJSON(v)
+		}
+
+		switch rv.Type().Kind() {
+		case reflect.Slice:
+			if !rv.IsNil() {
 				return marshalList(rv)
 			}
-		}
+		case reflect.Array:
+			return marshalList(rv)
+		case reflect.Struct:
+			typ := rv.Type()
+			for i := 0; i < rv.NumField(); i++ {
+				// Found an exported field.
+				if typ.Field(i).PkgPath == "" {
+					return encodeJSON(v)
+				}
+			}
 
-		b, err := json.Marshal(v)
-		if err != nil {
-			return encode(M(
-				Error(xerrors.Errorf("failed to marshal to JSON: %w", err)),
-				F("type", reflect.TypeOf(v)),
-				F("value", fmt.Sprintf("%+v", v)),
-			))
+			return encodeJSON(fmt.Sprintf("%+v", v))
+		case reflect.Chan, reflect.Complex64, reflect.Complex128, reflect.Func:
+			// These types cannot be directly encoded with json.Marshal.
+			// See https://golang.org/pkg/encoding/json/#Marshal
+			return encodeJSON(fmt.Sprintf("%+v", v))
 		}
-		return b
+
+		return encodeJSON(v)
+	}
+}
+
+func encodeJSON(v interface{}) []byte {
+	b, err := json.Marshal(v)
+	if err != nil {
+		return encode(M(
+			Error(xerrors.Errorf("failed to marshal to JSON: %w", err)),
+			F("type", reflect.TypeOf(v)),
+			F("value", fmt.Sprintf("%+v", v)),
+		))
 	}
+	return b
 }
 
 func errorChain(f xerrors.Formatter) []interface{} {

map.go

@@ -3,11 +3,11 @@ package slog_test
 import (
 	"bytes"
 	"encoding/json"
-	"fmt"
 	"io"
 	"runtime"
 	"strings"
 	"testing"
+	"time"
 
 	"golang.org/x/xerrors"
 
@@ -86,19 +86,19 @@ func TestMap(t *testing.T) {
 		mapTestFile := strings.Replace(mapTestFile, "_test", "", 1)
 
 		test(t, slog.M(
-			slog.F("meow", indentJSON),
+			slog.F("meow", slog.ForceJSON(complex(10, 10))),
 		), `{
 			"meow": {
 				"error": [
 					{
 						"msg": "failed to marshal to JSON",
-						"fun": "cdr.dev/slog.encode",
-						"loc": "`+mapTestFile+`:121"
+						"fun": "cdr.dev/slog.encodeJSON",
+						"loc": "`+mapTestFile+`:147"
 					},
-					"json: unsupported type: func(*testing.T, string) string"
+					"json: unsupported type: complex128"
 				],
-				"type": "func(*testing.T, string) string",
-				"value": "`+fmt.Sprint(interface{}(indentJSON))+`"
+				"type": "complex128",
+				"value": "(10+10i)"
 			}
 		}`)
 	})
@@ -145,6 +145,24 @@ func TestMap(t *testing.T) {
 		}`)
 	})
 
+	t.Run("array", func(t *testing.T) {
+		t.Parallel()
+
+		test(t, slog.M(
+			slog.F("meow", [3]string{
+				"1",
+				"2",
+				"3",
+			}),
+		), `{
+			"meow": [
+				"1",
+				"2",
+				"3"
+			]
+		}`)
+	})
+
 	t.Run("forceJSON", func(t *testing.T) {
 		t.Parallel()
 
@@ -174,6 +192,54 @@ func TestMap(t *testing.T) {
 			"slice": null
 		}`)
 	})
+
+	t.Run("nil", func(t *testing.T) {
+		t.Parallel()
+
+		test(t, slog.M(
+			slog.F("val", nil),
+		), `{
+			"val": null
+		}`)
+	})
+
+	t.Run("json.Marshaler", func(t *testing.T) {
+		t.Parallel()
+
+		test(t, slog.M(
+			slog.F("val", time.Date(2000, 02, 05, 4, 4, 4, 0, time.UTC)),
+		), `{
+			"val": "2000-02-05T04:04:04Z"
+		}`)
+	})
+
+	t.Run("complex", func(t *testing.T) {
+		t.Parallel()
+
+		test(t, slog.M(
+			slog.F("val", complex(10, 10)),
+		), `{
+			"val": "(10+10i)"
+		}`)
+	})
+
+	t.Run("privateStruct", func(t *testing.T) {
+		t.Parallel()
+
+		test(t, slog.M(
+			slog.F("val", struct {
+				meow string
+				bar  int
+				far  uint
+			}{
+				meow: "hi",
+				bar:  23,
+				far:  600,
+			}),
+		), `{
+			"val": "{meow:hi bar:23 far:600}"
+		}`)
+	})
 }
 
 type meow struct {