proposal: encoding/json/v2: permit SkipFunc for MarshalTo/UnmarshalFrom methods

[dsnet]

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

[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?