From a75a883932d50a6c7241a3e37b31d65418920572 Mon Sep 17 00:00:00 2001 From: George Blue Date: Tue, 16 Jun 2020 16:03:42 +0100 Subject: [PATCH] fix: clarify behavior of empty vs nil slices --- marshal.go | 9 ++-- marshal_test.go | 36 ++++++++++---- unmarshal.go | 4 ++ unmarshal_test.go | 122 +++++++++++++++++++++++++++++++++++----------- 4 files changed, 130 insertions(+), 41 deletions(-) diff --git a/marshal.go b/marshal.go index 712a24b..3610ff5 100644 --- a/marshal.go +++ b/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 diff --git a/marshal_test.go b/marshal_test.go index 25577e2..363bcab 100644 --- a/marshal_test.go +++ b/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}}`) diff --git a/unmarshal.go b/unmarshal.go index 0b8829d..3f210d9 100644 --- a/unmarshal.go +++ b/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) diff --git a/unmarshal_test.go b/unmarshal_test.go index 7df6586..3f39aad 100644 --- a/unmarshal_test.go +++ b/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() {