Option JSON serialization/deserialization does not handle the null case correctly

[AlisCode]

Hi,

I believe something is wrong in the behavior of Option's UnmarshalJSON. Say I have the very basic struct :

type Person struct {
	Name string
	Age  mo.Option[int]
}

and a test that ensures that the person can be serialized to JSON back and forth (roundtrip) :

func TestRoundtripJSONPerson(t *testing.T) {
	person := bugmooption.Person{
		Name: "MyPerson",
		Age:  mo.None[int](),
	}

	serialized, err := json.Marshal(&person)
        if err != nil {
		t.Fatalf("Failed to serialize Person")
	}

	var deserialized bugmooption.Person
	err := json.Unmarshal(serialized, &deserialized)
	if err != nil {
		t.Fatalf("Failed to deserialize Person")
	}

	if person != deserialized {
		t.Fatalf("person should be the same before and after JSON serialization roundtrip")
	}
}

The test currently fails. Indeed, after serializing, my person has a JSON field age: null. After deserializing, I get an age which is of the form :

{
    value: 0,
    isPresent: true,
}

So I've had a look at the implementation. At a glance, this comment :

// if user manually set the field to be `null`, then `isPresent` should be `true` since that is want user intends the value to be
// this makes sure `isPresent` makes semantic sense, and solves the ambiguity of pointer completely

sounds very wrong to me. If the field is set to null, then I expect isPresent to be false, not true.

I'm not sure why no one else has found this behavior strange, am I missing something ?

[AlisCode]

This is something that works at least for my test case above :

// UnmarshalJSON decodes Option from json.
func (o *Option[T]) UnmarshalJSON(b []byte) error {
	if bytes.Equal([]byte("null"), b) {
		o.isPresent = false
		o.value = empty[T]()
		return nil
	}

	err := json.Unmarshal(b, &o.value)
	if err != nil {
		return err
	}

	o.isPresent = true
	return nil
}

[pushin]

Things get even more interesting when Optional is applied to Optional:

type Person struct {  
	Name string  
	Age  Option[Option[int]]  
}  

It seems that for serialization to work correctly, the JSON representation for the Optional type should look like this:

{  
	"isPresent": <bool>,  
	"value": <value>  
}  

[eloonstra]

We're experiencing this issue as well, the behavior of mo.Option has changed between 1.13 and 1.14 with #65. This is breaking for us.

[liruohrh]

This is indeed a bit strange, accoding to #65 . null is also set value, only has not the prop mean it has not set value.

he treat omitted a field or set it to null manually as a same thing. but i think set to null make the Option has not value, mean isPresent=false too, rather than set to null mean set a value.

now, we has two way to solve it. (to remove all null json value to avoid being different during deserialization)

1. upgrade to go1.24 and use omitzero to Option field.
2. use regexp or deserialize to map、filter nil value and serialize map to json again.(anyway, strictly control deserialization input to ensure there are no null values)

[liruohrh]

I use github.com/json-iterator/go to ignore serialize the field which IsPresent=false. you can see here json_test.

[samber]

I just reverted the PR #65

Please upgrade to v1.14.1

[kavin-sm]

I also would like to point out that IsZero implementation is also looks weird to me.

// IsZero assists `omitzero` tag introduced in Go 1.24
func (o Option[T]) IsZero() bool {
	if !o.isPresent {
		return true
	}

	var v any = o.value
	if v, ok := v.(zeroer); ok {
		return v.IsZero()
	}

	return reflect.ValueOf(o.value).IsZero()
}

The last part also make the method return false even if isPresent = false but value is empty

This serialize and deserialization on a field with omitzero tag will not be the same when applying with isPresent = true but has zero value.

type Person struct {
	Name   mo.Option[string] `json:"name,omitzero"`
	Age    mo.Option[int]    `json:"age,omitzero"`
	IsUser mo.Option[bool]   `json:"isUser,omitzero"`
}

func TestJsonEncodingAndDecodingWithZeroValue(t *testing.T) {
	p := Person{
		Name:   mo.Some(""),
		Age:    mo.Some(0),
		IsUser: mo.Some(false),
	}

	data, err := json.Marshal(p)
	assert.NoError(t, err)
	expectedJSON := `{"name":"","age":0,"isUser":false}`
	assert.Equal(t, expectedJSON, string(data))
	var decoded Person
	err = json.Unmarshal(data, &decoded)
	assert.NoError(t, err)
	assert.Equal(t, p, decoded)
}

So currently there's no way to serialize field as 0, false, "" when using with omitzero which is the only way to omit the field if isPresent = false

Please let me know if I should create new issue.

[taman9333]

@kavin-sm For your first point:

The last part also make the method return false even if isPresent = false but value is empty

I don’t think that’s correct. If isPresent = false, the early return is executed, so IsZero() will return true immediately(Correct me if I’m misunderstanding your point, but I don’t see a path where the reflect check runs when isPresent = false)

For the second point:

I believe this is more of a design choice. For me it makes sense that Option.IsZero() considers both “not present” and “present with an inner zero value” as zero, which makes sense if you want to omit fields whenever the inner type is at its zero value. On the other hand, I also see your perspective if you’d rather treat only None as zero, then Some(""), Some(0), Some(false) would be included in JSON instead of omitted. That’s a valid alternative semantics depending on how strictly you want Option to represent “presence” versus “value”. I believe @samber could help with that

[liruohrh]

I think Zero is only equal to None, but not equal like Some(0), because zero in go, like A{v int} is A{v: 0}, so zero Option sholud be Option[int]{value: 0, isPresent: false}, it just is None. Zero value should follow the design of go.

If want to ignore something like Some(0) where the value is zero, then I think it can be controlled by var IsOptionValueZeroAsZero bool, or it can implement serialization and deserialization by self.