From 4091639b4944e479038375225014cb6adda23796 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Sun, 9 Aug 2026 06:32:05 +0300 Subject: [PATCH 1/4] fix(compilers/openapi): read $id past an empty pointer segment declaresResourceIDAbove walks a JSON Pointer from the document root looking for the $id that starts a schema resource of its own, and skipped every empty segment on the way. Only the leading one is an artifact of splitting on '/': every later one is a real RFC 6901 reference token naming the key "", which is how a component schema named "" is addressed (/components/schemas/). A pointer ending in one therefore stopped the walk at the parent map and never read the position's own $id. dynamicChainVerdict then let a $dynamicRef expansion cross a schema resource boundary it should have degraded at, which is the worse of the two directions to err in: a missed boundary mints a reference the IR cannot express, where a false one only costs an expansion that would have been safe. Split the pointer through pointerTokens, which drops the leading empty segment and nothing else, so every remaining token takes a step of its own. The empty pointer and any string without a leading '/' yield no tokens at all, leaving the walk reading the document root as before. --- compilers/openapi/internal/schema/schema.go | 23 +++++++++--- .../internal/schema/schema_internal_test.go | 36 +++++++++++++++++++ .../openapi/internal/schema/schema_test.go | 11 ++++++ 3 files changed, 66 insertions(+), 4 deletions(-) diff --git a/compilers/openapi/internal/schema/schema.go b/compilers/openapi/internal/schema/schema.go index d06d1eb..cd594ea 100644 --- a/compilers/openapi/internal/schema/schema.go +++ b/compilers/openapi/internal/schema/schema.go @@ -1443,20 +1443,35 @@ func componentSchemaAt(c lowering.Ctx, pointer string) *oas3.Schema { func declaresResourceIDAbove(c lowering.Ctx, pointer string) bool { view := nodeview.New() cur := nodeview.DocumentRoot(nodeview.Deref(c.Doc.GetRootNode())) - for seg := range strings.SplitSeq(pointer, "/") { + for _, token := range pointerTokens(pointer) { if cur == nil { return false } if view.ChildByToken(cur, "$id") != nil { return true } - if seg != "" { // every pointer starts with the empty segment - cur = nodeview.Deref(view.ChildByToken(cur, ids.UnescapeSegment(seg))) - } + cur = nodeview.Deref(view.ChildByToken(cur, ids.UnescapeSegment(token))) } return cur != nil && view.ChildByToken(cur, "$id") != nil } +// pointerTokens returns the RFC 6901 reference tokens of pointer, still escaped. +// Splitting on '/' yields one leading empty segment that is the split's artifact +// rather than a token, and only that one is: a later empty segment names the key +// "", which is how a component schema named "" is addressed +// (/components/schemas/). Dropping every empty segment stopped the walk above +// such a position and read the $id of its parent instead of its own. +// +// A string with no leading '/' has no tokens at all: the empty pointer names the +// whole document, and a relative pointer names no position in it. +func pointerTokens(pointer string) []string { + rest, found := strings.CutPrefix(pointer, "/") + if !found { + return nil + } + return strings.Split(rest, "/") +} + // dynamicFragment returns the plain fragment name a $dynamicRef addresses. Only // the same-document `#name` spelling resolves here: a URI part names another // resource (Milestone 1 interns only same-file targets) and a `#/…` pointer diff --git a/compilers/openapi/internal/schema/schema_internal_test.go b/compilers/openapi/internal/schema/schema_internal_test.go index e1b560a..7594173 100644 --- a/compilers/openapi/internal/schema/schema_internal_test.go +++ b/compilers/openapi/internal/schema/schema_internal_test.go @@ -212,6 +212,42 @@ func TestDeclaresResourceIDAbove_WithoutARawTree(t *testing.T) { "a document with no raw tree declares no resource anywhere") } +// TestDeclaresResourceIDAbove_EmptySegmentIsATokenNotAnArtifact pins which empty +// segments the path walk may drop. Splitting a pointer on '/' produces one +// leading empty segment that no reference token stands behind; every later one +// is the token naming the key "", which is how a schema literally named "" is +// addressed. Dropping those stopped the walk above the position and read the +// $id of the components/schemas map instead of the schema's own. +func TestDeclaresResourceIDAbove_EmptySegmentIsATokenNotAnArtifact(t *testing.T) { + t.Parallel() + l, diags := loweredFor(t, `openapi: 3.1.0 +info: {title: T, version: "1"} +paths: {} +components: + schemas: + "": + $id: https://example.com/empty + properties: {x: {type: string}} + Sibling: {type: string} +`) + requireNoErrorDiags(t, diags) + + for name, tc := range map[string]struct { + pointer string + want bool + }{ + "the position is named by a trailing empty token": {"/components/schemas/", true}, + "an interior empty token still descends": {"/components/schemas//properties/x", true}, + "a sibling sits above no $id at all": {"/components/schemas/Sibling", false}, + "the empty pointer names the document root": {"", false}, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tc.want, declaresResourceIDAbove(l.ctx, tc.pointer)) + }) + } +} + // TestDynamicAnchors_WalksEveryNodeShape drives the raw-tree walk over the // shapes a YAML document can present, rather than only the mappings a schema // happens to be written as. The walk reads the raw tree because oas3.Schema has diff --git a/compilers/openapi/internal/schema/schema_test.go b/compilers/openapi/internal/schema/schema_test.go index f4b1c91..d05980f 100644 --- a/compilers/openapi/internal/schema/schema_test.go +++ b/compilers/openapi/internal/schema/schema_test.go @@ -3021,6 +3021,17 @@ func TestDynamicRef_IrreducibleIsKeptAndSaysWhy(t *testing.T) { schemas: anchor + " A: {$id: 'https://example.com/a', $dynamicRef: '#m'}\n", wantWhy: `an $id at or above "/components/schemas/A"`, }, + { + // The pointer of a schema named "" ends in an empty reference token, + // which the path walk once dropped as if it were the artifact of + // splitting a pointer on '/' — reading the components/schemas map for + // the $id instead of the schema itself, and expanding across a + // boundary it should have stopped at. + name: `a schema named "" is still in a resource of its own`, + schemas: anchor + " \"\": {$id: 'https://example.com/empty', $dynamicRef: '#m'}\n", + wantWhy: `an $id at or above "/components/schemas/"`, + at: "t/anon/components/schemas/", + }, { name: "an enclosing schema starts the resource", schemas: anchor + From fe7ccf50ffc0aeca2c47ac53ee49d9d122903baa Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 11 Aug 2026 23:43:00 +0300 Subject: [PATCH 2/4] refactor(compilers/openapi): reuse PointerPath for the $id walk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The resource-boundary walk grew a pointer tokenizer of its own, which duplicated nodeview.PointerPath — already fixed for this same empty-token bug class in #304, in the layer below, a month earlier. Reusing it drops the second tokenizer and with it three defects the copy carried: it read a lone "/" as one empty token where the sibling walk reads it as the root, it lost every boundary above a pointer with no leading "/", and its loop had no explicit bound where PointerPath counts against maxPointerSegments. The walk also built a nodeview.View per call, discarding the expansion memo the type exists to hold. The view now lives on AnchorIndex beside the anchor memo, so one document builds one. Compiling merge-heavy specs timed the same either way and the IR is byte-identical; what changes is that the memo is reachable at all. Sweeping the same empty-name mechanism turned up a second site the original sweep missed. ids.ComponentEntry rejects an empty name, which is deliberate — an entry keyed "" earns no named TypeID, as testdata/conformance/openapi/empty-names.yaml records — but the $dynamicAnchor path reported it as "declared at /components/schemas/ rather than on a component schema", which the document contradicts: that pointer addresses a component schema. The verdict is unchanged; the reason now says an empty name earns no named type to expand to. Coverage: no committed spec combined an empty component name with $id and $dynamicRef, so no oracle drove the fixed path. dynamic-ref.yaml now writes that combination, and reverting the fix reddens the conformance case. The empty-pointer unit case asserted nothing before — both readings produced false — so the fixture now parks an $id under a root member keyed "", which a walk that took "" for a token would descend into and find. --- .../openapi/conformance_unmodeled_test.go | 136 +- compilers/openapi/internal/ids/ids.go | 48 +- compilers/openapi/internal/ids/ids_test.go | 69 + compilers/openapi/internal/schema/schema.go | 823 +++++++++--- .../internal/schema/schema_internal_test.go | 163 ++- .../openapi/internal/schema/schema_test.go | 1109 ++++++++++++----- .../openapi/dynamic-ref.golden.json | 111 +- testdata/conformance/openapi/dynamic-ref.yaml | 15 + 8 files changed, 1961 insertions(+), 513 deletions(-) diff --git a/compilers/openapi/conformance_unmodeled_test.go b/compilers/openapi/conformance_unmodeled_test.go index 7244a8c..4527937 100644 --- a/compilers/openapi/conformance_unmodeled_test.go +++ b/compilers/openapi/conformance_unmodeled_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/dexpace/morphic/compilers/openapi/internal/openapitest" "github.com/dexpace/morphic/ir" ) @@ -90,6 +91,76 @@ func assertUnhomedKeywords(t *testing.T, doc *ir.Document, diags []ir.Diagnostic string(unmodeledEntry(t, u.Unmodeled, "openapi:items").Value)) assert.JSONEq(t, `[{"type":"string"},{"type":"integer"}]`, string(unmodeledEntry(t, u.Unmodeled, "openapi:oneOf").Value)) + + assertRefSiteKeywords(t, doc, diags) + assertElectedLoweringKeywords(t, doc, diags) +} + +// assertRefSiteKeywords pins the same census at a $ref site, where it did not +// run at all (GitHub #283). $ref is an ordinary 2020-12 keyword, so its siblings +// are conjoined with it; the position lowers to an alias over the target, which +// has none of their fields, and the five below reached no field, no Unmodeled +// entry and no diagnostic. +func assertRefSiteKeywords(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) { + for name, want := range map[string]struct{ keyword, target, raw string }{ + "RefFormat": {"format", "RefTargetStr", `"email"`}, + "RefEnum": {"enum", "RefTargetStr", `["a","b"]`}, + "RefConst": {"const", "RefTargetStr", `"a"`}, + "RefRequired": {"required", "RefTargetObj", `["a"]`}, + "RefAdditionalProperties": {"additionalProperties", "RefTargetObj", `false`}, + } { + sc, ok := doc.Types[namedID(name)].(*ir.Scalar) + require.True(t, ok, "%s hoists an alias to hold what it wrote beside its $ref", name) + require.NotNil(t, sc.Base) + assert.Equal(t, namedID(want.target), sc.Base.Target, "%s still aliases its target", name) + entry := unmodeledEntry(t, sc.Unmodeled, "openapi:"+want.keyword) + assert.Equal(t, ir.ReasonDegradedLowering, entry.Reason) + assert.JSONEq(t, want.raw, string(entry.Value)) + assert.Equal(t, []ir.Severity{ir.SeverityInfo}, + diagsAt(diags, "openapi/degraded-construct", "/components/schemas/"+name)) + } + + ctl, ok := doc.Types[namedID("RefConstrained")].(*ir.Scalar) + require.True(t, ok) + require.NotNil(t, ctl.Constraints) + assert.Equal(t, int64(3), *ctl.Constraints.MinLength, "a bound beside a $ref always had a home") + assert.Equal(t, "kept", ctl.Docs.Description) + assert.Empty(t, ctl.Unmodeled, "so it keeps nothing verbatim") + + carrier, ok := doc.Types[namedID("RefCarrier")].(*ir.Model) + require.True(t, ok) + p, ok := propByWire(carrier, "p") + require.True(t, ok) + assert.Equal(t, namedID("RefTargetStr"), p.Type.Target, + "a carrier hoists no node, so it still resolves straight to the target") + assert.JSONEq(t, `"email"`, string(unmodeledEntry(t, p.Unmodeled, "openapi:format").Value), + "and keeps the keyword on itself instead") +} + +// assertElectedLoweringKeywords pins the keywords the *winning* family's +// lowering never reads (GitHub #268). The census asks the node that was built, +// which is why the two allOf cases differ: a composed Model asserts `object` +// already and loses nothing, and asserts nothing about `string` at all. +func assertElectedLoweringKeywords(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) { + scalarType, ok := doc.Types[namedID("AllOfWithScalarType")].(*ir.Model) + require.True(t, ok) + assert.JSONEq(t, `"string"`, + string(unmodeledEntry(t, scalarType.Unmodeled, "openapi:type").Value)) + + objectType, ok := doc.Types[namedID("AllOfWithObjectType")].(*ir.Model) + require.True(t, ok) + assert.Empty(t, objectType.Unmodeled, "the composed Model already asserts `object`") + assert.Empty(t, diagsAt(diags, "openapi/degraded-construct", "/components/schemas/AllOfWithObjectType")) + + format, ok := doc.Types[namedID("ConstWithFormat")].(*ir.Literal) + require.True(t, ok) + assert.JSONEq(t, `"int32"`, string(unmodeledEntry(t, format.Unmodeled, "openapi:format").Value), + "a Literal has no Encoding field") + + bound, ok := doc.Types[namedID("ConstWithBound")].(*ir.Literal) + require.True(t, ok) + assert.JSONEq(t, `1`, string(unmodeledEntry(t, bound.Unmodeled, "openapi:maxLength").Value), + "nor a Constraints field, and it owns its pointer so no alias carries one either") } // assertAllOfBooleanBranch pins the lowering of a boolean allOf branch, which @@ -308,6 +379,10 @@ func assertDialectKeywords(t *testing.T, doc *ir.Document, diags []ir.Diagnostic // declared exactly once on a component schema has one possible target whatever // path evaluation took, so it expands; anything else is irreducible and the // reference is kept verbatim beside whatever did lower. +// +// The escaped spelling is here because the fragment is URI text: RFC 3986 §2.3 +// makes '%2D' and '-' one character, so it must reach the same anchor the plain +// spelling would (GitHub #233). func assertDynamicRef(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) { tree, ok := doc.Types[namedID("Tree")].(*ir.Model) require.True(t, ok) @@ -320,6 +395,14 @@ func assertDynamicRef(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) { assert.Equal(t, []ir.Severity{ir.SeverityInfo}, diagsAt(diags, "openapi/dynamic-ref-expanded", "/components/schemas/Tree/properties/child/$dynamicRef")) + escaped, ok := propByWire(tree, "escaped") + require.True(t, ok) + assert.Equal(t, namedID("Leaf"), escaped.Type.Target, + "a percent-encoded fragment names the anchor its decoded spelling names") + assert.Empty(t, escaped.Unmodeled, "an expanded reference must not also be preserved") + assert.Equal(t, []ir.Severity{ir.SeverityInfo}, diagsAt(diags, "openapi/dynamic-ref-expanded", + "/components/schemas/Tree/properties/escaped/$dynamicRef")) + ghost, ok := propByWire(tree, "ghost") require.True(t, ok) entry := unmodeledEntry(t, ghost.Unmodeled, "openapi:$dynamicRef") @@ -327,6 +410,36 @@ func assertDynamicRef(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) { assert.JSONEq(t, `"#Missing"`, string(entry.Value)) assert.Equal(t, []ir.Severity{ir.SeverityInfo}, diagsAt(diags, "openapi/degraded-construct", "/components/schemas/Tree/properties/ghost/$dynamicRef")) + + assertDynamicRefAcrossAnEmptyName(t, doc, diags) +} + +// assertDynamicRefAcrossAnEmptyName pins the reference declared beside an $id on +// the component schema keyed "". Its pointer, /components/schemas/, ends in an +// empty reference token, and a walk that drops that token reads the +// components/schemas map instead of the schema — a map declaring no $id, so the +// resource boundary disappears and the reference expands across it (GitHub +// #302). +// +// It lives in the corpus rather than only in a unit test because the oracles run +// here: order-invariance, determinism, JSON round-trip and irverify each drive +// this construct only if some committed spec writes it, and until this one did, +// no spec combined an empty component name with $id and $dynamicRef at all. +func assertDynamicRefAcrossAnEmptyName(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) { + t.Helper() + // An empty name earns no named TypeID, so the schema hoists anonymously. + empty, ok := doc.Types[ir.TypeID("t/anon/components/schemas/")].(*ir.Scalar) + require.True(t, ok, `the component schema keyed "" owns a node`) + + entry := unmodeledEntry(t, empty.Unmodeled, "openapi:$dynamicRef") + assert.Equal(t, ir.ReasonDegradedLowering, entry.Reason) + assert.JSONEq(t, `"#T"`, string(entry.Value), + "the reference is kept verbatim rather than expanded across the $id") + require.NotNil(t, empty.Base) + assert.Equal(t, ir.TypeID("t/prim/any"), empty.Base.Target, + "an expansion would have made the anchor's own node this one's base instead") + assert.Equal(t, []ir.Severity{ir.SeverityInfo}, diagsAt(diags, "openapi/degraded-construct", + "/components/schemas//$dynamicRef")) } // assertInlineResidue pins the other half of ir-design §14's OpenAPI row: a @@ -340,7 +453,7 @@ func assertDynamicRef(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) { func assertInlineResidue(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { op, ok := opByName(doc, "getThing") require.True(t, ok) - bodyID := op.Responses[0].Payload.Contents[0].Type.Target + bodyID := openapitest.BodyTarget(t, op.Responses[0].Payload) body, ok := doc.Types[bodyID] require.True(t, ok, "the response body owns a node") assertResidue(t, body.Common().Unmodeled, map[string]string{ @@ -387,9 +500,15 @@ func assertResidue(t *testing.T, p ir.Unmodeled, want map[string]string) { } } -// assertResponseLinks pins a response's links: ir.Response has no field for the -// link objects OpenAPI declares there, so they are kept verbatim on the response -// rather than dropped while the operation they name lowers normally. +// assertResponseLinks pins a response's links: neither ir.Response nor +// ir.ErrorCase has a field for the link objects OpenAPI declares there, so they +// are kept verbatim rather than dropped while the operation they name lowers +// normally. +// +// Both status ranges, because only the success one used to keep them: the same +// declaration survived on a 2xx and vanished on a 4xx, with no diagnostic either +// way, purely because the error branch had no links rule of its own +// (GitHub #275). func assertResponseLinks(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { op, ok := opByName(doc, "createOrder") require.True(t, ok) @@ -399,6 +518,15 @@ func assertResponseLinks(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { assert.JSONEq(t, `{"GetOrder":{"operationId":"getOrder","parameters":{"orderId":"$response.body#/id"}}}`, string(entry.Value)) + + require.Len(t, op.Errors, 1) + errEntry := unmodeledEntry(t, op.Errors[0].Unmodeled, "openapi:links") + assert.Equal(t, ir.ReasonNoIRHome, errEntry.Reason) + assert.JSONEq(t, + `{"GetConflicting":{"operationId":"getOrder","parameters":{"orderId":"$response.body#/existingId"}}}`, + string(errEntry.Value), + "an error response keeps its links by the same rule the success one does") + _, ok = opByName(doc, "getOrder") assert.True(t, ok, "the operation a link names is an ordinary operation") } diff --git a/compilers/openapi/internal/ids/ids.go b/compilers/openapi/internal/ids/ids.go index 1150521..0f23a9f 100644 --- a/compilers/openapi/internal/ids/ids.go +++ b/compilers/openapi/internal/ids/ids.go @@ -31,6 +31,23 @@ func Ptr(segments ...string) string { return b.String() } +// Scope joins segments into an Unmodeled key scope: the same escaping Ptr +// applies, without the leading separator, since a scope is a relative path +// rather than a pointer (ir-design §12). +// +// It exists for the scopes holding a segment the document chooses — a form +// part's name, a callback's — where an unescaped "/" makes one segment read as +// two. Two parts named "q" and "q/x-a" then wrote one key between them and the +// surviving entry followed declaration order, silently, which is what §4.3 +// forbids a minted node and §12 promises a scoped key. +func Scope(segments ...string) string { + escaped := make([]string, 0, len(segments)) + for _, seg := range segments { + escaped = append(escaped, escapeSegment(seg)) + } + return strings.Join(escaped, "/") +} + // escapeSegment applies RFC 6901 escaping: ~ first, then /. func escapeSegment(s string) string { s = strings.ReplaceAll(s, "~", "~0") @@ -113,15 +130,42 @@ func DeclarationHint(pointer, fallback string) string { // and ComponentSchemaName narrows it to the schemas kind, which is the only one // that earns a named TypeID. func ComponentEntry(pointer string) (kind, name string, ok bool) { + kind, name, ok = componentEntrySplit(pointer) + if !ok || name == "" { + return "", "", false + } + return kind, UnescapeSegment(name), true +} + +// componentEntrySplit splits the /components// shape without judging +// the name, so a caller that must tell "not that shape at all" from "that shape +// with an empty name" can. ComponentEntry folds the two together on purpose — an +// entry keyed "" earns no named TypeID either way — but the two are different +// facts about a document, and a diagnostic naming the wrong one is simply false. +func componentEntrySplit(pointer string) (kind, name string, ok bool) { const prefix = "/components/" if !strings.HasPrefix(pointer, prefix) { return "", "", false } kind, name, found := strings.Cut(pointer[len(prefix):], "/") - if !found || kind == "" || name == "" || strings.Contains(name, "/") { + if !found || kind == "" || strings.Contains(name, "/") { return "", "", false } - return kind, UnescapeSegment(name), true + return kind, name, true +} + +// ComponentSchemaNamedEmpty reports whether pointer addresses the top-level +// component schema keyed "" — the position /components/schemas/ addresses. +// +// It exists because that schema is a component schema that ComponentSchemaName +// still refuses: an empty name earns no named TypeID, so the schema hoists +// anonymously (testdata/conformance/openapi/empty-names.yaml records that +// policy). A caller that reports the refusal needs the distinction to word it +// truthfully, since a reader who follows the pointer finds a component schema +// sitting exactly where a "not a component schema" message denies one is. +func ComponentSchemaNamedEmpty(pointer string) bool { + kind, name, ok := componentEntrySplit(pointer) + return ok && kind == "schemas" && name == "" } // componentEntryName returns the unescaped name of a top-level component entry diff --git a/compilers/openapi/internal/ids/ids_test.go b/compilers/openapi/internal/ids/ids_test.go index 6c37f52..df1c160 100644 --- a/compilers/openapi/internal/ids/ids_test.go +++ b/compilers/openapi/internal/ids/ids_test.go @@ -34,6 +34,50 @@ func TestPtr_EscapesPerRFC6901(t *testing.T) { } } +// TestScope_EscapesLikePtrWithoutTheLeadingSeparator pins the Unmodeled key +// scope: Ptr's escaping, joined without a leading separator because a scope is a +// relative path. +// +// The slash case is the one that matters. A scope addressing an object the +// document names — a form part, a callback — takes that name as one segment, and +// leaving a "/" in it unescaped let two such objects spell one key between them +// with the survivor following declaration order. +func TestScope_EscapesLikePtrWithoutTheLeadingSeparator(t *testing.T) { + t.Parallel() + tests := []struct { + name string + segments []string + want string + }{ + {name: "plain", segments: []string{"encoding", "avatar"}, want: "encoding/avatar"}, + {name: "slash in a document-chosen name stays one segment", + segments: []string{"encoding", "q/x-a"}, want: "encoding/q~1x-a"}, + {name: "tilde too", segments: []string{"callbacks", "a~b"}, want: "callbacks/a~0b"}, + {name: "tilde before slash, so a ~1 in the source survives", + segments: []string{"callbacks", "~/"}, want: "callbacks/~0~1"}, + {name: "one segment takes no separator", segments: []string{"itemEncoding"}, want: "itemEncoding"}, + {name: "no segments is the unscoped key", segments: nil, want: ""}, + {name: "an empty segment is still a segment", segments: []string{"encoding", ""}, want: "encoding/"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tc.want, ids.Scope(tc.segments...)) + }) + } +} + +// TestScope_DistinguishesNamesThatDifferOnlyBySeparator is the property the +// escaping exists for, stated directly: two names one of which spells the +// other's scope plus a segment must not produce one key. +func TestScope_DistinguishesNamesThatDifferOnlyBySeparator(t *testing.T) { + t.Parallel() + plain := ids.Scope("encoding", "q") + "/x-a/x-b" + slashed := ids.Scope("encoding", "q/x-a") + "/x-b" + assert.NotEqual(t, plain, slashed, + "a part named q/x-a must not land on the key a part named q writes") +} + // TestUnescapeSegment_ReversesPtr pins the other direction, which recovers a // component's on-wire name from a pointer segment. The two must round-trip or a // name containing a slash or a tilde comes back as a different name. @@ -108,6 +152,31 @@ func TestComponentSchemaName_NarrowsToSchemas(t *testing.T) { } } +// TestComponentSchemaNamedEmpty_SeparatesAnEmptyNameFromNoEntry pins the one +// distinction ComponentEntry deliberately throws away. Both answer "no named +// type", but they are different facts: a component schema keyed "" exists at +// /components/schemas/ and earns none, while the other pointers name no +// component-schema entry at all. Only a caller that can tell them apart can +// report the first without denying the schema the document plainly declares. +func TestComponentSchemaNamedEmpty_SeparatesAnEmptyNameFromNoEntry(t *testing.T) { + t.Parallel() + assert.True(t, ids.ComponentSchemaNamedEmpty("/components/schemas/"), + `/components/schemas/ addresses the component schema keyed ""`) + + for _, pointer := range []string{ + "/components/schemas/User", // a named entry + "/components/headers/", // an empty name of another kind + "/components/schemas", // the kind alone, no trailing token + "/components/schemas//properties/id", // a position inside the empty-named schema + "/components//", // no kind + "/paths/~1x/get", // not under components + "", // the empty pointer + } { + assert.False(t, ids.ComponentSchemaNamedEmpty(pointer), + `%q does not address the component schema keyed ""`, pointer) + } +} + // TestForPointer_ChoosesTheNamespace pins the split ForPointer exists for: a // component schema keeps its named ID, everything else is anonymous. The two // namespaces are what stop a hoisted inline type colliding with a declared one. diff --git a/compilers/openapi/internal/schema/schema.go b/compilers/openapi/internal/schema/schema.go index cd594ea..3d0923c 100644 --- a/compilers/openapi/internal/schema/schema.go +++ b/compilers/openapi/internal/schema/schema.go @@ -1,12 +1,15 @@ package schema import ( + "context" "fmt" + "net/url" "slices" "strconv" "strings" oas3 "github.com/speakeasy-api/openapi/jsonschema/oas3" + "github.com/speakeasy-api/openapi/values" yaml "gopkg.in/yaml.v3" "github.com/dexpace/morphic/compilers/compile" @@ -24,7 +27,14 @@ import ( // LowerComponentSchemas interns every named component schema in source order. // It is the entry Compile's run() calls before any operation lowering so that // $refs resolve to already-registered IDs. -func LowerComponentSchemas(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex) []ir.Diagnostic { +// +// ctx bounds the walk in time: a document declares as many components as it +// likes, so this loop is one of the two places a compile does work proportional +// to nothing the compiler chose. Cancellation stops it between components and +// returns what was lowered so far; the caller — run — sees ctx.Err() at the +// phase boundary immediately after and refuses the document there, so a partial +// registry never becomes a Document. +func LowerComponentSchemas(ctx context.Context, c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex) []ir.Diagnostic { comps := c.Doc.Components if comps == nil { return nil @@ -38,6 +48,9 @@ func LowerComponentSchemas(c lowering.Ctx, ts *compile.Types, anchors *AnchorInd // is derived at entry (lowering.New), so a component declared later in the // document is already a valid target here regardless of source order. for name, js := range schemas.All() { + if ctx.Err() != nil { + return diags + } diags = append(diags, lowerComponentSchema(c, ts, anchors, js, ids.Ptr("components", "schemas", name), name)...) } return diags @@ -56,9 +69,10 @@ func lowerComponentSchema(c lowering.Ctx, ts *compile.Types, anchors *AnchorInde if _, owned := ts.Lookup(pointer); owned { return diags } - cons, consDiags := schemaConstraints(c, s.Node, pointer) + var kept ir.Unmodeled + cons, consDiags := schemaConstraints(c, &kept, s.Node, pointer) diags = append(diags, consDiags...) - internAlias(c, ts, pointer, name, ref, cons) + internAlias(c, ts, pointer, name, ref, cons, kept) // This alias is the first node the pointer owns, so the annotations // schemaBody had nowhere to put now have a home. if s.Node != nil { @@ -153,11 +167,17 @@ func recordResidue(c lowering.Ctx, common *ir.TypeCommon, s *oas3.Schema, pointe // internal sub-schema (hoistSubSchema) — so a scalar that aliases a shared // primitive never drops the constraints it carried, including a bound written // beside a $ref, which constrains the position it is written at. -func schemaConstraints(c lowering.Ctx, s *oas3.Schema, pointer string) (*ir.Constraints, []ir.Diagnostic) { +// +// p is the Unmodeled map of the same carrier the constraints are about to land +// on. A co-declared numeric bound leaves one keyword with no field of +// ir.Constraints to reach, and it is kept there — beside the constraints it did +// not reach, wherever those go (GitHub #286). +func schemaConstraints(c lowering.Ctx, p *ir.Unmodeled, s *oas3.Schema, pointer string) (*ir.Constraints, []ir.Diagnostic) { if s == nil { return nil, nil } - cons, diags := annotation.Constraints(s, c.ExclusiveBoundIsBoolean()) + cons, kept, diags := annotation.Constraints(s, c.ExclusiveBoundIsBoolean(), pointer, c.SrcIndex) + *p = annotation.MergeUnmodeled(*p, kept) return cons, StampConstraintDiags(c, diags, pointer) } @@ -176,15 +196,16 @@ func StampConstraintDiags(c lowering.Ctx, diags []ir.Diagnostic, pointer string) // component (or a sibling-carrying schema) whose body lowered to a shared or // referenced target still owns a resolvable node at its own TypeID. Any value // constraints the schema carried are attached so a scalar component never drops -// them. +// them, and kept holds what those constraints had no field for — the co-declared +// bound keyword schemaConstraints read alongside them. func internAlias(c lowering.Ctx, ts *compile.Types, pointer, hint string, - target ir.TypeRef, constraints *ir.Constraints, + target ir.TypeRef, constraints *ir.Constraints, kept ir.Unmodeled, ) ir.TypeID { return internNode(c, ts, pointer, hint, func(common ir.TypeCommon) ir.TypeDef { base := target + common.Unmodeled = annotation.MergeUnmodeled(common.Unmodeled, kept) return &ir.Scalar{TypeCommon: common, Base: &base, Constraints: constraints} }) - } // schemaBody lowers a concrete (non-reference) schema body to a TypeRef and @@ -194,7 +215,9 @@ func internAlias(c lowering.Ctx, ts *compile.Types, pointer, hint string, // leading $ref. func schemaBody(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, depth int, schema *oas3.Schema, pointer, hint string, home annotation.Home) (ir.TypeRef, []ir.Diagnostic) { target, diags := lowerSchemaBody(c, ts, anchors, depth, schema, pointer, hint) - ref, homeDiags := homeDeclaration(c, ts, anchors, schema, target, pointer, hint, home) + // No census verdict: a body's census ran inside lower(), against the node the + // walk actually built, which is the only thing that can answer it. + ref, homeDiags := homeDeclaration(c, ts, anchors, schema, target, pointer, hint, home, false) return ref, append(diags, homeDiags...) } @@ -220,15 +243,24 @@ func schemaBody(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, depth i // declaration-order dependence ir-design §4.3 rules out. The lookup stays // behind the gate above so a position that declares nothing keeps resolving // straight to its target however many references hoisted an alias over it. -func hoistDeclarationHome(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, ref ir.TypeRef, pointer, hint string, home annotation.Home) (ir.TypeRef, []ir.Diagnostic) { - if home != annotation.HomeOwnNode || s == nil || !declaresPositionScoped(s) { +// unhomed is the caller's census verdict: a $ref site's keywords bind the +// position exactly as an annotation does, but declaresPositionScoped cannot say +// so, because the same keywords written on a *body* are the shape it lowers to +// rather than something the position needs a node for. Only the caller knows +// which of the two it is holding. +func hoistDeclarationHome(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, ref ir.TypeRef, pointer, hint string, home annotation.Home, unhomed bool) (ir.TypeRef, []ir.Diagnostic) { + if home != annotation.HomeOwnNode || s == nil { + return ref, nil + } + if !unhomed && !declaresPositionScoped(s) { return ref, nil } if id, owned := ts.Lookup(pointer); owned { return ir.TypeRef{Target: id, Nullable: ref.Nullable}, nil } - cons, diags := schemaConstraints(c, s, pointer) - id := internAlias(c, ts, pointer, hint, ref, cons) + var kept ir.Unmodeled + cons, diags := schemaConstraints(c, &kept, s, pointer) + id := internAlias(c, ts, pointer, hint, ref, cons, kept) return ir.TypeRef{Target: id, Nullable: ref.Nullable}, diags } @@ -262,25 +294,41 @@ func declaresAnnotations(s *oas3.Schema) bool { return s.GetExample() != nil || len(s.GetExamples()) > 0 } +// valueConstraintKeywords are the keywords annotation.Constraints reads into an +// ir.Constraints, sorted — which is both the order declaredConstraints walks +// them in and what makes the list comparable to the reader it must keep pace +// with. +// +// One list, read by the predicate that hoists a node for them +// (declaresValueConstraints) and by the recorder that keeps the ones no node can +// hold (declaredConstraints), so the two can never disagree about what a schema +// wrote. Collection bounds are absent because they are List-owned and read by +// listConstraints, not here. +var valueConstraintKeywords = []string{ + "exclusiveMaximum", "exclusiveMinimum", "maxLength", "maxProperties", + "maximum", "minLength", "minProperties", "minimum", "multipleOf", "pattern", +} + // declaresValueConstraints reports whether s sets any keyword // annotation.Constraints reads. It does not call it: that reports a malformed -// bound, and a predicate must not emit diagnostics. The three numeric bounds -// are detected on their raw nodes for the same reason numericBounds reads them -// there — a magnitude beyond float64 leaves the model field nil while the -// keyword is plainly written. +// bound, and a predicate must not emit diagnostics. The keywords are detected on +// their raw nodes for the same reason numericBounds reads them there — a +// magnitude beyond float64 leaves the model field nil while the keyword is +// plainly written. func declaresValueConstraints(s *oas3.Schema) bool { - for _, keyword := range []string{"minimum", "maximum", "multipleOf"} { + return annotation.DeclaresAny(s, valueConstraintKeywords) +} + +// declaredConstraints returns the value-constraint keywords s writes, in +// valueConstraintKeywords order. +func declaredConstraints(s *oas3.Schema) []string { + out := make([]string, 0, len(valueConstraintKeywords)) + for _, keyword := range valueConstraintKeywords { if annotation.RawPropertyNode(s, keyword) != nil { - return true + out = append(out, keyword) } } - if s.GetExclusiveMinimum() != nil || s.GetExclusiveMaximum() != nil { - return true - } - if s.MinLength != nil || s.MaxLength != nil || s.GetPattern() != "" { - return true - } - return s.MinProperties != nil || s.MaxProperties != nil + return out } // declaresValidationOnly reports whether s writes a §4.7 validation-only @@ -357,7 +405,7 @@ func declaresShape(s *oas3.Schema) bool { if props := s.GetProperties(); props != nil && props.Len() > 0 { return true } - if s.GetConst() != nil || len(s.GetEnum()) > 0 || len(s.GetAllOf()) > 0 { + if s.GetConst() != nil || enumWritten(s) || len(s.GetAllOf()) > 0 { return true } if s.GetAdditionalProperties() != nil { @@ -381,7 +429,12 @@ func lowerBesideUnmodeledUnion(c lowering.Ctx, ts *compile.Types, anchors *Ancho // 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) + // + // 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) } return owner, append(diags, preserveUnionSiblings(c, ts, owner, s, pointer, reason, why)...) } @@ -454,7 +507,7 @@ func declaresFamily(s *oas3.Schema, family string) bool { case "const": return s.GetConst() != nil case "enum": - return len(s.GetEnum()) > 0 + return enumWritten(s) case "allOf": return len(s.GetAllOf()) > 0 default: @@ -462,6 +515,23 @@ func declaresFamily(s *oas3.Schema, family string) bool { } } +// enumWritten reports whether s writes `enum` at all, an empty member list +// included. The three predicates that read the keyword — this family guard, +// declaresShape and composesAsModel — each spelled it `len(...) > 0`, which +// cannot tell `enum: []` from no enum keyword at all, so the degenerate spelling +// was elected by none of them and preserved by none of them either. The position +// then widened to whatever its siblings admitted, in silence (GitHub #278). +// +// `enum: []` is legal JSON Schema and it fixes the value space to the empty set, +// so it declares one exactly as a populated list does. Nilness is the +// distinction the parser keeps: an absent keyword leaves the field nil, an empty +// list leaves it non-nil and empty. A member list the model layer could not +// parse at all reads as absent here, which is a document the loader already +// refuses. +func enumWritten(s *oas3.Schema) bool { + return s.GetEnum() != nil +} + // dispatch records how lower() resolved a schema's competing keyword families: // the one it lowered, and the ones it passed over. won is "" when the schema // declares none of them and the type set decides the lowering instead. @@ -509,11 +579,11 @@ func dispatchOf(s *oas3.Schema) dispatch { // passed over are kept verbatim beside it (recordSkippedFamilies), never // dropped. // -// What that does not cover is a keyword the *elected* lowering never reads — -// `type: string` beside an allOf, `format` beside a const. Deciding those needs a -// per-winner rule rather than a keyword list, since `allOf` beside `type: object` -// is the common case and loses nothing, so it is left open at GitHub #268 rather -// than settled here. +// A keyword the *elected* lowering never reads — `type: string` beside an allOf, +// `format` beside a const — rides the same path through preserveUnhomedKeywords, +// which asks the node that was built whether it has a field for it rather than +// consulting a list of keywords worth keeping. `allOf` beside `type: object` is +// the common case and loses nothing, and a Model answers that for itself. func lower(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, depth int, s *oas3.Schema, pointer, hint string) (ir.TypeID, []ir.Diagnostic) { d := dispatchOf(s) unhomed := func(id ir.TypeID, diags []ir.Diagnostic) (ir.TypeID, []ir.Diagnostic) { @@ -545,68 +615,200 @@ func lower(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, depth int, s } } -// shapeApplicators are the JSON Schema applicator keywords that constrain -// instance shape. Each has exactly one IR home: a Model's property set, openness -// and pattern bindings, or a List's element and a Tuple's positions. +// censusKeywords are the JSON Schema keywords whose IR home depends on what the +// position lowered to rather than being fixed: a Model carries a property set, a +// List an element type and the collection bounds, a Literal a value, an Enum a +// member set, and each of them carries none of the others. Which of these a +// position kept is therefore a question only the node that was built can answer, +// and keywordHome answers it. +// +// The rest of what a schema can write is captured where it is written — +// annotations by attachDeclaredAnnotations, the §4.7 validation-only family by +// preserveKeyword, the content vocabulary by recordUnplacedContent, use-site +// keywords by recordResidue, and value constraints wherever the node has a +// Constraints field, kept by declaredConstraints where it has none. +// +// That division is a claim about this compiler rather than a proof of itself, +// and the collection bounds are what got past it: minItems beside an object, or +// beside the prefixItems that hoists a Tuple with no Constraints field, reached +// the IR in no form at all until they joined the list below. // -// Everything else a schema can write is captured elsewhere — annotations by -// attachDeclaredAnnotations, bounds by schemaConstraints, the §4.7 -// validation-only family by preserveKeyword, the content vocabulary by -// recordUnplacedContent, use-site keywords by recordResidue. The keywords listed -// here had no such recorder, so a position writing one that its lowering could -// not consume dropped it in silence. -var shapeApplicators = []string{ - "properties", "patternProperties", "additionalProperties", "required", - "items", "prefixItems", +// The list and keywordHome's switch name the same set. A keyword listed here +// with no arm there is reported homeless at every position and preserved beside +// every node it is written on, which is noisy but never lossy; a keyword in +// neither is dropped in silence, which is what GitHub #268 and #283 were. +var censusKeywords = []string{ + "additionalProperties", "const", "enum", "format", "items", "maxItems", + "minItems", "patternProperties", "prefixItems", "properties", "required", + "type", "uniqueItems", +} + +// keywordHome reports whether td — the node this position's own declaration +// lowered to — has the field the named keyword lowers into. It asks the node +// rather than re-deriving lower()'s dispatch, so the two cannot drift apart (the +// rule recordUnplacedContent states). +// +// A nil td is a position that lowered to no node of its own: a $ref site, whose +// declaration becomes an alias over the target. That alias carries the position's +// constraints and annotations and nothing else, and the *target's* fields belong +// to the referent's declaration rather than to this one — a `format` beside a +// $ref must not read the referent's Encoding as its own home — so every keyword +// here is homeless at a $ref site. +func keywordHome(td ir.TypeDef, s *oas3.Schema, keyword string) bool { + switch keyword { + case "properties", "patternProperties", "additionalProperties", "required": + return isKind(td, ir.KindModel) + case "items", "prefixItems": + return isKind(td, ir.KindList) || isKind(td, ir.KindTuple) + case "maxItems", "minItems", "uniqueItems": + // Only a List. listConstraints is their sole reader and lowerArray its + // sole caller, while ir.Tuple has no Constraints field at all — so a + // collection bound beside prefixItems reaches as little as one written on + // an object does. Being List-owned is also why valueConstraintKeywords + // leaves them out, which is what puts them in this census rather than in + // declaredConstraints. + return isKind(td, ir.KindList) + case "const": + // Any counts as well as Literal. hoistLiteral degrades a value ir.Value + // cannot represent to the top type and reports it, so the const was read + // — badly, and already announced — rather than left unread. Claiming it + // here would report one keyword twice, the second time as an error, since + // a value no converter can read is one no preserver can read either. + return isKind(td, ir.KindLiteral) || isKind(td, ir.KindAny) + case "enum": + // Any counts here for the reason it counts for const: lowerEnum degrades a + // member set past the caller's budget to the top type and reports it, so + // the enum was read and announced rather than left unread. Claiming it + // again would also undo the budget — the census preserves a homeless + // keyword verbatim, which would put every member back into the IR as raw + // bytes and leave only the per-member amplification bounded (GitHub #75). + return isKind(td, ir.KindEnum) || isKind(td, ir.KindUnion) || isKind(td, ir.KindAny) + case "format": + return formatHome(td, s) + case "type": + return typeHome(td, s) + default: + return false + } } -// applicatorHome reports whether the node a position lowered to has a field that -// carries the named applicator. It asks the node rather than re-deriving lower()'s -// dispatch, so the two cannot drift apart (the rule recordUnplacedContent states). -func applicatorHome(td ir.TypeDef, keyword string) bool { - switch td.(type) { - case *ir.Model: - switch keyword { - case "properties", "patternProperties", "additionalProperties", "required": - return true - default: +// isKind reports whether td is a live node of kind k. A nil td — the $ref site's +// alias, which is no node of the walk's making — is of no kind at all. +func isKind(td ir.TypeDef, k ir.TypeKind) bool { + return !ir.IsNilTypeDef(td) && td.Kind() == k +} + +// formatHome reports whether the position's `format` reached a field. A Scalar +// hoisted for it carries it in Encoding; a (type, format) pairing formatTable +// knows selects a primitive, which carries the pairing in the primitive kind +// itself. With no type declared there was no pairing to make and nothing read it, +// and a node of any other kind has no Encoding field at all. +func formatHome(td ir.TypeDef, s *oas3.Schema) bool { + switch n := td.(type) { + case *ir.Scalar: + return n.Encoding != nil + case *ir.Primitive: + return len(effectiveTypes(s)) > 0 + default: + return false + } +} + +// typeHome reports whether every shape s's declared type set names is a shape td +// can be. One that td cannot be was read by nothing: the walk built a different +// shape, so `type: string` beside an allOf that composed a Model states something +// the IR no longer holds. +// +// A type set that reduces to nothing — `type: "null"` on its own — is carried by +// TypeRef.Nullable rather than by a node, so it is homed wherever it is written. +func typeHome(td ir.TypeDef, s *oas3.Schema) bool { + for _, st := range effectiveTypes(s) { + if !typeShapedBy(td, st) { return false } + } + return true +} + +// typeShapedBy reports whether td is a node kind that carries the shape st names. +// +// An Enum carries a scalar type in ValueType, which enumValueType fills from +// exactly that type set. A Literal carries only its value, so a `type` written +// beside a const reached no field at all — including the case where the two +// agree, since the IR then holds the value and nothing about what was declared +// about it. Any, External and the $ref site's nil carry no shape. +func typeShapedBy(td ir.TypeDef, st oas3.SchemaType) bool { + switch td.(type) { + case *ir.Model: + return st == oas3.SchemaTypeObject case *ir.List, *ir.Tuple: - return keyword == "items" || keyword == "prefixItems" + return st == oas3.SchemaTypeArray + case *ir.Primitive, *ir.Scalar, *ir.Enum: + return st != oas3.SchemaTypeObject && st != oas3.SchemaTypeArray + case *ir.Any: + // The top type admits every shape, so a declared type says nothing it + // contradicts. A position only reaches Any with a type declared by being + // degraded there — an unrepresentable const, an enum past its budget — and + // that degradation is already reported, so restating the type beside it + // would announce the same collapse twice. + return true + default: + return false + } +} + +// constraintsHome reports whether the value constraints a position declared +// reached td's Constraints field. A position that owns its node hoists no alias +// over it (hoistDeclarationHome returns the node already at the pointer), so +// anything that missed the field here reaches no field anywhere. +// +// Having the field is not reading it, so the two node kinds that have one ask +// whether it was filled: a Model lowerAllOf composed carries no constraints at +// all, where one lowerModel built carries whatever schemaConstraints read. A +// List is absent for the stronger reason — lowerArray fills its Constraints from +// listConstraints, whose collection bounds valueConstraintKeywords deliberately +// excludes, so no value constraint ever reaches it however full the field looks. +func constraintsHome(td ir.TypeDef) bool { + switch n := td.(type) { + case *ir.Scalar: + return n.Constraints != nil + case *ir.Model: + return n.Constraints != nil default: - // An Enum, Literal, Scalar, Primitive or Union carries no property set - // and no element type, so every applicator written beside one is homeless. + // An Enum, Literal, Union, Tuple or Primitive has no Constraints field at + // all, so a bound written beside one was read by nothing. return false } } -// unhomedKeywords returns the keywords s declares that nothing reading this -// position can carry, in the order shapeApplicators lists them, with `format` -// last. It reads the raw nodes for the reason declaresValueConstraints does: a -// keyword the model layer failed to parse is still plainly written. +// unhomedKeywords returns the census keywords s declares that td has nowhere to +// carry, in censusKeywords order. It reads the raw nodes for the reason +// declaresValueConstraints does: a keyword the model layer failed to parse is +// still plainly written. // -// `format` is homed by a declared type, not by a node kind: scalarTypeID pairs -// the two to select a primitive or hoist a Scalar carrying the encoding. Written -// with no type beside it, nothing reads it at all — which is the one case that -// can be decided here without asking how the pairing turned out. -func unhomedKeywords(s *oas3.Schema, td ir.TypeDef) []string { - var out []string - for _, keyword := range shapeApplicators { - if annotation.RawPropertyNode(s, keyword) == nil || applicatorHome(td, keyword) { +// handled names the keywords another reader at this position already accounts +// for, which are homeless by this test but are not this census's to keep: the +// families lower()'s election passed over go to recordSkippedFamilies, which says +// why they lost an election — a fact this census does not know — and an allOf +// branch's `required` is consumed by applyCompositionRequired. Recording them +// here too would report one keyword twice under two messages. +func unhomedKeywords(s *oas3.Schema, td ir.TypeDef, handled []string) []string { + out := make([]string, 0, len(censusKeywords)) + for _, keyword := range censusKeywords { + if annotation.RawPropertyNode(s, keyword) == nil || keywordHome(td, s, keyword) { + continue + } + if slices.Contains(handled, keyword) { continue } out = append(out, keyword) } - if len(effectiveTypes(s)) == 0 && annotation.RawPropertyNode(s, "format") != nil { - out = append(out, "format") - } return out } -// preserveUnhomedKeywords keeps verbatim the shape applicators a position -// declared that the node it lowered to has nowhere to carry, and returns the ID -// the position resolves to afterwards. +// preserveUnhomedKeywords keeps verbatim the census keywords and value +// constraints a position declared that the node it lowered to has nowhere to +// carry, and returns the ID the position resolves to afterwards. // // Two losses share this shape. A contradictory schema — `{type: string, enum: // [a, b], properties: {f: ...}}` — cannot be both a two-member string enum and an @@ -632,7 +834,10 @@ func unhomedKeywords(s *oas3.Schema, td ir.TypeDef) []string { // A lowering that reduced to a shared node gets an alias of its own first: a // shared primitive must never carry one declaration's keywords. The alias takes // the position's constraints too, because owning the pointer is what stops -// hoistDeclarationHome attaching them afterwards. +// hoistDeclarationHome attaching them afterwards — and by the same token, a +// position whose lowering already owns the pointer gets no alias at all, so a +// bound written beside a node with no Constraints field (an Enum, a Literal) is +// kept verbatim here rather than reaching a field that does not exist. // // The families lower()'s election passed over ride the same path: they too were // declared here, they too have no home on the node the election produced, and @@ -642,17 +847,24 @@ func preserveUnhomedKeywords(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, if !ok { return id, diags } - unhomed := unhomedKeywords(s, td) + owns, _ := ts.Lookup(pointer) + unhomed := unhomedKeywords(s, td, d.skipped) + if owns == id && !constraintsHome(td) { + // No alias will be hoisted over a node the position already owns, so the + // bounds it wrote reach no Constraints field anywhere. + unhomed = append(unhomed, declaredConstraints(s)...) + } if len(unhomed) == 0 && len(d.skipped) == 0 { return id, diags } owner := id - if got, _ := ts.Lookup(pointer); got != id { - cons, consDiags := schemaConstraints(c, s, pointer) + if owns != id { + var kept ir.Unmodeled + cons, consDiags := schemaConstraints(c, &kept, s, pointer) diags = append(diags, consDiags...) - owner = internAlias(c, ts, pointer, hint, ir.TypeRef{Target: id}, cons) + owner = internAlias(c, ts, pointer, hint, ir.TypeRef{Target: id}, cons, kept) } - diags = append(diags, recordUnhomedKeywords(c, ts, owner, s, unhomed, td.Kind(), pointer)...) + diags = append(diags, recordUnhomedKeywords(c, ts, owner, s, unhomed, nodeShape(td.Kind()), pointer)...) return owner, append(diags, recordSkippedFamilies(c, ts, owner, s, d, pointer)...) } @@ -699,20 +911,37 @@ func recordSkippedFamilies(c lowering.Ctx, ts *compile.Types, owner ir.TypeID, s skipped, d.won, d.won, skipped)) } -// recordUnhomedKeywords stores each unhomed applicator on the owning node and -// reports them once, naming the form the lowering took so a reader is told which -// half of a contradictory schema the IR describes. -func recordUnhomedKeywords(c lowering.Ctx, ts *compile.Types, owner ir.TypeID, s *oas3.Schema, unhomed []string, kind ir.TypeKind, pointer string) []ir.Diagnostic { +// refSiteShape describes what a $ref position's own declaration lowers to, for +// the diagnostic recordUnhomedAt reports. +const refSiteShape = "an alias over its $ref target" + +// nodeShape describes a node of kind k, for that same diagnostic. +func nodeShape(kind ir.TypeKind) string { return fmt.Sprintf("a node of kind %q", kind) } + +// recordUnhomedKeywords stores each unhomed keyword on the node owner names. +// shape describes what the position lowered to, so a reader is told which half +// of a contradictory schema the IR describes. +func recordUnhomedKeywords(c lowering.Ctx, ts *compile.Types, owner ir.TypeID, s *oas3.Schema, unhomed []string, shape, pointer string) []ir.Diagnostic { + if len(unhomed) == 0 { + return nil + } td, ok, diags := registeredNode(c, ts, owner, pointer) if !ok { return diags } - common := td.Common() - // Only the keywords actually written are named, so the message cannot claim - // one that failed to convert and was reported unpreservable instead. + return append(diags, recordUnhomedAt(c, &td.Common().Unmodeled, s, unhomed, shape, pointer)...) +} + +// recordUnhomedAt keeps each unhomed keyword verbatim in p and reports them once. +// +// It names the keywords it actually stored, never the ones it was handed, so the +// message cannot claim one that failed to convert and was reported unpreservable +// instead (GitHub #144). +func recordUnhomedAt(c lowering.Ctx, p *ir.Unmodeled, s *oas3.Schema, unhomed []string, shape, pointer string) []ir.Diagnostic { + var diags []ir.Diagnostic kept := make([]string, 0, len(unhomed)) for _, keyword := range unhomed { - ok, keptDiags := PreserveSchemaKeyword(c, &common.Unmodeled, s, keyword, + ok, keptDiags := PreserveSchemaKeyword(c, p, s, keyword, ir.ReasonDegradedLowering, pointer+ids.Ptr(keyword)) diags = append(diags, keptDiags...) if ok { @@ -723,8 +952,8 @@ func recordUnhomedKeywords(c lowering.Ctx, ts *compile.Types, owner ir.TypeID, s return diags } return append(diags, c.DiagAt(ir.SeverityInfo, diag.DegradedConstruct, pointer, - "this position lowered to a node of kind %q, which has no home for %s declared beside it; kept verbatim under Unmodeled", - kind, strings.Join(kept, ", "))) + "this position lowered to %s, which has no home for %s declared beside it; kept verbatim under Unmodeled", + shape, strings.Join(kept, ", "))) } // lowerTyped dispatches a single-typed schema to its structural or scalar form. @@ -784,7 +1013,7 @@ func lowerUnion(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, depth i func lowerModel(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, depth int, s *oas3.Schema, pointer, hint string) (ir.TypeID, []ir.Diagnostic) { var diags []ir.Diagnostic id := internNode(c, ts, pointer, hint, func(common ir.TypeCommon) ir.TypeDef { - cons, consDiags := schemaConstraints(c, s, pointer) + cons, consDiags := schemaConstraints(c, &common.Unmodeled, s, pointer) diags = append(diags, consDiags...) m := &ir.Model{TypeCommon: common, Constraints: cons} diags = append(diags, fillModelProperties(c, ts, anchors, depth, m, s, pointer)...) @@ -851,9 +1080,30 @@ func FillPropertyDetail(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, if ref.GetFormat() == "password" { p.Secret = true } - p.Visibility = annotation.EffectiveVisibility(ref, tgt) + diags = append(diags, fillPropertyVisibility(c, p, ref, tgt, pointer)...) diags = append(diags, fillPropertyConstraints(c, p, ref, pointer)...) - return append(diags, fillPropertyAnnotations(c, ts, anchors, p, ref, tgt, pointer)...) + diags = append(diags, fillPropertyAnnotations(c, ts, anchors, p, ref, tgt, pointer)...) + return append(diags, PreserveRefSiteKeywords(c, ts, &p.Unmodeled, js, p.Type, pointer)...) +} + +// fillPropertyVisibility lowers readOnly/writeOnly onto the property and reports +// the pairing that leaves it visible nowhere. +// +// The report is made here rather than inside the reader for the reason +// merge.reconcileProperty reports its own disjoint intersection from outside +// mergeVisibility: provenance is built in one place (lowering.Ctx), and this is +// the caller that knows the position being filled. It is the same finding under +// the same code as the allOf spelling, so a consumer filtering on +// diag.DisjointVisibility sees both. +func fillPropertyVisibility(c lowering.Ctx, p *ir.Property, ref, tgt *oas3.Schema, pointer string) []ir.Diagnostic { + visibility, disjoint := annotation.EffectiveVisibility(ref, tgt) + p.Visibility = visibility + if !disjoint { + return nil + } + return []ir.Diagnostic{c.DiagAt(ir.SeverityWarning, diag.DisjointVisibility, pointer, + "readOnly and writeOnly are both in force for field %q; they admit disjoint lifecycles, "+ + "so the field is visible in none", p.WireName)} } // fillPropertyAnnotations records the annotations the property's schema @@ -891,10 +1141,12 @@ func fillPropertyAnnotations(c lowering.Ctx, ts *compile.Types, anchors *AnchorI p.Examples = a.Examples } p.Unmodeled = annotation.MergeUnmodeled(p.Unmodeled, a.Unmodeled) + diags = append(diags, c.PromoteDeprecation(p.Unmodeled, p.Deprecation, &p.Provenance)...) // nil node: this arm runs only when the schema lowered to no node of its own, // so nothing here can be carrying an Encoding. diags = append(diags, recordUnplacedContent(c, &p.Unmodeled, ref, nil, pointer)...) - return append(diags, recordUnexpandedDynamicRef(c, anchors, &p.Unmodeled, ref, pointer)...) + diags = append(diags, recordUnexpandedDynamicRef(c, anchors, &p.Unmodeled, ref, pointer)...) + return append(diags, PreserveUnknownKeywords(c, &p.Unmodeled, ref, pointer)...) } // LoweredToOwnNode reports whether the declaration at pointer lowered to a type @@ -930,14 +1182,16 @@ func fillPropertyDefault(c lowering.Ctx, p *ir.Property, ref, tgt *oas3.Schema, return nil } -// fillPropertyConstraints attaches the property's scalar constraints and stamps -// each constraint diagnostic with the property's provenance. +// fillPropertyConstraints attaches the property's scalar constraints, and the +// co-declared bound keyword that reached none of them, to the property itself. +// ir.Property is the carrier at this position: a property's schema is read +// through CarriedRef, so it hoists no node of its own to hold either. func fillPropertyConstraints(c lowering.Ctx, p *ir.Property, ref *oas3.Schema, pointer string) []ir.Diagnostic { - cons, diags := annotation.Constraints(ref, c.ExclusiveBoundIsBoolean()) + cons, diags := schemaConstraints(c, &p.Unmodeled, ref, pointer) if cons != nil { p.Constraints = cons } - return StampConstraintDiags(c, diags, pointer) + return diags } // attachDeclaredAnnotations records every annotation s declares on the type @@ -974,11 +1228,13 @@ func attachDeclaredAnnotations(c lowering.Ctx, ts *compile.Types, anchors *Ancho common.XML = a.XML } common.Unmodeled = annotation.MergeUnmodeled(common.Unmodeled, a.Unmodeled) + diags = append(diags, c.PromoteDeprecation(common.Unmodeled, common.Deprecation, &common.Provenance)...) if len(a.Examples) > 0 { common.Examples = a.Examples } diags = append(diags, recordUnplacedContent(c, &common.Unmodeled, s, td, pointer)...) - return append(diags, recordUnexpandedDynamicRef(c, anchors, &common.Unmodeled, s, pointer)...) + diags = append(diags, recordUnexpandedDynamicRef(c, anchors, &common.Unmodeled, s, pointer)...) + return append(diags, PreserveUnknownKeywords(c, &common.Unmodeled, s, pointer)...) } // fillAdditional lowers additionalProperties, patternProperties, and @@ -1091,7 +1347,7 @@ func hoistByteScalar(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, pointer, enc, encDiags := scalarEncoding(c, s, "base64", &common, pointer) diags = append(diags, encDiags...) enc.WireType = &wire - cons, consDiags := schemaConstraints(c, s, pointer) + cons, consDiags := schemaConstraints(c, &common.Unmodeled, s, pointer) diags = append(diags, consDiags...) return &ir.Scalar{ TypeCommon: common, @@ -1111,7 +1367,7 @@ func hoistFormatScalar(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, base i baseRef := ts.PrimRef(base) enc, encDiags := scalarEncoding(c, s, format, &common, pointer) diags = append(diags, encDiags...) - cons, consDiags := schemaConstraints(c, s, pointer) + cons, consDiags := schemaConstraints(c, &common.Unmodeled, s, pointer) diags = append(diags, consDiags...) return &ir.Scalar{ TypeCommon: common, @@ -1133,7 +1389,7 @@ func hoistContentScalar(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, prim base := ts.PrimRef(prim) enc, encDiags := scalarEncoding(c, s, "", &common, pointer) diags = append(diags, encDiags...) - cons, consDiags := schemaConstraints(c, s, pointer) + cons, consDiags := schemaConstraints(c, &common.Unmodeled, s, pointer) diags = append(diags, consDiags...) return &ir.Scalar{ TypeCommon: common, @@ -1280,7 +1536,7 @@ func dynamicExpansion(c lowering.Ctx, anchors *AnchorIndex, s *oas3.Schema, poin } id, resolved, handled := c.RefScope().ComponentRef(at) if !handled || !resolved { - return "", fmt.Sprintf("$dynamicAnchor %q is declared at %q rather than on a component schema", name, at), false, diags + return "", unnamedAnchorSiteWhy(name, at), false, diags } chainWhy, chainOK, chainDiags := dynamicChainVerdict(c, anchors, at, pointer) diags = append(diags, chainDiags...) @@ -1290,6 +1546,25 @@ func dynamicExpansion(c lowering.Ctx, anchors *AnchorIndex, s *oas3.Schema, poin return id, "", true, diags } +// unnamedAnchorSiteWhy words why the position declaring an anchor is not a +// target the IR can name, for the two document shapes that reach it. +// +// A pointer deeper than a top-level component schema names a position with no +// TypeID stable enough to expand to, which is the ordinary case. A component +// schema keyed "" is the other, and it is a component schema — /components/ +// schemas/ addresses it — that earns no named TypeID all the same, because an +// empty name is not one. Wording that as "rather than on a component schema" +// states something the document contradicts: a reader who follows the pointer +// lands on the very thing the message says is not there. The verdict is the same +// either way; only the reason differs. +func unnamedAnchorSiteWhy(name, at string) string { + if ids.ComponentSchemaNamedEmpty(at) { + return fmt.Sprintf(`$dynamicAnchor %q is declared on the component schema keyed "" at %q, `+ + "and an empty name earns no named type to expand to", name, at) + } + return fmt.Sprintf("$dynamicAnchor %q is declared at %q rather than on a component schema", name, at) +} + // dynamicRefName returns the $dynamicAnchor name the $dynamicRef s writes // addresses, or the reason it addresses none. It stops short of the index, so // the chain walk and the lowering path ask one function what a schema requests. @@ -1304,11 +1579,7 @@ func dynamicRefName(s *oas3.Schema) (name, why string, ok bool) { if node.Kind != yaml.ScalarNode { return "", "its value is not a reference string", false } - fragment, found := dynamicFragment(node.Value) - if !found { - return "", fmt.Sprintf("%q is not a plain same-document fragment", node.Value), false - } - return fragment, "", true + return dynamicFragment(node.Value) } // dynamicRefSiblings reports whether s writes anything beside its $dynamicRef @@ -1363,7 +1634,8 @@ func soleAnchorSite(c lowering.Ctx, anchors *AnchorIndex, name string) (at, why // The loop is bounded by seen: cur only ever takes values from the anchor // index, and each turn either returns or adds one of them. func dynamicChainVerdict(c lowering.Ctx, anchors *AnchorIndex, at, from string) (why string, ok bool, diags []ir.Diagnostic) { - if declaresResourceIDAbove(c, from) { + view := anchors.nodes() + if declaresResourceIDAbove(c, view, from) { return resourceBoundaryWhy(from), false, nil } seen := map[string]bool{} @@ -1372,7 +1644,7 @@ func dynamicChainVerdict(c lowering.Ctx, anchors *AnchorIndex, at, from string) return fmt.Sprintf("expanding it closes a cycle of $dynamicRef expansions back onto %q, "+ "leaving a type whose own base chain never terminates", from), false, diags } - if declaresResourceIDAbove(c, cur) { + if declaresResourceIDAbove(c, view, cur) { return resourceBoundaryWhy(cur), false, diags } seen[cur] = true @@ -1440,48 +1712,61 @@ func componentSchemaAt(c lowering.Ctx, pointer string) *oas3.Schema { // there. That is the same direction the anchor index errs in: a false boundary // costs an expansion that would have been safe, where a missed one mints a // reference the IR cannot express. -func declaresResourceIDAbove(c lowering.Ctx, pointer string) bool { - view := nodeview.New() - cur := nodeview.DocumentRoot(nodeview.Deref(c.Doc.GetRootNode())) - for _, token := range pointerTokens(pointer) { - if cur == nil { - return false - } - if view.ChildByToken(cur, "$id") != nil { +// +// The path comes from nodeview.PointerPath, which already reads a pointer the +// way this walk needs: it drops only the leading empty segment, so a later empty +// token still takes a step of its own and names the key "" — which is how a +// component schema named "" is addressed (/components/schemas/). Walking the +// tokens here instead had dropped every empty segment, stopping the walk above +// such a position and reading the $id of the components/schemas map rather than +// the schema's own. An incomplete walk needs no separate arm: PointerPath yields +// the nodes it did reach, and a boundary above a pointer that falls off the tree +// still binds. +func declaresResourceIDAbove(c lowering.Ctx, view *nodeview.View, pointer string) bool { + root := nodeview.DocumentRoot(nodeview.Deref(c.Doc.GetRootNode())) + path, _ := view.PointerPath(root, pointer) + for _, n := range path { + if view.ChildByToken(n, "$id") != nil { return true } - cur = nodeview.Deref(view.ChildByToken(cur, ids.UnescapeSegment(token))) } - return cur != nil && view.ChildByToken(cur, "$id") != nil + return false } -// pointerTokens returns the RFC 6901 reference tokens of pointer, still escaped. -// Splitting on '/' yields one leading empty segment that is the split's artifact -// rather than a token, and only that one is: a later empty segment names the key -// "", which is how a component schema named "" is addressed -// (/components/schemas/). Dropping every empty segment stopped the walk above -// such a position and read the $id of its parent instead of its own. +// dynamicFragment returns the anchor name a $dynamicRef addresses, or the reason +// it addresses none. Only the same-document `#name` spelling resolves here: a +// URI part names another resource (Milestone 1 interns only same-file targets) +// and a `#/…` pointer addresses a position rather than an anchor. // -// A string with no leading '/' has no tokens at all: the empty pointer names the -// whole document, and a relative pointer names no position in it. -func pointerTokens(pointer string) []string { - rest, found := strings.CutPrefix(pointer, "/") - if !found { - return nil - } - return strings.Split(rest, "/") -} - -// dynamicFragment returns the plain fragment name a $dynamicRef addresses. Only -// the same-document `#name` spelling resolves here: a URI part names another -// resource (Milestone 1 interns only same-file targets) and a `#/…` pointer -// addresses a position rather than an anchor. -func dynamicFragment(ref string) (string, bool) { - name, found := strings.CutPrefix(ref, "#") - if !found || name == "" || strings.Contains(name, "/") { - return "", false +// The name is the fragment percent-decoded exactly once, because a fragment is +// URI text (RFC 3986 §3.5) in which an unreserved character and its escape are +// the same character (§2.3, §6.2.2.2): `#my%2Danchor` and `#my-anchor` name one +// anchor, and §2.1 leaves the escape's hex case insignificant. A second decode +// would make `#my%252Danchor` name it too, though that spells the distinct name +// `my%2Danchor` (GitHub #233). PathUnescape rather than the QueryUnescape +// nodeview uses to mirror the resolver: `+` is a literal plus in a fragment. +// +// The $dynamicAnchor this is matched against is deliberately *not* decoded. +// 2020-12 §8.2.3.2 makes $dynamicRef a URI-reference, while §8.2.2 makes an +// anchor a plain name whose production admits no `%` at all — so decoding that +// side could only rewrite a name the dialect already rejects, while collapsing +// the distinct names `a-b` and `a%2Db` into one. How many declarations share a +// name is what decides whether a reference expands, so keeping them apart is +// load-bearing. +// +// A fragment that is not valid percent-encoded text is refused rather than +// matched raw, since it is not URI text and so names no anchor; the caller keeps +// the reference verbatim with this reason beside it. +func dynamicFragment(ref string) (name, why string, ok bool) { + fragment, found := strings.CutPrefix(ref, "#") + if !found || fragment == "" || strings.Contains(fragment, "/") { + return "", fmt.Sprintf("%q is not a plain same-document fragment", ref), false + } + name, err := url.PathUnescape(fragment) + if err != nil { + return "", fmt.Sprintf("%q is not valid percent-encoded text: %s", ref, err.Error()), false } - return name, true + return name, "", true } // recordUnexpandedDynamicRef keeps a $dynamicRef verbatim on p when it was not @@ -1525,6 +1810,25 @@ func declaresDynamicRef(s *oas3.Schema) bool { // trade for a keyword almost no document uses. type AnchorIndex struct { byName map[string][]string + view *nodeview.View +} + +// nodes returns the raw-tree view the resource-boundary walks share, building it +// on first use like the index beside it. +// +// A verdict walks the path once for the position and once per chain link, so a +// document reaches this several times per $dynamicRef it writes; a view built +// per walk throws away the expansion memo before anything can hit it. Sharing +// one is about keeping that memo rather than about a measured win — compiling +// merge-heavy specs (a 60-link chain on the walked path, 40 deep reference +// positions) timed the same either way, and the IR is byte-identical. What it +// buys is that the memo is now reachable at all, which is the reason the view +// carries one. +func (a *AnchorIndex) nodes() *nodeview.View { + if a.view == nil { + a.view = nodeview.New() + } + return a.view } // sites returns the pointers declaring the named $dynamicAnchor, building the @@ -1688,33 +1992,202 @@ func effectiveTypes(s *oas3.Schema) []oas3.SchemaType { return out } -// schemaHasNull reports whether a schema admits null via either dialect: 3.0 -// nullable: true or a 3.1 type array containing "null". -func schemaHasNull(s *oas3.Schema) bool { +// nullVerdict is what one keyword family says about the null value. JSON Schema +// conjoins keywords, so the families are read together rather than first-match: +// a schema admits null when one family puts it in the value space and no other +// takes it out. +type nullVerdict int + +const ( + // nullSilent is a family the schema does not declare, or one that constrains + // only non-null instances. It forbids nothing. + nullSilent nullVerdict = iota + // nullAdmitted is a family that puts null in the value space. + nullAdmitted + // nullForbidden is a family that takes null out of it. + nullForbidden +) + +// maxNullConjuncts bounds the conjunct walk below (styleguide bounded-recursion +// rule). One budget unit is spent per schema visited, so it caps the walk's +// depth as well as its breadth — an `allOf` naming its own schema, or a diamond +// of conjunctions, terminates on it rather than on the shape of the source. A +// conjunction deeper or wider than this reads as silent, which is the answer +// that claims the least. +const maxNullConjuncts = 256 + +// schemaAdmitsNull reports whether a schema admits the null value. Lowering +// lifts every spelling of that onto the enclosing TypeRef rather than into the +// type node, so this is the one predicate every site computing a Nullable bit +// goes through — a definition site, a union, an allOf conjunct and a $ref use +// site must never disagree about the same schema. +func schemaAdmitsNull(s *oas3.Schema) bool { + budget := maxNullConjuncts + return schemaNullVerdict(s, &budget) == nullAdmitted +} + +// schemaNullVerdict reads what a whole schema says about null, by conjoining +// what each of its keyword families says (foldNullVerdicts). +// +// 3.0 `nullable: true` is the one keyword that does not conjoin: it widens the +// schema it is written on, which is what it exists to do, so it decides alone. +// The 3.1 spelling is an ordinary `type` member and conjoins like any other — +// which is why `{type: [string, "null"], enum: [red, green]}` does not admit +// null while `{type: string, nullable: true, enum: [red, green]}` does. +func schemaNullVerdict(s *oas3.Schema, budget *int) nullVerdict { + if s == nil || *budget <= 0 { + return nullSilent + } + *budget-- if s.Nullable != nil && *s.Nullable { - return true + return nullAdmitted + } + return foldNullVerdicts(typeNullVerdict(s), constNullVerdict(s), enumNullVerdict(s), + unionNullVerdict(s), allOfNullVerdict(s, budget)) +} + +// foldNullVerdicts conjoins keyword verdicts: one forbidding family decides the +// schema, otherwise one admitting family does, and a schema no family speaks for +// stays silent — which is not the same answer as forbidding, since a silent +// conjunct must not veto a sibling that admits. +func foldNullVerdicts(verdicts ...nullVerdict) nullVerdict { + out := nullSilent + for _, v := range verdicts { + if v == nullForbidden { + return nullForbidden + } + if v == nullAdmitted { + out = nullAdmitted + } } - return slices.Contains(s.GetType(), oas3.SchemaTypeNull) + return out } -// schemaAdmitsNull reports whether a schema admits null in any spelling: the two -// keyword dialects (schemaHasNull) or a oneOf/anyOf null branch. Lowering lifts -// all of them onto the enclosing TypeRef rather than into the type node, so this -// is the one predicate every site computing a Nullable bit goes through — a -// definition site, a union, and a $ref use site must never disagree about the -// same schema. +// typeNullVerdict reads the `type` keyword. A schema writing none constrains no +// instance kind at all, so it is silent rather than forbidding. +func typeNullVerdict(s *oas3.Schema) nullVerdict { + types := s.GetType() + if len(types) == 0 { + return nullSilent + } + if slices.Contains(types, oas3.SchemaTypeNull) { + return nullAdmitted + } + return nullForbidden +} + +// constNullVerdict reads `const`, which fixes the value space to one member. +func constNullVerdict(s *oas3.Schema) nullVerdict { + node := s.GetConst() + if node == nil { + return nullSilent + } + if isNullValue(node) { + return nullAdmitted + } + return nullForbidden +} + +// enumNullVerdict reads `enum`, which fixes the value space to its members: the +// position admits null exactly when a member is null. // -// A null branch counts only when the union is the type itself. Structural -// siblings intersect with the union (JSON Schema conjoins keywords), so +// An empty enum is silent rather than forbidding. It lists no member at all, so +// reading it as "no null member" would let a degenerate keyword strip a +// co-declared type array's null; what an empty enum lowers to is GitHub #278's +// question, and this rule leaves it open. +func enumNullVerdict(s *oas3.Schema) nullVerdict { + nodes := s.GetEnum() + if len(nodes) == 0 { + return nullSilent + } + if slices.ContainsFunc(nodes, isNullValue) { + return nullAdmitted + } + return nullForbidden +} + +// isNullValue reports whether an enum member or const node is the null literal, +// read through the same converter enumMembers drops a member by. Both must +// recognize one spelling: the null this predicate lifts onto a reference is +// exactly the member that lowering strips. +func isNullValue(node values.Value) bool { + val, err := value.FromNode(node) + return err == nil && val.Kind == ir.ValueNull +} + +// unionNullVerdict reads a oneOf/anyOf null branch, which counts only when the +// union is the type itself. Structural siblings intersect with the union, so // `{type: object, oneOf: [{type: string}, {type: null}]}` admits neither string // nor null; that union is kept verbatim under Unmodeled instead. A `type: null` // branch is written inline, so it also blocks distribution — no distributed // union can strip a null branch out from under this rule. -func schemaAdmitsNull(s *oas3.Schema) bool { - if schemaHasNull(s) { - return true +// +// It never forbids. A union with no null branch says nothing about a null a +// sibling keyword admits — `{nullable: true, oneOf: [...]}` is the 3.0 spelling +// of a nullable union, and a 3.1 `{type: [X, "null"]}` beside a union is the +// same statement. +func unionNullVerdict(s *oas3.Schema) nullVerdict { + if oneOfAnyOfHasNull(s) && !hasUnionSiblings(s) { + return nullAdmitted + } + return nullSilent +} + +// allOfNullVerdict conjoins what the allOf branches say. A conjunction admits +// null when a branch does and none forbids it, which is what makes +// `{allOf: [{$ref: T}]}` answer the same as `{$ref: T}` — the composition +// declares no nullability of its own, and the usage naming it has nowhere else +// to derive the bit from (GitHub #279). Model.Base and Mixins still carry no +// Nullable bit: they name a conjunct, and nullability is a property of the +// usage that names the conjunction. +func allOfNullVerdict(s *oas3.Schema, budget *int) nullVerdict { + out := nullSilent + for _, b := range s.GetAllOf() { + out = foldNullVerdicts(out, conjunctNullVerdict(b, budget)) + } + return out +} + +// conjunctNullVerdict reads one allOf branch. A `false` branch admits no +// instance whatever, null included; a `true` branch constrains nothing. +func conjunctNullVerdict(b *oas3.JSONSchema[oas3.Referenceable], budget *int) nullVerdict { + if b == nil { + return nullSilent + } + if b.IsBool() { + if v := b.GetBool(); v != nil && !*v { + return nullForbidden + } + return nullSilent + } + s := b.GetSchema() + if resolve.IsRefSite(b, s) { + return refNullVerdict(b, budget) + } + return schemaNullVerdict(s, budget) +} + +// refNullVerdict reads a $ref usage: the reference site or its resolved target +// admitting null is enough, since a site's keywords widen the referent as often +// as they narrow it (3.0 writes `{$ref: T, nullable: true}` for exactly that). +// Only when neither admits does a forbidding side decide. +// +// The ref site must be read at all because a target interned at its own ID — a +// model, a union — discards the TypeRef its definition produced, so the bit +// survives nowhere else. +func refNullVerdict(js *oas3.JSONSchema[oas3.Referenceable], budget *int) nullVerdict { + site := schemaNullVerdict(js.GetSchema(), budget) + if site == nullAdmitted { + return nullAdmitted + } + var target nullVerdict + if resolved := js.GetResolvedSchema(); resolved != nil { + target = schemaNullVerdict(resolved.GetSchema(), budget) + } + if target == nullAdmitted { + return nullAdmitted } - return oneOfAnyOfHasNull(s) && !hasUnionSiblings(s) + return foldNullVerdicts(site, target) } // nullUnionCollapse detects a oneOf/anyOf that has exactly one non-null branch @@ -1723,14 +2196,14 @@ func schemaAdmitsNull(s *oas3.Schema) bool { // (ir-design §3.3). A set with two or more non-null branches falls through to a // Union (with its null branches stripped and lifted onto the enclosing ref). // -// The hint it returns for the surviving branch is the *enclosing* schema's, -// while an outside $ref to that same branch pointer derives variant_ -// through subSchemaHint — so which of the two lowerings reaches the pointer -// first decides the name, and the two declaration orders produce different -// documents. That predates this function's co-declaration rule and is #181's -// mechanism at a site #181 did not sweep; GitHub #281 holds it. It is narrowed -// but not settled here: declining the collapse below removes the one order in -// which a co-declared anyOf could reach it. +// The hint is the branch's own (branchHint), not the enclosing schema's. The +// pointer returned is the branch's, so an outside $ref can name it too and +// derives its hint through subSchemaHint; only the first lowering to arrive +// interns the node, so a hint derived differently here made the document depend +// on declaration order — silently, since either spelling is a valid hint. That +// was #181's mechanism at a site #181 did not sweep (GitHub #281), and the +// repair is #181's: both paths ask branchHint's question, so they agree whichever +// arrives first. // // A schema declaring both combinators collapses neither. The collapse says the // position *is* nullable X, and a co-declared anyOf conjoins with it, so it is @@ -1739,7 +2212,7 @@ func schemaAdmitsNull(s *oas3.Schema) bool { // (preserveUnusedCombinator). Collapsing here would resolve the position // straight to X's own node — a shared primitive for `{type: string}` — leaving // the loser nowhere to sit that is not shared with every other declaration of X. -func nullUnionCollapse(s *oas3.Schema, pointer, hint string) (*oas3.JSONSchema[oas3.Referenceable], string, string, bool) { +func nullUnionCollapse(s *oas3.Schema, pointer string) (*oas3.JSONSchema[oas3.Referenceable], string, string, bool) { if len(s.GetOneOf()) > 0 && len(s.GetAnyOf()) > 0 { return nil, "", "", false } @@ -1757,7 +2230,7 @@ func nullUnionCollapse(s *oas3.Schema, pointer, hint string) (*oas3.JSONSchema[o if nullCount == 0 || nonNullCount != 1 { return nil, "", "", false } - return nonNull, pointer + ids.Ptr(key, strconv.Itoa(nonNullIdx)), hint, true + return nonNull, pointer + ids.Ptr(key, strconv.Itoa(nonNullIdx)), branchHint(nonNull, nonNullIdx), true } // isNullSchema reports whether a variant schema is the bare null-typed schema. diff --git a/compilers/openapi/internal/schema/schema_internal_test.go b/compilers/openapi/internal/schema/schema_internal_test.go index 7594173..e7ea9fa 100644 --- a/compilers/openapi/internal/schema/schema_internal_test.go +++ b/compilers/openapi/internal/schema/schema_internal_test.go @@ -17,6 +17,8 @@ import ( "github.com/dexpace/morphic/compilers/openapi/internal/diag" "github.com/dexpace/morphic/compilers/openapi/internal/ids" "github.com/dexpace/morphic/compilers/openapi/internal/lowering" + "github.com/dexpace/morphic/compilers/openapi/internal/nodeview" + "github.com/dexpace/morphic/compilers/openapi/internal/openapitest" "github.com/dexpace/morphic/ir" ) @@ -31,7 +33,7 @@ func TestLower_DepthCapExceeded(t *testing.T) { indent += " " } b.WriteString(indent + "type: string\n") - doc, diags := lowerSpec(t, componentSpec(b.String())) + doc, diags := lowerSpec(t, openapitest.ComponentSpec(b.String())) require.NotNil(t, doc) var sawCap bool for _, d := range diags { @@ -44,7 +46,7 @@ func TestLower_DepthCapExceeded(t *testing.T) { func TestIsNullSchema_EmptyEitherFalse(t *testing.T) { t.Parallel() - assert.False(t, isNullSchema(emptyEitherSchema()), "empty either is not a null schema") + assert.False(t, isNullSchema(openapitest.EmptyEitherSchema()), "empty either is not a null schema") } func TestPreserveUnionSiblings_MissingNode(t *testing.T) { @@ -91,9 +93,11 @@ func TestSchemaConstraints_NonSchemaInputs(t *testing.T) { annotation.SchemaOf(oas3.NewJSONSchemaFromBool(true)), annotation.SchemaOf(oas3.NewJSONSchemaFromReference("#/components/schemas/Other")), } { - cons, diags := schemaConstraints(l.ctx, js, "/p") + var kept ir.Unmodeled + cons, diags := schemaConstraints(l.ctx, &kept, js, "/p") assert.Nil(t, cons) assert.Empty(t, diags) + assert.Empty(t, kept) } } @@ -107,9 +111,11 @@ func TestSchemaConstraints_EmptyRefSchema(t *testing.T) { l := newRawLowerer(&soa.OpenAPI{}) emptyRef := references.Reference("") js := oas3.NewJSONSchemaFromSchema[oas3.Referenceable](&oas3.Schema{Ref: &emptyRef}) - cons, diags := schemaConstraints(l.ctx, annotation.SchemaOf(js), "/p") + var kept ir.Unmodeled + cons, diags := schemaConstraints(l.ctx, &kept, annotation.SchemaOf(js), "/p") assert.Nil(t, cons) assert.Empty(t, diags) + assert.Empty(t, kept) } func TestResolveSchemaRef_ReusesInternedSubSchema(t *testing.T) { @@ -117,7 +123,7 @@ func TestResolveSchemaRef_ReusesInternedSubSchema(t *testing.T) { l := newRawLowerer(&soa.OpenAPI{}) l.types.Intern(deepPointer, "t/anon/prev", func() ir.TypeDef { return &ir.Any{} }) - id, ok, diags := resolveSchemaRef(l.ctx, l.types, &l.anchors, TopLevelDepth, emptyEitherSchema(), "#"+deepPointer) + id, ok, diags := resolveSchemaRef(l.ctx, l.types, &l.anchors, TopLevelDepth, openapitest.EmptyEitherSchema(), "#"+deepPointer) require.True(t, ok, "a $ref to an already-hoisted sub-schema reuses its ID") assert.Equal(t, ir.TypeID("t/anon/prev"), id) assert.Empty(t, diags, "reusing an interned node reports nothing") @@ -208,7 +214,7 @@ func TestDeclaresResourceIDAbove_WithoutARawTree(t *testing.T) { t.Parallel() l := newRawLowerer(&soa.OpenAPI{}) - assert.False(t, declaresResourceIDAbove(l.ctx, "/components/schemas/A"), + assert.False(t, declaresResourceIDAbove(l.ctx, nodeview.New(), "/components/schemas/A"), "a document with no raw tree declares no resource anywhere") } @@ -218,11 +224,18 @@ func TestDeclaresResourceIDAbove_WithoutARawTree(t *testing.T) { // is the token naming the key "", which is how a schema literally named "" is // addressed. Dropping those stopped the walk above the position and read the // $id of the components/schemas map instead of the schema's own. +// +// The document root carries a member named "" of its own, and it is what makes +// the two rootward cases discriminating rather than decorative: both must read +// as "the root and nothing below it", so a walk that took "" for a token would +// descend into that member and find the $id parked there. Without it, "stopped +// at the root" and "descended and fell off the tree" are the same false. func TestDeclaresResourceIDAbove_EmptySegmentIsATokenNotAnArtifact(t *testing.T) { t.Parallel() l, diags := loweredFor(t, `openapi: 3.1.0 info: {title: T, version: "1"} paths: {} +"": {$id: https://example.com/root-member} components: schemas: "": @@ -230,20 +243,37 @@ components: properties: {x: {type: string}} Sibling: {type: string} `) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) for name, tc := range map[string]struct { pointer string want bool + why string }{ - "the position is named by a trailing empty token": {"/components/schemas/", true}, - "an interior empty token still descends": {"/components/schemas//properties/x", true}, - "a sibling sits above no $id at all": {"/components/schemas/Sibling", false}, - "the empty pointer names the document root": {"", false}, + "the position is named by a trailing empty token": { + "/components/schemas/", true, + "the trailing token names the schema itself, whose own $id is the boundary", + }, + "an interior empty token still descends": { + "/components/schemas//properties/x", true, + "the $id above a property is still a boundary over it", + }, + "a sibling sits above no $id at all": { + "/components/schemas/Sibling", false, + "a neighbour of the named schema inherits no boundary from it", + }, + "the empty pointer names the document root": { + "", false, + `the empty pointer stops at the root, so the $id under the root's "" member is out of reach`, + }, + "a lone slash names the document root too": { + "/", false, + `the resolver lands a lone slash on the root, so the $id under the root's "" member stays out of reach`, + }, } { t.Run(name, func(t *testing.T) { t.Parallel() - assert.Equal(t, tc.want, declaresResourceIDAbove(l.ctx, tc.pointer)) + assert.Equal(t, tc.want, declaresResourceIDAbove(l.ctx, nodeview.New(), tc.pointer), tc.why) }) } } @@ -264,32 +294,32 @@ func TestDynamicAnchors_WalksEveryNodeShape(t *testing.T) { {"a nil node yields nothing", nil, map[string][]string{}}, { "a bare scalar declares no anchor", - yamlNode(t, `just-a-string`), + openapitest.YAMLNode(t, `just-a-string`), map[string][]string{}, }, { "a sequence indexes its elements by ordinal", - yamlNode(t, "- {$dynamicAnchor: first}\n- {other: 1}\n- {$dynamicAnchor: third}\n"), + openapitest.YAMLNode(t, "- {$dynamicAnchor: first}\n- {other: 1}\n- {$dynamicAnchor: third}\n"), map[string][]string{"first": {"/0"}, "third": {"/2"}}, }, { "a sequence element standing in for a mapping is followed", - yamlNode(t, "- &a {$dynamicAnchor: first}\n- *a\n"), + openapitest.YAMLNode(t, "- &a {$dynamicAnchor: first}\n- *a\n"), map[string][]string{"first": {"/0", "/1"}}, }, { "a non-string key cannot name a keyword and is skipped", - yamlNode(t, "? [a, b]\n: {$dynamicAnchor: buried}\n$dynamicAnchor: reached\n"), + openapitest.YAMLNode(t, "? [a, b]\n: {$dynamicAnchor: buried}\n$dynamicAnchor: reached\n"), map[string][]string{"reached": {""}}, }, { "an empty anchor name is not indexed", - yamlNode(t, `{$dynamicAnchor: ""}`), + openapitest.YAMLNode(t, `{$dynamicAnchor: ""}`), map[string][]string{}, }, { "a non-scalar anchor value is not indexed", - yamlNode(t, `{$dynamicAnchor: [a]}`), + openapitest.YAMLNode(t, `{$dynamicAnchor: [a]}`), map[string][]string{}, }, } @@ -334,7 +364,7 @@ func TestDynamicAnchors_CountsWhatAnAliasBringsIn(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { t.Parallel() - got, complete := dynamicAnchors(yamlNode(t, tc.source)) + got, complete := dynamicAnchors(openapitest.YAMLNode(t, tc.source)) assert.True(t, complete) assert.Equal(t, tc.want, got["tail"]) }) @@ -379,7 +409,7 @@ func TestDynamicAnchors_StopsAtTheDepthCap(t *testing.T) { // many more paths than the tree has nodes; the budget is what caps the total. func TestAnchorWalk_StopsAtTheNodeBudget(t *testing.T) { t.Parallel() - source := yamlNode(t, "a: {$dynamicAnchor: first}\nb: {$dynamicAnchor: second}\n") + source := openapitest.YAMLNode(t, "a: {$dynamicAnchor: first}\nb: {$dynamicAnchor: second}\n") w := newAnchorWalk(2) // the root mapping and its first value, and no more w.walk(source, "", 0) @@ -397,8 +427,8 @@ func TestDynamicAnchorIndex_ReportsATruncatedWalk(t *testing.T) { // The walk descends into every key, so an extension at the document root // nests the tree past the cap without the schema lowering ever seeing it. deep := strings.Repeat("{a: ", maxDynamicAnchorDepth) + "1" + strings.Repeat("}", maxDynamicAnchorDepth) - l, diags := loweredFor(t, componentSpec(" A: {type: string}\n")+"x-deep: "+deep+"\n") - requireNoErrorDiags(t, diags) + l, diags := loweredFor(t, openapitest.ComponentSpec(" A: {type: string}\n")+"x-deep: "+deep+"\n") + openapitest.RequireNoErrorDiags(t, diags) _, got := l.anchors.sites(l.ctx, "absent") require.NotNil(t, l.anchors.byName, "the index is built even when partial") @@ -429,7 +459,7 @@ func TestPreserveUnhomedKeywords_MissingNode(t *testing.T) { func TestRecordUnhomedKeywords_MissingOwner(t *testing.T) { t.Parallel() l := newRawLowerer(&soa.OpenAPI{}) - diags := recordUnhomedKeywords(l.ctx, l.types, "t/anon/missing", &oas3.Schema{}, []string{"items"}, ir.KindPrimitive, "/p") + diags := recordUnhomedKeywords(l.ctx, l.types, "t/anon/missing", &oas3.Schema{}, []string{"items"}, nodeShape(ir.KindPrimitive), "/p") assertInternalInvariant(t, diags) } @@ -482,6 +512,43 @@ func TestRefNullable_AnUnresolvedRefIsNotNullable(t *testing.T) { assert.False(t, refNullable(js)) } +// TestSchemaNullVerdict_TheConjunctWalkIsBounded pins the budget the conjunct +// walk runs on. Whether a schema admits null is decided partly by its allOf +// conjuncts, each reached through a $ref whose target is asked the same +// question, so a schema conjoining itself would otherwise not terminate. +// +// A budget spent per schema visited caps depth as well as breadth, and an +// exhausted walk answers "silent" — the verdict that claims the least, so a +// spec too deep to read is never reported as admitting a null it does not. +func TestSchemaNullVerdict_TheConjunctWalkIsBounded(t *testing.T) { + t.Parallel() + nullable := &oas3.Schema{ + Type: oas3.NewTypeFromArray([]oas3.SchemaType{oas3.SchemaTypeString, oas3.SchemaTypeNull}), + } + + budget := 1 + require.Equal(t, nullAdmitted, schemaNullVerdict(nullable, &budget), + "the fixture admits null while there is budget to read it") + + spent := 0 + assert.Equal(t, nullSilent, schemaNullVerdict(nullable, &spent), + "an exhausted budget stops the walk without claiming anything") + assert.Positive(t, maxNullConjuncts, "the cap is a real bound, not zero") +} + +// TestConjunctNullVerdict_ABranchWithNoSchemaSaysNothing pins the guard on an +// absent allOf entry. The parser never produces a nil branch, so nothing in the +// corpus reaches it; a conjunct that is not there constrains nothing, which is +// silence rather than a refusal — reading it as forbidding would let one +// missing entry strip a sibling's null. +func TestConjunctNullVerdict_ABranchWithNoSchemaSaysNothing(t *testing.T) { + t.Parallel() + budget := maxNullConjuncts + assert.Equal(t, nullSilent, conjunctNullVerdict(nil, &budget)) + assert.Equal(t, nullSilent, conjunctNullVerdict(openapitest.EmptyEitherSchema(), &budget), + "a branch whose either-value holds neither schema nor bool says nothing either") +} + // TestComponentSchemaAt_OnlyATopLevelComponentPointerHasABody pins the split // the function exists for: a component pointer has a body, and a pointer into // that same component does not. @@ -495,11 +562,11 @@ func TestRefNullable_AnUnresolvedRefIsNotNullable(t *testing.T) { // the wrong answer visible: it is a name like any other here. func TestComponentSchemaAt_OnlyATopLevelComponentPointerHasABody(t *testing.T) { t.Parallel() - l, diags := loweredFor(t, componentSpec( + l, diags := loweredFor(t, openapitest.ComponentSpec( " Outer:\n type: object\n title: outer\n"+ " properties: {inner: {type: string, title: inner}}\n"+ " \"\": {type: string, title: empty}\n")) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) tests := []struct { name, pointer, wantTitle string @@ -578,8 +645,8 @@ func TestDynamicHop_HopsOnlyWhenExactlyOneAnchorSiteIsNamed(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { t.Parallel() - l, diags := loweredFor(t, componentSpec(tc.schemas)) - requireNoErrorDiags(t, diags) + l, diags := loweredFor(t, openapitest.ComponentSpec(tc.schemas)) + openapitest.RequireNoErrorDiags(t, diags) if tc.anchor != "" { sites, siteDiags := l.anchors.sites(l.ctx, tc.anchor) require.Len(t, sites, tc.wantSites, "the fixture must set up the case it is named for") @@ -594,3 +661,45 @@ func TestDynamicHop_HopsOnlyWhenExactlyOneAnchorSiteIsNamed(t *testing.T) { }) } } + +// TestKeywordHome_AnUnknownKeywordIsHomedNowhere reaches the arm censusKeywords +// cannot reach today, the way TestDeclaresFamily_AnUnknownNameDeclaresNothing +// reaches its own: every keyword unhomedKeywords asks about is one with a case +// here. +// +// "No home" is the safe half of that default. A keyword added to censusKeywords +// without an arm is kept verbatim beside every node it is written on — noisy, +// never lossy — where the opposite default would drop it in exactly the silence +// GitHub #268 was. +func TestKeywordHome_AnUnknownKeywordIsHomedNowhere(t *testing.T) { + t.Parallel() + assert.False(t, keywordHome(&ir.Model{}, &oas3.Schema{}, "unevaluatedItems"), + "a keyword with no arm has no home on any node") +} + +// TestKeywordHome_EveryCensusKeywordHasANodeThatCarriesIt pins the agreement +// censusKeywords and keywordHome's switch have to keep, in the direction the +// default cannot fake. +// +// A listed keyword with no arm answers "homeless" everywhere, and that is the +// answer a $ref site expects from every one of them, so no ref-site test can see +// the omission. Asking instead for the node kind that *does* carry the keyword +// is what makes it visible. +func TestKeywordHome_EveryCensusKeywordHasANodeThatCarriesIt(t *testing.T) { + t.Parallel() + // A declared scalar type, which is what homes `format` on a primitive and + // `type` on anything at all. + s := &oas3.Schema{Type: oas3.NewTypeFromString(oas3.SchemaTypeString)} + nodes := []ir.TypeDef{ + &ir.Model{}, &ir.List{}, &ir.Tuple{}, &ir.Literal{}, &ir.Enum{}, &ir.Union{}, + &ir.Scalar{Encoding: &ir.Encoding{}}, &ir.Primitive{}, + } + require.NotEmpty(t, censusKeywords, "an empty census would make this pass vacuously") + for _, keyword := range censusKeywords { + homed := false + for _, td := range nodes { + homed = homed || keywordHome(td, s, keyword) + } + assert.True(t, homed, "%q is in the census with no node kind that carries it", keyword) + } +} diff --git a/compilers/openapi/internal/schema/schema_test.go b/compilers/openapi/internal/schema/schema_test.go index d05980f..2175fd3 100644 --- a/compilers/openapi/internal/schema/schema_test.go +++ b/compilers/openapi/internal/schema/schema_test.go @@ -2,6 +2,7 @@ package schema_test import ( "encoding/json" + "sort" "strings" "testing" @@ -16,6 +17,7 @@ import ( "github.com/dexpace/morphic/compilers/openapi/internal/annotation" "github.com/dexpace/morphic/compilers/openapi/internal/diag" "github.com/dexpace/morphic/compilers/openapi/internal/lowering" + "github.com/dexpace/morphic/compilers/openapi/internal/openapitest" "github.com/dexpace/morphic/compilers/openapi/internal/overlay" "github.com/dexpace/morphic/compilers/openapi/internal/schema" "github.com/dexpace/morphic/ir" @@ -46,7 +48,7 @@ func TestSchemaRef_NullableNormalization(t *testing.T) { " properties:\n" + " p: " + tc.schema + "\n" doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) model, ok := doc.Types[componentID("S")].(*ir.Model) require.True(t, ok) require.Len(t, model.Properties, 1) @@ -60,14 +62,14 @@ func TestLower_NamedScalarComponentResolves(t *testing.T) { t.Parallel() // A named component whose body is a plain scalar must register a resolvable // node at its own component pointer, so a $ref to it never dangles. - spec := componentSpec(` MyId: {type: string, format: uuid} + spec := openapitest.ComponentSpec(` MyId: {type: string, format: uuid} Holder: type: object properties: id: {$ref: "#/components/schemas/MyId"} `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) scalar, ok := doc.Types[componentID("MyId")].(*ir.Scalar) require.True(t, ok, "named scalar component registers a Scalar at its own ID") @@ -91,7 +93,7 @@ func TestLower_ConstraintOnlyUnionIsValidationOnly(t *testing.T) { // logic, not shape — dependentRequired's sibling — so the structural body // survives and the union is preserved under ReasonValidationOnly, the reason // a validation emitter selects on (ir-design §4.7). - spec := componentSpec(` Thing: + spec := openapitest.ComponentSpec(` Thing: type: object additionalProperties: false required: [common] @@ -102,7 +104,7 @@ func TestLower_ConstraintOnlyUnionIsValidationOnly(t *testing.T) { - {required: [b]} `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m, ok := doc.Types[componentID("Thing")].(*ir.Model) require.True(t, ok, "structural body lowers to a Model, not a bare Union") @@ -116,7 +118,7 @@ func TestLower_ConstraintOnlyUnionIsValidationOnly(t *testing.T) { assert.Equal(t, ir.ReasonValidationOnly, raw.Reason, "constraint-only branches narrow the body without reshaping it (ir-design §4.7)") assert.Equal(t, "/components/schemas/Thing/oneOf", raw.Provenance.Pointer) - assert.Equal(t, 1, countDiagsAt(diags, diag.ValidationOnlyKeyword, ir.SeverityInfo), + assert.Equal(t, 1, openapitest.CountDiagsAt(diags, diag.ValidationOnlyKeyword, ir.SeverityInfo), "the union is reported with §4.7's keyword family; got %+v", diags) } @@ -128,7 +130,7 @@ func TestLower_BooleanUnionBranchDeclaresNoShape(t *testing.T) { // as a constraint-only branch does, and takes the same validation-only // lowering. The branch carries no schema object at all, which is what // separates it from a branch that declares nothing structural. - spec := componentSpec(` Flag: + spec := openapitest.ComponentSpec(` Flag: type: object required: [common] properties: @@ -138,7 +140,7 @@ func TestLower_BooleanUnionBranchDeclaresNoShape(t *testing.T) { - false `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m, ok := doc.Types[componentID("Flag")].(*ir.Model) require.True(t, ok, "the structural body survives as a Model") @@ -155,7 +157,7 @@ func TestLower_BooleanUnionBranchDeclaresNoShape(t *testing.T) { func TestLower_AllOfWithOneOfKeepsBoth(t *testing.T) { t.Parallel() // allOf co-declared with oneOf must not drop the allOf composition. - spec := componentSpec(` Base: + spec := openapitest.ComponentSpec(` Base: type: object properties: id: {type: string} @@ -167,7 +169,7 @@ func TestLower_AllOfWithOneOfKeepsBoth(t *testing.T) { - {type: integer} `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m, ok := doc.Types[componentID("Combo")].(*ir.Model) require.True(t, ok, "allOf composition survives (Model), oneOf preserved raw") require.NotNil(t, m.Base, "the allOf $ref becomes Base") @@ -178,13 +180,13 @@ func TestLower_AllOfWithOneOfKeepsBoth(t *testing.T) { func TestLower_RecursiveSchemaTerminates(t *testing.T) { t.Parallel() - spec := componentSpec(` Node: + spec := openapitest.ComponentSpec(` Node: type: object properties: next: {$ref: "#/components/schemas/Node"} `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) node, ok := doc.Types[componentID("Node")].(*ir.Model) require.True(t, ok) require.Equal(t, ir.TypeRef{Target: "t/openapi/components/schemas/Node"}, node.Properties[0].Type) @@ -192,7 +194,7 @@ func TestLower_RecursiveSchemaTerminates(t *testing.T) { func TestLower_InlineSchemaHoistedOnce(t *testing.T) { t.Parallel() - spec := componentSpec(` S: + spec := openapitest.ComponentSpec(` S: type: object properties: tags: @@ -203,7 +205,7 @@ func TestLower_InlineSchemaHoistedOnce(t *testing.T) { name: {type: string} `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) itemsID := ir.TypeID("t/anon/components/schemas/S/properties/tags/items") item, ok := doc.Types[itemsID].(*ir.Model) require.True(t, ok, "items object should be hoisted as a model") @@ -214,7 +216,7 @@ func TestLower_InlineSchemaHoistedOnce(t *testing.T) { func TestSchemaRef_BooleanAndUntypedShapes(t *testing.T) { t.Parallel() - spec := componentSpec(` S: + spec := openapitest.ComponentSpec(` S: type: object properties: anything: true @@ -223,9 +225,9 @@ func TestSchemaRef_BooleanAndUntypedShapes(t *testing.T) { withprops: {properties: {x: {type: string}}} `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m := typeByName(doc, "S").(*ir.Model) - byWire := propsByWire(m.Properties) + byWire := openapitest.PropsByWire(m.Properties) assert.Equal(t, ir.TypeID("t/prim/any"), byWire["anything"].Type.Target) // `false` schema lowered to a closed empty model. nothing := doc.Types[byWire["nothing"].Type.Target] @@ -235,18 +237,18 @@ func TestSchemaRef_BooleanAndUntypedShapes(t *testing.T) { assert.Equal(t, ir.TypeID("t/prim/any"), byWire["untyped"].Type.Target) assert.Equal(t, ir.KindModel, doc.Types[byWire["withprops"].Type.Target].Kind()) - assert.True(t, hasDiagAt(diags, diag.FalseSchema, ir.SeverityInfo), "false schema info diagnostic") + assert.True(t, openapitest.HasDiagAt(diags, diag.FalseSchema, ir.SeverityInfo), "false schema info diagnostic") } func TestLower_MultiTypeUnion(t *testing.T) { t.Parallel() - spec := componentSpec(` MT: + spec := openapitest.ComponentSpec(` MT: type: [object, array, string] properties: {x: {type: string}} items: {type: integer} `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) u, ok := typeByName(doc, "MT").(*ir.Union) require.True(t, ok) require.Len(t, u.Variants, 3) @@ -255,7 +257,7 @@ func TestLower_MultiTypeUnion(t *testing.T) { func TestScalar_UnknownFormatPerBaseType(t *testing.T) { t.Parallel() - spec := componentSpec(` S: + spec := openapitest.ComponentSpec(` S: type: object properties: i: {type: integer, format: weird} @@ -264,7 +266,7 @@ func TestScalar_UnknownFormatPerBaseType(t *testing.T) { s: {type: string, format: weird} `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m := typeByName(doc, "S").(*ir.Model) bases := map[string]ir.PrimKind{} for _, p := range m.Properties { @@ -282,13 +284,13 @@ func TestScalar_UnknownFormatPerBaseType(t *testing.T) { func TestLower_TupleWithTrailingItems(t *testing.T) { t.Parallel() - spec := componentSpec(` Tup: + spec := openapitest.ComponentSpec(` Tup: type: array prefixItems: [{type: string}, {type: integer}] items: {type: boolean} `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) tup, ok := typeByName(doc, "Tup").(*ir.Tuple) require.True(t, ok) require.Len(t, tup.Elems, 2) @@ -315,7 +317,7 @@ func hasDegradedDiag(diags []ir.Diagnostic, want string) bool { func TestLower_ListConstraints(t *testing.T) { t.Parallel() - spec := componentSpec(` L: + spec := openapitest.ComponentSpec(` L: type: array items: {type: string} minItems: 1 @@ -323,7 +325,7 @@ func TestLower_ListConstraints(t *testing.T) { uniqueItems: true `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) l, ok := typeByName(doc, "L").(*ir.List) require.True(t, ok) require.NotNil(t, l.Constraints) @@ -335,8 +337,8 @@ func TestLower_ListConstraints(t *testing.T) { func TestLower_ListWithoutItems(t *testing.T) { t.Parallel() // No `items` → schema.Ref(nil) → element is `any`. - doc, diags := lowerSpec(t, componentSpec(" L: {type: array}\n")) - requireNoErrorDiags(t, diags) + doc, diags := lowerSpec(t, openapitest.ComponentSpec(" L: {type: array}\n")) + openapitest.RequireNoErrorDiags(t, diags) l, ok := typeByName(doc, "L").(*ir.List) require.True(t, ok) assert.Equal(t, ir.TypeID("t/prim/any"), l.Elem.Target) @@ -344,7 +346,7 @@ func TestLower_ListWithoutItems(t *testing.T) { func TestLower_ValidationOnlyKeywords(t *testing.T) { t.Parallel() - spec := componentSpec(` V: + spec := openapitest.ComponentSpec(` V: type: object properties: {a: {type: string}} if: {required: [a]} @@ -358,7 +360,7 @@ func TestLower_ValidationOnlyKeywords(t *testing.T) { unevaluatedItems: {type: integer} `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m := typeByName(doc, "V").(*ir.Model) // An entry the source writes as one keyword is located at that keyword; the // three that synthesize several into one object fall back to the schema, @@ -375,7 +377,7 @@ func TestLower_ValidationOnlyKeywords(t *testing.T) { require.True(t, ok, "keyword %s preserved", key) assert.Equal(t, want, entry.Provenance.Pointer, "entry provenance for %s", key) } - assert.GreaterOrEqual(t, countDiagsAt(diags, diag.ValidationOnlyKeyword, ir.SeverityInfo), 5) + assert.GreaterOrEqual(t, openapitest.CountDiagsAt(diags, diag.ValidationOnlyKeyword, ir.SeverityInfo), 5) } // TestLower_PropertyNamesPreserved pins the whole §4.7 contract for @@ -383,13 +385,13 @@ func TestLower_ValidationOnlyKeywords(t *testing.T) { // emitter selects on, and the one info diagnostic naming the keyword (#117). func TestLower_PropertyNamesPreserved(t *testing.T) { t.Parallel() - spec := componentSpec(` Codes: + spec := openapitest.ComponentSpec(` Codes: type: object propertyNames: {type: string, pattern: "^[a-z]+$"} additionalProperties: {type: integer} `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m, ok := typeByName(doc, "Codes").(*ir.Model) require.True(t, ok) @@ -397,7 +399,7 @@ func TestLower_PropertyNamesPreserved(t *testing.T) { require.True(t, ok, "propertyNames kept verbatim; got %v", m.Unmodeled) assert.JSONEq(t, `{"type":"string","pattern":"^[a-z]+$"}`, string(entry.Value)) assert.Equal(t, ir.ReasonValidationOnly, entry.Reason) - assert.Equal(t, 1, countDiagsAt(diags, diag.ValidationOnlyKeyword, ir.SeverityInfo)) + assert.Equal(t, 1, openapitest.CountDiagsAt(diags, diag.ValidationOnlyKeyword, ir.SeverityInfo)) // The keyword constrains keys only: the map's value lowering is untouched. require.NotNil(t, m.AdditionalProps) @@ -407,7 +409,7 @@ func TestLower_PropertyNamesPreserved(t *testing.T) { func TestLower_PropertyDetailRichSchema(t *testing.T) { t.Parallel() - spec := componentSpec(` D: + spec := openapitest.ComponentSpec(` D: type: object externalDocs: {url: 'https://x', description: more} properties: @@ -421,7 +423,7 @@ func TestLower_PropertyDetailRichSchema(t *testing.T) { doc, diags := lowerSpec(t, spec) m := typeByName(doc, "D").(*ir.Model) assert.NotEmpty(t, m.Docs.ExternalDocs) - byWire := propsByWire(m.Properties) + byWire := openapitest.PropsByWire(m.Properties) require.NotNil(t, byWire["withXml"].XML) assert.Equal(t, "attribute", byWire["withXml"].XML.NodeType) assert.Equal(t, "urn:x", byWire["withXml"].XML.Namespace) @@ -439,7 +441,7 @@ func TestLower_PropertyDetailRichSchema(t *testing.T) { func TestLower_RefTargetDescriptionFallback(t *testing.T) { t.Parallel() - spec := componentSpec(` Owner: + spec := openapitest.ComponentSpec(` Owner: type: object properties: ref: {$ref: '#/components/schemas/Target'} @@ -448,26 +450,26 @@ func TestLower_RefTargetDescriptionFallback(t *testing.T) { description: target-desc `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m := typeByName(doc, "Owner").(*ir.Model) assert.Equal(t, "target-desc", m.Properties[0].Docs.Description) } func TestLower_UnresolvedRefDiagnostics(t *testing.T) { t.Parallel() - spec := componentSpec(` Owner: + spec := openapitest.ComponentSpec(` Owner: type: object properties: ghost: {$ref: '#/components/schemas/Ghost'} `) doc, diags := lowerSpec(t, spec) require.NotNil(t, doc) - assert.True(t, hasDiag(diags, diag.UnresolvedRef), "unresolved ref diagnostic emitted") + assert.True(t, openapitest.HasDiag(diags, diag.UnresolvedRef), "unresolved ref diagnostic emitted") } func TestLower_UnionWithStructuralSiblingVariants(t *testing.T) { t.Parallel() - spec := componentSpec(` A: + spec := openapitest.ComponentSpec(` A: type: object properties: {x: {type: string}} required: [x] @@ -484,7 +486,7 @@ func TestLower_UnionWithStructuralSiblingVariants(t *testing.T) { oneOf: [{$ref: '#/components/schemas/A'}, {type: integer}] `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) for _, name := range []string{"A", "B", "C", "D"} { td := typeByName(doc, name) require.NotNil(t, td, "type %s present", name) @@ -497,7 +499,7 @@ func TestLower_UnionWithStructuralSiblingVariants(t *testing.T) { func TestModel_FourOptionalityStates(t *testing.T) { t.Parallel() - spec := componentSpec(` S: + spec := openapitest.ComponentSpec(` S: type: object required: [reqPlain, reqNull] properties: @@ -507,11 +509,11 @@ func TestModel_FourOptionalityStates(t *testing.T) { optNull: {type: [string, "null"]} `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m, ok := doc.Types[componentID("S")].(*ir.Model) require.True(t, ok) require.Len(t, m.Properties, 4) - byName := propsByWire(m.Properties) + byName := openapitest.PropsByWire(m.Properties) assert.True(t, byName["reqPlain"].Required) assert.False(t, byName["reqPlain"].Type.Nullable) assert.True(t, byName["reqNull"].Required) @@ -524,7 +526,7 @@ func TestModel_FourOptionalityStates(t *testing.T) { func TestModel_ValidationOnlyKeywordPreserved(t *testing.T) { t.Parallel() - spec := componentSpec(` S: + spec := openapitest.ComponentSpec(` S: type: object properties: {a: {type: string}} not: {required: [b]} @@ -551,13 +553,13 @@ func TestModel_ValidationOnlyKeywordPreserved(t *testing.T) { func TestModel_DefaultBigLiteral(t *testing.T) { t.Parallel() - spec := componentSpec(` S: + spec := openapitest.ComponentSpec(` S: type: object properties: n: {type: integer, default: 9007199254740993} `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m := doc.Types[componentID("S")].(*ir.Model) require.NotNil(t, m.Properties[0].Default) assert.Equal(t, ir.ValueNumber, m.Properties[0].Default.Kind) @@ -569,13 +571,13 @@ func TestFillPropertyDetail_UnconvertibleExampleDiagnosed(t *testing.T) { // A custom tag is structurally unconvertible; the example must be skipped // (an example is an annotation, not a structural hole) but never silently — // the conversion error was previously discarded on the floor. - spec := componentSpec(" S:\n type: object\n properties:\n n:\n type: string\n example: !foo bar\n") + spec := openapitest.ComponentSpec(" S:\n type: object\n properties:\n n:\n type: string\n example: !foo bar\n") doc, diags := lowerSpec(t, spec) m, ok := doc.Types[componentID("S")].(*ir.Model) require.True(t, ok) assert.Empty(t, m.Properties[0].Examples, "the unconvertible example is skipped, not appended") - require.Equal(t, 1, countDiagsAt(diags, diag.DegradedConstruct, ir.SeverityWarning)) - d, ok := firstDegradedWarning(diags) + require.Equal(t, 1, openapitest.CountDiagsAt(diags, diag.DegradedConstruct, ir.SeverityWarning)) + d, ok := openapitest.FirstDegradedWarning(diags) require.True(t, ok) assert.Equal(t, "/components/schemas/S/properties/n/example", d.Provenance.Pointer) assert.Contains(t, d.Message, "example:") @@ -583,54 +585,82 @@ func TestFillPropertyDetail_UnconvertibleExampleDiagnosed(t *testing.T) { func TestModel_ReadOnlyWriteOnlyVisibility(t *testing.T) { t.Parallel() - spec := componentSpec(` S: + spec := openapitest.ComponentSpec(` S: type: object properties: r: {type: string, readOnly: true} w: {type: string, writeOnly: true} `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m := doc.Types[componentID("S")].(*ir.Model) - byName := propsByWire(m.Properties) + byName := openapitest.PropsByWire(m.Properties) assert.Equal(t, ir.Visibility{Only: []ir.Lifecycle{ir.LifecycleRead, ir.LifecycleDelete, ir.LifecycleQuery}}, byName["r"].Visibility) assert.Equal(t, ir.Visibility{Only: []ir.Lifecycle{ir.LifecycleCreate, ir.LifecycleUpdate}}, byName["w"].Visibility) } +// TestModel_ReadOnlyAndWriteOnlyTogetherAreVisibleNowhere pins the answer for a +// property whose schema writes both flags: they admit disjoint lifecycle sets, +// so the property is admitted by none. Declaring both used to lower exactly as +// declaring readOnly alone did, in silence, while the same pairing spread over +// two allOf branches already intersected to None and warned (GitHub #276). +func TestModel_ReadOnlyAndWriteOnlyTogetherAreVisibleNowhere(t *testing.T) { + t.Parallel() + spec := openapitest.ComponentSpec(` S: + type: object + properties: + both: {type: string, readOnly: true, writeOnly: true} + r: {type: string, readOnly: true} +`) + doc, diags := lowerSpec(t, spec) + openapitest.RequireNoErrorDiags(t, diags) + m := doc.Types[componentID("S")].(*ir.Model) + byName := openapitest.PropsByWire(m.Properties) + + assert.Equal(t, ir.Visibility{None: true}, byName["both"].Visibility) + assert.NotEqual(t, byName["r"].Visibility, byName["both"].Visibility, + "declaring both flags must not lower exactly as declaring readOnly alone does") + msg := openapitest.DiagMessageAt(t, diags, diag.DisjointVisibility, ir.SeverityWarning, + "/components/schemas/S/properties/both") + assert.Contains(t, msg, "writeOnly", "the report names the keyword that was being dropped") + assert.Equal(t, 1, openapitest.CountDiagsAt(diags, diag.DisjointVisibility, ir.SeverityWarning), + "the property declaring one flag is not reported") +} + func TestModel_PasswordFormatSecret(t *testing.T) { t.Parallel() - spec := componentSpec(` S: + spec := openapitest.ComponentSpec(` S: type: object properties: pw: {type: string, format: password} `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m := doc.Types[componentID("S")].(*ir.Model) assert.True(t, m.Properties[0].Secret) } func TestModel_AdditionalPropertiesFalseClosed(t *testing.T) { t.Parallel() - spec := componentSpec(` S: + spec := openapitest.ComponentSpec(` S: type: object properties: {a: {type: string}} additionalProperties: false `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m := doc.Types[componentID("S")].(*ir.Model) assert.Equal(t, ir.AdditionalClosed, m.Additional) } func TestModel_AdditionalPropertiesSchema(t *testing.T) { t.Parallel() - spec := componentSpec(` S: + spec := openapitest.ComponentSpec(` S: type: object additionalProperties: {type: integer} `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m := doc.Types[componentID("S")].(*ir.Model) require.NotNil(t, m.AdditionalProps) assert.Equal(t, ir.TypeID("t/prim/integer"), m.AdditionalProps.Value.Target) @@ -638,14 +668,14 @@ func TestModel_AdditionalPropertiesSchema(t *testing.T) { func TestModel_PatternPropertiesOrder(t *testing.T) { t.Parallel() - spec := componentSpec(` S: + spec := openapitest.ComponentSpec(` S: type: object patternProperties: "^x-": {type: string} "^y-": {type: integer} `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m := doc.Types[componentID("S")].(*ir.Model) require.NotNil(t, m.AdditionalProps) require.Len(t, m.AdditionalProps.Patterns, 2) @@ -655,26 +685,26 @@ func TestModel_PatternPropertiesOrder(t *testing.T) { func TestModel_UnevaluatedPropertiesClosedAfterComposition(t *testing.T) { t.Parallel() - spec := componentSpec(` S: + spec := openapitest.ComponentSpec(` S: type: object properties: {a: {type: string}} unevaluatedProperties: false `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m := doc.Types[componentID("S")].(*ir.Model) assert.Equal(t, ir.AdditionalClosedAfterComposition, m.Additional) } func TestModel_SchemaExtensionPreserved(t *testing.T) { t.Parallel() - spec := componentSpec(` S: + spec := openapitest.ComponentSpec(` S: type: object x-rate-limit: 100 properties: {a: {type: string}} `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m := doc.Types[componentID("S")].(*ir.Model) raw, ok := m.Unmodeled["openapi:x-rate-limit"] require.True(t, ok) @@ -686,14 +716,14 @@ func TestModel_SchemaExtensionPreserved(t *testing.T) { func TestModel_TitleDescriptionDocs(t *testing.T) { t.Parallel() - spec := componentSpec(` S: + spec := openapitest.ComponentSpec(` S: type: object title: "My Title" description: "My Desc" properties: {a: {type: string}} `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m := doc.Types[componentID("S")].(*ir.Model) assert.Equal(t, "My Title", m.Docs.Summary) assert.Equal(t, "My Desc", m.Docs.Description) @@ -701,26 +731,26 @@ func TestModel_TitleDescriptionDocs(t *testing.T) { func TestModel_PropertyDeprecation(t *testing.T) { t.Parallel() - spec := componentSpec(` S: + spec := openapitest.ComponentSpec(` S: type: object properties: old: {type: string, deprecated: true} `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m := doc.Types[componentID("S")].(*ir.Model) assert.NotNil(t, m.Properties[0].Deprecation) } func TestModel_PropertyXML(t *testing.T) { t.Parallel() - spec := componentSpec(` S: + spec := openapitest.ComponentSpec(` S: type: object properties: p: {type: string, xml: {name: n, attribute: true}} `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m := doc.Types[componentID("S")].(*ir.Model) require.NotNil(t, m.Properties[0].XML) assert.Equal(t, "n", m.Properties[0].XML.Name) @@ -729,7 +759,7 @@ func TestModel_PropertyXML(t *testing.T) { func TestModel_RefSiblingDescriptionWins(t *testing.T) { t.Parallel() - spec := componentSpec(` Target: {type: string, description: "target desc"} + spec := openapitest.ComponentSpec(` Target: {type: string, description: "target desc"} S: type: object properties: @@ -738,7 +768,7 @@ func TestModel_RefSiblingDescriptionWins(t *testing.T) { description: "sibling desc" `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m := doc.Types[componentID("S")].(*ir.Model) assert.Equal(t, "sibling desc", m.Properties[0].Docs.Description) } @@ -746,7 +776,7 @@ func TestModel_RefSiblingDescriptionWins(t *testing.T) { func TestSchemaRef_EmptyEitherIsAny(t *testing.T) { t.Parallel() l := newRawLowerer(&soa.OpenAPI{}) - ref, diags := schema.Ref(l.ctx, l.types, &l.anchors, schema.TopLevelDepth, emptyEitherSchema(), "/p", "h") + ref, diags := schema.Ref(l.ctx, l.types, &l.anchors, schema.TopLevelDepth, openapitest.EmptyEitherSchema(), "/p", "h") assert.Equal(t, ir.TypeID("t/prim/any"), ref.Target) assert.Empty(t, diags, "an empty either lowers to any without complaint") } @@ -775,7 +805,7 @@ components: in: query schema: {type: string, example: sub-example} `) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) td, ok := doc.Types["t/anon/components/parameters/P/schema"] require.True(t, ok, "the referenced sub-schema is hoisted at its own pointer") require.Len(t, td.Common().Examples, 1) @@ -804,7 +834,7 @@ components: $ref: '#/components/schemas/Base' example: alias-level `) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) alias, ok := doc.Types["t/openapi/components/schemas/Alias"] require.True(t, ok) require.Len(t, alias.Common().Examples, 1) @@ -827,7 +857,7 @@ func TestSiteSchema_BodylessPositions(t *testing.T) { func TestSchema_Ref30NullableSiblings(t *testing.T) { t.Parallel() - spec := componentSpecVer("3.0.3", ` Owner: + spec := openapitest.ComponentSpecVer("3.0.3", ` Owner: type: object properties: p: {$ref: '#/components/schemas/Target', nullable: true} @@ -1134,9 +1164,9 @@ func TestSchema_RefNullableAcrossSpellings(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { t.Parallel() - spec := componentSpecVer(tc.version, tc.schemas) + spec := openapitest.ComponentSpecVer(tc.version, tc.schemas) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m := typeByName(doc, "Owner").(*ir.Model) require.Len(t, m.Properties, 1) assert.Equal(t, tc.wantNullable, m.Properties[0].Type.Nullable, tc.msg) @@ -1146,6 +1176,57 @@ func TestSchema_RefNullableAcrossSpellings(t *testing.T) { } } +// TestSchema_NullabilityAgreesAcrossEnumSpellings pins that a value set and a +// type keyword conjoin, and that both ways of writing the same conjunction get +// the same answer. +// +// `{type: [string, "null"], enum: [red, green]}` and +// `{enum: [red, green], oneOf: [{type: string}, {type: "null"}]}` say one thing: +// null is in the type space and out of the value space, so the position does not +// admit it. The type-array spelling used to read the type keyword alone and call +// the position nullable while the oneOf spelling read the enum and called it +// not — two answers for one constraint in a single document (GitHub #288). +// +// Both members of a pair are written into one document on purpose: "the same +// schema, two spellings" is then a property of one compile rather than of two +// runs that could differ for unrelated reasons. The admitting pair is here for +// the same reason the excluding one is — a predicate hardcoded either way fails +// exactly one of them. +func TestSchema_NullabilityAgreesAcrossEnumSpellings(t *testing.T) { + t.Parallel() + spec := openapitest.ComponentSpec(` S: + type: object + properties: + excludedByType: {type: [string, "null"], enum: [red, green]} + excludedByUnion: {enum: [red, green], oneOf: [{type: string}, {type: "null"}]} + admittedByType: {type: [string, "null"], enum: [red, green, null]} + admittedByUnion: {enum: [red, green, null], oneOf: [{type: string}, {type: "null"}]} +`) + doc, diags := lowerSpec(t, spec) + openapitest.RequireNoErrorDiags(t, diags) + m, ok := typeByName(doc, "S").(*ir.Model) + require.True(t, ok, "S is a model") + props := openapitest.PropsByWire(m.Properties) + require.Len(t, props, 4) + + pairs := []struct { + name, typeSpelling, unionSpelling string + want bool + }{ + {"an enum listing no null member", "excludedByType", "excludedByUnion", false}, + {"an enum listing one", "admittedByType", "admittedByUnion", true}, + } + for _, p := range pairs { + t.Run(p.name, func(t *testing.T) { + t.Parallel() + got := props[p.typeSpelling].Type.Nullable + assert.Equal(t, p.want, got, "the enum decides whether the position admits null") + assert.Equal(t, got, props[p.unionSpelling].Type.Nullable, + "the type-array and oneOf spellings of one constraint must agree") + }) + } +} + // TestSchema_RefNullableMatchesInlineForUnionSiblings pins that one schema body // lowers to the same Nullable bit whether it is written inline or reached // through a $ref. The $ref site recomputes nullability, so it is the one place @@ -1161,7 +1242,7 @@ oneOf: [{type: string}, {type: "null"}]` pad := strings.Repeat(" ", n) return pad + strings.ReplaceAll(body, "\n", "\n"+pad) + "\n" } - spec := componentSpec(" Target:\n" + indent(6) + + spec := openapitest.ComponentSpec(" Target:\n" + indent(6) + ` Owner: type: object properties: @@ -1169,9 +1250,9 @@ oneOf: [{type: string}, {type: "null"}]` inline: ` + indent(10)) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m := typeByName(doc, "Owner").(*ir.Model) - props := propsByWire(m.Properties) + props := openapitest.PropsByWire(m.Properties) require.Len(t, props, 2) assert.Equal(t, props["inline"].Type.Nullable, props["viaRef"].Type.Nullable, @@ -1184,7 +1265,7 @@ oneOf: [{type: string}, {type: "null"}]` // $ref used as a list element, not just a model property. func TestSchema_RefNullableAtNonPropertyPosition(t *testing.T) { t.Parallel() - spec := componentSpec(` Owner: + spec := openapitest.ComponentSpec(` Owner: type: object properties: p: @@ -1194,7 +1275,7 @@ func TestSchema_RefNullableAtNonPropertyPosition(t *testing.T) { oneOf: [{type: string}, {type: integer}, {type: "null"}] `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) list, ok := doc.Types["t/anon/components/schemas/Owner/properties/p"].(*ir.List) require.True(t, ok) assert.True(t, list.Elem.Nullable, @@ -1204,7 +1285,7 @@ func TestSchema_RefNullableAtNonPropertyPosition(t *testing.T) { func TestSchema_UnionSiblingsAdditionalAndRequired(t *testing.T) { t.Parallel() - spec := componentSpec(` A: + spec := openapitest.ComponentSpec(` A: additionalProperties: {type: string} oneOf: [{type: string}, {type: integer}] B: @@ -1212,7 +1293,7 @@ func TestSchema_UnionSiblingsAdditionalAndRequired(t *testing.T) { oneOf: [{type: string}, {type: integer}] `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) for _, name := range []string{"A", "B"} { _, ok := typeByName(doc, name).Common().Unmodeled["openapi:oneOf"] assert.True(t, ok, "%s preserves its union", name) @@ -1221,28 +1302,28 @@ func TestSchema_UnionSiblingsAdditionalAndRequired(t *testing.T) { func TestSchema_RefTargetReadOnlyVisibility(t *testing.T) { t.Parallel() - spec := componentSpec(` Owner: + spec := openapitest.ComponentSpec(` Owner: type: object properties: p: {$ref: '#/components/schemas/RO'} RO: {type: string, readOnly: true} `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m := typeByName(doc, "Owner").(*ir.Model) assert.NotEmpty(t, m.Properties[0].Visibility.Only, "readOnly from the ref target applies") } func TestSchema_UnserializableExtension(t *testing.T) { t.Parallel() - spec := componentSpec(` S: + spec := openapitest.ComponentSpec(` S: type: object properties: {a: {type: string}} x-bad: {1: intkey} `) doc, diags := lowerSpec(t, spec) require.NotNil(t, doc) - assert.True(t, hasDiagAt(diags, diag.DegradedConstruct, ir.SeverityWarning), "unserializable extension warns") + assert.True(t, openapitest.HasDiagAt(diags, diag.DegradedConstruct, ir.SeverityWarning), "unserializable extension warns") m := typeByName(doc, "S").(*ir.Model) _, hasBad := m.Unmodeled["openapi:x-bad"] assert.False(t, hasBad, "unserializable extension is dropped, not stored") @@ -1250,14 +1331,14 @@ func TestSchema_UnserializableExtension(t *testing.T) { func TestSchema_EmptyFragmentRef(t *testing.T) { t.Parallel() - spec := componentSpec(` Owner: + spec := openapitest.ComponentSpec(` Owner: type: object properties: p: {$ref: '#'} `) doc, diags := lowerSpec(t, spec) require.NotNil(t, doc) - assert.True(t, hasDiag(diags, diag.UnresolvedRef), "the '#' ref form is unresolved") + assert.True(t, openapitest.HasDiag(diags, diag.UnresolvedRef), "the '#' ref form is unresolved") } func TestSchema_EmptyStringRefMirrorBranches(t *testing.T) { @@ -1265,7 +1346,7 @@ func TestSchema_EmptyStringRefMirrorBranches(t *testing.T) { // An empty-string $ref has IsReference()==false (the ref value is "") yet a // non-nil oas3 Ref pointer, exercising that mirror path in schema.Ref and the // branchHint fallback. - spec := componentSpec(` Owner: + spec := openapitest.ComponentSpec(` Owner: type: object properties: p: {$ref: ''} @@ -1276,7 +1357,7 @@ func TestSchema_EmptyStringRefMirrorBranches(t *testing.T) { `) doc, diags := lowerSpec(t, spec) require.NotNil(t, doc) - assert.GreaterOrEqual(t, countDiagsAt(diags, diag.UnresolvedRef, ir.SeverityError), 2, "both empty refs are unresolved") + assert.GreaterOrEqual(t, openapitest.CountDiagsAt(diags, diag.UnresolvedRef, ir.SeverityError), 2, "both empty refs are unresolved") u, ok := typeByName(doc, "U").(*ir.Union) require.True(t, ok) assert.Contains(t, []string{u.Variants[0].Name.Hint, u.Variants[1].Name.Hint}, "variant_0") @@ -1287,7 +1368,7 @@ func TestAllOf_UntypedRedeclarationDoesNotConflict(t *testing.T) { // One branch leaves the field schemaless (the top type), the other types it. // `any` intersects with everything under allOf, so this is a narrowing, not a // contradiction — it must not be reported. - spec := componentSpec(` Anyish: + spec := openapitest.ComponentSpec(` Anyish: allOf: - type: object properties: @@ -1297,7 +1378,7 @@ func TestAllOf_UntypedRedeclarationDoesNotConflict(t *testing.T) { id: {type: integer} `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m, ok := typeByName(doc, "Anyish").(*ir.Model) require.True(t, ok, "Anyish should be a model") require.Len(t, m.Properties, 1, "id reconciles to one property") @@ -1309,7 +1390,7 @@ func TestAllOf_EquivalentNumericBoundsDoNotConflict(t *testing.T) { t.Parallel() // The same bound spelled two ways (10 and 10.0) denotes one value, so it must // compare equal by magnitude and stay silent. - spec := componentSpec(` Boundish: + spec := openapitest.ComponentSpec(` Boundish: allOf: - type: object properties: @@ -1319,7 +1400,7 @@ func TestAllOf_EquivalentNumericBoundsDoNotConflict(t *testing.T) { n: {type: number, minimum: 10.0} `) _, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) assert.Empty(t, conflictDiags(diags), "10 and 10.0 are the same numeric bound, not a conflict") } @@ -1328,7 +1409,7 @@ func TestAllOf_DifferingNumericBoundsConflict(t *testing.T) { t.Parallel() // Two branches pin the same lower bound to different magnitudes; the kept // winner is arbitrary source order, so the dropped bound is surfaced. - spec := componentSpec(` Boundish: + spec := openapitest.ComponentSpec(` Boundish: allOf: - type: object properties: @@ -1338,7 +1419,7 @@ func TestAllOf_DifferingNumericBoundsConflict(t *testing.T) { n: {type: integer, minimum: 10} `) _, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) conflicts := conflictDiags(diags) require.Len(t, conflicts, 1, "differing numeric bounds are diagnosed once") assert.Contains(t, conflicts[0].Message, `"n"`) @@ -1347,7 +1428,7 @@ func TestAllOf_DifferingNumericBoundsConflict(t *testing.T) { func TestAllOf_ScalarVersusObjectRedeclarationConflicts(t *testing.T) { t.Parallel() // A scalar in one branch and a structural type in the other cannot both hold. - spec := componentSpec(` Mixed: + spec := openapitest.ComponentSpec(` Mixed: allOf: - type: object properties: @@ -1360,7 +1441,7 @@ func TestAllOf_ScalarVersusObjectRedeclarationConflicts(t *testing.T) { x: {type: string} `) _, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) conflicts := conflictDiags(diags) require.Len(t, conflicts, 1, "a scalar against an object is diagnosed once") assert.Contains(t, conflicts[0].Message, `"f"`) @@ -1372,7 +1453,7 @@ func TestAllOf_DistinctInlineObjectsDoNotConflict(t *testing.T) { // own model at its own pointer, so the targets differ — but two objects of the // same kind are not provably contradictory, and conflict detection never // guesses. - spec := componentSpec(` Objish: + spec := openapitest.ComponentSpec(` Objish: allOf: - type: object properties: @@ -1388,7 +1469,7 @@ func TestAllOf_DistinctInlineObjectsDoNotConflict(t *testing.T) { x: {type: string} `) _, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) assert.Empty(t, conflictDiags(diags), "two distinct inline objects for one field are not a provable conflict") } @@ -1397,7 +1478,7 @@ func TestAllOf_DistinctInlineObjectsDoNotConflict(t *testing.T) { // declare field v with the given flow-style schemas, for exercising per-keyword // redeclaration conflict detection. func allOfConflictSpec(schemaA, schemaB string) string { - return componentSpec( + return openapitest.ComponentSpec( " T:\n" + " allOf:\n" + " - type: object\n" + @@ -1408,13 +1489,14 @@ func allOfConflictSpec(schemaA, schemaB string) string { func TestAllOf_ConstraintAndFormatConflictsDiagnosed(t *testing.T) { t.Parallel() - // Each keyword class that can contradict across branches: an inclusive vs + // Each way a redeclaration can contradict across branches: an inclusive vs // exclusive bound of equal magnitude, a differing pattern, a differing - // multipleOf, and two format-derived primitives (string vs uuid). Each keeps - // the first declaration but surfaces exactly one conflict naming the field, - // both branch sites, and — for the constraint cases — the offending keyword - // with both of its conflicting values, so the author never has to diff both - // branches by hand. + // multipleOf, a bound and a multipleOf that differ past the magnitude a + // rational will build, and two format-derived primitives (string vs uuid). + // Each keeps the first declaration but surfaces exactly one conflict naming + // the field, both branch sites, and — for the constraint cases — the + // offending keyword with both of its conflicting values, so the author + // never has to diff both branches by hand. cases := []struct { name, a, b, wantDetail string }{ @@ -1436,6 +1518,23 @@ func TestAllOf_ConstraintAndFormatConflictsDiagnosed(t *testing.T) { b: "{type: integer, multipleOf: 3}", wantDetail: "conflicting multipleOf (2 and 3)", }, + { + // The other half of the past-a-rational rows in + // TestAllOf_CompatibleConstraintRedeclarationsStaySilent: a + // magnitude no rational holds still has to be ordered, not waved + // through, or the fix for the equal pair would just silence every + // pair that large. + name: "minimum too large for a rational", + a: "{type: number, minimum: 1e1000001}", + b: "{type: number, minimum: 2e1000001}", + wantDetail: "conflicting minimum (1e1000001 and 2e1000001)", + }, + { + name: "multipleOf too large for a rational", + a: "{type: number, multipleOf: 1e1000001}", + b: "{type: number, multipleOf: 1e1000002}", + wantDetail: "conflicting multipleOf (1e1000001 and 1e1000002)", + }, { name: "string vs uuid", a: "{type: string}", @@ -1449,7 +1548,7 @@ func TestAllOf_ConstraintAndFormatConflictsDiagnosed(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() _, diags := lowerSpec(t, allOfConflictSpec(tc.a, tc.b)) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) conflicts := conflictDiags(diags) require.Len(t, conflicts, 1, "%s is diagnosed exactly once", tc.name) assert.Contains(t, conflicts[0].Message, `"v"`, "the diagnostic names the field") @@ -1467,9 +1566,10 @@ func TestAllOf_CompatibleConstraintRedeclarationsStaySilent(t *testing.T) { // only one branch (both branches still carry constraints) is intersected, // not a conflict — and the merge must genuinely carry both keywords // forward on the reconciled property, not just stay quiet about the one it - // used to drop; equal multipleOf spelled two ways is one value; and an - // unknown-format scalar resolves through its Base to the same primitive as - // the plain type, so it is not a type conflict. + // used to drop; one magnitude spelled two ways is one value, whether or not + // it is one a rational will build; and an unknown-format scalar resolves + // through its Base to the same primitive as the plain type, so it is not a + // type conflict. cases := []struct { name string a, b string @@ -1514,13 +1614,27 @@ func TestAllOf_CompatibleConstraintRedeclarationsStaySilent(t *testing.T) { }, }, {name: "equivalent multipleOf", a: "{type: number, multipleOf: 2}", b: "{type: number, multipleOf: 2.0}"}, + // Both BigVal keywords at a magnitude math/big will not build as a + // rational. The value is the same on either branch, so there is nothing + // to report; a comparison that gave up on the magnitude instead would + // call one value two and fail a --fail-on warning build. + { + name: "equal minimum too large for a rational", + a: "{type: number, minimum: 1e1000001}", + b: "{type: number, minimum: 10e1000000}", + }, + { + name: "equal multipleOf too large for a rational", + a: "{type: number, multipleOf: 1e1000001}", + b: "{type: number, multipleOf: 10e1000000}", + }, {name: "custom format over base", a: "{type: string, format: weird}", b: "{type: string}"}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { t.Parallel() doc, diags := lowerSpec(t, allOfConflictSpec(tc.a, tc.b)) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) assert.Empty(t, conflictDiags(diags), "%s must not be reported as a conflict", tc.name) if tc.assertMerged == nil { return @@ -1538,7 +1652,7 @@ func TestAllOf_OpaqueScalarVsPrimitiveNoConflict(t *testing.T) { // An opaque scalar (format without a base type) is unknown, not structural, // so it's not provably incompatible with a primitive. The "never guess" // principle means we don't flag this as a conflict. - spec := componentSpec(` OpaqueTest: + spec := openapitest.ComponentSpec(` OpaqueTest: allOf: - type: object properties: @@ -1548,7 +1662,7 @@ func TestAllOf_OpaqueScalarVsPrimitiveNoConflict(t *testing.T) { id: {format: custom} `) _, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) assert.Empty(t, conflictDiags(diags), "opaque scalar vs primitive is not flagged as a conflict") } @@ -1558,7 +1672,7 @@ func TestAllOf_ThreeWayRedeclarationProducesTwoDiagnostics(t *testing.T) { // When three allOf branches declare the same field with different types, // reconciliation runs twice: branch[1] vs branch[0], then branch[2] vs branch[0]. // Each incompatible pair produces one diagnostic, so we expect two total. - spec := componentSpec(` ThreeWay: + spec := openapitest.ComponentSpec(` ThreeWay: allOf: - type: object properties: @@ -1571,7 +1685,7 @@ func TestAllOf_ThreeWayRedeclarationProducesTwoDiagnostics(t *testing.T) { id: {type: boolean} `) _, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) conflicts := conflictDiags(diags) require.Len(t, conflicts, 2, "three-way redeclaration produces two diagnostics") } @@ -1580,7 +1694,7 @@ func TestAllOf_ThreeWayCompatibleRedeclarationStaysSilent(t *testing.T) { t.Parallel() // When three allOf branches declare the same field with compatible types // (all the same), no conflict is reported. - spec := componentSpec(` ThreeWayCompat: + spec := openapitest.ComponentSpec(` ThreeWayCompat: allOf: - type: object properties: @@ -1593,7 +1707,7 @@ func TestAllOf_ThreeWayCompatibleRedeclarationStaysSilent(t *testing.T) { id: {type: string} `) _, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) assert.Empty(t, conflictDiags(diags), "three-way compatible redeclaration stays silent") } @@ -1617,7 +1731,7 @@ func TestAllOf_SatisfiableNarrowingsStaySilent(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() _, diags := lowerSpec(t, allOfConflictSpec(tc.a, tc.b)) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) assert.Empty(t, conflictDiags(diags), "%s must not be reported as a conflict", tc.name) }) } @@ -1632,7 +1746,7 @@ func TestAllOf_EnumVersusIncompatibleTypeStillConflicts(t *testing.T) { // goes through the same aok&&bok path as two plain scalars. _, diags := lowerSpec(t, allOfConflictSpec( "{type: string, enum: [active, inactive]}", "{type: integer}")) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) conflicts := conflictDiags(diags) require.Len(t, conflicts, 1, "enum of strings vs integer is still a provable conflict") assert.Contains(t, conflicts[0].Message, `"v"`, "the diagnostic names the conflicting field") @@ -1644,7 +1758,7 @@ func TestAllOf_TypeConflictMessageNamesBothTypes(t *testing.T) { // that something did: the two conflicting type identities, so the author // can see at a glance what disagreed without cross-referencing the spec. _, diags := lowerSpec(t, allOfConflictSpec("{type: string}", "{type: integer}")) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) conflicts := conflictDiags(diags) require.Len(t, conflicts, 1) assert.Contains(t, conflicts[0].Message, "t/prim/string", "names the first branch's type") @@ -1659,7 +1773,7 @@ func TestAllOf_PropertyAlongsideAllOfConflictMessageIsAccurate(t *testing.T) { // so the message must not claim "allOf branches redeclare" — it must read // correctly for a co-declared sibling property too (mergeProperty folds // both cases the same way; see its doc comment). - spec := componentSpec(` Along: + spec := openapitest.ComponentSpec(` Along: type: object properties: id: {type: string} @@ -1669,7 +1783,7 @@ func TestAllOf_PropertyAlongsideAllOfConflictMessageIsAccurate(t *testing.T) { id: {type: integer} `) _, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) conflicts := conflictDiags(diags) require.Len(t, conflicts, 1, "the co-declared property conflict is diagnosed once") d := conflicts[0] @@ -1694,10 +1808,10 @@ func TestRawFromNode_SeparatesAbsentFromUnconvertible(t *testing.T) { }{ {name: "absent node is neither raw nor error", node: nil}, {name: "undecodable node errors", node: &yaml.Node{Kind: yaml.Kind(99)}, wantErr: true}, - {name: "non-string key does not decode", node: yamlNode(t, "? [a, b]\n: v"), wantErr: true}, - {name: "int key does not marshal", node: yamlNode(t, "1: a\n2: b"), wantErr: true}, - {name: "nan decodes but does not marshal", node: yamlNode(t, "{a: .nan}"), wantErr: true}, - {name: "convertible node yields json", node: yamlNode(t, "{a: 1}"), want: `{"a":1}`}, + {name: "non-string key does not decode", node: openapitest.YAMLNode(t, "? [a, b]\n: v"), wantErr: true}, + {name: "int key does not marshal", node: openapitest.YAMLNode(t, "1: a\n2: b"), wantErr: true}, + {name: "nan decodes but does not marshal", node: openapitest.YAMLNode(t, "{a: .nan}"), wantErr: true}, + {name: "convertible node yields json", node: openapitest.YAMLNode(t, "{a: 1}"), want: `{"a":1}`}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -1725,13 +1839,13 @@ func TestRawPropertyNode_NilSchema(t *testing.T) { func TestSchema_OneOfWithBoolBranch(t *testing.T) { t.Parallel() - spec := componentSpec(` U: + spec := openapitest.ComponentSpec(` U: anyOf: - {type: string} - true `) doc, diags := lowerSpec(t, spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) u, ok := typeByName(doc, "U").(*ir.Union) require.True(t, ok) assert.Len(t, u.Variants, 2, "the boolean branch is a variant, not a null strip") @@ -1755,7 +1869,7 @@ components: $ref: '#/components/schemas/Base' minimum: 5 `) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) bounded, ok := doc.Types["t/openapi/components/schemas/Bounded"].(*ir.Scalar) require.True(t, ok, "a component aliasing another schema interns as a Scalar") require.NotNil(t, bounded.Constraints) @@ -1799,7 +1913,7 @@ components: minimum: 7 example: 9 `) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) inner, ok := doc.Types["t/anon/components/schemas/Holder/properties/inner"].(*ir.Scalar) require.True(t, ok, "the referenced sub-schema hoists an alias at its own pointer") @@ -1841,7 +1955,7 @@ components: properties: inner: {$ref: '#/components/schemas/Base'} `) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) inner, ok := doc.Types["t/anon/components/schemas/Holder/properties/inner"].(*ir.Scalar) require.True(t, ok, "a bare $ref sub-schema aliases rather than copying its target") require.NotNil(t, inner.Base) @@ -1851,16 +1965,6 @@ components: assert.True(t, isModel, "the structure lives at the component it was declared at") } -// inlineProbeBody is the body every inline-position case below writes: one -// annotation of each kind attachDeclaredAnnotations reads, one validation-only -// keyword, and one value constraint — all of them position-scoped, so a -// position that lowers this to the shared string primitive loses every one. All -// three documentation keywords are here because a home that keeps only the -// description passes a probe that writes only a description. -const inlineProbeBody = `{type: string, title: SUM, description: DOC, ` + - `externalDocs: {url: 'https://e.example', description: ED}, deprecated: true, ` + - `example: abc, x-vendor: V, xml: {name: X}, not: {const: N}, maxLength: 3}` - // inlinePosition is one schema position with no ir.Property or ir.Parameter to // carry a declaration's annotations, so the declaration must own a node. type inlinePosition struct { @@ -1876,33 +1980,33 @@ type inlinePosition struct { func inlinePositions() []inlinePosition { const prim = ir.TypeID("t/prim/string") return []inlinePosition{ - {"items", func(b string) string { return componentSpec(" A: {type: array, items: " + b + "}\n") }, + {"items", func(b string) string { return openapitest.ComponentSpec(" A: {type: array, items: " + b + "}\n") }, "t/anon/components/schemas/A/items", prim}, {"nested-items", func(b string) string { - return componentSpec(" A: {type: array, items: {type: array, items: " + b + "}}\n") + return openapitest.ComponentSpec(" A: {type: array, items: {type: array, items: " + b + "}}\n") }, "t/anon/components/schemas/A/items/items", prim}, {"additionalProperties", func(b string) string { - return componentSpec(" A: {type: object, additionalProperties: " + b + "}\n") + return openapitest.ComponentSpec(" A: {type: object, additionalProperties: " + b + "}\n") }, "t/anon/components/schemas/A/additionalProperties", prim}, {"prefixItems", func(b string) string { - return componentSpec(" A: {type: array, prefixItems: [" + b + "]}\n") + return openapitest.ComponentSpec(" A: {type: array, prefixItems: [" + b + "]}\n") }, "t/anon/components/schemas/A/prefixItems/0", prim}, {"patternProperties", func(b string) string { - return componentSpec(" A: {type: object, patternProperties: {\"^x\": " + b + "}}\n") + return openapitest.ComponentSpec(" A: {type: object, patternProperties: {\"^x\": " + b + "}}\n") }, "t/anon/components/schemas/A/patternProperties/^x", prim}, {"oneOf-branch", func(b string) string { - return componentSpec(" A: {oneOf: [" + b + ", {type: integer}]}\n") + return openapitest.ComponentSpec(" A: {oneOf: [" + b + ", {type: integer}]}\n") }, "t/anon/components/schemas/A/oneOf/0", prim}, {"anyOf-branch", func(b string) string { - return componentSpec(" A: {anyOf: [" + b + ", {type: integer}]}\n") + return openapitest.ComponentSpec(" A: {anyOf: [" + b + ", {type: integer}]}\n") }, "t/anon/components/schemas/A/anyOf/0", prim}, {"request-media-type", func(b string) string { - return pathsSpec(" /x:\n post:\n operationId: p\n requestBody:\n" + + return openapitest.PathsSpec(" /x:\n post:\n operationId: p\n requestBody:\n" + " content: {application/json: {schema: " + b + "}}\n" + " responses: {\"204\": {description: ok}}\n") }, "t/anon/paths/~1x/post/requestBody/content/application~1json/schema", prim}, {"response-media-type", func(b string) string { - return pathsSpec(" /x:\n get:\n operationId: g\n responses:\n" + + return openapitest.PathsSpec(" /x:\n get:\n operationId: g\n responses:\n" + " \"200\":\n description: ok\n" + " content: {application/json: {schema: " + b + "}}\n") }, "t/anon/paths/~1x/get/responses/200/content/application~1json/schema", prim}, @@ -1919,8 +2023,8 @@ func TestInlinePosition_DeclarationOwnsANode(t *testing.T) { for _, pos := range inlinePositions() { t.Run(pos.name, func(t *testing.T) { t.Parallel() - doc, diags := parseFull(t, pos.spec(inlineProbeBody)) - requireNoErrorDiags(t, diags) + doc, diags := parseFull(t, pos.spec(openapitest.InlineProbeBody)) + openapitest.RequireNoErrorDiags(t, diags) sc, ok := doc.Types[pos.id].(*ir.Scalar) require.True(t, ok, "the declaration owns a Scalar at its own pointer; got %v", doc.Types[pos.id]) @@ -1931,13 +2035,13 @@ func TestInlinePosition_DeclarationOwnsANode(t *testing.T) { } } -// assertProbeAnnotationsKept checks every keyword inlineProbeBody writes +// assertProbeAnnotationsKept checks every keyword openapitest.InlineProbeBody writes // survived onto the node the declaration owns. func assertProbeAnnotationsKept(t *testing.T, sc *ir.Scalar) { t.Helper() - assertProbeDocsKept(t, sc.Docs) + openapitest.AssertProbeDocsKept(t, sc.Docs) assert.NotNil(t, sc.Deprecation, "deprecation") - assertProbeExample(t, sc.Examples) + openapitest.AssertProbeExample(t, sc.Examples) if assert.NotNil(t, sc.XML, "xml hints") { assert.Equal(t, "X", sc.XML.Name) } @@ -1956,29 +2060,6 @@ func assertProbeAnnotationsKept(t *testing.T, sc *ir.Scalar) { } } -// assertProbeDocsKept checks all three documentation keywords inlineProbeBody -// writes reached d, wherever the position's home turned out to be. -func assertProbeDocsKept(t *testing.T, d ir.Docs) { - t.Helper() - assert.Equal(t, "SUM", d.Summary, "title") - assert.Equal(t, "DOC", d.Description, "description") - if assert.Len(t, d.ExternalDocs, 1, "externalDocs") { - assert.Equal(t, "https://e.example", d.ExternalDocs[0].URL) - assert.Equal(t, "ED", d.ExternalDocs[0].Description) - } -} - -// assertProbeExample checks the single example inlineProbeBody writes reached -// the home under test with its value intact. -func assertProbeExample(t *testing.T, examples []ir.Example) { - t.Helper() - if !assert.Len(t, examples, 1, "examples") { - return - } - require.NotNil(t, examples[0].Value) - assert.Equal(t, "abc", examples[0].Value.Str) -} - // TestInlinePosition_BareScalarStaysShared is the control that bounds the fix: // a position declaring nothing of its own gains nothing by owning a node, so it // must keep resolving straight to the shared primitive rather than growing an @@ -1989,7 +2070,7 @@ func TestInlinePosition_BareScalarStaysShared(t *testing.T) { t.Run(pos.name, func(t *testing.T) { t.Parallel() doc, diags := parseFull(t, pos.spec("{type: string}")) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) assert.NotContains(t, doc.Types, pos.id, "a bare declaration must not hoist a node") assert.Contains(t, doc.Types, pos.target, "it resolves to the shared primitive instead") }) @@ -2002,9 +2083,9 @@ func TestInlinePosition_BareScalarStaysShared(t *testing.T) { // shared node a bare string does. Nullability still lifts onto the refs. func TestInlinePosition_NullStrippedScalarKeepsAnnotations(t *testing.T) { t.Parallel() - doc, diags := parseFull(t, componentSpec( + doc, diags := parseFull(t, openapitest.ComponentSpec( " A: {type: array, items: {type: [string, \"null\"], description: DOC}}\n")) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) sc, ok := doc.Types["t/anon/components/schemas/A/items"].(*ir.Scalar) require.True(t, ok, "a null-stripped scalar declaration owns a node like any other") @@ -2023,10 +2104,10 @@ func TestInlinePosition_NullStrippedScalarKeepsAnnotations(t *testing.T) { // them itself. func TestInlinePosition_RefSiblingsKeepTheirPosition(t *testing.T) { t.Parallel() - doc, diags := parseFull(t, componentSpec( + doc, diags := parseFull(t, openapitest.ComponentSpec( " S: {type: string}\n"+ " A: {type: array, items: {$ref: '#/components/schemas/S', maxLength: 25, description: D}}\n")) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) sc, ok := doc.Types["t/anon/components/schemas/A/items"].(*ir.Scalar) require.True(t, ok, "siblings beside a $ref give the position a node of its own") @@ -2047,10 +2128,10 @@ func TestInlinePosition_RefSiblingsKeepTheirPosition(t *testing.T) { // $ref with nothing beside it still points straight at its target. func TestInlinePosition_BareRefStaysDirect(t *testing.T) { t.Parallel() - doc, diags := parseFull(t, componentSpec( + doc, diags := parseFull(t, openapitest.ComponentSpec( " S: {type: string}\n"+ " A: {type: array, items: {$ref: '#/components/schemas/S'}}\n")) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) list, ok := doc.Types[componentID("A")].(*ir.List) require.True(t, ok) @@ -2074,22 +2155,22 @@ type stolenPosition struct { func (p stolenPosition) spec(refFirst bool) string { outsider := " Outsider: {$ref: '#" + strings.TrimPrefix(string(p.id), "t/anon") + "'}\n" if refFirst { - return componentSpec(outsider + p.owner) + return openapitest.ComponentSpec(outsider + p.owner) } - return componentSpec(p.owner + outsider) + return openapitest.ComponentSpec(p.owner + outsider) } func stolenPositions() []stolenPosition { return []stolenPosition{ - {"items", " A: {type: array, items: " + inlineProbeBody + "}\n", + {"items", " A: {type: array, items: " + openapitest.InlineProbeBody + "}\n", "t/anon/components/schemas/A/items"}, - {"additionalProperties", " A: {type: object, additionalProperties: " + inlineProbeBody + "}\n", + {"additionalProperties", " A: {type: object, additionalProperties: " + openapitest.InlineProbeBody + "}\n", "t/anon/components/schemas/A/additionalProperties"}, - {"prefixItems", " A: {type: array, prefixItems: [" + inlineProbeBody + "]}\n", + {"prefixItems", " A: {type: array, prefixItems: [" + openapitest.InlineProbeBody + "]}\n", "t/anon/components/schemas/A/prefixItems/0"}, - {"patternProperties", " A: {type: object, patternProperties: {\"^x\": " + inlineProbeBody + "}}\n", + {"patternProperties", " A: {type: object, patternProperties: {\"^x\": " + openapitest.InlineProbeBody + "}}\n", "t/anon/components/schemas/A/patternProperties/^x"}, - {"oneOf-branch", " A: {oneOf: [" + inlineProbeBody + ", {type: integer}]}\n", + {"oneOf-branch", " A: {oneOf: [" + openapitest.InlineProbeBody + ", {type: integer}]}\n", "t/anon/components/schemas/A/oneOf/0"}, } } @@ -2110,12 +2191,12 @@ func TestInlinePosition_OutsideRefDoesNotMoveTheHome(t *testing.T) { for _, pos := range stolenPositions() { t.Run(pos.name, func(t *testing.T) { t.Parallel() - alone, diags := parseFull(t, componentSpec(pos.owner)) - requireNoErrorDiags(t, diags) + alone, diags := parseFull(t, openapitest.ComponentSpec(pos.owner)) + openapitest.RequireNoErrorDiags(t, diags) refFirst, diags := parseFull(t, pos.spec(true)) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) refLast, diags := parseFull(t, pos.spec(false)) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) for _, with := range []*ir.Document{refFirst, refLast} { assert.Empty(t, cmp.Diff(alone.Types[componentID("A")], with.Types[componentID("A")]), @@ -2135,13 +2216,23 @@ func TestInlinePosition_OutsideRefDoesNotMoveTheHome(t *testing.T) { // differ by construction when the same components are declared in two orders. // // SourceInfo.Hash digests the source bytes, which are the thing being permuted. -// Naming.Hint is a live gap: the hint a node hoisted at a pointer carries is -// minted by whichever of the two namers reaches the pointer first — the -// declaration's context ("A_item") or the reference's last pointer segment -// ("items") — because intern keeps the first name it is given. That divergence -// is naming only, and predates the annotation work: a $ref to an object-bodied -// `items` shows it with no annotations involved at all. Everything else is -// compared. +// +// Naming.Hint is a live gap, and a narrower one than it was: the hint a node +// hoisted at a pointer carries is minted by whichever of the two namers reaches +// the pointer first, because intern keeps the first name it is given. The +// composition-branch family no longer diverges — both namers ask branchHint's +// question there (GitHub #181, #281) — so what is left is the four inline +// structural positions, where the declaration's context ("a_item") and the +// reference's last pointer segment ("items") genuinely hold different +// information: GitHub #353, which permutes exactly the positions +// TestInlinePosition_OutsideRefDoesNotMoveTheHome does. That divergence is +// naming only, and predates the annotation work: a $ref to an object-bodied +// `items` shows it with no annotations involved at all. +// +// Everything else is compared, and a caller whose shapes settle their hints +// identically both ways should compare the registry a second time with nothing +// excluded rather than rely on this — see TestNullCollapse_BranchHintIsOrder- +// Independent. func orderInvariantIR() []cmp.Option { return []cmp.Option{ cmpopts.IgnoreFields(ir.Naming{}, "Hint"), @@ -2157,24 +2248,24 @@ func orderInvariantIR() []cmp.Option { // second. func TestPropertyAnnotations_KeptWhenAnOutsideRefNamesTheProperty(t *testing.T) { t.Parallel() - owner := " A: {type: object, properties: {p: " + inlineProbeBody + "}}\n" + owner := " A: {type: object, properties: {p: " + openapitest.InlineProbeBody + "}}\n" outsider := " Outsider: {$ref: '#/components/schemas/A/properties/p'}\n" for _, tc := range []struct{ name, spec string }{ - {"reference declared first", componentSpec(outsider + owner)}, - {"reference declared last", componentSpec(owner + outsider)}, + {"reference declared first", openapitest.ComponentSpec(outsider + owner)}, + {"reference declared last", openapitest.ComponentSpec(owner + outsider)}, } { t.Run(tc.name, func(t *testing.T) { t.Parallel() doc, diags := parseFull(t, tc.spec) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m, ok := doc.Types[componentID("A")].(*ir.Model) require.True(t, ok) - p, ok := propsByWire(m.Properties)["p"] + p, ok := openapitest.PropsByWire(m.Properties)["p"] require.True(t, ok) assert.Equal(t, ir.TypeID("t/prim/string"), p.Type.Target, "the property's own type is unchanged by the outside reference") - assertProbeDocsKept(t, p.Docs) + openapitest.AssertProbeDocsKept(t, p.Docs) assert.Contains(t, p.Unmodeled, "openapi:x-vendor") assert.Contains(t, p.Unmodeled, "openapi:not") @@ -2191,14 +2282,14 @@ func TestPropertyAnnotations_KeptWhenAnOutsideRefNamesTheProperty(t *testing.T) // copies can never drift apart. func TestPropertyAnnotations_OneHomeWhenSchemaOwnsANode(t *testing.T) { t.Parallel() - doc, diags := parseFull(t, componentSpec( + doc, diags := parseFull(t, openapitest.ComponentSpec( " A:\n type: object\n properties:\n"+ " p: {type: object, description: DOC, deprecated: true, xml: {name: X}, x-vendor: V}\n")) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) owner, ok := doc.Types[componentID("A")].(*ir.Model) require.True(t, ok) - p, ok := propsByWire(owner.Properties)["p"] + p, ok := openapitest.PropsByWire(owner.Properties)["p"] require.True(t, ok) assert.Empty(t, p.Docs.Description, "the node the schema owns is the one home") assert.Nil(t, p.Deprecation) @@ -2220,19 +2311,19 @@ func TestPropertyAnnotations_OneHomeWhenSchemaOwnsANode(t *testing.T) { // property's declaration has ir.Property to land on already. func TestPropertyAnnotations_CarriedWhenSchemaOwnsNoNode(t *testing.T) { t.Parallel() - doc, diags := parseFull(t, componentSpec( - " A:\n type: object\n properties:\n p: "+inlineProbeBody+"\n")) - requireNoErrorDiags(t, diags) + doc, diags := parseFull(t, openapitest.ComponentSpec( + " A:\n type: object\n properties:\n p: "+openapitest.InlineProbeBody+"\n")) + openapitest.RequireNoErrorDiags(t, diags) owner, ok := doc.Types[componentID("A")].(*ir.Model) require.True(t, ok) - p, ok := propsByWire(owner.Properties)["p"] + p, ok := openapitest.PropsByWire(owner.Properties)["p"] require.True(t, ok) assert.Equal(t, ir.TypeID("t/prim/string"), p.Type.Target, "a property keeps resolving to the shared primitive") assert.NotContains(t, doc.Types, ir.TypeID("t/anon/components/schemas/A/properties/p")) - assertProbeDocsKept(t, p.Docs) + openapitest.AssertProbeDocsKept(t, p.Docs) assert.NotNil(t, p.Deprecation) - assertProbeExample(t, p.Examples) + openapitest.AssertProbeExample(t, p.Examples) require.NotNil(t, p.XML) assert.Equal(t, "X", p.XML.Name) assert.Contains(t, p.Unmodeled, "openapi:x-vendor") @@ -2287,7 +2378,7 @@ func propertyOf(t *testing.T, doc *ir.Document, model, wire string) ir.Property t.Helper() m, ok := doc.Types[componentID(model)].(*ir.Model) require.True(t, ok, "%s lowers to a model", model) - p, ok := propsByWire(m.Properties)[wire] + p, ok := openapitest.PropsByWire(m.Properties)[wire] require.True(t, ok, "%s declares a property %q", model, wire) return p } @@ -2303,9 +2394,9 @@ func TestPropertyDocs_RefTargetReachesTheCarrier(t *testing.T) { for _, kw := range carrierDocKeywords() { t.Run(kw.name, func(t *testing.T) { t.Parallel() - doc, diags := parseFull(t, componentSpec(docTarget()+ + doc, diags := parseFull(t, openapitest.ComponentSpec(docTarget()+ " Owner: {type: object, properties: {p: {$ref: '#/components/schemas/Target'}}}\n")) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) p := propertyOf(t, doc, "Owner", "p") assert.Equal(t, componentID("Target"), p.Type.Target, "a bare $ref needs no alias") @@ -2325,10 +2416,10 @@ func TestPropertyDocs_UseSiteWinsKeywordByKeyword(t *testing.T) { for _, written := range carrierDocKeywords() { t.Run(written.name, func(t *testing.T) { t.Parallel() - doc, diags := parseFull(t, componentSpec(docTarget()+ + doc, diags := parseFull(t, openapitest.ComponentSpec(docTarget()+ " Owner: {type: object, properties: {p: {$ref: '#/components/schemas/Target', "+ written.write("SITE")+"}}}\n")) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) p := propertyOf(t, doc, "Owner", "p") assert.Equal(t, componentID("Target"), p.Type.Target, "a carrier hoists no alias for its siblings") @@ -2395,9 +2486,9 @@ func TestInlinePosition_HoistGateFollowsWhatIsKept(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { t.Parallel() - doc, diags := parseFull(t, componentSpec( + doc, diags := parseFull(t, openapitest.ComponentSpec( " A: {type: array, items: {type: string, "+tc.keyword+"}}\n")) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) assert.Contains(t, doc.Types, ir.TypeID("t/anon/components/schemas/A/items"), "%s binds the position it is written at, so the position must own a node", tc.keyword) }) @@ -2417,9 +2508,9 @@ func TestInlinePosition_NothingToHoldHoistsNoNode(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { t.Parallel() - doc, diags := parseFull(t, componentSpec( + doc, diags := parseFull(t, openapitest.ComponentSpec( " A: {type: array, items: {"+tc.keyword+"}}\n")) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) assert.NotContains(t, doc.Types, ir.TypeID("t/anon/components/schemas/A/items")) list, ok := doc.Types[componentID("A")].(*ir.List) require.True(t, ok) @@ -2454,8 +2545,8 @@ func TestInlinePosition_ResidueIsKeptAtEveryHomeOwnNodePosition(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { t.Parallel() - doc, diags := parseFull(t, componentSpec(strings.ReplaceAll(tc.body, "RESIDUE", residue))) - requireNoErrorDiags(t, diags) + doc, diags := parseFull(t, openapitest.ComponentSpec(strings.ReplaceAll(tc.body, "RESIDUE", residue))) + openapitest.RequireNoErrorDiags(t, diags) td, ok := doc.Types[ir.TypeID(tc.at)] require.True(t, ok, "the position owns a node to hold what it wrote") @@ -2491,10 +2582,10 @@ func assertResidueKeptAndAnnounced(t *testing.T, p ir.Unmodeled, diags []ir.Diag // residue would restate what the carrier already holds. func TestPropertyPosition_ResidueStaysOutOfTheTypeNode(t *testing.T) { t.Parallel() - doc, diags := parseFull(t, componentSpec( + doc, diags := parseFull(t, openapitest.ComponentSpec( " A: {type: object, properties: {p: {type: array, items: {type: string}, "+ "default: [], readOnly: true}}}\n")) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) p := propertyOf(t, doc, "A", "p") require.NotNil(t, p.Default, "default lands in the property's own field") @@ -2768,8 +2859,8 @@ func TestVocabulary2020_12_EveryKeywordIsLoweredOrKept(t *testing.T) { // case "differ" for free. func compileVocabIR(t *testing.T, schemas string) string { t.Helper() - doc, diags := parseFull(t, componentSpec(schemas)) - requireNoErrorDiags(t, diags) + doc, diags := parseFull(t, openapitest.ComponentSpec(schemas)) + openapitest.RequireNoErrorDiags(t, diags) doc.Diagnostics = nil doc.Sources = nil out, err := json.Marshal(doc) @@ -2822,8 +2913,8 @@ func TestContentVocabulary_LowersToEncoding(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { t.Parallel() - doc, diags := parseFull(t, componentSpec(tc.schemas)) - requireNoErrorDiags(t, diags) + doc, diags := parseFull(t, openapitest.ComponentSpec(tc.schemas)) + openapitest.RequireNoErrorDiags(t, diags) sc, ok := doc.Types[tc.at].(*ir.Scalar) require.True(t, ok, "the content vocabulary needs a Scalar of its own at %s", tc.at) @@ -2864,8 +2955,8 @@ func TestContentVocabulary_KeepsTheBoundsWrittenBesideIt(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { t.Parallel() - doc, diags := parseFull(t, componentSpec(tc.schemas)) - requireNoErrorDiags(t, diags) + doc, diags := parseFull(t, openapitest.ComponentSpec(tc.schemas)) + openapitest.RequireNoErrorDiags(t, diags) sc, ok := doc.Types[tc.at].(*ir.Scalar) require.True(t, ok, "the content vocabulary hoists a Scalar at %s", tc.at) @@ -2904,8 +2995,8 @@ func TestContentVocabulary_KeptWhereNoEncodingHolds(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { t.Parallel() - doc, diags := parseFull(t, componentSpec(tc.schemas)) - requireNoErrorDiags(t, diags) + doc, diags := parseFull(t, openapitest.ComponentSpec(tc.schemas)) + openapitest.RequireNoErrorDiags(t, diags) td, ok := doc.Types[tc.at] require.True(t, ok) @@ -2913,7 +3004,7 @@ func TestContentVocabulary_KeptWhereNoEncodingHolds(t *testing.T) { require.True(t, ok, "%s must be kept verbatim under Unmodeled", tc.key) assert.JSONEq(t, tc.wantJSON, string(entry.Value)) assert.Equal(t, ir.ReasonNoIRHome, entry.Reason) - assertInfoDiagAt(t, diags, entry.Provenance.Pointer) + openapitest.AssertInfoDiagAt(t, diags, entry.Provenance.Pointer) }) } } @@ -2923,16 +3014,16 @@ func TestContentVocabulary_KeptWhereNoEncodingHolds(t *testing.T) { // primitive, where the ir.Property is the only home the keyword has. func TestContentVocabulary_KeptOnACarrierWithNoNode(t *testing.T) { t.Parallel() - doc, diags := parseFull(t, componentSpec( + doc, diags := parseFull(t, openapitest.ComponentSpec( " A: {type: object, properties: {p: {contentMediaType: application/json}}}\n")) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) p := propertyOf(t, doc, "A", "p") assert.Equal(t, ir.TypeID("t/prim/any"), p.Type.Target, "an untyped schema stays schemaless") entry, ok := p.Unmodeled["openapi:contentMediaType"] require.True(t, ok, "the carrier is the only home when the schema hoisted no node") assert.Equal(t, ir.ReasonNoIRHome, entry.Reason) - assertInfoDiagAt(t, diags, entry.Provenance.Pointer) + openapitest.AssertInfoDiagAt(t, diags, entry.Provenance.Pointer) } // TestDynamicRef_ExpandsAgainstTheOneMatchingAnchor pins the resolvable half of @@ -2940,11 +3031,11 @@ func TestContentVocabulary_KeptOnACarrierWithNoNode(t *testing.T) { // the keyword worth having. func TestDynamicRef_ExpandsAgainstTheOneMatchingAnchor(t *testing.T) { t.Parallel() - doc, diags := parseFull(t, componentSpec( + doc, diags := parseFull(t, openapitest.ComponentSpec( " Meta: {$dynamicAnchor: meta, type: object, properties: {n: {type: string}}}\n"+ " Uses: {type: object, properties: {m: {$dynamicRef: '#meta'}}}\n"+ " Tree: {$dynamicAnchor: node, type: object, properties: {kids: {type: array, items: {$dynamicRef: '#node'}}}}\n")) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) assert.Equal(t, componentID("Meta"), propertyOf(t, doc, "Uses", "m").Type.Target, "the reference resolves to the anchor's own component, not to the top type") @@ -2964,10 +3055,62 @@ func TestDynamicRef_ExpandsAgainstTheOneMatchingAnchor(t *testing.T) { // Expansion collapses an indirection the source left to evaluation, so it is // announced under its own code rather than sharing the composition one. - assert.Equal(t, 2, countDiagsAt(diags, diag.DynamicRefExpanded, ir.SeverityInfo), + assert.Equal(t, 2, openapitest.CountDiagsAt(diags, diag.DynamicRefExpanded, ir.SeverityInfo), "each expanded reference is announced once") } +// TestDynamicRef_FragmentIsPercentDecoded pins that a $dynamicRef's fragment is +// read as the URI text it is. RFC 3986 §2.3 makes an unreserved character and +// its percent-encoded octet the same character, so `#my%2Danchor` names the +// anchor `my-anchor`; §2.1 makes the escape's hex digits case-insignificant. All +// three spellings below therefore address one declaration (GitHub #233). +func TestDynamicRef_FragmentIsPercentDecoded(t *testing.T) { + t.Parallel() + const anchor = " B: {$dynamicAnchor: my-anchor, type: string}\n" + cases := map[string]string{ + "an escaped hyphen is the hyphen": "#my%2Danchor", + "the escape's hex case is not significant": "#my%2danchor", + "the unescaped spelling names the same one": "#my-anchor", + } + for name, ref := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + doc, diags := parseFull(t, openapitest.ComponentSpec(anchor+" A: {$dynamicRef: '"+ref+"'}\n")) + openapitest.RequireNoErrorDiags(t, diags) + + sc, ok := doc.Types[componentID("A")].(*ir.Scalar) + require.True(t, ok, "the reference position owns a node") + require.NotNil(t, sc.Base) + assert.Equal(t, componentID("B"), sc.Base.Target, "every spelling names the one anchor") + assert.NotContains(t, sc.Unmodeled, "openapi:$dynamicRef", + "an expanded reference is the position's type, so it is not also kept") + }) + } +} + +// TestDynamicRef_ReachesAnAnchorSpelledWithAPercent pins that the decode runs on +// the reference alone: an anchor carrying a literal `%` is addressed by escaping +// it as `%25`, and the anchor's own text is matched exactly as declared. The two +// sides mean different things — 2020-12 §8.2.2 makes `$dynamicAnchor` a plain +// name, §8.2.3.2 makes `$dynamicRef` a URI-reference — so only one is decoded. +// +// §8.2.2's production admits no `%` in an anchor name, so the document validator +// reports the declaration and this case arrives with an error diagnostic beside +// it, the same shape TestDynamicRef_NonScalarValueIsKeptNotExpanded documents. +// The lowering still has to agree with itself about what each side spells. +func TestDynamicRef_ReachesAnAnchorSpelledWithAPercent(t *testing.T) { + t.Parallel() + doc, _ := parseFull(t, openapitest.ComponentSpec( + " B: {$dynamicAnchor: 'pct%name', type: string}\n"+ + " A: {$dynamicRef: '#pct%25name'}\n")) + + sc, ok := doc.Types[componentID("A")].(*ir.Scalar) + require.True(t, ok, "the reference position owns a node") + require.NotNil(t, sc.Base) + assert.Equal(t, componentID("B"), sc.Base.Target, + "an escaped percent addresses the percent itself, so the reference reaches the anchor as declared") +} + // TestDynamicRef_IrreducibleIsKeptAndSaysWhy pins the other half of that promise. // Each case is a reference no static lowering can resolve, and each must survive // verbatim with the reason naming which case it was. @@ -2988,6 +3131,15 @@ func TestDynamicRef_IrreducibleIsKeptAndSaysWhy(t *testing.T) { wantWhy: "is not a plain same-document fragment"}, {name: "a pointer, not an anchor", schemas: " A: {$dynamicRef: '#/components/schemas/M1'}\n", wantWhy: "not a plain same-document fragment"}, + {name: "an invalid percent escape", schemas: " A: {$dynamicRef: '#my%zzanchor'}\n", + wantWhy: `"#my%zzanchor" is not valid percent-encoded text: invalid URL escape "%zz"`}, + {name: "an escaped percent decodes to the character", schemas: " A: {$dynamicRef: '#pct%25name'}\n", + wantWhy: `no $dynamicAnchor "pct%name" is declared`}, + // The anchor here is what a second decode would land on: it would read + // this fragment as "#my-anchor" and expand. Being kept is the proof. + {name: "the decode is not applied twice", + schemas: " H: {$dynamicAnchor: my-anchor, type: string}\n A: {$dynamicRef: '#my%252Danchor'}\n", + wantWhy: `no $dynamicAnchor "my%2Danchor" is declared`}, {name: "declared twice", schemas: anchors + " A: {$dynamicRef: '#dup'}\n", wantWhy: "declared 2 times"}, {name: "not a component", schemas: anchors + " A: {$dynamicRef: '#deep'}\n", @@ -3032,6 +3184,17 @@ func TestDynamicRef_IrreducibleIsKeptAndSaysWhy(t *testing.T) { wantWhy: `an $id at or above "/components/schemas/"`, at: "t/anon/components/schemas/", }, + { + // The mirror of the case above, on the anchor's side. /components/ + // schemas/ addresses a component schema, so the refusal may not say it + // does not — the reason is that an empty name earns no named TypeID, + // which is the policy testdata/conformance/openapi/empty-names.yaml + // records rather than a shape the compiler failed to recognise. + name: `an anchor on a schema named "" says why, not that it is no component`, + schemas: " \"\": {$dynamicAnchor: m, type: string}\n A: {$dynamicRef: '#m'}\n", + wantWhy: `is declared on the component schema keyed "" at "/components/schemas/", ` + + `and an empty name earns no named type to expand to`, + }, { name: "an enclosing schema starts the resource", schemas: anchor + @@ -3054,8 +3217,8 @@ func TestDynamicRef_IrreducibleIsKeptAndSaysWhy(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { t.Parallel() - doc, diags := parseFull(t, componentSpec(tc.schemas)) - requireNoErrorDiags(t, diags) + doc, diags := parseFull(t, openapitest.ComponentSpec(tc.schemas)) + openapitest.RequireNoErrorDiags(t, diags) at := tc.at if at == "" { @@ -3079,11 +3242,11 @@ func TestDynamicRef_IrreducibleIsKeptAndSaysWhy(t *testing.T) { // irverify checks for aliases. func TestDynamicRef_CycleIsRefusedAtEveryEdge(t *testing.T) { t.Parallel() - doc, diags := parseFull(t, componentSpec( + doc, diags := parseFull(t, openapitest.ComponentSpec( " A: {$dynamicAnchor: a, $dynamicRef: '#b'}\n"+ " B: {$dynamicAnchor: b, $dynamicRef: '#a'}\n"+ " Self: {$dynamicAnchor: s, $dynamicRef: '#s'}\n")) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) for _, name := range []string{"A", "B", "Self"} { sc, ok := doc.Types[componentID(name)].(*ir.Scalar) @@ -3094,7 +3257,7 @@ func TestDynamicRef_CycleIsRefusedAtEveryEdge(t *testing.T) { assert.Contains(t, sc.Unmodeled, "openapi:$dynamicRef", "%s keeps the reference it could not take", name) } - assert.Equal(t, 0, countDiagsAt(diags, diag.DynamicRefExpanded, ir.SeverityInfo), + assert.Equal(t, 0, openapitest.CountDiagsAt(diags, diag.DynamicRefExpanded, ir.SeverityInfo), "no edge of the cycle is reported as expanded") } @@ -3104,11 +3267,11 @@ func TestDynamicRef_CycleIsRefusedAtEveryEdge(t *testing.T) { // top type rather than a link that loops. func TestDynamicRef_ExpandsIntoACycleItIsNotPartOf(t *testing.T) { t.Parallel() - doc, diags := parseFull(t, componentSpec( + doc, diags := parseFull(t, openapitest.ComponentSpec( " A: {$dynamicAnchor: a, $dynamicRef: '#b'}\n"+ " B: {$dynamicAnchor: b, $dynamicRef: '#a'}\n"+ " Outside: {$dynamicRef: '#a'}\n")) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) sc, ok := doc.Types[componentID("Outside")].(*ir.Scalar) require.True(t, ok) @@ -3133,8 +3296,8 @@ func TestDynamicRef_ChainEndsAtAnAnchorItCannotFollow(t *testing.T) { for name, schemas := range cases { t.Run(name, func(t *testing.T) { t.Parallel() - doc, diags := parseFull(t, componentSpec(schemas+" A: {$dynamicRef: '#mid'}\n")) - requireNoErrorDiags(t, diags) + doc, diags := parseFull(t, openapitest.ComponentSpec(schemas+" A: {$dynamicRef: '#mid'}\n")) + openapitest.RequireNoErrorDiags(t, diags) sc, ok := doc.Types[componentID("A")].(*ir.Scalar) require.True(t, ok) @@ -3159,14 +3322,14 @@ func TestDialectKeywords_KeptOutOfScope(t *testing.T) { require.ElementsMatch(t, want, annotation.DialectKeywords, "a keyword joining or leaving the exclusion must be decided here too") - doc, diags := parseFull(t, componentSpec( + doc, diags := parseFull(t, openapitest.ComponentSpec( " A:\n"+ " $id: 'urn:example:a'\n"+ " $schema: 'https://json-schema.org/draft/2020-12/schema'\n"+ " $vocabulary: {'https://json-schema.org/draft/2020-12/vocab/core': true}\n"+ " $comment: not for end users\n"+ " type: string\n")) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) td, ok := doc.Types[componentID("A")] require.True(t, ok) @@ -3174,23 +3337,12 @@ func TestDialectKeywords_KeptOutOfScope(t *testing.T) { entry, ok := td.Common().Unmodeled["openapi:"+keyword] require.True(t, ok, "%s must be kept verbatim", keyword) assert.Equal(t, ir.ReasonOutOfScope, entry.Reason) - assertInfoDiagAt(t, diags, entry.Provenance.Pointer) + openapitest.AssertInfoDiagAt(t, diags, entry.Provenance.Pointer) } assert.NotContains(t, td.Common().Unmodeled, "openapi:$comment", "2020-12 §8.3 forbids presenting $comment, so it is dropped rather than kept") } -// assertInfoDiagAt requires one info diagnostic stamped at pointer. -func assertInfoDiagAt(t *testing.T, diags []ir.Diagnostic, pointer string) { - t.Helper() - for _, d := range diags { - if d.Severity == ir.SeverityInfo && d.Provenance.Pointer == pointer { - return - } - } - assert.Fail(t, "nothing announced this", "no info diagnostic at %q; got %+v", pointer, diags) -} - // assertDiagContains requires one diagnostic at pointer whose message carries // substr, so a case asserts the reason it was given and not merely that it was // reported. @@ -3218,7 +3370,7 @@ func TestDynamicRef_NonScalarValueIsKeptNotExpanded(t *testing.T) { for name, schemas := range cases { t.Run(name, func(t *testing.T) { t.Parallel() - doc, diags := parseFull(t, componentSpec( + doc, diags := parseFull(t, openapitest.ComponentSpec( " M1: {$dynamicAnchor: ok, type: string}\n"+schemas)) td, ok := doc.Types[componentID("A")] @@ -3236,10 +3388,10 @@ func TestDynamicRef_NonScalarValueIsKeptNotExpanded(t *testing.T) { // prototype changes, so a site that fills in a name or a description keeps it. func TestAppendExample_ConvertsAndAppends(t *testing.T) { t.Parallel() - c := lowering.New(0, &soa.OpenAPI{}, ir.SourceInfo{}, "", overlay.Origin{}) + c := lowering.New(0, &soa.OpenAPI{}, ir.SourceInfo{}, "", lowering.Limits{}, lowering.StreamingMedia{}, lowering.ExtensionPromotions{}, overlay.Origin{}) proto := ir.Example{Name: "n", Summary: "s", Description: "d"} - out, diags := schema.AppendExample(c, nil, proto, strNode("hello"), "/p", "examples", "n") + out, diags := schema.AppendExample(c, nil, proto, openapitest.StrNode("hello"), "/p", "examples", "n") assert.Empty(t, diags, "a convertible node is announced by nothing") require.Len(t, out, 1) @@ -3254,7 +3406,7 @@ func TestAppendExample_ConvertsAndAppends(t *testing.T) { // that joins them, so a wrong join shows up nowhere else. func TestAppendExample_UnconvertibleValueIsReported(t *testing.T) { t.Parallel() - c := lowering.New(0, &soa.OpenAPI{}, ir.SourceInfo{}, "", overlay.Origin{}) + c := lowering.New(0, &soa.OpenAPI{}, ir.SourceInfo{}, "", lowering.Limits{}, lowering.StreamingMedia{}, lowering.ExtensionPromotions{}, overlay.Origin{}) nan := &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!float", Value: ".nan"} out, diags := schema.AppendExample(c, nil, ir.Example{}, nan, "/p", "examples", "n") @@ -3271,7 +3423,7 @@ func TestAppendExample_UnconvertibleValueIsReported(t *testing.T) { // it, not at the position that declared it. func TestStampConstraintDiags_RelocatesEveryDiagnosticToTheReadingPointer(t *testing.T) { t.Parallel() - c := lowering.New(0, &soa.OpenAPI{}, ir.SourceInfo{}, "", overlay.Origin{}) + c := lowering.New(0, &soa.OpenAPI{}, ir.SourceInfo{}, "", lowering.Limits{}, lowering.StreamingMedia{}, lowering.ExtensionPromotions{}, overlay.Origin{}) in := []ir.Diagnostic{ {Code: diag.DegradedConstruct, Provenance: ir.Provenance{Pointer: "/elsewhere"}}, {Code: diag.NumericPrecision, Provenance: ir.Provenance{Source: 9, Pointer: "/other"}}, @@ -3293,13 +3445,13 @@ func TestStampConstraintDiags_RelocatesEveryDiagnosticToTheReadingPointer(t *tes // the same answer a bare `false` schema gets in its own right. func TestAllOf_FalseBranchClosesTheComposition(t *testing.T) { t.Parallel() - doc, diags := lowerSpec(t, componentSpec(` Never: + doc, diags := lowerSpec(t, openapitest.ComponentSpec(` Never: allOf: - false - type: object properties: {id: {type: string}} `)) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m, ok := typeByName(doc, "Never").(*ir.Model) require.True(t, ok, "the composition still lowers to a model") @@ -3307,7 +3459,7 @@ func TestAllOf_FalseBranchClosesTheComposition(t *testing.T) { require.Len(t, m.Unmodeled, 1, "the branch is kept verbatim") assert.Equal(t, ir.RawValue("false"), m.Unmodeled["openapi:allOf/0"].Value, "keyed by the branch index, so sibling branches cannot overwrite one another") - assert.True(t, hasDiagAt(diags, diag.FalseSchema, ir.SeverityInfo), "and it is announced") + assert.True(t, openapitest.HasDiagAt(diags, diag.FalseSchema, ir.SeverityInfo), "and it is announced") } // TestUnhomedApplicator_KeptOnAListAndAnnounced pins the arm of the home @@ -3316,18 +3468,18 @@ func TestAllOf_FalseBranchClosesTheComposition(t *testing.T) { // kept verbatim and reported rather than silently dropped. func TestUnhomedApplicator_KeptOnAListAndAnnounced(t *testing.T) { t.Parallel() - doc, diags := lowerSpec(t, componentSpec(` Odd: + doc, diags := lowerSpec(t, openapitest.ComponentSpec(` Odd: type: array items: {type: string} properties: {p: {type: string}} `)) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) td := typeByName(doc, "Odd") require.NotNil(t, td, "the array still lowers") assert.Contains(t, td.Common().Unmodeled, "openapi:properties", "a list has no home for properties, so it is kept") - assert.True(t, hasDiagAt(diags, diag.DegradedConstruct, ir.SeverityInfo), + assert.True(t, openapitest.HasDiagAt(diags, diag.DegradedConstruct, ir.SeverityInfo), "and the position says which keywords it could not carry") } @@ -3343,12 +3495,12 @@ const unpreservableValue = ".nan" // read as though the object were an array. func TestUnhomedApplicator_ObjectCarriesNoItemKeyword(t *testing.T) { t.Parallel() - doc, diags := lowerSpec(t, componentSpec(` Odd: + doc, diags := lowerSpec(t, openapitest.ComponentSpec(` Odd: type: object properties: {p: {type: string}} items: {type: string} `)) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) m, ok := typeByName(doc, "Odd").(*ir.Model) require.True(t, ok, "it is still a model") @@ -3363,7 +3515,7 @@ func TestUnhomedApplicator_ObjectCarriesNoItemKeyword(t *testing.T) { // looking in Unmodeled for something that is not there. func TestUnhomedApplicator_UnpreservableKeywordIsNotAnnounced(t *testing.T) { t.Parallel() - doc, diags := lowerSpec(t, componentSpec(` Odd: + doc, diags := lowerSpec(t, openapitest.ComponentSpec(` Odd: type: object properties: {p: {type: string}} items: {type: string, x-t: `+unpreservableValue+`} @@ -3372,7 +3524,7 @@ func TestUnhomedApplicator_UnpreservableKeywordIsNotAnnounced(t *testing.T) { m, ok := typeByName(doc, "Odd").(*ir.Model) require.True(t, ok) assert.NotContains(t, m.Unmodeled, "openapi:items", "the conversion failed, so nothing was kept") - assert.True(t, hasDiag(diags, diag.UnpreservableConstruct), "the failure itself is reported") + assert.True(t, openapitest.HasDiag(diags, diag.UnpreservableConstruct), "the failure itself is reported") assert.Empty(t, preservationClaims(diags), "nothing was written under Unmodeled, so nothing may announce that it was") } @@ -3383,7 +3535,7 @@ func TestUnhomedApplicator_UnpreservableKeywordIsNotAnnounced(t *testing.T) { // happen either. func TestAllOf_UnpreservableBranchIsNotAnnounced(t *testing.T) { t.Parallel() - doc, diags := lowerSpec(t, componentSpec(` M: + doc, diags := lowerSpec(t, openapitest.ComponentSpec(` M: allOf: - {type: string, maxLength: 3, x-t: `+unpreservableValue+`} `)) @@ -3391,7 +3543,7 @@ func TestAllOf_UnpreservableBranchIsNotAnnounced(t *testing.T) { td := typeByName(doc, "M") require.NotNil(t, td) assert.NotContains(t, td.Common().Unmodeled, "openapi:allOf/0", "the branch did not convert") - assert.True(t, hasDiag(diags, diag.UnpreservableConstruct)) + assert.True(t, openapitest.HasDiag(diags, diag.UnpreservableConstruct)) assert.Empty(t, preservationClaims(diags), "no degradation is announced for a branch that was not kept") } @@ -3404,7 +3556,7 @@ func TestAllOf_UnpreservableBranchIsNotAnnounced(t *testing.T) { // preserved construct that a claim could be about. func TestUnionSiblings_UnpreservableIsReportedNotClaimed(t *testing.T) { t.Parallel() - _, diags := lowerSpec(t, componentSpec(` S: + _, diags := lowerSpec(t, openapitest.ComponentSpec(` S: items: {type: string, x-t: `+unpreservableValue+`} oneOf: [{type: string, x-t: `+unpreservableValue+`}, {type: integer}] `)) @@ -3463,14 +3615,14 @@ func TestResidueKeywords_HandsBackACopy(t *testing.T) { // rather than dropped, which is the whole of §4.8 for this keyword. func TestUnhomedApplicator_FormatWithNoTypeIsKept(t *testing.T) { t.Parallel() - doc, diags := lowerSpec(t, componentSpec(" Odd: {format: date-time}\n")) - requireNoErrorDiags(t, diags) + doc, diags := lowerSpec(t, openapitest.ComponentSpec(" Odd: {format: date-time}\n")) + openapitest.RequireNoErrorDiags(t, diags) td := typeByName(doc, "Odd") require.NotNil(t, td, "the position still lowers to something") assert.Contains(t, td.Common().Unmodeled, "openapi:format", "a format with no type to pair with reaches no field, so it is kept") - assert.True(t, hasDiag(diags, diag.DegradedConstruct), "and the position says so") + assert.True(t, openapitest.HasDiag(diags, diag.DegradedConstruct), "and the position says so") } // TestCoDeclaredFamily_PassedOverKeywordIsKept covers the families lower()'s @@ -3514,9 +3666,9 @@ func TestCoDeclaredFamily_PassedOverKeywordIsKept(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { t.Parallel() - doc, diags := lowerSpec(t, componentSpec( + doc, diags := lowerSpec(t, openapitest.ComponentSpec( " Base: {type: object, properties: {id: {type: string}}}\n S:\n"+tc.body)) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) td := typeByName(doc, "S") require.NotNil(t, td) @@ -3528,7 +3680,7 @@ func TestCoDeclaredFamily_PassedOverKeywordIsKept(t *testing.T) { assert.Equal(t, "/components/schemas/S/"+tc.skipped, entry.Provenance.Pointer, "routable to where it was written") assert.Contains(t, - diagMessageAt(t, diags, diag.DegradedConstruct, ir.SeverityInfo, "/components/schemas/S"), + openapitest.DiagMessageAt(t, diags, diag.DegradedConstruct, ir.SeverityInfo, "/components/schemas/S"), tc.skipped+" kept verbatim under Unmodeled") }) } @@ -3539,19 +3691,19 @@ func TestCoDeclaredFamily_PassedOverKeywordIsKept(t *testing.T) { // together in one report rather than one of them standing in for the rest. func TestCoDeclaredFamily_EveryPassedOverKeywordIsKept(t *testing.T) { t.Parallel() - doc, diags := lowerSpec(t, componentSpec(` Base: {type: object, properties: {id: {type: string}}} + doc, diags := lowerSpec(t, openapitest.ComponentSpec(` Base: {type: object, properties: {id: {type: string}}} S: const: a enum: [a, b] allOf: [{$ref: '#/components/schemas/Base'}] `)) - requireNoErrorDiags(t, diags) + openapitest.RequireNoErrorDiags(t, diags) p := typeByName(doc, "S").Common().Unmodeled assert.Contains(t, p, "openapi:enum") assert.Contains(t, p, "openapi:allOf") assert.Contains(t, - diagMessageAt(t, diags, diag.DegradedConstruct, ir.SeverityInfo, "/components/schemas/S"), + openapitest.DiagMessageAt(t, diags, diag.DegradedConstruct, ir.SeverityInfo, "/components/schemas/S"), "declares enum and allOf beside its const", "both are named in the one report") } @@ -3562,7 +3714,7 @@ func TestCoDeclaredFamily_EveryPassedOverKeywordIsKept(t *testing.T) { // survived. func TestCoDeclaredFamily_UnpreservableIsNotAnnounced(t *testing.T) { t.Parallel() - doc, diags := lowerSpec(t, componentSpec(` S: + doc, diags := lowerSpec(t, openapitest.ComponentSpec(` S: allOf: [{type: object, x-t: `+unpreservableValue+`}] enum: [a, b] `)) @@ -3570,7 +3722,356 @@ func TestCoDeclaredFamily_UnpreservableIsNotAnnounced(t *testing.T) { td := typeByName(doc, "S") require.NotNil(t, td) assert.NotContains(t, td.Common().Unmodeled, "openapi:allOf", "the conversion failed") - assert.True(t, hasDiag(diags, diag.UnpreservableConstruct), "the failure itself is reported") + assert.True(t, openapitest.HasDiag(diags, diag.UnpreservableConstruct), "the failure itself is reported") assert.Empty(t, preservationClaims(diags), "nothing was written under Unmodeled, so nothing may announce that it was") } + +// keywordCensusSpec wraps the two $ref targets every ref-site row aliases in a +// component block, so a row states only what it declares beside its $ref. +func keywordCensusSpec(schemas string) string { + return openapitest.ComponentSpec(" BaseStr: {type: string}\n" + + " BaseObj: {type: object, properties: {a: {type: string}}}\n" + schemas) +} + +// assertKeptVerbatim asserts p holds key as a degraded-lowering entry whose raw +// payload is raw. +func assertKeptVerbatim(t *testing.T, p ir.Unmodeled, key, raw string) { + t.Helper() + entry, ok := p[key] + require.True(t, ok, "%q is kept verbatim; kept instead: %v", key, unmodeledKeys(p)) + assert.Equal(t, ir.ReasonDegradedLowering, entry.Reason) + assert.JSONEq(t, raw, string(entry.Value)) +} + +// unmodeledKeys returns p's keys in sorted order, which is what makes an +// expected key set assertable as a whole: a census that keeps too much fails the +// same test as one that keeps too little. +func unmodeledKeys(p ir.Unmodeled) []string { + if len(p) == 0 { + return nil + } + out := make([]string, 0, len(p)) + for key := range p { + out = append(out, key) + } + sort.Strings(out) + return out +} + +// TestRefSiteKeywords_KeptAtEveryPosition covers the census the $ref path never +// ran (GitHub #283). +// +// In JSON Schema 2020-12 — and so in OpenAPI 3.1 — `$ref` is an ordinary keyword +// and what stands beside it is conjoined with it. The position lowers to an alias +// over the target, which has no property set, no member set, no value and no +// encoding of its own, so each of these keywords reached no IR field at all: no +// field, no Unmodeled entry and no diagnostic either. +// +// Every row is checked at both positions, because they take different paths: a +// component is an annotation.HomeOwnNode position and keeps the keyword on the +// alias it hoists, a property is an annotation.HomeCarrier one and keeps it on +// itself, and only the first of the two ran any census at all. +func TestRefSiteKeywords_KeptAtEveryPosition(t *testing.T) { + t.Parallel() + for _, tc := range []struct{ keyword, sibling, target, raw string }{ + {"format", "format: email", "BaseStr", `"email"`}, + {"enum", "enum: [a, b]", "BaseStr", `["a","b"]`}, + {"const", "const: a", "BaseStr", `"a"`}, + {"required", "required: [a]", "BaseObj", `["a"]`}, + {"additionalProperties", "additionalProperties: false", "BaseObj", `false`}, + } { + t.Run(tc.keyword, func(t *testing.T) { + t.Parallel() + site := "{$ref: '#/components/schemas/" + tc.target + "', " + tc.sibling + "}" + doc, diags := lowerSpec(t, keywordCensusSpec( + " Alias: "+site+"\n"+ + " Holder: {type: object, properties: {p: "+site+"}}\n")) + openapitest.RequireNoErrorDiags(t, diags) + key := "openapi:" + tc.keyword + + alias, ok := typeByName(doc, "Alias").(*ir.Scalar) + require.True(t, ok, "the component position hoists an alias to hold what it wrote") + require.NotNil(t, alias.Base) + assert.Equal(t, componentID(tc.target), alias.Base.Target, "and still aliases the target") + assertKeptVerbatim(t, alias.Unmodeled, key, tc.raw) + assert.Contains(t, + openapitest.DiagMessageAt(t, diags, diag.DegradedConstruct, ir.SeverityInfo, "/components/schemas/Alias"), + "no home for "+tc.keyword) + + holder, ok := typeByName(doc, "Holder").(*ir.Model) + require.True(t, ok) + p, ok := openapitest.PropsByWire(holder.Properties)["p"] + require.True(t, ok) + assert.Equal(t, componentID(tc.target), p.Type.Target, + "the carrier still resolves straight to the target; only the loss is now recorded") + assertKeptVerbatim(t, p.Unmodeled, key, tc.raw) + assert.Contains(t, + openapitest.DiagMessageAt(t, diags, diag.DegradedConstruct, ir.SeverityInfo, + "/components/schemas/Holder/properties/p"), + "no home for "+tc.keyword) + }) + } +} + +// TestRefSiteKeywords_SiblingsWithATypedHomeAreUntouched is the control the +// census has to leave alone. A bound and a description written at the identical +// position always did reach a field — the alias's Constraints and the property's +// own — so neither may acquire an Unmodeled entry or a diagnostic now. +func TestRefSiteKeywords_SiblingsWithATypedHomeAreUntouched(t *testing.T) { + t.Parallel() + site := "{$ref: '#/components/schemas/BaseStr', minLength: 3, description: kept}" + doc, diags := lowerSpec(t, keywordCensusSpec( + " Alias: "+site+"\n"+ + " Holder: {type: object, properties: {p: "+site+"}}\n")) + openapitest.RequireNoErrorDiags(t, diags) + assert.Zero(t, openapitest.CountDiagsAt(diags, diag.DegradedConstruct, ir.SeverityInfo), + "nothing was degraded: %+v", diags) + + alias, ok := typeByName(doc, "Alias").(*ir.Scalar) + require.True(t, ok) + require.NotNil(t, alias.Constraints) + assert.Equal(t, int64(3), *alias.Constraints.MinLength) + assert.Equal(t, "kept", alias.Docs.Description) + assert.Empty(t, unmodeledKeys(alias.Unmodeled)) + + holder, ok := typeByName(doc, "Holder").(*ir.Model) + require.True(t, ok) + p, ok := openapitest.PropsByWire(holder.Properties)["p"] + require.True(t, ok) + require.NotNil(t, p.Constraints) + assert.Equal(t, int64(3), *p.Constraints.MinLength) + assert.Equal(t, "kept", p.Docs.Description) + assert.Empty(t, unmodeledKeys(p.Unmodeled)) +} + +// TestUnhomedKeywords_ElectedLoweringKeepsWhatItCannotRead covers the keywords +// the winning lowering never reads (GitHub #268). +// +// lower() elects one keyword family per position; what the elected form has no +// field for was dropped, because the census that ran was a fixed list of shape +// applicators. A Model has no type token, a Literal has no encoding and no +// Constraints, and none of that is visible from a keyword list — only from the +// node that was built, which is what the census asks now. +// +// The kept set is asserted whole, so a census that keeps too much fails here as +// loudly as one that keeps too little; the `type: object` row is the case that +// makes that matter, since a Model does restate it and nothing may be recorded. +func TestUnhomedKeywords_ElectedLoweringKeepsWhatItCannotRead(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name, body string + kind ir.TypeKind + kept []string + }{ + {"non-object type beside allOf", "{allOf: [{$ref: '#/components/schemas/BaseObj'}], type: string}", + ir.KindModel, []string{"openapi:type"}}, + {"object type beside allOf", "{allOf: [{$ref: '#/components/schemas/BaseObj'}], type: object}", + ir.KindModel, nil}, + {"format beside const", "{const: 5, type: integer, format: int32}", + ir.KindLiteral, []string{"openapi:format", "openapi:type"}}, + {"contradictory type beside const", "{const: 5, type: string}", + ir.KindLiteral, []string{"openapi:type"}}, + {"value constraint beside const", "{const: ab, type: string, maxLength: 1}", + ir.KindLiteral, []string{"openapi:maxLength", "openapi:type"}}, + // A collection bound is homed by ir.List.Constraints and by nothing else. + // listConstraints is its only reader and only lowerArray calls it, so an + // object keeps it here; so does a Tuple, which has no Constraints field at + // all. Both reached the IR in no form before they joined the census. + {"collection bound on an object", "{type: object, properties: {f: {type: string}}, minItems: 3}", + ir.KindModel, []string{"openapi:minItems"}}, + {"collection bound beside prefixItems", "{type: array, prefixItems: [{type: string}], maxItems: 2}", + ir.KindTuple, []string{"openapi:maxItems"}}, + // A Scalar rather than the shared Primitive the type alone would reach: + // the entry needs a node this pointer owns, so the census hoists the alias + // that carries it. + {"unique items on a string", "{type: string, uniqueItems: true}", + ir.KindScalar, []string{"openapi:uniqueItems"}}, + // The control: the one node that does carry them keeps nothing. + {"collection bounds on an array", "{type: array, items: {type: string}, minItems: 3, maxItems: 9, uniqueItems: true}", + ir.KindList, nil}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + doc, diags := lowerSpec(t, keywordCensusSpec(" S: "+tc.body+"\n")) + openapitest.RequireNoErrorDiags(t, diags) + + td := typeByName(doc, "S") + require.NotNil(t, td, "the position still lowers") + assert.Equal(t, tc.kind, td.Kind(), "and lowers to the form the election picked") + assert.Equal(t, tc.kept, unmodeledKeys(td.Common().Unmodeled)) + + want := 1 + if len(tc.kept) == 0 { + want = 0 + } + assert.Equal(t, want, len(diagsAtPointer(diags, diag.DegradedConstruct, "/components/schemas/S")), + "a position keeps nothing quietly and reports everything it keeps: %+v", diags) + }) + } +} + +// TestCoDeclaredBound_KeptOnTheCarrierThatReadIt pins the two carriers this +// package owns for a 2020-12 side that declares both of its bound keywords +// (GitHub #286). ir.Constraints holds one bound per side, so one keyword reaches +// no field of it, and without an entry beside those constraints +// {minimum: 10, exclusiveMinimum: 0} lowers to exactly what {minimum: 10} does. +// +// Both directions run at both carriers. A case where the exclusive keyword is +// the one kept verbatim passes just as well on a reader that always kept that +// one, so on its own it would say nothing about which keyword the carrier holds. +func TestCoDeclaredBound_KeptOnTheCarrierThatReadIt(t *testing.T) { + t.Parallel() + doc, diags := parseFull(t, openapitest.ComponentSpec( + " Alias: {type: integer, minimum: 10, exclusiveMinimum: 0}\n"+ + " Tight: {type: integer, maximum: 100, exclusiveMaximum: 5}\n"+ + " Holder:\n type: object\n properties:\n"+ + " low: {type: integer, minimum: 10, exclusiveMinimum: 0}\n"+ + " high: {type: integer, maximum: 100, exclusiveMaximum: 5}\n")) + openapitest.RequireNoErrorDiags(t, diags) + + tests := []struct { + name string + unmod ir.Unmodeled + bound *ir.Constraints + wantKept string + wantRaw string + at string + }{ + { + name: "alias node keeps the exclusive bound the minimum implies", + unmod: typeByName(doc, "Alias").Common().Unmodeled, + bound: typeByName(doc, "Alias").(*ir.Scalar).Constraints, + wantKept: "openapi:exclusiveMinimum", wantRaw: "0", + at: "/components/schemas/Alias/exclusiveMinimum", + }, + { + name: "alias node keeps the inclusive bound the exclusive one implies", + unmod: typeByName(doc, "Tight").Common().Unmodeled, + bound: typeByName(doc, "Tight").(*ir.Scalar).Constraints, + wantKept: "openapi:maximum", wantRaw: "100", + at: "/components/schemas/Tight/maximum", + }, + { + name: "property keeps the exclusive bound the minimum implies", + unmod: propertyOf(t, doc, "Holder", "low").Unmodeled, + bound: propertyOf(t, doc, "Holder", "low").Constraints, + wantKept: "openapi:exclusiveMinimum", wantRaw: "0", + at: "/components/schemas/Holder/properties/low/exclusiveMinimum", + }, + { + name: "property keeps the inclusive bound the exclusive one implies", + unmod: propertyOf(t, doc, "Holder", "high").Unmodeled, + bound: propertyOf(t, doc, "Holder", "high").Constraints, + wantKept: "openapi:maximum", wantRaw: "100", + at: "/components/schemas/Holder/properties/high/maximum", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + require.NotNil(t, tc.bound, "the tighter bound still reaches ir.Constraints") + entry, ok := tc.unmod[tc.wantKept] + require.True(t, ok, "%s is kept beside the constraints it did not reach; got %v", + tc.wantKept, tc.unmod) + assert.Equal(t, ir.ReasonDegradedLowering, entry.Reason) + assert.JSONEq(t, tc.wantRaw, string(entry.Value)) + assert.Equal(t, tc.at, entry.Provenance.Pointer) + }) + } +} + +// diagsAtPointer returns the diagnostics in diags with the exact code whose +// provenance is pointer. +func diagsAtPointer(diags []ir.Diagnostic, code, pointer string) []ir.Diagnostic { + var out []ir.Diagnostic + for _, d := range diags { + if d.Code == code && d.Provenance.Pointer == pointer { + out = append(out, d) + } + } + return out +} + +// TestUnhomedKeywords_BoundsThatLandedAreNotAlsoKept is the other half of the +// value-constraint census: a bound is kept only where the node has no field it +// reached, never where it did. The two rows are the two node kinds that read +// annotation.Constraints, so a census asking the kind rather than the field +// would double-record both. +func TestUnhomedKeywords_BoundsThatLandedAreNotAlsoKept(t *testing.T) { + t.Parallel() + for _, tc := range []struct{ name, body, unhomed string }{ + {"scalar", "{type: string, format: email, minLength: 3, required: [a]}", "openapi:required"}, + {"model", "{type: object, properties: {a: {type: string}}, minProperties: 1, items: {type: string}}", "openapi:items"}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + doc, diags := lowerSpec(t, keywordCensusSpec(" S: "+tc.body+"\n")) + openapitest.RequireNoErrorDiags(t, diags) + + td := typeByName(doc, "S") + require.NotNil(t, td) + assert.Equal(t, []string{tc.unhomed}, unmodeledKeys(td.Common().Unmodeled), + "the bound reached the node's Constraints, so only the homeless keyword is kept") + }) + } +} + +// TestUnhomedKeywords_ArrayBoundsKeepWhatListConstraintsDoesNotRead pins the +// reason the census asks what filled a Constraints field rather than which kinds +// have one. An ir.List has the field, but lowerArray fills it from +// listConstraints — collection bounds only — so a string bound written on an +// array reaches nothing however full the field looks. +func TestUnhomedKeywords_ArrayBoundsKeepWhatListConstraintsDoesNotRead(t *testing.T) { + t.Parallel() + doc, diags := lowerSpec(t, keywordCensusSpec( + " S: {type: array, items: {type: string}, minItems: 1, minLength: 3}\n")) + openapitest.RequireNoErrorDiags(t, diags) + + l, ok := typeByName(doc, "S").(*ir.List) + require.True(t, ok) + require.NotNil(t, l.Constraints) + assert.Equal(t, int64(1), *l.Constraints.MinItems, "the collection bound lowers as it always did") + assert.Equal(t, []string{"openapi:minLength"}, unmodeledKeys(l.Unmodeled), + "and only the bound listConstraints does not read is kept") +} + +// TestRefSiteKeywords_AllOfBranchKeepsWhatTheAliasCannotHold runs the same +// census at the third $ref site: an allOf branch spelled as a reference, which +// homes its siblings on an alias exactly as a component does. +// +// `required` is the branch keyword this composition does read — +// applyCompositionRequired ORs every branch's list onto the composed model — so +// it must not be kept, or one keyword would be reported twice. +func TestRefSiteKeywords_AllOfBranchKeepsWhatTheAliasCannotHold(t *testing.T) { + t.Parallel() + doc, diags := lowerSpec(t, keywordCensusSpec( + " S: {allOf: [{$ref: '#/components/schemas/BaseObj', format: email, required: [a]}]}\n")) + openapitest.RequireNoErrorDiags(t, diags) + + m, ok := typeByName(doc, "S").(*ir.Model) + require.True(t, ok) + require.NotNil(t, m.Base, "the branch still composes") + branch, ok := doc.Types[m.Base.Target].(*ir.Scalar) + require.True(t, ok, "through an alias hoisted at the branch position") + assert.Equal(t, []string{"openapi:format"}, unmodeledKeys(branch.Unmodeled), + "required is read by the composition, so only the format is kept") + assert.Contains(t, + openapitest.DiagMessageAt(t, diags, diag.DegradedConstruct, ir.SeverityInfo, "/components/schemas/S/allOf/0"), + "no home for format") +} + +// TestCoDeclaredBound_ASingleKeywordKeepsNothing is the other half of the case +// above: a side writing one keyword has it in a field, so an entry restating it +// would give one bound two homes and make the two source shapes indistinguishable +// in the opposite direction. +func TestCoDeclaredBound_ASingleKeywordKeepsNothing(t *testing.T) { + t.Parallel() + doc, diags := parseFull(t, openapitest.ComponentSpec( + " Alias: {type: integer, minimum: 10}\n"+ + " Holder: {type: object, properties: {low: {type: integer, exclusiveMinimum: 0}}}\n")) + openapitest.RequireNoErrorDiags(t, diags) + + assert.Empty(t, typeByName(doc, "Alias").Common().Unmodeled) + assert.Empty(t, propertyOf(t, doc, "Holder", "low").Unmodeled) +} diff --git a/testdata/conformance/openapi/dynamic-ref.golden.json b/testdata/conformance/openapi/dynamic-ref.golden.json index 187d786..b3a4879 100644 --- a/testdata/conformance/openapi/dynamic-ref.golden.json +++ b/testdata/conformance/openapi/dynamic-ref.golden.json @@ -18,6 +18,61 @@ } ], "types": { + "t/anon/components/schemas/": { + "kind": "scalar", + "id": "t/anon/components/schemas/", + "name": { + "hint": "empty" + }, + "anonymous": true, + "docs": {}, + "sensitive": false, + "unmodeled": { + "openapi:$dynamicRef": { + "reason": "degraded_lowering", + "value": "#T", + "provenance": { + "source": 0, + "pointer": "/components/schemas//$dynamicRef" + } + }, + "openapi:$id": { + "reason": "out_of_scope", + "value": "https://example.com/empty", + "provenance": { + "source": 0, + "pointer": "/components/schemas//$id" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/components/schemas/" + }, + "base": { + "target": "t/prim/any", + "nullable": false + } + }, + "t/openapi/components/schemas/Leaf": { + "kind": "scalar", + "id": "t/openapi/components/schemas/Leaf", + "name": { + "source": "Leaf", + "canonical": "leaf" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Leaf" + }, + "base": { + "target": "t/prim/string", + "nullable": false + } + }, "t/openapi/components/schemas/Node": { "kind": "model", "id": "t/openapi/components/schemas/Node", @@ -107,6 +162,33 @@ "pointer": "/components/schemas/Tree/properties/child" } }, + { + "id": "p/openapi/components/schemas/Tree/properties/escaped", + "name": { + "source": "escaped", + "canonical": "escaped" + }, + "wireName": "escaped", + "type": { + "target": "t/openapi/components/schemas/Leaf", + "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/Tree/properties/escaped" + } + }, { "id": "p/openapi/components/schemas/Tree/properties/ghost", "name": { @@ -194,6 +276,15 @@ "pointer": "/components/schemas/Tree/properties/child/$dynamicRef" } }, + { + "severity": "info", + "code": "openapi/dynamic-ref-expanded", + "message": "$dynamicRef expanded to \"t/openapi/components/schemas/Leaf\", the one matching $dynamicAnchor in this document", + "provenance": { + "source": 0, + "pointer": "/components/schemas/Tree/properties/escaped/$dynamicRef" + } + }, { "severity": "info", "code": "openapi/degraded-construct", @@ -202,13 +293,31 @@ "source": 0, "pointer": "/components/schemas/Tree/properties/ghost/$dynamicRef" } + }, + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "$id identifies or configures a JSON Schema resource rather than describing data; the IR models no such axis, so it is kept verbatim under Unmodeled and is not honoured for reference resolution", + "provenance": { + "source": 0, + "pointer": "/components/schemas//$id" + } + }, + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "$dynamicRef was not expanded because an $id at or above \"/components/schemas/\" starts a schema resource of its own, and the IR resolves no resource base URIs; it is kept verbatim under Unmodeled", + "provenance": { + "source": 0, + "pointer": "/components/schemas//$dynamicRef" + } } ], "sources": [ { "format": "openapi@3.1", "path": "dynamic-ref.yaml", - "hash": "5232d8fc85addaa79bbe944d295786dbd6f206d0c74bdb02f1b1ee2a4f829baa" + "hash": "4ae3f35d31355eedd294ecae9d7a312ec39cb830a1ed5826b90404d29eaf529b" } ] } diff --git a/testdata/conformance/openapi/dynamic-ref.yaml b/testdata/conformance/openapi/dynamic-ref.yaml index 130fad4..abe2f71 100644 --- a/testdata/conformance/openapi/dynamic-ref.yaml +++ b/testdata/conformance/openapi/dynamic-ref.yaml @@ -8,12 +8,27 @@ components: type: object properties: label: {type: string} + Leaf: + $dynamicAnchor: leaf-node + type: string Tree: type: object properties: # One $dynamicAnchor of this name is declared on a component schema, so # the reference expands to it. child: {$dynamicRef: '#T'} + # A fragment is URI text, in which '%2D' and '-' are the same character + # (RFC 3986 section 2.3), so this names the anchor 'leaf-node'. + escaped: {$dynamicRef: '#leaf%2Dnode'} # No anchor declares this name, so the reference is irreducible and is # kept verbatim instead of being dropped. ghost: {$dynamicRef: '#Missing'} + # A component keyed "" is addressed by /components/schemas/, a pointer whose + # last reference token is empty. The $id here starts a schema resource of its + # own, so this reference may not expand across it — and deciding that means + # reading the trailing empty token as this schema's own position instead of + # dropping it and reading the components/schemas map above it, which declares + # no $id and would have let the expansion through. + "": + $id: https://example.com/empty + $dynamicRef: '#T' From 30adb6603853148569c56f23e434a863a55a7c77 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Wed, 12 Aug 2026 00:09:20 +0300 Subject: [PATCH 3/4] docs(compilers/openapi): record why the pointer bound cannot bite here MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The walk discards PointerPath's completeness flag, which is right for a pointer that falls off the tree — the nodes above it were still read, and a boundary there still binds. It is not obviously right for the helper's other way of stopping short: giving up past maxPointerSegments cuts the walk off before the boundary and reports none, the one direction this function must not err in. That case is unreachable rather than handled, so say so and say why. maxSchemaDepth caps nesting at 256 and each level spends at most two reference tokens, leaving the longest pointer that arrives here around 515 against a bound of 1024. Measured, not reasoned: a 700-level spec degrades at depth 256, and a probe that panicked past 1024 segments never fired. --- compilers/openapi/internal/schema/schema.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/compilers/openapi/internal/schema/schema.go b/compilers/openapi/internal/schema/schema.go index 3d0923c..bc77c89 100644 --- a/compilers/openapi/internal/schema/schema.go +++ b/compilers/openapi/internal/schema/schema.go @@ -1722,6 +1722,14 @@ func componentSchemaAt(c lowering.Ctx, pointer string) *oas3.Schema { // the schema's own. An incomplete walk needs no separate arm: PointerPath yields // the nodes it did reach, and a boundary above a pointer that falls off the tree // still binds. +// +// Its other way of stopping short would matter, since a walk cut off before the +// boundary reports no boundary — the direction this function must not err in. +// PointerPath gives up past maxPointerSegments (1024), which schema lowering +// cannot reach: maxSchemaDepth caps nesting at 256 and each level spends at most +// two reference tokens, so the longest pointer arriving here runs about 515. +// Measured rather than reasoned — a 700-level spec degrades at depth 256 and the +// deepest pointer this saw was well inside the bound. func declaresResourceIDAbove(c lowering.Ctx, view *nodeview.View, pointer string) bool { root := nodeview.DocumentRoot(nodeview.Deref(c.Doc.GetRootNode())) path, _ := view.PointerPath(root, pointer) From 8499518b66a8a7891f6d28199681ef10263f74af Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Wed, 12 Aug 2026 00:54:13 +0300 Subject: [PATCH 4/4] fix(compilers/openapi): read "/" as a position, not as a reference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects, both the empty-token loss this branch exists to close. The boundary walk read "/" through nodeview.PointerPath, whose tokenless rule lands it on the document root because that is where a *reference* spelled that way resolves. Every pointer reaching this walk is a position this compiler built with ids.Ptr, and ids.Ptr("") spells the root member keyed "" exactly "/" — so an $id written on that member was walked past, and a $dynamicRef whose chain hops through it expanded across a resource boundary. DocumentPath now carries the position reading beside PointerPath's reference reading, sharing one walk. The test that asserted the old answer asserted the bug; it now expects the boundary. Sharing one nodeview.View across the boundary walks made the verdict depend on declaration order. nodeview memoizes a mapping's merge expansion, so a node first expanded shallowly is served from that memo to a later walk reaching it deeper than MergeDepthLimit allows: one document kept a reference verbatim declared P-then-Q and expanded it declared Q-then-P. The view is per call again. Its claimed benefit never reproduced — timings were identical and PointerPath already memoizes each path node within a single call, so the per-call view was being hit anyway. The order-invariance oracle cannot reach this: the construct needs YAML anchors and reverseMappings declines to permute them, so the sweep returns ok either way. A two-order diff is added directly, and reverting to a surviving view reddens it. Three pre-existing defects found while probing are filed rather than fixed here: a merge chain past MergeDepthLimit hides an $id from this same walk (#401), the cycle pre-scan's bound warning depends on declaration order through its own shared view (#402), and the oracle's silent declines (#403). The first is noted on the function it affects. --- compilers/openapi/internal/ids/ids.go | 5 ++ .../openapi/internal/nodeview/nodeview.go | 25 ++++++- .../nodeview/nodeview_internal_test.go | 28 ++++++++ compilers/openapi/internal/schema/schema.go | 71 ++++++++----------- .../internal/schema/schema_internal_test.go | 22 +++--- .../openapi/internal/schema/schema_test.go | 62 ++++++++++++++++ 6 files changed, 160 insertions(+), 53 deletions(-) diff --git a/compilers/openapi/internal/ids/ids.go b/compilers/openapi/internal/ids/ids.go index 0f23a9f..a1c1e53 100644 --- a/compilers/openapi/internal/ids/ids.go +++ b/compilers/openapi/internal/ids/ids.go @@ -189,6 +189,11 @@ func ForPointer(pointer string) ir.TypeID { // schema (/components/schemas/ with no deeper path) and returns its name. // Only this kind of component declares a named type in OpenAPI, which is why it // alone gates NamedType. +// +// It answers false for two unlike documents: one that declares no such entry, +// and one that declares it keyed "". Reporting the refusal as "not a component +// schema" is false for the second, since the pointer addresses exactly that — +// ComponentSchemaNamedEmpty separates them for a caller that has to say why. func ComponentSchemaName(pointer string) (string, bool) { kind, name, ok := ComponentEntry(pointer) return name, ok && kind == "schemas" diff --git a/compilers/openapi/internal/nodeview/nodeview.go b/compilers/openapi/internal/nodeview/nodeview.go index 724a8c8..e86b886 100644 --- a/compilers/openapi/internal/nodeview/nodeview.go +++ b/compilers/openapi/internal/nodeview/nodeview.go @@ -374,12 +374,35 @@ func InternalPointer(ref string) (string, bool) { // the node it stops at, and the caller's re-entrancy check exempts exactly that // node (GitHub #238). func (v *View) PointerPath(root *yaml.Node, pointer string) (path []*yaml.Node, complete bool) { + return v.walkPointer(root, pointer, tokenless(pointer)) +} + +// DocumentPath walks a pointer that names a position in this document rather +// than a reference some source wrote, and is otherwise PointerPath. +// +// The two part company on '/'. PointerPath lands it on the root because that is +// where the resolver lands it, a departure from RFC 6901 that tokenless records. +// A position built by ids.Ptr carries no such departure: ids.Ptr("") spells the +// root member whose key is the empty string exactly '/', so reading that as the +// root walks past the member the pointer names. Only the empty pointer names the +// root here. +// +// The distinction is load-bearing for a caller reading $id down a path: taking +// '/' for the root hides an $id written on that member, which is the same +// dropped-empty-token loss the rest of this walk exists to avoid. +func (v *View) DocumentPath(root *yaml.Node, pointer string) (path []*yaml.Node, complete bool) { + return v.walkPointer(root, pointer, pointer == "") +} + +// walkPointer is the shared walk; atRoot says whether pointer carries no tokens +// at all, which is the one question the two readings answer differently. +func (v *View) walkPointer(root *yaml.Node, pointer string, atRoot bool) (path []*yaml.Node, complete bool) { cur := Deref(root) if cur == nil { return nil, false } path = append(path, cur) - if tokenless(pointer) { + if atRoot { return path, true } diff --git a/compilers/openapi/internal/nodeview/nodeview_internal_test.go b/compilers/openapi/internal/nodeview/nodeview_internal_test.go index 8fec91a..542f53a 100644 --- a/compilers/openapi/internal/nodeview/nodeview_internal_test.go +++ b/compilers/openapi/internal/nodeview/nodeview_internal_test.go @@ -491,6 +491,34 @@ func TestPointerPath_RootTokenlessAndNil(t *testing.T) { assert.Nil(t, path, "a nil root reaches nothing") } +// TestDocumentPath_SeparatesTheLoneSlashFromTheRoot pins the one question the +// two readings answer differently. +// +// PointerPath lands '/' on the root because that is where the resolver lands a +// reference spelled that way. A pointer naming a position in this document +// carries no such departure: ids.Ptr("") spells the root member keyed "" exactly +// '/', so reading it as the root walks past the member the pointer names — and a +// caller reading $id down the path would miss one written there. +func TestDocumentPath_SeparatesTheLoneSlashFromTheRoot(t *testing.T) { + t.Parallel() + member := ymap(yscalar("$id"), yscalar("https://example.com/root-member")) + root := ymap(yscalar(""), member) + + path, complete := New().DocumentPath(root, "/") + assert.True(t, complete, `"/" resolves the one token it carries`) + assert.Equal(t, []*yaml.Node{root, member}, path, + `ids.Ptr("") spells the root member keyed "" as "/", so the walk descends into it`) + + path, complete = New().PointerPath(root, "/") + assert.True(t, complete) + assert.Equal(t, []*yaml.Node{root}, path, + "the reference reading stops at the root, which is what tokenless records") + + path, complete = New().DocumentPath(root, "") + assert.True(t, complete, "only the empty pointer names the root here") + assert.Equal(t, []*yaml.Node{root}, path) +} + // TestPointerPath_EmptyTokenIsARealToken pins the one token a walk must not // normalize away. RFC 6901 makes "" a reference token naming the key "", and the // resolver's own parser agrees (jsonpointer/navigation.go getNavigationStack, diff --git a/compilers/openapi/internal/schema/schema.go b/compilers/openapi/internal/schema/schema.go index bc77c89..97ddafb 100644 --- a/compilers/openapi/internal/schema/schema.go +++ b/compilers/openapi/internal/schema/schema.go @@ -1634,8 +1634,7 @@ func soleAnchorSite(c lowering.Ctx, anchors *AnchorIndex, name string) (at, why // The loop is bounded by seen: cur only ever takes values from the anchor // index, and each turn either returns or adds one of them. func dynamicChainVerdict(c lowering.Ctx, anchors *AnchorIndex, at, from string) (why string, ok bool, diags []ir.Diagnostic) { - view := anchors.nodes() - if declaresResourceIDAbove(c, view, from) { + if declaresResourceIDAbove(c, from) { return resourceBoundaryWhy(from), false, nil } seen := map[string]bool{} @@ -1644,7 +1643,7 @@ func dynamicChainVerdict(c lowering.Ctx, anchors *AnchorIndex, at, from string) return fmt.Sprintf("expanding it closes a cycle of $dynamicRef expansions back onto %q, "+ "leaving a type whose own base chain never terminates", from), false, diags } - if declaresResourceIDAbove(c, view, cur) { + if declaresResourceIDAbove(c, cur) { return resourceBoundaryWhy(cur), false, diags } seen[cur] = true @@ -1713,26 +1712,35 @@ func componentSchemaAt(c lowering.Ctx, pointer string) *oas3.Schema { // costs an expansion that would have been safe, where a missed one mints a // reference the IR cannot express. // -// The path comes from nodeview.PointerPath, which already reads a pointer the -// way this walk needs: it drops only the leading empty segment, so a later empty -// token still takes a step of its own and names the key "" — which is how a -// component schema named "" is addressed (/components/schemas/). Walking the -// tokens here instead had dropped every empty segment, stopping the walk above -// such a position and reading the $id of the components/schemas map rather than -// the schema's own. An incomplete walk needs no separate arm: PointerPath yields -// the nodes it did reach, and a boundary above a pointer that falls off the tree -// still binds. -// -// Its other way of stopping short would matter, since a walk cut off before the -// boundary reports no boundary — the direction this function must not err in. -// PointerPath gives up past maxPointerSegments (1024), which schema lowering -// cannot reach: maxSchemaDepth caps nesting at 256 and each level spends at most -// two reference tokens, so the longest pointer arriving here runs about 515. -// Measured rather than reasoned — a 700-level spec degrades at depth 256 and the -// deepest pointer this saw was well inside the bound. -func declaresResourceIDAbove(c lowering.Ctx, view *nodeview.View, pointer string) bool { +// The path comes from nodeview.DocumentPath, which drops only the leading empty +// segment: a later empty token still takes a step of its own and names the key +// "", which is how a component schema named "" is addressed +// (/components/schemas/). Walking the tokens here instead had dropped every +// empty segment, stopping above such a position and reading the $id of the +// components/schemas map rather than the schema's own. DocumentPath rather than +// PointerPath because every pointer arriving here is a position this compiler +// built with ids.Ptr: the two differ only on "/", which ids.Ptr("") uses to +// spell the root member named "", and which PointerPath lands on the root +// instead because that is where a *reference* resolves. +// +// An incomplete walk needs no separate arm: the path holds the nodes it did +// reach, and a boundary above a pointer that falls off the tree still binds. +// +// The view is built per call and deliberately not shared. nodeview memoizes a +// mapping's merge expansion, and a node first expanded shallowly is served from +// that memo to a later walk that reaches it deeper than MergeDepthLimit would +// allow — so a view outliving one walk makes this answer depend on which schema +// lowered first, which invariant #7 forbids. A per-call view costs one expansion +// per path node and is order-invariant. +// +// Known gap: a path node whose own merge chain exceeds MergeDepthLimit expands +// to nothing, so an $id written there is invisible and this reports no boundary +// — the direction it must not err in. That predates this walk and needs a +// reporting channel of its own; GitHub #401 carries it. +func declaresResourceIDAbove(c lowering.Ctx, pointer string) bool { + view := nodeview.New() root := nodeview.DocumentRoot(nodeview.Deref(c.Doc.GetRootNode())) - path, _ := view.PointerPath(root, pointer) + path, _ := view.DocumentPath(root, pointer) for _, n := range path { if view.ChildByToken(n, "$id") != nil { return true @@ -1818,25 +1826,6 @@ func declaresDynamicRef(s *oas3.Schema) bool { // trade for a keyword almost no document uses. type AnchorIndex struct { byName map[string][]string - view *nodeview.View -} - -// nodes returns the raw-tree view the resource-boundary walks share, building it -// on first use like the index beside it. -// -// A verdict walks the path once for the position and once per chain link, so a -// document reaches this several times per $dynamicRef it writes; a view built -// per walk throws away the expansion memo before anything can hit it. Sharing -// one is about keeping that memo rather than about a measured win — compiling -// merge-heavy specs (a 60-link chain on the walked path, 40 deep reference -// positions) timed the same either way, and the IR is byte-identical. What it -// buys is that the memo is now reachable at all, which is the reason the view -// carries one. -func (a *AnchorIndex) nodes() *nodeview.View { - if a.view == nil { - a.view = nodeview.New() - } - return a.view } // sites returns the pointers declaring the named $dynamicAnchor, building the diff --git a/compilers/openapi/internal/schema/schema_internal_test.go b/compilers/openapi/internal/schema/schema_internal_test.go index e7ea9fa..f312ef8 100644 --- a/compilers/openapi/internal/schema/schema_internal_test.go +++ b/compilers/openapi/internal/schema/schema_internal_test.go @@ -17,7 +17,6 @@ import ( "github.com/dexpace/morphic/compilers/openapi/internal/diag" "github.com/dexpace/morphic/compilers/openapi/internal/ids" "github.com/dexpace/morphic/compilers/openapi/internal/lowering" - "github.com/dexpace/morphic/compilers/openapi/internal/nodeview" "github.com/dexpace/morphic/compilers/openapi/internal/openapitest" "github.com/dexpace/morphic/ir" ) @@ -214,7 +213,7 @@ func TestDeclaresResourceIDAbove_WithoutARawTree(t *testing.T) { t.Parallel() l := newRawLowerer(&soa.OpenAPI{}) - assert.False(t, declaresResourceIDAbove(l.ctx, nodeview.New(), "/components/schemas/A"), + assert.False(t, declaresResourceIDAbove(l.ctx, "/components/schemas/A"), "a document with no raw tree declares no resource anywhere") } @@ -225,11 +224,11 @@ func TestDeclaresResourceIDAbove_WithoutARawTree(t *testing.T) { // addressed. Dropping those stopped the walk above the position and read the // $id of the components/schemas map instead of the schema's own. // -// The document root carries a member named "" of its own, and it is what makes -// the two rootward cases discriminating rather than decorative: both must read -// as "the root and nothing below it", so a walk that took "" for a token would -// descend into that member and find the $id parked there. Without it, "stopped -// at the root" and "descended and fell off the tree" are the same false. +// The document root carries a member named "" of its own, carrying an $id. It is +// what makes the two rootward cases discriminating rather than decorative: the +// empty pointer names the root and must not reach it, while "/" is precisely how +// ids.Ptr spells that member and must. Without the $id there, "stopped at the +// root" and "descended and fell off the tree" are the same false. func TestDeclaresResourceIDAbove_EmptySegmentIsATokenNotAnArtifact(t *testing.T) { t.Parallel() l, diags := loweredFor(t, `openapi: 3.1.0 @@ -266,14 +265,15 @@ components: "", false, `the empty pointer stops at the root, so the $id under the root's "" member is out of reach`, }, - "a lone slash names the document root too": { - "/", false, - `the resolver lands a lone slash on the root, so the $id under the root's "" member stays out of reach`, + "a lone slash names the root member keyed \"\"": { + "/", true, + `ids.Ptr("") spells that member "/", so its own $id is the boundary — reading "/" as the root, ` + + "the way a reference resolves, would walk straight past it", }, } { t.Run(name, func(t *testing.T) { t.Parallel() - assert.Equal(t, tc.want, declaresResourceIDAbove(l.ctx, nodeview.New(), tc.pointer), tc.why) + assert.Equal(t, tc.want, declaresResourceIDAbove(l.ctx, tc.pointer), tc.why) }) } } diff --git a/compilers/openapi/internal/schema/schema_test.go b/compilers/openapi/internal/schema/schema_test.go index 2175fd3..83ba96f 100644 --- a/compilers/openapi/internal/schema/schema_test.go +++ b/compilers/openapi/internal/schema/schema_test.go @@ -2,6 +2,7 @@ package schema_test import ( "encoding/json" + "fmt" "sort" "strings" "testing" @@ -3234,6 +3235,67 @@ func TestDynamicRef_IrreducibleIsKeptAndSaysWhy(t *testing.T) { } } +// mergeBoundOrderSpec writes one document whose resource boundary is reachable +// only through a YAML merge chain, with the two schemas that walk it declared in +// the given order. P's chain is 51 links and expands whole; Q's rides on P's tail +// for 71, past nodeview's MergeDepthLimit of 64. +func mergeBoundOrderSpec(pFirst bool) string { + var b strings.Builder + b.WriteString("openapi: 3.1.0\ninfo: {title: MergeBound, version: \"1\"}\npaths: {}\n") + b.WriteString("x-c0: &c0 {$id: 'https://example.com/boundary'}\n") + for i := 1; i <= 50; i++ { + fmt.Fprintf(&b, "x-c%d: &c%d {<<: *c%d}\n", i, i, i-1) + } + b.WriteString("x-d0: &d0 {<<: *c50}\n") + for i := 1; i <= 20; i++ { + fmt.Fprintf(&b, "x-d%d: &d%d {<<: *d%d}\n", i, i, i-1) + } + b.WriteString("components:\n schemas:\n Node: {$dynamicAnchor: T, type: string}\n") + const p = " P:\n <<: *c50\n $dynamicRef: '#T'\n" + const q = " Q:\n <<: *d20\n $dynamicRef: '#T'\n" + if pFirst { + b.WriteString(p + q) + return b.String() + } + b.WriteString(q + p) + return b.String() +} + +// TestDynamicRef_ResourceBoundaryVerdictIsOrderInvariant pins that which schema +// lowered first cannot decide whether the other sees the $id above it. +// +// declaresResourceIDAbove builds its nodeview.View per call for this reason. A +// view outliving one walk memoizes a mapping's merge expansion, and a node first +// expanded shallowly is then served from that memo to a walk reaching it deeper +// than MergeDepthLimit permits. Sharing one made P-then-Q keep Q's reference +// verbatim where Q-then-P expanded it, in the same document. +// +// The order-invariance oracle cannot ask this. The construct needs YAML anchors, +// and reverseMappings declines to permute a document whose aliases the reversal +// would lift above their anchors, so the sweep returns ok whichever way the +// walk answers — which is what earns this a two-order diff of its own. +// +// It asserts the two orders agree rather than which verdict they agree on: a +// chain past the bound expands to nothing, so the $id is invisible and both +// currently miss the boundary (GitHub #401). Fixing that changes the shared +// answer, not this test. +func TestDynamicRef_ResourceBoundaryVerdictIsOrderInvariant(t *testing.T) { + t.Parallel() + first, diags := parseFull(t, mergeBoundOrderSpec(true)) + openapitest.RequireNoErrorDiags(t, diags) + last, diags := parseFull(t, mergeBoundOrderSpec(false)) + openapitest.RequireNoErrorDiags(t, diags) + + // The registry rather than the whole document: the cycle pre-scan shares one + // view across its own walk, so whether it reports stopping at the same + // 64-level bound depends on which schema it reached first, and that warning + // appears in one order only (GitHub #402). It is the same mechanism in a + // different walk, and it is not what this test governs — the boundary verdict + // lands in the registry. + assert.Empty(t, cmp.Diff(first.Types, last.Types, orderInvariantIR()...), + "the declaration order must not decide a resource-boundary verdict") +} + // TestDynamicRef_CycleIsRefusedAtEveryEdge pins that a cycle of $dynamicRef // expansions is preserved whole rather than broken at whichever edge lowered // first. Each member reaches the verdict from its own position, so no Scalar