Skip to content

Commit d9f1cd9

Browse files
nhooyrnhooyr
committed
Remove Value interface and ForceJSON
Realized we can replace ForceJSON now that we only marshal structs as JSON if they have a json struct tag and json.Marshaller can be used in place of Value as well so its not necessary.
1 parent a13a242 commit d9f1cd9

File tree

8 files changed

+68
-162
lines changed

8 files changed

+68
-162
lines changed

‎example_forcejson_test.go

Lines changed: 0 additions & 30 deletions
This file was deleted.

‎example_value_test.go

Lines changed: 0 additions & 32 deletions
This file was deleted.

‎go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ require (
66
cloud.google.com/go v0.43.0
77
github.com/alecthomas/chroma v0.6.6
88
github.com/fatih/color v1.7.0
9+
github.com/google/go-cmp v0.3.2-0.20191216165815-3838af334ff4
910
github.com/kylelemons/godebug v1.1.0
1011
github.com/mattn/go-colorable v0.1.2 // indirect
1112
github.com/mattn/go-isatty v0.0.9 // indirect

‎go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ
4848
github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M=
4949
github.com/google/go-cmp v0.3.0 h1:crn/baboCvb5fXaQ0IJ1SGTsTVrWpDsCWC8EGETZijY=
5050
github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
51+
github.com/google/go-cmp v0.3.2-0.20191216165815-3838af334ff4 h1:OQgDOEA9XvIORxecFMQtjbuB+C726/UOre43n3HgLsU=
52+
github.com/google/go-cmp v0.3.2-0.20191216165815-3838af334ff4/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
5153
github.com/google/martian v2.1.0+incompatible h1:/CP5g8u/VJHijgedC/Legn3BAbAaWPgecwXBIDzw5no=
5254
github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs=
5355
github.com/google/pprof v0.0.0-20181206194817-3ea8567a2e57/go.mod h1:zfwlbNMJ+OItoe0UupaVj+oy1omPYYDuagoSzA8v9mc=

‎internal/assert/assert.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,17 @@ import (
55
"reflect"
66
"testing"
77

8-
"github.com/kylelemons/godebug/pretty"
8+
"github.com/google/go-cmp/cmp"
99
)
1010

1111
// Equal asserts exp == act.
1212
func Equal(t testing.TB, name string, exp, act interface{}) {
1313
t.Helper()
14-
if !reflect.DeepEqual(exp, act) {
14+
if diff := cmp.Diff(exp, act, cmp.Exporter(func(r reflect.Type) bool {
15+
return true
16+
})); diff != "" {
1517
t.Fatalf(`unexpected %v: diff:
16-
%v`, name, pretty.Compare(exp, act))
18+
%v`, name, diff)
1719
}
1820
}
1921

‎map.go

Lines changed: 50 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,6 @@ import (
1313
// Map represents an ordered map of fields.
1414
type Map []Field
1515

16-
// SlogValue implements Value.
17-
func (m Map) SlogValue() interface{} {
18-
return ForceJSON(m)
19-
}
20-
2116
var _ json.Marshaler = Map(nil)
2217

2318
// MarshalJSON implements json.Marshaler.
@@ -27,23 +22,20 @@ var _ json.Marshaler = Map(nil)
2722
//
2823
// Every field value is encoded with the following process:
2924
//
30-
// 1. slog.Value is checked to allow any type to replace its representation for logging.
31-
//
32-
// 2. json.Marshaller is handled.
25+
// 1. json.Marshaller is handled.
3326
//
3427
// 2. xerrors.Formatter is handled.
3528
//
36-
// 3. Protobufs are encoded with json.Marshal.
29+
// 3. structs that have a field with a json tag are encoded with json.Marshal.
3730
//
38-
// 4. error and fmt.Stringer are used if possible.
31+
// 4. error and fmt.Stringer is handled.
3932
//
4033
// 5. slices and arrays go through the encode function for every element.
4134
//
42-
// 6. If the value is a struct without exported fields or a type that
43-
// cannot be encoded with json.Marshal (like channels) then
44-
// fmt.Sprintf("%+v") is used.
35+
// 6. If the value cannot be encoded directly with json.Marshal (like channels)
36+
// then fmt.Sprintf("%+v") is used.
4537
//
46-
// 8. json.Marshal(v) is used for all other values.
38+
// 7. json.Marshal(v) is used for all other values.
4739
func (m Map) MarshalJSON() ([]byte, error) {
4840
b := &bytes.Buffer{}
4941
b.WriteByte('{')
@@ -62,19 +54,6 @@ func (m Map) MarshalJSON() ([]byte, error) {
6254
return b.Bytes(), nil
6355
}
6456

65-
// ForceJSON ensures the value is logged via json.Marshal.
66-
//
67-
// Use it when implementing SlogValue to ensure a structured
68-
// representation of a struct if you know it's capable even
69-
// when it implements fmt.Stringer or error.
70-
func ForceJSON(v interface{}) interface{} {
71-
return jsonVal{v: v}
72-
}
73-
74-
type jsonVal struct {
75-
v interface{}
76-
}
77-
7857
func marshalList(rv reflect.Value) []byte {
7958
b := &bytes.Buffer{}
8059
b.WriteByte('[')
@@ -93,51 +72,61 @@ func marshalList(rv reflect.Value) []byte {
9372

9473
func encode(v interface{}) []byte {
9574
switch v := v.(type) {
96-
case Value:
97-
return encode(v.SlogValue())
9875
case json.Marshaler:
9976
return encodeJSON(v)
100-
case jsonVal:
101-
return encodeJSON(v.v)
10277
case xerrors.Formatter:
10378
return encode(errorChain(v))
104-
case interface {
105-
ProtoMessage()
106-
}:
107-
return encode(ForceJSON(v))
79+
}
80+
81+
rv := reflect.Indirect(reflect.ValueOf(v))
82+
if !rv.IsValid() {
83+
return encodeJSON(v)
84+
}
85+
86+
if rv.Kind() == reflect.Struct {
87+
b, ok := encodeStruct(rv)
88+
if ok {
89+
return b
90+
}
91+
}
92+
93+
switch v.(type) {
10894
case error, fmt.Stringer:
10995
return encode(fmt.Sprint(v))
110-
default:
111-
rv := reflect.Indirect(reflect.ValueOf(v))
112-
if !rv.IsValid() {
113-
return encodeJSON(v)
114-
}
96+
}
11597

116-
switch rv.Type().Kind() {
117-
case reflect.Slice:
118-
if !rv.IsNil() {
119-
return marshalList(rv)
120-
}
121-
case reflect.Array:
98+
switch rv.Type().Kind() {
99+
case reflect.Slice:
100+
if !rv.IsNil() {
122101
return marshalList(rv)
123-
case reflect.Struct:
124-
typ := rv.Type()
125-
for i := 0; i < rv.NumField(); i++ {
126-
// Found an exported field.
127-
if typ.Field(i).PkgPath == "" {
128-
return encodeJSON(v)
129-
}
130-
}
131-
132-
return encodeJSON(fmt.Sprintf("%+v", v))
133-
case reflect.Chan, reflect.Complex64, reflect.Complex128, reflect.Func:
134-
// These types cannot be directly encoded with json.Marshal.
135-
// See https://golang.org/pkg/encoding/json/#Marshal
136-
return encodeJSON(fmt.Sprintf("%+v", v))
137102
}
103+
case reflect.Array:
104+
return marshalList(rv)
105+
case reflect.Struct, reflect.Chan, reflect.Complex64, reflect.Complex128, reflect.Func:
106+
// These types cannot be directly encoded with json.Marshal.
107+
// See https://golang.org/pkg/encoding/json/#Marshal
108+
return encodeJSON(fmt.Sprintf("%+v", v))
109+
}
138110

139-
return encodeJSON(v)
111+
return encodeJSON(v)
112+
}
113+
114+
func encodeStruct(rv reflect.Value) ([]byte, bool) {
115+
if rv.Kind() != reflect.Struct {
116+
return nil, false
140117
}
118+
119+
if rv.Kind() == reflect.Struct {
120+
for i := 0; i < rv.NumField(); i++ {
121+
ft := rv.Type().Field(i)
122+
// Found a field with a json tag.
123+
if ft.Tag.Get("json") != "" {
124+
return encodeJSON(rv.Interface()), true
125+
}
126+
}
127+
}
128+
129+
return nil, false
141130
}
142131

143132
func encodeJSON(v interface{}) []byte {

‎map_test.go

Lines changed: 10 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -86,18 +86,18 @@ func TestMap(t *testing.T) {
8686
mapTestFile := strings.Replace(mapTestFile, "_test", "", 1)
8787

8888
test(t, slog.M(
89-
slog.F("meow", slog.ForceJSON(complex(10, 10))),
89+
slog.F("meow", complexJSON(complex(10, 10))),
9090
), `{
9191
"meow": {
9292
"error": [
9393
{
9494
"msg": "failed to marshal to JSON",
9595
"fun": "cdr.dev/slog.encodeJSON",
96-
"loc": "`+mapTestFile+`:147"
96+
"loc": "`+mapTestFile+`:136"
9797
},
98-
"json: unsupported type: complex128"
98+
"json: error calling MarshalJSON for type slog_test.complexJSON: json: unsupported type: complex128"
9999
],
100-
"type": "complex128",
100+
"type": "slog_test.complexJSON",
101101
"value": "(10+10i)"
102102
}
103103
}`)
@@ -163,26 +163,6 @@ func TestMap(t *testing.T) {
163163
}`)
164164
})
165165

166-
t.Run("forceJSON", func(t *testing.T) {
167-
t.Parallel()
168-
169-
test(t, slog.M(
170-
slog.F("error", slog.ForceJSON(io.EOF)),
171-
), `{
172-
"error": {}
173-
}`)
174-
})
175-
176-
t.Run("value", func(t *testing.T) {
177-
t.Parallel()
178-
179-
test(t, slog.M(
180-
slog.F("error", meow{1}),
181-
), `{
182-
"error": "xdxd"
183-
}`)
184-
})
185-
186166
t.Run("nilSlice", func(t *testing.T) {
187167
t.Parallel()
188168

@@ -246,10 +226,6 @@ type meow struct {
246226
a int
247227
}
248228

249-
func (m meow) SlogValue() interface{} {
250-
return "xdxd"
251-
}
252-
253229
func indentJSON(t *testing.T, j string) string {
254230
b := &bytes.Buffer{}
255231
err := json.Indent(b, []byte(j), "", strings.Repeat(" ", 4))
@@ -263,3 +239,9 @@ func marshalJSON(t *testing.T, m slog.Map) string {
263239
assert.Success(t, "marshal map to JSON", err)
264240
return indentJSON(t, string(actb))
265241
}
242+
243+
type complexJSON complex128
244+
245+
func (c complexJSON) MarshalJSON() ([]byte, error) {
246+
return json.Marshal(complex128(c))
247+
}

‎slog.go

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -226,14 +226,6 @@ func M(fs ...Field) Map {
226226
return fs
227227
}
228228

229-
// Value represents a log value.
230-
//
231-
// Implement SlogValue in your own types to override
232-
// the value encoded when logging.
233-
type Value interface {
234-
SlogValue() interface{}
235-
}
236-
237229
// Error is the standard key used for logging a Go error value.
238230
func Error(err error) Field {
239231
return F("error", err)

0 commit comments

Comments
 (0)