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.

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

"omitempty" does not work with non-pointer structs #72

Closed
rehmsen opened this issue May 15, 2023 · 2 comments · Fixed by #75
Closed

"omitempty" does not work with non-pointer structs #72

rehmsen opened this issue May 15, 2023 · 2 comments · Fixed by #75
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@rehmsen
Copy link
Contributor

rehmsen commented May 15, 2023

This was reported and fixed for an individual case in #66. #66 (comment) lists a few more fields with the same problem, but that list is also not complete. As one more example, I ran into this for ErrorResponseBody.Error:

Error ErrorMessage `json:"error,omitempty"`

The issue is that Go cannot set this fields to nil and will instead when parsing create an empty struct. Then when that message is serialized again, it will not be omitted as requested, but the empty values will be written to JSON, in my case leading to something like

"body":{"error":{"id":0,"format":"","showUser":false}}

instead of

"body":{}

and that then confuses VS Code and others.

I think the correct fix would be to go through and make all non-pointer struct fields that have omitempty on them, and turn them into pointers. Do you agree with this solution? Then I could look into preparing a PR for this.

@suzmue
Copy link
Collaborator

suzmue commented May 18, 2023

Thanks for the bug! That seems like a reasonable solution. Please feel free to send a PR.

@suzmue suzmue added bug Something isn't working help wanted Extra attention is needed labels May 18, 2023
rehmsen added a commit to rehmsen/go-dap that referenced this issue May 19, 2023
This is needed so that unset JSON values get initialized to `nil` in Go
instead of the zero struct. The zero struct is indistinguishable from an
explicitly set struct with zero values, and will get serialized despite
"omitempty". `nil` on the other hand will be dropped, as expected.

Fixes google#72.
@rehmsen
Copy link
Contributor Author

rehmsen commented May 19, 2023

Thanks for confirming Suzy. I have send you a PR. Would love your review!

I cannot edit assignee, it seems. Feel free to assign the issue to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants