feat(irverify): reject a union that declares no variants

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

ir/irverify/irverify.go

@@ -33,6 +33,7 @@ func Verify(doc *ir.Document) []Violation {
 	vs = append(vs, checkPrimIDs(doc)...)
 	vs = append(vs, checkPrimKinds(doc)...)
 	vs = append(vs, checkAuthKinds(doc)...)
+	vs = append(vs, checkUnions(doc)...)
 	vs = append(vs, checkDiagnostics(doc)...)
 	vs = append(vs, checkVersion(doc)...)
 	vs = append(vs, runWalkChecks(doc)...)

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

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
+}

ir/irverify/unions_test.go

@@ -0,0 +1,114 @@
+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.
+//
+// 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()
+	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")
+		})
+	}
+}
+
+// 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
+}