From cbd83a9279ca8ff98ff30b7fbafe14aac9bb3600 Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Wed, 19 Aug 2026 18:03:22 -0500 Subject: [PATCH 1/3] feat(generator): unions expose a base even when the spec inlines the shared properties Base() keyed on allOf, so it never fired for producers that flatten composition: the branches reach the generator with identical inlined properties and no allOf anywhere, and a spec written that way got no accessor at all. The analyzer now falls back to intersecting the variants' declared fields and synthesizes Base from what they all agree on, with the accessor copying those fields out of the variant Value holds. Variants that do compose a shared schema still name it directly, which keeps returning the type the caller already knows. Fields match on everything that shapes the generated Go: name, wire name, type, required-ness, pointer-ness, omitempty, read/write-only, deprecation. The discriminator is left out, since it is how the variants differ and a spec that spells the base out keeps it out of the shared schema too. Synthesis needs two distinct struct variants, so a nullable anyOf does not duplicate its only variant, and the name goes through the naming scope so a spec's own PetBase keeps it. Variant structs are untouched: embedding the synthesized base into them would break every keyed composite literal in already-released clients. complex-schemas.yaml now yields 18 types rather than 17. Circle and Rectangle both declare shapeType and the body union dispatches on kind, so shapeType is a property the variants share and gets a base. --- README.md | 16 +- internal/analyzer/analyzer.go | 6 +- internal/analyzer/analyzer_test.go | 3 + internal/analyzer/unionbases.go | 121 +++++++++- internal/analyzer/unionbases_test.go | 263 +++++++++++++++++++++- internal/generator/e2e_union_base_test.go | 108 +++++++++ internal/generator/funcmap.go | 15 ++ internal/ir/types.go | 3 +- internal/templates/types.go.tmpl | 12 +- 9 files changed, 527 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 56d0770..fe2d011 100644 --- a/README.md +++ b/README.md @@ -152,9 +152,9 @@ for _, shape := range shapes { } ``` -When every variant composes the same schema through `allOf`, the wrapper also -exposes it, so the fields all variants share are readable without a type switch -that has to be revisited whenever a variant is added: +When every variant carries the same properties, the wrapper also exposes them, so +the fields all variants share are readable without a type switch that has to be +revisited whenever a variant is added: ```go for _, pet := range pets { @@ -164,8 +164,14 @@ for _, pet := range pets { } ``` -`Base()` returns nil for an unknown variant, and is generated only when *every* -variant composes the *same* single base. +`Base()` returns nil for an unknown variant. Variants that compose a shared schema +through `allOf` name it directly, and there has to be exactly one such schema. +Variants that inline the same properties instead, which is all some producers +emit, get a `Base` struct synthesized from the properties every variant +declares identically: same name, same type, same required-ness. The discriminator +is left out, since it is how the variants differ and a spec that spells the base +out keeps it out of the shared schema too. The result is a copy, so writing to it +does not change the variant the union holds. A payload that carries no discriminator property at all is still an error — there is nothing to identify it by — as is a union *without* a discriminator when no diff --git a/internal/analyzer/analyzer.go b/internal/analyzer/analyzer.go index 605bf5a..74c4419 100644 --- a/internal/analyzer/analyzer.go +++ b/internal/analyzer/analyzer.go @@ -94,9 +94,9 @@ func (a *Analyzer) Analyze(packageName string) (*ir.Package, error) { breakAliasCycles(pkg.Types) breakStructCycles(pkg.Types) - // A union whose variants all compose the same schema can expose it directly, - // which depends on the variants' final field shapes. - linkUnionBases(pkg.Types) + // A union whose variants all carry the same properties can expose them + // directly, which depends on the variants' final field shapes. + a.linkUnionBases(pkg) // Detect paginated operations. a.detectPagination(pkg) diff --git a/internal/analyzer/analyzer_test.go b/internal/analyzer/analyzer_test.go index 1a9cea9..e9ece4e 100644 --- a/internal/analyzer/analyzer_test.go +++ b/internal/analyzer/analyzer_test.go @@ -797,6 +797,9 @@ func TestComplexSchemas_AllTypesPresent(t *testing.T) { "ShapeCollection", "ShapeCollectionShapesValue", // Unions used directly as a request or response body. "CreateShapeBody", "CreateShapeResponse", "NamedShape", + // Circle and Rectangle both declare shapeType, and the body union + // dispatches on kind, so the shared property becomes a base. + "CreateShapeBodyBase", } if len(pkg.Types) != len(expectedTypes) { diff --git a/internal/analyzer/unionbases.go b/internal/analyzer/unionbases.go index e510322..2bcbfc5 100644 --- a/internal/analyzer/unionbases.go +++ b/internal/analyzer/unionbases.go @@ -1,18 +1,42 @@ package analyzer -import "github.com/parallelworks/openapi-client-generator/internal/ir" +import ( + naming "github.com/giraffesyo/openapi-go-naming" -// linkUnionBases records on every union the one type all of its variants embed, -// when there is exactly one. Reading a field the variants share is otherwise a -// type switch over every variant, which the compiler cannot keep exhaustive as + "github.com/parallelworks/openapi-client-generator/internal/ir" +) + +// linkUnionBases gives every union the type holding the properties its variants +// all carry, when there is one. Reading such a property is otherwise a type +// switch over every variant, which the compiler cannot keep exhaustive as // variants are added. -func linkUnionBases(types []*ir.TypeDef) { - byName := ir.TypesByName(types) - for _, td := range types { +// +// A variant set that composes a shared schema through allOf names it directly. +// One that inlines the same properties instead, as generators that flatten +// composition emit, gets a base synthesized from the properties they share. +func (a *Analyzer) linkUnionBases(pkg *ir.Package) { + byName := ir.TypesByName(pkg.Types) + for _, td := range pkg.Types { if td == nil || td.Kind != ir.TypeKindUnion || len(td.UnionTypes) == 0 { continue } - td.BaseType = sharedEmbeddedType(byName, td.UnionTypes) + if base := sharedEmbeddedType(byName, td.UnionTypes); base != "" { + td.BaseType = base + td.BaseEmbedded = true + continue + } + fields := sharedFields(byName, td.UnionTypes, td.Discriminator) + if len(fields) == 0 { + continue + } + base := &ir.TypeDef{ + Name: a.namer.Unique(naming.Exported(td.Name + "Base")), + Description: "The properties every variant of " + td.Name + " declares.", + Kind: ir.TypeKindStruct, + Fields: fields, + } + pkg.Types = append(pkg.Types, base) + td.BaseType = base.Name } } @@ -37,6 +61,87 @@ func sharedEmbeddedType(byName map[string]*ir.TypeDef, variants []*ir.UnionVaria return shared[0] } +// sharedFields returns copies of the fields every variant declares identically, +// in the first variant's order. It returns nil unless the union has at least two +// distinct struct variants: one variant shares everything with itself, which +// would make a base that only restates it. +func sharedFields(byName map[string]*ir.TypeDef, variants []*ir.UnionVariant, disc *ir.DiscriminatorDef) []*ir.Field { + var structs []*ir.TypeDef + seen := make(map[string]bool, len(variants)) + for _, v := range variants { + if seen[v.TypeName] { + continue + } + seen[v.TypeName] = true + td := ir.StructNamed(byName, v.TypeName) + if td == nil { + return nil + } + structs = append(structs, td) + } + if len(structs) < 2 { + return nil + } + + var shared []*ir.Field + for _, f := range structs[0].Fields { + if shareable(f, disc) { + shared = append(shared, f) + } + } + for _, td := range structs[1:] { + declared := make(map[string]bool, len(td.Fields)) + for _, f := range td.Fields { + if shareable(f, disc) { + declared[fieldKey(f)] = true + } + } + kept := shared[:0] + for _, f := range shared { + if declared[fieldKey(f)] { + kept = append(kept, f) + } + } + shared = kept + if len(shared) == 0 { + return nil + } + } + + copies := make([]*ir.Field, len(shared)) + for i, f := range shared { + field := *f + copies[i] = &field + } + return copies +} + +// shareable reports whether a field can belong to a synthesized base: an embedded +// type is the other path's business, a catch-all holds what its own schema left +// undeclared rather than a property the variants agree on, and the discriminator +// is how the variants differ, which is also why a spec that spells the base out +// keeps it out of the shared schema. +func shareable(f *ir.Field, disc *ir.DiscriminatorDef) bool { + if f.Embedded || f.CatchAll { + return false + } + return disc == nil || f.JSONName != disc.PropertyName +} + +// fieldKey identifies a field by everything that shapes the Go it generates, so +// two variants agree on a property only when they declare it the same way. +func fieldKey(f *ir.Field) string { + key := f.Name + "\x00" + f.JSONName + "\x00" + f.Type + for _, flag := range []bool{f.Required, f.IsPointer, f.OmitEmpty, f.ReadOnly, f.WriteOnly, f.Deprecated} { + if flag { + key += "1" + } else { + key += "0" + } + } + return key +} + // embeddedTypeNames returns the struct types goType embeds by value. func embeddedTypeNames(byName map[string]*ir.TypeDef, goType string) []string { td := ir.StructNamed(byName, goType) diff --git a/internal/analyzer/unionbases_test.go b/internal/analyzer/unionbases_test.go index 01d0738..173a41d 100644 --- a/internal/analyzer/unionbases_test.go +++ b/internal/analyzer/unionbases_test.go @@ -1,6 +1,10 @@ package analyzer -import "testing" +import ( + "testing" + + "github.com/parallelworks/openapi-client-generator/internal/ir" +) const sharedBaseSpec = `openapi: 3.1.0 info: { title: t, version: "1" } @@ -135,3 +139,260 @@ func TestUnionBase_AmbiguousBaseIsLeftAlone(t *testing.T) { t.Errorf("Either.BaseType = %q, want empty", either.BaseType) } } + +const inlinedSharedSpec = `openapi: 3.1.0 +info: { title: t, version: "1" } +paths: {} +components: + schemas: + Dog: + type: object + properties: + id: { type: string } + name: { type: string } + age: { type: integer } + kind: { type: string, enum: [dog] } + goodBoy: { type: boolean } + required: [id, name, age, kind, goodBoy] + Cat: + type: object + properties: + id: { type: string } + name: { type: string } + age: { type: integer } + kind: { type: string, enum: [cat] } + livesLeft: { type: integer } + required: [id, name, age, kind, livesLeft] + Pet: + oneOf: + - $ref: "#/components/schemas/Dog" + - $ref: "#/components/schemas/Cat" + discriminator: + propertyName: kind + mapping: { dog: "#/components/schemas/Dog", cat: "#/components/schemas/Cat" } +` + +// A spec that inlines the shared properties instead of composing them through +// allOf describes the same thing, and generators that flatten composition emit +// nothing else. +func TestUnionBase_SynthesizedFromInlinedProperties(t *testing.T) { + _, typeMap := analyzeSpec(t, inlinedSharedSpec) + + pet := typeMap["Pet"] + if pet == nil { + t.Fatal("Pet type not found") + } + if pet.BaseType != "PetBase" { + t.Fatalf("Pet.BaseType = %q, want PetBase", pet.BaseType) + } + if pet.BaseEmbedded { + t.Error("Pet.BaseEmbedded = true, want false: the variants declare the properties themselves") + } + + base := typeMap["PetBase"] + if base == nil { + t.Fatal("synthesized PetBase not found in package types") + } + if base.Kind != ir.TypeKindStruct { + t.Errorf("PetBase kind = %v, want struct", base.Kind) + } + var got []string + for _, f := range base.Fields { + got = append(got, f.Name+" "+f.Type) + } + want := []string{"ID string", "Name string", "Age int64"} + if len(got) != len(want) { + t.Fatalf("PetBase fields = %v, want %v", got, want) + } + for i, w := range want { + if got[i] != w { + t.Errorf("PetBase field %d = %q, want %q", i, got[i], w) + } + } +} + +func TestUnionBase_EmbeddedBaseWinsOverSynthesis(t *testing.T) { + _, typeMap := analyzeSpec(t, sharedBaseSpec) + + pet := typeMap["Pet"] + if pet == nil { + t.Fatal("Pet type not found") + } + if !pet.BaseEmbedded { + t.Error("Pet.BaseEmbedded = false, want true: the variants compose PetBase") + } + if typeMap["PetBase"] == nil { + t.Error("the spec's own PetBase should be the base, not a synthesized copy") + } +} + +const partiallySharedSpec = `openapi: 3.1.0 +info: { title: t, version: "1" } +paths: {} +components: + schemas: + Dog: + type: object + properties: + id: { type: string } + age: { type: integer } + kind: { type: string } + required: [id, age, kind] + Cat: + type: object + properties: + id: { type: string } + age: { type: string } + name: { type: string } + kind: { type: string } + required: [id, age, kind] + Pet: + oneOf: + - $ref: "#/components/schemas/Dog" + - $ref: "#/components/schemas/Cat" + discriminator: + propertyName: kind + mapping: { dog: "#/components/schemas/Dog", cat: "#/components/schemas/Cat" } +` + +// age is an integer in one variant and a string in the other, and name is absent +// from one, so neither is a property the variants agree on. +func TestUnionBase_SynthesizesOnlyIdenticalProperties(t *testing.T) { + _, typeMap := analyzeSpec(t, partiallySharedSpec) + + pet := typeMap["Pet"] + if pet == nil { + t.Fatal("Pet type not found") + } + base := typeMap[pet.BaseType] + if base == nil { + t.Fatalf("Pet.BaseType = %q, which is not a generated type", pet.BaseType) + } + if len(base.Fields) != 1 || base.Fields[0].Name != "ID" { + t.Errorf("%s fields = %+v, want ID alone", base.Name, base.Fields) + } +} + +const discriminatorOnlySpec = `openapi: 3.1.0 +info: { title: t, version: "1" } +paths: {} +components: + schemas: + Dog: + type: object + properties: + kind: { type: string } + goodBoy: { type: boolean } + required: [kind, goodBoy] + Cat: + type: object + properties: + kind: { type: string } + livesLeft: { type: integer } + required: [kind, livesLeft] + Pet: + oneOf: + - $ref: "#/components/schemas/Dog" + - $ref: "#/components/schemas/Cat" + discriminator: + propertyName: kind + mapping: { dog: "#/components/schemas/Dog", cat: "#/components/schemas/Cat" } +` + +// The discriminator is how the variants differ, and a spec that spells the base +// out keeps it out of the shared schema, so it is not a base of its own. +func TestUnionBase_DiscriminatorAloneIsNoBase(t *testing.T) { + _, typeMap := analyzeSpec(t, discriminatorOnlySpec) + + pet := typeMap["Pet"] + if pet == nil { + t.Fatal("Pet type not found") + } + if pet.BaseType != "" { + t.Errorf("Pet.BaseType = %q, want empty", pet.BaseType) + } +} + +const takenBaseNameSpec = `openapi: 3.1.0 +info: { title: t, version: "1" } +paths: {} +components: + schemas: + PetBase: + type: object + properties: + unrelated: { type: string } + Dog: + type: object + properties: + id: { type: string } + kind: { type: string } + required: [id, kind] + Cat: + type: object + properties: + id: { type: string } + kind: { type: string } + required: [id, kind] + Pet: + oneOf: + - $ref: "#/components/schemas/Dog" + - $ref: "#/components/schemas/Cat" + discriminator: + propertyName: kind + mapping: { dog: "#/components/schemas/Dog", cat: "#/components/schemas/Cat" } +` + +func TestUnionBase_SynthesizedNameAvoidsTheSpecsOwn(t *testing.T) { + _, typeMap := analyzeSpec(t, takenBaseNameSpec) + + pet := typeMap["Pet"] + if pet == nil { + t.Fatal("Pet type not found") + } + if pet.BaseType == "" || pet.BaseType == "PetBase" { + t.Fatalf("Pet.BaseType = %q, want a name the spec's own PetBase does not hold", pet.BaseType) + } + if base := typeMap[pet.BaseType]; base == nil || len(base.Fields) != 1 { + t.Errorf("%s = %+v, want the synthesized base holding id", pet.BaseType, base) + } + if spec := typeMap["PetBase"]; spec == nil || len(spec.Fields) != 1 || spec.Fields[0].Name != "Unrelated" { + t.Errorf("the spec's PetBase was overwritten: %+v", spec) + } +} + +const nullableStructSpec = `openapi: 3.1.0 +info: { title: t, version: "1" } +paths: {} +components: + schemas: + Person: + type: object + properties: + id: { type: string } + name: { type: string } + MaybePerson: + anyOf: + - $ref: "#/components/schemas/Person" + - type: "null" + PersonOrName: + anyOf: + - $ref: "#/components/schemas/Person" + - type: string +` + +// One variant shares every property with itself, and a variant with no fields at +// all shares none, so neither shape has a base to expose. +func TestUnionBase_NeedsTwoStructVariants(t *testing.T) { + _, typeMap := analyzeSpec(t, nullableStructSpec) + + for _, name := range []string{"MaybePerson", "PersonOrName"} { + td := typeMap[name] + if td == nil { + continue + } + if td.BaseType != "" { + t.Errorf("%s.BaseType = %q, want empty", name, td.BaseType) + } + } +} diff --git a/internal/generator/e2e_union_base_test.go b/internal/generator/e2e_union_base_test.go index 126e1ef..1b0f4ea 100644 --- a/internal/generator/e2e_union_base_test.go +++ b/internal/generator/e2e_union_base_test.go @@ -141,3 +141,111 @@ func TestBaseDoesNotAliasTheVariant(t *testing.T) { } `) } + +const inlinedUnionBaseSpec = `openapi: 3.1.0 +info: { title: pets, version: "1" } +paths: {} +components: + schemas: + Dog: + type: object + properties: + id: { type: string } + name: { type: string } + age: { type: integer } + kind: { type: string, enum: [dog] } + goodBoy: { type: boolean } + required: [id, name, age, kind, goodBoy] + Cat: + type: object + properties: + id: { type: string } + name: { type: string } + age: { type: integer } + kind: { type: string, enum: [cat] } + livesLeft: { type: integer } + required: [id, name, age, kind, livesLeft] + Pet: + oneOf: + - $ref: "#/components/schemas/Dog" + - $ref: "#/components/schemas/Cat" + discriminator: + propertyName: kind + mapping: { dog: "#/components/schemas/Dog", cat: "#/components/schemas/Cat" } +` + +// TestE2E_UnionBaseFromInlinedProperties covers the same union written without +// allOf, which is all a producer that flattens composition can emit: the shared +// properties are still reachable off the wrapper. +func TestE2E_UnionBaseFromInlinedProperties(t *testing.T) { + files, _ := generateFromSpec(t, inlinedUnionBaseSpec, "petsapi") + + runGeneratedWireTest(t, files, "inlinedunionbase", `package petsapi + +import ( + "encoding/json" + "testing" +) + +func TestBaseCopiesTheSharedProperties(t *testing.T) { + var pets []Pet + payload := `+"`"+`[ + {"kind":"dog","id":"1","name":"Rex","age":4,"goodBoy":true}, + {"kind":"cat","id":"2","name":"Momo","age":7,"livesLeft":9} + ]`+"`"+` + if err := json.Unmarshal([]byte(payload), &pets); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + want := []PetBase{{ID: "1", Name: "Rex", Age: 4}, {ID: "2", Name: "Momo", Age: 7}} + for i, p := range pets { + base := p.Base() + if base == nil { + t.Fatalf("pets[%d].Base() = nil, want the shared properties", i) + } + if *base != want[i] { + t.Errorf("pets[%d].Base() = %+v, want %+v", i, *base, want[i]) + } + } + if _, ok := pets[1].Value.(Cat); !ok { + t.Errorf("pets[1].Value = %T, want Cat", pets[1].Value) + } +} + +func TestBaseIsNilForAnUnknownVariant(t *testing.T) { + var p Pet + if err := json.Unmarshal([]byte(`+"`"+`{"kind":"parrot","id":"3","name":"Polly"}`+"`"+`), &p); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if p.Base() != nil { + t.Errorf("Base() = %+v, want nil for an unknown variant", p.Base()) + } +} + +func TestVariantsKeepTheirOwnFields(t *testing.T) { + var p Pet + if err := json.Unmarshal([]byte(`+"`"+`{"kind":"dog","id":"1","name":"Rex","age":4,"goodBoy":true}`+"`"+`), &p); err != nil { + t.Fatalf("unmarshal: %v", err) + } + dog, ok := p.Value.(Dog) + if !ok { + t.Fatalf("Value = %T, want Dog", p.Value) + } + if dog.Name != "Rex" || !dog.GoodBoy { + t.Errorf("Dog = %+v, want its own fields intact", dog) + } + + out, err := json.Marshal(p) + if err != nil { + t.Fatalf("marshal: %v", err) + } + var back Pet + if err := json.Unmarshal(out, &back); err != nil { + t.Fatalf("round trip: %v", err) + } + if *back.Base() != *p.Base() { + t.Errorf("round trip changed the base: %+v vs %+v", *back.Base(), *p.Base()) + } +} +`) +} diff --git a/internal/generator/funcmap.go b/internal/generator/funcmap.go index 949f246..3e9667c 100644 --- a/internal/generator/funcmap.go +++ b/internal/generator/funcmap.go @@ -32,6 +32,7 @@ func FuncMap() template.FuncMap { "hasUnions": hasUnions, "hasUntypedVariant": hasUntypedVariant, "distinctVariants": distinctVariants, + "unionBaseFields": unionBaseFields, "catchAllField": catchAllField, "catchAllValueType": catchAllValueType, "hasCatchAllTypes": hasCatchAllTypes, @@ -572,6 +573,20 @@ func distinctVariants(td *ir.TypeDef) []string { return names } +// unionBaseFields returns the fields a union's Base accessor copies out of the +// variant it holds. It is empty when the variants embed the base, which the +// accessor takes the address of instead. +func unionBaseFields(pkg *ir.Package, td *ir.TypeDef) []*ir.Field { + if td.BaseType == "" || td.BaseEmbedded { + return nil + } + base := typeIndex(pkg).byName[td.BaseType] + if base == nil { + return nil + } + return base.Fields +} + // discriminatorFieldName converts a JSON property name to a Go field name // for use in the discriminator struct in UnmarshalJSON. func discriminatorFieldName(propertyName string) string { diff --git a/internal/ir/types.go b/internal/ir/types.go index 1562e0c..5c05100 100644 --- a/internal/ir/types.go +++ b/internal/ir/types.go @@ -79,7 +79,8 @@ type TypeDef struct { EnumValues []*EnumVal // For enums EnumGoType string // For enums: the underlying Go type (e.g., "string", "int") UnionTypes []*UnionVariant // For oneOf/anyOf unions - BaseType string // For unions: the type every variant embeds + BaseType string // For unions: the type holding the properties every variant shares + BaseEmbedded bool // For unions: whether the variants embed BaseType rather than declaring its fields Discriminator *DiscriminatorDef // If polymorphic via discriminator IsNullable bool } diff --git a/internal/templates/types.go.tmpl b/internal/templates/types.go.tmpl index 890fb08..2558392 100644 --- a/internal/templates/types.go.tmpl +++ b/internal/templates/types.go.tmpl @@ -290,15 +290,23 @@ func (u {{ .Name }}) Raw() json.RawMessage { return json.RawMessage(u.raw) } {{ end }} -{{- if .BaseType }}{{ $base := .BaseType }} -// Base returns the {{ .BaseType }} that every variant of {{ .Name }} composes, or +{{- if .BaseType }}{{ $base := .BaseType }}{{ $baseFields := unionBaseFields $ . }} +// Base returns the {{ .BaseType }} that every variant of {{ .Name }} carries, or // nil when Value holds none of them. It is a copy: writing to it does not change // the value the union holds. func (u {{ .Name }}) Base() *{{ .BaseType }} { switch v := u.Value.(type) { {{- range distinctVariants . }} case {{ . }}: +{{- if $baseFields }} + return &{{ $base }}{ +{{- range $baseFields }} + {{ .Name }}: v.{{ .Name }}, +{{- end }} + } +{{- else }} return &v.{{ $base }} +{{- end }} {{- end }} } return nil From 50f4c16a582b2dd630a1f44922bc201e1d549fd4 Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Wed, 19 Aug 2026 18:24:25 -0500 Subject: [PATCH 2/3] refactor(analyzer): build the shared-field set into a fresh slice The in-place filter was correct, the write index never leading the read index, but a reader has to prove that before moving on. The intersection runs once per union at analysis time. --- internal/analyzer/unionbases.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/analyzer/unionbases.go b/internal/analyzer/unionbases.go index 2bcbfc5..1467b74 100644 --- a/internal/analyzer/unionbases.go +++ b/internal/analyzer/unionbases.go @@ -96,7 +96,7 @@ func sharedFields(byName map[string]*ir.TypeDef, variants []*ir.UnionVariant, di declared[fieldKey(f)] = true } } - kept := shared[:0] + var kept []*ir.Field for _, f := range shared { if declared[fieldKey(f)] { kept = append(kept, f) From 114dfcc5e6fc54ee4c9b7ec27608879626486033 Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Wed, 19 Aug 2026 18:38:28 -0500 Subject: [PATCH 3/3] docs(generator): synthesized union base says it is derived, not declared A consumer reading ClusterCreateBase in a generated package cannot tell it from a schema the spec defines, which matters because its field set is a function of what the variants happen to share: an unrelated edit to one variant moves it. Also guards the field copy the synthesis does. ir.Field holds only strings and bools today, so copying with *f is complete, and a reference member added later would alias into the base silently rather than failing to compile. --- README.md | 4 +++- internal/analyzer/unionbases.go | 10 ++++++---- internal/analyzer/unionbases_test.go | 17 +++++++++++++++++ 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index fe2d011..60755a3 100644 --- a/README.md +++ b/README.md @@ -168,7 +168,9 @@ for _, pet := range pets { through `allOf` name it directly, and there has to be exactly one such schema. Variants that inline the same properties instead, which is all some producers emit, get a `Base` struct synthesized from the properties every variant -declares identically: same name, same type, same required-ness. The discriminator +declares identically: same name, same type, same required-ness. That type is +derived from the variants rather than declared by the spec, so it changes when +they do. The discriminator is left out, since it is how the variants differ and a spec that spells the base out keeps it out of the shared schema too. The result is a copy, so writing to it does not change the variant the union holds. diff --git a/internal/analyzer/unionbases.go b/internal/analyzer/unionbases.go index 1467b74..3b4792e 100644 --- a/internal/analyzer/unionbases.go +++ b/internal/analyzer/unionbases.go @@ -30,10 +30,12 @@ func (a *Analyzer) linkUnionBases(pkg *ir.Package) { continue } base := &ir.TypeDef{ - Name: a.namer.Unique(naming.Exported(td.Name + "Base")), - Description: "The properties every variant of " + td.Name + " declares.", - Kind: ir.TypeKindStruct, - Fields: fields, + Name: a.namer.Unique(naming.Exported(td.Name + "Base")), + Description: "The properties every variant of " + td.Name + " declares.\n" + + "Derived from the variants rather than declared by the spec, so it\n" + + "changes when they do.", + Kind: ir.TypeKindStruct, + Fields: fields, } pkg.Types = append(pkg.Types, base) td.BaseType = base.Name diff --git a/internal/analyzer/unionbases_test.go b/internal/analyzer/unionbases_test.go index 173a41d..a57ad07 100644 --- a/internal/analyzer/unionbases_test.go +++ b/internal/analyzer/unionbases_test.go @@ -1,6 +1,7 @@ package analyzer import ( + "reflect" "testing" "github.com/parallelworks/openapi-client-generator/internal/ir" @@ -396,3 +397,19 @@ func TestUnionBase_NeedsTwoStructVariants(t *testing.T) { } } } + +// sharedFields copies each field it keeps with *f, which is a complete copy only +// while ir.Field holds nothing by reference. A member added later that the copy +// would alias has to be copied deliberately, and the aliasing is silent until +// something writes through it. +func TestSharedFieldsCopyStaysComplete(t *testing.T) { + field := reflect.TypeFor[ir.Field]() + for i := range field.NumField() { + member := field.Field(i) + switch member.Type.Kind() { + case reflect.String, reflect.Bool: + default: + t.Errorf("ir.Field.%s is a %s, which sharedFields now aliases into the synthesized base", member.Name, member.Type.Kind()) + } + } +}