Skip to content

Commit d88b551

Browse files
committed
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.
1 parent f19ab36 commit d88b551

3 files changed

Lines changed: 17 additions & 8 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@codama/validators': patch
3+
---
4+
5+
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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@ export function getValidationItemsVisitor(): Visitor<readonly ValidationItem[]>
2323
() => [] as readonly ValidationItem[],
2424
(_, items) => items.flat(),
2525
),
26-
v => recordLinkablesOnFirstVisitVisitor(v, linkables),
27-
v => recordNodeStackVisitor(v, stack),
2826
v =>
2927
extendVisitor(v, {
3028
visitAccount(node, { next }) {
@@ -243,5 +241,11 @@ export function getValidationItemsVisitor(): Visitor<readonly ValidationItem[]>
243241
return [...items, ...next(node)];
244242
},
245243
}),
244+
// Pipe stages run outermost-first, i.e. in reverse of their listing here:
245+
// pipe(init, g, h, i) => i(h(g(init))) -- i is the outer layer, runs first
246+
// so these record the node (onto the stack, and into linkables) BEFORE the
247+
// extendVisitor logic above runs and reads that state to validate the node.
248+
v => recordNodeStackVisitor(v, stack),
249+
v => recordLinkablesOnFirstVisitVisitor(v, linkables),
246250
);
247251
}

packages/validators/test/getValidationItemsVisitor.test.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,10 @@ test('it validates program nodes', () => {
3232

3333
// Then we expect the following validation errors.
3434
expect(items).toEqual([
35-
validationItem('error', 'Program has no name.', node, []),
36-
validationItem('error', 'Program has no public key.', node, []),
37-
validationItem('warn', 'Program has no version.', node, []),
38-
validationItem('info', 'Program has no origin.', node, []),
35+
validationItem('error', 'Program has no name.', node, [node]),
36+
validationItem('error', 'Program has no public key.', node, [node]),
37+
validationItem('warn', 'Program has no version.', node, [node]),
38+
validationItem('info', 'Program has no origin.', node, [node]),
3939
]);
4040
});
4141

@@ -56,8 +56,8 @@ test('it validates nested nodes', () => {
5656
const tupleNode = node.items[0];
5757
const structNode = node.items[1];
5858
expect(items).toEqual([
59-
validationItem('warn', 'Tuple has no items.', tupleNode, [node]),
60-
validationItem('error', 'Struct field name "owner" is not unique.', structNode.fields[0], [node]),
59+
validationItem('warn', 'Tuple has no items.', tupleNode, [node, tupleNode]),
60+
validationItem('error', 'Struct field name "owner" is not unique.', structNode.fields[0], [node, structNode]),
6161
]);
6262
});
6363

0 commit comments

Comments
 (0)