diff --git a/internal/test/schemas/recursive/doc.go b/internal/test/schemas/recursive/doc.go index bbf4b1b58..2d0254cbf 100644 --- a/internal/test/schemas/recursive/doc.go +++ b/internal/test/schemas/recursive/doc.go @@ -1,8 +1,11 @@ // Package schemasrecursive exercises recursive and cyclic schema patterns: // self-referencing types via additionalProperties (issue #52), cyclic oneOf -// references (issue #936, requires circular-reference-limit), and recursive -// $ref inside allOf (issue #1373). All cases are models-only; the point is -// that code generation and compilation succeed without infinite loops. +// references (issue #936, requires circular-reference-limit), recursive $ref +// inside allOf (issue #1373), and allOf compositions whose members refer back +// into a body still being generated (issue #2542, in its object, union, +// transitive, mutual and bystander forms). All cases are models-only; the +// point is that code generation and compilation succeed without infinite +// loops, and that a recursive composition keeps both halves of the allOf. package schemasrecursive //go:generate go run github.com/oapi-codegen/oapi-codegen/v2/cmd/oapi-codegen --config=config.yaml spec.yaml diff --git a/internal/test/schemas/recursive/recursive.gen.go b/internal/test/schemas/recursive/recursive.gen.go index 27e5e248b..d843faae6 100644 --- a/internal/test/schemas/recursive/recursive.gen.go +++ b/internal/test/schemas/recursive/recursive.gen.go @@ -5,6 +5,7 @@ package schemasrecursive import ( "encoding/json" + "fmt" "github.com/oapi-codegen/runtime" ) @@ -82,6 +83,133 @@ type FilterValue1 = string // FilterValue2 defines model for FilterValue.2. type FilterValue2 = bool +// MutualA defines model for MutualA. +type MutualA struct { + B *struct { + C *MutualA_B_C `json:"c,omitempty"` + Tag *string `json:"tag,omitempty"` + } `json:"b,omitempty"` +} + +// MutualA_B_C defines model for MutualA.b.c. +type MutualA_B_C struct { + Back *struct { + C *MutualA_B_C `json:"c,omitempty"` + Y *string `json:"y,omitempty"` + } `json:"back,omitempty"` + X *string `json:"x,omitempty"` +} + +// MutualB defines model for MutualB. +type MutualB struct { + C *MutualB_C `json:"c,omitempty"` +} + +// MutualB_C defines model for MutualB.c. +type MutualB_C struct { + Back *struct { + C *MutualB_C `json:"c,omitempty"` + Y *string `json:"y,omitempty"` + } `json:"back,omitempty"` + X *string `json:"x,omitempty"` +} + +// MutualC defines model for MutualC. +type MutualC struct { + Back *MutualC_Back `json:"back,omitempty"` +} + +// MutualC_Back defines model for MutualC.back. +type MutualC_Back struct { + C *struct { + Back *MutualC_Back `json:"back,omitempty"` + X *string `json:"x,omitempty"` + } `json:"c,omitempty"` + Y *string `json:"y,omitempty"` +} + +// Node defines model for Node. +type Node struct { + union json.RawMessage +} + +// Node0 defines model for Node.0. +type Node0 struct { + Leaf *string `json:"leaf,omitempty"` +} + +// Node1 defines model for Node.1. +type Node1 struct { + Children *[]Node_1_Children_Item `json:"children,omitempty"` +} + +// Node1Children0 defines model for Node.1.Children.0. +type Node1Children0 struct { + Leaf *string `json:"leaf,omitempty"` +} + +// Node1Children1 defines model for Node.1.Children.1. +type Node1Children1 struct { + Children *[]Node_1_Children_Item `json:"children,omitempty"` +} + +// Node_1_Children_Item defines model for Node.1.children.Item. +type Node_1_Children_Item struct { + Extra *string `json:"extra,omitempty"` + union json.RawMessage +} + +// NodeMap defines model for NodeMap. +type NodeMap map[string]NodeMap_AdditionalProperties + +// NodeMap_AdditionalProperties defines model for NodeMap.AdditionalProperties. +type NodeMap_AdditionalProperties struct { + Extra *string `json:"extra,omitempty"` + AdditionalProperties map[string]NodeMap_AdditionalProperties `json:"-"` +} + +// NodeNestedAllOf defines model for NodeNestedAllOf. +type NodeNestedAllOf struct { + union json.RawMessage +} + +// NodeNestedAllOf0 defines model for NodeNestedAllOf.0. +type NodeNestedAllOf0 struct { + Leaf *string `json:"leaf,omitempty"` +} + +// NodeNestedAllOf1 defines model for NodeNestedAllOf.1. +type NodeNestedAllOf1 struct { + Children *[]NodeNestedAllOf_1_Children_Item `json:"children,omitempty"` +} + +// NodeNestedAllOf1Children0 defines model for NodeNestedAllOf.1.Children.0. +type NodeNestedAllOf1Children0 struct { + Leaf *string `json:"leaf,omitempty"` +} + +// NodeNestedAllOf1Children1 defines model for NodeNestedAllOf.1.Children.1. +type NodeNestedAllOf1Children1 struct { + Children *[]NodeNestedAllOf_1_Children_Item `json:"children,omitempty"` +} + +// NodeNestedAllOf_1_Children_Item defines model for NodeNestedAllOf.1.children.Item. +type NodeNestedAllOf_1_Children_Item struct { + Extra *string `json:"extra,omitempty"` + union json.RawMessage +} + +// NodeObject defines model for NodeObject. +type NodeObject struct { + Children *[]NodeObject_Children_Item `json:"children,omitempty"` +} + +// NodeObject_Children_Item defines model for NodeObject.children.Item. +type NodeObject_Children_Item struct { + Children *[]NodeObject_Children_Item `json:"children,omitempty"` + Extra *string `json:"extra,omitempty"` +} + // NonRecursiveObject defines model for NonRecursiveObject. type NonRecursiveObject struct { FieldInNonRecursive *string `json:"FieldInNonRecursive,omitempty"` @@ -99,6 +227,88 @@ type Value struct { StringValue *string `json:"stringValue,omitempty"` } +// Wrapper defines model for Wrapper. +type Wrapper struct { + N *struct { + Children *[]Wrapper_N_Children_Item `json:"children,omitempty"` + Extra *string `json:"extra,omitempty"` + } `json:"n,omitempty"` +} + +// Wrapper_N_Children_Item defines model for Wrapper.n.children.Item. +type Wrapper_N_Children_Item struct { + Children *[]Wrapper_N_Children_Item `json:"children,omitempty"` + Extra *string `json:"extra,omitempty"` +} + +// Getter for additional properties for NodeMap_AdditionalProperties. Returns the specified +// element and whether it was found +func (a NodeMap_AdditionalProperties) Get(fieldName string) (value NodeMap_AdditionalProperties, found bool) { + if a.AdditionalProperties != nil { + value, found = a.AdditionalProperties[fieldName] + } + return +} + +// Setter for additional properties for NodeMap_AdditionalProperties +func (a *NodeMap_AdditionalProperties) Set(fieldName string, value NodeMap_AdditionalProperties) { + if a.AdditionalProperties == nil { + a.AdditionalProperties = make(map[string]NodeMap_AdditionalProperties) + } + a.AdditionalProperties[fieldName] = value +} + +// Override default JSON handling for NodeMap_AdditionalProperties to handle AdditionalProperties +func (a *NodeMap_AdditionalProperties) UnmarshalJSON(b []byte) error { + object := make(map[string]json.RawMessage) + err := json.Unmarshal(b, &object) + if err != nil { + return err + } + + if raw, found := object["extra"]; found { + err = json.Unmarshal(raw, &a.Extra) + if err != nil { + return fmt.Errorf("error reading 'extra': %w", err) + } + delete(object, "extra") + } + + if len(object) != 0 { + a.AdditionalProperties = make(map[string]NodeMap_AdditionalProperties) + for fieldName, fieldBuf := range object { + var fieldVal NodeMap_AdditionalProperties + err := json.Unmarshal(fieldBuf, &fieldVal) + if err != nil { + return fmt.Errorf("error unmarshaling field %s: %w", fieldName, err) + } + a.AdditionalProperties[fieldName] = fieldVal + } + } + return nil +} + +// Override default JSON handling for NodeMap_AdditionalProperties to handle AdditionalProperties +func (a NodeMap_AdditionalProperties) MarshalJSON() ([]byte, error) { + var err error + object := make(map[string]json.RawMessage) + + if a.Extra != nil { + object["extra"], err = json.Marshal(a.Extra) + if err != nil { + return nil, fmt.Errorf("error marshaling 'extra': %w", err) + } + } + + for fieldName, field := range a.AdditionalProperties { + object[fieldName], err = json.Marshal(field) + if err != nil { + return nil, fmt.Errorf("error marshaling '%s': %w", fieldName, err) + } + } + return json.Marshal(object) +} + // AsFilterValue returns the union data inside the FilterPredicate as a FilterValue func (t FilterPredicate) AsFilterValue() (FilterValue, error) { var body FilterValue @@ -460,3 +670,319 @@ func (t *FilterValue) UnmarshalJSON(b []byte) error { err := t.union.UnmarshalJSON(b) return err } + +// AsNode0 returns the union data inside the Node as a Node0 +func (t Node) AsNode0() (Node0, error) { + var body Node0 + err := json.Unmarshal(t.union, &body) + return body, err +} + +// FromNode0 overwrites any union data inside the Node as the provided Node0 +func (t *Node) FromNode0(v Node0) error { + b, err := json.Marshal(v) + t.union = b + return err +} + +// MergeNode0 performs a merge with any union data inside the Node, using the provided Node0 +func (t *Node) MergeNode0(v Node0) error { + b, err := json.Marshal(v) + if err != nil { + return err + } + + merged, err := runtime.JSONMerge(t.union, b) + t.union = merged + return err +} + +// AsNode1 returns the union data inside the Node as a Node1 +func (t Node) AsNode1() (Node1, error) { + var body Node1 + err := json.Unmarshal(t.union, &body) + return body, err +} + +// FromNode1 overwrites any union data inside the Node as the provided Node1 +func (t *Node) FromNode1(v Node1) error { + b, err := json.Marshal(v) + t.union = b + return err +} + +// MergeNode1 performs a merge with any union data inside the Node, using the provided Node1 +func (t *Node) MergeNode1(v Node1) error { + b, err := json.Marshal(v) + if err != nil { + return err + } + + merged, err := runtime.JSONMerge(t.union, b) + t.union = merged + return err +} + +func (t Node) MarshalJSON() ([]byte, error) { + b, err := t.union.MarshalJSON() + return b, err +} + +func (t *Node) UnmarshalJSON(b []byte) error { + err := t.union.UnmarshalJSON(b) + return err +} + +// AsNode1Children0 returns the union data inside the Node_1_Children_Item as a Node1Children0 +func (t Node_1_Children_Item) AsNode1Children0() (Node1Children0, error) { + var body Node1Children0 + err := json.Unmarshal(t.union, &body) + return body, err +} + +// FromNode1Children0 overwrites any union data inside the Node_1_Children_Item as the provided Node1Children0 +func (t *Node_1_Children_Item) FromNode1Children0(v Node1Children0) error { + b, err := json.Marshal(v) + t.union = b + return err +} + +// MergeNode1Children0 performs a merge with any union data inside the Node_1_Children_Item, using the provided Node1Children0 +func (t *Node_1_Children_Item) MergeNode1Children0(v Node1Children0) error { + b, err := json.Marshal(v) + if err != nil { + return err + } + + merged, err := runtime.JSONMerge(t.union, b) + t.union = merged + return err +} + +// AsNode1Children1 returns the union data inside the Node_1_Children_Item as a Node1Children1 +func (t Node_1_Children_Item) AsNode1Children1() (Node1Children1, error) { + var body Node1Children1 + err := json.Unmarshal(t.union, &body) + return body, err +} + +// FromNode1Children1 overwrites any union data inside the Node_1_Children_Item as the provided Node1Children1 +func (t *Node_1_Children_Item) FromNode1Children1(v Node1Children1) error { + b, err := json.Marshal(v) + t.union = b + return err +} + +// MergeNode1Children1 performs a merge with any union data inside the Node_1_Children_Item, using the provided Node1Children1 +func (t *Node_1_Children_Item) MergeNode1Children1(v Node1Children1) error { + b, err := json.Marshal(v) + if err != nil { + return err + } + + merged, err := runtime.JSONMerge(t.union, b) + t.union = merged + return err +} + +func (t Node_1_Children_Item) MarshalJSON() ([]byte, error) { + b, err := t.union.MarshalJSON() + if err != nil { + return nil, err + } + object := make(map[string]json.RawMessage) + if t.union != nil { + err = json.Unmarshal(b, &object) + if err != nil { + return nil, err + } + } + + if t.Extra != nil { + object["extra"], err = json.Marshal(t.Extra) + if err != nil { + return nil, fmt.Errorf("error marshaling 'extra': %w", err) + } + } + b, err = json.Marshal(object) + return b, err +} + +func (t *Node_1_Children_Item) UnmarshalJSON(b []byte) error { + err := t.union.UnmarshalJSON(b) + if err != nil { + return err + } + object := make(map[string]json.RawMessage) + err = json.Unmarshal(b, &object) + if err != nil { + return err + } + + if raw, found := object["extra"]; found { + err = json.Unmarshal(raw, &t.Extra) + if err != nil { + return fmt.Errorf("error reading 'extra': %w", err) + } + } + + return err +} + +// AsNodeNestedAllOf0 returns the union data inside the NodeNestedAllOf as a NodeNestedAllOf0 +func (t NodeNestedAllOf) AsNodeNestedAllOf0() (NodeNestedAllOf0, error) { + var body NodeNestedAllOf0 + err := json.Unmarshal(t.union, &body) + return body, err +} + +// FromNodeNestedAllOf0 overwrites any union data inside the NodeNestedAllOf as the provided NodeNestedAllOf0 +func (t *NodeNestedAllOf) FromNodeNestedAllOf0(v NodeNestedAllOf0) error { + b, err := json.Marshal(v) + t.union = b + return err +} + +// MergeNodeNestedAllOf0 performs a merge with any union data inside the NodeNestedAllOf, using the provided NodeNestedAllOf0 +func (t *NodeNestedAllOf) MergeNodeNestedAllOf0(v NodeNestedAllOf0) error { + b, err := json.Marshal(v) + if err != nil { + return err + } + + merged, err := runtime.JSONMerge(t.union, b) + t.union = merged + return err +} + +// AsNodeNestedAllOf1 returns the union data inside the NodeNestedAllOf as a NodeNestedAllOf1 +func (t NodeNestedAllOf) AsNodeNestedAllOf1() (NodeNestedAllOf1, error) { + var body NodeNestedAllOf1 + err := json.Unmarshal(t.union, &body) + return body, err +} + +// FromNodeNestedAllOf1 overwrites any union data inside the NodeNestedAllOf as the provided NodeNestedAllOf1 +func (t *NodeNestedAllOf) FromNodeNestedAllOf1(v NodeNestedAllOf1) error { + b, err := json.Marshal(v) + t.union = b + return err +} + +// MergeNodeNestedAllOf1 performs a merge with any union data inside the NodeNestedAllOf, using the provided NodeNestedAllOf1 +func (t *NodeNestedAllOf) MergeNodeNestedAllOf1(v NodeNestedAllOf1) error { + b, err := json.Marshal(v) + if err != nil { + return err + } + + merged, err := runtime.JSONMerge(t.union, b) + t.union = merged + return err +} + +func (t NodeNestedAllOf) MarshalJSON() ([]byte, error) { + b, err := t.union.MarshalJSON() + return b, err +} + +func (t *NodeNestedAllOf) UnmarshalJSON(b []byte) error { + err := t.union.UnmarshalJSON(b) + return err +} + +// AsNodeNestedAllOf1Children0 returns the union data inside the NodeNestedAllOf_1_Children_Item as a NodeNestedAllOf1Children0 +func (t NodeNestedAllOf_1_Children_Item) AsNodeNestedAllOf1Children0() (NodeNestedAllOf1Children0, error) { + var body NodeNestedAllOf1Children0 + err := json.Unmarshal(t.union, &body) + return body, err +} + +// FromNodeNestedAllOf1Children0 overwrites any union data inside the NodeNestedAllOf_1_Children_Item as the provided NodeNestedAllOf1Children0 +func (t *NodeNestedAllOf_1_Children_Item) FromNodeNestedAllOf1Children0(v NodeNestedAllOf1Children0) error { + b, err := json.Marshal(v) + t.union = b + return err +} + +// MergeNodeNestedAllOf1Children0 performs a merge with any union data inside the NodeNestedAllOf_1_Children_Item, using the provided NodeNestedAllOf1Children0 +func (t *NodeNestedAllOf_1_Children_Item) MergeNodeNestedAllOf1Children0(v NodeNestedAllOf1Children0) error { + b, err := json.Marshal(v) + if err != nil { + return err + } + + merged, err := runtime.JSONMerge(t.union, b) + t.union = merged + return err +} + +// AsNodeNestedAllOf1Children1 returns the union data inside the NodeNestedAllOf_1_Children_Item as a NodeNestedAllOf1Children1 +func (t NodeNestedAllOf_1_Children_Item) AsNodeNestedAllOf1Children1() (NodeNestedAllOf1Children1, error) { + var body NodeNestedAllOf1Children1 + err := json.Unmarshal(t.union, &body) + return body, err +} + +// FromNodeNestedAllOf1Children1 overwrites any union data inside the NodeNestedAllOf_1_Children_Item as the provided NodeNestedAllOf1Children1 +func (t *NodeNestedAllOf_1_Children_Item) FromNodeNestedAllOf1Children1(v NodeNestedAllOf1Children1) error { + b, err := json.Marshal(v) + t.union = b + return err +} + +// MergeNodeNestedAllOf1Children1 performs a merge with any union data inside the NodeNestedAllOf_1_Children_Item, using the provided NodeNestedAllOf1Children1 +func (t *NodeNestedAllOf_1_Children_Item) MergeNodeNestedAllOf1Children1(v NodeNestedAllOf1Children1) error { + b, err := json.Marshal(v) + if err != nil { + return err + } + + merged, err := runtime.JSONMerge(t.union, b) + t.union = merged + return err +} + +func (t NodeNestedAllOf_1_Children_Item) MarshalJSON() ([]byte, error) { + b, err := t.union.MarshalJSON() + if err != nil { + return nil, err + } + object := make(map[string]json.RawMessage) + if t.union != nil { + err = json.Unmarshal(b, &object) + if err != nil { + return nil, err + } + } + + if t.Extra != nil { + object["extra"], err = json.Marshal(t.Extra) + if err != nil { + return nil, fmt.Errorf("error marshaling 'extra': %w", err) + } + } + b, err = json.Marshal(object) + return b, err +} + +func (t *NodeNestedAllOf_1_Children_Item) UnmarshalJSON(b []byte) error { + err := t.union.UnmarshalJSON(b) + if err != nil { + return err + } + object := make(map[string]json.RawMessage) + err = json.Unmarshal(b, &object) + if err != nil { + return err + } + + if raw, found := object["extra"]; found { + err = json.Unmarshal(raw, &t.Extra) + if err != nil { + return fmt.Errorf("error reading 'extra': %w", err) + } + } + + return err +} diff --git a/internal/test/schemas/recursive/recursive_test.go b/internal/test/schemas/recursive/recursive_test.go index b66f230b2..d0f471137 100644 --- a/internal/test/schemas/recursive/recursive_test.go +++ b/internal/test/schemas/recursive/recursive_test.go @@ -1,5 +1,13 @@ package schemasrecursive +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + // issue #52: recursion via additionalProperties — compile-only. // The original test called codegen.Generate() to verify no infinite loop; // here the generated types compiling is sufficient evidence. @@ -20,3 +28,92 @@ var _ FilterValue // The original test verified generation succeeds; compilation confirms it. var _ RecursiveObject var _ NonRecursiveObject + +// issue #2542: an allOf that composes a schema whose own body contains that +// allOf. Generation used to overflow the stack. The composition now becomes a +// named type that refers back to itself, which is how Go expresses this and +// how the generator already represents every non-recursive allOf: flattened +// and concrete, with no union wrapper. +var _ Node +var _ Node0 +var _ Node1 +var _ Node_1_Children_Item + +// Object variant: no union anywhere, just a struct that contains a slice of +// itself. +var _ NodeObject +var _ NodeObject_Children_Item + +// Transitive variant: the self-$ref hides behind an allOf nested inside +// another allOf member. +var _ NodeNestedAllOf +var _ NodeNestedAllOf_1_Children_Item + +// Mutual variant: the cycle never passes through the component being +// generated. +var _ MutualA +var _ MutualB +var _ MutualC + +// Bystander variant: the cycle is reached from a component outside it. +var _ Wrapper +var _ Wrapper_N_Children_Item + +// additionalProperties variant: the cycle closes through a map, at the +// component root. items and additionalProperties reuse their parent's path, +// so this sits at path length 1. +var _ NodeMap +var _ NodeMap_AdditionalProperties + +// TestIssue2542ObjectRoundTrip checks that the composed type carries both +// halves of the allOf — the referenced schema's fields and the sibling's — +// and that the recursion nests to arbitrary depth. +func TestIssue2542ObjectRoundTrip(t *testing.T) { + grandchild := NodeObject_Children_Item{Extra: ptr("deep")} + child := NodeObject_Children_Item{ + Extra: ptr("e"), + Children: &[]NodeObject_Children_Item{grandchild}, + } + root := NodeObject{Children: &[]NodeObject_Children_Item{child}} + + b, err := json.Marshal(root) + require.NoError(t, err) + assert.JSONEq(t, `{"children":[{"extra":"e","children":[{"extra":"deep"}]}]}`, string(b)) + + var back NodeObject + require.NoError(t, json.Unmarshal(b, &back)) + require.NotNil(t, back.Children) + require.Len(t, *back.Children, 1) + assert.Equal(t, "e", *(*back.Children)[0].Extra) + inner := (*back.Children)[0].Children + require.NotNil(t, inner) + require.Len(t, *inner, 1) + assert.Equal(t, "deep", *(*inner)[0].Extra) +} + +// TestIssue2542UnionRoundTrip covers the shape from the issue itself. Node is +// an anyOf, so the composed type is still a union — over Node's own branches +// regenerated at this position, which is what a non-recursive allOf over a +// union already produces. +func TestIssue2542UnionRoundTrip(t *testing.T) { + var leaf Node_1_Children_Item + require.NoError(t, leaf.FromNode1Children0(Node1Children0{Leaf: ptr("l")})) + leaf.Extra = ptr("e") + + var root Node + require.NoError(t, root.FromNode1(Node1{Children: &[]Node_1_Children_Item{leaf}})) + + b, err := json.Marshal(root) + require.NoError(t, err) + assert.JSONEq(t, `{"children":[{"leaf":"l","extra":"e"}]}`, string(b)) + + var back Node + require.NoError(t, json.Unmarshal(b, &back)) + n1, err := back.AsNode1() + require.NoError(t, err) + require.NotNil(t, n1.Children) + require.Len(t, *n1.Children, 1) + assert.Equal(t, "e", *(*n1.Children)[0].Extra) +} + +func ptr[T any](v T) *T { return &v } diff --git a/internal/test/schemas/recursive/spec.yaml b/internal/test/schemas/recursive/spec.yaml index bf41a04c7..d356714f3 100644 --- a/internal/test/schemas/recursive/spec.yaml +++ b/internal/test/schemas/recursive/spec.yaml @@ -108,3 +108,130 @@ components: properties: FieldInNonRecursive: type: string + + # --- issue #2542: recursive anyOf wrapping self-$ref in allOf --- + # Node is a union; one branch has a children array whose items are + # allOf: [$ref: Node, {extra}]. Merging that allOf used to + # re-propagate Node's anyOf and overflow the stack; it must + # terminate instead. + Node: + anyOf: + - type: object + properties: + leaf: + type: string + - type: object + properties: + children: + type: array + items: + allOf: + - $ref: '#/components/schemas/Node' + - type: object + properties: + extra: + type: string + + # --- issue #2542 (object variant): plain recursive object + allOf --- + # The same crash shape without a union: children items are + # allOf: [$ref: NodeObject, {extra}], which also overflowed the + # stack and now terminates with a reference back to NodeObject. + NodeObject: + type: object + properties: + children: + type: array + items: + allOf: + - $ref: '#/components/schemas/NodeObject' + - type: object + properties: + extra: + type: string + + # --- issue #2542 (transitive variant): self-$ref behind nested allOf --- + # The recursive $ref sits inside an allOf nested inside another + # allOf member. This escaped the cycle guard (PR #2543 review) and + # hung generation indefinitely; it must terminate with the + # reference preserved. + NodeNestedAllOf: + anyOf: + - type: object + properties: + leaf: + type: string + - type: object + properties: + children: + type: array + items: + allOf: + - allOf: + - $ref: '#/components/schemas/NodeNestedAllOf' + - type: object + properties: + extra: + type: string + + # --- issue #2542 (mutual variant): a cycle that never reaches the + # component being generated. Generating MutualA walks into MutualB and + # MutualC, which compose each other, so a guard keyed on "refers back to + # the schema at the root of this generation" never fires. + MutualA: + type: object + properties: + b: + allOf: + - $ref: '#/components/schemas/MutualB' + - type: object + properties: + tag: + type: string + MutualB: + type: object + properties: + c: + allOf: + - $ref: '#/components/schemas/MutualC' + - type: object + properties: + x: + type: string + MutualC: + type: object + properties: + back: + allOf: + - $ref: '#/components/schemas/MutualB' + - type: object + properties: + y: + type: string + + # --- issue #2542 (bystander variant): the cycle is reached from a + # component that is not part of it. Wrapper inlines NodeObject's body, + # and it is that inlined body that re-enters itself. + Wrapper: + type: object + properties: + n: + allOf: + - $ref: '#/components/schemas/NodeObject' + - type: object + properties: + extra: + type: string + + # --- issue #2542 (additionalProperties variant): the cycle closes + # through a map at the component root. items and additionalProperties + # reuse their parent's path, so this sits at path length 1 and escapes + # any guard keyed on nesting depth. + NodeMap: + type: object + additionalProperties: + allOf: + - $ref: '#/components/schemas/NodeMap' + - type: object + properties: + extra: + type: string diff --git a/pkg/codegen/codegen.go b/pkg/codegen/codegen.go index 4b3840d37..ec5d084d4 100644 --- a/pkg/codegen/codegen.go +++ b/pkg/codegen/codegen.go @@ -1164,7 +1164,7 @@ func GenerateTypesForSchemas(t *template.Template, schemas map[string]*openapi3. } schemaRef := schemas[schemaName] - goSchema, err := GenerateGoSchema(schemaRef, []string{schemaName}) + goSchema, err := generateGoSchema(newRootGenContext([]string{schemaName}), schemaRef, []string{schemaName}) if err != nil { return nil, fmt.Errorf("error converting Schema %s to Go type: %w", schemaName, err) } diff --git a/pkg/codegen/gencontext.go b/pkg/codegen/gencontext.go new file mode 100644 index 000000000..3874c58d8 --- /dev/null +++ b/pkg/codegen/gencontext.go @@ -0,0 +1,104 @@ +package codegen + +import ( + "slices" + + "github.com/getkin/kin-openapi/openapi3" +) + +// genContext carries the state that has to survive the mutual recursion +// between generateGoSchema and the allOf merging code in merge_schemas.go. +// +// The rest of V2 keeps generator state in globalState, but this state is +// positional rather than per-run: it describes where inside one schema's +// expansion we currently are. Threading it keeps generateGoSchema reentrant. +// +// A genContext is passed by value. The map inside it is shared by every copy +// — that is the point of inProgress — while the scalar fields are per-frame, +// so descending can deepen them without the caller having to restore them. +type genContext struct { + // inProgress maps a schema node that owns an allOf to the type being + // generated for that allOf's merged result. mergeSchemas registers an + // entry before generating the merged body and removes it afterwards, so + // a member that refers back into a body an enclosing frame is still + // generating resolves to that type's name instead of being inlined + // again, which is what used to recurse forever (issue #2542). + // + // The key is the schema node that owns the allOf, not the allOf slice: + // the sibling-injection path in generateGoSchema rebuilds that slice on + // every call, so only the owner is stable across re-entry. + inProgress map[*openapi3.Schema]*mergeFrame + + // nameHint is the path a type generated at this position would be named + // from — the argument the calling frame would hand to PathToTypeName. + // It is deliberately not `path`: items and additionalProperties reuse + // their parent's path but name their types with an extra element, and + // changing that would rename nested types across every spec. + nameHint []string + + // rootPosition marks the top of a components/schemas entry, where + // GenerateTypesForSchemas defines the type under a name renameSchema + // chooses rather than one derived from the path. A composition there + // cannot name itself, so mergeSchemas reports the shape instead of + // guessing. No spec is known to reach it — a $ref returns before the + // allOf block, and the only other way back to a component's own allOf + // node is valueWithPropagatedRef's copy, which mergeAllOf flattens + // rather than handing to generateGoSchema — so this is a diagnostic for + // a case believed impossible, not a supported path. + rootPosition bool +} + +// mergeFrame describes an allOf merge that an enclosing frame is part-way +// through generating. +type mergeFrame struct { + // typeName is the Go type the merged result will be defined as, if the + // composition turns out to be recursive. + typeName string + // consulted records whether anything below actually referred back to + // this merge. When nothing did, the composition is not recursive and + // mergeSchemas returns the inline anonymous struct it always has, so + // non-recursive output stays byte-identical. + consulted bool +} + +// newGenContext returns a context rooted at a top-level schema position. +func newGenContext(nameHint []string) genContext { + return genContext{ + inProgress: make(map[*openapi3.Schema]*mergeFrame), + nameHint: slices.Clone(nameHint), + } +} + +// newRootGenContext returns a context rooted at a components/schemas entry. +func newRootGenContext(nameHint []string) genContext { + ctx := newGenContext(nameHint) + ctx.rootPosition = true + return ctx +} + +// at returns a copy of ctx positioned at nameHint. Descending always leaves +// the caller-names-it position behind. +func (ctx genContext) at(nameHint []string) genContext { + ctx.nameHint = slices.Clone(nameHint) + ctx.rootPosition = false + return ctx +} + +// typeName is the Go type a type generated at this position is called. +// +// The name has to be settled before the body is generated, because a +// reference handed to a recursive member needs it, so this predicts what will +// end up defining the type: under generate-types-for-anonymous-schemas +// generateGoSchema hoists an anonymous object under its own path and that +// name wins, otherwise the property, items and additionalProperties naming +// blocks name it after this position. The anonymous-schema rule also depends +// on the generated result, so the prediction is deliberately loose and +// mergeSchemas re-checks it against whatever actually got defined. +// +// PathToTypeName rewrites the slice it is handed, so it gets a copy. +func (ctx genContext) typeName(path []string) string { + if globalState.options.OutputOptions.GenerateTypesForAnonymousSchemas && len(path) > 1 { + return PathToTypeName(slices.Clone(path)) + } + return PathToTypeName(slices.Clone(ctx.nameHint)) +} diff --git a/pkg/codegen/merge_schemas.go b/pkg/codegen/merge_schemas.go index 14b8306d1..ee70bc537 100644 --- a/pkg/codegen/merge_schemas.go +++ b/pkg/codegen/merge_schemas.go @@ -10,22 +10,29 @@ import ( "github.com/getkin/kin-openapi/openapi3" ) -// MergeSchemas merges all the fields in the schemas supplied into one giant schema. -// The idea is that we merge all fields together into one schema. +// MergeSchemas merges all the fields in the schemas supplied into one giant +// schema. The idea is that we merge all fields together into one schema. +// +// It starts a fresh generation context; within the package, prefer +// mergeSchemasCtx so the recursion state survives the descent. func MergeSchemas(allOf []*openapi3.SchemaRef, path []string) (Schema, error) { + return mergeSchemasCtx(newGenContext(path), allOf, path) +} + +func mergeSchemasCtx(ctx genContext, allOf []*openapi3.SchemaRef, path []string) (Schema, error) { // If someone asked for the old way, for backward compatibility, return the // old style result. if globalState.options.Compatibility.OldMergeSchemas { return mergeSchemasV1(allOf, path) } - return mergeSchemas(allOf, path) + return mergeSchemas(ctx, allOf, path) } -func mergeSchemas(allOf []*openapi3.SchemaRef, path []string) (Schema, error) { +func mergeSchemas(ctx genContext, allOf []*openapi3.SchemaRef, path []string) (Schema, error) { n := len(allOf) if n == 1 { - return GenerateGoSchema(allOf[0], path) + return generateGoSchema(ctx, allOf[0], path) } // Distinguish two uses of allOf: @@ -70,7 +77,6 @@ func mergeSchemas(allOf []*openapi3.SchemaRef, path []string) (Schema, error) { } for i := 1; i < n; i++ { - var err error oneOfSchema, err := valueWithPropagatedRef(allOf[i]) if err != nil { return Schema{}, err @@ -109,7 +115,7 @@ func mergeSchemas(allOf []*openapi3.SchemaRef, path []string) (Schema, error) { schema.Extensions = ext } - return GenerateGoSchema(openapi3.NewSchemaRef("", &schema), path) + return generateGoSchema(ctx, openapi3.NewSchemaRef("", &schema), path) } // isExtensionOnlySchema reports whether a schema carries only extensions, @@ -226,7 +232,6 @@ func propagateRemoteRefs(remoteComponent string, schema *openapi3.Schema) { func mergeAllOf(allOf []*openapi3.SchemaRef, seenSchemaRef map[string]bool) (openapi3.Schema, error) { var schema openapi3.Schema for _, schemaRef := range allOf { - var err error if schemaRef.Ref != "" && seenSchemaRef[schemaRef.Ref] { continue } diff --git a/pkg/codegen/merge_schemas_test.go b/pkg/codegen/merge_schemas_test.go index 7daab333f..f47575ec2 100644 --- a/pkg/codegen/merge_schemas_test.go +++ b/pkg/codegen/merge_schemas_test.go @@ -1,6 +1,8 @@ package codegen import ( + "regexp" + "strings" "testing" "github.com/getkin/kin-openapi/openapi3" @@ -227,3 +229,426 @@ func TestMergeOpenapiSchemas_NullableUnion(t *testing.T) { assert.False(t, result.Nullable) }) } + +// generateSpec loads an inline OpenAPI spec and generates models from it. +func generateSpec(t *testing.T, spec string, opts ...func(*Configuration)) string { + t.Helper() + code, err := generateSpecErr(spec, opts...) + require.NoError(t, err) + return code +} + +// generateSpecErr is generateSpec for the cases that are meant to fail. +func generateSpecErr(spec string, opts ...func(*Configuration)) (string, error) { + loader := openapi3.NewLoader() + swagger, err := loader.LoadFromData([]byte(spec)) + if err != nil { + return "", err + } + cfg := Configuration{ + PackageName: "repro", + Generate: GenerateOptions{ + Models: true, + }, + OutputOptions: OutputOptions{ + SkipPrune: true, + }, + } + for _, opt := range opts { + opt(&cfg) + } + return Generate(swagger, cfg) +} + +// assertField asserts that code declares a struct field of the given name and +// Go type, ignoring the column alignment gofmt applies. +func assertField(t *testing.T, code, name, goType string) { + t.Helper() + assert.Regexp(t, `\n\t`+regexp.QuoteMeta(name)+`\s+`+regexp.QuoteMeta(goType)+`\s`, code, + "expected a field %s %s", name, goType) +} + +// specRecursiveObject is the plain-object shape of issue #2542: a tree whose +// array items compose the tree type itself with an extra field. +const specRecursiveObject = `openapi: 3.0.0 +info: {title: repro, version: "1.0.0"} +paths: {} +components: + schemas: + Node: + type: object + properties: + children: + type: array + items: + allOf: + - $ref: '#/components/schemas/Node' + - type: object + properties: + extra: + type: string +` + +// TestMergeSchemasRecursiveObjectAllOf covers the object-recursion variant of +// issue #2542. Generation used to overflow the stack; the composition now +// becomes a named Go type that refers back to itself through the slice, which +// is how the generator represents every other allOf composition — flattened, +// concrete, no json.RawMessage in sight. +func TestMergeSchemasRecursiveObjectAllOf(t *testing.T) { + code := generateSpec(t, specRecursiveObject) + + assert.Contains(t, code, "type Node_Children_Item struct {") + // Node ∧ {extra}: the referenced schema's own fields are merged in, and + // the recursion closes on the composed type, not on Node — grandchildren + // carry `extra` too, which is what the spec says. + assertField(t, code, "Children", "*[]Node_Children_Item") + assertField(t, code, "Extra", "*string") + assert.NotContains(t, code, "union json.RawMessage", + "an allOf composition must not be represented as a union") + assert.NotContains(t, code, "AsNode(") +} + +// TestMergeSchemasRecursiveAnyOfAllOf reproduces the exact schema from issue +// #2542. Node is genuinely an anyOf, so the composed type is still a union — +// but over Node's own branches, regenerated, exactly as a non-recursive +// allOf over a union already generates. The recursion closes on the composed +// item type. +func TestMergeSchemasRecursiveAnyOfAllOf(t *testing.T) { + const spec = `openapi: 3.0.0 +info: {title: repro, version: "1.0.0"} +paths: {} +components: + schemas: + Node: + anyOf: + - type: object + properties: + leaf: + type: string + - type: object + properties: + children: + type: array + items: + allOf: + - $ref: '#/components/schemas/Node' + - type: object + properties: + extra: + type: string +` + + code := generateSpec(t, spec) + + assert.Contains(t, code, "type Node_1_Children_Item struct {") + assertField(t, code, "Extra", "*string") + // The branches of the composed union are Node's branches regenerated at + // this position, and the recursive one points back at the composed type. + assert.Contains(t, code, "type Node1Children1 struct {") + assertField(t, code, "Children", "*[]Node_1_Children_Item") + assert.Contains(t, code, "func (t Node_1_Children_Item) AsNode1Children1() (Node1Children1, error)") +} + +// TestMergeSchemasNestedAllOfSelfRef covers the self-$ref hiding behind a +// nested allOf member, which escapes a guard that only inspects the members +// of the allOf it was handed. +func TestMergeSchemasNestedAllOfSelfRef(t *testing.T) { + const spec = `openapi: 3.0.0 +info: {title: repro, version: "1.0.0"} +paths: {} +components: + schemas: + Node: + type: object + properties: + children: + type: array + items: + allOf: + - allOf: + - $ref: '#/components/schemas/Node' + - type: object + properties: + extra: + type: string +` + + code := generateSpec(t, spec) + assert.Contains(t, code, "type Node_Children_Item struct {") + assertField(t, code, "Children", "*[]Node_Children_Item") + assertField(t, code, "Extra", "*string") +} + +// TestMergeSchemasRecursionThroughAdditionalProperties covers a cycle that +// closes through additionalProperties. items and additionalProperties reuse +// their parent's path, so a guard keyed on path depth misses this one even +// though it sits at the component root. +func TestMergeSchemasRecursionThroughAdditionalProperties(t *testing.T) { + const spec = `openapi: 3.0.0 +info: {title: repro, version: "1.0.0"} +paths: {} +components: + schemas: + Node: + type: object + additionalProperties: + allOf: + - $ref: '#/components/schemas/Node' + - type: object + properties: + extra: + type: string +` + + code := generateSpec(t, spec) + assert.Contains(t, code, "Node_AdditionalProperties") + assert.Contains(t, code, "map[string]Node_AdditionalProperties") +} + +// TestMergeSchemasMutualRecursion covers a cycle that never passes through +// the component being generated: generating A walks into B and C, which +// compose each other. A guard keyed on "refers to the schema at path[0]" +// never fires here. +func TestMergeSchemasMutualRecursion(t *testing.T) { + const spec = `openapi: 3.0.0 +info: {title: repro, version: "1.0.0"} +paths: {} +components: + schemas: + A: + type: object + properties: + b: + allOf: + - $ref: '#/components/schemas/B' + - type: object + properties: + tag: + type: string + B: + type: object + properties: + c: + allOf: + - $ref: '#/components/schemas/C' + - type: object + properties: + x: + type: string + C: + type: object + properties: + back: + allOf: + - $ref: '#/components/schemas/B' + - type: object + properties: + y: + type: string +` + + code := generateSpec(t, spec) + for _, want := range []string{"type A struct {", "type B struct {", "type C struct {"} { + assert.Contains(t, code, want) + } + // The cycle has to close on a named type rather than unrolling: the type + // generated for B.c contains a field whose type is itself. + assert.Regexp(t, `type B_C struct \{(.|\n)*\*B_C`, code) + assert.NotContains(t, code, "union json.RawMessage") +} + +// TestMergeSchemasRecursionViaOtherComponent covers a cycle reached from a +// component that is not part of it: Wrapper inlines Node's body, and it is +// that inlined body that re-enters itself. +func TestMergeSchemasRecursionViaOtherComponent(t *testing.T) { + const spec = `openapi: 3.0.0 +info: {title: repro, version: "1.0.0"} +paths: {} +components: + schemas: + Node: + type: object + properties: + children: + type: array + items: + allOf: + - $ref: '#/components/schemas/Node' + - type: object + properties: + extra: + type: string + Wrapper: + type: object + properties: + n: + allOf: + - $ref: '#/components/schemas/Node' + - type: object + properties: + extra: + type: string +` + + code := generateSpec(t, spec) + assert.Contains(t, code, "type Wrapper struct {") + assert.Contains(t, code, "type Node_Children_Item struct {") +} + +// TestMergeSchemasValueRecursionGeneratesUncompilableGo pins a deliberate +// choice. A composition that refers to itself with no pointer, slice or map +// in between describes a Go value that contains itself. The generator emits +// what the spec declares and leaves the objection to the compiler, which says +// `invalid recursive type: T_Child refers to itself` and points at the line. +// Detecting it here would mean predicting, before generating the body, what +// the field rendering will do with it — which cannot be done for a +// SkipOptionalPointer that the merge itself produces (the #1957 decorator +// idiom). If this test starts failing, that trade-off is being revisited. +func TestMergeSchemasValueRecursionGeneratesUncompilableGo(t *testing.T) { + const spec = `openapi: 3.0.0 +info: {title: repro, version: "1.0.0"} +paths: {} +components: + schemas: + T: + type: object + required: [child] + properties: + child: + allOf: + - $ref: '#/components/schemas/T' + - type: object + properties: + w: + type: integer +` + + code := generateSpec(t, spec) + // Generation terminates rather than overflowing the stack (issue #2542). + assertField(t, code, "Child", "T_Child") + assert.Contains(t, code, "type T_Child struct {") +} + +// TestMergeSchemasValueRecursionOptionalIsFine is the same shape with the +// property left optional, which renders as a pointer and so terminates. +func TestMergeSchemasValueRecursionOptionalIsFine(t *testing.T) { + const spec = `openapi: 3.0.0 +info: {title: repro, version: "1.0.0"} +paths: {} +components: + schemas: + T: + type: object + properties: + child: + allOf: + - $ref: '#/components/schemas/T' + - type: object + properties: + w: + type: integer +` + + code := generateSpec(t, spec) + assertField(t, code, "Child", "*T_Child") + assertField(t, code, "W", "*int") +} + +// TestMergeSchemasNonRecursiveAllOfUnaffected pins the property this change +// depends on for backwards compatibility: a composition nothing refers back +// to is still emitted as the inline anonymous struct it always was, with no +// named type invented for it. +func TestMergeSchemasNonRecursiveAllOfUnaffected(t *testing.T) { + const spec = `openapi: 3.0.0 +info: {title: repro, version: "1.0.0"} +paths: {} +components: + schemas: + Base: + type: object + properties: + name: + type: string + Holder: + type: object + properties: + list: + type: array + items: + allOf: + - $ref: '#/components/schemas/Base' + - type: object + properties: + weight: + type: integer +` + + code := generateSpec(t, spec) + assert.Contains(t, code, "List *[]struct {") + assert.NotContains(t, code, "Holder_List_Item", + "a non-recursive composition must not gain a named type") + assert.Equal(t, 1, strings.Count(code, "type Holder struct {")) +} + +// TestMergeSchemasRecursiveWithAnonymousSchemaTypes covers the interaction +// with generate-types-for-anonymous-schemas, which hoists the merged body +// under its own path. The name a recursive member was handed has to be the +// one that ends up defined, or the generated code refers to a type that does +// not exist. +func TestMergeSchemasRecursiveWithAnonymousSchemaTypes(t *testing.T) { + anon := func(c *Configuration) { c.OutputOptions.GenerateTypesForAnonymousSchemas = true } + + t.Run("array items", func(t *testing.T) { + code := generateSpec(t, specRecursiveObject, anon) + assertDefinesWhatItReferences(t, code) + }) + + t.Run("additionalProperties", func(t *testing.T) { + const spec = `openapi: 3.0.0 +info: {title: repro, version: "1.0.0"} +paths: {} +components: + schemas: + Node: + type: object + properties: + kids: + type: object + additionalProperties: + allOf: + - $ref: '#/components/schemas/Node' + - type: object + properties: + extra: + type: string +` + code := generateSpec(t, spec, anon) + assertDefinesWhatItReferences(t, code) + }) +} + +// assertDefinesWhatItReferences checks that every generated type referenced +// from a struct field is also declared, which is the cheapest stand-in for +// "the output compiles". +func assertDefinesWhatItReferences(t *testing.T, code string) { + t.Helper() + declared := map[string]bool{} + for _, m := range regexp.MustCompile(`(?m)^type (\w+) `).FindAllStringSubmatch(code, -1) { + declared[m[1]] = true + } + referenced := regexp.MustCompile(`\*?\[?\]?\*?(Node\w*)\s+`+"`json:").FindAllStringSubmatch(code, -1) + require.NotEmpty(t, referenced, "expected the output to reference a generated type") + for _, m := range referenced { + assert.True(t, declared[m[1]], "field refers to %s, which is never declared:\n%s", m[1], code) + } +} + +// TestMergeSchemasRecursionUnderOldMergeSchemas pins that the legacy merge +// path is unaffected. It embeds $ref members instead of inlining them, so it +// never recursed, and it bypasses the state this fix threads. +func TestMergeSchemasRecursionUnderOldMergeSchemas(t *testing.T) { + code := generateSpec(t, specRecursiveObject, func(c *Configuration) { + c.Compatibility.OldMergeSchemas = true + }) + assert.Contains(t, code, "type Node struct {") +} diff --git a/pkg/codegen/schema.go b/pkg/codegen/schema.go index a8bcf076a..c85602e6e 100644 --- a/pkg/codegen/schema.go +++ b/pkg/codegen/schema.go @@ -985,7 +985,17 @@ func schemaUnionTypes(t *openapi3.Types) []string { return primary.Slice() } +// GenerateGoSchema converts an OpenAPI schema into a Go type definition. +// +// It starts a fresh generation context, so a call made from outside the +// package behaves exactly as it always has. Within the package, prefer +// generateGoSchema so that the recursion state described by genContext +// survives the descent. func GenerateGoSchema(sref *openapi3.SchemaRef, path []string) (Schema, error) { + return generateGoSchema(newGenContext(path), sref, path) +} + +func generateGoSchema(ctx genContext, sref *openapi3.SchemaRef, path []string) (Schema, error) { // Add a fallback value in case the sref is nil. // i.e. the parent schema defines a type:array, but the array has // no items defined. Therefore, we have at least valid Go-Code. @@ -1065,8 +1075,25 @@ func GenerateGoSchema(sref *openapi3.SchemaRef, path []string) (Schema, error) { // so that in a RESTful paradigm, the Create operation can return // (object, id), so that other operations can refer to (id) if schema.AllOf != nil { - var mergedSchema Schema + // An enclosing frame is already generating this composition. Refer to + // the type it is building instead of inlining the body a second time, + // which is what used to recurse until the stack ran out (issue #2542). + if frame, ok := ctx.inProgress[schema]; ok { + frame.consulted = true + return Schema{ + GoType: frame.typeName, + RefType: frame.typeName, + DefineViaAlias: true, + SkipOptionalPointer: skipOptionalPointer, + OAPISchema: schema, + }, nil + } var err error + frame := &mergeFrame{typeName: ctx.typeName(path)} + ctx.inProgress[schema] = frame + defer delete(ctx.inProgress, schema) + + var mergedSchema Schema // Behavior is gated on Compatibility.OldAllOfSiblingMerging: // when set, the parent's structural siblings and Description are // silently discarded (the historical behavior). When unset @@ -1088,13 +1115,13 @@ func GenerateGoSchema(sref *openapi3.SchemaRef, path []string) (Schema, error) { allOfRefs := make([]*openapi3.SchemaRef, 0, len(schema.AllOf)+1) allOfRefs = append(allOfRefs, schema.AllOf...) allOfRefs = append(allOfRefs, &openapi3.SchemaRef{Value: &s}) - mergedSchema, err = MergeSchemas(allOfRefs, path) + mergedSchema, err = mergeSchemasCtx(ctx, allOfRefs, path) } else { // Either the user opted into legacy behavior, or the parent is // a pure wrapper with no structural siblings. In the wrapper // case, MergeSchemas' single-element fast path returns the // referenced type unchanged, preserving named-type identity. - mergedSchema, err = MergeSchemas(schema.AllOf, path) + mergedSchema, err = mergeSchemasCtx(ctx, schema.AllOf, path) } if err != nil { return Schema{}, fmt.Errorf("error merging schemas: %w", err) @@ -1116,6 +1143,41 @@ func GenerateGoSchema(sref *openapi3.SchemaRef, path []string) (Schema, error) { if _, ok := extensions[extPropGoTypeSkipOptionalPointer]; ok { mergedSchema.SkipOptionalPointer = skipOptionalPointer } + // Something underneath referred back to this composition, so it has + // to resolve to a named type. When nothing did — the overwhelmingly + // common case — fall through with the anonymous struct this has + // always produced, byte for byte. + if frame.consulted { + switch { + case mergedSchema.RefType == frame.typeName: + // Already defined under the promised name: generating the + // merged body hoisted it (generate-types-for-anonymous-schemas). + case mergedSchema.RefType != "": + // The name handed to the recursive members is not the one the + // type ended up with, so those references would dangle. Fail + // loudly rather than emit code that does not compile. + return Schema{}, fmt.Errorf( + "recursive allOf composition at %s was generated as %q but its self-references were resolved to %q", + strings.Join(ctx.nameHint, "."), mergedSchema.RefType, frame.typeName) + case ctx.rootPosition: + // GenerateTypesForSchemas names this one, from renameSchema + // rather than from the path, so the name handed to the + // members above is not the one it will be defined under. + // Believed unreachable (see genContext.rootPosition); say so + // rather than emit code that will not compile. + return Schema{}, fmt.Errorf( + "recursive allOf composition at the root of %s is not supported: give the composition its own schema", + strings.Join(ctx.nameHint, ".")) + default: + typeDef := TypeDefinition{ + TypeName: frame.typeName, + JsonName: strings.Join(ctx.nameHint, "."), + Schema: mergedSchema, + } + mergedSchema.AdditionalTypes = append(mergedSchema.AdditionalTypes, typeDef) + mergedSchema.RefType = frame.typeName + } + } return mergedSchema, nil } @@ -1132,7 +1194,7 @@ func GenerateGoSchema(sref *openapi3.SchemaRef, path []string) (Schema, error) { return Schema{}, err } if items != nil { - if err := oapiSchemaToGoType(typeSource, path, &outSchema); err != nil { + if err := oapiSchemaToGoType(ctx, typeSource, path, &outSchema); err != nil { return Schema{}, fmt.Errorf("error resolving primitive type for enum-via-oneOf: %w", err) } // Force a typed declaration -- enums must not be aliased. @@ -1211,7 +1273,8 @@ func GenerateGoSchema(sref *openapi3.SchemaRef, path []string) (Schema, error) { // If additional properties are defined, we will override the default // above with the specific definition. if schema.AdditionalProperties.Schema != nil { - additionalSchema, err := GenerateGoSchema(schema.AdditionalProperties.Schema, path) + apHint := append(slices.Clone(path), "AdditionalProperties") + additionalSchema, err := generateGoSchema(ctx.at(apHint), schema.AdditionalProperties.Schema, path) if err != nil { return Schema{}, fmt.Errorf("error generating type for additional properties: %w", err) } @@ -1253,8 +1316,8 @@ func GenerateGoSchema(sref *openapi3.SchemaRef, path []string) (Schema, error) { // We've got an object with some properties. for _, pName := range SortedSchemaKeys(schema.Properties) { p := schema.Properties[pName] - propertyPath := append(path, pName) - pSchema, err := GenerateGoSchema(p, propertyPath) + propertyPath := append(slices.Clone(path), pName) + pSchema, err := generateGoSchema(ctx.at(propertyPath), p, propertyPath) if err != nil { return Schema{}, fmt.Errorf("error generating Go schema for property '%s': %w", pName, err) } @@ -1300,12 +1363,12 @@ func GenerateGoSchema(sref *openapi3.SchemaRef, path []string) (Schema, error) { } if schema.AnyOf != nil { - if err := generateUnion(&outSchema, schema.AnyOf, schema.Discriminator, path); err != nil { + if err := generateUnion(ctx, &outSchema, schema.AnyOf, schema.Discriminator, path); err != nil { return Schema{}, fmt.Errorf("error generating type for anyOf: %w", err) } } if schema.OneOf != nil { - if err := generateUnion(&outSchema, schema.OneOf, schema.Discriminator, path); err != nil { + if err := generateUnion(ctx, &outSchema, schema.OneOf, schema.Discriminator, path); err != nil { return Schema{}, fmt.Errorf("error generating type for oneOf: %w", err) } } @@ -1393,7 +1456,7 @@ func GenerateGoSchema(sref *openapi3.SchemaRef, path []string) (Schema, error) { // A multi-type union is excluded for the same reason: it lowers to // `any`, and `const X any = ...` is not a valid Go constant either. // Falling through generates the plain `any` the union maps to. - err := oapiSchemaToGoType(schema, path, &outSchema) + err := oapiSchemaToGoType(ctx, schema, path, &outSchema) // Enums need to be typed, so that the values aren't interchangeable, // so no matter what schema conversion thinks, we need to define a // new type. @@ -1463,7 +1526,7 @@ func GenerateGoSchema(sref *openapi3.SchemaRef, path []string) (Schema, error) { outSchema.RefType = typeName } } else { - err := oapiSchemaToGoType(schema, path, &outSchema) + err := oapiSchemaToGoType(ctx, schema, path, &outSchema) if err != nil { return Schema{}, fmt.Errorf("error resolving primitive type: %w", err) } @@ -1473,7 +1536,7 @@ func GenerateGoSchema(sref *openapi3.SchemaRef, path []string) (Schema, error) { // oapiSchemaToGoType converts an OpenApi schema into a Go type definition for // all non-object types. -func oapiSchemaToGoType(schema *openapi3.Schema, path []string, outSchema *Schema) error { +func oapiSchemaToGoType(ctx genContext, schema *openapi3.Schema, path []string, outSchema *Schema) error { f := schema.Format // In OpenAPI 3.1, `type` may be a multi-element array including "null" // to express nullability. The dispatch below uses `*Types.Is("...")`, @@ -1486,7 +1549,8 @@ func oapiSchemaToGoType(schema *openapi3.Schema, path []string, outSchema *Schem if t.Is("array") { // For arrays, we'll get the type of the Items and throw a // [] in front of it. - arrayType, err := GenerateGoSchema(schema.Items, path) + itemHint := append(slices.Clone(path), "Item") + arrayType, err := generateGoSchema(ctx.at(itemHint), schema.Items, path) if err != nil { return fmt.Errorf("error generating type for array: %w", err) } @@ -1820,7 +1884,7 @@ func paramToGoType(param *openapi3.Parameter, path []string) (Schema, error) { return GenerateGoSchema(mt.Schema, path) } -func generateUnion(outSchema *Schema, elements openapi3.SchemaRefs, discriminator *openapi3.Discriminator, path []string) error { +func generateUnion(ctx genContext, outSchema *Schema, elements openapi3.SchemaRefs, discriminator *openapi3.Discriminator, path []string) error { if discriminator != nil { outSchema.Discriminator = &Discriminator{ Property: discriminator.PropertyName, @@ -1864,7 +1928,7 @@ func generateUnion(outSchema *Schema, elements openapi3.SchemaRefs, discriminato // union specs that may rely on the wrapper shape. The narrow // condition keeps this change scoped to the bug fix. if effectiveCount == 1 && hadNullBranch && discriminator == nil { - elementSchema, err := GenerateGoSchema(soleEffective, path) + elementSchema, err := generateGoSchema(ctx.at(path), soleEffective, path) if err != nil { return err } @@ -1891,7 +1955,7 @@ func generateUnion(outSchema *Schema, elements openapi3.SchemaRefs, discriminato continue } elementPath := append(path, fmt.Sprint(i)) - elementSchema, err := GenerateGoSchema(element, elementPath) + elementSchema, err := generateGoSchema(ctx.at(elementPath), element, elementPath) if err != nil { return err } diff --git a/pkg/codegen/schema_test.go b/pkg/codegen/schema_test.go index 88ee2a072..b4cbdaca9 100644 --- a/pkg/codegen/schema_test.go +++ b/pkg/codegen/schema_test.go @@ -535,7 +535,7 @@ func TestProperty_ZeroValueIsNil(t *testing.T) { func TestOapiSchemaToGoType_NullType(t *testing.T) { schema := &openapi3.Schema{Type: &openapi3.Types{"null"}} var out Schema - require.NoError(t, oapiSchemaToGoType(schema, []string{"Challenger"}, &out)) + require.NoError(t, oapiSchemaToGoType(newGenContext(nil), schema, []string{"Challenger"}, &out)) assert.Equal(t, "any", out.GoType) assert.True(t, out.SkipOptionalPointer) assert.True(t, out.DefineViaAlias) @@ -833,7 +833,7 @@ func TestOapiSchemaToGoType_MultiTypeUnion(t *testing.T) { globalState.typeMapping = DefaultTypeMapping var out Schema - err := oapiSchemaToGoType(&openapi3.Schema{Type: &tc.types}, []string{"Value"}, &out) + err := oapiSchemaToGoType(newGenContext(nil), &openapi3.Schema{Type: &tc.types}, []string{"Value"}, &out) if tc.wantErr { assert.ErrorContains(t, err, "unhandled Schema type") return @@ -996,7 +996,7 @@ func TestOapiSchemaToGoType_MultiTypeUnionRequires31(t *testing.T) { globalState.typeMapping = DefaultTypeMapping var out Schema - err := oapiSchemaToGoType(&openapi3.Schema{Type: &openapi3.Types{"string", "number"}}, []string{"Value"}, &out) + err := oapiSchemaToGoType(newGenContext(nil), &openapi3.Schema{Type: &openapi3.Types{"string", "number"}}, []string{"Value"}, &out) assert.ErrorContains(t, err, "unhandled Schema type") }