diff --git a/compilers/openapi/conformance_test.go b/compilers/openapi/conformance_test.go index a43d694..baf978b 100644 --- a/compilers/openapi/conformance_test.go +++ b/compilers/openapi/conformance_test.go @@ -759,6 +759,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) { @@ -780,6 +783,15 @@ 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") + entry, ok = bounded.Unmodeled["openapi:oneOf"] + require.True(t, ok, "which is the node the kept union sits on") + assert.Equal(t, ir.ReasonDegradedLowering, entry.Reason) + 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 d43e629..b0da6c8 100644 --- a/compilers/openapi/internal/schema/compose_test.go +++ b/compilers/openapi/internal/schema/compose_test.go @@ -2330,6 +2330,90 @@ func TestOneOf_CoDeclaredNotDistributedReasons(t *testing.T) { "each declined shape is reported once; got %+v", diags) } +// TestUnionCombinators_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 reasons that keep a union hoist the same alias, and anyOf rides the same +// path as oneOf, so each is covered. The last case pins what reading the bounds +// also produces: the co-declared-bound reconciliation reports at a position +// nothing used to read, and the keyword it cannot home is kept on the node the +// bounds landed on. The key set is asserted whole — carrying the constraints +// without that keyword leaves the diagnostic naming an entry the node lacks. +func TestUnionCombinators_CoDeclaredKeepsTheBoundsWrittenBesideIt(t *testing.T) { + t.Parallel() + three := int64(3) + ten, five := ir.BigVal("10"), ir.BigVal("5") + cases := []struct { + name, schemas, unionKey string + reason ir.UnmodeledReason + want ir.Constraints + wantKept []string + wantDiag string + }{ + { + name: "a validation-only union", + schemas: " A: {type: string, minLength: 3, oneOf: [{minLength: 1}, {minLength: 2}]}\n", + unionKey: "openapi:oneOf", + reason: ir.ReasonValidationOnly, + want: ir.Constraints{MinLength: &three}, + wantKept: []string{"openapi:oneOf"}, + }, + { + name: "a union kept as a degraded lowering", + schemas: " A: {type: number, minimum: 10, multipleOf: 5, oneOf: [{type: string}, {type: integer}]}\n", + unionKey: "openapi:oneOf", + reason: ir.ReasonDegradedLowering, + want: ir.Constraints{Min: &ten, MultipleOf: &five}, + wantKept: []string{"openapi:oneOf"}, + }, + { + name: "an anyOf kept in place of a oneOf", + schemas: " A: {type: string, minLength: 3, anyOf: [{minLength: 1}, {minLength: 2}]}\n", + unionKey: "openapi:anyOf", + reason: ir.ReasonValidationOnly, + want: ir.Constraints{MinLength: &three}, + wantKept: []string{"openapi:anyOf"}, + }, + { + name: "co-declared bounds beside a union", + schemas: " A: {type: number, minimum: 10, exclusiveMinimum: 0, oneOf: [{minLength: 1}, {minLength: 2}]}\n", + unionKey: "openapi:oneOf", + reason: ir.ReasonValidationOnly, + want: ir.Constraints{Min: &ten}, + wantKept: []string{"openapi:exclusiveMinimum", "openapi:oneOf"}, + 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, openapitest.ComponentSpec(tc.schemas)) + openapitest.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[tc.unionKey] + 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)) + assert.Equal(t, tc.wantKept, unmodeledKeys(sc.Unmodeled), + "the bound keyword that reaches no Constraints field is kept on the same node") + if tc.wantDiag == "" { + return + } + assert.Contains(t, + openapitest.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 97ddafb..e1d1c8c 100644 --- a/compilers/openapi/internal/schema/schema.go +++ b/compilers/openapi/internal/schema/schema.go @@ -428,13 +428,15 @@ 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. - // - // Alone among the alias hoists this one reads no constraints, so the - // position's bounds — and with them the co-declared keyword kept beside - // them — reach no field here. That is GitHub #343, deliberately left as - // it was rather than settled as a side effect of the keyword's own fix. - owner = internAlias(c, ts, pointer, hint, ir.TypeRef{Target: inner}, nil, 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. + // kept travels with them, so the co-declared bound keyword that reaches no + // Constraints field lands on the same node as the bounds it lost to. + var kept ir.Unmodeled + cons, consDiags := schemaConstraints(c, &kept, s, pointer) + diags = append(diags, consDiags...) + owner = internAlias(c, ts, pointer, hint, ir.TypeRef{Target: inner}, cons, kept) } 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 4a1d4d5..cbc6758 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": "580c2ffd45b8b24e175b4c5b7f1af6c237a920de7d1942d84b10a00ad2dc1e33" + "hash": "9deaf845e7a310c701113bd61e60a7e0d170cbf96860921816623667813e458f" } ] } diff --git a/testdata/conformance/openapi/allof-oneof-cooccurrence.yaml b/testdata/conformance/openapi/allof-oneof-cooccurrence.yaml index 042727d..7413018 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.