Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

proposal: encoding/json: allow skipping field by returning an error in MarshalJSON() ([]byte, error) #57554

Closed
johan-lejdung opened this issue Jan 3, 2023 · 5 comments
Closed

proposal: encoding/json: allow skipping field by returning an error in MarshalJSON() ([]byte, error) #57554

johan-lejdung opened this issue Jan 3, 2023 · 5 comments
Labels
FrozenDueToAge Proposal
Milestone
Proposal

Comments

@johan-lejdung
Copy link

With the introduction of generics I have started using the following pattern to properly deal with PATCH requests by clients.

type Field[T any] struct {
	value T
	IsSet bool `json:"-"` // Set means the value has been set
}

type MyStruct struct {
	value Field[string] `json:"value"`
}

This allows me to differentiate between zero values and non-set json fields. This field has a unique UnMarshaler and Marshaler

// UnmarshalJSON is a custom function to unmarshal JSON into a Field
func (i *Field[T]) UnmarshalJSON(data []byte) error {
	// If this method was called, the value was set.
	i.IsSet = true

	var temp T
	if err := json.Unmarshal(data, &temp); err != nil {
		return err
	}
	i.value = temp
	return nil
}

// MarshalJSON is a custom function to marshal a field into JSON
func (i Field[T]) MarshalJSON() ([]byte, error) {
	return json.Marshal(i.value)
}

The issue is that if I choose to UnMarshal the MyStruct when the value field has IsSet=false I would end up with the following JSON: {"value":""} rather than {}.

I propose the ability to return a specific error json.ErrSkipField from the MarshalJSON that would cause the field to be skipped. This simplifies handling in these scenarios, and allow the logic to hide/show the struct live together with the implementation.

Writing a custom MarshalJSON function for every struct that happens to use the Field will quickly get tiring and introduce risk of errors.
@gopherbot gopherbot added this to the Proposal milestone Jan 3, 2023
@mateusz834
Copy link
Member

mateusz834 commented Jan 3, 2023 •
edited
Loading

I recently stumbled across the same issue, and also thought that having a error to skip a field would be something nice to have.

For now you can use a pointer to a type with omitempty to fix that, like:

type MyStruct struct {
	value *string `json:",omitempty"`
}

encoding/Json docs:

The "omitempty" option specifies that the field should be omitted from the encoding if the field has an empty value, defined as false, 0, a nil pointer, a nil interface value, and any empty array, slice, map, or string.

I think that it would be nice to have something like json.ErrSkipField, but I feel like it should be somehow integrated with the omitempty, so that the json.ErrSkipField only makes an effect when the omitempty is specified. (Probably then it should be named differently like: json.ErrEmptyField)

@ianlancetaylor ianlancetaylor moved this to Incoming in Proposals Jan 3, 2023
@earthboundkid
Copy link
Contributor

FYI, changes to the JSON package should probably be raised at https://github.com/go-json-experiment/json since AFAICT, that repo is being experimentally primed to replace the existing JSON package.

@seankhliao
Copy link
Member

Duplicate of #50480

@seankhliao seankhliao marked this as a duplicate of #50480 Jan 3, 2023
@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Jan 3, 2023
@johan-lejdung
Copy link
Author

johan-lejdung commented Jan 4, 2023 •
edited
Loading

@mateusz834

I recently stumbled across the same issue, and also thought that having a error to skip a field would be something nice to have.

For now you can use a pointer to a type with omitempty to fix that, like:

type MyStruct struct {
	value *string `json:",omitempty"`
}

encoding/Json docs:

The "omitempty" option specifies that the field should be omitted from the encoding if the field has an empty value, defined as false, 0, a nil pointer, a nil interface value, and any empty array, slice, map, or string.

I think that it would be nice to have something like json.ErrSkipField, but I feel like it should be somehow integrated with the omitempty, so that the json.ErrSkipField only makes an effect when the omitempty is specified. (Probably then it should be named differently like: json.ErrEmptyField)

Thanks for the tip, it's just a shame that a simple thing like this has to be achieved by introducing more risks and headache for the developer by introducing pointers. But it's the solution we have to go with for the time being at least.

@carlmjohnson

FYI, changes to the JSON package should probably be raised at https://github.com/go-json-experiment/json since AFAICT, that repo is being experimentally primed to replace the existing JSON package.

Thank you, I weren't aware of that 👍

@seankhliao

Duplicate of #50480

That was itself marked as a duplicate as mentioned by rsc:

This proposal is a duplicate of a previously discussed proposal, as noted above,
and there is no significant new information to justify reopening the discussion.
The issue has therefore been [declined as a duplicate](https://golang.org/s/proposal-status#declined-as-duplicate).
— rsc for the proposal review group

But I couldn't find any link to the duplicate he was referring to or reasoning for closing, would you mind sharing if you have that available?

@ggicci ggicci mentioned this issue Jan 10, 2023
Open
@rsc rsc moved this from Incoming to Declined in Proposals Mar 29, 2023
@rsc
Copy link
Contributor

rsc commented Mar 29, 2023

This proposal is a duplicate of a previously discussed proposal, as noted above,
and there is no significant new information to justify reopening the discussion.
The issue has therefore been declined as a duplicate.
— rsc for the proposal review group

@rsc rsc removed this from Proposals Jan 10, 2024
@golang golang locked and limited conversation to collaborators Mar 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal
Projects
None yet
Milestone
Proposal
Development

No branches or pull requests
6 participants
@rsc @earthboundkid @gopherbot @johan-lejdung @seankhliao @mateusz834