From c482f760fe73684570d9c6a75150d1b25ca5047f Mon Sep 17 00:00:00 2001 From: Loris Leiva Date: Fri, 22 Nov 2024 12:23:54 +0000 Subject: [PATCH 1/5] Add tests for unwrapInstructionArgsDefinedTypesVisitor Including a failing one to fix. --- ...InstructionArgsDefinedTypesVisitor.test.ts | 190 ++++++++++++++++++ 1 file changed, 190 insertions(+) create mode 100644 packages/visitors/test/unwrapInstructionArgsDefinedTypesVisitor.test.ts diff --git a/packages/visitors/test/unwrapInstructionArgsDefinedTypesVisitor.test.ts b/packages/visitors/test/unwrapInstructionArgsDefinedTypesVisitor.test.ts new file mode 100644 index 000000000..620a16579 --- /dev/null +++ b/packages/visitors/test/unwrapInstructionArgsDefinedTypesVisitor.test.ts @@ -0,0 +1,190 @@ +import { + arrayTypeNode, + definedTypeLinkNode, + definedTypeNode, + fixedCountNode, + instructionArgumentNode, + instructionNode, + numberTypeNode, + programNode, + rootNode, + structFieldTypeNode, + structTypeNode, +} from '@codama/nodes'; +import { visit } from '@codama/visitors-core'; +import { expect, test } from 'vitest'; + +import { unwrapInstructionArgsDefinedTypesVisitor } from '../src'; + +test('it unwraps defined type link nodes used as instruction arguments', () => { + // Given a program with a type used only once as a direct instruction argument. + const node = rootNode( + programNode({ + definedTypes: [ + definedTypeNode({ + name: 'typeA', + type: structTypeNode([structFieldTypeNode({ name: 'foo', type: numberTypeNode('u8') })]), + }), + definedTypeNode({ + name: 'typeB', + type: structTypeNode([structFieldTypeNode({ name: 'bar', type: numberTypeNode('u8') })]), + }), + ], + instructions: [ + instructionNode({ + arguments: [instructionArgumentNode({ name: 'argA', type: definedTypeLinkNode('typeA') })], + name: 'myInstruction', + }), + ], + name: 'MyProgram', + publicKey: '1111', + version: '1.2.3', + }), + ); + + // When the defined type link nodes are unwrapped. + const result = visit(node, unwrapInstructionArgsDefinedTypesVisitor()); + + // Then we expect the following node. + expect(result).toStrictEqual( + rootNode( + programNode({ + definedTypes: [ + definedTypeNode({ + name: 'typeB', + type: structTypeNode([structFieldTypeNode({ name: 'bar', type: numberTypeNode('u8') })]), + }), + ], + instructions: [ + instructionNode({ + arguments: [ + instructionArgumentNode({ + name: 'argA', + type: structTypeNode([ + structFieldTypeNode({ name: 'foo', type: numberTypeNode('u8') }), + ]), + }), + ], + name: 'myInstruction', + }), + ], + name: 'MyProgram', + publicKey: '1111', + version: '1.2.3', + }), + ), + ); +}); + +test('it does not unwrap defined type link nodes that are used in more than one place.', () => { + // Given a link node used in an instruction argument and in another place. + const node = rootNode( + programNode({ + definedTypes: [ + definedTypeNode({ + name: 'myType', + type: structTypeNode([structFieldTypeNode({ name: 'foo', type: numberTypeNode('u8') })]), + }), + definedTypeNode({ name: 'myCopyType', type: definedTypeLinkNode('myType') }), + ], + instructions: [ + instructionNode({ + arguments: [instructionArgumentNode({ name: 'myArg', type: definedTypeLinkNode('myType') })], + name: 'myInstruction', + }), + ], + name: 'MyProgram', + publicKey: '1111', + version: '1.2.3', + }), + ); + + // When we try to unwrap defined type link nodes for instruction arguments. + const result = visit(node, unwrapInstructionArgsDefinedTypesVisitor()); + + // Then we expect the same node. + expect(result).toStrictEqual(node); +}); + +test('it only unwraps defined type link nodes if they are direct instruction arguments', () => { + // Given a link node used in an instruction argument but not as a direct argument. + const node = rootNode( + programNode({ + definedTypes: [ + definedTypeNode({ + name: 'myType', + type: structTypeNode([structFieldTypeNode({ name: 'foo', type: numberTypeNode('u8') })]), + }), + ], + instructions: [ + instructionNode({ + arguments: [ + instructionArgumentNode({ + name: 'myArg', + type: arrayTypeNode(definedTypeLinkNode('myType'), fixedCountNode(3)), + }), + ], + name: 'myInstruction', + }), + ], + name: 'MyProgram', + publicKey: '1111', + version: '1.2.3', + }), + ); + + // When we try to unwrap defined type link nodes for instruction arguments. + const result = visit(node, unwrapInstructionArgsDefinedTypesVisitor()); + + // Then we expect the same node. + expect(result).toStrictEqual(node); +}); + +test('it does not unwrap defined type link nodes from other programs', () => { + // Given a program that defines the + const programA = programNode({ + definedTypes: [definedTypeNode({ name: 'myType', type: numberTypeNode('u8') })], + instructions: [ + instructionNode({ + arguments: [instructionArgumentNode({ name: 'myArg', type: definedTypeLinkNode('myType') })], + name: 'myInstruction', + }), + ], + name: 'programA', + publicKey: '1111', + version: '1.2.3', + }); + + // And another program with a defined type sharing the same name. + const programB = programNode({ + definedTypes: [ + definedTypeNode({ name: 'myType', type: numberTypeNode('u16') }), + definedTypeNode({ name: 'myCopyType', type: definedTypeLinkNode('myType') }), + ], + name: 'programB', + publicKey: '2222', + version: '1.2.3', + }); + + // When we unwrap defined type link nodes for instruction arguments for both of them. + const node = rootNode(programA, [programB]); + const result = visit(node, unwrapInstructionArgsDefinedTypesVisitor()); + + // Then we expect program A to have been modified but not program B. + expect(result).toStrictEqual( + rootNode( + programNode({ + instructions: [ + instructionNode({ + arguments: [instructionArgumentNode({ name: 'myArg', type: numberTypeNode('u8') })], + name: 'myInstruction', + }), + ], + name: 'programA', + publicKey: '1111', + version: '1.2.3', + }), + [programB], + ), + ); +}); From a1b653a0bb6bd4d4867e0cad1dbdae1d81f19c00 Mon Sep 17 00:00:00 2001 From: Loris Leiva Date: Fri, 22 Nov 2024 12:24:36 +0000 Subject: [PATCH 2/5] Add Vitest workspace config to use Vitest extensions locally --- pnpm-lock.yaml | 2 +- vitest.workspace.ts | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 vitest.workspace.ts diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 90fd47be2..bb26dd3fb 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -3348,7 +3348,7 @@ snapshots: chokidar@3.6.0: dependencies: anymatch: 3.1.3 - braces: 3.0.2 + braces: 3.0.3 glob-parent: 5.1.2 is-binary-path: 2.1.0 is-glob: 4.0.3 diff --git a/vitest.workspace.ts b/vitest.workspace.ts new file mode 100644 index 000000000..c5742e592 --- /dev/null +++ b/vitest.workspace.ts @@ -0,0 +1,3 @@ +import { defineWorkspace } from 'vitest/config'; + +export default defineWorkspace(['./packages/internals/vitest.config.node.mts']); From 3e8304cc791848abd77012dd957483f72c9b6c72 Mon Sep 17 00:00:00 2001 From: Loris Leiva Date: Fri, 22 Nov 2024 12:30:18 +0000 Subject: [PATCH 3/5] Add more failing tests to fix --- .../test/unwrapDefinedTypesVisitor.test.ts | 39 +++++++++++++++++++ ...InstructionArgsDefinedTypesVisitor.test.ts | 7 ---- 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/packages/visitors/test/unwrapDefinedTypesVisitor.test.ts b/packages/visitors/test/unwrapDefinedTypesVisitor.test.ts index 5810abe74..db57a991a 100644 --- a/packages/visitors/test/unwrapDefinedTypesVisitor.test.ts +++ b/packages/visitors/test/unwrapDefinedTypesVisitor.test.ts @@ -72,3 +72,42 @@ test('it follows linked nodes using the correct paths', () => { definedTypeNode({ name: 'typeA', type: numberTypeNode('u64') }), ); }); + +test('it does not unwrap types from the wrong programs', () => { + // Given a program node with a defined type used in another type. + const programA = programNode({ + definedTypes: [ + definedTypeNode({ name: 'myType', type: numberTypeNode('u8') }), + definedTypeNode({ name: 'myCopyType', type: definedTypeLinkNode('myType') }), + ], + name: 'programA', + publicKey: '1111', + }); + + // And another program with a defined type sharing the same name. + const programB = programNode({ + definedTypes: [ + definedTypeNode({ name: 'myType', type: numberTypeNode('u16') }), + definedTypeNode({ name: 'myCopyType', type: definedTypeLinkNode('myType') }), + ], + name: 'programB', + publicKey: '2222', + }); + + // When we unwrap the defined type from programA. + const node = rootNode(programA, [programB]); + const result = visit(node, unwrapDefinedTypesVisitor(['myType'])); + + // Then we expect programA to have been modified but not programB. + assertIsNode(result, 'rootNode'); + expect(result).toStrictEqual( + rootNode( + programNode({ + definedTypes: [definedTypeNode({ name: 'myCopyType', type: numberTypeNode('u8') })], + name: 'programA', + publicKey: '1111', + }), + [programB], + ), + ); +}); diff --git a/packages/visitors/test/unwrapInstructionArgsDefinedTypesVisitor.test.ts b/packages/visitors/test/unwrapInstructionArgsDefinedTypesVisitor.test.ts index 620a16579..38d884f25 100644 --- a/packages/visitors/test/unwrapInstructionArgsDefinedTypesVisitor.test.ts +++ b/packages/visitors/test/unwrapInstructionArgsDefinedTypesVisitor.test.ts @@ -38,7 +38,6 @@ test('it unwraps defined type link nodes used as instruction arguments', () => { ], name: 'MyProgram', publicKey: '1111', - version: '1.2.3', }), ); @@ -70,7 +69,6 @@ test('it unwraps defined type link nodes used as instruction arguments', () => { ], name: 'MyProgram', publicKey: '1111', - version: '1.2.3', }), ), ); @@ -95,7 +93,6 @@ test('it does not unwrap defined type link nodes that are used in more than one ], name: 'MyProgram', publicKey: '1111', - version: '1.2.3', }), ); @@ -129,7 +126,6 @@ test('it only unwraps defined type link nodes if they are direct instruction arg ], name: 'MyProgram', publicKey: '1111', - version: '1.2.3', }), ); @@ -152,7 +148,6 @@ test('it does not unwrap defined type link nodes from other programs', () => { ], name: 'programA', publicKey: '1111', - version: '1.2.3', }); // And another program with a defined type sharing the same name. @@ -163,7 +158,6 @@ test('it does not unwrap defined type link nodes from other programs', () => { ], name: 'programB', publicKey: '2222', - version: '1.2.3', }); // When we unwrap defined type link nodes for instruction arguments for both of them. @@ -182,7 +176,6 @@ test('it does not unwrap defined type link nodes from other programs', () => { ], name: 'programA', publicKey: '1111', - version: '1.2.3', }), [programB], ), From 72ec63878888e75203bdab3b4e425e9f1a7b0a4a Mon Sep 17 00:00:00 2001 From: Loris Leiva Date: Fri, 22 Nov 2024 13:59:40 +0000 Subject: [PATCH 4/5] Fix new tests --- .../src/getDefinedTypeHistogramVisitor.ts | 22 ++++++-- .../visitors/src/unwrapDefinedTypesVisitor.ts | 20 ++++++-- ...nwrapInstructionArgsDefinedTypesVisitor.ts | 21 ++++---- .../getDefinedTypeHistogramVisitor.test.ts | 50 ++++++++++++++++++- .../test/unwrapDefinedTypesVisitor.test.ts | 2 +- 5 files changed, 93 insertions(+), 22 deletions(-) diff --git a/packages/visitors/src/getDefinedTypeHistogramVisitor.ts b/packages/visitors/src/getDefinedTypeHistogramVisitor.ts index 067800c20..008bb7649 100644 --- a/packages/visitors/src/getDefinedTypeHistogramVisitor.ts +++ b/packages/visitors/src/getDefinedTypeHistogramVisitor.ts @@ -1,8 +1,20 @@ import { CamelCaseString } from '@codama/nodes'; -import { extendVisitor, interceptVisitor, mergeVisitor, pipe, visit, Visitor } from '@codama/visitors-core'; +import { + extendVisitor, + findProgramNodeFromPath, + interceptVisitor, + mergeVisitor, + NodeStack, + pipe, + recordNodeStackVisitor, + visit, + Visitor, +} from '@codama/visitors-core'; + +type DefinedTypeHistogramKey = CamelCaseString | `${CamelCaseString}.${CamelCaseString}`; export type DefinedTypeHistogram = { - [key: CamelCaseString]: { + [key: DefinedTypeHistogramKey]: { directlyAsInstructionArgs: number; inAccounts: number; inDefinedTypes: number; @@ -33,6 +45,7 @@ function mergeHistograms(histograms: DefinedTypeHistogram[]): DefinedTypeHistogr } export function getDefinedTypeHistogramVisitor(): Visitor { + const stack = new NodeStack(); let mode: 'account' | 'definedType' | 'instruction' | null = null; let stackLevel = 0; @@ -67,8 +80,10 @@ export function getDefinedTypeHistogramVisitor(): Visitor }, visitDefinedTypeLink(node) { + const program = findProgramNodeFromPath(stack.getPath()); + const key = program ? `${program.name}.${node.name}` : node.name; return { - [node.name]: { + [key]: { directlyAsInstructionArgs: Number(mode === 'instruction' && stackLevel <= 1), inAccounts: Number(mode === 'account'), inDefinedTypes: Number(mode === 'definedType'), @@ -88,5 +103,6 @@ export function getDefinedTypeHistogramVisitor(): Visitor return mergeHistograms([...dataHistograms, ...extraHistograms, ...subHistograms]); }, }), + v => recordNodeStackVisitor(v, stack), ); } diff --git a/packages/visitors/src/unwrapDefinedTypesVisitor.ts b/packages/visitors/src/unwrapDefinedTypesVisitor.ts index 56da06df2..4e9b0cf19 100644 --- a/packages/visitors/src/unwrapDefinedTypesVisitor.ts +++ b/packages/visitors/src/unwrapDefinedTypesVisitor.ts @@ -1,6 +1,7 @@ import { assertIsNodeFilter, camelCase, CamelCaseString, programNode } from '@codama/nodes'; import { extendVisitor, + findProgramNodeFromPath, getLastNodeFromPath, LinkableDictionary, NodeStack, @@ -14,16 +15,25 @@ import { export function unwrapDefinedTypesVisitor(typesToInline: string[] | '*' = '*') { const linkables = new LinkableDictionary(); const stack = new NodeStack(); - const typesToInlineMainCased = typesToInline === '*' ? '*' : typesToInline.map(camelCase); - const shouldInline = (definedType: CamelCaseString): boolean => - typesToInlineMainCased === '*' || typesToInlineMainCased.includes(definedType); + const typesToInlineCamelCased = (typesToInline === '*' ? [] : typesToInline).map(fullPath => { + if (!fullPath.includes('.')) return camelCase(fullPath); + const [programName, typeName] = fullPath.split('.'); + return `${camelCase(programName)}.${camelCase(typeName)}`; + }); + const shouldInline = (typeName: CamelCaseString, programName: CamelCaseString | undefined): boolean => { + if (typesToInline === '*') return true; + const fullPath = `${programName}.${typeName}`; + if (!!programName && typesToInlineCamelCased.includes(fullPath)) return true; + return typesToInlineCamelCased.includes(typeName); + }; return pipe( nonNullableIdentityVisitor(), v => extendVisitor(v, { visitDefinedTypeLink(linkType, { self }) { - if (!shouldInline(linkType.name)) { + const programName = linkType.program?.name ?? findProgramNodeFromPath(stack.getPath())?.name; + if (!shouldInline(linkType.name, programName)) { return linkType; } const definedTypePath = linkables.getPathOrThrow(stack.getPath('definedTypeLinkNode')); @@ -42,7 +52,7 @@ export function unwrapDefinedTypesVisitor(typesToInline: string[] | '*' = '*') { .map(account => visit(account, self)) .filter(assertIsNodeFilter('accountNode')), definedTypes: program.definedTypes - .filter(definedType => !shouldInline(definedType.name)) + .filter(definedType => !shouldInline(definedType.name, program.name)) .map(type => visit(type, self)) .filter(assertIsNodeFilter('definedTypeNode')), instructions: program.instructions diff --git a/packages/visitors/src/unwrapInstructionArgsDefinedTypesVisitor.ts b/packages/visitors/src/unwrapInstructionArgsDefinedTypesVisitor.ts index 0e9d23958..ec2faa6c2 100644 --- a/packages/visitors/src/unwrapInstructionArgsDefinedTypesVisitor.ts +++ b/packages/visitors/src/unwrapInstructionArgsDefinedTypesVisitor.ts @@ -1,5 +1,5 @@ -import { assertIsNode, CamelCaseString, getAllDefinedTypes, isNode } from '@codama/nodes'; -import { rootNodeVisitor, visit } from '@codama/visitors-core'; +import { assertIsNode, CamelCaseString, definedTypeLinkNode, isNode } from '@codama/nodes'; +import { getRecordLinkablesVisitor, LinkableDictionary, rootNodeVisitor, visit } from '@codama/visitors-core'; import { getDefinedTypeHistogramVisitor } from './getDefinedTypeHistogramVisitor'; import { unwrapDefinedTypesVisitor } from './unwrapDefinedTypesVisitor'; @@ -7,18 +7,17 @@ import { unwrapDefinedTypesVisitor } from './unwrapDefinedTypesVisitor'; export function unwrapInstructionArgsDefinedTypesVisitor() { return rootNodeVisitor(root => { const histogram = visit(root, getDefinedTypeHistogramVisitor()); - const allDefinedTypes = getAllDefinedTypes(root); + const linkables = new LinkableDictionary(); + visit(root, getRecordLinkablesVisitor(linkables)); - const definedTypesToInline: string[] = Object.keys(histogram) + const definedTypesToInline = (Object.keys(histogram) as CamelCaseString[]) // Get all defined types used exactly once as an instruction argument. - .filter( - name => - (histogram[name as CamelCaseString].total ?? 0) === 1 && - (histogram[name as CamelCaseString].directlyAsInstructionArgs ?? 0) === 1, - ) + .filter(key => (histogram[key].total ?? 0) === 1 && (histogram[key].directlyAsInstructionArgs ?? 0) === 1) // Filter out enums which are better defined as external types. - .filter(name => { - const found = allDefinedTypes.find(type => type.name === name); + .filter(key => { + const names = key.split('.'); + const link = names.length == 2 ? definedTypeLinkNode(names[1], names[0]) : definedTypeLinkNode(key); + const found = linkables.get([link]); return found && !isNode(found.type, 'enumTypeNode'); }); diff --git a/packages/visitors/test/getDefinedTypeHistogramVisitor.test.ts b/packages/visitors/test/getDefinedTypeHistogramVisitor.test.ts index 85c6bed08..a429f5444 100644 --- a/packages/visitors/test/getDefinedTypeHistogramVisitor.test.ts +++ b/packages/visitors/test/getDefinedTypeHistogramVisitor.test.ts @@ -5,7 +5,9 @@ import { enumTypeNode, instructionArgumentNode, instructionNode, + numberTypeNode, programNode, + rootNode, structFieldTypeNode, structTypeNode, } from '@codama/nodes'; @@ -65,14 +67,14 @@ test('it counts the amount of times defined types are used within the tree', () // Then we expect the following histogram. expect(histogram).toEqual({ - myEnum: { + 'customProgram.myEnum': { directlyAsInstructionArgs: 0, inAccounts: 1, inDefinedTypes: 0, inInstructionArgs: 0, total: 1, }, - myStruct: { + 'customProgram.myStruct': { directlyAsInstructionArgs: 1, inAccounts: 1, inDefinedTypes: 0, @@ -81,3 +83,47 @@ test('it counts the amount of times defined types are used within the tree', () }, }); }); + +test('it counts links from different programs separately', () => { + // Given a program node with a defined type used in another type. + const programA = programNode({ + definedTypes: [ + definedTypeNode({ name: 'myType', type: numberTypeNode('u8') }), + definedTypeNode({ name: 'myCopyType', type: definedTypeLinkNode('myType') }), + ], + name: 'programA', + publicKey: '1111', + }); + + // And another program with a defined type sharing the same name. + const programB = programNode({ + definedTypes: [ + definedTypeNode({ name: 'myType', type: numberTypeNode('u16') }), + definedTypeNode({ name: 'myCopyType', type: definedTypeLinkNode('myType') }), + ], + name: 'programB', + publicKey: '2222', + }); + + // When we unwrap the defined type from programA. + const node = rootNode(programA, [programB]); + const histogram = visit(node, getDefinedTypeHistogramVisitor()); + + // Then we expect programA to have been modified but not programB. + expect(histogram).toStrictEqual({ + 'programA.myType': { + directlyAsInstructionArgs: 0, + inAccounts: 0, + inDefinedTypes: 1, + inInstructionArgs: 0, + total: 1, + }, + 'programB.myType': { + directlyAsInstructionArgs: 0, + inAccounts: 0, + inDefinedTypes: 1, + inInstructionArgs: 0, + total: 1, + }, + }); +}); diff --git a/packages/visitors/test/unwrapDefinedTypesVisitor.test.ts b/packages/visitors/test/unwrapDefinedTypesVisitor.test.ts index db57a991a..4e27c00a8 100644 --- a/packages/visitors/test/unwrapDefinedTypesVisitor.test.ts +++ b/packages/visitors/test/unwrapDefinedTypesVisitor.test.ts @@ -96,7 +96,7 @@ test('it does not unwrap types from the wrong programs', () => { // When we unwrap the defined type from programA. const node = rootNode(programA, [programB]); - const result = visit(node, unwrapDefinedTypesVisitor(['myType'])); + const result = visit(node, unwrapDefinedTypesVisitor(['programA.myType'])); // Then we expect programA to have been modified but not programB. assertIsNode(result, 'rootNode'); From a1377ba34c0ab545bf71ddb3c7ecaeabe7d094cc Mon Sep 17 00:00:00 2001 From: Loris Leiva Date: Fri, 22 Nov 2024 14:10:19 +0000 Subject: [PATCH 5/5] Add changeset --- .changeset/giant-items-sniff.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/giant-items-sniff.md diff --git a/.changeset/giant-items-sniff.md b/.changeset/giant-items-sniff.md new file mode 100644 index 000000000..f4eca2a7f --- /dev/null +++ b/.changeset/giant-items-sniff.md @@ -0,0 +1,5 @@ +--- +'@codama/visitors': minor +--- + +Use program names when unwrapping link nodes