GeezerVison is showing github.com — Overhaul anonymous schema hoisting by mromaszewicz · Pull Request #2348 · oapi-codegen/oapi-codegen
Conversation
Closes: #1900, #1496
BREAKING: this rename affects generated type names for inline oneOf, anyOf, and additionalProperties response schemas. Two prior name patterns are gone and folded into one canonical form:
GetPets_200_Data_Item → GetPets200JSONResponseBody_Data_Item
GetPets200JSONResponse_Data_Item → GetPets200JSONResponseBody_Data_Item
The pre-existing types had no As/From/Merge accessors and the response roots were anonymous structs that could not be method-bound — that's the bug being fixed (oapi-codegen issues #1900 and #1496). User code referencing the old names could only have done so via reflection or json.RawMessage round-trips, since the types were practically unusable. The documented workaround (hoist to #/components/schemas/ and $ref it) sidesteps this surface entirely, so users who hit the bug and switched to the workaround see no name change. Component schema names, <Op>Params, request body types, the strict envelope name <Op><Status><Tag>Response, and the client-with-responses envelope name <Op>Response are all stable.
Inline oneOf/anyOf schemas at any operation root or nested below previously bypassed the union-accessor / additionalProperties boilerplate passes, leaving generated wrapper types method-less (or, for response roots, anonymous structs that no method could be attached to). This change unifies the boilerplate-emission pipeline so every schema root — components, parameters, request bodies, response bodies — gets the same method-emitting passes.
Codegen pipeline:
- GenerateTypeDefinitions now builds two slices. allTypes stays components-only and feeds GenerateTypes (typedef.tmpl). boilerplateTypes is the wider set — components plus op.TypeDefinitions plus each ResponseTypeDefinition's AdditionalTypeDefinitions — and feeds the method-emitting passes (enum scanning, additionalProperties marshalers, union accessors, union+additionalProperties combined). Operation-derived types now get the same method emission as components, while declarations stay in their per-context templates.
- GenerateResponseDefinitions hoists inline response-root schemas with UnionElements / HasAdditionalProperties to a synthetic TypeDefinition named ResponseBody. The strict envelope keeps its Response name and references the body type from inside (struct envelope) or as an alias (no-headers case). Two distinct names so the strict envelope never self-references.
- GetResponseTypeDefinitions (client-with-responses path) computes the same body name and points the JSON field at it.
- Multi-content-type disambiguation generalized: the responseTypeName itself includes the content-type tag, so separate JSON/XML/YAML content types in the same response naturally produce distinct names. New isMediaTypeSupported helper hardcodes today's expectations (intended to be user-configurable in a future change).
- GenerateBodyDefinitions and GetResponseTypeDefinitions skip the local hoist when the operation came from an externally-ref'd path item — the imported package already declared the same hoisted type (PathToTypeName is deterministic), so the schema's RefType is set to externalPkg. instead. Avoids the AsExternalRef0Foo accessor names that would otherwise appear in the importing package.
- New OperationDefinition.PathItemRef carries the path item ref through to GetResponseTypeDefinitions; GenerateBodyDefinitions and GenerateResponseDefinitions take it as an explicit parameter for symmetry.
- ensureExternalRefsInSchema also patches UnionElements with the imported package qualifier when the enclosing schema came from an external file.
- Schema.HasCustomMarshalJSON returns true for inline-but-external union shapes. The strict envelope is rendered as a defined type (
type X externalRef0.Y), so methods on Y don't transfer; the delegator emitted by the strict template (which calls externalRef0.Y(t).MarshalJSON()) is needed for the response to serialize the union payload instead of the empty struct value.
- GenerateUnionBoilerplate and GenerateUnionAndAdditionalProopertiesBoilerplate dedup by TypeName, matching the pre-existing pattern in GenerateAdditionalPropertyBoilerplate.
Templates:
- client-with-responses.tmpl no longer emits AdditionalTypeDefinitions declarations inline. Those nested response types now flow through op.TypeDefinitions (collected in GenerateTypeDefsForOperation by upstream PR Moving all middleware into a single Git repo #2284) and get declared once via param-types.tmpl with union/additionalProperties accessor methods attached.
- strict-server templates (strict-interface.tmpl, strict-fiber-interface.tmpl, strict-iris-interface.tmpl) encode the response body without touching the unexported union field when the schema is from an external package — the type's MarshalJSON delegator handles it instead.
Tests:
- New internal/test/anonymous_inner_hoisting fixture exercises As/From/Merge round-trips on every shape: response root oneOf, response root anyOf, response items oneOf, deeply nested oneOf, request body root oneOf, request body property oneOf, plus a cross-branch Merge test.
- New internal/test/issues/issue-1378/fooservice_test.go TestExternalRefUnionResponseSerialization locks in the cross-package union response serialization fix; without the HasCustomMarshalJSON change it produces
{} instead of the union payload.
Generated fixtures across internal/test and examples regenerated to reflect the new method emission and naming.
Two follow-up cleanups in the strict-server templates were identified during review and deferred — extracting the .union-vs-MarshalJSON access decision into a Go-side EncodeAccessor helper, and pre-computing the full strict envelope declaration line in Go. Both are notes, not blockers. Issue #2010 (cross-module strict-server type embedding) is adjacent but a separate fix surface.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
greptile-apps[bot] reacted with thumbs up emoji
BREAKING: this rename affects generated type names for inline `oneOf`,
`anyOf`, and `additionalProperties` response schemas. Two prior name
patterns are gone and folded into one canonical form:
GetPets_200_Data_Item → GetPets200JSONResponseBody_Data_Item
GetPets200JSONResponse_Data_Item → GetPets200JSONResponseBody_Data_Item
The pre-existing types had no As/From/Merge accessors and the response
roots were anonymous structs that could not be method-bound — that's
the bug being fixed (oapi-codegen issues oapi-codegen#1900 and oapi-codegen#1496). User code
referencing the old names could only have done so via reflection or
json.RawMessage round-trips, since the types were practically
unusable. The documented workaround (hoist to `#/components/schemas/`
and `$ref` it) sidesteps this surface entirely, so users who hit the
bug and switched to the workaround see no name change. Component
schema names, `<Op>Params`, request body types, the strict envelope
name `<Op><Status><Tag>Response`, and the client-with-responses
envelope name `<Op>Response` are all stable.
Inline `oneOf`/`anyOf` schemas at any operation root or nested below
previously bypassed the union-accessor / additionalProperties
boilerplate passes, leaving generated wrapper types method-less (or,
for response roots, anonymous structs that no method could be
attached to). This change unifies the boilerplate-emission pipeline
so every schema root — components, parameters, request bodies,
response bodies — gets the same method-emitting passes.
Codegen pipeline:
- GenerateTypeDefinitions now builds two slices. allTypes stays
components-only and feeds GenerateTypes (typedef.tmpl).
boilerplateTypes is the wider set — components plus
op.TypeDefinitions plus each ResponseTypeDefinition's
AdditionalTypeDefinitions — and feeds the method-emitting passes
(enum scanning, additionalProperties marshalers, union accessors,
union+additionalProperties combined). Operation-derived types now
get the same method emission as components, while declarations
stay in their per-context templates.
- GenerateResponseDefinitions hoists inline response-root schemas
with UnionElements / HasAdditionalProperties to a synthetic
TypeDefinition named <Op><Status><Tag>ResponseBody. The strict
envelope keeps its <Op><Status><Tag>Response name and references
the body type from inside (struct envelope) or as an alias
(no-headers case). Two distinct names so the strict envelope
never self-references.
- GetResponseTypeDefinitions (client-with-responses path) computes
the same body name and points the JSON<status> field at it.
- Multi-content-type disambiguation generalized: the
responseTypeName itself includes the content-type tag, so
separate JSON/XML/YAML content types in the same response
naturally produce distinct names. New isMediaTypeSupported
helper hardcodes today's expectations (intended to be
user-configurable in a future change).
- GenerateBodyDefinitions and GetResponseTypeDefinitions skip the
local hoist when the operation came from an externally-ref'd
path item — the imported package already declared the same
hoisted type (PathToTypeName is deterministic), so the schema's
RefType is set to externalPkg.<name> instead. Avoids the
AsExternalRef0Foo accessor names that would otherwise appear in
the importing package.
- New OperationDefinition.PathItemRef carries the path item ref
through to GetResponseTypeDefinitions; GenerateBodyDefinitions
and GenerateResponseDefinitions take it as an explicit
parameter for symmetry.
- ensureExternalRefsInSchema also patches UnionElements with the
imported package qualifier when the enclosing schema came from
an external file.
- Schema.HasCustomMarshalJSON returns true for inline-but-external
union shapes. The strict envelope is rendered as a defined type
(`type X externalRef0.Y`), so methods on Y don't transfer; the
delegator emitted by the strict template (which calls
externalRef0.Y(t).MarshalJSON()) is needed for the response to
serialize the union payload instead of the empty struct value.
- GenerateUnionBoilerplate and
GenerateUnionAndAdditionalProopertiesBoilerplate dedup by
TypeName, matching the pre-existing pattern in
GenerateAdditionalPropertyBoilerplate.
Templates:
- client-with-responses.tmpl no longer emits AdditionalTypeDefinitions
declarations inline. Those nested response types now flow through
op.TypeDefinitions (collected in GenerateTypeDefsForOperation by
upstream PR oapi-codegen#2284) and get declared once via param-types.tmpl with
union/additionalProperties accessor methods attached.
- strict-server templates (strict-interface.tmpl,
strict-fiber-interface.tmpl, strict-iris-interface.tmpl) encode
the response body without touching the unexported union field
when the schema is from an external package — the type's
MarshalJSON delegator handles it instead.
Tests:
- New internal/test/anonymous_inner_hoisting fixture exercises
As/From/Merge round-trips on every shape: response root oneOf,
response root anyOf, response items oneOf, deeply nested
oneOf, request body root oneOf, request body property oneOf,
plus a cross-branch Merge test.
- New internal/test/issues/issue-1378/fooservice_test.go
TestExternalRefUnionResponseSerialization locks in the
cross-package union response serialization fix; without the
HasCustomMarshalJSON change it produces `{}` instead of the
union payload.
Generated fixtures across internal/test and examples regenerated
to reflect the new method emission and naming.
Two follow-up cleanups in the strict-server templates were
identified during review and deferred — extracting the
.union-vs-MarshalJSON access decision into a Go-side
EncodeAccessor helper, and pre-computing the full strict envelope
declaration line in Go. Both are notes, not blockers. Issue oapi-codegen#2010
(cross-module strict-server type embedding) is adjacent but a
separate fix surface.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Greptile Summary
This PR overhauls anonymous schema hoisting so that inline oneOf/anyOf/additionalProperties schemas at every operation root (response bodies, request bodies, nested positions) receive the same As/From/Merge accessor methods and MarshalJSON/UnmarshalJSON boilerplate that component-level schemas already got. It introduces a deliberate breaking rename — ...<Op>_<Status>_Data_Item and ...<Op><Status>JSONResponse_Data_Item → ...<Op><Status>JSONResponseBody_Data_Item — and fixes cross-package union serialization for strict-server envelopes wrapping external union types. The implementation is well-structured and the new anonymous_inner_hoisting fixture provides thorough round-trip coverage. The PR description's claim that the renamed types were "practically unusable" slightly understates the migration surface: types like Test200JSONResponse_Item carried exported struct fields (Field1, Field2, AdditionalProperties) that downstream users could reference directly.
Confidence Score: 4/5
Broadly safe to merge; the breaking renames are intentional and documented, all three strict-server template variants are updated, and the new test fixtures give good coverage of the fixed paths.
Only P2 findings: an undocumented dead-code removal in GenerateParamsTypes, a silent behavioral shift in the supportedCount refType gate for mixed JSON+XML responses, and a double GetResponseTypeDefinitions call that is correct but worth a comment. No P0/P1 issues found.
pkg/codegen/operations.go — the supportedCount gating change and GenerateParamsTypes description removal deserve a second look before merge.
Important Files Changed
| Filename |
Overview |
| pkg/codegen/codegen.go |
Splits allTypes into allTypes (components) + boilerplateTypes (wider); adds dedup seen-maps to GenerateUnionBoilerplate and GenerateUnionAndAdditionalProopertiesBoilerplate; introduces double GetResponseTypeDefinitions call |
| pkg/codegen/operations.go |
Core codegen logic: adds PathItemRef to OperationDefinition, adds responseBodyTypeName hoist, changes supportedCount gating (removes ResolveTypeNameCollisions gate), removes s.Description assignment from GenerateParamsTypes, removes per-operation additionalProperties boilerplate from GenerateTypesForOperations |
| pkg/codegen/schema.go |
HasCustomMarshalJSON now returns true for external inline unions (IsExternalRef), enabling MarshalJSON delegator for strict envelopes that alias an external package type |
| pkg/codegen/externalref.go |
Adds externalPackageFor helper; ensureExternalRefsInSchema now qualifies UnionElements with the imported package name and removes the struct-prefix early-return guard |
| pkg/codegen/template_helpers.go |
Adds isMediaTypeSupported helper consolidating JSON/YAML/XML detection for supportedCount; clean and consistent with the switch in GetResponseTypeDefinitions |
| pkg/codegen/templates/client-with-responses.tmpl |
Removes inline AdditionalTypeDefinitions declarations; those types now flow through op.TypeDefinitions and are declared via param-types.tmpl |
| pkg/codegen/templates/strict/strict-interface.tmpl |
Correctly guards .union access with (not .Schema.IsExternalRef) — external union types use MarshalJSON delegation instead; change mirrors fiber and iris templates |
| internal/test/anonymous_inner_hoisting/client_test.go |
New test fixture covering As/From/Merge round-trips for all hoisted union shapes; comprehensive and well-structured |
| internal/test/issues/issue-1378/fooservice/fooservice_test.go |
New regression test locking in cross-package union response serialization fix; would have produced {} without HasCustomMarshalJSON change |
| internal/test/any_of/codegen/inline/openapi.gen.go |
Breaking rename GetPets200JSONResponse_Data_Item → GetPets200JSONResponseBody_Data_Item; adds As/From/Merge accessors for the union type; removes defunct inline anonymous struct |
| internal/test/strict-server/chi/types.gen.go |
UnionExample200JSONResponseBody replaces anonymous inline struct; gains full As/From/Merge/MarshalJSON/UnmarshalJSON methods |
Reviews (1): Last reviewed commit: "Overhaul anonymous schema hoisting" | Re-trigger Greptile
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Comment on lines
486
to
491
|
|
if err != nil {
|
|
|
return nil, fmt.Errorf("error dereferencing response Ref: %w", err)
|
|
|
}
|
|
|
if jsonCount > 1 && util.IsMediaTypeJson(contentTypeName) {
|
|
|
if supportedCount > 1 {
|
|
|
if resolved := resolvedNameForRefPath(responseRef.Ref, contentTypeName); resolved != "" {
|
|
|
refType = resolved + mediaTypeToCamelCase(contentTypeName)
|
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.
supportedCount change silently removes ResolveTypeNameCollisions gate
The old refType disambiguation block was gated by jsonCount > 1 && util.IsMediaTypeJson(contentTypeName) — it fired only for multiple JSON content-types. The new supportedCount > 1 also fires when a single JSON response is combined with at least one XML or YAML content-type for the same status code. For those specs the resolvedNameForRefPath branch now runs unconditionally, which can change the rendered refType for $ref-ed responses. This behavioral shift is not mentioned in the PR description; users relying on the old stable refType for JSON+XML mixed responses could see unexpected renames here.
if supportedCount > 1 {
if resolved := resolvedNameForRefPath(responseRef.Ref, contentTypeName); resolved != "" {
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced May 1, 2026
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
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.
|