fix(golang): don't apply omitempty to nullable/union fields under --omit-empty#3057
Merged
Conversation
…mit-empty (#2515) When a oneOf branch unification makes a nullable/union-typed property optional, --omit-empty tagged it json:"field,omitempty". Since Go's omitempty on a pointer conflates "absent" with an explicit JSON null, this silently dropped required null values (e.g. {"kind":"one","b":null}) from marshalled output, producing schema-invalid JSON. canOmitEmpty already excluded union/null/any-typed properties from omitempty in the default (flag-off) path; --omit-empty unconditionally bypassed that exclusion. Now the exclusion applies regardless of the flag, while --omit-empty still widens omitempty to other optional property types as intended. Added test/inputs/schema/nullable-optional-one-of.schema (a oneOf where one variant requires a nullable property, the other doesn't) run with omit-empty via GoLanguage.quickTestRendererOptions, and updated the existing omit-empty fixture's expected output to reflect that nullable fields now preserve null instead of being dropped. Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
…3057) Co-Authored-By: Claude <noreply@anthropic.com>
…a on cjson (#2515) The .1.fail.enum.json sample added in #3057 relies on the generated code rejecting an invalid "kind" enum value, but cjson's enum decoder silently falls back to the first variant instead of failing. Add the schema to the existing skipsEnumValueValidation list, which already covers this same gap for cjson/crystal/swift/haskell/typescript-zod/typescript-effect-schema. Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
--omit-emptytags all optional Go struct fields with,omitempty,including fields whose type is nullable/union (e.g.
*stringfor a["string", "null"]schema type). Since Go'sencoding/jsontreats anilpointer withomitemptyas "absent", this conflates "field notpresent" with "field explicitly
null" and silently drops legitimatenullvalues from marshalled output.This surfaces concretely with
oneOf: when a property isrequiredandnullable in one branch but optional in another, unifying the branches
makes it an optional property on the merged Go struct. With
--omit-emptythat property gets,omitempty, so a document validagainst the branch that requires it as
null(e.g.{"kind":"one","b":null}) round-trips to{"kind":"one"}— no longervalid against any branch.
Root cause
canOmitEmptyinpackages/quicktype-core/src/language/Golang/utils.tsalready excludes
union/null/any-typed properties fromomitemptyin the default (flag-off) path, specifically because of this
absent-vs-null conflation. But the
--omit-emptybranch bypassed thatexclusion unconditionally:
Fix
Apply the same
union/null/anyexclusion under--omit-empty:--omit-emptystill widensomitemptyto other optional property types(its intended purpose); it now no longer overrides the deliberate
exclusion for nullable/union types.
Test coverage
test/inputs/schema/nullable-optional-one-of.schema— aoneOfwith two variants sharing a nullable property
bthat'srequiredinone branch and optional in the other (mirrors the issue's repro), plus
a positive sample
nullable-optional-one-of.1.json({"kind":"one","b":null}).Wired into
GoLanguage.quickTestRendererOptionsintest/languages.tswith
{"omit-empty": "true"}so it runs as a schema fixture test.omit-emptyJSON fixture's expected output(
test/inputs/json/priority/omit-empty.out.omit-empty.json) to reflectthat a nullable optional field now preserves
nullinstead of beingdropped by
omitempty.I verified locally, before involving the fix, that reverting it makes
the new fixture's generated Go tag regress back to
json:"b,omitempty"(reproducing the bug), confirming the test actuallyexercises the fix.
Verification
npm run buildpasses.(
node dist/index.js --src-lang schema --lang go --just-types-and-package --omit-empty def.json):now generates
B *string \json:"b"`(noomitempty`), matching theexpected output.
QUICKTEST=true FIXTURE=golang script/test— 47/47 pass.QUICKTEST=true FIXTURE=schema-golang script/test— 73/73 pass(including the new
nullable-optional-one-of.schemacase, both withand without
omit-empty).npm run test:unit— 176/176 pass.npx biome checkon changed files — clean.--omit-emptystill applies to genuinely non-nullableoptional fields (unaffected by this change).
runner, CI matrix across other languages) is left to CI.
Fixes #2515
🤖 Generated with Claude Code