Proposal Details
TL;DR, I propose permitting SkipFunc
as being allowed for MarshalerTo
or UnmarshalerFrom
methods. However, this also means that we will document that code should avoid directly calling MarshalerTo
and UnmarshalFrom
methods.
Currently, the SkipFunc
sentinel error may only be returned by MarshalToFunc
and UnmarshalFromFunc
caller-specified functions, but may not be returned by a MarshalerTo
or UnmarshalerFrom
type-specified method.
The rationale for this restriction is because it is relatively common for users to directly call the legacy MarshalJSON
method declared on a type (e.g., v.MarshalJSON()
), rather than indirectly calling MarshalJSON
by calling json.Marshal
(e.g., json.Marshal(v)
) and letting it auto-detect the MarshalJSON
method.
Based on prior precedence, it is conceivably common that users could also directly call the newer MarshalJSONTo
or UnmarshalJSONFrom
methods. We forbade SkipFunc
from being returned, otherwise it would be a potentially surprising error for a direct caller to receive.
However, I'm looking at existing use cases of the legacy MarshalJSON
method that I would like to migrate to MarshalJSONTo
, and I see cases where supporting SkipFunc
in a method would be beneficial. For example, I see logic like the following:
func (d device) MarshalJSON() ([]byte, error) {
if condition1 {
// Declare the deviceCopy type as being memory equivalent to device,
// but without any of the methods of device to avoid infinite recursion.
type deviceCopy device
return json.Marshal(deviceCopy(d))
}
... // some other custom marshal logic
}
This pattern of declaring a type is a hack and every single use has the same comment about why a type was being declared. It would be more ideal to do:
func (d device) MarshalJSONTo(enc *jsontext.Encoder) error {
if condition1 {
return json.SkipFunc
}
... // some other custom marshal logic
}
If we want to make this change, it needs to be before the final release of "encoding/json/v2" (doesn't need to block Go 1.25) since it cannot be changed after the fact as it requires a documented semantic on the MarshalerTo
and UnmarshalerFrom
interfaces that such methods should probably never be directly called by users unless want to directly deal with sentinel errors.
\cc @johanbrandhorst @mvdan @neild
Comment From: mitar
I agree with the goal here, but I am not sure if this captures all common cases for using a custom type. In my experience, it is even more common to use a custom type to skip custom marshal but then augment the result in some way. For example:
func (d device) MarshalJSON() ([]byte, error) {
if condition1 {
type deviceCopy device
b, err := json.Marshal(deviceCopy(d))
if err != nil {
return nil, err
}
// inefficient, just for demonstration purposes
return []byte(`"` + string(b) + `"`), nil
}
... // some other custom marshal logic
}
So I think sentinel error is useful when you want to control from inside which implementation is called, which makes it easier to combine things. But I am not if it really works the best for cases where you want to skip some implementation when calling from outside, like in the example above, you are calling from outside json.Marshal(deviceCopy(d))
and want to skip the implementation on the struct itself. But maybe you want to skip implementation on the type. So is there a better way to control that? An option?