From 88fb679d554efa6e793c273fad24f377c7817dfc Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Wed, 19 Aug 2026 17:23:55 -0500 Subject: [PATCH] feat(generator): unions expose the base every variant composes When every arm of a oneOf/anyOf embeds the same schema through allOf, the generated wrapper only offered Value any, so reading a field present on all variants meant a type switch that goes stale as variants are added. The union now carries that base and generates an accessor for it: func (u Pet) Base() *PetBase The analyzer sets TypeDef.BaseType after cycle-breaking, when the embedded type sets of all variants intersect in exactly one struct. Unions whose variants share no base, share more than one, or reach it through a cycle-broken pointer keep their current shape. A discriminator is not required: Value holds the same variant values either way, and Base returns nil when it holds none of them. --- README.md | 15 +++ internal/analyzer/analyzer.go | 4 + internal/analyzer/unionbases.go | 73 +++++++++++ internal/analyzer/unionbases_test.go | 137 +++++++++++++++++++++ internal/generator/e2e_union_base_test.go | 143 ++++++++++++++++++++++ internal/generator/funcmap.go | 13 ++ internal/ir/types.go | 1 + internal/templates/types.go.tmpl | 14 +++ 8 files changed, 400 insertions(+) create mode 100644 internal/analyzer/unionbases.go create mode 100644 internal/analyzer/unionbases_test.go create mode 100644 internal/generator/e2e_union_base_test.go diff --git a/README.md b/README.md index 114f9b5..56d0770 100644 --- a/README.md +++ b/README.md @@ -152,6 +152,21 @@ 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: + +```go +for _, pet := range pets { + if base := pet.Base(); base != nil { + fmt.Println(base.ID, base.Name) + } +} +``` + +`Base()` returns nil for an unknown variant, and is generated only when *every* +variant composes the *same* single base. + 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 variant matches. When the schema declares a `discriminator` but no `mapping`, the diff --git a/internal/analyzer/analyzer.go b/internal/analyzer/analyzer.go index 8e5680f..605bf5a 100644 --- a/internal/analyzer/analyzer.go +++ b/internal/analyzer/analyzer.go @@ -94,6 +94,10 @@ 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) + // Detect paginated operations. a.detectPagination(pkg) diff --git a/internal/analyzer/unionbases.go b/internal/analyzer/unionbases.go new file mode 100644 index 0000000..e510322 --- /dev/null +++ b/internal/analyzer/unionbases.go @@ -0,0 +1,73 @@ +package analyzer + +import "github.com/parallelworks/openapi-client-generator/internal/ir" + +// 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 +// variants are added. +func linkUnionBases(types []*ir.TypeDef) { + byName := ir.TypesByName(types) + for _, td := range types { + if td == nil || td.Kind != ir.TypeKindUnion || len(td.UnionTypes) == 0 { + continue + } + td.BaseType = sharedEmbeddedType(byName, td.UnionTypes) + } +} + +// sharedEmbeddedType returns the single type every variant embeds by value, or "" +// when the variants share none or share more than one. +func sharedEmbeddedType(byName map[string]*ir.TypeDef, variants []*ir.UnionVariant) string { + var shared []string + for i, v := range variants { + embedded := embeddedTypeNames(byName, v.TypeName) + if i == 0 { + shared = embedded + } else { + shared = intersection(shared, embedded) + } + if len(shared) == 0 { + return "" + } + } + if len(shared) != 1 { + return "" + } + return shared[0] +} + +// embeddedTypeNames returns the struct types goType embeds by value. +func embeddedTypeNames(byName map[string]*ir.TypeDef, goType string) []string { + td := ir.StructNamed(byName, goType) + if td == nil { + return nil + } + var names []string + for _, f := range td.Fields { + // A cycle-broken embed is already a pointer, and its field name no longer + // matches the type expression an accessor would have to name. + if !f.Embedded || f.IsPointer { + continue + } + if ir.StructNamed(byName, f.Type) == nil { + continue + } + names = append(names, f.Name) + } + return names +} + +func intersection(a, b []string) []string { + inB := make(map[string]bool, len(b)) + for _, name := range b { + inB[name] = true + } + var both []string + for _, name := range a { + if inB[name] { + both = append(both, name) + } + } + return both +} diff --git a/internal/analyzer/unionbases_test.go b/internal/analyzer/unionbases_test.go new file mode 100644 index 0000000..01d0738 --- /dev/null +++ b/internal/analyzer/unionbases_test.go @@ -0,0 +1,137 @@ +package analyzer + +import "testing" + +const sharedBaseSpec = `openapi: 3.1.0 +info: { title: t, version: "1" } +paths: {} +components: + schemas: + PetBase: + type: object + properties: + id: { type: string } + name: { type: string } + required: [id, name] + Timestamps: + type: object + properties: + createdAt: { type: string, format: date-time } + Dog: + allOf: + - $ref: "#/components/schemas/PetBase" + - type: object + properties: + kind: { type: string } + goodBoy: { type: boolean } + required: [kind, goodBoy] + Cat: + allOf: + - $ref: "#/components/schemas/PetBase" + - $ref: "#/components/schemas/Timestamps" + - type: object + properties: + kind: { type: string } + livesLeft: { type: integer } + required: [kind, livesLeft] + Rock: + type: object + properties: + kind: { type: string } + required: [kind] + Pet: + oneOf: + - $ref: "#/components/schemas/Dog" + - $ref: "#/components/schemas/Cat" + discriminator: + propertyName: kind + mapping: { dog: "#/components/schemas/Dog", cat: "#/components/schemas/Cat" } + Thing: + oneOf: + - $ref: "#/components/schemas/Dog" + - $ref: "#/components/schemas/Rock" + discriminator: + propertyName: kind + mapping: { dog: "#/components/schemas/Dog", rock: "#/components/schemas/Rock" } + Untagged: + oneOf: + - $ref: "#/components/schemas/Dog" + - $ref: "#/components/schemas/Cat" +` + +func TestUnionBase_SharedByEveryVariant(t *testing.T) { + _, typeMap := analyzeSpec(t, sharedBaseSpec) + + pet := typeMap["Pet"] + if pet == nil { + t.Fatal("Pet type not found") + } + if pet.BaseType != "PetBase" { + t.Errorf("Pet.BaseType = %q, want PetBase", pet.BaseType) + } +} + +// Timestamps is embedded by Cat alone, so it is not a base of the union. +func TestUnionBase_IgnoresBaseOnlySomeVariantsCompose(t *testing.T) { + _, typeMap := analyzeSpec(t, sharedBaseSpec) + + thing := typeMap["Thing"] + if thing == nil { + t.Fatal("Thing type not found") + } + if thing.BaseType != "" { + t.Errorf("Thing.BaseType = %q, want empty: Rock composes nothing", thing.BaseType) + } +} + +func TestUnionBase_NeedsNoDiscriminator(t *testing.T) { + _, typeMap := analyzeSpec(t, sharedBaseSpec) + + untagged := typeMap["Untagged"] + if untagged == nil { + t.Fatal("Untagged type not found") + } + if untagged.BaseType != "PetBase" { + t.Errorf("Untagged.BaseType = %q, want PetBase", untagged.BaseType) + } +} + +const ambiguousBaseSpec = `openapi: 3.1.0 +info: { title: t, version: "1" } +paths: {} +components: + schemas: + Named: + type: object + properties: + name: { type: string } + Owned: + type: object + properties: + owner: { type: string } + A: + allOf: + - $ref: "#/components/schemas/Named" + - $ref: "#/components/schemas/Owned" + B: + allOf: + - $ref: "#/components/schemas/Named" + - $ref: "#/components/schemas/Owned" + Either: + oneOf: + - $ref: "#/components/schemas/A" + - $ref: "#/components/schemas/B" +` + +// Two shared bases give no single one to expose, so the union keeps its old shape. +func TestUnionBase_AmbiguousBaseIsLeftAlone(t *testing.T) { + _, typeMap := analyzeSpec(t, ambiguousBaseSpec) + + either := typeMap["Either"] + if either == nil { + t.Fatal("Either type not found") + } + if either.BaseType != "" { + t.Errorf("Either.BaseType = %q, want empty", either.BaseType) + } +} diff --git a/internal/generator/e2e_union_base_test.go b/internal/generator/e2e_union_base_test.go new file mode 100644 index 0000000..126e1ef --- /dev/null +++ b/internal/generator/e2e_union_base_test.go @@ -0,0 +1,143 @@ +package generator + +import ( + "strings" + "testing" +) + +const unionBaseSpec = `openapi: 3.1.0 +info: { title: pets, version: "1" } +paths: + /pets: + get: + operationId: listPets + responses: + "200": + description: ok + content: + application/json: + schema: + type: array + items: { $ref: "#/components/schemas/Pet" } +components: + schemas: + PetBase: + type: object + properties: + id: { type: string } + name: { type: string } + age: { type: integer } + required: [id, name, age] + Dog: + allOf: + - $ref: "#/components/schemas/PetBase" + - type: object + properties: + kind: { type: string, enum: [dog] } + goodBoy: { type: boolean } + required: [kind, goodBoy] + Cat: + allOf: + - $ref: "#/components/schemas/PetBase" + - type: object + properties: + kind: { type: string, enum: [cat] } + 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" } + Rock: + type: object + properties: + kind: { type: string, enum: [rock] } + required: [kind] + Thing: + oneOf: + - $ref: "#/components/schemas/Dog" + - $ref: "#/components/schemas/Rock" + discriminator: + propertyName: kind + mapping: { dog: "#/components/schemas/Dog", rock: "#/components/schemas/Rock" } +` + +// TestE2E_UnionBaseAccessor covers a discriminated oneOf whose variants all +// compose the same schema: the fields they share are readable off the union +// itself, without a type switch that goes stale when a variant is added. +func TestE2E_UnionBaseAccessor(t *testing.T) { + files, _ := generateFromSpec(t, unionBaseSpec, "petsapi") + + var types string + for _, f := range files { + if f.Name == "types.go" { + types = string(f.Content) + } + } + if strings.Contains(types, "func (u Thing) Base()") { + t.Error("Thing has no base shared by every variant, but got a Base accessor") + } + + runGeneratedWireTest(t, files, "unionbase", `package petsapi + +import ( + "encoding/json" + "testing" +) + +func TestBaseReadsTheSharedSchema(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 := []string{"Rex", "Momo"} + for i, p := range pets { + base := p.Base() + if base == nil { + t.Fatalf("pets[%d].Base() = nil, want the shared PetBase", i) + } + if base.Name != want[i] { + t.Errorf("pets[%d].Base().Name = %q, want %q", i, base.Name, want[i]) + } + } + if pets[0].Base().Age != 4 { + t.Errorf("pets[0].Base().Age = %d, want 4", pets[0].Base().Age) + } + 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.IsUnknownVariant() { + t.Fatal("expected an unknown variant") + } + if p.Base() != nil { + t.Errorf("Base() = %+v, want nil for an unknown variant", p.Base()) + } +} + +func TestBaseDoesNotAliasTheVariant(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) + } + p.Base().Name = "Fido" + if p.Value.(Dog).Name != "Rex" { + t.Error("writing through Base() changed the value the union holds") + } +} +`) +} diff --git a/internal/generator/funcmap.go b/internal/generator/funcmap.go index c7e910d..949f246 100644 --- a/internal/generator/funcmap.go +++ b/internal/generator/funcmap.go @@ -31,6 +31,7 @@ func FuncMap() template.FuncMap { "paramType": paramType, "hasUnions": hasUnions, "hasUntypedVariant": hasUntypedVariant, + "distinctVariants": distinctVariants, "catchAllField": catchAllField, "catchAllValueType": catchAllValueType, "hasCatchAllTypes": hasCatchAllTypes, @@ -559,6 +560,18 @@ func hasUntypedVariant(td *ir.TypeDef) bool { return slices.ContainsFunc(td.UnionTypes, func(v *ir.UnionVariant) bool { return v.TypeName == "any" }) } +// distinctVariants returns each variant Go type of a union once, so a type switch +// over them cannot repeat a case. +func distinctVariants(td *ir.TypeDef) []string { + var names []string + for _, v := range td.UnionTypes { + if !slices.Contains(names, v.TypeName) { + names = append(names, v.TypeName) + } + } + return names +} + // 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 564b6c0..1562e0c 100644 --- a/internal/ir/types.go +++ b/internal/ir/types.go @@ -79,6 +79,7 @@ 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 Discriminator *DiscriminatorDef // If polymorphic via discriminator IsNullable bool } diff --git a/internal/templates/types.go.tmpl b/internal/templates/types.go.tmpl index 6683bbf..890fb08 100644 --- a/internal/templates/types.go.tmpl +++ b/internal/templates/types.go.tmpl @@ -290,6 +290,20 @@ 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 +// 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 {{ . }}: + return &v.{{ $base }} +{{- end }} + } + return nil +} +{{ end }} // MarshalJSON implements json.Marshaler for {{ .Name }}. func (u {{ .Name }}) MarshalJSON() ([]byte, error) { {{- if .Discriminator }}