Proposal Details

One issue that we faced in the old Go JSON implementation is the inability to remove a field entirely when marshalling to JSON (when implementing the MarshalJSON func).

This feature is still not possible in the json/v2 implementation. While it is possible to set the exported field value to JSON null, removing the field completely does not seem possible.

Is there anything I am missing or there is currently no way to achieve this?

Previously, there was a proposal to return a SkipFieldErr error from the MarshalJSON which the encoder would interpret as to remove the field from the generated json.

Comment From: dsnet

Right now we support a SkipFunc sentinel error that says that the current marshal functionality should be skipped. This is easier to implement as the semantics simply require that the marshal func simply not perform any mutating functionality. If so, the json package itself will delegate marshaling to the next available marshaling for the type. Of particular note, SkipFunc is only permitted on a MarshalToFunc and not a MarshalerTo method (of relevance, I just filed #74324).

A SkipFieldErr could also be implemented, but is little more complicated. Fortunately, the omitempty feature in v2 omits a field if it would have serialized as an empty JSON object, array, string, or null. The implementation needs to be able to roll-back the previously written object name. We can leverage the same mechanism to implement SkipFieldErr.

As a side-note, this should probably be called SkipMember since we prefer to use JSON terminology (rather than Go terminology).

Comment From: mitar

I am very for supporting SkipMember sentinel error. This is still my biggest issue with v2 API. It is really sad that custom marshal function cannot re-implement everything struct tags can do.

But to implement this feature, maybe it could be easier if instead of SkipMember sentinel error, this behavior would be triggered if implementation would call something like Skip() method on the Encoder? (For v1 my suggestion would be to return nil from MarshalJSON, and I think calling (new method) Skip() is analogous in v2.)

Comment From: mhoury

@dsnet, Thank you for your reply (and your time). I really appreciate the work on the json2 package.

Fortunately, the omitempty feature in v2 omits a field if it would have serialized as an empty JSON object, array, string, or null

The problem is when we talk JSON, we talk integrations with other systems that may or may not be written in Go. Sometimes there is a need to send a JSON property with null value or as empty string. As you know, There is a difference between the following as they have different interpretations.

{
    "name": "John Doe"
}

{
    "name": ""
}

{
    "name": null
}

{

}

and, unfortunately, "omitempty" removes all nil, empty, fields from JSON.

There are also scenarios where attaching certain fields/members to JSON needs to be done on the fly while marshaling the struct.

I have looked into MarshalToFunc and looks like it can be used, but it is a bit more work.

If I may be honest, SkipMember looks a lot more appealing than MarshalToFunc.

Finally, if adding support for a SkipMember is much work and we don't want to break existing logic, we can look into the possibility of adding a completely new function/interface like MarshalerToWithSkip, so to not modify the existing one. It would be similar to the idea of context WithCancel function.

I will take a look at the other proposal #74324 as I haven't done so yet.

Comment From: dsnet

I have looked into MarshalToFunc and looks like it can be used, but it is a bit more work. If I may be honest, SkipMember looks a lot more appealing than MarshalToFunc.

To be clear, SkipMember is an orthogonal, but related concern from MarshalToFunc.

Without #74324, we can only make SkipMember work for MarshalToFunc. With #74324, we can make SkipMember work for both MarshalJSONTo and MarshalToFunc.

Comment From: dsnet

maybe it could be easier if instead of SkipMember sentinel error, this behavior would be triggered if implementation would call something like Skip() method on the Encoder?

I'm personally not fond of this because it means that MarshalJSONTo or MarshalToFunc subtly lies about what it did. It would report nil, which would imply that it did indeed encode a single JSON value, but in reality it didn't. Returning a sentinel error signals to the caller that "I'm refusing to encode a single JSON value and the caller should go figure out what to do instead".

Of particular note, since the "json" package is what usually dispatches the call to MarshalJSONTo or MarshalToFunc, it can do the internal logic to unwrite the JSON member name, which has already been encoded up until that point.

Comment From: dsnet

Thinking about this more. If we add this, we should probably call it SkipValue with the semantic: * If the next token to encode is a JSON object name, then the entire member is skipped within the immediate parent object. * If the next token to encode is a JSON object value, then the previously written name is rolled back and the entire member is skipped within the immediate parent object. * If the next token to encode is a JSON array element, then the element is skipped within the immediate parent array.

This covers every possible case of where a JSON value can appear within a composite type (i.e., objects and arrays). I don't think there's any reason to restrict this feature to just JSON object member values as the logic within a MarshalJSONTo can introspect the current encoder state using the jsontext.Encoder.StackIndex accessor to determine which of the 3 cases above it is currently operating under.

Comment From: mhoury

The 3 points you mentioned sound pretty rational to me.

Regarding the second point, however,

If the next token to encode is a JSON object value, then the previously written name is rolled back and the entire member is skipped within the immediate parent object.

Isn't best to wait before writing the name to the buffer until the MarshalJSONTo returns instead of writing the name, checking the value and performing a roll back?

Comment From: dsnet

Isn't best to wait before writing the name to the buffer until the MarshalJSONTo returns

The challenge is that the JSON object name itself might be a type that has a MarshalJSONTo. In such a case, we need to call it with a jsontext.Encoder before we encode the value. Once you need to support that possibility, we're pretty much constrained to calling MarshalJSONTo in depth-first order. It's still possible that we still do something like this by creating a "fake" jsontext.Encoder that the name encodes into, but fundamentally doesn't encode anything.

Given the above challenge, it was deemed easier to simply have jsontext.Encoder avoid flushing its internal buffer to the underlying io.Writer anytime it is between a JSON object name and value. Thus, we can rollback the encoded name purely within the buffer alone.

Comment From: mhoury

tbh, I am still looking and analyzing the json2 codebase so I can't say I am familiar with it yet.

but it seems like this entire problem can be solved using recursion, where as you mentioned, on each new level (i,e, if the current member being marshaled is an object/array), a new (not fake but another instance) encoder is passed.

Once all child members are marshaled they are written to the parent buffer.

doing so will eliminate the need for many functions like UnwriteEmptyObjectMember, UnwriteOnlyObjectMemberName and avoidFlush.

maybe I am oversimplifying things 🙄

Comment From: dsnet

I think we're getting into the weeds of how this could be implemented. We already have the groundwork set up by how omitempty functions so we should just leverage that.

a new (not fake but another instance) encoder is passed

A jsontext.Encoder isn't just an object for encoding JSON tokens/values, but it also allows for querying the current state machine via StackPointer, StackDepth, and StackIndex accessors. You'll need to somehow replicate the state as if the MarshalJSONTo calls had been occurring in order, when in reality they are not.

There's also performance issues with repeatedly creating new encoders and merging their state into the parent encoder.