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 diff --git a/ir/irverify/irverify.go b/ir/irverify/irverify.go index be62e35..3ab5913 100644 --- a/ir/irverify/irverify.go +++ b/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)...) 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 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..e0851ad --- /dev/null +++ b/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 +}