From ceeb448f5ddc6b42593f2f9a100a8ea1aa5f5701 Mon Sep 17 00:00:00 2001 From: Fabio Forni Date: Wed, 26 Jan 2022 11:21:56 +0100 Subject: [PATCH 1/4] Fix marshaling with nil pointer fields --- marshal.go | 6 +++++- marshal_test.go | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/marshal.go b/marshal.go index f31e9e2..2139d0f 100644 --- a/marshal.go +++ b/marshal.go @@ -102,7 +102,7 @@ func marshalRecursive(mp map[string]string, prefix string, stru reflect.Value) e tags.name = field.Name } - for value.Kind() == reflect.Ptr { + for value.Kind() == reflect.Ptr && !value.IsNil() { value = value.Elem() } @@ -134,6 +134,10 @@ func structToMap(mp map[string]string, prefix string, stru reflect.Value) error } func fieldToString(val reflect.Value) (string, error) { + for val.Kind() == reflect.Ptr { + underlying := reflect.TypeOf(val.Interface()).Elem() + val = reflect.New(underlying).Elem() + } typ := val.Type() if typ.Implements(textMarshalerType) { str, err := val.Interface().(encoding.TextMarshaler).MarshalText() diff --git a/marshal_test.go b/marshal_test.go index a73b99c..3e7143a 100644 --- a/marshal_test.go +++ b/marshal_test.go @@ -251,3 +251,21 @@ func TestInnerMapMarshaler(t *testing.T) { t.Fatalf("Marshal's output doesn't respect struct tags\n\tExpected: %v\n\tOut: %v", expected, out) } } + +func TestMarshalNilField(t *testing.T) { + stru := struct { + Field *int + FieldOmit *int `redmap:",omitempty"` + FieldDouble **int + FieldOmitDouble **int `redmap:",omitempty"` + }{} + expected := map[string]string{"Field": "0", "FieldDouble": "0"} + out, err := redmap.Marshal(stru) + redmap.Marshal(stru) + if err != nil { + t.Fatalf("Marshal returned unexpected error %q", err) + } + if !reflect.DeepEqual(out, expected) { + t.Fatalf("Marshal's output doesn't respect struct tags\n\tExpected: %v\n\tOut: %v", expected, out) + } +} From 3998d7367ba689d3bad9c4d238d432e2c6e2a89b Mon Sep 17 00:00:00 2001 From: Fabio Forni Date: Wed, 26 Jan 2022 11:27:54 +0100 Subject: [PATCH 2/4] Rename tests consistently --- marshal_test.go | 4 ++-- unmarshal_test.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/marshal_test.go b/marshal_test.go index 3e7143a..c43a11c 100644 --- a/marshal_test.go +++ b/marshal_test.go @@ -211,7 +211,7 @@ func TestMarshalWithTags(t *testing.T) { } } -func TestMapMarshaler(t *testing.T) { +func TestMarshalMapMarshaler(t *testing.T) { tests := []struct { In redmap.StringMapMarshaler Out map[string]string @@ -230,7 +230,7 @@ func TestMapMarshaler(t *testing.T) { } } -func TestInnerMapMarshaler(t *testing.T) { +func TestMarshalInnerMapMarshaler(t *testing.T) { stru := struct { RegularField string Struct stubMapMarshaler `redmap:",inline"` diff --git a/unmarshal_test.go b/unmarshal_test.go index 6212311..89590f4 100644 --- a/unmarshal_test.go +++ b/unmarshal_test.go @@ -196,7 +196,7 @@ func TestUnmarshalInnerStructs(t *testing.T) { } } -func TestUnarshalUnexported(t *testing.T) { +func TestUnmarshalUnexported(t *testing.T) { mp := map[string]string{"Exp": "atest"} tests := []struct { Out interface{} @@ -253,7 +253,7 @@ func TestUnmarshalWithTags(t *testing.T) { } } -func TestMapUnmarshaler(t *testing.T) { +func TestUnmarshalMapUnmarshaler(t *testing.T) { intUn := stubIntMapUnmarshaler(666) tests := []struct { In map[string]string @@ -274,7 +274,7 @@ func TestMapUnmarshaler(t *testing.T) { } } -func TestInnerMapUnmarshaler(t *testing.T) { +func TestUnmarshalInnerMapUnmarshaler(t *testing.T) { tests := []struct { In map[string]string Out interface{} From 481c69378f346e714e93a9b5ebc8304c676edc2d Mon Sep 17 00:00:00 2001 From: Fabio Forni Date: Wed, 26 Jan 2022 11:50:59 +0100 Subject: [PATCH 3/4] Fix unmarshaling with nil pointer fields --- unmarshal.go | 2 +- unmarshal_test.go | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/unmarshal.go b/unmarshal.go index 8456a75..3e95ec0 100644 --- a/unmarshal.go +++ b/unmarshal.go @@ -78,7 +78,7 @@ func unmarshalRecursive(mp map[string]string, prefix string, stru reflect.Value) tags.name = prefix + tags.name for value.Kind() == reflect.Ptr { - if value.IsNil() { + if value.IsNil() && !tags.omitempty { if !value.CanSet() { return fmt.Errorf("cannot set embedded pointer to unexported type %s", value.Elem().Type()) } diff --git a/unmarshal_test.go b/unmarshal_test.go index 89590f4..259894c 100644 --- a/unmarshal_test.go +++ b/unmarshal_test.go @@ -99,14 +99,14 @@ func TestUnmarshalInvalidType(t *testing.T) { val: func() reflect.Value { return reflect.ValueOf(100) }, expErr: redmap.ErrNotPointer}, { - // Int is not a struct. + // Int is not a struct nor StringMapUnmarshaler. val: func() reflect.Value { v := 100 return reflect.ValueOf(&v) }, expErr: redmap.ErrNoCodec}, { - // Interface is not a struct. + // Interface is not a struct nor StringMapUnmarshaler. val: func() reflect.Value { v := fmt.Stringer(stubStringer{}) return reflect.ValueOf(&v) @@ -304,3 +304,31 @@ func TestUnmarshalInnerMapUnmarshaler(t *testing.T) { } } } + +func TestUnmarshalNilField(t *testing.T) { + var ( + zero = 0 + zeroDouble = &zero + ) + type stru struct { + Field *int + FieldOmit *int `redmap:",omitempty"` + FieldDouble **int + FieldOmitDouble **int `redmap:",omitempty"` + } + expected := stru{ + Field: &zero, + FieldOmit: nil, + FieldDouble: &zeroDouble, + FieldOmitDouble: nil, + } + mp := map[string]string{"Field": "0", "FieldDouble": "0"} + var out stru + err := redmap.Unmarshal(mp, &out) + if err != nil { + t.Fatalf("Unmarshal returned unexpected error %q", err) + } + if !reflect.DeepEqual(out, expected) { + t.Fatalf("Unmarshal's output doesn't match the expected value\n\tIn: %v\n\tExpected: %v\n\tOut: %v", mp, expected, out) + } +} From 942a2cfbb5448cc524bd269d6a53e9b00a27debf Mon Sep 17 00:00:00 2001 From: Fabio Forni Date: Wed, 26 Jan 2022 12:02:04 +0100 Subject: [PATCH 4/4] marshal.go: Document behavior when pointer field is nil --- marshal.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/marshal.go b/marshal.go index 2139d0f..b57ef43 100644 --- a/marshal.go +++ b/marshal.go @@ -19,8 +19,11 @@ type StringMapMarshaler interface { // from its method along with the error value. When not, Marshal reads every exported field and translates it // into a (key, value) pair to be added to the resulting map. Interfaces or pointers to struct are also accepted. // -// Marshal converts all fields of non-reference built-in types except arrays, plus +// Marshal converts all fields with built-in types except arrays, functions and channels, plus // structs implementing encoding.TextMarshaler or fmt.Stringer, checked in this exact order. +// If a field is a pointer to a supported type, the underlying type's value is marshaled. +// If the pointer is nil, it is marshaled as it had the underlying type's zero value unless `omitempty` +// is specified. // // The encoding of each struct field can be customized by the format string stored under the "redmap" // key in the struct field's tag. The format string gives the name of the field, possibly followed by @@ -33,13 +36,13 @@ type StringMapMarshaler interface { // // Field appears in the map as key "customName". // Field int `redmap:"customName"` // -// // Field appears in the map as key "customName" and -// // the field is omitted from the map if its value -// // is empty as defined by the Go language. +// // Field appears in the map as key "customName" unless +// // it has the zero value as defined by the Go specifications. +// // In such case, the field is not added to the map. // Field int `redmap:"customName,omitempty"` // // // Field appears in the map as key "Field" (the default), but -// // the field is skipped if empty. Note the leading comma. +// // the field is skipped if zero. Note the leading comma. // Field int `redmap:",omitempty"` // // // Field is ignored by this package.