diff --git a/pass/validate.go b/pass/validate.go index c510b47..2050935 100644 --- a/pass/validate.go +++ b/pass/validate.go @@ -232,7 +232,11 @@ func checkPropIDRefs(doc *ir.Document) []ir.Diagnostic { // Only this pass reports it today. // // The fields that carry a Payload are named here — Operation.Request, -// Response.Payload and Message.Payload — so a new one has to be added by hand. +// Response.Payload and Message.Payload — because nothing in a Payload's Go type +// says who owns one, so a new one has to be added by hand. That coupling is +// guarded: TestEncodingCarriers_AreEveryPayloadFieldInTheIR +// (validate_carriers_test.go) walks the IR for Payload-bearing fields and fails +// the moment one of them is not walked here. func checkEncodingKeys(doc *ir.Document) []ir.Diagnostic { var diags []ir.Diagnostic forEachOperation(doc, func(op ir.Operation) { diff --git a/pass/validate_carriers_test.go b/pass/validate_carriers_test.go new file mode 100644 index 0000000..cc941ed --- /dev/null +++ b/pass/validate_carriers_test.go @@ -0,0 +1,228 @@ +package pass_test + +import ( + "go/ast" + "go/parser" + "go/token" + "path/filepath" + "reflect" + "runtime" + "slices" + "strconv" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/dexpace/morphic/ir" +) + +// payloadType is the node whose carriers this file enumerates. +var payloadType = reflect.TypeFor[ir.Payload]() + +const ( + // maxTypeGraphDepth bounds the walk over the IR's static type graph and + // maxFieldTypeDepth the walk through one field's type constructors (the + // bounded-recursion rule). Each distinct reflect.Type is visited once, so the + // seen set already terminates the first, and a field type is a finite tower of + // pointers, slices and maps — except that `type T *T` is legal Go, which is + // what the second counter is for. Reaching either cap means the walk stopped + // being a walk over the IR, so it fails rather than truncating. + maxTypeGraphDepth = 512 + maxFieldTypeDepth = 32 +) + +// TestEncodingCarriers_NameEveryPayloadFieldInTheIR fails when the IR declares a +// field carrying an ir.Payload that encodingCarriers does not name. +// +// checkEncodingKeys reaches every Content by naming the Payload-bearing fields by +// hand, because nothing in a Payload's Go type says who owns one. Naming them +// costs a coupling the compiler cannot check, and this is what checks it: a +// fourth carrier added to the IR would otherwise be walked by neither the check +// nor the cases below, its encoding keys resolved against nothing, with the whole +// suite green. +// +// The guard holds both lists at once, in two steps. Here it holds +// encodingCarriers against the IR; TestValidate_EncodingKeyAddressesNoProperty +// then holds checkEncodingKeys against encodingCarriers, by requiring a +// diagnostic from every entry. So a carrier added to the IR reddens this test, +// and adding it here reddens that one until checkEncodingKeys walks it too. +func TestEncodingCarriers_NameEveryPayloadFieldInTheIR(t *testing.T) { + t.Parallel() + carriers := encodingCarriers() + listed := make([]string, 0, len(carriers)) + for _, c := range carriers { + listed = append(listed, c.field) + } + slices.Sort(listed) + + found := payloadFields(t) + require.NotEmpty(t, found, "the walk found no ir.Payload field at all, so it reached "+ + "nothing and proves nothing about the ones encodingCarriers names") + assert.Empty(t, cmp.Diff(found, listed), + "encodingCarriers must name every ir field that carries an ir.Payload, once each "+ + "(-declared +listed); a new one also has to be walked by checkEncodingKeys") +} + +// payloadFields returns "Owner.Field", sorted, for every struct field the IR +// declares whose type is or contains an ir.Payload. +// +// The walk starts at ir.Document and visits each distinct reflect.Type once, so +// recursive shapes terminate. The sealed TypeDef sum is reached only through an +// interface, which a walk over the static type graph cannot descend into, so each +// concrete kind is walked from its own root as well — seeded from the kinds the +// ir sources declare, so a variant is covered the day it is added rather than the +// day someone remembers a list here (ir/nofloat_test.go walks the same two halves +// for the same reason). +func payloadFields(t *testing.T) []string { + t.Helper() + var found []string + seen := map[reflect.Type]bool{} + + var walk func(rt reflect.Type, depth int) + walk = func(rt reflect.Type, depth int) { + require.Less(t, depth, maxTypeGraphDepth, "the IR type graph nests past %d", maxTypeGraphDepth) + if seen[rt] { + return + } + seen[rt] = true + switch rt.Kind() { + case reflect.Pointer, reflect.Slice, reflect.Array: + walk(rt.Elem(), depth+1) + case reflect.Map: + walk(rt.Key(), depth+1) + walk(rt.Elem(), depth+1) + case reflect.Struct: + for f := range rt.Fields() { + if carriesPayload(t, f.Type, 0) { + found = append(found, rt.Name()+"."+f.Name) + } + walk(f.Type, depth+1) + } + default: + // A leaf: no other kind has a component type to descend into. + } + } + + walk(reflect.TypeFor[ir.Document](), 0) + for _, kind := range irTypeKinds(t) { + td, ok := ir.NewTypeDef(kind) + require.True(t, ok, "no concrete type is registered for kind %q", kind) + rt := reflect.TypeOf(td) + require.Equal(t, reflect.Pointer, rt.Kind(), "NewTypeDef must return a pointer for %q", kind) + walk(rt.Elem(), 0) + } + slices.Sort(found) + return found +} + +// carriesPayload reports whether a field's type is an ir.Payload, or a pointer, +// slice, array or map of one. +// +// It does not descend into structs. A field whose type merely reaches a Payload +// further down is the spine leading to a carrier — Document.Services reaches +// every one of today's — and naming the spine would name most of the IR. +func carriesPayload(t *testing.T, rt reflect.Type, depth int) bool { + t.Helper() + require.Less(t, depth, maxFieldTypeDepth, "resolving a field type exceeded %d steps", maxFieldTypeDepth) + if rt == payloadType { + return true + } + switch rt.Kind() { + case reflect.Pointer, reflect.Slice, reflect.Array: + return carriesPayload(t, rt.Elem(), depth+1) + case reflect.Map: + return carriesPayload(t, rt.Key(), depth+1) || carriesPayload(t, rt.Elem(), depth+1) + default: + return false // a struct is a carrier's owner, not a carrier + } +} + +// irTypeKinds returns every TypeKind constant the ir package's production sources +// declare. Derived from the source because a list of the kinds written here is +// one commit away from disagreeing with the sum it claims to enumerate, which is +// the failure this whole file exists to catch. +// +// It reads the same declarations ir's own declaredTypeKinds does, spelled out +// again because a helper in one package's test binary is not linked into +// another's. +func irTypeKinds(t *testing.T) []ir.TypeKind { + t.Helper() + var kinds []ir.TypeKind + for _, path := range irSourcePaths(t) { + f, err := parser.ParseFile(token.NewFileSet(), path, nil, parser.SkipObjectResolution) + require.NoError(t, err, "parsing %s", path) + for _, decl := range f.Decls { + gd, isGen := decl.(*ast.GenDecl) + if !isGen || gd.Tok != token.CONST { + continue + } + kinds = append(kinds, typeKindsIn(t, gd)...) + } + } + require.NotEmpty(t, kinds, "the ir sources must declare TypeKind constants") + return kinds +} + +// typeKindsIn returns the TypeKind constants one const group declares. A spec +// naming neither type nor value repeats the previous one, so the group's last +// explicit type carries forward; a spec with a value of its own declares its own +// type. +func typeKindsIn(t *testing.T, gd *ast.GenDecl) []ir.TypeKind { + t.Helper() + var kinds []ir.TypeKind + isKind := false + for _, spec := range gd.Specs { + vs, isValue := spec.(*ast.ValueSpec) + require.True(t, isValue, "const spec is not a ValueSpec: %#v", spec) + switch { + case vs.Type != nil: + id, isIdent := vs.Type.(*ast.Ident) + isKind = isIdent && id.Name == "TypeKind" + case len(vs.Values) > 0: + isKind = false + } + if !isKind { + continue + } + for i, name := range vs.Names { + require.Less(t, i, len(vs.Values), "TypeKind constant %s must declare its own value", name.Name) + kinds = append(kinds, ir.TypeKind(stringLit(t, name.Name, vs.Values[i]))) + } + } + return kinds +} + +// stringLit returns the string a constant's value expression spells out. +func stringLit(t *testing.T, constName string, expr ast.Expr) string { + t.Helper() + lit, isLit := expr.(*ast.BasicLit) + require.True(t, isLit, "constant %s must be declared as a string literal", constName) + require.Equal(t, token.STRING, lit.Kind, "constant %s must be declared as a string literal", constName) + unquoted, err := strconv.Unquote(lit.Value) + require.NoError(t, err, "unquoting the value of %s", constName) + return unquoted +} + +// irSourcePaths lists the ir package's non-test Go files, resolved against this +// file's own directory so the result does not depend on the working directory the +// suite runs from. +func irSourcePaths(t *testing.T) []string { + t.Helper() + _, self, _, ok := runtime.Caller(0) + require.True(t, ok, "runtime.Caller must report this test's path") + matches, err := filepath.Glob(filepath.Join(filepath.Dir(self), "..", "ir", "*.go")) + require.NoError(t, err) + + paths := make([]string, 0, len(matches)) + for _, path := range matches { + if strings.HasSuffix(path, "_test.go") { + continue + } + paths = append(paths, path) + } + require.NotEmpty(t, paths, "the ir package must hold production Go sources") + return paths +} diff --git a/pass/validate_encoding_test.go b/pass/validate_encoding_test.go index 5c02145..2c78ff5 100644 --- a/pass/validate_encoding_test.go +++ b/pass/validate_encoding_test.go @@ -18,27 +18,30 @@ func multipartContent(enc map[ir.PropID]ir.PartEncoding) ir.Content { } // encodingCarrier is one field that carries a Payload — and so an Encoding map — -// paired with the location a diagnostic about its first content must point at. +// named as the ir field it is, and paired with the location a diagnostic about +// its first content must point at. type encodingCarrier struct { - name string + field string at string plant func(doc *ir.Document, enc map[ir.PropID]ir.PartEncoding) } -// encodingCarriers enumerates the Payload-bearing fields checkEncodingKeys walks, -// so one added to the IR has to appear here as well as there. +// encodingCarriers enumerates the Payload-bearing fields checkEncodingKeys walks. +// TestEncodingCarriers_AreEveryPayloadFieldInTheIR holds this list against the +// IR and the cases below hold checkEncodingKeys against this list, so a carrier +// added to the IR has to reach both. func encodingCarriers() []encodingCarrier { return []encodingCarrier{ - {"request", "op/request/contents/0", func(d *ir.Document, enc map[ir.PropID]ir.PartEncoding) { + {"Operation.Request", "op/request/contents/0", func(d *ir.Document, enc map[ir.PropID]ir.PartEncoding) { requestContent(d).Encoding = enc }}, - {"response payload", "op/responses/0/contents/0", func(d *ir.Document, enc map[ir.PropID]ir.PartEncoding) { + {"Response.Payload", "op/responses/0/contents/0", func(d *ir.Document, enc map[ir.PropID]ir.PartEncoding) { firstOp(d).Responses = []ir.Response{{ Name: ir.Naming{Source: "ok"}, Payload: &ir.Payload{Contents: []ir.Content{multipartContent(enc)}}, }} }}, - {"message payload", "msg/a/contents/0", func(d *ir.Document, enc map[ir.PropID]ir.PartEncoding) { + {"Message.Payload", "msg/a/contents/0", func(d *ir.Document, enc map[ir.PropID]ir.PartEncoding) { putMessage(d, func(m *ir.Message) { m.Payload = ir.Payload{Contents: []ir.Content{multipartContent(enc)}} }) @@ -55,7 +58,7 @@ func encodingCarriers() []encodingCarrier { func TestValidate_EncodingKeyAddressesNoProperty(t *testing.T) { t.Parallel() for _, tc := range encodingCarriers() { - t.Run(tc.name, func(t *testing.T) { + t.Run(tc.field, func(t *testing.T) { t.Parallel() doc := validDoc() tc.plant(doc, map[ir.PropID]ir.PartEncoding{"p/m/ghost": {Multi: true}})