-
-
Notifications
You must be signed in to change notification settings - Fork 957
feat(extensions): add support for x-omitzero
#1944
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.
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
Conversation
x-omitempty: true | ||
x-omitzero: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do both of these need to be set, here? Or should we only have x-omitzero
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, probably a leftover. Rebased.
e0142e6
to
48dc3d2
Compare
Any other comments? This aligns with prior art and does not break anything existing. |
fieldTags["json"] = p.JsonFieldName + | ||
stringOrEmpty(omitEmpty, ",omitempty") + | ||
stringOrEmpty(omitZero, ",omitzero") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if we should have both omitempty
and omitzero
🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, it is a valid Go struct syntax and while I cannot come up with a valid usecase now, it does not mean it does not exist. Let me put it this way: why would be a problem to support both at the same time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, thought I'd replied to this! It turns out there is likely no issue (via):
If both "omitempty" and "omitzero" are specified, the field will be omitted if the value is either empty or zero (or both).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I was not sure, I saw some articled explaining that having both would make sense but I could not google them back.
This comment was marked as resolved.
This comment was marked as resolved.
Kusari PR Analysis rerun based on - 2e18604 performed at: 2025-07-14T20:21:54Z - link to updated analysis |
(Ignore that check, it's fine!) I'm still working on this one - trying to get a good example of exactly how |
Kusari PR Analysis rerun based on - 9cfaf20 performed at: 2025-07-14T20:31:16Z - link to updated analysis |
Kusari PR Analysis rerun based on - e5d75ff performed at: 2025-07-15T08:10:07Z - link to updated analysis |
I'll follow-up with a PR to allow tuning only one or the other of |
One of the big improvements added in Go 1.24 is `omitzero`[0][1][2], which makes it possible to omit the "zero value" of a given Go type when marshaling to JSON, or if the `IsZero() bool` function on the type returns `true`. To make it possible for `oapi-codegen` to use this, we can wire in a new extension, `x-omitzero` which allows per-field tuning of this behaviour. This is something that is safe to generate alongside the `omitempty` tags[3]: > If both "omitempty" and "omitzero" are specified, the field will be omitted if the value is either empty or zero (or both). To validate this, we can add a new example for the extension, which provides testing of the expected behaviour. As this requires Go 1.24, we need to set up the boilerplate for a new module that only runs Go 1.24. Closes oapi-codegen#1899. [0]: https://www.jvt.me/posts/2025/02/12/go-omitzero-124/ [1]: https://www.bytesizego.com/blog/go-124-omitzero [2]: https://antonz.org/go-1-24/#omit-zero-values-in-json [3]: https://pkg.go.dev/encoding/json
Kusari PR Analysis rerun based on - 83987ce performed at: 2025-07-15T08:36:58Z - link to updated analysis |
x-omitzero
x-omitzero
x-omitzero
SSIA
I do not want to add this by default, not sure what consequences would be, but I want to be able to start playing around with it.
I can do add documentation as well to the README if you like it.