Fix nested allOf/anyOf merging#2471
Conversation
Closes: oapi-codegen#1905 In `mergeOpenapiSchemas`, `result.OneOf` was assigned from `s1.OneOf` and `s2.OneOf` before the transitive AllOf flattening loop, which then overwrote `s1` and `s2` entirely. Any `OneOf` (or `AnyOf`) that surfaced from a nested allOf member was silently dropped, causing the union field and variant types to disappear from the generated output. Fix: capture top-level OneOf/AnyOf first, then accumulate whatever bubbles up from each allOf merge before assigning to result. AnyOf had the same ordering bug (no `result.AnyOf` assignment at all), fixed in the same pass. Adds a reproduction schema (`NestedOneOfInAllOf`) and `TestIssue1905` to the `aggregates/allof` test fixture to lock in the correct generated output. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Greptile SummaryFixes two silent-drop bugs in
Confidence Score: 5/5Safe to merge — the change is a focused, correct fix in the schema-merge walk with a generated-output regression test locking in the expected result. The logic change is minimal and clearly correct: two accumulator variables replace an early assignment that lost data, and the missing No files require special attention.
|
| Filename | Overview |
|---|---|
| pkg/codegen/merge_schemas.go | Correctly fixes two related bugs: (1) OneOf from nested allOf members was silently dropped because result.OneOf was assigned before the transitive allOf flattening; (2) result.AnyOf was never assigned at all, causing top-level AnyOf to also be dropped. Fix captures top-level values first, accumulates bubbled-up values from each mergeAllOf call, then assigns once at the end. |
| internal/test/aggregates/allof/allof_test.go | Adds TestIssue1905 regression test that uses reflection to verify the union field is present and the two variant types are generated; follows the existing naming convention in this file. |
| internal/test/aggregates/allof/allof.gen.go | Generated output correctly reflects the new NestedOneOfInAllOf schema: struct with flat AFoo field plus union json.RawMessage, two variant types, and the full set of As/From/Merge/Marshal/Unmarshal helpers. New runtime import is appropriate. Diff is proportional to the schema change with no unrelated churn. |
| internal/test/aggregates/allof/spec.yaml | Adds NestedOneOfInAllOf schema to the existing aggregates/allof leaf — correct placement per project conventions. Provenance comment follows the established format used by other entries in the file. |
Reviews (1): Last reviewed commit: "Fix nested allOf/anyOf merging" | Re-trigger Greptile
Closes: #1905
In
mergeOpenapiSchemas,result.OneOfwas assigned froms1.OneOfands2.OneOfbefore the transitive AllOf flattening loop, which then overwrotes1ands2entirely. AnyOneOf(orAnyOf) that surfaced from a nested allOf member was silently dropped, causing the union field and variant types to disappear from the generated output.Fix: capture top-level OneOf/AnyOf first, then accumulate whatever bubbles up from each allOf merge before assigning to result. AnyOf had the same ordering bug (no
result.AnyOfassignment at all), fixed in the same pass.Adds a reproduction schema (
NestedOneOfInAllOf) andTestIssue1905to theaggregates/alloftest fixture to lock in the correct generated output.