fix: clarify behavior of empty vs nil slices

Author: blgm

marshal.go

@@ -88,16 +88,19 @@ func marshal(ctx context.Context, in reflect.Value) (r interface{}, err error) {
 	return
 }
 
-func marshalList(ctx context.Context, in reflect.Value) ([]interface{}, error) {
-	var out []interface{}
+func marshalList(ctx context.Context, in reflect.Value) (out []interface{}, err error) {
+	if in.Type().Kind() == reflect.Slice && in.IsNil() {
+		return out, nil
+	}
 
+	out = make([]interface{}, in.Len())
 	for i := 0; i < in.Len(); i++ {
 		ctx := ctx.WithIndex(i, in.Type())
 		r, err := marshal(ctx, in.Index(i))
 		if err != nil {
 			return nil, err
 		}
-		out = append(out, r)
+		out[i] = r
 	}
 
 	return out, nil

marshal_test.go

@@ -141,14 +141,6 @@ var _ = Describe("Marshal", func() {
 			expectToMarshal(struct{ P, p string }{P: "foo", p: "bar"}, `{"P":"foo"}`)
 		})
 
-		It("marshals a slice", func() {
-			s := []interface{}{"hello", true, 42}
-			expectToMarshal(struct{ S []interface{} }{S: s}, `{"S":["hello",true,42]}`)
-			expectToMarshal(struct {
-				S *[]interface{}
-			}{S: &s}, `{"S":["hello",true,42]}`)
-		})
-
 		It("marshals an array", func() {
 			s := [3]interface{}{"hello", true, 42}
 			expectToMarshal(struct{ S [3]interface{} }{S: s}, `{"S":["hello",true,42]}`)
@@ -157,8 +149,34 @@ var _ = Describe("Marshal", func() {
 			}{S: &s}, `{"S":["hello",true,42]}`)
 		})
 
+		Context("slices", func() {
+			It("marshals slices with interface{} values", func() {
+				s := []interface{}{"hello", true, 42}
+				expectToMarshal(struct{ S []interface{} }{S: s}, `{"S":["hello",true,42]}`)
+				expectToMarshal(struct{ S *[]interface{} }{S: &s}, `{"S":["hello",true,42]}`)
+			})
+
+			It("marshals slices with string values", func() {
+				s := []string{"hello", "true", "42"}
+				expectToMarshal(struct{ S []string }{S: s}, `{"S":["hello","true","42"]}`)
+				expectToMarshal(struct{ S *[]string }{S: &s}, `{"S":["hello","true","42"]}`)
+			})
+
+			It("marshals empty slices", func() {
+				s := make([]string, 0)
+				expectToMarshal(struct{ S []string }{S: s}, `{"S":[]}`)
+				expectToMarshal(struct{ S *[]string }{S: &s}, `{"S":[]}`)
+			})
+
+			It("marshals nil slices", func() {
+				var s []string
+				expectToMarshal(struct{ S []string }{S: s}, `{"S":null}`)
+				expectToMarshal(struct{ S *[]string }{S: &s}, `{"S":null}`)
+			})
+		})
+
 		Context("maps", func() {
-			It("marshals maps with interface values", func() {
+			It("marshals maps with interface{} values", func() {
 				mi := map[string]interface{}{"foo": "hello", "bar": true, "baz": 42}
 				expectToMarshal(struct{ M map[string]interface{} }{M: mi}, `{"M":{"foo":"hello","bar":true,"baz":42}}`)
 				expectToMarshal(struct{ M *map[string]interface{} }{M: &mi}, `{"M":{"foo":"hello","bar":true,"baz":42}}`)

unmarshal.go

@@ -167,6 +167,10 @@ func unmarshalIntoSlice(ctx context.Context, target reflect.Value, found bool, s
 		return nil
 	}
 
+	if source == nil {
+		return nil
+	}
+
 	src, ok := source.([]interface{})
 	if !ok {
 		return newConversionError(ctx, source)

unmarshal_test.go

@@ -214,36 +214,100 @@ var _ = Describe("Unmarshal", func() {
 			expectToFail(&s, `{"J":"foo"}`, `cannot unmarshal "foo" type "string" into field "J" (type "*int")`)
 		})
 
-		It("unmarshals into a slice field", func() {
-			var s struct {
-				S []string
-				N []int
-				I []interface{}
-				E []string
-			}
-			unmarshal(&s, `{"S":["a","b","c"],"N":[1,2,3],"I":["a",2,true]}`)
+		It("rejects an array field", func() {
+			var s struct{ S [3]string }
+			expectToFail(&s, `{}`, `unsupported type "[3]string" at field "S" (type "[3]string")`)
+		})
 
-			Expect(s).To(MatchAllFields(Fields{
-				"S": Equal([]string{"a", "b", "c"}),
-				"N": Equal([]int{1, 2, 3}),
-				"I": Equal([]interface{}{"a", 2, true}),
-				"E": BeEmpty(),
-			}))
+		Context("slices", func() {
+			It("unmarshals into slices of interface{}", func() {
+				By("slice", func() {
+					var s struct{ I []interface{} }
+					unmarshal(&s, `{"I": ["a",2,true]}`)
+					Expect(s.I).To(Equal([]interface{}{"a", 2, true}))
+				})
 
-			expectToFail(&s, `{"S":"foo"}`, `cannot unmarshal "foo" type "string" into field "S" (type "[]string")`)
-		})
+				By("pointer", func() {
+					var s struct{ I *[]interface{} }
+					unmarshal(&s, `{"I": ["a",2,true]}`)
+					Expect(s.I).To(PointTo(Equal([]interface{}{"a", 2, true})))
+				})
+			})
 
-		It("unmarshals into a slice pointer field", func() {
-			var s struct{ S *[]string }
-			unmarshal(&s, `{"S":["a","b","c"]}`)
-			Expect(s.S).To(PointTo(Equal([]string{"a", "b", "c"})))
+			It("unmarshals into slices of string", func() {
+				By("slice", func() {
+					var s struct{ S []string }
+					unmarshal(&s, `{"S":["a","b","c"]}`)
+					Expect(s.S).To(Equal([]string{"a", "b", "c"}))
+				})
 
-			expectToFail(&s, `{"S":"foo"}`, `cannot unmarshal "foo" type "string" into field "S" (type "*[]string")`)
-		})
+				By("pointer", func() {
+					var s struct{ S *[]string }
+					unmarshal(&s, `{"S":["a","b","c"]}`)
+					Expect(s.S).To(PointTo(Equal([]string{"a", "b", "c"})))
+				})
+			})
 
-		It("rejects an array field", func() {
-			var s struct{ S [3]string }
-			expectToFail(&s, `{}`, `unsupported type "[3]string" at field "S" (type "[3]string")`)
+			It("unmarshals into slices of int", func() {
+				By("slice", func() {
+					var s struct{ N []int }
+					unmarshal(&s, `{"N":[1,2,3]}`)
+					Expect(s.N).To(Equal([]int{1, 2, 3}))
+				})
+
+				By("pointer", func() {
+					var s struct{ N *[]int }
+					unmarshal(&s, `{"N":[1,2,3]}`)
+					Expect(s.N).To(PointTo(Equal([]int{1, 2, 3})))
+				})
+			})
+
+			It("unmarshals an omitted slice", func() {
+				By("slice", func() {
+					var s struct{ I []interface{} }
+					unmarshal(&s, `{}`)
+					Expect(s.I).To(BeNil())
+					Expect(s.I).To(BeEmpty())
+				})
+
+				By("pointer", func() {
+					var s struct{ I *[]interface{} }
+					unmarshal(&s, `{}`)
+					Expect(s.I).To(BeNil())
+				})
+			})
+
+			It("unmarshals a null slice", func() {
+				By("slice", func() {
+					var s struct{ I []interface{} }
+					unmarshal(&s, `{"I": null}`)
+					Expect(s.I).To(BeNil())
+					Expect(s.I).To(BeEmpty())
+				})
+
+				By("pointer", func() {
+					var s struct{ I *[]interface{} }
+					unmarshal(&s, `{"I": null}`)
+					Expect(s.I).To(BeNil())
+				})
+			})
+
+			It("unmarshals an empty slice", func() {
+				By("slice", func() {
+					var s struct{ I []interface{} }
+					unmarshal(&s, `{"I": []}`)
+					Expect(s.I).NotTo(BeNil())
+					Expect(s.I).To(BeEmpty())
+				})
+
+				By("pointer", func() {
+					var s struct{ I *[]interface{} }
+					unmarshal(&s, `{"I": []}`)
+					Expect(s.I).NotTo(BeNil())
+					Expect(s.I).To(PointTo(Not(BeNil())))
+					Expect(s.I).To(PointTo(BeEmpty()))
+				})
+			})
 		})
 
 		Context("maps", func() {
@@ -335,11 +399,11 @@ var _ = Describe("Unmarshal", func() {
 					Expect(s.I).To(PointTo(BeEmpty()))
 				})
 			})
-		})
 
-		It("rejects an map field that does not have string keys", func() {
-			var s struct{ S map[int]string }
-			expectToFail(&s, `{}`, `maps must only have string keys for "int" at field "S" (type "map[int]string")`)
+			It("rejects an map field that does not have string keys", func() {
+				var s struct{ S map[int]string }
+				expectToFail(&s, `{}`, `maps must only have string keys for "int" at field "S" (type "map[int]string")`)
+			})
 		})
 
 		It("unmarshals into json.Unmarshaler field", func() {