golang/go

proposal: encoding/json, encoding/xml: support zero values of structs with omitempty

joeshaw opened this issue Β· 116 comments

Support zero values of structs with omitempty in encoding/json and encoding/xml.

This bites people a lot, especially with time.Time. Open bugs include #4357 (which has many dups) and #10648. There may be others.

Proposal

Check for zero struct values by adding an additional case to the isEmptyValue function:

case reflect.Struct:
        return reflect.Zero(v.Type()).Interface() == v.Interface()

This will solve the vast majority of cases.

(Optional) Introduce a new encoding.IsZeroer interface, and use this to check for emptiness:

Update: I am dropping this part of the proposal, see below.

type IsZeroer interface {
        IsZero() bool
}

Visit this playground link and note that the unmarshaled time.Time value does not have a nil Location field. This prevents the reflection-based emptiness check from working. IsZero() already exists on time.Time, has the correct semantics, and has been adopted as a convention by Go code outside the standard library.

An additional check can be added to the isEmptyValue() functions before checking the value's Kind:

if z, ok := v.Interface().(encoding.IsZeroer); ok {
        return z.IsZero()
}

Compatibility

The encoding.IsZeroer interface could introduce issues with existing non-struct types that may have implemented IsZero() without consideration of omitempty. If this is undesirable, the encoding.IsZeroer interface check could be moved only within the struct case:

case reflect.Struct:
        val := v.Interface()
        if z, ok := val.(encoding.IsZeroer); ok {
                return z.IsZero()
        }
        return reflect.Zero(v.Type()).Interface() == val

Otherwise, this change is backward-compatible with existing valid uses of omitempty. Users who have applied omitempty to struct fields incorrectly will get their originally intended behavior for free.

Implementation

I (@joeshaw) have implemented and tested this change locally, and will send the CL when the Go 1.6 tree opens.

CL https://golang.org/cl/13914 mentions this issue.

CL https://golang.org/cl/13977 mentions this issue.

The empty struct approach is implemented in CL 13914 and the IsZeroer interface is implemented in CL 13977.

In order for them to be reviewable separately they conflict a bit -- mostly in the documentation -- but I will fix for one if the other is merged.

In the CLs @rsc said,

I'd really like to stop adding to these packages. I think we need to leave well enough alone at some point.

I see what he's getting at. CL 13977, which implements the IsZeroer interface is clearly an enhancement and adds API to the standard library that needs to be maintained forever. So, I am abandoning that CL and that part of the proposal.

However, I still feel strongly about omitempty with empty structs, and I want to push for CL 13914 to land for Go 1.6.

