diff --git a/compilers/openapi/conformance_test.go b/compilers/openapi/conformance_test.go index 17ae7d9..8d07828 100644 --- a/compilers/openapi/conformance_test.go +++ b/compilers/openapi/conformance_test.go @@ -641,6 +641,9 @@ func assertAllOfRequiredOnly(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) // assertAllOfOneOfCooccurrence pins both halves of the co-declared composition // rule: §4.3 distributes a union whose branches all name referents, and §4.8 // keeps one with an inline branch verbatim rather than distributing it halfway. +// The verbatim half is covered over a model body, which owns a node already, and +// over a scalar one, which does not — there the alias hoisted for the union +// carries what the position wrote beside it too. // The outside reference to a branch pointer pins the third thing: a composed // variant is Morphic's own node, so it cannot be taken by, or take from, a $ref. func assertAllOfOneOfCooccurrence(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { @@ -662,6 +665,14 @@ func assertAllOfOneOfCooccurrence(t *testing.T, doc *ir.Document, _ []ir.Diagnos require.True(t, ok, "and the union it could not absorb survives beside it") assert.Equal(t, ir.ReasonDegradedLowering, entry.Reason) + bounded, ok := doc.Types[namedID("BoundedKinds")].(*ir.Scalar) + require.True(t, ok, "a body that is not a model reduces to a shared primitive and hoists an alias") + _, ok = bounded.Unmodeled["openapi:oneOf"] + require.True(t, ok, "which is the node the kept union sits on") + require.NotNil(t, bounded.Constraints, "and the bounds written beside the union sit on it too") + require.NotNil(t, bounded.Constraints.MinLength) + assert.Equal(t, int64(3), *bounded.Constraints.MinLength) + outsider, ok := doc.Types[namedID("Outsider")].(*ir.Model) require.True(t, ok) require.Len(t, outsider.Properties, 1) diff --git a/compilers/openapi/internal/schema/compose_test.go b/compilers/openapi/internal/schema/compose_test.go index a44e5d0..ee48612 100644 --- a/compilers/openapi/internal/schema/compose_test.go +++ b/compilers/openapi/internal/schema/compose_test.go @@ -1959,6 +1959,72 @@ func TestOneOf_CoDeclaredNotDistributedReasons(t *testing.T) { "each declined shape is reported once; got %+v", diags) } +// TestOneOf_CoDeclaredKeepsTheBoundsWrittenBesideIt pins that keeping a union +// verbatim does not cost the position the value constraints written beside it. +// The alias exists so the union attaches to a node this pointer owns rather than +// to the shared primitive the body reduced to, and owning a node is what stops +// hoistDeclarationHome hoisting the alias that would otherwise carry the bounds +// — so this alias has to carry them itself, as every other hoist here does +// (GitHub #343). +// +// Both routes to the verbatim lowering hoist the same alias, so both are covered: +// the bounds went the same way whichever reason kept the union. The third case +// covers what reading them also produces — bounds nothing reads are bounds +// nothing can report on, so the co-declared-bound reconciliation reached no such +// position until now. +func TestOneOf_CoDeclaredKeepsTheBoundsWrittenBesideIt(t *testing.T) { + t.Parallel() + three := int64(3) + ten, five := ir.BigVal("10"), ir.BigVal("5") + cases := []struct { + name, schemas string + reason ir.UnmodeledReason + want ir.Constraints + wantDiag string + }{ + { + name: "a validation-only union", + schemas: " A: {type: string, minLength: 3, oneOf: [{minLength: 1}, {minLength: 2}]}\n", + reason: ir.ReasonValidationOnly, + want: ir.Constraints{MinLength: &three}, + }, + { + name: "a union kept as a degraded lowering", + schemas: " A: {type: number, minimum: 10, multipleOf: 5, oneOf: [{type: string}, {type: integer}]}\n", + reason: ir.ReasonDegradedLowering, + want: ir.Constraints{Min: &ten, MultipleOf: &five}, + }, + { + name: "co-declared bounds beside a union", + schemas: " A: {type: number, minimum: 10, exclusiveMinimum: 0, oneOf: [{minLength: 1}, {minLength: 2}]}\n", + reason: ir.ReasonValidationOnly, + want: ir.Constraints{Min: &ten}, + wantDiag: "kept minimum as the tighter of the two", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + doc, diags := lowerSpec(t, componentSpec(tc.schemas)) + requireNoErrorDiags(t, diags) + + sc, ok := typeByName(doc, "A").(*ir.Scalar) + require.True(t, ok, "the preserved union hoists an alias over the shared primitive") + entry, ok := sc.Unmodeled["openapi:oneOf"] + require.True(t, ok, "and keeps the union on it") + assert.Equal(t, tc.reason, entry.Reason) + require.NotNil(t, sc.Constraints, "while keeping the bounds written beside it") + assert.Empty(t, cmp.Diff(tc.want, *sc.Constraints)) + if tc.wantDiag == "" { + return + } + assert.Contains(t, + diagMessageAt(t, diags, diag.DegradedConstruct, ir.SeverityInfo, "/components/schemas/A"), + tc.wantDiag, "reading the bounds is what reports on them") + }) + } +} + // TestUnionCombinators_PassedOverBranchSetIsKept covers the preference nothing // used to record (GitHub #35). unionBranches takes oneOf whenever it is written // and falls back to anyOf only when it is not, so a schema declaring both lost diff --git a/compilers/openapi/internal/schema/schema.go b/compilers/openapi/internal/schema/schema.go index d06d1eb..a44918e 100644 --- a/compilers/openapi/internal/schema/schema.go +++ b/compilers/openapi/internal/schema/schema.go @@ -380,8 +380,12 @@ func lowerBesideUnmodeledUnion(c lowering.Ctx, ts *compile.Types, anchors *Ancho if got, _ := ts.Lookup(pointer); got != inner { // The structural body reduced to a shared/aliased target; hoist an alias // so the preserved union attaches to a node this pointer owns, never to a - // shared primitive. - owner = internAlias(c, ts, pointer, hint, ir.TypeRef{Target: inner}, nil) + // shared primitive. The alias carries the position's value constraints for + // the reason hoistByteScalar records: owning the node is what stops + // hoistDeclarationHome hoisting the alias that would otherwise carry them. + cons, consDiags := schemaConstraints(c, s, pointer) + diags = append(diags, consDiags...) + owner = internAlias(c, ts, pointer, hint, ir.TypeRef{Target: inner}, cons) } return owner, append(diags, preserveUnionSiblings(c, ts, owner, s, pointer, reason, why)...) } diff --git a/testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json b/testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json index c43e993..80c2b91 100644 --- a/testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json +++ b/testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json @@ -251,6 +251,48 @@ "positional": false, "inputOnly": false }, + "t/openapi/components/schemas/BoundedKinds": { + "kind": "scalar", + "id": "t/openapi/components/schemas/BoundedKinds", + "name": { + "source": "BoundedKinds", + "canonical": "bounded_kinds" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "unmodeled": { + "openapi:oneOf": { + "reason": "degraded_lowering", + "value": [ + { + "$ref": "#/components/schemas/A" + }, + { + "type": "string" + } + ], + "provenance": { + "source": 0, + "pointer": "/components/schemas/BoundedKinds/oneOf" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/components/schemas/BoundedKinds" + }, + "base": { + "target": "t/prim/string", + "nullable": false + }, + "constraints": { + "exclusiveMin": false, + "exclusiveMax": false, + "minLength": 3, + "uniqueItems": false + } + }, "t/openapi/components/schemas/Combo": { "kind": "union", "id": "t/openapi/components/schemas/Combo", @@ -542,13 +584,22 @@ "source": 0, "pointer": "/components/schemas/MixedKinds" } + }, + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "oneOf/anyOf co-declared with structural keywords intersects with them, and the body is not a model, so it carries no composition to distribute into; union branches kept verbatim under Unmodeled", + "provenance": { + "source": 0, + "pointer": "/components/schemas/BoundedKinds" + } } ], "sources": [ { "format": "openapi@3.1", "path": "allof-oneof-cooccurrence.yaml", - "hash": "cb7fb6bc613222b0211fc363b9028e7739b44a1d14b04f74cf709b8d603655f2" + "hash": "4befdad1ab433d53b430993675f127b586c7ec0bfb765dca31c66d5c819b4c10" } ] } diff --git a/testdata/conformance/openapi/allof-oneof-cooccurrence.yaml b/testdata/conformance/openapi/allof-oneof-cooccurrence.yaml index f085da9..22b77c2 100644 --- a/testdata/conformance/openapi/allof-oneof-cooccurrence.yaml +++ b/testdata/conformance/openapi/allof-oneof-cooccurrence.yaml @@ -32,6 +32,17 @@ components: oneOf: - {$ref: '#/components/schemas/A'} - {type: string} + # The same conjunction over a body that is not a model. It reduces to the + # shared string primitive, so the position hoists an alias for the kept union + # to sit on — and that alias has to carry the bounds written here too, since + # owning a node is what stops the declaration-home fallback carrying them + # (GitHub #343). + BoundedKinds: + type: string + minLength: 3 + oneOf: + - {$ref: '#/components/schemas/A'} + - {type: string} # A branch pointer denotes the branch schema, and a reference to it must get # that schema — not the variant Morphic composes for the same branch, which # is a node of its own with no pointer a $ref can name.