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

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)...)
 

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