Skip to content

Commit c29d918

Browse files
authored
Preserve typed property when allOf branches overlap (#84)
1 parent 6121bcb commit c29d918

3 files changed

Lines changed: 120 additions & 2 deletions

File tree

pkg/codegen/schema_merge.go

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -713,18 +713,34 @@ func mergeOpenapiSchemas(s1, s2 *base.Schema) (*base.Schema, error) {
713713
// Required. We merge these.
714714
result.Required = append(s1.Required, s2.Required...)
715715

716-
// We merge all properties
716+
// We merge all properties. When both schemas declare a property
717+
// with the same name, an annotation-only override (carrying only
718+
// fields like `example` or `description`) must not overwrite a
719+
// sibling that actually shapes the Go type. Otherwise we keep the
720+
// previous "s2 wins" behavior, since both sides genuinely shape
721+
// the type and the second declaration is the more specific one in
722+
// an allOf chain.
723+
//
724+
// We deliberately pick one of the original SchemaProxies rather
725+
// than fabricating a merged proxy via CreateSchemaProxy: downstream
726+
// code reads GoLow().GetReference() to attribute properties back to
727+
// the spec, and a synthetic proxy has no low-level backing.
717728
for k, v := range s1.Properties.FromOldest() {
718729
if result.Properties == nil {
719730
result.Properties = orderedmap.New[string, *base.SchemaProxy]()
720731
}
721732
result.Properties.Set(k, v)
722733
}
723734
for k, v := range s2.Properties.FromOldest() {
724-
// TODO: detect conflicts
725735
if result.Properties == nil {
726736
result.Properties = orderedmap.New[string, *base.SchemaProxy]()
727737
}
738+
739+
if existing, exists := result.Properties.Get(k); exists {
740+
if isAnnotationOnlySchema(v.Schema()) && !isAnnotationOnlySchema(existing.Schema()) {
741+
continue
742+
}
743+
}
728744
result.Properties.Set(k, v)
729745
}
730746

@@ -791,6 +807,45 @@ func getSchemaType(schema *base.Schema) []string {
791807
return nil
792808
}
793809

810+
// isAnnotationOnlySchema reports whether a schema has no fields that
811+
// influence the generated Go type. Schemas like `{ example: cancelled }`
812+
// or `{ description: "..." }` are annotation-only and must not be
813+
// allowed to overwrite a sibling declaration that actually carries a
814+
// type, enum, or sub-schema in an allOf merge.
815+
func isAnnotationOnlySchema(s *base.Schema) bool {
816+
if s == nil {
817+
return true
818+
}
819+
if len(s.Type) > 0 {
820+
return false
821+
}
822+
if len(s.AllOf) > 0 || len(s.OneOf) > 0 || len(s.AnyOf) > 0 {
823+
return false
824+
}
825+
if s.If != nil || s.Then != nil || s.Else != nil || s.Not != nil {
826+
return false
827+
}
828+
if s.Properties != nil && s.Properties.Len() > 0 {
829+
return false
830+
}
831+
if s.PatternProperties != nil && s.PatternProperties.Len() > 0 {
832+
return false
833+
}
834+
if s.Items != nil {
835+
return false
836+
}
837+
if len(s.Enum) > 0 {
838+
return false
839+
}
840+
if s.AdditionalProperties != nil {
841+
return false
842+
}
843+
if s.Const != nil {
844+
return false
845+
}
846+
return true
847+
}
848+
794849
// mergeNullable merges two nullable pointers using a union (more permissive) approach.
795850
// If either is true, the result is true. If both are false or nil, the result is nil.
796851
// This allows merging schemas where one specifies nullable and another doesn't.

pkg/codegen/schema_merge_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,35 @@ func TestMergeOpenapiSchemas(t *testing.T) {
119119
assert.NotEmpty(t, code)
120120
})
121121

122+
t.Run("overlapping property keeps typed declaration", func(t *testing.T) {
123+
// When two allOf branches declare the same property and only
124+
// one of them gives it a type (the other only attaches an
125+
// annotation such as `example`), the typed declaration must
126+
// survive the merge. Previously the second branch silently
127+
// overwrote the first, turning a `status: string` into an
128+
// empty struct.
129+
contents, err := os.ReadFile("testdata/merge-overlapping-property-typed-wins.yml")
130+
require.NoError(t, err)
131+
132+
opts := Configuration{
133+
PackageName: "testpkg",
134+
Output: &Output{
135+
UseSingleFile: true,
136+
},
137+
}
138+
139+
code, err := Generate(contents, opts)
140+
require.NoError(t, err)
141+
combined := code.GetCombined()
142+
143+
assert.Contains(t, combined, "type InvoiceCancellation struct",
144+
"InvoiceCancellation should be a struct")
145+
assert.Regexp(t, `(?m)Status\s+\*?string`, combined,
146+
"Status should keep the string type from the referenced Invoice schema, not collapse to an empty struct from the annotation-only override")
147+
assert.NotRegexp(t, `(?m)Status\s+\*?struct\{\}`, combined,
148+
"Status must not be generated as struct{}; that would mean the annotation-only override dropped the typed declaration")
149+
})
150+
122151
t.Run("allOf with sibling properties preserves all fields", func(t *testing.T) {
123152
contents, err := os.ReadFile("testdata/allof-with-sibling-properties.yml")
124153
require.NoError(t, err)
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
openapi: 3.0.0
2+
info:
3+
title: Overlapping property in allOf preserves the typed declaration
4+
version: 1.0.0
5+
paths:
6+
/test:
7+
get:
8+
responses:
9+
'200':
10+
description: OK
11+
content:
12+
application/json:
13+
schema:
14+
$ref: '#/components/schemas/InvoiceCancellation'
15+
components:
16+
schemas:
17+
Invoice:
18+
type: object
19+
properties:
20+
id:
21+
type: string
22+
status:
23+
type: string
24+
enum:
25+
- open
26+
- paid
27+
- cancelled
28+
InvoiceCancellation:
29+
allOf:
30+
- $ref: '#/components/schemas/Invoice'
31+
- type: object
32+
properties:
33+
status:
34+
example: cancelled

0 commit comments

Comments
 (0)