Propagate one-sided type and format in allOf merges - #2526
Merged
mromaszewicz merged 1 commit intoAug 16, 2026
Merged
mromaszewicz merged 1 commit into
mromaszewicz merged 1 commit into
Conversation
MergeSchemas only errored on a type conflict when both members declared a type; otherwise it kept the first member's type, silently discarding the second's. Because allOf members merge pairwise in order, the generated Go shape depended on member ordering. Format had the mirror-image problem: any mismatch errored, including set-vs-unset, which broke the allOf decorator idiom over refs to format-carrying scalars. Both now follow the same rule: when exactly one member declares a type/format it propagates to the merged schema; when both declare and they differ, the existing errors remain. Fixes oapi-codegen#2524 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Greptile SummaryThe PR makes the current allOf merger propagate a type or format declared by only one member while preserving errors for conflicting declarations.
Confidence Score: 5/5The PR appears safe to merge with no actionable correctness, security, or maintainability issues identified. The merge logic handles absent declarations symmetrically, retains conflict errors when both members declare incompatible values, preserves the legacy compatibility path, and includes focused unit and generated-output regression coverage.
|
| Filename | Overview |
|---|---|
| pkg/codegen/merge_schemas.go | Propagates one-sided type and format values during current-path allOf merging while retaining explicit conflict detection. |
| pkg/codegen/merge_schemas_test.go | Adds focused tests for symmetric propagation, unchanged typeless behavior, and preserved type/format conflict errors. |
| internal/test/aggregates/allof/spec_new_merge.yaml | Adds category-appropriate regression schemas for order-independent primitive, union, and format-carrying allOf merges. |
| internal/test/aggregates/allof/allof_test.go | Adds compile-time shape checks for the newly generated aliases. |
| internal/test/aggregates/allof/allof_new_merge.gen.go | Contains the expected generated string, any, and UUID aliases without unrelated generated-code drift. |
Reviews (1): Last reviewed commit: "Propagate one-sided type and format in a..." | Re-trigger Greptile
This was referenced Aug 19, 2026
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
1 participant
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.
Fixes #2524.
Problem
mergeOpenapiSchemasonly errored on a type conflict when both members declared a type; otherwise it kept the first member's type, silently discarding the second's. Because allOf members merge pairwise in order, the generated Go shape depended on member ordering (see the issue for the repro — a struct in one order,anyin the other).Format had the mirror-image problem: any mismatch errored, including set-vs-unset, so the allOf decorator idiom over a
$refto a format-carrying scalar failed generation outright:Fix
Both fields now follow the same one-sidedness rule: when exactly one member declares a type/format, it propagates to the merged schema; when both declare and they differ, the existing errors remain.
Compatibility notes
make generatechanges no existing committed.gen.go— the entire test corpus is unaffected.propertiesdoesn't constrain non-object instances, soallOf: [{properties…}, {type: string}]only ever validates strings). Specs hiding two conflicting types behind a typeless member (e.g.[{properties…}, {type: string}, {type: number}]) now hit the existing incompatible-types error instead of silently generating a struct; such specs are unsatisfiable.compatibility.old-merge-schemasroutes to the untouched legacy codepath and remains the escape hatch.Tests
merge_schemas_test.gofor both propagation rules, order symmetry, and the preserved error cases.internal/test/aggregates/allof/: the issue's repro in both member orders (both lower toany), single-type in both orders (both lower tostring), and the format-decorator idiom in both orders (both lower toopenapi_types.UUID), pinned byTestIssue2524.make tidy && make test && make generate && make lintall pass.🤖 Generated with Claude Code