From 71749cb9ffe007906f39e0144529393de652f06f Mon Sep 17 00:00:00 2001
From: George Blue <gblue@pivotal.io>
Date: Fri, 22 May 2020 14:15:48 +0100
Subject: [PATCH] fix(omitempty): must omit empty slices

The `omitempty` tag was omitting `nil` slices but not slices that were
allocated but had length zero. This has been fixes for consistency with
json.Marshal. Also empty structs are no longer omitted for consistency
with json.Marhsal.
---
 marshal.go      | 16 +++++++++-
 marshal_test.go | 85 +++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 84 insertions(+), 17 deletions(-)

diff --git a/marshal.go b/marshal.go
index 280e77d..9f9cbd9 100644
--- a/marshal.go
+++ b/marshal.go
@@ -44,7 +44,7 @@ func marshalStruct(ctx context.Context, in reflect.Value) (map[string]interface{
 
 		if public(f) {
 			p := path.ComputePath(f)
-			shouldSkip := p.OmitAlways || (p.OmitEmpty && in.Field(i).IsZero())
+			shouldSkip := p.OmitAlways || (p.OmitEmpty && isEmpty(in.Field(i)))
 
 			if !shouldSkip {
 				r, err := marshal(ctx.WithField(f.Name, f.Type), in.Field(i))
@@ -138,3 +138,17 @@ func marshalJSONMarshaler(ctx context.Context, in reflect.Value) (interface{}, e
 
 	return r, nil
 }
+
+func isEmpty(v reflect.Value) bool {
+	k := v.Kind()
+	switch {
+	case k == reflect.Interface, k == reflect.Ptr:
+		return v.IsZero() || v.IsNil()
+	case k == reflect.String, k == reflect.Map, k == reflect.Slice, k == reflect.Array:
+		return v.Len() == 0
+	case basicType(k):
+		return v.IsZero()
+	default:
+		return false
+	}
+}
diff --git a/marshal_test.go b/marshal_test.go
index ce23ac9..9253afc 100644
--- a/marshal_test.go
+++ b/marshal_test.go
@@ -1,6 +1,7 @@
 package jsonry_test
 
 import (
+	"encoding/json"
 	"errors"
 
 	"code.cloudfoundry.org/jsonry"
@@ -209,50 +210,102 @@ var _ = Describe("Marshal", func() {
 	})
 
 	Describe("omitempty", func() {
-		It("omits zero values of basic types", func() {
+		It("reads the `omitempty` field from JSON and JSONry tags with and without names", func() {
 			s := struct {
 				A string `json:",omitempty"`
 				B string `json:"bee,omitempty"`
 				C string `jsonry:",omitempty"`
 				D string `jsonry:"dee,omitempty"`
-				E string
 			}{}
-			expectToMarshal(s, `{"E":""}`)
+			expectToMarshal(s, `{}`)
+		})
+
+		//		and any empty array, slice, map, or string.
+
+		It("omits false", func() {
+			s := struct {
+				A bool `jsonry:",omitempty"`
+			}{}
+			expectToMarshal(s, `{}`)
+		})
+
+		It("omits 0", func() {
+			s := struct {
+				A int     `jsonry:",omitempty"`
+				B uint    `jsonry:",omitempty"`
+				C float64 `jsonry:",omitempty"`
+			}{}
+			expectToMarshal(s, `{}`)
 		})
 
-		It("omits zero value structs", func() {
+		It("omits nil pointers", func() {
 			type t struct{ A string }
 			s := struct {
-				B t `jsonry:",omitempty"`
+				A *string      `jsonry:",omitempty"`
+				B *t           `jsonry:",omitempty"`
+				C *[]string    `jsonry:",omitempty"`
+				D *map[int]int `jsonry:",omitempty"`
+				E *interface{} `jsonry:",omitempty"`
 			}{}
 			expectToMarshal(s, `{}`)
 		})
 
-		It("omits empty lists", func() {
+		It("omits nil interface values", func() {
 			s := struct {
-				A []string  `jsonry:",omitempty"`
-				D [0]string `jsonry:",omitempty"`
+				A interface{}    `jsonry:",omitempty"`
+				B json.Marshaler `jsonry:",omitempty"`
 			}{}
 			expectToMarshal(s, `{}`)
 		})
 
+		It("omits empty arrays", func() {
+			s := struct {
+				A [0]int `jsonry:",omitempty"`
+			}{A: [0]int{}}
+			expectToMarshal(s, `{}`)
+		})
+
+		It("omits empty slices", func() {
+			s := struct {
+				A []int `jsonry:",omitempty"`
+				B []int `jsonry:",omitempty"`
+				C []int `jsonry:",omitempty"`
+			}{
+				B: []int{},
+				C: make([]int, 0, 1),
+			}
+			expectToMarshal(s, `{}`)
+		})
+
 		It("omits empty maps", func() {
 			s := struct {
 				A map[interface{}]interface{} `jsonry:",omitempty"`
 				D map[int]int                 `jsonry:",omitempty"`
-			}{}
+			}{
+				D: make(map[int]int),
+			}
 			expectToMarshal(s, `{}`)
 		})
 
-		It("omits nil pointers", func() {
+		It("omits empty strings", func() {
 			s := struct {
-				A *string `json:",omitempty"`
-				B *string `json:"bee,omitempty"`
-				C *string `jsonry:",omitempty"`
-				D *string `jsonry:"dee,omitempty"`
-				E *string
+				A string `jsonry:",omitempty"`
+				B string `jsonry:",omitempty"`
+			}{
+				B: "",
+			}
+			expectToMarshal(s, `{}`)
+		})
+
+		// For consistency with json.Marshal, see https://github.com/golang/go/issues/11939
+		It("does not omit empty structs", func() {
+			type t struct {
+				A string `jsonry:",omitempty"`
+			}
+			s := struct {
+				B t `jsonry:",omitempty"`
 			}{}
-			expectToMarshal(s, `{"E":null}`)
+			expectToMarshal(s, `{"B":{}}`)
 		})
 	})