Add option ZeroAsNull (Fixes: #30)

[dionysius]

Since EmptyAsNull got introduced in v1.1 with its framework, adding this feature was quite straightforward.

The slight performance toll of using reflect should only happen when actually opting in ZeroAsNull and I think there will be no better way of doing it without knowing the types beforehand.

Definition of omitempty:

Struct values encode as JSON objects. Each exported struct field becomes a member of the object, using the field name as the object key, unless the field is omitted for one of the reasons given below.

The encoding of each struct field can be customized by the format string stored under the "json" key in the struct field's tag. The format string gives the name of the field, possibly followed by a comma-separated list of options. The name may be empty in order to specify options without overriding the default field name.

The "omitempty" option specifies that the field should be omitted from the encoding if the field has an empty value, defined as false, 0, a nil pointer, a nil interface value, and any empty array, slice, map, or string.

Difference of reflect.Value.IsValid and reflect.Value.IsZero to prevent confusion:

The documentation for reflect.Value.IsValid says "It returns false if v is the zero Value". Note the uppercase Value, it's referring to the reflect.Value, an abstract representation of any Go value.

The documentation for reflect.Value.IsZero says "IsZero reports whether v is the zero value for its type". Note the lowercase value, it's referring the underlying Go value that the reflect.Value is representing.

So we can safely say that reflect.Value.IsZero is matching exactly the case of omitempty, if the user decides to opt in to this behaviour using ZeroAsNull.

There is strange behavior in tests! Even though I added the yaml testcase as last document in yamlA and yamlB all existing results (resultOfNoOptions, resultOfWithEmpty and the new resultOfWithZero) strangely expect the diff inbetween. You might want to look into that.

Edits to my branch are allowed if you want to extend the PR.

[dionysius]

Some thoughts and notes:

• I don't quite understand why diffcount is 0 in all those new test cases e.g. here. Shouldn't those be 1? EmptyAsNull cases have those e.g. here
• I expected ZeroAsNull to implicitly also behave like EmptyAsNull due to reflect.Value.IsValid and reflect.Value.IsZero, since an empty property should be some form of zero, no? Compare the line from resultOfWithEmpty with resultOfWithZero
  • I think this has to do with reflect.ValueOf(nil).IsValid() would return false, maybe my lines should be (or maybe this is just fine the current way):

- if r.option.zeroAsNull && (reflect.ValueOf(rawB).IsValid() && reflect.ValueOf(rawB).IsZero())`
+ if r.option.zeroAsNull && (!reflect.ValueOf(rawB).IsValid() || (reflect.ValueOf(rawB).IsValid() && reflect.ValueOf(rawB).IsZero()))`

[sters]

@dionysius

Sorry for the late. Thanks for your contribution! 👍

[dionysius]

Thanks! Do you have a minute to comment on my first note here? I'd like to understand the internals a bit better :)

[sters]

@dionysius

• I don't quite understand why diffcount is 0 in all those new test cases e.g. here. Shouldn't those be 1? EmptyAsNull cases have those e.g. here

Because, that diffCount:5 test has nil here. And diffCound is calculated from len([]rune(fmt.Sprint(rawA))) starts here. In you wrote diffCount:0 test case, it has empty string here. That len([]rune(fmt.Sprint(rawA))) returns 0.

https://go.dev/play/p/4IM_sloD2VV

• I expected ZeroAsNull to implicitly also behave like EmptyAsNull due to reflect.Value.IsValid and reflect.Value.IsZero, since an empty property should be some form of zero, no? Compare the line from resultOfWithEmpty with resultOfWithZero
  • I think this has to do with reflect.ValueOf(nil).IsValid() would return false, maybe my lines should be (or maybe this is just fine the current way):

Yes, that's correct. Because nil is nil, it cannot be decides any interface or primitive values. ValueOf returns zeroValue. https://pkg.go.dev/reflect#ValueOf
But maybe wrapped nil can be used to...? e.g. https://go.dev/play/p/f-Z4SD0gcUT