diff --git a/compilers/openapi/internal/schema/allof.go b/compilers/openapi/internal/schema/allof.go index 6b952f7d..3e2f7922 100644 --- a/compilers/openapi/internal/schema/allof.go +++ b/compilers/openapi/internal/schema/allof.go @@ -41,10 +41,6 @@ func lowerAllOf(c lowering.Ctx, ts *compile.Types, depth int, s *oas3.Schema, po m := &ir.Model{TypeCommon: common, Constraints: cons} comp := readAllOf(c, s, pointer) // the one flattened list; the readers below take it, not s diags = append(diags, diagDisjointConjuncts(c, comp, composedShape)...) - if allOfDecides(c, s) { - v := voicingOf(c, s) - comp.speaks, comp.rebased = v.speaks, v.rebased - } fields := newModelFields(c, ts, m) diags = append(diags, fillAllOf(c, ts, depth, m, fields, comp)...) diags = append(diags, fillModelProperties(c, ts, depth, fields, s, pointer)...) @@ -167,8 +163,8 @@ type allOfComposition struct { disjoint []string // speaks marks each branch that folds at the position (voicesAmong), and // rebased each whose folding keywords write a relative $ref it reads against - // another base, so it does not. They are nil for a composition distributed - // across a union, whose variants each keep every branch. + // another base, so it does not. They are nil where the allOf is not what the + // position lowers (allOfDecides). speaks []bool rebased []bool } @@ -191,6 +187,22 @@ func allOfBranches(s *oas3.Schema, pointer jsontext.Pointer) allOfComposition { return allOfComposition{pointer: pointer, branches: out} } +// readAllOf is allOfBranches of the schema written at pointer, with what its +// conjuncts jointly admit read and, where the allOf is what the position lowers, +// which branches fold there (voicingOf), for a lowering that composes it. That +// gate is positionVoices', so a branch gives up a keyword only where a reader +// of the position takes it up; both composers pass it, and the report of an +// allOf the election passed over does not. +func readAllOf(c lowering.Ctx, s *oas3.Schema, pointer jsontext.Pointer) allOfComposition { + comp := allOfBranches(s, pointer) + comp.disjoint = disjointConjuncts(c, s, comp) + if allOfDecides(c, s) { + v := voicingOf(c, s) + comp.speaks, comp.rebased = v.speaks, v.rebased + } + return comp +} + // allOfRefBranches yields the $ref branches of s's allOf after flattening, // schemas only: the view an ancestor walk needs, which reads no pointer. func allOfRefBranches(s *oas3.Schema) iter.Seq[*oas3.JSONSchema[oas3.Referenceable]] { diff --git a/compilers/openapi/internal/schema/allof_disjoint.go b/compilers/openapi/internal/schema/allof_disjoint.go index 7981b7b6..4c1fdd01 100644 --- a/compilers/openapi/internal/schema/allof_disjoint.go +++ b/compilers/openapi/internal/schema/allof_disjoint.go @@ -4,7 +4,6 @@ package schema import ( - "encoding/json/jsontext" "strings" "github.com/speakeasy-api/openapi/jsonschema/oas3" @@ -231,11 +230,3 @@ func diagDisjointConjuncts(c lowering.Ctx, comp allOfComposition, shape string) // composedShape is the shape a composition lowers to, for diagDisjointConjuncts. const composedShape = "a composition" - -// readAllOf is allOfBranches of the schema written at pointer, with what its -// conjuncts jointly admit read, for a lowering that composes it. -func readAllOf(c lowering.Ctx, s *oas3.Schema, pointer jsontext.Pointer) allOfComposition { - comp := allOfBranches(s, pointer) - comp.disjoint = disjointConjuncts(c, s, comp) - return comp -} diff --git a/compilers/openapi/internal/schema/allof_residue_test.go b/compilers/openapi/internal/schema/allof_residue_test.go index 758f0f96..2bfc2b77 100644 --- a/compilers/openapi/internal/schema/allof_residue_test.go +++ b/compilers/openapi/internal/schema/allof_residue_test.go @@ -249,9 +249,10 @@ func TestAllOf_EachInlineBranchKeyedSeparately(t *testing.T) { // TestAllOf_BranchResidueRidesEveryDistributedVariant covers fillAllOf's second // caller: buildComposedVariant re-runs the same merge once per union variant, so -// the residue has to land on every variant the composition rides on. The -// diagnostic is announced once even so — the branch is one source construct -// however many variants carry it. +// the residue, the whole branch, has to land on every variant the composition +// rides on, while what the branch folds reaches the Union. The diagnostic is +// announced once even so — the branch is one source construct however many +// variants carry it. func TestAllOf_BranchResidueRidesEveryDistributedVariant(t *testing.T) { t.Parallel() spec := openapitest.ComponentSpec(` Base: {type: object, properties: {id: {type: string}}} @@ -263,6 +264,7 @@ func TestAllOf_BranchResidueRidesEveryDistributedVariant(t *testing.T) { - type: object properties: {c: {type: string}} description: BranchDoc + patternProperties: {'^x': {type: string}} oneOf: - {$ref: '#/components/schemas/A'} - {$ref: '#/components/schemas/B'} @@ -280,6 +282,7 @@ func TestAllOf_BranchResidueRidesEveryDistributedVariant(t *testing.T) { require.True(t, ok, "variant %d carries the branch residue; got %+v", i, variant.Unmodeled) assert.Contains(t, string(entry.Value), `"description":"BranchDoc"`, "variant %d", i) } + assert.Equal(t, "BranchDoc", u.Docs.Description) assert.Equal(t, 1, openapitest.CountDiagsAt(diags, diag.DegradedConstruct, ir.SeverityInfo), "one branch, one diagnostic, however many variants carry it; got %+v", diags) } diff --git a/compilers/openapi/internal/schema/nulls_test.go b/compilers/openapi/internal/schema/nulls_test.go index 5c98869e..d2231f53 100644 --- a/compilers/openapi/internal/schema/nulls_test.go +++ b/compilers/openapi/internal/schema/nulls_test.go @@ -394,10 +394,17 @@ func TestNullableWidening_ABranchWidensTheAllOfHoldingIt(t *testing.T) { assert.Equal(t, "d", propertyOf(t, doc, "Holder", "p").Docs.Description, name) } - besideUnion := openapitest.HolderSpec("3.0.3", - "{oneOf: [{type: string}, {type: integer}], allOf: [{$ref: '#/components/schemas/Plain'}, {nullable: true}]}", "") - doc, _ = compileNullable(t, besideUnion, false) - assert.False(t, holderP(t, doc).Nullable, "nor does one a union conjoins with") + for name, tc := range map[string]struct{ union, code string }{ + "degraded": {"oneOf: [{$ref: '#/components/schemas/Pet'}], anyOf: [{$ref: '#/components/schemas/Toy'}]", diag.DegradedConstruct}, + "distributed": {"oneOf: [{$ref: '#/components/schemas/Pet'}, {$ref: '#/components/schemas/Toy'}]", diag.CompositionLowering}, + } { + besideUnion := openapitest.HolderSpec("3.0.3", "{"+tc.union+", allOf: [{$ref: '#/components/schemas/Pet'}, {nullable: true}]}", + " Toy: {type: object, properties: {kind: {type: string}}}\n") + doc, diags = compileNullable(t, besideUnion, false) + require.True(t, openapitest.HasDiagCodeAt(diags, tc.code, "/components/schemas/Holder/properties/p"), name) + assert.True(t, holderP(t, doc).Nullable, "%s: so does one beside a union, as beside the allOf", name) + assert.NotEmpty(t, holderP(t, doc).Inferred, name) + } properties := openapitest.HolderSpec("3.0.3", "{allOf: [{$ref: '#/components/schemas/Pet'}, {nullable: true, properties: {a: {type: string}}}]}", "") diff --git a/compilers/openapi/internal/schema/refinement.go b/compilers/openapi/internal/schema/refinement.go index 736bc9f8..9f83f73c 100644 --- a/compilers/openapi/internal/schema/refinement.go +++ b/compilers/openapi/internal/schema/refinement.go @@ -76,7 +76,7 @@ type decision struct { // the bindings, never the registry, so a decision turns on no lowering order, and // each position's many readers share one reading. func refinementOf(c lowering.Ctx, s *oas3.Schema, pointer jsontext.Pointer) (refinement, bool) { - if !allOfConjoins(s) { + if !allOfMayRefine(s) { return refinement{}, false } d := lowering.Remembered(c, refinementAt{s, pointer}, func() decision { return decide(c, s, pointer) }) diff --git a/compilers/openapi/internal/schema/union.go b/compilers/openapi/internal/schema/union.go index b4db1a23..765a3b31 100644 --- a/compilers/openapi/internal/schema/union.go +++ b/compilers/openapi/internal/schema/union.go @@ -616,7 +616,8 @@ const refSiteUnionWhy = "oneOf/anyOf beside a $ref conjoin with it rather than c func lowerDistributedUnion(c lowering.Ctx, ts *compile.Types, depth int, s *oas3.Schema, pointer jsontext.Pointer) (ir.TypeID, []ir.Diagnostic) { var diags []ir.Diagnostic id := internNode(c, ts, pointer, func(common ir.TypeCommon) ir.TypeDef { - body := composedBody{schema: s, comp: readAllOf(c, s, pointer), id: common.ID} + body := composedBody{schema: s, comp: readAllOf(c, s, pointer), + voices: positionVoices(c, ts, s, pointer, ir.TypeRef{}), id: common.ID} diags = append(diags, diagDisjointConjuncts(c, body.comp, composedShape)...) def, unionDiags := buildUnion(c, ts, s, common, pointer, func(b *oas3.JSONSchema[oas3.Referenceable], vptr jsontext.Pointer) ir.TypeRef { @@ -638,7 +639,8 @@ func lowerDistributedUnion(c lowering.Ctx, ts *compile.Types, depth int, s *oas3 // declares one schema, not one per branch. type composedBody struct { schema *oas3.Schema - comp allOfComposition // schema's flattened allOf, built once for every variant + comp allOfComposition // schema's flattened allOf, built once for every variant + voices []annotation.Voice // what comp's folding branches say of the position, read once too id ir.TypeID } @@ -693,11 +695,11 @@ func composedVariantRef(c lowering.Ctx, id ir.TypeID, body *oas3.Schema, branch // declared, named after the enclosing schema, not after whichever branch built // them first. // -// The enclosing schema's value constraints are read as lowerAllOf reads them -// (GitHub #407). Each variant carries a copy, since a bound on the schema -// constrains both sides of the disjunction. +// The enclosing schema's value constraints, its folding branches' among them, +// are read as lowerAllOf reads them (GitHub #407). Each variant carries a copy, +// since a bound on the schema constrains both sides of the disjunction. func buildComposedVariant(c lowering.Ctx, ts *compile.Types, depth int, body composedBody, branch ir.TypeID, common ir.TypeCommon) (ir.TypeDef, []ir.Diagnostic) { - cons, diags := ConstraintsOf(c, &common.Unmodeled, body.schema, nil, body.comp.pointer) + cons, diags := ConstraintsOf(c, &common.Unmodeled, body.schema, body.voices, body.comp.pointer) m := &ir.Model{TypeCommon: common, Constraints: cons} fields := newModelFields(c, ts, m) diags = append(diags, fillAllOf(c, ts, depth, m, fields, body.comp)...) diff --git a/compilers/openapi/internal/schema/use.go b/compilers/openapi/internal/schema/use.go index 3517448f..f3d16724 100644 --- a/compilers/openapi/internal/schema/use.go +++ b/compilers/openapi/internal/schema/use.go @@ -176,7 +176,7 @@ func throughRef(s *oas3.Schema) *oas3.JSONSchema[oas3.Referenceable] { if slices.Contains(dispatchOf(s).skipped, "allOf") { return refs[0] } - if !allOfConjoins(s) || annotation.DeclaresAny(s, objectKeywords) || !refinableBranches(allOfBranches(s, "")) { + if !allOfMayRefine(s) || annotation.DeclaresAny(s, objectKeywords) || !refinableBranches(allOfBranches(s, "")) { return nil } return refs[0] diff --git a/compilers/openapi/internal/schema/voices.go b/compilers/openapi/internal/schema/voices.go index 6752c917..a7e5dfe3 100644 --- a/compilers/openapi/internal/schema/voices.go +++ b/compilers/openapi/internal/schema/voices.go @@ -88,21 +88,30 @@ func foldValue(s *oas3.Schema, entry string) foldRendering { return r } -// allOfConjoins reports whether s's allOf conjoins with what its position -// lowers: it is no keyword beside a $ref, a $dynamicRef or a union, whose -// branches keep what they write on each variant -// (docs/compilers/openapi.md#union-beside-structure). -func allOfConjoins(s *oas3.Schema) bool { - return len(s.GetAllOf()) > 0 && s.Ref == nil && !declaresUnion(s) && !declaresDynamicRef(s) +// allOfBesideNoRef reports whether s writes an allOf with no $ref beside it, +// which would make the position a $ref site, whose alias keeps the allOf +// verbatim (docs/compilers/openapi.md#allof-beside-ref). +func allOfBesideNoRef(s *oas3.Schema) bool { + return len(s.GetAllOf()) > 0 && s.Ref == nil +} + +// allOfMayRefine reports whether s's allOf may refine: no $ref, union or +// $dynamicRef stands beside it, the last two keeping it the composition it would +// otherwise be (docs/compilers/openapi.md#allof-refinement). +func allOfMayRefine(s *oas3.Schema) bool { + return allOfBesideNoRef(s) && !declaresUnion(s) && !declaresDynamicRef(s) } // allOfDecides reports whether s's allOf is what its position lowers: one the // election of competing keywords made the value, or one refining, whose rival // there, an enum or a const beside it, is one of its conjuncts. An allOf that // value set beats otherwise stays verbatim -// (docs/compilers/openapi.md#competing-keywords). +// (docs/compilers/openapi.md#competing-keywords). A union beside the composing +// allOf leaves it deciding, composed into each variant the union distributes +// across or into the Model keeping the union verbatim, and so does a $dynamicRef +// it keeps from expanding. func allOfDecides(c lowering.Ctx, s *oas3.Schema) bool { - if !allOfConjoins(s) { + if !allOfBesideNoRef(s) { return false } if dispatchOf(s).won == "allOf" { diff --git a/compilers/openapi/internal/schema/voices_test.go b/compilers/openapi/internal/schema/voices_test.go index de3ff637..56ee567f 100644 --- a/compilers/openapi/internal/schema/voices_test.go +++ b/compilers/openapi/internal/schema/voices_test.go @@ -4,8 +4,12 @@ package schema_test import ( + "slices" + "strings" "testing" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -15,15 +19,37 @@ import ( "github.com/dexpace/morphic/ir" ) -// voicesDoc is a document of Base, an object, and a component S written as written. +// voicesDoc is a document of Base, A and B, objects, Meta, an object anchored +// dynamically, Holder, an object using S through a $ref, and a component S +// written as written. func voicesDoc(written string) string { return openapitest.ComponentSpecVer("3.1.0", " Base: {type: object, properties: {id: {type: string}}}\n"+ + " A: {type: object, properties: {a: {type: string}}}\n"+ + " B: {type: object, properties: {b: {type: string}}}\n"+ + " Meta: {$dynamicAnchor: meta, type: object}\n"+ + " Holder: {type: object, properties: {s: {$ref: '#/components/schemas/S'}}}\n"+ " S: "+written+"\n") } +// besideTheAllOf is what S writes beside its allOf for each outcome the allOf +// composes at: nothing, a union it distributes across, a oneOf and an anyOf kept +// beside its Model, a union narrowing it alone +// (docs/compilers/openapi.md#union-beside-structure), and a $dynamicRef it keeps +// from expanding (docs/compilers/openapi.md#dynamic-ref). code is the report +// showing the outcome reached, drawn at S or at the keyword at names below it. +var besideTheAllOf = map[string]struct{ beside, code, at string }{ + "alone": {}, + "distributed": {", oneOf: [{$ref: '#/components/schemas/A'}, {$ref: '#/components/schemas/B'}]", diag.CompositionLowering, ""}, + "beside a degraded union": {", oneOf: [{$ref: '#/components/schemas/A'}], anyOf: [{$ref: '#/components/schemas/B'}]", + diag.DegradedConstruct, ""}, + "beside a narrowing union": {", oneOf: [{required: [a]}, {required: [b]}]", diag.ValidationOnlyKeyword, ""}, + "beside a $dynamicRef": {", $dynamicRef: '#meta'", diag.DegradedConstruct, "/$dynamicRef"}, +} + // TestVoices_AKeywordFoldsWhereTheAllOfWouldKeepIt holds a keyword on an inline -// branch to the same keyword beside the allOf: for each one the two lower to the -// same node, the branch's kept nowhere else and reported nowhere. +// branch to the same keyword beside the allOf, at each outcome besideTheAllOf +// names: for each one the two lower to the same nodes, a $ref use of S among them, +// the branch's kept nowhere else and reported nowhere but where it is written. func TestVoices_AKeywordFoldsWhereTheAllOfWouldKeepIt(t *testing.T) { t.Parallel() for name, keyword := range map[string]string{ @@ -31,6 +57,8 @@ func TestVoices_AKeywordFoldsWhereTheAllOfWouldKeepIt(t *testing.T) { "description": "description: mine", "externalDocs": "externalDocs: {url: 'https://example.com'}", "deprecated": "deprecated: true", + "readOnly": "readOnly: true", + "default": "default: {id: a}", "example": "example: {id: a}", "examples": "examples: [{id: a}]", "xml": "xml: {name: mine}", @@ -48,34 +76,145 @@ func TestVoices_AKeywordFoldsWhereTheAllOfWouldKeepIt(t *testing.T) { "propertyNames": "propertyNames: {pattern: '^a'}", "a redaction": "format: password", } { - t.Run(name, func(t *testing.T) { - t.Parallel() - ref := "{$ref: '#/components/schemas/Base'}" - beside, besideDiags := parseFull(t, voicesDoc("{allOf: ["+ref+"], "+keyword+"}")) - branch, branchDiags := parseFull(t, voicesDoc("{allOf: ["+ref+", {"+keyword+"}]}")) - assert.Empty(t, withCode(branchDiags, diag.DegradedConstruct), "nothing is left to keep or report") - assert.Len(t, branchDiags, len(besideDiags)) + for shape, tc := range besideTheAllOf { + t.Run(shape+"/"+name, func(t *testing.T) { + t.Parallel() + ref := "{$ref: '#/components/schemas/Base'}" + want, wantDiags := parseFull(t, voicesDoc("{allOf: ["+ref+"], "+keyword+tc.beside+"}")) + got, gotDiags := parseFull(t, voicesDoc("{allOf: ["+ref+", {"+keyword+"}]"+tc.beside+"}")) + if tc.code != "" { + require.True(t, openapitest.HasDiagCodeAt(gotDiags, tc.code, "/components/schemas/S"+tc.at), + "the outcome this row names; got %v", gotDiags) + } + assert.False(t, openapitest.HasDiagCodeAt(gotDiags, diag.DegradedConstruct, "/components/schemas/S/allOf/1"), + "nothing is left to keep or report; got %v", gotDiags) + assert.Equal(t, severityCodes(wantDiags), severityCodes(gotDiags)) - got, ok := typeByName(branch, "S").(*ir.Model) - require.True(t, ok) - want, ok := typeByName(beside, "S").(*ir.Model) - require.True(t, ok) - assert.Equal(t, want.Docs, got.Docs) - assert.Equal(t, want.Deprecation, got.Deprecation) - assert.Equal(t, want.Sensitive, got.Sensitive) - assert.Equal(t, want.XML, got.XML) - assert.Equal(t, want.Constraints, got.Constraints) - require.Len(t, got.Examples, len(want.Examples)) - assert.Len(t, got.Unmodeled, len(want.Unmodeled)) - for key, entry := range want.Unmodeled { - if assert.Contains(t, got.Unmodeled, key) { - assert.JSONEq(t, string(entry.Value), string(got.Unmodeled[key].Value), key) + holder := componentID("Holder") + assertNodeAlike(t, want.Types[holder], got.Types[holder]) + id := componentID("S") + assertNodeAlike(t, want.Types[id], got.Types[id]) + wantUnion, ok := want.Types[id].(*ir.Union) + if !ok { + return } - } - }) + gotUnion, ok := got.Types[id].(*ir.Union) + require.True(t, ok) + require.Len(t, gotUnion.Variants, len(wantUnion.Variants)) + for i, v := range wantUnion.Variants { + target := gotUnion.Variants[i].Type.Target + require.Equal(t, v.Type.Target, target) + assertNodeAlike(t, want.Types[target], got.Types[target]) + } + }) + } + } +} + +// assertNodeAlike holds got to want whole but for where provenance places each +// part, which differs between the two spellings: what it marks inferred and +// which document wrote it do not. +func assertNodeAlike(t *testing.T, want, got ir.TypeDef) { + t.Helper() + require.NotNil(t, want) + require.NotNil(t, got) + at := cmpopts.IgnoreFields(ir.Provenance{}, "Pointer", "Position", "Node") + if diff := cmp.Diff(want, got, at); diff != "" { + t.Errorf("node mismatch (-beside the allOf +on a branch):\n%s", diff) + } +} + +// severityCodes returns the severity and code of each of diags, sorted, so two +// runs reporting alike at other pointers compare equal. +func severityCodes(diags []ir.Diagnostic) []string { + out := make([]string, 0, len(diags)) + for _, d := range diags { + out = append(out, string(d.Severity)+" "+d.Code) + } + slices.Sort(out) + return out +} + +// TestVoices_KeptAndMetBranchesHoldAtEveryOutcome pins that what keeps a branch +// whole, or meets two, holds at each outcome besideTheAllOf names as it does +// alone (docs/compilers/openapi.md#allof-inline-residue): a branch disagreeing +// with a conjunct, or reading a $ref against another base, stays whole on each +// Model the composition lowers to, with one report saying why and nothing it +// writes folded, and two bounds meet there in either order. +func TestVoices_KeptAndMetBranchesHoldAtEveryOutcome(t *testing.T) { + t.Parallel() + const ref = "{$ref: '#/components/schemas/Base'}" + const n = "\n N: {$id: 'https://ex.test/a/n.json', type: string}" // what `n.json` names under https://ex.test/a/ + met := &ir.Constraints{MaxProps: new(int64(3))} + for name, tc := range map[string]struct { + branches string + kept []string + why string + cons *ir.Constraints + }{ + "a disagreement": {ref + ", {description: a}, {description: b}", []string{"allOf/1", "allOf/2"}, + "the conjuncts of this allOf disagree", nil}, + "a disagreement swapped": {"{description: b}, {description: a}, " + ref, []string{"allOf/0", "allOf/1"}, + "the conjuncts of this allOf disagree", nil}, + "another base": {ref + ", {$id: 'https://ex.test/a/x.json', not: {$ref: 'n.json'}, description: d}", + []string{"allOf/1"}, "reads its $refs against another base", nil}, + "bounds meeting": {ref + ", {maxProperties: 3}, {maxProperties: 5}", nil, "", met}, + "bounds meeting swapped": {ref + ", {maxProperties: 5}, {maxProperties: 3}", nil, "", met}, + } { + for shape, outcome := range besideTheAllOf { + t.Run(shape+"/"+name, func(t *testing.T) { + t.Parallel() + doc, diags := parseFull(t, voicesDoc("{allOf: ["+tc.branches+"]"+outcome.beside+"}"+n)) + openapitest.RequireNoErrorDiags(t, diags) + if outcome.code != "" { + require.True(t, openapitest.HasDiagCodeAt(diags, outcome.code, "/components/schemas/S"+outcome.at), + "the outcome this row names; got %v", diags) + } + for _, path := range tc.kept { + got := openapitest.DiagMessageAt(t, diags, diag.DegradedConstruct, ir.SeverityInfo, "/components/schemas/S/"+path) + assert.Contains(t, got, tc.why, path) + } + assert.Empty(t, typeByName(doc, "S").Common().Docs.Description, "nothing a kept branch writes folds") + for _, m := range composedModels(t, doc) { + assert.ElementsMatch(t, tc.kept, keptBranches(m), m.ID) + assert.Equal(t, tc.cons, m.Constraints, m.ID) + } + }) + } } } +// composedModels returns the Models S's composition lowers to: S itself, or each +// variant of the Union a distributed union lowers it to. +func composedModels(t *testing.T, doc *ir.Document) []*ir.Model { + t.Helper() + s := typeByName(doc, "S") + u, distributed := s.(*ir.Union) + if !distributed { + m, ok := s.(*ir.Model) + require.True(t, ok, "S lowered to a %T, want a Model or a Union", s) + return []*ir.Model{m} + } + out := make([]*ir.Model, 0, len(u.Variants)) + for _, v := range u.Variants { + m, ok := doc.Types[v.Type.Target].(*ir.Model) + require.True(t, ok, "variant %s lowered to a %T, want a Model", v.Type.Target, doc.Types[v.Type.Target]) + out = append(out, m) + } + return out +} + +// keptBranches returns the path of each allOf branch m keeps whole. +func keptBranches(m *ir.Model) []string { + var out []string + for key := range m.Unmodeled { + if path, ok := strings.CutPrefix(key, "openapi:"); ok && strings.HasPrefix(path, "allOf/") { + out = append(out, path) + } + } + return out +} + // TestVoices_ABranchDisagreeingFoldsNothingInEveryOrder pins that a keyword two // conjuncts write differently has no winner: the keywords beside the allOf stand, // and each branch writing another value stays whole, whichever comes first. @@ -247,13 +386,6 @@ func TestVoices_ADisagreementIsReportedAsOne(t *testing.T) { got = openapitest.DiagMessageAt(t, diags, diag.DegradedConstruct, ir.SeverityInfo, "/components/schemas/T/allOf/1") assert.NotContains(t, got, disagreed, "a branch that folds keeps what it declares of a shape, disagreed on by nothing") - _, diags = parseFull(t, openapitest.ComponentSpec(" Obj: {type: object}\n Other: {type: object}\n"+ - " T:\n oneOf: [{$ref: '#/components/schemas/Other'}, {type: object}]\n"+ - " allOf: [{$ref: '#/components/schemas/Obj'}, {title: t}]\n")) - for _, d := range withCode(diags, diag.DegradedConstruct) { - assert.NotContains(t, d.Message, disagreed, "a distributed branch folds nowhere, so it disagrees with nothing") - } - deep := "{description: deep, allOf: [{type: object}]}" for range schema.MaxAllOfFlattenDepth { deep = "{allOf: [" + deep + "]}" @@ -335,40 +467,32 @@ func TestVoices_UnevaluatedFalseStaysTheModels(t *testing.T) { assert.Contains(t, m.Unmodeled, "openapi:allOf/1") } -// TestVoices_ADistributedCompositionKeepsEveryBranch pins that a branch distributed -// across a union folds nothing: each variant keeps it whole, and the Union the -// position lowers to takes nothing of it. -func TestVoices_ADistributedCompositionKeepsEveryBranch(t *testing.T) { - t.Parallel() - doc, diags := parseFull(t, voicesDoc(`{allOf: [{$ref: '#/components/schemas/Base'}, {description: mine, properties: {c: {type: string}}}], - oneOf: [{$ref: '#/components/schemas/A'}, {$ref: '#/components/schemas/B'}]} - A: {type: object, properties: {a: {type: string}}} - B: {type: object, properties: {b: {type: string}}}`)) - openapitest.RequireNoErrorDiags(t, diags) - u, ok := typeByName(doc, "S").(*ir.Union) - require.True(t, ok) - assert.Empty(t, u.Docs.Description) - for _, v := range u.Variants { - assert.Contains(t, doc.Types[v.Type.Target].Common().Unmodeled, "openapi:allOf/1") - } -} - // TestVoices_AUseBindingKeywordReachesItsCarrierOrItsNode pins readOnly and default -// on a branch: a carrier holds them as beside the allOf, and with no carrier a node -// the position owns keeps them verbatim, reported at the keyword. +// on a branch: a carrier holds them as beside the allOf, a union distributed across +// it or not, and with no carrier a node the position owns keeps them verbatim, +// reported at the keyword. func TestVoices_AUseBindingKeywordReachesItsCarrierOrItsNode(t *testing.T) { t.Parallel() doc, diags := parseFull(t, openapitest.ComponentSpecVer("3.1.0", ` Base: {type: object, properties: {id: {type: string}}} + A: {type: object, properties: {a: {type: string}}} + B: {type: object, properties: {b: {type: string}}} Holder: type: object properties: f: {allOf: [{$ref: '#/components/schemas/Base'}, {readOnly: true, default: {id: a}}]} + g: + allOf: [{$ref: '#/components/schemas/Base'}, {readOnly: true, default: {id: a}}] + oneOf: [{$ref: '#/components/schemas/A'}, {$ref: '#/components/schemas/B'}] Bare: {allOf: [{$ref: '#/components/schemas/Base'}, {readOnly: true}, {readOnly: true}], writeOnly: false} `)) openapitest.RequireNoErrorDiags(t, diags) - f := propertyOf(t, doc, "Holder", "f") - assert.NotEmpty(t, f.Visibility.Only) - assert.NotNil(t, f.Default) + for _, name := range []string{"f", "g"} { + p := propertyOf(t, doc, "Holder", name) + assert.NotEmpty(t, p.Visibility.Only, name) + assert.NotNil(t, p.Default, name) + assert.Empty(t, doc.Types[p.Type.Target].Common().Unmodeled, name) + } + assert.True(t, openapitest.HasDiagCodeAt(diags, diag.CompositionLowering, "/components/schemas/Holder/properties/g")) bare, ok := typeByName(doc, "Bare").(*ir.Model) require.True(t, ok) entry, ok := bare.Unmodeled["openapi:readOnly"] @@ -417,16 +541,3 @@ func TestVoices_ARefBranchIsNoVoice(t *testing.T) { require.NotNil(t, m.Base) assert.Equal(t, "beside", doc.Types[m.Base.Target].Common().Docs.Description) } - -// TestVoices_AUnionBesideKeepsEveryBranch pins an allOf beside a union kept verbatim: -// its Model is no use a branch can speak for, so each branch stays whole. -func TestVoices_AUnionBesideKeepsEveryBranch(t *testing.T) { - t.Parallel() - doc, diags := parseFull(t, voicesDoc(`{allOf: [{$ref: '#/components/schemas/Base'}, {description: mine}], - oneOf: [{required: [a]}, {required: [b]}]}`)) - openapitest.RequireNoErrorDiags(t, diags) - m, ok := typeByName(doc, "S").(*ir.Model) - require.True(t, ok) - assert.Empty(t, m.Docs.Description) - assert.Contains(t, m.Unmodeled, "openapi:allOf/1") -} diff --git a/docs/compilers/openapi.md b/docs/compilers/openapi.md index 37fc983b..c517e0a2 100644 --- a/docs/compilers/openapi.md +++ b/docs/compilers/openapi.md @@ -1454,9 +1454,15 @@ fold, being a composing model's openness; a bound does not where the position ha as one beside the `allOf` reaches none ([`constraints`](#constraints)): a value bound on a node with no `Constraints` for one, and a collection bound on any node but a `List` or a `Tuple`, the `Union` of a type list holding one where such a variant does; a collection bound `Constraints` cannot state, -a `uniqueItems` of `false` or a count past 64 bits, never folds; and a composition distributed -across a union keeps every branch on each variant -([`union-beside-structure`](#union-beside-structure)). +a `uniqueItems` of `false` or a count past 64 bits, never folds. A `oneOf`, an `anyOf` or a +`$dynamicRef` beside the `allOf` keeps it from refining ([`allof-refinement`](#allof-refinement)) +and changes nothing else here: where the `allOf` composes, its branches fold into the composition +as they do with none of them there, beside a `$dynamicRef` it keeps from expanding +([`dynamic-ref`](#dynamic-ref)) or a union kept verbatim +([`union-beside-structure`](#union-beside-structure)). Where the union distributes, a branch's +annotations and what a use binds reach the `Union` and its carriers, and a value bound each variant, +as the same keyword beside the `allOf` does, and a branch with no residue is kept on no variant +([models](../ir/models.md#composition-beside-a-union)). Folding needs no precedence, because a branch that disagrees with any conjunct on one of these, the keywords beside the `allOf` among them, folds nothing, in every order: the keywords beside the @@ -1515,7 +1521,9 @@ every entry reads that one twice. `schema.TestVoices_BoundsMeetInEveryOrder`, `schema.TestVoices_KeywordsOneEntryHoldsDisagreeAsOne`, `schema.TestVoices_UnevaluatedFalseStaysTheModels`, - `schema.TestVoices_ADistributedCompositionKeepsEveryBranch`, + `schema.TestVoices_AUseBindingKeywordReachesItsCarrierOrItsNode`, + `schema.TestVoices_KeptAndMetBranchesHoldAtEveryOutcome`, + `schema.TestAllOf_BranchResidueRidesEveryDistributedVariant`, `schema.TestAllOf_ABranchReadAgainstAnotherBaseFoldsNothing`, `schema.TestAllOf_ABranchWritingNoRelativeReferenceFoldsUnderAnyBase`, `schema.TestRefinement_ABranchReadAgainstAnotherBaseFoldsNothing`, diff --git a/docs/ir/models.md b/docs/ir/models.md index 95e6adc3..47c7bc70 100644 --- a/docs/ir/models.md +++ b/docs/ir/models.md @@ -286,6 +286,14 @@ one or dropped, so nothing degrades and no `allOf` branch is flattened. A proper appears on each variant under its single source-derived `PropID`, since the source declares it once. +What `S` says of the position rather than of its shape, its docs, deprecation, examples and what a +use binds, is said once, on the `Union` that stands there and the carriers using it, and a value +bound, which constrains both sides of the disjunction, on each variant. That holds whether `S` +writes it beside the `allOf` or on an inline branch of it, which conjoins the same way. Keeping such +a branch whole on each variant instead would let the source be rebuilt from any one variant, but it +would copy one fact onto as many nodes as there are variants, none of them where a reader of the +position looks, and tell apart two spellings that mean the same. + The variant models are anonymous and **synthesized**: no schema in the source occupies them, so each takes an ID in a namespace of its own, derived from — but not equal to — the ID the branch pointer would yield. Provenance still points at the branch, the nearest thing the source wrote. diff --git a/docs/ir/version-history.md b/docs/ir/version-history.md index 4be04176..979b39af 100644 --- a/docs/ir/version-history.md +++ b/docs/ir/version-history.md @@ -377,3 +377,14 @@ it sees. Changes made together in one bump are listed together under it. source's index with the overlay as its writer, where `Source` had named the overlay at a position of another document. A consumer pinned to 0.36.0 reading such a pointer against the overlay's file reads it at the wrong place. +- **0.38.0** — no shape change: an inline branch of an `allOf` beside a `oneOf`, an `anyOf` or a + `$dynamicRef` folds what it writes for the position into the composition, as a composing `allOf`'s + branch does with none of them there, where the OpenAPI compiler had kept it whole under + `Unmodeled["openapi:allOf/"]` + ([`allof-inline-residue`](../compilers/openapi.md#allof-inline-residue)). Where a union + distributes ([models](./models.md#composition-beside-a-union)), the branch's docs, deprecation, + examples, secrecy and XML hints reach the `Union`, what a use binds its carriers, and a value + bound each composed variant, and a branch with no residue left is kept on no variant; beside a + union or a `$dynamicRef` kept verbatim they reach the `Model`, as in a composition with neither + beside it. A consumer pinned to 0.37.0 that read the branch under that key no longer finds it, and + refuses the document as another generation's. diff --git a/ir/document.go b/ir/document.go index 2246d749..6db9566e 100644 --- a/ir/document.go +++ b/ir/document.go @@ -12,7 +12,7 @@ package ir // docs/ir/ir-version.md holds that policy, docs/ir/version-history.md what each // bump changed, and TestIRVersion_PinsTheEncodedShape and // TestIRVersion_PinsTheNames hold it to the shape and the names. -const IRVersion = "0.37.0" +const IRVersion = "0.38.0" // CompatibleVersion reports whether a document stamped version can be read by // this build: exact equality with IRVersion, per docs/ir/ir-version.md. A diff --git a/ir/irversion_test.go b/ir/irversion_test.go index 23cf6db5..fe74e7be 100644 --- a/ir/irversion_test.go +++ b/ir/irversion_test.go @@ -93,6 +93,9 @@ var generations = []generation{ names: "5506159ecd463736fdbbad0c4cabfad3fd50b48509b46fbea55862e82eca9675"}, {version: "0.37.0", format: 1, shape: "371d8ded3a52ab677a8318b202ab570ce495fb8c335b703477b00643a473275a", names: "5506159ecd463736fdbbad0c4cabfad3fd50b48509b46fbea55862e82eca9675"}, + {version: "0.38.0", format: 1, shape: "371d8ded3a52ab677a8318b202ab570ce495fb8c335b703477b00643a473275a", + names: "5506159ecd463736fdbbad0c4cabfad3fd50b48509b46fbea55862e82eca9675", + meaning: "an inline branch of an allOf beside a oneOf, an anyOf or a $dynamicRef folds into the composition: what it says of the position reaches the Union a distributed union lowers to and its carriers, and a value bound each variant"}, } // shapeGolden holds the description the newest row's shape hashes, so a change diff --git a/testdata/conformance/openapi/allof-inline-residue.golden.json b/testdata/conformance/openapi/allof-inline-residue.golden.json index c5b6a4ea..13d9be62 100644 --- a/testdata/conformance/openapi/allof-inline-residue.golden.json +++ b/testdata/conformance/openapi/allof-inline-residue.golden.json @@ -58,6 +58,126 @@ } ], "types": { + "t/composed/components/schemas/Distributed/oneOf/0": { + "kind": "model", + "id": "t/composed/components/schemas/Distributed/oneOf/0", + "name": { + "hint": "distributed_feline" + }, + "anonymous": true, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Distributed/oneOf/0" + }, + "properties": [ + { + "id": "p/openapi/components/schemas/Distributed/allOf/1/properties/name", + "name": { + "source": "name", + "canonical": "name" + }, + "wireName": "name", + "type": { + "target": "t/prim/string", + "nullable": false + }, + "required": false, + "clientOptional": false, + "defaultAdded": false, + "visibility": { + "none": false + }, + "flatten": false, + "eventHeader": false, + "eventPayload": false, + "secret": false, + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Distributed/allOf/1/properties/name" + } + } + ], + "base": { + "target": "t/openapi/components/schemas/Base", + "nullable": false + }, + "mixins": [ + { + "target": "t/openapi/components/schemas/Feline", + "nullable": false + } + ], + "constraints": { + "uniqueItems": false, + "maxProps": 4 + }, + "abstract": false, + "positional": false, + "inputOnly": false + }, + "t/composed/components/schemas/Distributed/oneOf/1": { + "kind": "model", + "id": "t/composed/components/schemas/Distributed/oneOf/1", + "name": { + "hint": "distributed_canine" + }, + "anonymous": true, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Distributed/oneOf/1" + }, + "properties": [ + { + "id": "p/openapi/components/schemas/Distributed/allOf/1/properties/name", + "name": { + "source": "name", + "canonical": "name" + }, + "wireName": "name", + "type": { + "target": "t/prim/string", + "nullable": false + }, + "required": false, + "clientOptional": false, + "defaultAdded": false, + "visibility": { + "none": false + }, + "flatten": false, + "eventHeader": false, + "eventPayload": false, + "secret": false, + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Distributed/allOf/1/properties/name" + } + } + ], + "base": { + "target": "t/openapi/components/schemas/Base", + "nullable": false + }, + "mixins": [ + { + "target": "t/openapi/components/schemas/Canine", + "nullable": false + } + ], + "constraints": { + "uniqueItems": false, + "maxProps": 4 + }, + "abstract": false, + "positional": false, + "inputOnly": false + }, "t/openapi/components/schemas/Base": { "kind": "model", "id": "t/openapi/components/schemas/Base", @@ -105,6 +225,53 @@ "positional": false, "inputOnly": false }, + "t/openapi/components/schemas/Canine": { + "kind": "model", + "id": "t/openapi/components/schemas/Canine", + "name": { + "source": "Canine", + "canonical": "canine" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Canine" + }, + "properties": [ + { + "id": "p/openapi/components/schemas/Canine/properties/barks", + "name": { + "source": "barks", + "canonical": "barks" + }, + "wireName": "barks", + "type": { + "target": "t/prim/bool", + "nullable": false + }, + "required": false, + "clientOptional": false, + "defaultAdded": false, + "visibility": { + "none": false + }, + "flatten": false, + "eventHeader": false, + "eventPayload": false, + "secret": false, + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Canine/properties/barks" + } + } + ], + "abstract": false, + "positional": false, + "inputOnly": false + }, "t/openapi/components/schemas/Contradicted": { "kind": "model", "id": "t/openapi/components/schemas/Contradicted", @@ -144,6 +311,96 @@ "positional": false, "inputOnly": false }, + "t/openapi/components/schemas/Distributed": { + "kind": "union", + "id": "t/openapi/components/schemas/Distributed", + "name": { + "source": "Distributed", + "canonical": "distributed" + }, + "anonymous": false, + "docs": { + "description": "A pet of either kind." + }, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Distributed" + }, + "variants": [ + { + "name": { + "hint": "feline" + }, + "type": { + "target": "t/composed/components/schemas/Distributed/oneOf/0", + "nullable": false + }, + "docs": {} + }, + { + "name": { + "hint": "canine" + }, + "type": { + "target": "t/composed/components/schemas/Distributed/oneOf/1", + "nullable": false + }, + "docs": {} + } + ], + "exclusive": true, + "wireTagged": false, + "closed": true, + "inputOnly": false + }, + "t/openapi/components/schemas/Feline": { + "kind": "model", + "id": "t/openapi/components/schemas/Feline", + "name": { + "source": "Feline", + "canonical": "feline" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Feline" + }, + "properties": [ + { + "id": "p/openapi/components/schemas/Feline/properties/purrs", + "name": { + "source": "purrs", + "canonical": "purrs" + }, + "wireName": "purrs", + "type": { + "target": "t/prim/bool", + "nullable": false + }, + "required": false, + "clientOptional": false, + "defaultAdded": false, + "visibility": { + "none": false + }, + "flatten": false, + "eventHeader": false, + "eventPayload": false, + "secret": false, + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Feline/properties/purrs" + } + } + ], + "abstract": false, + "positional": false, + "inputOnly": false + }, "t/openapi/components/schemas/ItemA": { "kind": "scalar", "id": "t/openapi/components/schemas/ItemA", @@ -395,6 +652,18 @@ "positional": false, "inputOnly": false }, + "t/prim/bool": { + "kind": "primitive", + "id": "t/prim/bool", + "name": {}, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": -1 + }, + "prim": "bool" + }, "t/prim/integer": { "kind": "primitive", "id": "t/prim/integer", @@ -452,6 +721,15 @@ "pointer": "/components/schemas/Contradicted/allOf/1" } }, + { + "severity": "info", + "code": "openapi/composition-lowering", + "message": "oneOf/anyOf co-declared with structural keywords; the composition is distributed across the union variants", + "provenance": { + "source": 0, + "pointer": "/components/schemas/Distributed" + } + }, { "severity": "info", "code": "openapi/degraded-construct", @@ -511,7 +789,7 @@ { "format": "openapi@3.1", "path": "allof-inline-residue.yaml", - "hash": "b6cea8de9f6660e43b6d6cfee543a954e832f9c0c800dcb527922e908bea49bd" + "hash": "1ab5e69bcc867baf4cf76bf738b90d538c76ca8e53eb752ffe8028462fae9987" } ] } diff --git a/testdata/conformance/openapi/allof-inline-residue.yaml b/testdata/conformance/openapi/allof-inline-residue.yaml index 030f16ac..c208dbb7 100644 --- a/testdata/conformance/openapi/allof-inline-residue.yaml +++ b/testdata/conformance/openapi/allof-inline-residue.yaml @@ -32,6 +32,21 @@ components: allOf: - {$ref: '#/components/schemas/Untyped'} - {type: string, maxLength: 3} + # Beside a union the branch folds as it does alone: the Union the + # distribution lowers to takes its description, each composed variant its + # bound, and no variant keeps the branch. + Feline: {type: object, properties: {purrs: {type: boolean}}} + Canine: {type: object, properties: {barks: {type: boolean}}} + Distributed: + allOf: + - {$ref: '#/components/schemas/Base'} + - properties: + name: {type: string} + description: A pet of either kind. + maxProperties: 4 + oneOf: + - {$ref: '#/components/schemas/Feline'} + - {$ref: '#/components/schemas/Canine'} # Under the $id beside its allOf, `item.json` names ItemB; under the # nested wrapper's, ItemA. The nested branch's `not` would name ItemB once # folded here, so it folds nothing and is kept whole, while the outer