From 59e88dda2fe84b3e98a87c5a5ed043898ad8d081 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Thu, 6 Aug 2026 03:54:26 +0300 Subject: [PATCH 1/5] fix(compilers/openapi): keep the keywords a schema co-declares MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Schema lowering picks one keyword per position and never revisits the rest of the body, so a schema writing more than one lost everything but the winner — with no Unmodeled entry and no diagnostic. Both halves violate "lossless by default": a compiler that cannot model something must keep it verbatim and say why. Two independent sites, one mechanism. lower() elects one keyword family in the order const, enum, allOf, and first match wins. `allOf: [{$ref: Base}]` beside an `enum` is the ordinary narrowing idiom, not a malformed document, and it compiled to a bare Enum with the whole relationship to Base gone. The same happened for allOf beside const, and for enum beside const. The election is now derived once (dispatchOf) and drives both the arm lower() takes and the set recordSkippedFamilies keeps, so the winner and the losers cannot be read off two tests that disagree. unionBranches takes oneOf whenever it is written and falls back to anyOf only when it is not, so a schema declaring both silently lost the anyOf. The elected set still becomes the Union — that is a shape the IR can express, and discarding it would model nothing at all — and the set passed over is kept on the Union node. nullUnionCollapse shared the same preference and the same loss; a {X, null} oneOf beside an anyOf now declines to collapse, since the collapse asserts the position *is* nullable X while a co-declared anyOf conjoins with it, and collapsing would resolve the position to a shared primitive that must never carry one declaration's keywords. Neither half flattens or merges anything. Every kept entry records ReasonDegradedLowering and the pointer it was written at, and each position reports only what it actually stored, so a payload that fails to convert is reported as unpreservable rather than announced as kept. A keyword the *elected* lowering never reads — `type: string` beside an allOf, `format` beside a const — is a different question, since `allOf` beside `type: object` is the common case and loses nothing. It needs a per-winner rule rather than a keyword list and is left open at #268. --- compilers/openapi/conformance_test.go | 1 + .../openapi/conformance_unmodeled_test.go | 55 +++ compilers/openapi/internal/schema/compose.go | 41 +- .../openapi/internal/schema/compose_test.go | 99 +++++ compilers/openapi/internal/schema/schema.go | 143 ++++++- .../internal/schema/schema_internal_test.go | 14 +- .../openapi/internal/schema/schema_test.go | 102 +++++ .../openapi/codeclared-keywords.golden.json | 392 ++++++++++++++++++ .../openapi/codeclared-keywords.yaml | 54 +++ 9 files changed, 885 insertions(+), 16 deletions(-) create mode 100644 testdata/conformance/openapi/codeclared-keywords.golden.json create mode 100644 testdata/conformance/openapi/codeclared-keywords.yaml diff --git a/compilers/openapi/conformance_test.go b/compilers/openapi/conformance_test.go index 3d0ca67..e1e236a 100644 --- a/compilers/openapi/conformance_test.go +++ b/compilers/openapi/conformance_test.go @@ -152,6 +152,7 @@ func conformanceCases() []conformanceCase { {"discriminator-inheritance", assertDiscriminatorInheritance}, {"discriminator-default-mapping", assertDiscriminatorDefaultMapping}, {"unhomed-keywords", assertUnhomedKeywords}, + {"codeclared-keywords", assertCoDeclaredKeywords}, {"anyof-untagged", assertAnyOfUntagged}, {"negation-not", assertNegationNot}, {"dependent-required", assertDependentRequired}, diff --git a/compilers/openapi/conformance_unmodeled_test.go b/compilers/openapi/conformance_unmodeled_test.go index 48560d1..7244a8c 100644 --- a/compilers/openapi/conformance_unmodeled_test.go +++ b/compilers/openapi/conformance_unmodeled_test.go @@ -402,3 +402,58 @@ func assertResponseLinks(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { _, ok = opByName(doc, "getOrder") assert.True(t, ok, "the operation a link names is an ordinary operation") } + +// assertCoDeclaredKeywords pins the keywords a schema writes that compete for +// one position (GitHub #35). +// +// The dispatch elects one of them and the rest were dropped outright — a whole +// relationship to a Base, or half a union, gone with no Unmodeled entry and no +// diagnostic at all. Every case here asserts the same three things: the elected +// keyword still lowers, the ones passed over are kept verbatim under +// ReasonDegradedLowering, and the position says so. +func assertCoDeclaredKeywords(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) { + base := `[{"$ref":"#/components/schemas/Base"}]` + e, ok := doc.Types[namedID("NarrowedEnum")].(*ir.Enum) + require.True(t, ok, "the enum is the value") + assertKeptRaw(t, e.Unmodeled, "openapi:allOf", base) + + lit, ok := doc.Types[namedID("NarrowedConst")].(*ir.Literal) + require.True(t, ok, "const outranks the composition beside it") + assertKeptRaw(t, lit.Unmodeled, "openapi:allOf", base) + + within, ok := doc.Types[namedID("ConstWithinEnum")].(*ir.Literal) + require.True(t, ok, "const is the narrower of the two") + assertKeptRaw(t, within.Unmodeled, "openapi:enum", `["a","b"]`) + + anyOf := `[{"type":"number"},{"type":"boolean"}]` + both, ok := doc.Types[namedID("BothCombinators")].(*ir.Union) + require.True(t, ok, "oneOf becomes the union") + assert.Len(t, both.Variants, 2, "with a variant per oneOf branch") + assertKeptRaw(t, both.Unmodeled, "openapi:anyOf", anyOf) + + // Suppressing the {X, null} collapse is what gives this one a node of its + // own: collapsing would resolve the position to the shared string primitive, + // which must never carry one declaration's keywords. + nullable, ok := doc.Types[namedID("NullableWithCombinators")].(*ir.Union) + require.True(t, ok, "a co-declared anyOf stops the {X, null} collapse") + assert.Len(t, nullable.Variants, 1, "the null branch still lifts off the variant list") + assertKeptRaw(t, nullable.Unmodeled, "openapi:anyOf", anyOf) + + for _, name := range []string{ + "NarrowedEnum", "NarrowedConst", "ConstWithinEnum", + "BothCombinators", "NullableWithCombinators", + } { + assert.Equal(t, []ir.Severity{ir.SeverityInfo}, + diagsAt(diags, "openapi/degraded-construct", "/components/schemas/"+name), + "%s announces the keyword it passed over", name) + } +} + +// assertKeptRaw requires p to hold key under ReasonDegradedLowering with the +// given JSON payload. +func assertKeptRaw(t *testing.T, p ir.Unmodeled, key, want string) { + t.Helper() + entry := unmodeledEntry(t, p, key) + assert.Equal(t, ir.ReasonDegradedLowering, entry.Reason) + assert.JSONEq(t, want, string(entry.Value)) +} diff --git a/compilers/openapi/internal/schema/compose.go b/compilers/openapi/internal/schema/compose.go index fb45a5a..909a558 100644 --- a/compilers/openapi/internal/schema/compose.go +++ b/compilers/openapi/internal/schema/compose.go @@ -671,6 +671,7 @@ type variantTypeFunc func(b *oas3.JSONSchema[oas3.Referenceable], vptr, vhint st // (internNode), so buildUnion needs no hint of its own to build one. func buildUnion(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, common ir.TypeCommon, pointer string, variantType variantTypeFunc) (ir.TypeDef, []ir.Diagnostic) { branches, key, exclusive := unionBranches(s) + diags := preserveUnusedCombinator(c, &common.Unmodeled, s, key, pointer) variants := make([]ir.Variant, 0, len(branches)) for i, b := range branches { if isNullSchema(b) { @@ -689,9 +690,45 @@ func buildUnion(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, common ir.Typ Exclusive: exclusive, WireTagged: false, } - disc, diags := lowerDiscriminator(c, ts, s, nil, pointer) + disc, discDiags := lowerDiscriminator(c, ts, s, nil, pointer) u.Discriminator = disc - return u, diags + return u, append(diags, discDiags...) +} + +// otherCombinator names each union keyword's counterpart, so the branch set +// unionBranches passed over is read off the choice it returned rather than +// restated beside it. +var otherCombinator = map[string]string{"oneOf": "anyOf", "anyOf": "oneOf"} + +// preserveUnusedCombinator keeps the branch set unionBranches passed over +// verbatim on the Union the position lowers to, and reports it. A schema +// declaring only one combinator passes over nothing and is left alone. +// +// A schema writing both conjoins them — an instance must satisfy the oneOf *and* +// the anyOf — and one ir.Union carries one branch set, so the loser had no place +// in the node and was dropped in silence. It is the union half of the same §4.8 +// rule recordSkippedFamilies applies to the keyword families: the position keeps +// lowering to the branch set unionBranches elects, because that Union is a shape +// the IR can express and discarding it too would model nothing at all, and the +// set it did not elect stays recoverable beside it. +// +// Where a *structural* sibling is written as well, classifyUnionSiblings reaches +// unionBothCombinators first and neither set is elected — there the sibling body +// is the most the IR can express, and distributing either union across it would +// drop the other (lowerBesideUnmodeledUnion). +func preserveUnusedCombinator(c lowering.Ctx, p *ir.Unmodeled, s *oas3.Schema, won, pointer string) []ir.Diagnostic { + if len(s.GetOneOf()) == 0 || len(s.GetAnyOf()) == 0 { + return nil + } + unused := otherCombinator[won] + kept, diags := PreserveSchemaKeyword(c, p, s, unused, ir.ReasonDegradedLowering, pointer+ids.Ptr(unused)) + if !kept { + return diags + } + return append(diags, c.DiagAt(ir.SeverityInfo, diag.DegradedConstruct, pointer, + "oneOf and anyOf are both declared here and conjoin, and one Union carries one "+ + "branch set; this position lowered as its %s, with %s kept verbatim under Unmodeled", + won, unused)) } // lowerDistributedUnion emits the Union that is the schema's value, distributing diff --git a/compilers/openapi/internal/schema/compose_test.go b/compilers/openapi/internal/schema/compose_test.go index fc218a3..0c55d28 100644 --- a/compilers/openapi/internal/schema/compose_test.go +++ b/compilers/openapi/internal/schema/compose_test.go @@ -1779,3 +1779,102 @@ func TestOneOf_CoDeclaredNotDistributedReasons(t *testing.T) { assert.Equal(t, 5, countDiagsAt(diags, diag.DegradedConstruct, ir.SeverityInfo), "each declined shape is reported once; got %+v", diags) } + +// TestUnionCombinators_PassedOverBranchSetIsKept covers the preference nothing +// used to record (GitHub #35). unionBranches takes oneOf whenever it is written +// and falls back to anyOf only when it is not, so a schema declaring both lost +// the anyOf outright — no variant, no Unmodeled entry, no diagnostic. The two +// conjoin, and one ir.Union carries one branch set, so the elected set stays the +// union and the other is kept beside it. +func TestUnionCombinators_PassedOverBranchSetIsKept(t *testing.T) { + t.Parallel() + doc, diags := lowerSpec(t, componentSpec(` S: + oneOf: [{type: string}, {type: integer}] + anyOf: [{type: number}, {type: boolean}] +`)) + requireNoErrorDiags(t, diags) + + u, ok := typeByName(doc, "S").(*ir.Union) + require.True(t, ok, "the elected branch set still becomes the union") + assert.Len(t, u.Variants, 2, "one variant per oneOf branch") + entry, ok := u.Unmodeled["openapi:anyOf"] + require.True(t, ok, "the set unionBranches passed over is kept; got %v", u.Unmodeled) + assert.Equal(t, ir.ReasonDegradedLowering, entry.Reason) + assert.JSONEq(t, `[{"type":"number"},{"type":"boolean"}]`, string(entry.Value)) + assert.Contains(t, + diagMessageAt(t, diags, diag.DegradedConstruct, ir.SeverityInfo, "/components/schemas/S"), + "lowered as its oneOf, with anyOf kept verbatim under Unmodeled") +} + +// TestUnionCombinators_NullBranchDoesNotCollapsePastTheAnyOf covers the second +// site of the same preference. A {X, null} oneOf normally collapses to nullable +// X, which resolves the position straight to X's own node — the shared string +// primitive here — leaving the co-declared anyOf nowhere of its own to sit; it +// was dropped there too. A schema writing both combinators is not nullable X in +// the first place, so it stays a Union and keeps the loser on it. +func TestUnionCombinators_NullBranchDoesNotCollapsePastTheAnyOf(t *testing.T) { + t.Parallel() + doc, diags := lowerSpec(t, componentSpec(` S: + oneOf: [{type: string}, {type: "null"}] + anyOf: [{type: number}, {type: boolean}] +`)) + requireNoErrorDiags(t, diags) + + u, ok := typeByName(doc, "S").(*ir.Union) + require.True(t, ok, "the collapse is declined; got %v", typeByName(doc, "S")) + require.Len(t, u.Variants, 1, "the null branch still lifts off the variant list") + assert.Equal(t, ir.TypeID("t/prim/string"), u.Variants[0].Type.Target) + assert.Contains(t, u.Unmodeled, "openapi:anyOf", + "and the node the position now owns carries the set passed over") +} + +// TestUnionCombinators_UnpreservableIsNotAnnounced pins the pairing GitHub #144 +// exists for, at this site: a position may only announce what it actually kept. +// The anyOf here fails to convert, so the union lowers, the failure is reported +// at the keyword, and nothing claims the branch set survived. +func TestUnionCombinators_UnpreservableIsNotAnnounced(t *testing.T) { + t.Parallel() + doc, diags := lowerSpec(t, componentSpec(` S: + oneOf: [{type: string}, {type: integer}] + anyOf: [{type: number, x-t: `+unpreservableValue+`}] +`)) + + u, ok := typeByName(doc, "S").(*ir.Union) + require.True(t, ok) + assert.NotContains(t, u.Unmodeled, "openapi:anyOf", "the conversion failed, so nothing was kept") + assert.True(t, 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") +} + +// TestUnionCombinators_KeepingIsOrderIndependent states the property over the +// whole document: declining the collapse gives this position a Union of its own +// and turns its branches into ordinary union branches, so an outside $ref naming +// one of them must reach the same IR whichever of the two is declared first. +// +// The branch writes a description on purpose. A bare `{type: string}` branch +// interns nothing through the union — it resolves to the shared primitive — so +// only the outside $ref would ever hoist a node there and the two could not +// disagree about one. Declaring something position-scoped makes both lowerings +// hoist the branch's home, which is the state where only the first to arrive +// interns it and the hints have to agree (branchHint, subSchemaHint, #181). +func TestUnionCombinators_KeepingIsOrderIndependent(t *testing.T) { + t.Parallel() + host := ` S: + oneOf: + - {type: string, description: the branch} + - {type: "null"} + anyOf: [{type: number}, {type: boolean}] +` + outsider := " Outsider: {$ref: '#/components/schemas/S/oneOf/0'}\n" + first, diags := parseFull(t, componentSpec(outsider+host)) + requireNoErrorDiags(t, diags) + last, diags := parseFull(t, componentSpec(host+outsider)) + requireNoErrorDiags(t, diags) + + require.Contains(t, first.Types, ir.TypeID("t/anon/components/schemas/S/oneOf/0"), + "the branch owns a node both lowerings reach, or there is nothing to race for") + assert.Empty(t, cmp.Diff(first, last, orderInvariantIR()...), + "declaring the reference before or after the union must not change the IR") + assert.Empty(t, cmp.Diff(first.Types, last.Types), "nor any name hint in the registry") +} diff --git a/compilers/openapi/internal/schema/schema.go b/compilers/openapi/internal/schema/schema.go index 133406c..d83ef6e 100644 --- a/compilers/openapi/internal/schema/schema.go +++ b/compilers/openapi/internal/schema/schema.go @@ -435,26 +435,87 @@ func falseSchema(c lowering.Ctx, ts *compile.Types, pointer, hint string) (ir.Ty return id, diags } +// familyOrder is the order lower() tries the keyword families that outrank the +// structural type, and dispatchOf is the sole reader of it. First match wins, so +// a schema declaring more than one of them lowers as the first and passes over +// the rest — which is why the two are derived together rather than separately. +var familyOrder = []string{"const", "enum", "allOf"} + +// declaresFamily reports whether s declares the named family. It is the single +// definition of each family's guard: lower() lowers what dispatchOf elects and +// recordSkippedFamilies keeps what the same walk passed over, so the winner and +// the losers can never be read off two tests that disagree. +func declaresFamily(s *oas3.Schema, family string) bool { + switch family { + case "const": + return s.GetConst() != nil + case "enum": + return len(s.GetEnum()) > 0 + default: // allOf + return len(s.GetAllOf()) > 0 + } +} + +// 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. +type dispatch struct { + won string + skipped []string +} + +// dispatchOf elects the family lower() lowers and collects the rest. A schema +// declaring none leaves won empty and skipped nil, so the type-set arms preserve +// nothing — and a family added to familyOrder but to no arm of lower() would be +// kept verbatim rather than dropped, which is the safe way for the two to +// disagree. +func dispatchOf(s *oas3.Schema) dispatch { + var d dispatch + for _, family := range familyOrder { + if !declaresFamily(s, family) { + continue + } + if d.won == "" { + d.won = family + continue + } + d.skipped = append(d.skipped, family) + } + return d +} + // lower interns the inline schema at pointer and returns its TypeID. Value // constraints (const, enum) and allOf composition take precedence over the // structural type; otherwise it dispatches on the effective (null-stripped) // type set. const hoists through hoistLiteral — the same primitive that // hoists each individual member of a heterogeneous enum (enumAsUnion) — since // a bare `const` schema is exactly a Literal at its own pointer. +// +// The families are conjoined where a schema writes more than one, so electing a +// winner is a §4.8 degradation rather than a reading of the source: the ones +// 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. 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) { - owner, ownDiags := preserveUnhomedKeywords(c, ts, s, pointer, hint, id) + owner, ownDiags := preserveUnhomedKeywords(c, ts, s, pointer, hint, id, d) return owner, append(diags, ownDiags...) } - if constVal := s.GetConst(); constVal != nil { - return unhomed(hoistLiteral(c, ts, constVal, pointer, hint)) - } - if len(s.GetEnum()) > 0 { + switch d.won { + case "const": + return unhomed(hoistLiteral(c, ts, s.GetConst(), pointer, hint)) + case "enum": return unhomed(lowerEnum(c, ts, s, pointer, hint)) - } - if len(s.GetAllOf()) > 0 { + case "allOf": return unhomed(lowerAllOf(c, ts, anchors, depth, s, pointer, hint)) } + // won is "" — the schema declares no family, so the type set decides. types := effectiveTypes(s) switch { case len(types) > 1: @@ -559,13 +620,17 @@ func unhomedKeywords(s *oas3.Schema, td ir.TypeDef) []string { // 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. -func preserveUnhomedKeywords(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, pointer, hint string, id ir.TypeID) (ir.TypeID, []ir.Diagnostic) { +// +// 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 +// they too must not land on a shared one. +func preserveUnhomedKeywords(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, pointer, hint string, id ir.TypeID, d dispatch) (ir.TypeID, []ir.Diagnostic) { td, ok, diags := registeredNode(c, ts, id, pointer) if !ok { return id, diags } unhomed := unhomedKeywords(s, td) - if len(unhomed) == 0 { + if len(unhomed) == 0 && len(d.skipped) == 0 { return id, diags } owner := id @@ -574,7 +639,51 @@ func preserveUnhomedKeywords(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, diags = append(diags, consDiags...) owner = internAlias(c, ts, pointer, hint, ir.TypeRef{Target: id}, cons) } - return owner, append(diags, recordUnhomedKeywords(c, ts, owner, s, unhomed, td.Kind(), pointer)...) + diags = append(diags, recordUnhomedKeywords(c, ts, owner, s, unhomed, td.Kind(), pointer)...) + return owner, append(diags, recordSkippedFamilies(c, ts, owner, s, d, pointer)...) +} + +// recordSkippedFamilies keeps verbatim the keyword families lower()'s election +// passed over, and reports them once. +// +// JSON Schema conjoins keywords, so `{allOf: [{$ref: Base}], enum: [a, b]}` is a +// narrowing of Base rather than a malformed document, and `{const: a, enum: [a, +// b]}` is a redundant but legal restatement. The IR has no intersection +// combinator (ir-design §15), so only one of them can be the value — but the +// loser was dropped outright, taking the whole relationship to Base with it and +// reporting nothing at all. Keeping it beside the elected form is §4.8's rule for +// every other unrepresentable conjunction here (preserveUnionSiblings, +// buildTuple's open tuple). +// +// It reports 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. +func recordSkippedFamilies(c lowering.Ctx, ts *compile.Types, owner ir.TypeID, s *oas3.Schema, d dispatch, pointer string) []ir.Diagnostic { + if len(d.skipped) == 0 { + return nil + } + td, ok, diags := registeredNode(c, ts, owner, pointer) + if !ok { + return diags + } + common := td.Common() + kept := make([]string, 0, len(d.skipped)) + for _, keyword := range d.skipped { + stored, storedDiags := PreserveSchemaKeyword(c, &common.Unmodeled, s, keyword, + ir.ReasonDegradedLowering, pointer+ids.Ptr(keyword)) + diags = append(diags, storedDiags...) + if stored { + kept = append(kept, keyword) + } + } + if len(kept) == 0 { + return diags + } + skipped := strings.Join(kept, " and ") + return append(diags, c.DiagAt(ir.SeverityInfo, diag.DegradedConstruct, pointer, + "this position declares %s beside its %s, and JSON Schema conjoins them where only "+ + "one can be the value; it lowered as the %s, with %s kept verbatim under Unmodeled", + skipped, d.won, d.won, skipped)) } // recordUnhomedKeywords stores each unhomed applicator on the owning node and @@ -1585,11 +1694,19 @@ func schemaAdmitsNull(s *oas3.Schema) bool { // pointer, and hint so it lowers as nullable X rather than a union node // (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). +// +// 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 +// not; the position falls through to the Union instead, which is the one node +// that keeps the branch set unionBranches passed over +// (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) { - variants, key := s.GetOneOf(), "oneOf" - if len(variants) == 0 { - variants, key = s.GetAnyOf(), "anyOf" + if len(s.GetOneOf()) > 0 && len(s.GetAnyOf()) > 0 { + return nil, "", "", false } + variants, key, _ := unionBranches(s) var nonNull *oas3.JSONSchema[oas3.Referenceable] nonNullIdx, nonNullCount, nullCount := -1, 0, 0 for i, v := range variants { diff --git a/compilers/openapi/internal/schema/schema_internal_test.go b/compilers/openapi/internal/schema/schema_internal_test.go index d292abe..82afe19 100644 --- a/compilers/openapi/internal/schema/schema_internal_test.go +++ b/compilers/openapi/internal/schema/schema_internal_test.go @@ -381,7 +381,7 @@ func TestDynamicAnchorIndex_ReportsATruncatedWalk(t *testing.T) { func TestPreserveUnhomedKeywords_MissingNode(t *testing.T) { t.Parallel() l := newRawLowerer(&soa.OpenAPI{}) - got, diags := preserveUnhomedKeywords(l.ctx, l.types, &oas3.Schema{}, "/p", "h", "t/anon/missing") + got, diags := preserveUnhomedKeywords(l.ctx, l.types, &oas3.Schema{}, "/p", "h", "t/anon/missing", dispatch{}) assert.Equal(t, ir.TypeID("t/anon/missing"), got, "the lowering's own ID still stands") assertInternalInvariant(t, diags) } @@ -397,6 +397,18 @@ func TestRecordUnhomedKeywords_MissingOwner(t *testing.T) { assertInternalInvariant(t, diags) } +// TestRecordSkippedFamilies_MissingOwner drives the same invariant on the +// keyword families the dispatch passed over. It is reached directly for the +// reason its unhomed-applicator twin is: the caller hands it either the node it +// just looked up or one internAlias just interned, so no source reaches this. +func TestRecordSkippedFamilies_MissingOwner(t *testing.T) { + t.Parallel() + l := newRawLowerer(&soa.OpenAPI{}) + diags := recordSkippedFamilies(l.ctx, l.types, "t/anon/missing", &oas3.Schema{}, + dispatch{won: "const", skipped: []string{"enum"}}, "/p") + assertInternalInvariant(t, diags) +} + // TestRefNullable_AnUnresolvedRefIsNotNullable pins the guard on the second half // of the question. A $ref site admits null when its own spelling says so or when // its target does; a reference the loader never resolved has no target to ask, diff --git a/compilers/openapi/internal/schema/schema_test.go b/compilers/openapi/internal/schema/schema_test.go index 927be7a..29cbe74 100644 --- a/compilers/openapi/internal/schema/schema_test.go +++ b/compilers/openapi/internal/schema/schema_test.go @@ -3442,3 +3442,105 @@ func TestUnhomedApplicator_FormatWithNoTypeIsKept(t *testing.T) { "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") } + +// TestCoDeclaredFamily_PassedOverKeywordIsKept covers the families lower()'s +// first-match dispatch used to drop (GitHub #35). +// +// const, enum and allOf compete for one position and only the first declared was +// read; the rest left no Unmodeled entry and no diagnostic. `allOf: [{$ref: +// Base}]` beside an enum is the narrowing idiom rather than a malformed +// document, so what vanished was the entire relationship to Base. +// +// Each case names the family the dispatch elects and the one it passes over, so +// a reordered dispatch fails here rather than quietly changing which half of a +// schema the IR describes. +func TestCoDeclaredFamily_PassedOverKeywordIsKept(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name, body, skipped, raw string + lowered ir.TypeKind + }{ + { + name: "allOf beside enum", + body: " allOf: [{$ref: '#/components/schemas/Base'}]\n enum: [a, b]\n", + skipped: "allOf", + raw: `[{"$ref":"#/components/schemas/Base"}]`, + lowered: ir.KindEnum, + }, + { + name: "allOf beside const", + body: " allOf: [{$ref: '#/components/schemas/Base'}]\n const: a\n", + skipped: "allOf", + raw: `[{"$ref":"#/components/schemas/Base"}]`, + lowered: ir.KindLiteral, + }, + { + name: "enum beside const", + body: " const: a\n enum: [a, b]\n", + skipped: "enum", + raw: `["a","b"]`, + lowered: ir.KindLiteral, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + doc, diags := lowerSpec(t, componentSpec( + " Base: {type: object, properties: {id: {type: string}}}\n S:\n"+tc.body)) + requireNoErrorDiags(t, diags) + + td := typeByName(doc, "S") + require.NotNil(t, td) + assert.Equal(t, tc.lowered, td.Kind(), "the elected family still lowers") + entry, ok := td.Common().Unmodeled["openapi:"+tc.skipped] + require.True(t, ok, "%s is kept verbatim; got %v", tc.skipped, td.Common().Unmodeled) + assert.Equal(t, ir.ReasonDegradedLowering, entry.Reason) + assert.JSONEq(t, tc.raw, string(entry.Value)) + 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"), + tc.skipped+" kept verbatim under Unmodeled") + }) + } +} + +// TestCoDeclaredFamily_EveryPassedOverKeywordIsKept pins the plural arm: a +// schema writing all three keeps both of the two it did not elect, named +// 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}}} + S: + const: a + enum: [a, b] + allOf: [{$ref: '#/components/schemas/Base'}] +`)) + 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"), + "declares enum and allOf beside its const", + "both are named in the one report") +} + +// TestCoDeclaredFamily_UnpreservableIsNotAnnounced pins the pairing GitHub #144 +// exists for, at this site: the allOf fails to convert, so the enum lowers, the +// failure is reported at the keyword, and nothing claims the composition +// survived. +func TestCoDeclaredFamily_UnpreservableIsNotAnnounced(t *testing.T) { + t.Parallel() + doc, diags := lowerSpec(t, componentSpec(` S: + allOf: [{type: object, x-t: `+unpreservableValue+`}] + enum: [a, b] +`)) + + 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.Empty(t, preservationClaims(diags), + "nothing was written under Unmodeled, so nothing may announce that it was") +} diff --git a/testdata/conformance/openapi/codeclared-keywords.golden.json b/testdata/conformance/openapi/codeclared-keywords.golden.json new file mode 100644 index 0000000..3987666 --- /dev/null +++ b/testdata/conformance/openapi/codeclared-keywords.golden.json @@ -0,0 +1,392 @@ +{ + "irVersion": "0.3.0", + "name": "CoDeclaredKeywords", + "version": "1.0.0", + "docs": {}, + "services": [ + { + "id": "s/openapi/0", + "name": { + "source": "CoDeclaredKeywords", + "canonical": "co_declared_keywords" + }, + "docs": {}, + "auth": null, + "provenance": { + "source": 0 + } + } + ], + "types": { + "t/openapi/components/schemas/Base": { + "kind": "model", + "id": "t/openapi/components/schemas/Base", + "name": { + "source": "Base", + "canonical": "base" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Base" + }, + "properties": [ + { + "id": "p/openapi/components/schemas/Base/properties/id", + "name": { + "source": "id", + "canonical": "id" + }, + "wireName": "id", + "type": { + "target": "t/prim/string", + "nullable": false + }, + "required": false, + "clientOptional": false, + "defaultAdded": false, + "visibility": { + "none": false + }, + "flatten": false, + "eventHeader": false, + "eventPayload": false, + "secret": false, + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Base/properties/id" + } + } + ], + "abstract": false, + "positional": false, + "inputOnly": false + }, + "t/openapi/components/schemas/BothCombinators": { + "kind": "union", + "id": "t/openapi/components/schemas/BothCombinators", + "name": { + "source": "BothCombinators", + "canonical": "both_combinators" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "unmodeled": { + "openapi:anyOf": { + "reason": "degraded_lowering", + "value": [ + { + "type": "number" + }, + { + "type": "boolean" + } + ], + "provenance": { + "source": 0, + "pointer": "/components/schemas/BothCombinators/anyOf" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/components/schemas/BothCombinators" + }, + "variants": [ + { + "name": { + "hint": "variant_0" + }, + "type": { + "target": "t/prim/string", + "nullable": false + }, + "docs": {} + }, + { + "name": { + "hint": "variant_1" + }, + "type": { + "target": "t/prim/integer", + "nullable": false + }, + "docs": {} + } + ], + "exclusive": true, + "wireTagged": false + }, + "t/openapi/components/schemas/ConstWithinEnum": { + "kind": "literal", + "id": "t/openapi/components/schemas/ConstWithinEnum", + "name": { + "source": "ConstWithinEnum", + "canonical": "const_within_enum" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "unmodeled": { + "openapi:enum": { + "reason": "degraded_lowering", + "value": [ + "a", + "b" + ], + "provenance": { + "source": 0, + "pointer": "/components/schemas/ConstWithinEnum/enum" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/components/schemas/ConstWithinEnum" + }, + "value": { + "kind": "string", + "str": "a", + "bytes": null, + "list": null, + "object": null + } + }, + "t/openapi/components/schemas/NarrowedConst": { + "kind": "literal", + "id": "t/openapi/components/schemas/NarrowedConst", + "name": { + "source": "NarrowedConst", + "canonical": "narrowed_const" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "unmodeled": { + "openapi:allOf": { + "reason": "degraded_lowering", + "value": [ + { + "$ref": "#/components/schemas/Base" + } + ], + "provenance": { + "source": 0, + "pointer": "/components/schemas/NarrowedConst/allOf" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/components/schemas/NarrowedConst" + }, + "value": { + "kind": "string", + "str": "a", + "bytes": null, + "list": null, + "object": null + } + }, + "t/openapi/components/schemas/NarrowedEnum": { + "kind": "enum", + "id": "t/openapi/components/schemas/NarrowedEnum", + "name": { + "source": "NarrowedEnum", + "canonical": "narrowed_enum" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "unmodeled": { + "openapi:allOf": { + "reason": "degraded_lowering", + "value": [ + { + "$ref": "#/components/schemas/Base" + } + ], + "provenance": { + "source": 0, + "pointer": "/components/schemas/NarrowedEnum/allOf" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/components/schemas/NarrowedEnum" + }, + "valueType": "string", + "members": [ + { + "name": { + "source": "a", + "canonical": "a" + }, + "value": { + "kind": "string", + "str": "a", + "bytes": null, + "list": null, + "object": null + }, + "docs": {} + }, + { + "name": { + "source": "b", + "canonical": "b" + }, + "value": { + "kind": "string", + "str": "b", + "bytes": null, + "list": null, + "object": null + }, + "docs": {} + } + ], + "closed": true, + "flags": false + }, + "t/openapi/components/schemas/NullableWithCombinators": { + "kind": "union", + "id": "t/openapi/components/schemas/NullableWithCombinators", + "name": { + "source": "NullableWithCombinators", + "canonical": "nullable_with_combinators" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "unmodeled": { + "openapi:anyOf": { + "reason": "degraded_lowering", + "value": [ + { + "type": "number" + }, + { + "type": "boolean" + } + ], + "provenance": { + "source": 0, + "pointer": "/components/schemas/NullableWithCombinators/anyOf" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/components/schemas/NullableWithCombinators" + }, + "variants": [ + { + "name": { + "hint": "variant_0" + }, + "type": { + "target": "t/prim/string", + "nullable": false + }, + "docs": {} + } + ], + "exclusive": true, + "wireTagged": false + }, + "t/prim/integer": { + "kind": "primitive", + "id": "t/prim/integer", + "name": {}, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0 + }, + "prim": "integer" + }, + "t/prim/string": { + "kind": "primitive", + "id": "t/prim/string", + "name": {}, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0 + }, + "prim": "string" + } + }, + "servers": [ + { + "name": { + "hint": "server" + }, + "urlTemplate": "/", + "description": {}, + "auth": null + } + ], + "diagnostics": [ + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "this position declares allOf beside its enum, and JSON Schema conjoins them where only one can be the value; it lowered as the enum, with allOf kept verbatim under Unmodeled", + "provenance": { + "source": 0, + "pointer": "/components/schemas/NarrowedEnum" + } + }, + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "this position declares allOf beside its const, and JSON Schema conjoins them where only one can be the value; it lowered as the const, with allOf kept verbatim under Unmodeled", + "provenance": { + "source": 0, + "pointer": "/components/schemas/NarrowedConst" + } + }, + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "this position declares enum beside its const, and JSON Schema conjoins them where only one can be the value; it lowered as the const, with enum kept verbatim under Unmodeled", + "provenance": { + "source": 0, + "pointer": "/components/schemas/ConstWithinEnum" + } + }, + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "oneOf and anyOf are both declared here and conjoin, and one Union carries one branch set; this position lowered as its oneOf, with anyOf kept verbatim under Unmodeled", + "provenance": { + "source": 0, + "pointer": "/components/schemas/BothCombinators" + } + }, + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "oneOf and anyOf are both declared here and conjoin, and one Union carries one branch set; this position lowered as its oneOf, with anyOf kept verbatim under Unmodeled", + "provenance": { + "source": 0, + "pointer": "/components/schemas/NullableWithCombinators" + } + } + ], + "sources": [ + { + "format": "openapi@3.1", + "path": "codeclared-keywords.yaml", + "hash": "699e4df4c601cd1039456e8a0ed12de42986dc49393b208ed0eb79d20e6e3355" + } + ] +} diff --git a/testdata/conformance/openapi/codeclared-keywords.yaml b/testdata/conformance/openapi/codeclared-keywords.yaml new file mode 100644 index 0000000..81bc0f4 --- /dev/null +++ b/testdata/conformance/openapi/codeclared-keywords.yaml @@ -0,0 +1,54 @@ +openapi: 3.1.0 +info: {title: CoDeclaredKeywords, version: "1.0.0"} +paths: {} +components: + schemas: + Base: + type: object + properties: + id: {type: string} + # JSON Schema conjoins keywords, so a schema may write several that the IR + # can only lower one of. Every case here lowers the one the dispatch elects + # and keeps the rest verbatim beside it (ir-design 4.8); none of them may + # lower in silence. + # + # allOf beside enum is the narrowing idiom: a Base whose value set this + # declaration restricts. The enum is the value and the composition is kept, + # so the relationship to Base stays recoverable. + NarrowedEnum: + allOf: + - {$ref: '#/components/schemas/Base'} + enum: [a, b] + # The same shape with a single value rather than a set. const outranks both + # of the others, so this keeps the composition against the Literal. + NarrowedConst: + allOf: + - {$ref: '#/components/schemas/Base'} + const: a + # const and enum co-declared: a legal, redundant restatement where the const + # must be one of the enum members. The Literal is the narrower of the two, + # and the enum it came from is kept. + ConstWithinEnum: + const: a + enum: [a, b] + # Two union combinators at one level conjoin: the value matches exactly one + # of the oneOf branches AND at least one of the anyOf branches. One ir.Union + # carries one branch set, so oneOf becomes the union and anyOf is kept. + BothCombinators: + oneOf: + - {type: string} + - {type: integer} + anyOf: + - {type: number} + - {type: boolean} + # A {X, null} oneOf normally collapses to nullable X, which would resolve + # this position straight to the shared string primitive and leave the + # co-declared anyOf nowhere of its own to sit. With both combinators written + # it stays a Union — of the one non-null branch — and keeps the anyOf on it. + NullableWithCombinators: + oneOf: + - {type: string} + - {type: "null"} + anyOf: + - {type: number} + - {type: boolean} From 14ea1fa45e32769c36d2193cc4233dd3b3547650 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Thu, 6 Aug 2026 11:24:16 +0300 Subject: [PATCH 2/5] test(compilers/openapi): guard the branch pointer both lowerings reach MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The order-independence test asserted that a node exists at the union branch's pointer, with the message that it is one "both lowerings reach". Those are not the same claim, and only the weaker one was checked: the outside $ref hoists that node on its own, so the assertion holds even for a bare `{type: string}` branch — the exact fixture the test was written away from, because such a branch resolves through the union to the shared primitive and never competes for the pointer. Verified by planting a branchHint that disagrees with subSchemaHint and reducing the fixture to a bare branch: the test passed, guard included. It now pins the union's own variant to the branch's node, in both documents. That is the state where two lowerings reach one pointer and only the first to arrive interns it, so a hint disagreement changes the IR — and reducing the branch reddens the guard instead of slipping past it. --- .../openapi/internal/schema/compose_test.go | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/compilers/openapi/internal/schema/compose_test.go b/compilers/openapi/internal/schema/compose_test.go index 0c55d28..928178f 100644 --- a/compilers/openapi/internal/schema/compose_test.go +++ b/compilers/openapi/internal/schema/compose_test.go @@ -1852,12 +1852,14 @@ func TestUnionCombinators_UnpreservableIsNotAnnounced(t *testing.T) { // and turns its branches into ordinary union branches, so an outside $ref naming // one of them must reach the same IR whichever of the two is declared first. // -// The branch writes a description on purpose. A bare `{type: string}` branch -// interns nothing through the union — it resolves to the shared primitive — so -// only the outside $ref would ever hoist a node there and the two could not -// disagree about one. Declaring something position-scoped makes both lowerings -// hoist the branch's home, which is the state where only the first to arrive -// interns it and the hints have to agree (branchHint, subSchemaHint, #181). +// The branch writes a description on purpose, and the guard below is what keeps +// that load-bearing. A bare `{type: string}` branch resolves through the union to +// the shared primitive, so the outside $ref is the only lowering that ever hoists +// a node at the branch pointer: the node is still *there*, which is why asserting +// its presence proves nothing — one lowering put it there and no hint ever had to +// agree with another. Pinning the union's own variant to that node is what puts +// both lowerings on the pointer, which is the state where only the first to +// arrive interns it (branchHint, subSchemaHint, #181). func TestUnionCombinators_KeepingIsOrderIndependent(t *testing.T) { t.Parallel() host := ` S: @@ -1872,8 +1874,13 @@ func TestUnionCombinators_KeepingIsOrderIndependent(t *testing.T) { last, diags := parseFull(t, componentSpec(host+outsider)) requireNoErrorDiags(t, diags) - require.Contains(t, first.Types, ir.TypeID("t/anon/components/schemas/S/oneOf/0"), - "the branch owns a node both lowerings reach, or there is nothing to race for") + for _, doc := range []*ir.Document{first, last} { + u, ok := typeByName(doc, "S").(*ir.Union) + require.True(t, ok, "the position is the Union that keeps the passed-over set") + require.Len(t, u.Variants, 1, "the null branch still lifts off the variant list") + require.Equal(t, ir.TypeID("t/anon/components/schemas/S/oneOf/0"), u.Variants[0].Type.Target, + "the union reaches the branch's own node, or the $ref is the only lowering that does") + } assert.Empty(t, cmp.Diff(first, last, orderInvariantIR()...), "declaring the reference before or after the union must not change the IR") assert.Empty(t, cmp.Diff(first.Types, last.Types), "nor any name hint in the registry") From 8a96be4a673232d422be8efe1bd5aa858b6a618b Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Thu, 6 Aug 2026 11:24:25 +0300 Subject: [PATCH 3/5] docs: record the co-declared keyword degradation the compiler now makes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Electing one of several conjoined keywords and keeping the rest verbatim is a new source-construct degradation, and the code that performs it cites section 4.8 as its authority — but 4.8 enumerated only the structural-sibling case, and the OpenAPI row of the spec matrix listed neither. Both now describe what the compiler does: which families compete, which wins, where the losers land, and why the elected combinator still becomes a Union rather than degrading to the top type. The keyword an elected lowering never reads is named there as unsettled, so the boundary is stated where the rule is, not only in a tracker entry. unionBranches carried the premise the whole bug grew from — that only the verbatim lowering ever sees a schema writing both combinators. That was already false when it was written, and it is now the sentence a reader would have to disbelieve to understand the callers. It names what each caller owes the set it passed over instead. --- compilers/openapi/internal/schema/compose.go | 8 +++++-- docs/ir-design.md | 23 +++++++++++++++++++- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/compilers/openapi/internal/schema/compose.go b/compilers/openapi/internal/schema/compose.go index 909a558..8129dab 100644 --- a/compilers/openapi/internal/schema/compose.go +++ b/compilers/openapi/internal/schema/compose.go @@ -617,8 +617,12 @@ func composesAsModel(s *oas3.Schema) bool { // unionBranches returns the branches of whichever combinator the schema // declares, the keyword's name (for pointers), and whether it is exclusive. -// oneOf wins when both are present; only the verbatim lowering ever sees that -// shape, and it keeps both keywords. +// +// oneOf wins when both are written, so every caller owns the set it passed over: +// buildUnion keeps it on the Union (preserveUnusedCombinator), nullUnionCollapse +// declines to collapse past it, and the verbatim lowering keeps both keywords +// (preserveUnionSiblings). Reading the preference as one only that last lowering +// could reach is what dropped the anyOf in silence (GitHub #35). func unionBranches(s *oas3.Schema) ([]*oas3.JSONSchema[oas3.Referenceable], string, bool) { if branches := s.GetOneOf(); len(branches) > 0 { return branches, "oneOf", true diff --git a/docs/ir-design.md b/docs/ir-design.md index 3a5e035..150e8b5 100644 --- a/docs/ir-design.md +++ b/docs/ir-design.md @@ -655,6 +655,27 @@ from it is exact and nothing is reported. A union whose branches declare no shape at all (`oneOf: [{required: [a]}, {required: [b]}]`) is not a degradation and is not listed here: it narrows the body without reshaping it, which is validation logic, so it is preserved under `ReasonValidationOnly` with §4.7's keyword family. +- **Competing keywords at one position, where only one can be the value** — keywords conjoin, so + `{allOf: [{$ref: Base}], enum: [a, b]}` is a narrowing of `Base` rather than a malformed + document and `{oneOf: […], anyOf: […]}` demands both, yet a position lowers to one node. Two + families compete: the value keywords `const`, `enum` and `allOf`, elected in that order, and the + two union combinators, where `oneOf` wins. **The IR** has no intersection combinator (§15, as + above), so the position lowers as the elected keyword — the most it can express there — and each + keyword the election passed over is kept verbatim in `Unmodeled["openapi:"]` under + `ReasonDegradedLowering` at that keyword's own pointer, plus one `openapi/degraded-construct` + `info` naming what was elected and what was kept. A position reports only what it actually + stored, so a payload that fails to convert is reported unpreservable rather than announced as + kept. Two consequences are deliberate: the elected combinator still becomes a `Union` rather + than degrading to the top type, because that `Union` is a shape the IR *can* express and + discarding it as well would model nothing at all; and a `{X, null}` `oneOf` written beside an + `anyOf` does **not** collapse to nullable `X` (§3.3), because the collapse asserts the position + *is* nullable `X` while the co-declared `anyOf` conjoins with it, and collapsing would resolve + the position to a shared primitive — which must never carry one declaration's keywords. Where a + *structural* sibling is written as well, the bullet above applies instead and both branch sets + are kept. A keyword the *elected* lowering never reads — `type: string` beside an `allOf`, + `format` beside a `const` — is a separate question this document has not settled: `allOf` beside + `type: object` is the common case and loses nothing, so it wants a per-winner rule rather than a + keyword list. - **An inline `allOf` branch declaring more than the merge consumes** — a branch written inline rather than as a `$ref` owns no node, so §4.3's composition has nowhere to point and the branch is merged into the composing model in place. That merge consumes the branch's `properties`, its @@ -1677,7 +1698,7 @@ How each format's distinctive concepts land in the IR (full details live with ea | Format | Lowering highlights | |---|---| -| **OpenAPI 3.x** | components/schemas → registry (IDs from pointers); inline schemas hoisted with hints; `allOf` → Base/Mixins per §4.3; `oneOf`/`anyOf` → Union (Exclusive bit), null-variant → Nullable ref, co-declared with structural keywords → the composition distributed across the variants per §4.3, or — for the five shapes that cannot be distributed — structural body + verbatim union per §4.8 (branches that declare no shape at all are `validation_only` per §4.7); `discriminator` → Discriminator (3.2 `defaultMapping` → Discriminator.Default); `nullable`/type-arrays → Nullable; readOnly/writeOnly → Visibility and schema-level `default` → the referencing Property/Parameter's Default: both bind a *use* of the type rather than the type, so a declaration-site one is pushed down to referencing properties with use-site precedence and the declaration keeps its own copy verbatim (`no_ir_home` Unmodeled + diagnostic) — which is what a component nothing references would otherwise lose silently; `additionalProperties: false` → Additional=closed, `unevaluatedProperties: false` → closed_after_composition, `minProperties`/`maxProperties` → Model.Constraints (the property set's cardinality, as against Additional's openness); parameters → Params + HTTPBinding locations w/ style/explode, 3.2 `in: querystring` → querystring location; requestBody/responses all content types → Payload.Contents, a non-required requestBody → Payload.Unmodeled (`no_ir_home`, presence is the IR's only body optionality); 3.2 `itemSchema` → Content.Item and `itemEncoding` → Content.ItemEncoding — except beside a positional `prefixEncoding`, where both go to Content.Unmodeled (`no_ir_home`) because a single every-item encoding cannot state ordinals; per-status responses/default → Conditions + ranges, error-response `headers` and a multi-media error `content` map → ErrorCase.Unmodeled (`no_ir_home`, ErrorCase has neither field); webhooks → HTTPBinding.IsWebhook; callbacks → Callbacks; links → Response.Unmodeled (`no_ir_home`, promotable later); path-item `servers` → Operation.Unmodeled (`no_ir_home`: §10 scopes servers by index list at service and channel, and an operation has no such list yet); securitySchemes/security → Auth OR-of-ANDs, 3.2 device flow + `oauth2MetadataUrl` → Flows/OAuth2MetadataURL; servers+variables (3.2 named) → Servers; tags (3.2 parent/kind) → groups + TagDefs; info contact/license → Document; schema `example(s)` → Examples; `xml` object (incl. 3.2 nodeType) → XMLHints at type and property level; `not`/`if-then-else`/`dependentSchemas`/`dependentRequired`/`contains`/`propertyNames`/`unevaluated*` → verbatim Unmodeled per §4.7; `contentEncoding`/`contentMediaType` → `Encoding` on the scalar the position lowers to and `contentSchema` → Unmodeled (`no_ir_home`) per §4.7; `$id`/`$schema`/`$vocabulary` → Unmodeled (`out_of_scope`, `$id` not honoured for resolution); `$dynamicRef` → the anchored type by compiler expansion, else verbatim Unmodeled with the reason it was irreducible; an inline `allOf` branch declaring more than the merge consumes → verbatim Unmodeled (`degraded_lowering`) per §4.8; a boolean `false` `allOf` branch → the composed Model closed, branch verbatim Unmodeled (`degraded_lowering`) per §4.8, a `true` branch a silent no-op; a shape applicator (`properties`/`patternProperties`/`additionalProperties`/`required`/`items`/`prefixItems`, and `format` where no type is declared) the lowered node has no field for → verbatim Unmodeled (`degraded_lowering`) per §4.8; a parameter schema's `xml` and its `readOnly`/`writeOnly` → Parameter.Unmodeled (`no_ir_home`: Parameter has no field for either); `patternProperties` → AdditionalProps.Patterns; `prefixItems` → Tuple, with any trailing `items` → Tuple.Unmodeled (`degraded_lowering` per §4.8: an open tuple has no IR combinator, so the fixed head is lowered and the tail kept beside it); `x-*` → namespaced Unmodeled (legal on every object — hence Unmodeled on every node); `$ref`-adjacent sibling keywords (3.1) and ref-target annotations merge onto the referencing Property/Parameter with **use-site precedence**, applied uniformly (oagen's ad-hoc per-site patching is the counterexample), and at a position carrying no Property/Parameter — an `allOf`/`oneOf`/`anyOf` branch, `items`, a component — bind an alias hoisted at that position instead, per §4.3; a oneOf/anyOf whose variants are all string consts normalizes to a closed `Enum` in a `pass/` normalization — not in the compiler — so per-variant `Docs` survive until the collapse is chosen; mutually-exclusive parameter groups (`x-mutually-exclusive-parameter-groups`) stay as namespaced Unmodeled entries, and their documented *promotion* (no dedicated node needed) is a pass that synthesizes one logical `Parameter` typed by a `Union` of variant models, bound via `HTTPParamBinding.ParamPath` per field; pagination only via injectable policy, marked Inferred | +| **OpenAPI 3.x** | components/schemas → registry (IDs from pointers); inline schemas hoisted with hints; `allOf` → Base/Mixins per §4.3; `oneOf`/`anyOf` → Union (Exclusive bit), null-variant → Nullable ref, co-declared with structural keywords → the composition distributed across the variants per §4.3, or — for the five shapes that cannot be distributed — structural body + verbatim union per §4.8 (branches that declare no shape at all are `validation_only` per §4.7); competing keywords at one position — `const`/`enum`/`allOf`, elected in that order, and `oneOf` beside `anyOf`, where oneOf wins — lower as the elected keyword with every passed-over one verbatim Unmodeled (`degraded_lowering`) at its own pointer per §4.8, and a `{X, null}` oneOf beside an anyOf stays a Union rather than collapsing to a nullable ref; `discriminator` → Discriminator (3.2 `defaultMapping` → Discriminator.Default); `nullable`/type-arrays → Nullable; readOnly/writeOnly → Visibility and schema-level `default` → the referencing Property/Parameter's Default: both bind a *use* of the type rather than the type, so a declaration-site one is pushed down to referencing properties with use-site precedence and the declaration keeps its own copy verbatim (`no_ir_home` Unmodeled + diagnostic) — which is what a component nothing references would otherwise lose silently; `additionalProperties: false` → Additional=closed, `unevaluatedProperties: false` → closed_after_composition, `minProperties`/`maxProperties` → Model.Constraints (the property set's cardinality, as against Additional's openness); parameters → Params + HTTPBinding locations w/ style/explode, 3.2 `in: querystring` → querystring location; requestBody/responses all content types → Payload.Contents, a non-required requestBody → Payload.Unmodeled (`no_ir_home`, presence is the IR's only body optionality); 3.2 `itemSchema` → Content.Item and `itemEncoding` → Content.ItemEncoding — except beside a positional `prefixEncoding`, where both go to Content.Unmodeled (`no_ir_home`) because a single every-item encoding cannot state ordinals; per-status responses/default → Conditions + ranges, error-response `headers` and a multi-media error `content` map → ErrorCase.Unmodeled (`no_ir_home`, ErrorCase has neither field); webhooks → HTTPBinding.IsWebhook; callbacks → Callbacks; links → Response.Unmodeled (`no_ir_home`, promotable later); path-item `servers` → Operation.Unmodeled (`no_ir_home`: §10 scopes servers by index list at service and channel, and an operation has no such list yet); securitySchemes/security → Auth OR-of-ANDs, 3.2 device flow + `oauth2MetadataUrl` → Flows/OAuth2MetadataURL; servers+variables (3.2 named) → Servers; tags (3.2 parent/kind) → groups + TagDefs; info contact/license → Document; schema `example(s)` → Examples; `xml` object (incl. 3.2 nodeType) → XMLHints at type and property level; `not`/`if-then-else`/`dependentSchemas`/`dependentRequired`/`contains`/`propertyNames`/`unevaluated*` → verbatim Unmodeled per §4.7; `contentEncoding`/`contentMediaType` → `Encoding` on the scalar the position lowers to and `contentSchema` → Unmodeled (`no_ir_home`) per §4.7; `$id`/`$schema`/`$vocabulary` → Unmodeled (`out_of_scope`, `$id` not honoured for resolution); `$dynamicRef` → the anchored type by compiler expansion, else verbatim Unmodeled with the reason it was irreducible; an inline `allOf` branch declaring more than the merge consumes → verbatim Unmodeled (`degraded_lowering`) per §4.8; a boolean `false` `allOf` branch → the composed Model closed, branch verbatim Unmodeled (`degraded_lowering`) per §4.8, a `true` branch a silent no-op; a shape applicator (`properties`/`patternProperties`/`additionalProperties`/`required`/`items`/`prefixItems`, and `format` where no type is declared) the lowered node has no field for → verbatim Unmodeled (`degraded_lowering`) per §4.8; a parameter schema's `xml` and its `readOnly`/`writeOnly` → Parameter.Unmodeled (`no_ir_home`: Parameter has no field for either); `patternProperties` → AdditionalProps.Patterns; `prefixItems` → Tuple, with any trailing `items` → Tuple.Unmodeled (`degraded_lowering` per §4.8: an open tuple has no IR combinator, so the fixed head is lowered and the tail kept beside it); `x-*` → namespaced Unmodeled (legal on every object — hence Unmodeled on every node); `$ref`-adjacent sibling keywords (3.1) and ref-target annotations merge onto the referencing Property/Parameter with **use-site precedence**, applied uniformly (oagen's ad-hoc per-site patching is the counterexample), and at a position carrying no Property/Parameter — an `allOf`/`oneOf`/`anyOf` branch, `items`, a component — bind an alias hoisted at that position instead, per §4.3; a oneOf/anyOf whose variants are all string consts normalizes to a closed `Enum` in a `pass/` normalization — not in the compiler — so per-variant `Docs` survive until the collapse is chosen; mutually-exclusive parameter groups (`x-mutually-exclusive-parameter-groups`) stay as namespaced Unmodeled entries, and their documented *promotion* (no dedicated node needed) is a pass that synthesizes one logical `Parameter` typed by a `Union` of variant models, bound via `HTTPParamBinding.ParamPath` per field; pagination only via injectable policy, marked Inferred | | **Swagger 2.0** | lifted to OpenAPI 3.x shape first (body/formData → Payload; host/basePath/schemes → Servers; consumes/produces → content types), then the OpenAPI lowering runs | | **TypeSpec** | consumed post-check (monomorphized, `isFinished`); template instances → TypeCommon.Instantiation incl. value args → TemplateArg; models → Model w/ Base + spread provenance → Mixins; scalars → Scalar chains, constructors in values → Value.Ctor; `@encode`/`@format` → Encoding triple; `@encodedName` → WireNameByFormat at property AND type level; unions w/ named variants → Union, `@discriminated` → Discriminator.PropertyName/Envelope/EnvelopeValueName; `| null` → Nullable; visibility classes (incl. custom, `@invisible` → Visibility.None) → Visibility, op overrides → ParameterVisibility/ReturnTypeVisibility; `@patch` implicitOptionality → HTTPBinding.PatchImplicitOptionality; interfaces → OperationGroups (versionable); `@overload` → OverloadOf; `@sharedRoute` → SharedRoute; `@service` → Service; versioning decorators incl. `@typeChangedFrom`/`@madeOptional`/`@madeRequired` and add/remove cycles → Availability timeline (on members/variants/params too); pagination decorators incl. prev/first/last links and header continuation tokens → Pagination PropPaths (In:"header"); Azure.Core `@pollingOperation`/`@finalOperation` → LongRunning; multipart w/ parts → Content.Encoding/PartEncoding, `Http.File` → FileInfo (content-type set, contents chain, filename location); streams/SSE → StreamDetail + Variant.Event (contentType, terminal); `@error` → UsageFlags.Error; `@example`/`@opExample` → Examples (Input/Output pairs); `@pattern` message → Constraints.PatternMessage; `@mediaTypeHint` → TypeCommon.MediaTypeHint; `never` members deleted + diagnostic per §4.8; TCGC client-shaping decorators (`@clientName`, `@access`, `@usage`, `@scope`, `@override`, …) → namespaced Unmodeled (`out_of_scope`) consumed by emitter policy, never IR semantics; values/consts incl. enum-member refs → Values channel | | **Smithy 2.0** | structures → Model, mixins → Mixins (non-structure mixins flattened — spec-sanctioned); `document` → Any; unions → WireTagged Union, member `@jsonName` → Variant.WireName; enum/intEnum → Enum (open by default); `@sparse` → element Nullable; traits: constraints → Constraints, `@paginated` → Pagination (declared), `@retryable` → ErrorCase.Retryable + Throttling, `@error` fault → ErrorCase.Fault, `@readonly` → Idempotency safe, `@idempotent`/`@idempotencyToken` → Idempotency, `@sensitive` → Sensitive/Secret, `@tags` → Tags, `@clientOptional`/`@input` → Property.ClientOptional (+InputOnly), `@addedDefault` → DefaultAdded, root-shape `@default` pushed down to properties w/ provenance; `@streaming` blob → StreamDetail (+`@requiresLength` → RequiresLength); event streams → StreamDetail.Events union + Property.EventHeader/EventPayload + Initial messages; service-level errors → Service.CommonErrors; protocol traits → Service.Protocols; service `rename`/`version` → Service.Renames/Version; resources → OperationGroup + ResourceInfo (identifiers, properties, lifecycle incl. put/@noReplace, instance vs collection ops); http traits → HTTPBinding incl. `@endpoint`/`@hostLabel` → HostPrefix/host location (additive binding), `@httpPrefixHeaders`/`@httpQueryParams` → Prefix bindings, `@httpResponseCode` → Response.StatusCodeProp, `@requestCompression` → Compression, `@httpChecksumRequired` → ChecksumRequired; `@auth` order → priority-ordered Auth, `@optionalAuth` → empty option; `@jsonName` → WireName; `@mediaType` → Encoding.MediaType; xml traits → XMLHints at type and property level; `@examples` → Examples (Input/Output/Error); waiters + rules-engine traits → verbatim Unmodeled (`out_of_scope`, §15); `smithy.api#Unit` → nil payload / shared empty Model for tag-only variants; other traits → namespaced Unmodeled | From 70dcd0ddcec0c1b688570b001b4a20b0466b5e76 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Thu, 6 Aug 2026 13:02:16 +0300 Subject: [PATCH 4/5] fix(compilers/openapi): stop an unknown family inheriting allOf's guard declaresFamily answered allOf's guard from its default arm, so a name added to familyOrder without a case here would report "declared" on every schema that wrote an allOf, and be elected on schemas that never wrote it at all. allOf is now its own case and an unrecognised name declares nothing, which keeps it out of the election entirely. That direction matters because of what the losing direction costs. dispatchOf's comment claimed a family listed in familyOrder but absent from lower() "would be kept verbatim rather than dropped". Only when it loses. Dropping lower()'s allOf arm while leaving allOf in familyOrder and compiling a schema that declares only an allOf yields a bare scalar, an empty Unmodeled and zero diagnostics -- a keyword dropped in silence, which is the failure GitHub #35 exists to fix. A winner lower() cannot lower falls through the switch to the type-set arms and is neither lowered nor skipped. The comment now says which of the three pairings fail safely and which does not. TestDeclaresFamily_AnUnknownNameDeclaresNothing reaches the new arm directly, as TestRecordSkippedFamilies_MissingOwner already does for its own unreachable guard, and reddens if the default goes back to answering for allOf. --- compilers/openapi/internal/schema/schema.go | 21 +++++++++++++++---- .../internal/schema/schema_internal_test.go | 21 +++++++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/compilers/openapi/internal/schema/schema.go b/compilers/openapi/internal/schema/schema.go index d83ef6e..b3efc62 100644 --- a/compilers/openapi/internal/schema/schema.go +++ b/compilers/openapi/internal/schema/schema.go @@ -445,14 +445,20 @@ var familyOrder = []string{"const", "enum", "allOf"} // definition of each family's guard: lower() lowers what dispatchOf elects and // recordSkippedFamilies keeps what the same walk passed over, so the winner and // the losers can never be read off two tests that disagree. +// A name it does not know declares nothing, rather than falling through to +// another family's guard: a familyOrder entry added without a case here would +// otherwise report whichever guard the default happened to hold, electing that +// name on schemas that never wrote it. func declaresFamily(s *oas3.Schema, family string) bool { switch family { case "const": return s.GetConst() != nil case "enum": return len(s.GetEnum()) > 0 - default: // allOf + case "allOf": return len(s.GetAllOf()) > 0 + default: + return false } } @@ -466,9 +472,16 @@ type dispatch struct { // dispatchOf elects the family lower() lowers and collects the rest. A schema // declaring none leaves won empty and skipped nil, so the type-set arms preserve -// nothing — and a family added to familyOrder but to no arm of lower() would be -// kept verbatim rather than dropped, which is the safe way for the two to -// disagree. +// nothing. +// +// familyOrder, declaresFamily and lower()'s switch have to name the same three, +// and only two of the three pairings fail safely. A name familyOrder lists that +// declaresFamily does not know is never declared, so it is never elected; a name +// that loses the election reaches recordSkippedFamilies whether or not lower() +// can lower it. But a name that *wins* an election lower() has no arm for is +// neither lowered nor skipped: the switch falls through to the type-set arms and +// the keyword is dropped in silence — the very failure GitHub #35 is about. +// Adding a family means adding all three. func dispatchOf(s *oas3.Schema) dispatch { var d dispatch for _, family := range familyOrder { diff --git a/compilers/openapi/internal/schema/schema_internal_test.go b/compilers/openapi/internal/schema/schema_internal_test.go index 82afe19..e1b560a 100644 --- a/compilers/openapi/internal/schema/schema_internal_test.go +++ b/compilers/openapi/internal/schema/schema_internal_test.go @@ -409,6 +409,27 @@ func TestRecordSkippedFamilies_MissingOwner(t *testing.T) { assertInternalInvariant(t, diags) } +// TestDeclaresFamily_AnUnknownNameDeclaresNothing reaches the arm familyOrder +// cannot reach today, the way TestRecordSkippedFamilies_MissingOwner reaches its +// own: every name dispatchOf passes in is one of the three with a case here. +// +// It guards the direction the default falls. Answering an unknown name from +// another family's guard would let a familyOrder entry added without a case be +// elected on schemas that never wrote it — and a winner lower() has no arm for +// is dropped in silence, which is what this file exists to prevent. Declaring +// nothing keeps such a name out of the election entirely. +func TestDeclaresFamily_AnUnknownNameDeclaresNothing(t *testing.T) { + t.Parallel() + withAllOf := &oas3.Schema{AllOf: []*oas3.JSONSchema[oas3.Referenceable]{ + oas3.NewJSONSchemaFromSchema[oas3.Referenceable](&oas3.Schema{}), + }} + + require.True(t, declaresFamily(withAllOf, "allOf"), "allOf is named and declared") + assert.False(t, declaresFamily(withAllOf, "oneOf"), + "an unknown name must not inherit allOf's guard") + assert.False(t, declaresFamily(&oas3.Schema{}, "nosuchfamily")) +} + // TestRefNullable_AnUnresolvedRefIsNotNullable pins the guard on the second half // of the question. A $ref site admits null when its own spelling says so or when // its target does; a reference the loader never resolved has no target to ask, From da48be4ce0e12ddc1120349b6d737209a3241a86 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Thu, 6 Aug 2026 13:10:31 +0300 Subject: [PATCH 5/5] docs(compilers/openapi): record the collapse hint gap at its own site The {X, null} collapse hands the surviving branch the enclosing schema's name hint, while an outside $ref to that same branch pointer derives variant_, so the two declaration orders produce different documents. That is deliberately left alone here and tracked in #281, but it was recorded only in the pull request body -- a reader arriving at nullUnionCollapse had nothing telling them the hint it returns is a known open question, and this change edits that very function. Stated on nullUnionCollapse itself now, including what this change does and does not do to it: declining the collapse when both combinators are declared removes one order in which the discrepancy can be reached, and settles nothing else. --- compilers/openapi/internal/schema/schema.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/compilers/openapi/internal/schema/schema.go b/compilers/openapi/internal/schema/schema.go index b3efc62..d06d1eb 100644 --- a/compilers/openapi/internal/schema/schema.go +++ b/compilers/openapi/internal/schema/schema.go @@ -1708,6 +1708,15 @@ 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. +// // 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 // not; the position falls through to the Union instead, which is the one node