fix: clarify behavior of empty vs nil maps

cloudfoundry · Jun 16, 2020 · 3002a3d · 3002a3d
1 parent bc9f837
commit 3002a3d
Show file tree

Hide file tree

Showing 4 changed files with 121 additions and 44 deletions.
diff --git a/marshal.go b/marshal.go
@@ -103,9 +103,12 @@ func marshalList(ctx context.Context, in reflect.Value) ([]interface{}, error) {
 	return out, nil
 }
 
-func marshalMap(ctx context.Context, in reflect.Value) (map[string]interface{}, error) {
-	out := make(map[string]interface{})
+func marshalMap(ctx context.Context, in reflect.Value) (out map[string]interface{}, err error) {
+	if in.IsNil() {
+		return out, nil
+	}
 
+	out = make(map[string]interface{})
 	iter := in.MapRange()
 	for iter.Next() {
 		k := iter.Key()

diff --git a/marshal_test.go b/marshal_test.go
@@ -157,17 +157,34 @@ var _ = Describe("Marshal", func() {
 			}{S: &s}, `{"S":["hello",true,42]}`)
 		})
 
-		It("marshals a map", func() {
-			mi := map[string]interface{}{"foo": "hello", "bar": true, "baz": 42}
-			ms := map[string]string{"foo": "hello", "bar": "true", "baz": "42"}
-			mn := map[int]interface{}{4: 3}
-
-			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}}`)
-			expectToMarshal(struct{ M map[string]string }{M: ms}, `{"M":{"foo":"hello","bar":"true","baz":"42"}}`)
-			expectToMarshal(struct{ M *map[string]string }{M: &ms}, `{"M":{"foo":"hello","bar":"true","baz":"42"}}`)
-
-			expectToFail(struct{ M map[int]interface{} }{M: mn}, `maps must only have string keys for "map[int]interface {}" at field "M" (type "map[int]interface {}")`)
+		Context("maps", 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}}`)
+			})
+
+			It("marshals maps with string values", func() {
+				ms := map[string]string{"foo": "hello", "bar": "true", "baz": "42"}
+				expectToMarshal(struct{ M map[string]string }{M: ms}, `{"M":{"foo":"hello","bar":"true","baz":"42"}}`)
+				expectToMarshal(struct{ M *map[string]string }{M: &ms}, `{"M":{"foo":"hello","bar":"true","baz":"42"}}`)
+			})
+
+			It("marshals empty maps", func() {
+				me := make(map[string]string)
+				expectToMarshal(struct{ M map[string]string }{M: me}, `{"M":{}}`)
+				expectToMarshal(struct{ M *map[string]string }{M: &me}, `{"M":{}}`)
+			})
+
+			It("marshals nil maps", func() {
+				expectToMarshal(struct{ M map[string]string }{}, `{"M":null}`)
+				expectToMarshal(struct{ M *map[string]string }{}, `{"M":null}`)
+			})
+
+			It("fails with invalid keys", func() {
+				mn := map[int]interface{}{4: 3}
+				expectToFail(struct{ M map[int]interface{} }{M: mn}, `maps must only have string keys for "map[int]interface {}" at field "M" (type "map[int]interface {}")`)
+			})
 		})
 
 		It("marshals a json.Marshaler", func() {

diff --git a/unmarshal.go b/unmarshal.go
@@ -196,6 +196,10 @@ func unmarshalIntoMap(ctx context.Context, target reflect.Value, found bool, sou
 		return nil
 	}
 
+	if source == nil {
+		return nil
+	}
+
 	src, ok := source.(map[string]interface{})
 	if !ok {
 		return newConversionError(ctx, source)

diff --git a/unmarshal_test.go b/unmarshal_test.go
@@ -246,42 +246,95 @@ var _ = Describe("Unmarshal", func() {
 			expectToFail(&s, `{}`, `unsupported type "[3]string" at field "S" (type "[3]string")`)
 		})
 
-		It("unmarshals into a map field", func() {
-			var s struct {
-				S map[string]string
-				N map[string]int
-				I map[string]interface{}
-				E map[string]string
-			}
-			unmarshal(&s, `{"S":{"a":"b","c":"d"},"N":{"f":5},"I":{"a":"b","c":5,"d":true}}`)
+		Context("maps", func() {
+			It("unmarshals maps with interface values", func() {
+				By("map", func() {
+					var s struct{ I map[string]interface{} }
+					unmarshal(&s, `{"I":{"a":"b","c":5,"d":true}}`)
+					Expect(s.I).To(Equal(map[string]interface{}{"a": "b", "c": 5, "d": true}))
+				})
+
+				By("pointer", func() {
+					var s struct{ I *map[string]interface{} }
+					unmarshal(&s, `{"I":{"a":"b","c":5,"d":true}}`)
+					Expect(s.I).To(PointTo(Equal(map[string]interface{}{"a": "b", "c": 5, "d": true})))
+				})
+			})
 
-			Expect(s).To(MatchAllFields(Fields{
-				"S": Equal(map[string]string{"a": "b", "c": "d"}),
-				"N": Equal(map[string]int{"f": 5}),
-				"I": Equal(map[string]interface{}{"a": "b", "c": 5, "d": true}),
-				"E": BeEmpty(),
-			}))
+			It("unmarshals maps with string values", func() {
+				By("map", func() {
+					var s struct{ S map[string]string }
+					unmarshal(&s, `{"S":{"a":"b","c":"d"}}`)
+					Expect(s.S).To(Equal(map[string]string{"a": "b", "c": "d"}))
+				})
+
+				By("pointer", func() {
+					var s struct{ S *map[string]string }
+					unmarshal(&s, `{"S":{"a":"b","c":"d"}}`)
+					Expect(s.S).To(PointTo(Equal(map[string]string{"a": "b", "c": "d"})))
+				})
+			})
 
-			expectToFail(&s, `{"S":"foo"}`, `cannot unmarshal "foo" type "string" into field "S" (type "map[string]string")`)
-		})
+			It("unmarshals maps with number values", func() {
+				By("map", func() {
+					var s struct{ N map[string]int }
+					unmarshal(&s, `{"N":{"f":5}}`)
+					Expect(s.N).To(Equal(map[string]int{"f": 5}))
+				})
+
+				By("pointer", func() {
+					var s struct{ N *map[string]int }
+					unmarshal(&s, `{"N":{"f":5}}`)
+					Expect(s.N).To(PointTo(Equal(map[string]int{"f": 5})))
+				})
+			})
 
-		It("unmarshals into a map pointer field", func() {
-			var s struct {
-				S *map[string]string
-				N *map[string]int
-				I *map[string]interface{}
-				E *map[string]string
-			}
-			unmarshal(&s, `{"S":{"a":"b","c":"d"},"N":{"f":5},"I":{"a":"b","c":5,"d":true}}`)
+			It("unmarshals omitted maps", func() {
+				By("map", func() {
+					var s struct{ I map[string]interface{} }
+					unmarshal(&s, `{}`)
+					Expect(s.I).To(BeNil())
+					Expect(s.I).To(BeEmpty())
+				})
+
+				By("pointer", func() {
+					var s struct{ I *map[string]interface{} }
+					unmarshal(&s, `{}`)
+					Expect(s.I).To(BeNil())
+				})
+			})
 
-			Expect(s).To(MatchAllFields(Fields{
-				"S": PointTo(Equal(map[string]string{"a": "b", "c": "d"})),
-				"N": PointTo(Equal(map[string]int{"f": 5})),
-				"I": PointTo(Equal(map[string]interface{}{"a": "b", "c": 5, "d": true})),
-				"E": BeNil(),
-			}))
+			It("unmarshals null maps", func() {
+				By("map", func() {
+					var s struct{ I map[string]interface{} }
+					unmarshal(&s, `{"I": null}`)
+					Expect(s.I).To(BeNil())
+					Expect(s.I).To(BeEmpty())
+				})
+
+				By("pointer", func() {
+					var s struct{ I *map[string]interface{} }
+					unmarshal(&s, `{"I": null}`)
+					Expect(s.I).To(BeNil())
+				})
+			})
 
-			expectToFail(&s, `{"S":"foo"}`, `cannot unmarshal "foo" type "string" into field "S" (type "*map[string]string")`)
+			It("unmarshals empty maps", func() {
+				By("map", func() {
+					var s struct{ I map[string]interface{} }
+					unmarshal(&s, `{"I": {}}`)
+					Expect(s.I).NotTo(BeNil())
+					Expect(s.I).To(BeEmpty())
+				})
+
+				By("pointer", func() {
+					var s struct{ I *map[string]interface{} }
+					unmarshal(&s, `{"I": {}}`)
+					Expect(s.I).NotTo(BeNil())
+					Expect(s.I).To(PointTo(Not(BeNil())))
+					Expect(s.I).To(PointTo(BeEmpty()))
+				})
+			})
 		})
 
 		It("rejects an map field that does not have string keys", func() {