From 6333288136812aabdf906f494576af047a3843f3 Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Fri, 21 Aug 2026 13:46:58 -0500 Subject: [PATCH 1/2] feat(analyzer): allOf is honored wherever it appears, not only on named schemas allOf is JSON Schema 2020-12 composition, which 3.1 uses, and it is valid anywhere a schema is. convertSchema composes it for a named component schema and resolveGoType, which types every inline property, never looked at it, so a property that composed anything resolved to any. In a 3.1 spec today: allOf: [$ref, {inline object}] -> any allOf: [$ref] -> any $ref with sibling keywords -> the referenced type anyOf: [$ref, {type: null}] -> the referenced type The first two are the inconsistency: the same composition on a named schema builds a struct. A lone $ref inside an allOf says the value must match that schema, so it resolves to it; anything more composes a shape of its own and is named like other inline schemas, keeping both what it embeds and what it adds. The two idioms 3.1 offers instead already worked, and now have a test that keeps them working. --- internal/analyzer/schemas.go | 60 ++++++++ .../analyzer/schemas_allof_property_test.go | 142 ++++++++++++++++++ internal/generator/e2e_allof_property_test.go | 98 ++++++++++++ 3 files changed, 300 insertions(+) create mode 100644 internal/analyzer/schemas_allof_property_test.go create mode 100644 internal/generator/e2e_allof_property_test.go diff --git a/internal/analyzer/schemas.go b/internal/analyzer/schemas.go index 63fea63..700e9fc 100644 --- a/internal/analyzer/schemas.go +++ b/internal/analyzer/schemas.go @@ -629,6 +629,21 @@ func (a *Analyzer) resolveGoType(schema *highbase.Schema, nameHint string) strin return name } + // An allOf composes other schemas. A lone $ref inside one is how a 3.0 spec + // hangs a description or nullable on a reference, since keywords beside a + // $ref are ignored, and it means the type it references. Anything else + // composes a shape of its own and is named like any other inline schema. + if len(schema.AllOf) > 0 { + if len(schema.AllOf) == 1 { + if goType := a.goTypeForRef(schema.AllOf[0].GetReference()); goType != "" { + return goType + } + } + if name, ok := a.synthesizeInlineAllOf(schema, nameHint); ok { + return name + } + } + // Enum type referenced inline -- use the primary type. primaryType := primaryType(schema) @@ -769,6 +784,51 @@ func (a *Analyzer) synthesizeInlineObject(schema *highbase.Schema, nameHint stri return goName, true } +// synthesizeInlineAllOf declares a named struct for a composition written inline, +// so what it composes stays typed instead of collapsing into any. +func (a *Analyzer) synthesizeInlineAllOf(schema *highbase.Schema, nameHint string) (string, bool) { + if schema.Title != "" { + nameHint = schema.Title + } + if nameHint == "" { + return "", false + } + + key := a.inlineAllOfKey(schema, nameHint) + if existing, ok := a.synthesizedByKey[key]; ok { + return existing.Name, true + } + + goName := a.namer.Unique(naming.Exported(nameHint)) + td, err := a.convertAllOf(goName, schema, false, false) + if err != nil || td == nil { + return "", false + } + a.synthesizedByKey[key] = td + a.synthesized = append(a.synthesized, td) + return goName, true +} + +// inlineAllOfKey identifies a composition by what it composes, so the same one +// written twice lands on one type. +func (a *Analyzer) inlineAllOfKey(schema *highbase.Schema, nameHint string) string { + var b strings.Builder + b.WriteString("allOf") + for i, proxy := range schema.AllOf { + if ref := proxy.GetReference(); ref != "" { + b.WriteString("|" + ref) + continue + } + entry, err := proxy.BuildSchema() + if err != nil || entry == nil { + b.WriteString("|?") + continue + } + b.WriteString("|" + a.inlineObjectKey(entry, nameHint+"Part"+strconv.Itoa(i))) + } + return b.String() +} + // inlineObjectKey identifies an inline object by the Go it would generate, so two // properties declaring the same shape land on one type rather than two names for // it. diff --git a/internal/analyzer/schemas_allof_property_test.go b/internal/analyzer/schemas_allof_property_test.go new file mode 100644 index 0000000..2bcd4d2 --- /dev/null +++ b/internal/analyzer/schemas_allof_property_test.go @@ -0,0 +1,142 @@ +package analyzer + +import "testing" + +const allOfPropertySpec = `openapi: 3.1.0 +info: { title: t, version: "1" } +paths: {} +components: + schemas: + View: + type: object + properties: + creator: + allOf: + - $ref: "#/components/schemas/User" + author: + readOnly: true + description: The author. + allOf: + - $ref: "#/components/schemas/User" + same: + allOf: + - $ref: "#/components/schemas/User" + composed: + description: A user with a role beside it. + allOf: + - $ref: "#/components/schemas/User" + - type: object + properties: + role: { type: string } + inline: + allOf: + - type: object + properties: + only: { type: string } + User: + type: object + properties: + login: { type: string } + required: [login] +` + +// allOf is JSON Schema composition, and a lone $ref inside one says the value +// must match that schema, which is to say it is of that type. +func TestAllOfProperty_LoneRefResolvesToIt(t *testing.T) { + _, typeMap := analyzeSpec(t, allOfPropertySpec) + + view := typeMap["View"] + if view == nil { + t.Fatal("View not found") + } + byJSON := map[string]string{} + for _, f := range view.Fields { + byJSON[f.JSONName] = f.Type + } + + if byJSON["creator"] != "*User" { + t.Errorf("creator = %q, want *User", byJSON["creator"]) + } + // Keywords beside the allOf are handled where they belong, so the + // composition still resolves to what it composes. + if byJSON["author"] != "*User" { + t.Errorf("author = %q, want *User", byJSON["author"]) + } + if byJSON["same"] != "*User" { + t.Errorf("same = %q, want *User", byJSON["same"]) + } +} + +// A composition of more than a reference is a shape of its own, and gets a name +// like any other inline schema. +func TestAllOfProperty_CompositionIsNamed(t *testing.T) { + _, typeMap := analyzeSpec(t, allOfPropertySpec) + + view := typeMap["View"] + byJSON := map[string]string{} + for _, f := range view.Fields { + byJSON[f.JSONName] = f.Type + } + + if byJSON["composed"] != "*ViewComposed" { + t.Fatalf("composed = %q, want *ViewComposed", byJSON["composed"]) + } + composed := typeMap["ViewComposed"] + if composed == nil { + t.Fatal("ViewComposed not synthesized") + } + if len(composed.Fields) != 2 { + t.Fatalf("ViewComposed fields = %+v, want the embed and the property", composed.Fields) + } + if !composed.Fields[0].Embedded || composed.Fields[0].Type != "User" { + t.Errorf("ViewComposed field 0 = %+v, want an embedded User", composed.Fields[0]) + } + if composed.Fields[1].JSONName != "role" { + t.Errorf("ViewComposed field 1 = %+v, want role", composed.Fields[1]) + } + + // A single inline entry composes just as much as several do. + if byJSON["inline"] != "*ViewInline" { + t.Errorf("inline = %q, want *ViewInline", byJSON["inline"]) + } +} + +const refIdiomSpec = `openapi: 3.1.0 +info: { title: t, version: "1" } +paths: {} +components: + schemas: + Advisory: + type: object + properties: + siblings: + $ref: "#/components/schemas/User" + description: 3.1 allows keywords beside a reference. + readOnly: true + nullableRef: + anyOf: + - $ref: "#/components/schemas/User" + - type: "null" + User: + type: object + properties: { login: { type: string } } + required: [login] +` + +// The ways 3.1 says the same things without allOf: keywords beside a $ref, and a +// null member for a nullable one. Both resolve to the referenced type, and this +// keeps them that way. +func TestRefIdioms_3_1(t *testing.T) { + _, typeMap := analyzeSpec(t, refIdiomSpec) + + byJSON := map[string]string{} + for _, f := range typeMap["Advisory"].Fields { + byJSON[f.JSONName] = f.Type + } + if byJSON["siblings"] != "*User" { + t.Errorf("a $ref with sibling keywords = %q, want *User", byJSON["siblings"]) + } + if byJSON["nullableRef"] != "*User" { + t.Errorf("anyOf with a null member = %q, want *User", byJSON["nullableRef"]) + } +} diff --git a/internal/generator/e2e_allof_property_test.go b/internal/generator/e2e_allof_property_test.go new file mode 100644 index 0000000..0fa6a6d --- /dev/null +++ b/internal/generator/e2e_allof_property_test.go @@ -0,0 +1,98 @@ +package generator + +import "testing" + +const allOfPropertyAPISpec = `openapi: 3.1.0 +info: { title: advisories, version: "1" } +paths: + /advisories/{id}: + get: + operationId: getAdvisory + parameters: + - { name: id, in: path, required: true, schema: { type: string } } + responses: + "200": + description: ok + content: + application/json: { schema: { $ref: "#/components/schemas/Advisory" } } +components: + schemas: + Advisory: + type: object + properties: + id: { type: string } + author: + description: The author of the advisory. + allOf: + - $ref: "#/components/schemas/User" + - type: "null" + reviewer: + description: Who reviewed it, with their verdict. + allOf: + - $ref: "#/components/schemas/User" + - type: object + properties: + verdict: { type: string } + required: [id] + User: + type: object + properties: + login: { type: string } + required: [login] +` + +// TestE2E_AllOfPropertiesAreTyped covers allOf where a property is typed. The +// analyzer composed it for a named schema and not for an inline one, so a +// property resolved to any and a caller had to assert their way into a schema +// the spec names. +func TestE2E_AllOfPropertiesAreTyped(t *testing.T) { + files, _ := generateFromSpec(t, allOfPropertyAPISpec, "advapi") + + runGeneratedWireTest(t, files, "allofproperty", `package advapi + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" +) + +func TestWrappedRefDecodesAsItsType(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`+"`"+`{"id":"a-1","author":{"login":"ada"},"reviewer":{"login":"bo","verdict":"ok"}}`+"`"+`)) + })) + defer srv.Close() + + adv, err := NewClient(srv.URL).GetAdvisory(t.Context(), "a-1") + if err != nil { + t.Fatalf("GetAdvisory: %v", err) + } + + // A plain read rather than an assertion into map[string]any. + if adv.Author == nil || adv.Author.Login != "ada" { + t.Errorf("author = %+v, want the referenced User", adv.Author) + } + + // A composition keeps both halves: what it references and what it adds. + if adv.Reviewer == nil || adv.Reviewer.Login != "bo" { + t.Errorf("reviewer = %+v, want the embedded User", adv.Reviewer) + } + if adv.Reviewer.Verdict == nil || *adv.Reviewer.Verdict != "ok" { + t.Errorf("verdict = %v, want ok", adv.Reviewer.Verdict) + } +} + +// The pointer carries absence, so a null author stays distinguishable from one +// that is present and empty. +func TestNullWrappedRefIsNil(t *testing.T) { + var adv Advisory + if err := json.Unmarshal([]byte(`+"`"+`{"id":"a-2","author":null}`+"`"+`), &adv); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if adv.Author != nil { + t.Errorf("author = %+v, want nil", adv.Author) + } +} +`) +} From 31e3c38c04d64f45dec15a3faf2373f87269ab50 Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Fri, 21 Aug 2026 14:32:13 -0500 Subject: [PATCH 2/2] fix(analyzer): a multipart body composed through allOf lost its file parts The composition path this PR adds converted its schema as if it were sent as JSON, so a multipart body written as allOf typed its binary properties as byte slices and the encoder sent them as base64 text in ordinary fields, the same failure #90 fixed for a body written as an object. Multipart is a property of where a schema is used, so the composition path consults it the same way the object path does. --- internal/analyzer/schemas.go | 7 ++- .../analyzer/schemas_allof_property_test.go | 51 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/internal/analyzer/schemas.go b/internal/analyzer/schemas.go index 700e9fc..2e42b5b 100644 --- a/internal/analyzer/schemas.go +++ b/internal/analyzer/schemas.go @@ -787,6 +787,11 @@ func (a *Analyzer) synthesizeInlineObject(schema *highbase.Schema, nameHint stri // synthesizeInlineAllOf declares a named struct for a composition written inline, // so what it composes stays typed instead of collapsing into any. func (a *Analyzer) synthesizeInlineAllOf(schema *highbase.Schema, nameHint string) (string, bool) { + // Multipart is a property of where the schema is used, and a body composed + // through allOf is used the same way one written as an object is: its binary + // properties are file parts, not base64 text. + multipartBody := a.inlineMultipartBodies[nameHint] + if schema.Title != "" { nameHint = schema.Title } @@ -800,7 +805,7 @@ func (a *Analyzer) synthesizeInlineAllOf(schema *highbase.Schema, nameHint strin } goName := a.namer.Unique(naming.Exported(nameHint)) - td, err := a.convertAllOf(goName, schema, false, false) + td, err := a.convertAllOf(goName, schema, false, multipartBody) if err != nil || td == nil { return "", false } diff --git a/internal/analyzer/schemas_allof_property_test.go b/internal/analyzer/schemas_allof_property_test.go index 2bcd4d2..4aee7b9 100644 --- a/internal/analyzer/schemas_allof_property_test.go +++ b/internal/analyzer/schemas_allof_property_test.go @@ -140,3 +140,54 @@ func TestRefIdioms_3_1(t *testing.T) { t.Errorf("anyOf with a null member = %q, want *User", byJSON["nullableRef"]) } } + +const allOfMultipartSpec = `openapi: 3.1.0 +info: { title: t, version: "1" } +paths: + /upload: + post: + operationId: upload + requestBody: + required: true + content: + multipart/form-data: + schema: + allOf: + - $ref: "#/components/schemas/Meta" + - type: object + properties: + file: { type: string, format: binary } + required: [file] + responses: + "204": { description: ok } +components: + schemas: + Meta: + type: object + properties: + label: { type: string } + required: [label] +` + +// Multipart is a property of where a schema is used, and a body composed through +// allOf is used the same way one written as an object is: its binary properties +// are file parts rather than base64 text in an ordinary field. +func TestAllOfProperty_MultipartBodyKeepsItsFileParts(t *testing.T) { + _, typeMap := analyzeSpec(t, allOfMultipartSpec) + + body := typeMap["UploadBody"] + if body == nil { + t.Fatal("UploadBody not found") + } + byJSON := map[string]string{} + for _, f := range body.Fields { + byJSON[f.JSONName] = f.Type + } + if byJSON["file"] != "FormFile" { + t.Errorf("file = %q, want FormFile", byJSON["file"]) + } + // What it composes comes along. + if len(body.Fields) != 2 || !body.Fields[0].Embedded || body.Fields[0].Type != "Meta" { + t.Errorf("fields = %+v, want the embedded Meta beside the file", body.Fields) + } +}