I use the JSON encoding in Go a lot, as my work is mostly writing services that communicate with other services, in multiple languages, over HTTP. The fact that structs don't obey omitempty is a frequent source of confusion (see #4357 and its many dups and references, and #10648) and working around it is really annoying. Other programming languages do not conform to Go's ideal "zero value" idea, and as a result encoding a zero value is semantically different in JSON than omitting it or encoding it as null. People run into this most commonly with time.Time. (There is also the issue that decoding a zero time.Time does not result in an empty struct, see #4357 (comment) for background on that.)

I think it should be considered a bug that Go does not support omitempty for these types, and although it adds a small amount of additional code, it fixes a bug.

This proposal is marked as unplanned, yet the related bug report #10648 is marked as go1.7.
Is it still being worked /thought on?

rsc commented

To my knowledge, it is not being worked on. Honestly this seems fairly low
priority and will likely miss Go 1.7.

On Sun, Mar 27, 2016 at 12:22 PM JΓ©rΓ΄me Andrieux notifications@github.com
wrote:

This proposal is marked as unplanned, yet the related bug report #10648
#10648 is marked as go1.7.
Is it still being worked /thought on?

β€”
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#11939 (comment)

OK.

This is more of a convenience than a priority indeed.

It can be a pain point when dealing with libs that don't support "embedded structs" as pointer though.

I wonder if a low-impact alternative to the IsZeroer interface would be to allow one to return an error called json.CanOmit (or similar) from an implementation of the Marshaler interface. That way the dev is in control of determining what constitutes a zero value, and it doesn't impact other code.

It's not a perfect solution, since one can't add methods to types defined in another package, but this can be worked around to a degree.

Taking the time.Time example:

type MyTime struct {
  time.Time
}

// Implement the Marshaler interface
func (mt MyTime) MarshalJSON() ([]byte, error) {
  res, err := json.Marshal(mt.Time)

  if err == nil && mt.IsZero() {
    return res, json.CanOmit // Exclude zero value from fields with `omitempty`
  }
  return res, err
}

I haven't looked into implementation, but on the surface it would seem like a low-overhead solution, assuming the only work would be to check if an error returned equals json.CanOmit on fields where omitempty was included.

Using errors as a flag is not without precedent in the standard library, e.g. filepath#WalkFunc allows one to return filepath.SkipDir to skip recursion into a directory.

@Perelandric I mentioned a possible sentinel error value in https://golang.org/cl/13914 but I didn't get feedback on the idea or an opportunity to implement it before the Go 1.7 freeze. After Russ's comments on my original CL (and showing the unexpected difficulty in implementing this) I think that's the better way to go.

CL https://golang.org/cl/23088 mentions this issue.

adg commented

While it's clear that we can do this, it's not clear that we want to. I'd like to see a formal proposal document that weighs the advantages and drawbacks of this feature addition. In particular, I am concerned about compatibility and maintenance burden.

thanks Andrew. I worked on this a little bit at GopherCon. I will look into putting together a formal proposal.

@joeshaw we ran into this issue at my place of work and I'm eagerly awaiting your proposal. Feel free to contact me if you would like any help. Email is on my profile.

@joeshaw Is the proposal you're considering based on the sentinel object idea, or are you considering a different approach? Do you think you'll have time for this before the next release?

@Perelandric Yes, I think the sentinel object idea is the most straightforward way to go.

Other options include:

  • The IsZeroer interface (but I think this has potential backward compatibility issues)
  • "Backspacing" over objects that serialize to {} (but I think this requires too big a change to the JSON encoder code, and doesn't handle json.Marshaler implementers like time.Time)

I don't think I will be able to do this (proposal + implementation) before Go 1.8. If someone else wants to take it on for 1.8, I will gladly pass along my knowledge and partial implementation.

Thanks @joeshaw. I created an implementation using the sentinel error for the encoding/json package and will start to work on the proposal in a bit. I think I'll focus primarily on this approach.

The Marshaler interface in encoding/xml is different from that in encoding/json, and seems as though a custom zero-value can already be established without needing to return anything special. Did you find that to be true?

After I make a little more progress, I'll post a link to a branch in case you, @albrow or anyone else wishes to review and contribute.

If you have any additional thoughts or info in the meantime, please let me know. Thank you!

Change of heart on this. If there's resistance to adding to packages, then this won't fly. Maybe someone else wishes to advocate for this.

Mentioned in one of the duplicate issues was the idea to let MarshalJSON return (nil, nil) to skip the field. Borrowing your earlier example:

type MyTime struct { time.Time }

func (mt MyTime) MarshalJSON() ([]byte, error) {
    if mt.IsZero() {
        return nil, nil // Exclude zero value from fields with `omitempty`
    }

    return json.Marshal(mt.Time)
}

In Go 1.7, returning nil is not a valid implementation for MarshalJSON and leads to "unexpected end of JSON input" errors. This approach doesn't require any visible change to the encoding package (not even adding an error value).

For what it's worth, I just intuitively wrote a MarshalJSON method like that, expecting a field to be omitted from the JSON output.

@pschultz That approach seems reasonable to me, but it can't be used with omitempty.

The reason you get that error is because the JSON encoder checks for the validity of the JSON coming out of MarshalJSON and the result of returning a nil byte slice is (something like) "key":,. If returning a nil byte slice indicated that it should be omitted that'd be a different way to omit something from being encoded in the JSON than omitempty. (That might be fine, it seems ok to me.)

The benefit of the error value is that it could fit in with the existing omitempty because you'd return something like []byte(""), ErrOmitEmpty and it'd obey omitempty yet still return a valid value ("") if not set.

@pschultz: A nil return wouldn't be able to be used as a flag for omitempty without causing undesired errors when omitempty is not present, since the implementation of MarshalJSON doesn't know when omitempty is actually there.

I don't know if having nil as an alternative to omitempty would be the best either. Seems like the user of a type should be the one deciding when it is omitted. IMO, the implementation of MarshalJSON should always return some useful representation, or an error when that's impossible.

It appears there are only two ways to have omitempty work with a struct.

  • Use a pointer to the struct where the pointer is nil rather than a zero value struct.
  • Implement Marshaler interface for the surrounding type to provide special handling for zero value structs

The later can be somewhat cumbersome to maintain field parity for large structures. The former means data is no longer contiguous and requires care for multi-threaded use.

I came to this by way of #9719, which isn't unlike the time.Time case. Using code like below it's possible to change the JSON output for nullable SQL types from "field":{"String":"","Valid":false} to "field":null, but in itself it can't affect omitempty.

// NullString represents a string that may be null.
type NullString struct {
	sql.NullString
}

// MarshalJSON implements the json.Marshaler interface.
func (x *NullString) MarshalJSON() ([]byte, error) {
	if !x.Valid {
		return json.Marshal(nil)
	}
	return json.Marshal(x.String)
}

I do think this proposal can be simplified. Writing the code to check if a zero struct is empty or not isn't a huge problem (though it can be a bit cumbersome to maintain the field parity). (separate issue #7501)

The issue is not having the ability to tell Marshal that a struct is empty without using a pointer. With that, a wrapper around time.Time or sql.NullString would do the trick, rather than implementing Marshaler on every surrounding type.

@bradfitz In #10648 (comment) (which is now frozen), you said:

This bites me with time.Time{} values often. Camlistore has its own time.Time alias, just so we can do JSON differently.

Just wanted to check, is the named type you were referring to https://godoc.org/camlistore.org/vendor/go4.org/types#Time3339, or did you have another one in mind (that I couldn't find)?

@shurcooL, yeah, that's its current home. It used to live in Camlistore's tree.

Note its implementation:

func (t Time3339) MarshalJSON() ([]byte, error) {
	if t.Time().IsZero() {
		return null_b, nil
	}
	return json.Marshal(t.String())
}

func (t Time3339) String() string {
	return time.Time(t).UTC().Format(time.RFC3339Nano)
}

@bradfitz I see, thanks. The reason I asked is because even with that custom type, it still doesn't get omitted when time is zero. It gets encoded as "null" and the field is included. See https://play.golang.org/p/oxEdyj5TFL.

Based on what you said, I expected it to have more of an effect (well, encoding as "null" helps a little, but I wanted it to be omitted entirely). Given that, it seems no better than using *time.Time with omitempty option, unless I'm missing something.

I created a simple library that patches isEmptyValue function in memory and declares Zeroer interface: https://github.com/lokhman/json-zeroer. Use it at your own risk :)

Change https://golang.org/cl/102158 mentions this issue: encoding/json: encoding/xml: interface to omit marshalling empty structs

I'd really like to help out as this has also been a fairly large source of confusion in my organisation, but until this (or another) proposal is accepted it doesn't seem like there's anything to do?

@wayneashleyberry The work to be done here is to put together a proposal.

As I see it there are at least 3 different possible approaches:

I don't see any way to implement this without adding some surface to the encoding/json package, unfortunately, whether it's a new interface type or a sentinel error value.

I like the IsZeroer interface, but as @bradfitz points out in https://go-review.googlesource.com/c/go/+/13977#message-54fb3469327f16170962c57896824a4cdd5cd541 that could be pretty expensive and I've not done any performance analysis on this. There are potential backward compatibility issues (or at least potentially surprising behavior) surrounding people's misunderstanding of the omitempty tag with structs.

I also like the sentinel value, but that only works for types that implement json.Marshaler. But for the most commonly-desired types like time.Time that's fine, but it doesn't solve the asymmetry of the fact that omitempty doesn't work out of the box on structs. And this sentinel error value would be orthogonal to omitempty, so it is potentially confusing in that respect.

So what this needs is someone to do some further analysis, implement these (and/or take my lame implementations further), see which is best and put forth a formal proposal. (But that's not going to be me, as my failure to do this in the past two years shows πŸ˜„)

mibk commented

Are there any experience reports concerning types other than time.Time? If this issue is mostly about time.Time, maybe we should focus on that rather than come up with a more general solution covering mostly only hypothetical issues.

@mibk I often run into this issue using sql.NullString, mysql.NullTime etc.

mibk commented

@wayneashleyberry I see. But these cases could be handled using the third approach:

Whenever any struct would marshal to {}, omit it. This is an idea @rsc brought up in https://go-review.googlesource.com/c/go/+/13914#message-89b368b14fcdc0a03089aa34dc8185710144947f

which seems like a good idea regardless this issue. The third approach, IMO, also has the biggest potential to make it to the standard library.

Xe commented

Is this still being discussed for implementation? This just bit me today.

@Xe, the JSON and XML issues are on hold until at least Go 1.12. @rsc is focused on vgo at the moment.

rsc commented

I hope to work on this for Go 1.12.

That just made my week @rsc, thank you so much πŸ™

Xe commented

@bradfitz would a CL to fix this issue be accepted?

rsc commented

@Xe please hold off. We have not figured out the design yet. As I indicated above, I want to work on this for Go 1.12, probably in mid-September.

@rsc Is it helpful to add a slightly different use case for this issue?

We use a custom ID type that is represented as a [16]byte. Our type implements the MarshalJSON() method to encode the byte array as a more readable string. I believe the same issue occurs when using UUIDs from either popular uuid library. A zero ID is represents as [16]byte{0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0}, but that json serializes to [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0], so even hacks with unsafe.Pointer don't work.

If I'm reading this correctly, the third proposed solution (whenever any struct would marshal to {}, omit it.) would not work for this use case, at least not without requiring a wrapper struct. I'm personally a fan of using IsZero since we already use that extensively, but the sentinel value also seems to work.

rsc commented

On hold for JSON sweep.

Hey just wanted to ask if this proposal would consider when embedded struct field values are the zero value. For example:

https://play.golang.org/p/kjVSZsoor6p

Right now in order to get an empty struct I would need to add omitempty for every field inside the embedded struct. Sorry if it's implicit, just one to be clear on the matter.

Edit: Upon further review, it seems it is clear it does.

I know this is an old thread but I was able to work with nillable structs using jsoniter with RegisterTypeEncoderFunc. This works by evaluating the isEmptyFunc provided alongside the type string and encoder func, something similar to what's already been suggested above.

I can post a gist if people are interested.

Xe commented

@obashistu please do, i'd be interested to see how it's done.

@obashistu please do, i'd be interested to see how it's done.

This is a POC nillable string field we've been playing with recently. I've left out the boring marshalling and unmarshalling bits, if you need guidance on that (virtually the same as the standard lib with the addition of declaring a json var) check here : jsoniter.

https://gist.github.com/obashistu/caf5763a4e3b7f81de54b4c621aac634

Any chance this can get looked at again?

Qhesz commented

Slightly different implementation proposal #33400 :

Add an omitnull option, which will omit the given field if it's marshalled JSON value is null.

Similar to the sentinel return value proposal in terms of how it would need to be implemented.

Plus a little bit of justification: when designing a JSON api, you generally have to pick a scheme for optional values - omit them, or encode them as nulls - because in other programming languages (like Python, Javascript, jsonschema etc) those two options have to be handled differently. An omitnull option would map directly to me making that decision.

I would love to see this in the go 1.13 release. Is this proposal still on the roadmap? Is there anything we can do to help push this along?

In the meantime, for those who need this functionality for json right away, I created a mirror of latest go1.12.7 go1.13 encoding/json and added filtering for zero value structs (https://github.com/clarketm/json). I am detecting emptiness the same as go-yaml.

@clarketm I like your solution. It stays true to the original intent of omitempty. Curious of the performance of it, especially with larger structs and structs that many other structs in it.

I'd still personally like to see some interface that could be used though I do not think IsZero() should be used simply because it is already a well established pattern and could mean different things and so using for this might have unintended consequences.

Like you though I would definitely love to see this get in 1.13 if possible.

Like you though I would definitely love to see this get in 1.13 if possible.

1.13 is about to come out so the merge window has been closed for a while. The 1.14 merge window is about to open, so that would be the earliest place it could go (though I wouldn't hold your breath, there doesn't seem to be much interest from the Go team in working on the encoding package; they have a lot on their plate with all the new features, I suppose; hopefully one of them can supply more information).

Curious of the performance of it, especially with larger structs and structs that many other structs in it.

You are right, it is a linear scan of all struct (and nested struct) fields recursively. It will exclude private fields which helps a bit on performance.

I do not think IsZero() should be used simply because it is already a well established pattern and could mean different things and so using for this might have unintended consequences.

It looks like Go 1.13 Release Notes have a mention of a new Value.IsZero method for the reflect package which interestingly uses this same procedure (minus the private field exclusion) to determine if a struct is zero. Regardless, I agree, a generalized isZero() interface would be the most robust.

Is it possible someone from the Go team can give some feedback on this and what it might take to get this into 1.14? Would a PR with @clarketm implementation be ok? Is an interface the preferred approach? Are we all wasting our time thinking this will ever get acted on?

I'd still personally like to see some interface that could be used though I do not think IsZero() should be used simply because it is already a well established pattern and could mean different things and so using for this might have unintended consequences.

Can you explain what do you mean by "a well established pattern and could mean different things"...

@lukatendai Suppose you have a struct like:

type Foo struct { ... }

func (f Foo) IsZero() bool { ... }

And you have a *Foo in a struct serializing to JSON:

type JSONThing struct {
    Foo *Foo `json:"foo,omitempty"`
}

By making omitempty obey if there is an IsZeroer interface and omitting if it returns false, you are changing the behavior. Before if the pointer were nil, it would be omitted from the JSON output. Now it depends on both whether it is nil and whether IsZero returns false. We can't say with certainty that is what users want.

@lukatendai I meant that IsZero() is already used in a lot of places. For time.Time has a IsZero() method. So if we made an interface that used that method, it could cause a lot of people's structs to start omitting time fields when they currently do not and that could cause unintended consequences. I have also seen IsZero() used in other libraries outside the stdlib so it has become an established community pattern.

@MrTravisB Yes... this I understand but IMO it won't effect "these" people... because time cannot be "empty" in current implementation, nobody adds "omiempty" to the json tags so if IsZero becomes interface than it wont change anything unless somebody added "omitempy" which probably they don't want to have zero time anyway. I use IsZero as a method on other structures to know if the structure does contain the data worth marshaling... any kind of marshaling/serializing, json, bson, protobuf, etc...

If we assume people don't add omitempty to time fields then yes, it wouldn't effect anyone. I'm just not sure if that is a safe assumption. I still reflexively add omitempty to time (and other struct) fields even though I know it doesn't do anything simply because I expect that it should.

because time cannot be "empty" in current implementation, nobody adds "omiempty" to the json tags so if IsZero becomes interface than it wont change anything unless somebody added "omitempy" which probably they don't want to have zero time anyway.

But *time.Time can be empty, and it is common to add omitempty to those values, exactly to get the desired behavior of not serializing a zero time value. It is unfortunate that you have to use a pointer type for this, but at least it works.

I agree that time.Time is not the best example here because its zero value is very bad for JSON serialization, but the IsZero example I showed above illustrates that this is a behavioral change that can't be opted into, and which might break things broadly across the countless types that implement IsZero.

To do IsZero safely you'd need to create a new struct tag that opts into that behavior -- an omitzero tag was suggested earlier in the thread.

Qhesz commented

I think omitting zero values is the wrong approach semantically. It's weird that you have a type whose zero value has a valid JSON serialisation, but you are overriding that in the containing struct to omit it instead.

It makes a lot more sense to define your type such that it serialises to null when it isn't a meaningful value, and have a new omitnull struct tag that omits the key for the field if it serialised to null.

Only the library has to change to support this approach, it doesn't interact with any external interfaces like IsZero, and you don't even have to introduce a special sentinel err value for MarshalJSON - someone who wants to use it just marshals to null (which they should anyway), and tags their struct field with omitnull.

After 7 years of Go development I believe "omitempty" with IsZero() is a simply solution. If someone uses omitempty with time.Time right now, clearly they don't care. Also, I use custom marshalers but for more complicated things than getting rid of zero time.

I support what @Qhesz said.

@Qhesz

I think omitting zero values is the wrong approach semantically. It's weird that you have a type whose zero value has a valid JSON serialisation, but you are overriding that in the containing struct to omit it instead.

There is a fundamental mismatch in the JSON and Go type systems. Namely, values in JSON are nullable, and non-pointer types in Go are not.

The omitempty flag is a convenience for dealing with this mismatch, by observing that many APIs (those not written in Go, primarily) do not obey Go's view on zero values and treat them separately from a null value. In Ruby, for example, the empty string ("") evaluates true, whereas nil does not. Thus, a JSON document of {"foo": ""} versus {} or even {"foo":null} are very different things in Ruby. This issue deals with the fact that this convenience flag doesn't extend to struct types.

In your proposal, how do you deal with non-nullable types, particularly the built-in ones like string?

Qhesz commented

I understand the issue. omitempty on structs would let you choose between outputting {"foo":{"bar":0}} and {}, whereas omitnull would let you choose between outputting {"foo":null} and {}. (My point about semantics is that neither 0 nor a struct containing it are "empty". They're "zero" in golang terms, but 0 could still be a meaningful value.)

String is one type where I think omitempty makes sense. Primarily because it's called "the empty string", and the wording matches. Plus, historically, UI frameworks might not distinguish between absent and empty string inputs very well, so you might be stuck with that behaviour and genuinely want to special case the empty string to match.

If my JSON api included an optional integer, I would define a Golang type that matches. So if I wanted my Golang integer field to be optional (and didn't want to use pointers, eg. to preserve ==, copy by struct assignment etc.), I'd make it an explicit OptionalInt struct type, with a bool for presence and an int value. So 0 would be a valid value, I could test for presence separately, and I'd marshal it out as null if it wasn't present. And then use omitnull in the struct containing it, because my convention is ommited fields instead of null values in JSON.

And if my JSON api included a required integer, I'd use a normal Golang int field that defaults to 0 if it was absent from/null in the JSON. If I really wanted to validate that it must be present, that's outside the scope of this serialisation library, and I'd validate the JSON separately before deserialising it.

In direct response to your question: I deal with JSON/golang type mismatches, by not having type mismatches.

Qhesz commented

A note on implementation: I'd prefer a json.Decoder flag OmitNull() rather than a struct tag, so I can define it once for my whole project. The struct tag I'd end up adding to every struct field I define, which would be a lot of repetition.

With this, #33835 and #33854, encoding/json would have sane and consistent management of json null and absent fields, both when unmarshalling and marshalling. All three would be opt-in, but at least it would be possible.

I'm running into this issue when using null.String from https://pkg.go.dev/gopkg.in/guregu/null.v3. I'd be willing to help work on a solution.

There is a disconnect between real world JSON and go.

For example, given this struct:

type Foo struct {
	Bar int `json:"bar,omitempty"`
}

All three of these valid JSON strings will result in the same value:

{}
{"bar":null}
{"bar":0}

There's no way to tell the difference between an omitted value, a null value, and a zero value. While these distinctions aren't really present in Go, they can be important when parsing JSON from 3rd-party sources and when encoding data into JSON to send to those sources.

Some of this can be mitigated using custom UnmarshalJSON handlers: https://play.golang.org/p/XhF793wMKr7. However, outputting back to the original JSON string is still not possible in the case of an omitted value.

The original proposal from 2015, including the IsZeroer interface, would allow Go users to fully encode and decode JSON. I found this issue after implementing an almost identical solution myself. I am happy to help work on this.

@walkingeyerobot If you don't mind *int, then that should be enough to distinguish between null and 0.

@diamondburned Distinguishing between null and 0 is not an issue. The issue is to distinguish between null, 0, and omitted, which is not possible with the current JSON library.

@diamondburned pointers work great for unmarshaling if you need to distinguish between empty and 0, but they're not so friendly when you need to marshal. You would need to write a custom Marshaler interface for the parent struct or use two different structs and map between them

On hold for JSON sweep.

@rsc - any chance that there would be a solution to this issue? Its been 5 years

Change https://golang.org/cl/241179 mentions this issue: encoding/json: Enable the json encoder to distinguish between 0, null, and empty values.

Intuitively, a struct "feels" like a complex type, while "time" feels like a simple value. I believe that it is not necessary to generalize the notion of an "empty" struct. However, an "empty time" is already defined, with the IsZero() method. Add to encoding/json/encode.go/isEmptyValue():

case reflect.Struct:
	switch v := val.Interface().(type) {
        case time.Time:
    		if v.IsZero() {
			return true
		}
	}
}
Dynom commented

Intuitively, a struct "feels" like a complex type, while "time" feels like a simple value. I believe that it is not necessary to generalize the notion of an "empty" struct. However, an "empty time" is already defined, with the IsZero() method. Add to encoding/json/encode.go/isEmptyValue():

case reflect.Struct:
	switch v := val.Interface().(type) {
        case time.Time:
    		if v.IsZero() {
			return true
		}
	}
}

That could be improved to also support aliased time.Time types, so it'll need to include a step to see if it's: val.Type().ComparableTo(reflect.TypeOf((*time.Time)(nil)).Elem()).

Adding a Zeroer interface would make it a lot simpler, and you can easily do this in your own application, but I agree that being able to distinguish during unmarshalling should be part of stdlib.

Are there libraries that do solve this issue? I went through a couple, but most initiatives appear to be around Encoding, rather than the reverse.

With Mr. van der Velden's advice, I added the following case to the switch in encoding/json/encode.go:IsEmptyValue() to my Go deployment.

	case reflect.Struct:
		if v.Type().ConvertibleTo(reflect.TypeOf(time.Time{})) {
			return v.IsZero()
		}

I added the following type alttime, Optionals fields, and optionalsExpected lines to encoding/json/encode_test.go for the TestOmitEmpty test:

// issue golang.org/issues/11939
type alttime time.Time

func (t alttime) MarshalText() ([]byte, error) {
	return time.Time(t).MarshalText()
}
	// issue golang.org/issues/11939
	Tmr time.Time `json:"tmr"`
	Tmo time.Time `json:"tmo,omitempty"`
	Atr alttime   `json:"atr"`
	Ato alttime   `json:"ato,omitempty"`
 "tmr": "0001-01-01T00:00:00Z",
 "atr": "0001-01-01T00:00:00Z"

The test passes:

cd $GOROOT/src/encoding/json
go test -v
=== RUN   TestOmitEmpty
--- PASS: TestOmitEmpty (0.00s)

The omitempty tag option does not influence unmarshaling. Decoding is driven by the incoming JSON. It might be of value to add that. Here is an example, though the current behavior allows a structures fields to be updated with several Unmarshal calls, perhaps if one is initializing a structure from several JSON objects.

package main

import (
	"fmt"
	"encoding/json"
	"time"
)

func main() {
	type example struct {
		Word string    `json:",omitempty"`
		Time time.Time `json:",omitempty"`
	}

	x := example{Word: "abc", Time: time.Now()}
	buf, _ := json.MarshalIndent(x, "", "  ")
	fmt.Printf("%s\n", buf)

	buf = []byte(`{"Time": "2021-01-02T12:10:25.527-05:00"}`)
	json.Unmarshal(buf, &x)

	fmt.Printf("%+v\n", x)
} 

{
  "Word": "abc",
  "Time": "2009-11-10T23:00:00Z"
}
{**Word:abc** Time:2021-01-02 12:10:25.527 -0500 -0500}

Note that the x.Word value remains "abc". The omitempty option does not influence unmarshaling behavior.

Unmarshaling that would be sensitive to all the "json" tags in a structure requires adding a pass through the structure. I am not sure what I would even want unmarshaling to do in this case. If we want an existing structure value retained if not in the JSON, do we add a new tag option "ignoreabsent"?

For Marshaling however, I hope that the suggestion for time.Time handling of omitempty is reasonable. Documenting how to interpret "0001-01-01T00:00:00Z", and why, in REST API's does cause occasional confusion for my non-Go acquaintances. :)

So does anyone know if there has been any official word/movement on this issue? I see theres been a number of proposals but they all seems to have just been abandoned over the years?

It definitely feels like if we can tag fields as "omitempty" it should work for both marshaling and unmarshaling equally (ie, if a struct has a nill/zero field and tagged "omitempty" it should not produce a JSON field, just as a JSON field that is null/omitted will not populate a value into a struct field.)

If there was an interface like IsNiller that was very simple, just one function like IsNil() bool. Then add a tiny bit of logic into marshal.go / encode.go:

func isEmptyValue(v reflect.Value) bool {
...
	case reflect.Struct:
 		if sv, ok := v.interface().(nillerPackage.IsNiller); ok {
			return sv.IsNill()
		}
...
}

Then any struct that wanted to be skipped just implements that interface. And would pretty much take care of the issue from what I can see? Plus it leaves the exact behavior up to the developer who is implementing the struct's behavior. (ie. if time.Time doesn't want this to be the default behavior, it doesn't need to implement it, but it is still easy enough to wrap and implement it in your own struct, win win)

I might be missing something here, but it feels like a relatively small change for something that would be so helpful for everyone that works with Go for web APIs/ JSON tasks in general.

zwass commented

This feels like one of those issues which is causing real pain for the community, but sees no traction from @rsc, @adg, and others due to "not a problem at Google".

What would it take to move this proposal forward?

Xe commented

If there was an interface like IsNiller that was very simple, just one function like IsNil() bool. Then add a tiny bit of logic into marshal.go / encode.go:

I'd suggest something like this instead:

type IsZeroer interface {
  IsZero() bool
}

This matches time#Time.IsZero and semantically aligns with the Go idea that zero values are the default.

I would be willing to champion this proposal (or whatever equivalent metaphor is most accurate), I am not a Googler or on the Go team but I am willing to do the legwork needed to make this happen.

The original post(back mid 2015) suggested the IsZeroer interface, but noted there may be some compatibility issues:

"The encoding.IsZeroer interface could introduce issues with existing non-struct types that may have implemented IsZero() without consideration of omitempty."

Which is the only real reason I figured a change to IsNil would be appropriate, especially where in the cases Im thinking its useful for, will be used for omitting fields that are nil structs (or that wish to be considered nil, for example if they keep track of if they have been modified since creation or something?). Plus, if you need the new behavior for time.Time, you can very easily wrap it as a field in your own struct and implement whatever IsNil behavior you wish.

Essentially avoiding any backwards compatibility issues, and leaving the exact behavior in the hands of the developer who is defining the structs.

An approach I don't see listed in this issue is:

  • Add to the Encoder type a func (e *Encoder) SetEmptyPredicates(predicates map[reflect.Type]func(reflect.Value)bool) (see note at end)
  • Pass this map in the options used during encoding (obviously)
  • During encoding, we only call the given function for fields marked as omitempty which have a type in the map.

The advantage of this approach is:

  • We avoid a potentially expensive interface check in the vast majority of cases
  • There's no need to define an interface for people to implement (which might collide)
  • Users can decide if something is "empty" however they please, including on objects declared outside of their package (e.g. time.Time)

(The disadvantage of this solution is that it's hideous.)

I'd hesitate to let users modify the global encoding options, but maybe adapters for things in the standard library (e.g. time.Time) could be pre-added.

Note: of course we don't have to make users pass a map, they could pass one reflect.Type, func (reflect.Value) bool at a time. I recommend the function takes reflect.Value because the user can convert it to a usable thing in the most efficient way -- they might not need to convert it.

#5901 (accepted) is custom per encoder/decoder matshal/unmarshal functions

#5901 (accepted) is custom per encoder/decoder matshal/unmarshal functions

Thanks for that reference, it makes me think this approach might not get laughed all the way out of town-- however note that the omit-or-not decision is (at least in the current code) made by the caller of the marshal/unmarshal function, so it's not exactly the same problem.

as commented

I think this is a two part issue. The first part is a bug report and the second is a proposal.

Over the last ten years, it has been inconsistent and surprising that an empty struct value whose type has an omitempty tag is still not treated as empty when marshaled by default. Special cases for time.Time and opt-in interfaces are a red herring and distract from the bug present in this package.

If this bug is not going to be fixed due to perceived incompatibility (such as what happened to #31924), that should be clearly stated.

@joeshaw thanks for this, Our company really needed this feature so I just decided to implement it. https://github.com/PumpkinSeed/json

mitar commented

I made a stand-alone proposal which could address partially also concerns here: that returning nil from MarshalJSON would omit the field: #50480

It allows some other use cases (like dynamic decision based on data itself) but it can help here, too, I think (for some use cases, not all though).

I agree with everyone who thinks it's really unfortunate that all attempts to address this so far have been abandoned with the general reason of "let's not add to these packages." My only extra comment is this:

Now that Go has generics, there are strong reasons for people to implement a type like Optional[T] for representation of optional types (the main reasons are immutability and nil-safety). Such a type will likely (or necessarily?) be implemented as a struct like { defined bool; value T }. But currently there is no way to get omitempty behavior for such a thingβ€” so anyone who wants unset properties to be dropped must continue using pointers. And that's true even if T is a simple type; that is, you can get omitempty behavior with a plain bool, but that means false values will be dropped; if the semantics you want are "true, false, or undefined" and you can't use a type like Optional[bool], then your only option is to use *bool which has the disadvantages I mentioned. To me that's a not-insignificant usability problem for the package.

I think any of the approaches mentioned above would solve this for a type like Optional[T]. Using the interface approach, it would as simple as func (o Optional[T]) IsZero() bool { return !o.defined }. Using the other approaches, there would be a func (o Optional[T]) MarshalJSON which would return nil/a sentinel error if defined was false and otherwise it would call json.Marshal on the wrapped value. Etc. Being able to implement such wrapper types is a major use case for generics, and it's unfortunate that only a fixed set of value types can have this particular behavior.

@eli-darkly

I agree with everyone who thinks it's really unfortunate that all attempts to address this so far have been abandoned with the general reason of "let's not add to these packages." My only extra comment is this:

Now that Go has generics, there are strong reasons for people to implement a type like Optional[T] for representation of optional types...

Exactly our use-case, and it's infuriating that there is no way to omit fields using std library when you have an implementation such as this.

All above solutions work, but I'd personally find it most clean if return nil, ErrOmitEmpty were to be implemented. I'd love to help drive this forward, this has been an open issue for ~7 years now, what are the next steps?

what are the next steps?

Convincing the Go team to reconsider, I guess. :/

wOvAN commented

So, Go team thinks that it's ok?

https://go.dev/play/p/eWZ66utUFDZ

@wOvAN that's not an example of the problem in this issue. And yes, that's WAI. Make a new object before unmarshaling and you won't have that problem.

wOvAN commented

@lavalamp ok, if you think that the problem is in old object I'll give you another example

https://go.dev/play/p/0h94gw997Tu

to make it clear, there is no way to set "mistery_field" to null in this faked api call, that's the idea behind this and previuos pieces

Like I said, that's not the problem this issue is about. The intended way to not have that problem is to only unmarshal into newly constructed objects. Neither of your samples follows this rule.

You have a further problem, I think -- you're not going to be able to get JSON to differentiate between the concepts "I don't care" and "I want this to be nil" without putting in a fair amount of work. Kubernetes server-side apply is an example of how much work. overview; source code that does merging

wOvAN commented

null is a value. omitempty tagged field shouldn't be omitted with null value

wOvAN commented

@lavalamp Here is another example for u

https://go.dev/play/p/Cca09jBoHKG

with all new newlyConstructed object

This exchange doesn't belong on this issue. I hope an admin can delete or hide it.

It's been eight years and haven't figured out a solution yet?

tkuik commented

If this proposal is on hold, is there a new alternative that is being proposed?

There are some slow-moving plans for encoding/json/v2.

dsnet commented

Hi all, we kicked off a discussion for a possible "encoding/json/v2" package that addresses the spirit of this proposal at least for JSON. See the "omitzero" struct tag option under the "Struct tag options" section.

My team inherited maintenance duties for a Go application, and we just had to fix a bug caused by the legacy code trying to use the omitempty tag on a nested (inline) struct member for JSON serialization. I've read a lot of related issues here, and started digging through this one, but Github is terrible at presenting a large number of comments on a single issue. I'm trying to understand the current state of things -- if there's a good summary somewhere in the middle of the ~200 comments on this issue, I'd appreciate a link.

As a JS developer, I can tell you that sending an empty object value ({}) over the wire is, to a first approximation, never the desired outcome. I could point to a decade (!) of issues where people are trying to emit sensible JSON. From an outsider's perspective, "all" it would take to accomplish this is to make the existing keyword do what it says on the tin, instead of having no effect (!) -- we just want the package to follow the Principle of Least Surprise. It sounds like there is some implementation detail that makes avoiding empty-object output difficult, or would incur an unacceptable performance penalty. Personally, I would rather have good output slowly than bad output fast.

As a workaround, I've had good luck using https://github.com/clarketm/json (which is a re-implementation of encoding/json that omits empty structs) and using a type alias to prevent recursion

import (
	"encoding/json"

	jsonx "github.com/clarketm/json"
)

func (s OuterStruct) MarshalJSON() ([]byte, error) {
	// Use a type Alias to prevent Marshal recursion
	type TmpStruct OuterStruct
	// jsonx omits empty structs
	return jsonx.Marshal(TmpStruct(s))
}

The omitzero proposal in #45669 is pretty similar to the original proposal, and should cover the main use cases, so I am going to close this issue. A new issue will need to be raised for encoding/xml but it would be good for that to be its own issue.