cm: implement JSON [un]marshaling for Option types #301
Conversation
Signed-off-by: Brooks Townsend <[email protected]>
Signed-off-by: Brooks Townsend <[email protected]>
| @@ -57,3 +59,25 @@ func (o option[T]) Value() T { | |||
| } | |||
| return o.some | |||
| } | |||
|
|
|||
| // MarshalJSON implements the json.Marshaler interface. | |||
| func (o Option[T]) MarshalJSON() ([]byte, error) { | |||
There was a problem hiding this comment.
Methods should be on option[T] so named option types inherit the methods.
There was a problem hiding this comment.
Is there an example I can follow for a named option type? I assumed that if external implementations named a type it would be for Option[T], perhaps I should implement the un/marshaling for both?
There was a problem hiding this comment.
// This type would not have the JSON methods.
type OptionalI32 cm.Option[int32]This is why the methods are implemented on option, which is embedded in Option, so methods are preserved.
There was a problem hiding this comment.
@ydnar added a few more test cases that show that this works for named and nested options 🫡
I did find that I could remove the Option[T].MarshalJSON implementation and still have tests work, but removing Option[T].UnmarshalJSON did fail the cases that we have, so I left both in.
Signed-off-by: Brooks Townsend <[email protected]>
Signed-off-by: Brooks Townsend <[email protected]>
| } | ||
|
|
||
| // UnmarshalJSON implements the json.Unmarshaler interface for the public option type. | ||
| func (o *Option[T]) UnmarshalJSON(data []byte) error { |
| @@ -57,3 +59,41 @@ func (o option[T]) Value() T { | |||
| } | |||
| return o.some | |||
| } | |||
|
|
|||
| // MarshalJSON implements the json.Marshaler interface for the public option type. | |||
| func (o Option[T]) MarshalJSON() ([]byte, error) { | |||
This PR implements (naively) option marshaling/unmarshaling for Option types. This case is fairly straightforward, marshal to
nullif None and to the value if present.Opening this as a draft just to ensure that this is a change that's well received that follows contribution structure, open to changing anything here!
Addresses Option un/marshaling for #239