MapObjectEncoder: Empty arrays should not be nil (uber-go#614)

This change fixes a bug in MapObjectEncoder.AddArray where an empty slice would log nil instead of an empty array. Logging an empty array makes the MapObjectEncoder's behavior correctly reflect the JSON encoder's.
cgxxv · Jul 31, 2018 · f57081d · f57081d
1 parent aa7ff99
commit f57081d
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 23 deletions.
diff --git 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/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
@@ -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
@@ -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
@@ -43,7 +43,7 @@ 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)
 	m.cur[key] = arr.elems
 	return err

diff --git 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")) },