From 511e023c43b8586a60e805c1384a8d449a4e952c Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Sun, 9 Aug 2026 06:26:34 +0300 Subject: [PATCH 1/4] feat(irverify): reject a union that declares no variants An ir.Union carrying an empty Variants slice was reported by nothing. irverify had no rule reading Variants at all, and the one place pass reads them (checkUnionDiscriminator) folds them into a membership set and returns immediately when the union declares no discriminator. A union is the choice between its variants, so a union of none is a type no value inhabits, and no source format expresses one. A union that reaches the IR with none was built by a lowering that dropped every variant it meant to add -- our bug, which is what makes it a Violation rather than an ir.Diagnostic. Downstream it is worse than the missing variants: an emitter switching over the variants renders a type with no arms and no error, so the loss surfaces as generated code that compiles and can never be constructed. The two neighbouring shapes the issue raised are settled by what the compiler actually produces rather than by inspection. oneOf with one $ref lowers to a union of exactly one variant, and oneOf naming one $ref twice lowers to two variants sharing a target; both come from documents the specification allows, so neither is evidence of a compiler defect and neither is reported here. Both are pinned as clean so a later tightening has to argue with a test. --- ir/irverify/irverify.go | 1 + ir/irverify/unions.go | 51 ++++++++++++++++++++ ir/irverify/unions_test.go | 97 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 149 insertions(+) create mode 100644 ir/irverify/unions.go create mode 100644 ir/irverify/unions_test.go diff --git a/ir/irverify/irverify.go b/ir/irverify/irverify.go index ec779b6..9dc795b 100644 --- a/ir/irverify/irverify.go +++ b/ir/irverify/irverify.go @@ -31,6 +31,7 @@ func Verify(doc *ir.Document) []Violation { vs := checkRegistryKeys(doc) vs = append(vs, checkIDs(doc)...) vs = append(vs, checkPrimIDs(doc)...) + vs = append(vs, checkUnions(doc)...) vs = append(vs, checkDiagnostics(doc)...) vs = append(vs, runWalkChecks(doc)...) diff --git a/ir/irverify/unions.go b/ir/irverify/unions.go new file mode 100644 index 0000000..207b84f --- /dev/null +++ b/ir/irverify/unions.go @@ -0,0 +1,51 @@ +package irverify + +import ( + "github.com/dexpace/morphic/ir" +) + +// checkUnions asserts every union in the type registry declares at least one +// variant (ir-design §4.4). A union is the choice between its variants, so a +// union of none is a type no value inhabits, and it is not a shape any source +// format can express: `oneOf: []` is refused before it lowers, and every other +// format's sum requires at least one member. A union that reaches the IR with +// none was therefore built by a lowering that dropped every variant it meant to +// add — our bug, which is what makes it a Violation and not an ir.Diagnostic. +// +// Downstream it is worse than the missing variants are on their own: an emitter +// switching over a union's variants renders a type with no arms and no error, so +// the loss surfaces as generated code that compiles and can never be +// constructed. +// +// Two neighbouring shapes are deliberately not reported here, because the +// compiler produces both from documents the specification allows — a Violation +// claims a compiler defect, so neither belongs in this channel: +// +// - A union of exactly one variant. `oneOf: [{$ref: X}]` lowers to one, and +// invariant #2 forbids a compiler collapsing it. It is not a choice, but it +// is inhabited and it is a faithful lowering. +// - Two variants naming one target. `oneOf: [{$ref: X}, {$ref: X}]` lowers to +// exactly that. It is degenerate rather than impossible, so if it is worth +// reporting at all it is a spec-author problem for pass.Validate. +// +// Unions live only in the type registry — invariant #3 keeps every named entity +// there and lets no node embed another — so iterating it reaches every one and +// this check needs no walk of its own. +func checkUnions(doc *ir.Document) []Violation { + var vs []Violation + for id, td := range doc.Types { + if ir.IsNilTypeDef(td) { + continue // checkRegistryKeys reports the nil entry itself + } + u, isUnion := td.(*ir.Union) + if !isUnion || len(u.Variants) > 0 { + continue + } + vs = append(vs, Violation{ + Code: "ir/union-no-variants", + Message: "union declares no variants, so no value inhabits it", + Path: "types[" + string(id) + "]", + }) + } + return vs +} diff --git a/ir/irverify/unions_test.go b/ir/irverify/unions_test.go new file mode 100644 index 0000000..4b1bce8 --- /dev/null +++ b/ir/irverify/unions_test.go @@ -0,0 +1,97 @@ +package irverify_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/dexpace/morphic/ir" + "github.com/dexpace/morphic/ir/irverify" +) + +// unionOf returns a document holding one union over the given variant targets, +// plus the leaf each target names, so the document is referentially closed and +// the only thing a violation can be about is the union itself. +func unionOf(targets ...ir.TypeID) *ir.Document { + u := &ir.Union{TypeCommon: ir.TypeCommon{ + ID: "t/x/U", + Name: ir.Naming{Source: "U", Canonical: "u"}, + }} + types := ir.TypeRegistry{u.ID: u} + for _, target := range targets { + u.Variants = append(u.Variants, ir.Variant{Type: ir.TypeRef{Target: target}}) + types[target] = &ir.Scalar{TypeCommon: ir.TypeCommon{ + ID: target, + Name: ir.Naming{Source: "Leaf", Canonical: "leaf"}, + }} + } + return &ir.Document{Types: types} +} + +// unionViolations returns the ir/union-no-variants violations in doc, so a test +// asserting none is not satisfied by an unrelated violation being absent. +func unionViolations(t *testing.T, doc *ir.Document) []irverify.Violation { + t.Helper() + var out []irverify.Violation + for _, v := range irverify.Verify(doc) { + if v.Code == "ir/union-no-variants" { + out = append(out, v) + } + } + return out +} + +func TestVerify_UnionWithNoVariantsIsAViolation(t *testing.T) { + t.Parallel() + got := unionViolations(t, unionOf()) + require.Len(t, got, 1, "a union of nothing is reported exactly once") + assert.Equal(t, "ir/union-no-variants", got[0].Code) + assert.Equal(t, "types[t/x/U]", got[0].Path, "the violation locates the union") + assert.Contains(t, got[0].Message, "no variants") +} + +// TestVerify_SingleVariantUnionIsClean pins the first of the two shapes this +// check deliberately passes. `oneOf: [{$ref: X}]` is a legal schema and the +// OpenAPI compiler lowers it to a union of exactly one variant — verified by +// compiling that spec — so reporting it would fire on a faithful lowering. +// Invariant #2 is what forbids the compiler collapsing it in the first place. +func TestVerify_SingleVariantUnionIsClean(t *testing.T) { + t.Parallel() + assert.Empty(t, unionViolations(t, unionOf("t/x/Leaf"))) +} + +// TestVerify_RepeatedVariantTargetIsClean pins the second. `oneOf: [{$ref: X}, +// {$ref: X}]` also lowers to exactly what it says, so a repeated target is +// degenerate rather than impossible. A Violation claims a compiler defect, so +// this channel is the wrong one for it whatever is decided about reporting it +// elsewhere. +func TestVerify_RepeatedVariantTargetIsClean(t *testing.T) { + t.Parallel() + assert.Empty(t, unionViolations(t, unionOf("t/x/Leaf", "t/x/Leaf"))) +} + +// TestVerify_NilTypeBesideAUnionDoesNotPanic holds the report-only guarantee at +// this check: a nil registry entry is checkRegistryKeys' to report, and reaching +// past it here would crash Verify on the malformed document it exists to +// describe. +func TestVerify_NilTypeBesideAUnionDoesNotPanic(t *testing.T) { + t.Parallel() + doc := unionOf() + doc.Types["t/x/Nil"] = nil + + var got []irverify.Violation + require.NotPanics(t, func() { got = irverify.Verify(doc) }) + assert.Len(t, unionViolations(t, doc), 1, "the empty union is still reported") + assert.Contains(t, codes(got), "ir/nil-type", "the nil entry is reported by its own check") +} + +// codes returns the violation codes in vs, for assertions about which checks +// fired rather than about their order. +func codes(vs []irverify.Violation) []string { + out := make([]string, 0, len(vs)) + for _, v := range vs { + out = append(out, v.Code) + } + return out +} From 8f28f931d7762fea6bd8a77b83526ee35e1bfa43 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 11 Aug 2026 16:45:17 +0300 Subject: [PATCH 2/4] test(irverify): plant the nil shape that would actually panic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TestVerify_NilTypeBesideAUnionDoesNotPanic planted an untyped nil, which fails the *ir.Union assertion and is skipped by a check with no guard at all — so ir.IsNilTypeDef was removable from checkUnions with the whole suite green. A typed nil satisfies the assertion, and the variant count read behind it is a real dereference: with the guard gone, Verify panics on one. Both shapes are planted now, and removing the guard reddens the typed-nil case. --- ir/irverify/unions_test.go | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/ir/irverify/unions_test.go b/ir/irverify/unions_test.go index 4b1bce8..e0851ad 100644 --- a/ir/irverify/unions_test.go +++ b/ir/irverify/unions_test.go @@ -75,15 +75,32 @@ func TestVerify_RepeatedVariantTargetIsClean(t *testing.T) { // this check: a nil registry entry is checkRegistryKeys' to report, and reaching // past it here would crash Verify on the malformed document it exists to // describe. +// +// Both nil shapes, because only one of them is dangerous and it is not the +// obvious one. An untyped nil fails the *ir.Union assertion, so a check with no +// guard at all skips it and looks safe; a typed nil satisfies the assertion, and +// the variant count read behind it is a real dereference. Planting only the +// untyped one leaves ir.IsNilTypeDef removable with the whole suite green. func TestVerify_NilTypeBesideAUnionDoesNotPanic(t *testing.T) { t.Parallel() - doc := unionOf() - doc.Types["t/x/Nil"] = nil + for _, tc := range []struct { + name string + entry ir.TypeDef + }{ + {"an untyped nil entry", nil}, + {"a typed nil union", (*ir.Union)(nil)}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + doc := unionOf() + doc.Types["t/x/Nil"] = tc.entry - var got []irverify.Violation - require.NotPanics(t, func() { got = irverify.Verify(doc) }) - assert.Len(t, unionViolations(t, doc), 1, "the empty union is still reported") - assert.Contains(t, codes(got), "ir/nil-type", "the nil entry is reported by its own check") + var got []irverify.Violation + require.NotPanics(t, func() { got = irverify.Verify(doc) }) + assert.Len(t, unionViolations(t, doc), 1, "the empty union is still reported") + assert.Contains(t, codes(got), "ir/nil-type", "the nil entry is reported by its own check") + }) + } } // codes returns the violation codes in vs, for assertions about which checks From 5bbd4c481588ba1525b7ea61ce1bacb3c36adaa8 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 11 Aug 2026 16:51:53 +0300 Subject: [PATCH 3/4] test(irverify): pin the nil guard checkPrimKinds already carried MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every registry-iterating check in this package guards a nil entry with ir.IsNilTypeDef, and each guard was mutation-tested for whether a test holds it there. checkPrimKinds' did not: removing it left the suite green, while a typed-nil *ir.Primitive panics Verify without it, since the assertion succeeds and the kind read behind it is a real dereference. The gap was the same one #360 fixes for checkUnions and has the same cause — the existing cases plant an untyped nil, which fails the type assertion and is skipped whether the guard is there or not. All five guards are pinned now. --- ir/irverify/kinds_test.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/ir/irverify/kinds_test.go b/ir/irverify/kinds_test.go index 67d52eb..db21cb2 100644 --- a/ir/irverify/kinds_test.go +++ b/ir/irverify/kinds_test.go @@ -47,6 +47,36 @@ func TestVerify_UndeclaredPrimKindIsAViolation(t *testing.T) { } } +// TestVerify_NilTypeBesideAPrimitiveDoesNotPanic holds checkPrimKinds to the +// report-only guarantee, at the nil shape that can actually break it. +// +// An untyped nil fails the *ir.Primitive assertion, so a check with no guard at +// all skips it and looks safe; a typed nil satisfies the assertion, and the kind +// read behind it is a real dereference. Planting only the untyped one leaves +// ir.IsNilTypeDef removable here with the whole suite green — which is what it +// was, until this case was written. +func TestVerify_NilTypeBesideAPrimitiveDoesNotPanic(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + entry ir.TypeDef + }{ + {"an untyped nil entry", nil}, + {"a typed nil primitive", (*ir.Primitive)(nil)}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + doc := primDoc("flt") + doc.Types["t/x/Nil"] = tc.entry + + var got []irverify.Violation + require.NotPanics(t, func() { got = irverify.Verify(doc) }) + assert.Contains(t, codes(got), "ir/unknown-prim-kind", "the bad kind is still reported") + assert.Contains(t, codes(got), "ir/nil-type", "the nil entry is reported by its own check") + }) + } +} + // TestVerify_DeclaredPrimKindIsClean is the other half of the proof: a check // that cannot stay silent is no better than one that cannot fire. The documents // differ from the ones above only in the kind, so nothing but the kind can be From 981c438c43095b4dde180ff7bf89424c19ce8291 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 11 Aug 2026 17:28:29 +0300 Subject: [PATCH 4/4] docs(irverify): stop the package comment enumerating the checks The parenthetical listing the invariants read as the set and was not one: it was extended by #119, #271 and #329 as checks landed, then not by #319, which added three. This PR would have been the next to leave it behind. Verify's body and walkChecks are the enumeration and cannot fall out of step with themselves. The comment names a few invariants to orient a reader and points at them for the rest, which is what #359 did to the count beside a table for the same reason. --- ir/irverify/doc.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/ir/irverify/doc.go b/ir/irverify/doc.go index c2cb488..2418e50 100644 --- a/ir/irverify/doc.go +++ b/ir/irverify/doc.go @@ -1,8 +1,14 @@ // Package irverify checks a compiled ir.Document against the structural -// invariants every compiler must uphold (stable IDs, no two nodes claiming one +// invariants every compiler must uphold: stable IDs, no two nodes claiming one // identity, no dangling references, neutral naming, routable Unmodeled entries, -// in-range provenance, a readable schema stamp). Its findings are Violation -// values — our own compiler bugs, deliberately a separate channel from -// ir.Diagnostic, which reports problems in the source spec. Verify is pure and -// imports only ir. +// in-range provenance, a readable schema stamp, and more besides. +// +// Verify's body and walkChecks are the enumeration; the parenthetical that used +// to close that sentence was not, and had already fallen behind the checks +// beside it. A list here is one more thing that has to be kept true, and nothing +// fails when it stops being. +// +// Its findings are Violation values — our own compiler bugs, deliberately a +// separate channel from ir.Diagnostic, which reports problems in the source +// spec. Verify is pure and imports only ir. package irverify