fix(go): nullable additionalProperties produces wrong map value type#268
Closed
PatrickKoss wants to merge 4 commits intostackitcloud:mainfrom
Closed
fix(go): nullable additionalProperties produces wrong map value type#268PatrickKoss wants to merge 4 commits intostackitcloud:mainfrom
PatrickKoss wants to merge 4 commits intostackitcloud:mainfrom
Conversation
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.
Problem
When an OpenAPI schema has
additionalPropertieswithnullable: trueon the value type, the Go generator emitsmap[string]stringinstead ofmap[string]*string— dropping the nullability of the map value. This makes it impossible for SDK consumers to sendnullvalues, which is the documented signal in many of our APIs to "delete this entry."Example from the mlflow API:
Generated today:
Expected:
Root cause
This is an upstream limitation in
AbstractGoCodegen.getTypeDeclaration(verified by decompiling the generator JAR):nullable: trueflag triggers a*prefix on the item type —[]*stringis produced correctly.map[string]stringis always produced.We can't patch upstream Java codegen from this repo, so the fix lives in our Mustache templates.
Fix
Added a small reusable Mustache partial
field_type.mustachethat returns the corrected type when the property is a map with a nullable value, and the upstream-computeddataTypeotherwise:Applied to both generation paths:
languages/golang/templates/, OpenAPI generator v7.22.0) — copied the upstreammodel_simple.mustacheinto the repo and replaced{{{dataType}}}/{{{vendorExtensions.x-go-base-type}}}with{{>field_type}}at the struct field, both getters,*Okreturns, setter, doc comment, andWithoutEmbeddedStructblock.languages/golang/compat-layer/templates/, OpenAPI generator v6.6.0) — patched the three{{dataType}}sites in the{{#isContainer}}{{^isFreeFormObject}}{{^isArray}}block (AttributeType/ArgType/RetTypealiases).The discriminator is
additionalProperties.isNullable. Schemas withoutnullable: trueon the inner type are unaffected — the partial collapses to{{{dataType}}}and the existing rendering is preserved unchanged.Files changed
languages/golang/templates/field_type.mustachelanguages/golang/templates/model_simple.mustachelanguages/golang/compat-layer/templates/field_type.mustachelanguages/golang/compat-layer/templates/model_simple.mustache(3 lines)Verification
Generated against the mlflow API spec (which has both nullable and non-nullable
additionalPropertiesschemas) and confirmed:Schemas using
LabelPatch(nullable additionalProperties) — every reference is now consistent at*map[string]*string:Schemas using
Labels(non-nullable additionalProperties) — zero diff vs. main:go build ./...on the generated module succeeds. A pre-existing unused-import warning inclient.gois unrelated to this change (verified by regenerating with the unpatched template — same warning).Breaking change note
For any service already on the compat-layer path whose spec uses
additionalProperties: { ..., nullable: true }, the emitted Go type changes from*map[string]Tto*map[string]*T. This is a breaking change for downstream Go consumers of those SDKs, but it's the correct behavior — without it, callers can't representnullvalues at all. Worth flagging in the next SDK release notes.The compat-layer templates are already marked deprecated (
Will be removed after 2026-09-30), so this change is consistent with the migration trajectory.