From 307d065cc035f277ad8272d543bde212f2c7181f Mon Sep 17 00:00:00 2001 From: Minho Park Date: Mon, 30 Jul 2018 13:46:28 -0700 Subject: [PATCH 1/2] Replace null with empty slices for MapObjectEncoder AddArray --- array_test.go | 40 +++++++++++++++++----------------- error_test.go | 2 +- zapcore/field_test.go | 2 +- zapcore/memory_encoder.go | 3 +++ zapcore/memory_encoder_test.go | 7 ++++++ 5 files changed, 32 insertions(+), 22 deletions(-) diff --git a/array_test.go b/array_test.go index 54116dfbb..4f709f09c 100644 --- a/array_test.go +++ b/array_test.go @@ -55,26 +55,26 @@ func TestArrayWrappers(t *testing.T) { field Field expected []interface{} }{ - {"empty bools", Bools("", []bool{}), []interface{}(nil)}, - {"empty byte strings", ByteStrings("", [][]byte{}), []interface{}(nil)}, - {"empty complex128s", Complex128s("", []complex128{}), []interface{}(nil)}, - {"empty complex64s", Complex64s("", []complex64{}), []interface{}(nil)}, - {"empty durations", Durations("", []time.Duration{}), []interface{}(nil)}, - {"empty float64s", Float64s("", []float64{}), []interface{}(nil)}, - {"empty float32s", Float32s("", []float32{}), []interface{}(nil)}, - {"empty ints", Ints("", []int{}), []interface{}(nil)}, - {"empty int64s", Int64s("", []int64{}), []interface{}(nil)}, - {"empty int32s", Int32s("", []int32{}), []interface{}(nil)}, - {"empty int16s", Int16s("", []int16{}), []interface{}(nil)}, - {"empty int8s", Int8s("", []int8{}), []interface{}(nil)}, - {"empty strings", Strings("", []string{}), []interface{}(nil)}, - {"empty times", Times("", []time.Time{}), []interface{}(nil)}, - {"empty uints", Uints("", []uint{}), []interface{}(nil)}, - {"empty uint64s", Uint64s("", []uint64{}), []interface{}(nil)}, - {"empty uint32s", Uint32s("", []uint32{}), []interface{}(nil)}, - {"empty uint16s", Uint16s("", []uint16{}), []interface{}(nil)}, - {"empty uint8s", Uint8s("", []uint8{}), []interface{}(nil)}, - {"empty uintptrs", Uintptrs("", []uintptr{}), []interface{}(nil)}, + {"empty bools", Bools("", []bool{}), []interface{}{}}, + {"empty byte strings", ByteStrings("", [][]byte{}), []interface{}{}}, + {"empty complex128s", Complex128s("", []complex128{}), []interface{}{}}, + {"empty complex64s", Complex64s("", []complex64{}), []interface{}{}}, + {"empty durations", Durations("", []time.Duration{}), []interface{}{}}, + {"empty float64s", Float64s("", []float64{}), []interface{}{}}, + {"empty float32s", Float32s("", []float32{}), []interface{}{}}, + {"empty ints", Ints("", []int{}), []interface{}{}}, + {"empty int64s", Int64s("", []int64{}), []interface{}{}}, + {"empty int32s", Int32s("", []int32{}), []interface{}{}}, + {"empty int16s", Int16s("", []int16{}), []interface{}{}}, + {"empty int8s", Int8s("", []int8{}), []interface{}{}}, + {"empty strings", Strings("", []string{}), []interface{}{}}, + {"empty times", Times("", []time.Time{}), []interface{}{}}, + {"empty uints", Uints("", []uint{}), []interface{}{}}, + {"empty uint64s", Uint64s("", []uint64{}), []interface{}{}}, + {"empty uint32s", Uint32s("", []uint32{}), []interface{}{}}, + {"empty uint16s", Uint16s("", []uint16{}), []interface{}{}}, + {"empty uint8s", Uint8s("", []uint8{}), []interface{}{}}, + {"empty uintptrs", Uintptrs("", []uintptr{}), []interface{}{}}, {"bools", Bools("", []bool{true, false}), []interface{}{true, false}}, {"byte strings", ByteStrings("", [][]byte{{1, 2}, {3, 4}}), []interface{}{[]byte{1, 2}, []byte{3, 4}}}, {"complex128s", Complex128s("", []complex128{1 + 2i, 3 + 4i}), []interface{}{1 + 2i, 3 + 4i}}, diff --git a/error_test.go b/error_test.go index ad3418002..674b1596a 100644 --- a/error_test.go +++ b/error_test.go @@ -61,7 +61,7 @@ func TestErrorArrayConstructor(t *testing.T) { field Field expected []interface{} }{ - {"empty errors", Errors("", []error{}), []interface{}(nil)}, + {"empty errors", Errors("", []error{}), []interface{}{}}, { "errors", Errors("", []error{nil, errors.New("foo"), nil, errors.New("bar")}), diff --git a/zapcore/field_test.go b/zapcore/field_test.go index fb722b4b7..9a5fe0189 100644 --- a/zapcore/field_test.go +++ b/zapcore/field_test.go @@ -71,7 +71,7 @@ func TestFieldAddingError(t *testing.T) { t FieldType want interface{} }{ - {ArrayMarshalerType, []interface{}(nil)}, + {ArrayMarshalerType, []interface{}{}}, {ObjectMarshalerType, map[string]interface{}{}}, } for _, tt := range tests { diff --git a/zapcore/memory_encoder.go b/zapcore/memory_encoder.go index 5c46bc13d..f793f34e8 100644 --- a/zapcore/memory_encoder.go +++ b/zapcore/memory_encoder.go @@ -45,6 +45,9 @@ func NewMapObjectEncoder() *MapObjectEncoder { func (m *MapObjectEncoder) AddArray(key string, v ArrayMarshaler) error { arr := &sliceArrayEncoder{} err := v.MarshalLogArray(arr) + if arr.elems == nil { + arr.elems = []interface{}{} + } m.cur[key] = arr.elems return err } diff --git a/zapcore/memory_encoder_test.go b/zapcore/memory_encoder_test.go index 4523a0e11..5ca9577ae 100644 --- a/zapcore/memory_encoder_test.go +++ b/zapcore/memory_encoder_test.go @@ -75,6 +75,13 @@ func TestMapObjectEncoderAdd(t *testing.T) { }, expected: []interface{}{wantTurducken, wantTurducken}, }, + { + desc: "AddArray (empty)", + f: func(e ObjectEncoder) { + assert.NoError(t, e.AddArray("k", turduckens(0)), "Expected AddArray to succeed.") + }, + expected: []interface{}{}, + }, { desc: "AddBinary", f: func(e ObjectEncoder) { e.AddBinary("k", []byte("foo")) }, From 9e0ecf57c78f10b9d004a461a457d574cad61bb4 Mon Sep 17 00:00:00 2001 From: Minho Park Date: Mon, 30 Jul 2018 15:06:41 -0700 Subject: [PATCH 2/2] Add change to changelog, reflect code review --- CHANGELOG.md | 6 ++++++ zapcore/memory_encoder.go | 5 +---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c84f50aea..35c8bcf90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## Unreleased + +Bugfixes: +* [#614][]: MapObjectEncoder should not ignore empty slices. + ## v1.9.0 (19 Jul 2018) Enhancements: @@ -296,3 +301,4 @@ upgrade to the upcoming stable release. [#602]: https://github.com/uber-go/zap/pull/602 [#572]: https://github.com/uber-go/zap/pull/572 [#606]: https://github.com/uber-go/zap/pull/606 +[#614]: https://github.com/uber-go/zap/pull/614 diff --git a/zapcore/memory_encoder.go b/zapcore/memory_encoder.go index f793f34e8..6ef85b09c 100644 --- a/zapcore/memory_encoder.go +++ b/zapcore/memory_encoder.go @@ -43,11 +43,8 @@ func NewMapObjectEncoder() *MapObjectEncoder { // AddArray implements ObjectEncoder. func (m *MapObjectEncoder) AddArray(key string, v ArrayMarshaler) error { - arr := &sliceArrayEncoder{} + arr := &sliceArrayEncoder{elems: make([]interface{}, 0)} err := v.MarshalLogArray(arr) - if arr.elems == nil { - arr.elems = []interface{}{} - } m.cur[key] = arr.elems return err }