Skip to content

Commit bb4e2af

Browse files
mromaszewiczclaude
andauthored
experimental: merge allOf member extensions before type generation (#22)
Refactor allOf type generation to merge extensions from all allOf members into a composite before deciding what code to generate, mirroring V2's MergeSchemas approach. This was surfaced by the issue 1957 test, which uses the standard OpenAPI 3.0 pattern of combining a $ref with extension-only allOf members: id: allOf: - $ref: '#/components/schemas/ID' # has x-go-type - x-go-type-skip-optional-pointer: true # extension-only Previously, V3 preserved the allOf structure as-is, so generateAllOfType() only collected struct fields from each member. When a $ref target was a type-override alias (no struct properties), there were no fields to collect, producing an empty struct. Extensions on allOf members were also never propagated to the parent field. The fix adds three pieces: - mergeExtensions(dst, src) in extension.go: field-by-field merge helper for the Extensions struct (later values win). - mergeAllOfExtensions() + hasAnyFields() in codegen.go: iterates allOf member descriptors, resolves $ref targets from the schema index, and merges all extensions. generateAllOfType() now calls this at the top and, when the composite has no struct fields but carries a TypeOverride, emits a type alias instead of an empty struct. - GenerateStructFields() in typegen.go: when a property is itself an allOf, merges extensions from its allOf members into the property's extensions, so x-go-type-skip-optional-pointer propagates to the field declaration in the parent struct. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 3997e59 commit bb4e2af

File tree

5 files changed

+159
-20
lines changed

5 files changed

+159
-20
lines changed

experimental/codegen/internal/codegen.go

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -641,6 +641,7 @@ type allOfMemberInfo struct {
641641
unionType string // non-empty if this member is a oneOf/anyOf union
642642
unionDesc *SchemaDescriptor
643643
required []string // required fields from this allOf member
644+
refType string // non-empty if this member is a $ref (the JSON pointer)
644645
}
645646

646647
// generateAllOfType generates a struct with flattened properties from all allOf members.
@@ -652,6 +653,10 @@ func generateAllOfType(gen *TypeGenerator, desc *SchemaDescriptor) string {
652653
return ""
653654
}
654655

656+
// Merge extensions from all allOf members into a composite.
657+
// This mirrors V2's MergeSchemas which merges extensions before type generation.
658+
mergedExt := mergeAllOfExtensions(desc, gen.schemaIndex)
659+
655660
// Merge all fields, checking for conflicts
656661
mergedFields := make(map[string]StructField) // keyed by JSONName
657662
var fieldOrder []string // preserve order
@@ -687,6 +692,7 @@ func generateAllOfType(gen *TypeGenerator, desc *SchemaDescriptor) string {
687692
} else if proxy.IsReference() {
688693
// Reference to another schema - get its fields
689694
ref := proxy.GetReference()
695+
info.refType = ref
690696
if target, ok := gen.schemaIndex[ref]; ok {
691697
info.fields = gen.collectFieldsRecursive(target)
692698
}
@@ -703,6 +709,15 @@ func generateAllOfType(gen *TypeGenerator, desc *SchemaDescriptor) string {
703709
members = append(members, info)
704710
}
705711

712+
// After field collection: if no fields were found, check if the composite
713+
// should be a type alias (TypeOverride from a $ref target, no struct properties).
714+
if len(mergedFields) == 0 && !hasAnyFields(members) {
715+
if mergedExt != nil && mergedExt.TypeOverride != nil {
716+
desc.Extensions = mergedExt
717+
return generateTypeOverrideAlias(gen, desc)
718+
}
719+
}
720+
706721
// Merge fields from allOf members
707722
for _, member := range members {
708723
if member.unionType != "" {
@@ -778,6 +793,67 @@ func generateAllOfType(gen *TypeGenerator, desc *SchemaDescriptor) string {
778793
return code
779794
}
780795

796+
// mergeAllOfExtensions merges extensions from all allOf member descriptors into a
797+
// single composite. For $ref members, the target schema's extensions are merged first,
798+
// then the allOf member descriptor's own extensions (so inline overrides win).
799+
func mergeAllOfExtensions(desc *SchemaDescriptor, schemaIndex map[string]*SchemaDescriptor) *Extensions {
800+
if desc == nil || len(desc.AllOf) == 0 {
801+
return nil
802+
}
803+
804+
merged := &Extensions{}
805+
hasAny := false
806+
807+
for i, memberDesc := range desc.AllOf {
808+
if memberDesc == nil {
809+
continue
810+
}
811+
812+
// For $ref members, merge the target schema's extensions
813+
if memberDesc.IsReference() {
814+
if target, ok := schemaIndex[memberDesc.Ref]; ok && target.Extensions != nil {
815+
mergeExtensions(merged, target.Extensions)
816+
hasAny = true
817+
}
818+
}
819+
820+
// Merge the member descriptor's own extensions (inline extensions override $ref target)
821+
if memberDesc.Extensions != nil {
822+
mergeExtensions(merged, memberDesc.Extensions)
823+
hasAny = true
824+
}
825+
826+
// Also check if the allOf proxy in the schema has extensions that were parsed
827+
// on the descriptor at this index
828+
if desc.Schema != nil && i < len(desc.Schema.AllOf) {
829+
proxy := desc.Schema.AllOf[i]
830+
memberSchema := proxy.Schema()
831+
if memberSchema != nil && memberSchema.Extensions != nil {
832+
ext, err := ParseExtensions(memberSchema.Extensions, desc.Path.Append("allOf", fmt.Sprintf("%d", i)).String())
833+
if err == nil && ext != nil {
834+
mergeExtensions(merged, ext)
835+
hasAny = true
836+
}
837+
}
838+
}
839+
}
840+
841+
if !hasAny {
842+
return nil
843+
}
844+
return merged
845+
}
846+
847+
// hasAnyFields returns true if any allOf member contributed struct fields or union types.
848+
func hasAnyFields(members []allOfMemberInfo) bool {
849+
for _, m := range members {
850+
if len(m.fields) > 0 || m.unionType != "" {
851+
return true
852+
}
853+
}
854+
return false
855+
}
856+
781857
// generateAllOfStructWithUnions generates an allOf struct that contains union fields.
782858
func generateAllOfStructWithUnions(name string, fields []StructField, unionFields []StructField, doc string, tagGen *StructTagGenerator) string {
783859
b := NewCodeBuilder()

experimental/codegen/internal/extension.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,44 @@ func buildLegacyTypeOverride(typeName string, importVal any) *TypeOverride {
277277
return override
278278
}
279279

280+
// mergeExtensions copies non-zero/non-nil fields from src into dst.
281+
// Later values win (src overrides dst where set).
282+
func mergeExtensions(dst, src *Extensions) {
283+
if src == nil || dst == nil {
284+
return
285+
}
286+
if src.TypeOverride != nil {
287+
dst.TypeOverride = src.TypeOverride
288+
}
289+
if src.NameOverride != "" {
290+
dst.NameOverride = src.NameOverride
291+
}
292+
if src.TypeNameOverride != "" {
293+
dst.TypeNameOverride = src.TypeNameOverride
294+
}
295+
if src.SkipOptionalPointer != nil {
296+
dst.SkipOptionalPointer = src.SkipOptionalPointer
297+
}
298+
if src.JSONIgnore != nil {
299+
dst.JSONIgnore = src.JSONIgnore
300+
}
301+
if src.OmitEmpty != nil {
302+
dst.OmitEmpty = src.OmitEmpty
303+
}
304+
if src.OmitZero != nil {
305+
dst.OmitZero = src.OmitZero
306+
}
307+
if len(src.EnumVarNames) > 0 {
308+
dst.EnumVarNames = src.EnumVarNames
309+
}
310+
if src.DeprecatedReason != "" {
311+
dst.DeprecatedReason = src.DeprecatedReason
312+
}
313+
if src.Order != nil {
314+
dst.Order = src.Order
315+
}
316+
}
317+
280318
// Type conversion helpers that include the extension name in error messages
281319

282320
func asString(val any, extName string) (string, error) {

experimental/codegen/internal/test/previous_version/issues/issue_1957/output/types.gen.go

Lines changed: 2 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

experimental/codegen/internal/test/previous_version/issues/issue_1957/output/types_test.go

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -52,22 +52,25 @@ func TestTypeWithOptionalFieldJSONRoundTrip(t *testing.T) {
5252
}
5353
}
5454

55-
// TestTypeWithAllOfInstantiation verifies TypeWithAllOf with its nested ID field.
55+
// TestTypeWithAllOfInstantiation verifies TypeWithAllOf with its ID field.
56+
// After the allOf extension merge fix, ID should be googleuuid.UUID (value type, non-pointer)
57+
// because x-go-type-skip-optional-pointer is merged from the allOf member.
5658
func TestTypeWithAllOfInstantiation(t *testing.T) {
57-
idField := &TypeWithAllOfID{}
59+
id := uuid.MustParse("550e8400-e29b-41d4-a716-446655440000")
5860
tw := TypeWithAllOf{
59-
ID: idField,
61+
ID: id,
6062
}
6163

62-
if tw.ID == nil {
63-
t.Fatal("ID should not be nil")
64+
if tw.ID != id {
65+
t.Errorf("ID = %v, want %v", tw.ID, id)
6466
}
6567
}
6668

6769
// TestTypeWithAllOfJSONRoundTrip verifies JSON round-trip for TypeWithAllOf.
6870
func TestTypeWithAllOfJSONRoundTrip(t *testing.T) {
71+
id := uuid.MustParse("550e8400-e29b-41d4-a716-446655440000")
6972
original := TypeWithAllOf{
70-
ID: &TypeWithAllOfID{},
73+
ID: id,
7174
}
7275

7376
data, err := json.Marshal(original)
@@ -80,8 +83,8 @@ func TestTypeWithAllOfJSONRoundTrip(t *testing.T) {
8083
t.Fatalf("Unmarshal failed: %v", err)
8184
}
8285

83-
if decoded.ID == nil {
84-
t.Fatal("ID should not be nil after round trip")
86+
if decoded.ID != id {
87+
t.Errorf("ID = %v, want %v", decoded.ID, id)
8588
}
8689
}
8790

@@ -113,18 +116,19 @@ func TestTypeAliases(t *testing.T) {
113116
t.Errorf("TypeWithOptionalFieldAtRequired alias = %v, want %v", atReq, id)
114117
}
115118

116-
// TypeWithAllOfIDAllOf0 is an alias for googleuuid.UUID
117-
var allOf0 TypeWithAllOfIDAllOf0 = id
118-
if allOf0 != id {
119-
t.Errorf("TypeWithAllOfIDAllOf0 alias = %v, want %v", allOf0, id)
119+
// TypeWithAllOfID is now an alias for googleuuid.UUID (after allOf extension merge fix)
120+
var allOfID TypeWithAllOfID = id
121+
if allOfID != id {
122+
t.Errorf("TypeWithAllOfID alias = %v, want %v", allOfID, id)
120123
}
121124
}
122125

123126
// TestApplyDefaults verifies ApplyDefaults does not panic on all types.
124127
func TestApplyDefaults(t *testing.T) {
125128
(&TypeWithOptionalField{}).ApplyDefaults()
126129
(&TypeWithAllOf{}).ApplyDefaults()
127-
(&TypeWithAllOfID{}).ApplyDefaults()
130+
// TypeWithAllOfID is now a type alias (= googleuuid.UUID), not a struct,
131+
// so it does not have ApplyDefaults.
128132
}
129133

130134
// TestGetOpenAPISpecJSON verifies the embedded spec can be decoded.

experimental/codegen/internal/typegen.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,32 @@ func (g *TypeGenerator) GenerateStructFields(desc *SchemaDescriptor) []StructFie
570570
}
571571
}
572572

573+
// Merge extensions from allOf member descriptors for this property.
574+
// Mirrors V2's MergeSchemas: allOf member extensions apply to the field.
575+
if propDesc, ok := desc.Properties[propName]; ok && len(propDesc.AllOf) > 0 {
576+
for _, member := range propDesc.AllOf {
577+
if member == nil {
578+
continue
579+
}
580+
// For $ref allOf members, merge the target's extensions
581+
if member.IsReference() {
582+
if target, ok := g.schemaIndex[member.Ref]; ok && target.Extensions != nil {
583+
if propExtensions == nil {
584+
propExtensions = &Extensions{}
585+
}
586+
mergeExtensions(propExtensions, target.Extensions)
587+
}
588+
}
589+
// Merge the member descriptor's own extensions
590+
if member.Extensions != nil {
591+
if propExtensions == nil {
592+
propExtensions = &Extensions{}
593+
}
594+
mergeExtensions(propExtensions, member.Extensions)
595+
}
596+
}
597+
}
598+
573599
// Apply extensions to the field
574600
if propExtensions != nil {
575601
// Name override

0 commit comments

Comments
 (0)