Validate its own generated schemas (#1017)

* test(validators): reproduce the crash on a nested definedTypeLink

getValidationItemsVisitor throws `Expected node of kind [definedTypeLinkNode]`
whenever it walks a definedTypeLinkNode that sits below the top of the tree: a
struct field, an argument, an array element, or a PDA seed. That is most real
programs, and six of this repo's own test IDLs (example-idl, system, token,
token-2022, mpl-token-metadata, pmp) hit it.

Add two tests that build the smallest such shape from the node constructors,
both of which throw today: one asserts a resolvable link validates cleanly, the
other asserts a missing link is reported as an error. So the fix must restore
the visitor's ability to catch a broken link, not merely stop the crash.

* fix(validators): make recordNodeStackVisitor follow prefix traversal rules

getValidationItemsVisitor was the only visitor to run its per-node logic before
committing the current node to the stack, so validation ran in the gap between
arriving at a node and recording it. Two bugs fell out of that single ordering:

  - visitDefinedTypeLink calls stack.getPath(node.kind), which asserts the top
    of the stack is the link node; the link node was not committed yet, so the
    top was its parent, and any nested definedTypeLink (struct field, argument,
    array, PDA seed) threw `Expected node of kind [definedTypeLinkNode]`. Six of
    this repo's own test IDLs hit it, including example-idl.json, System, Token,
    and Token-2022.
  - every validationItem recorded an ancestors-only path, missing the node
    itself, which contradicts the documented ValidationItem.path contract
    ("the path of nodes that led to the node above, including the node itself").

Commit the node before the per-node logic runs, matching the eleven other
recordNodeStackVisitor consumers (getByteSizeVisitor, and the rest): the node
is on the stack when the visit runs, so getPath(node.kind) resolves, missing
links are reported instead of throwing, and recorded paths satisfy the
contract. The two existing tests asserted the off-by-one paths, so they are
corrected to the documented behaviour.

* prettier(validators): fix write :)

Author: cds-amal

.changeset/fix-validator-nested-defined-type-link.md

@@ -0,0 +1,5 @@
+---
+'@codama/validators': patch
+---
+
+Fix `getValidationItemsVisitor` throwing `Expected node of kind [definedTypeLinkNode]` on any nested `definedTypeLinkNode` (a struct field, argument, array element, or PDA seed), and recording `ValidationItem` paths that omitted the node itself. The visitor now composes `recordNodeStackVisitor` outermost, like every other visitor, so the current node is on the stack when validation runs.

packages/validators/src/getValidationItemsVisitor.ts

@@ -23,8 +23,6 @@ export function getValidationItemsVisitor(): Visitor<readonly ValidationItem[]>
             () => [] as readonly ValidationItem[],
             (_, items) => items.flat(),
         ),
-        v => recordLinkablesOnFirstVisitVisitor(v, linkables),
-        v => recordNodeStackVisitor(v, stack),
         v =>
             extendVisitor(v, {
                 visitAccount(node, { next }) {
@@ -243,5 +241,11 @@ export function getValidationItemsVisitor(): Visitor<readonly ValidationItem[]>
                     return [...items, ...next(node)];
                 },
             }),
+        // Pipe stages run outermost-first, i.e. in reverse of their listing here:
+        //   pipe(init, g, h, i) => i(h(g(init)))   -- i is the outer layer, runs first
+        // so these record the node (onto the stack, and into linkables) BEFORE the
+        // extendVisitor logic above runs and reads that state to validate the node.
+        v => recordNodeStackVisitor(v, stack),
+        v => recordLinkablesOnFirstVisitVisitor(v, linkables),
     );
 }

packages/validators/test/getValidationItemsVisitor.test.ts

@@ -1,4 +1,13 @@
-import { programNode, publicKeyTypeNode, structFieldTypeNode, structTypeNode, tupleTypeNode } from '@codama/nodes';
+import {
+    definedTypeLinkNode,
+    definedTypeNode,
+    numberTypeNode,
+    programNode,
+    publicKeyTypeNode,
+    structFieldTypeNode,
+    structTypeNode,
+    tupleTypeNode,
+} from '@codama/nodes';
 import { visit } from '@codama/visitors-core';
 import { expect, test } from 'vitest';
 
@@ -23,10 +32,10 @@ test('it validates program nodes', () => {
 
     // Then we expect the following validation errors.
     expect(items).toEqual([
-        validationItem('error', 'Program has no name.', node, []),
-        validationItem('error', 'Program has no public key.', node, []),
-        validationItem('warn', 'Program has no version.', node, []),
-        validationItem('info', 'Program has no origin.', node, []),
+        validationItem('error', 'Program has no name.', node, [node]),
+        validationItem('error', 'Program has no public key.', node, [node]),
+        validationItem('warn', 'Program has no version.', node, [node]),
+        validationItem('info', 'Program has no origin.', node, [node]),
     ]);
 });
 
@@ -47,7 +56,65 @@ test('it validates nested nodes', () => {
     const tupleNode = node.items[0];
     const structNode = node.items[1];
     expect(items).toEqual([
-        validationItem('warn', 'Tuple has no items.', tupleNode, [node]),
-        validationItem('error', 'Struct field name "owner" is not unique.', structNode.fields[0], [node]),
+        validationItem('warn', 'Tuple has no items.', tupleNode, [node, tupleNode]),
+        validationItem('error', 'Struct field name "owner" is not unique.', structNode.fields[0], [node, structNode]),
+    ]);
+});
+
+test('it validates a defined type link nested within a struct field', () => {
+    // Given a program whose defined type links to another through a struct field.
+    const node = programNode({
+        accounts: [],
+        definedTypes: [
+            definedTypeNode({
+                name: 'foo',
+                type: structTypeNode([structFieldTypeNode({ name: 'bar', type: definedTypeLinkNode('baz') })]),
+            }),
+            definedTypeNode({ name: 'baz', type: numberTypeNode('u64') }),
+        ],
+        errors: [],
+        instructions: [],
+        name: 'test',
+        origin: 'anchor',
+        publicKey: '11111111111111111111111111111111',
+        version: '1.0.0',
+    });
+
+    // When we get the validation items using a visitor.
+    const items = visit(node, getValidationItemsVisitor());
+
+    // Then we expect no validation errors.
+    expect(items).toEqual([]);
+});
+
+test('it reports a nested defined type link that points at a missing type', () => {
+    // Given a program whose defined type field links to a type that does not exist.
+    const link = definedTypeLinkNode('missing');
+    const field = structFieldTypeNode({ name: 'bar', type: link });
+    const struct = structTypeNode([field]);
+    const foo = definedTypeNode({ name: 'foo', type: struct });
+    const node = programNode({
+        accounts: [],
+        definedTypes: [foo],
+        errors: [],
+        instructions: [],
+        name: 'test',
+        origin: 'anchor',
+        publicKey: '11111111111111111111111111111111',
+        version: '1.0.0',
+    });
+
+    // When we get the validation items using a visitor.
+    const items = visit(node, getValidationItemsVisitor());
+
+    // Then the missing link is reported as an error.
+    expect(items).toEqual([
+        validationItem('error', 'Pointing to a missing defined type named "missing"', link, [
+            node,
+            foo,
+            struct,
+            field,
+            link,
+        ]),
     ]);
 });