Overhaul anonymous schema hoisting#2348
Conversation
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) <noreply@anthropic.com>
Greptile SummaryThis PR overhauls anonymous schema hoisting so that inline Confidence Score: 4/5Broadly 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.
|
| 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
| 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.
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 != "" {
Closes: #1900, #1496
BREAKING: this rename affects generated type names for inline
oneOf,anyOf, andadditionalPropertiesresponse 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$refit) 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>Responseare all stable.Inline
oneOf/anyOfschemas 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:
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.Templates:
Tests:
{}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.