From 511e023c43b8586a60e805c1384a8d449a4e952c Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Sun, 9 Aug 2026 06:26:34 +0300 Subject: [PATCH] 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 +}