From 7728f6f685cc3909194bc8d0d557cc597e0a2fc1 Mon Sep 17 00:00:00 2001 From: CJ42 Date: Thu, 13 Jul 2023 17:30:24 +0100 Subject: [PATCH 1/3] feat: add support to render Natspec of internal functions --- src/abiDecoder.ts | 1 + src/dodocTypes.ts | 3 + src/index.ts | 233 ++++++++++++++++++++++++++++++++++------------ src/template.sqrl | 47 ++++++++++ 4 files changed, 222 insertions(+), 62 deletions(-) diff --git a/src/abiDecoder.ts b/src/abiDecoder.ts index 0ec7c7a..7994cf4 100644 --- a/src/abiDecoder.ts +++ b/src/abiDecoder.ts @@ -61,6 +61,7 @@ export function decodeAbi(abi: AbiElement[]): Doc { methods: {}, events: {}, errors: {}, + internalMethods: {}, }; for (let i = 0; i < abi.length; i += 1) { diff --git a/src/dodocTypes.ts b/src/dodocTypes.ts index 28161bd..032dce4 100644 --- a/src/dodocTypes.ts +++ b/src/dodocTypes.ts @@ -154,4 +154,7 @@ export interface Doc { errors: { [key: string]: Error; }; + internalMethods: { + [key: string]: Method; + }; } diff --git a/src/index.ts b/src/index.ts index e31f073..5a32fa4 100644 --- a/src/index.ts +++ b/src/index.ts @@ -71,14 +71,14 @@ async function generateDocumentation(hre: HardhatRuntimeEnvironment): Promise { + const parseNatspecFromAST = (functionSig: string, functionASTNode: any) => { const tags = functionASTNode.documentation.text.split('@'); + const docEntry = doc.methods[functionSig] || doc.internalMethods[functionSig]; + tags.forEach((natspecTag: any) => { if (natspecTag.replace(' ', '').length === 0) { return; } if (natspecTag.startsWith('dev ')) { - doc.methods[`${functionName}()`].details = natspecTag.replace('dev ', '').trim(); + docEntry.details = natspecTag.replace('dev ', '').trim(); } if (natspecTag.startsWith('notice ')) { - doc.methods[`${functionName}()`].notice = natspecTag.replace('notice ', '').trim(); + docEntry.notice = natspecTag.replace('notice ', '').trim(); } // add custom any `@custom:` tags if (natspecTag.startsWith('custom:')) { const customTagName = natspecTag.substring('custom:'.length, natspecTag.trim().indexOf(' ')); - doc.methods[`${functionName}()`][`custom:${customTagName}`] = natspecTag.replace( - `custom:${customTagName} `, - '', - ); + docEntry[`custom:${customTagName}`] = natspecTag.replace(`custom:${customTagName} `, ''); } }); }; @@ -211,51 +215,42 @@ async function generateDocumentation(hre: HardhatRuntimeEnvironment): Promise { - const paramDoc = fallbackASTNode.documentation.text + const parseParamsAndReturnNatspecFromAST = ( + astNode: any, + functionName: string, + docEntry: 'methods' | 'internalMethods' | 'events' | 'errors', + ) => { + const paramDoc = astNode.documentation.text .match(/@.*/g) .filter((text: string) => text.match(/@param.*/)); if (paramDoc.length !== 0) { - const paramName = fallbackASTNode.parameters.parameters[0].name; - doc.methods['fallback()'].inputs[paramName] = { - type: 'bytes', - description: paramDoc[0].replace(`@param ${paramName} `, ''), - }; + astNode.parameters.parameters.forEach((param: any, index: number) => { + const paramName = param.name; + const paramType = param.typeName.name; + + doc[docEntry][functionName].inputs[paramName] = { + type: paramType, + description: paramDoc[index].replace(`@param ${paramName} `, ''), + }; + }); } - const returnDoc = fallbackASTNode.documentation.text + const returnDoc = astNode.documentation.text .match(/@.*/g) .filter((text: string) => text.match(/@return.*/)); - if (returnDoc.length !== 0) { - const returnVariableName = - fallbackASTNode.returnParameters.parameters[0].name === '' - ? '' - : fallbackASTNode.returnParameters.parameters[0].name; - - doc.methods['fallback()'].outputs[returnVariableName] = { - type: 'bytes', - description: returnDoc[0].replace(`@return ${returnVariableName} `, ''), - }; - } - }; + // custom errors and events do not have return parameters + if (returnDoc.length !== 0 && docEntry !== 'errors' && docEntry !== 'events') { + astNode.returnParameters.parameters.forEach((returnParam: any, index: number) => { + const returnVariableName = returnParam.name === '' ? `_${index}` : returnParam.name; + const returnParamType = returnParam.typeName.name; - const parseNatspecFromFallback = (fallbackASTNode: any) => { - parseNatspecFromAST('fallback', fallbackASTNode); - - // parse any @param or @return tags if fallback function is written as - // `fallback(bytes calldata fallbackParam) external returns (bytes memory)` - // - // Note: we should ideally have only a single `@param` or `@return` tag in this case - parseParamsAndReturnNatspecsForFallback(fallbackASTNode); - - // modify the code if the fallback is written as `fallback(bytes calldata fallbackParam) external returns (bytes memory)` - if ( - fallbackASTNode.parameters.parameters.length === 1 && - fallbackASTNode.returnParameters.parameters.length === 1 - ) { - modifyFallbackFunctionSyntax(fallbackASTNode); + doc[docEntry][functionName].outputs[returnVariableName] = { + type: returnParamType, + description: returnDoc[index].replace(`@return ${returnVariableName} `, ''), + }; + }); } }; @@ -263,14 +258,14 @@ async function generateDocumentation(hre: HardhatRuntimeEnvironment): Promise node.contractKind === 'contract')[0]; if (doc.methods['receive()'] !== undefined) { const receiveASTNode = contractNode.nodes.find((node: any) => node.kind === 'receive'); if (receiveASTNode !== undefined && receiveASTNode.hasOwnProperty('documentation')) { - parseNatspecFromAST('receive', receiveASTNode); + parseNatspecFromAST('receive()', receiveASTNode); } else { // search in the parent contracts // eslint-disable-next-line no-lonely-if @@ -293,7 +288,7 @@ async function generateDocumentation(hre: HardhatRuntimeEnvironment): Promise node.kind === 'fallback'); + const derivedFallbackASTNode = contractNode.nodes.find((node: any) => node.kind === 'fallback'); + + const parseNatspecFromFallback = (fallbackASTNode: any) => { + parseNatspecFromAST('fallback()', fallbackASTNode); + + // parse any @param or @return tags if fallback function is written as + // `fallback(bytes calldata fallbackParam) external returns (bytes memory)` + // + // Note: we should ideally have only a single `@param` or `@return` tag in this case + parseParamsAndReturnNatspecFromAST(fallbackASTNode, 'fallback()', 'methods'); + + // modify the code if the fallback is written as `fallback(bytes calldata fallbackParam) external returns (bytes memory)` + if ( + fallbackASTNode.parameters.parameters.length === 1 && + fallbackASTNode.returnParameters.parameters.length === 1 + ) { + modifyFallbackFunctionSyntax(fallbackASTNode); + } + }; - if (fallbackASTNode !== undefined && fallbackASTNode.hasOwnProperty('documentation')) { - parseNatspecFromFallback(fallbackASTNode); + if (derivedFallbackASTNode !== undefined && derivedFallbackASTNode.hasOwnProperty('documentation')) { + parseNatspecFromFallback(derivedFallbackASTNode); } else { // search in the parent contracts // eslint-disable-next-line no-lonely-if, no-prototype-builtins @@ -324,15 +337,15 @@ async function generateDocumentation(hre: HardhatRuntimeEnvironment): Promise 0 && baseContract.baseName.referencedDeclaration === inheritedContractAST[0].id ) { - const fallbackParentASTNode = inheritedContractAST[0].nodes.find( + const parentFallbackASTNode = inheritedContractAST[0].nodes.find( (node: any) => node.kind === 'fallback', ); if ( - fallbackParentASTNode !== undefined && - fallbackParentASTNode.hasOwnProperty('documentation') + parentFallbackASTNode !== undefined && + parentFallbackASTNode.hasOwnProperty('documentation') ) { - parseNatspecFromFallback(fallbackParentASTNode); + parseNatspecFromFallback(parentFallbackASTNode); // stop searching as soon as we find the most overriden function in the most derived contract break; @@ -344,6 +357,102 @@ async function generateDocumentation(hre: HardhatRuntimeEnvironment): Promise { + // Get Natspec of internal functions from AST + const internalFunctionsNodes = contract.nodes.filter( + (node: any) => + node.kind === 'function' && + node.nodeType === 'FunctionDefinition' && + node.visibility === 'internal', + ); + + if (internalFunctionsNodes.length > 0) { + // create entries for internal functions in doc.internalMethods + internalFunctionsNodes.forEach((internalFunctionNode: any) => { + const { + name: functionName, + stateMutability, + parameters: { parameters: params } = { parameters: [] }, + returnParameters: { parameters: returnParams } = { parameters: [] }, + } = internalFunctionNode; + + // this is non-standard, but our best attempt to create unique property name for each internal functions in the object + // there are no concept of function signatures and selector for internal functions + // (internal functions are not callable from outside the contract, and are of type 'function type) + // but we are using this way to store the natspec for each internal functions and differentiate them uniquely. + const internalFunctionSig = `${functionName}(${params + .map((param: any) => param.typeName.name) + .join(',')})`; + + let internalFunctionCode = `function ${functionName}(${params + .map((param: any) => `${param.typeName.name} ${param.name}`) + .join(',')}) internal`; + + internalFunctionCode += ` ${stateMutability}`; + + if (returnParams.length > 0) { + internalFunctionCode += ` returns (${returnParams + .map((returnParam: any) => `${returnParam.typeName.name} ${returnParam.name}`) + .join(',')})`; + } + + internalFunctionCode += ';'; + + if (!doc.internalMethods) { + doc.internalMethods = {}; + } + + doc.internalMethods[internalFunctionSig] = { + code: internalFunctionCode, + inputs: {}, + outputs: {}, + }; + + parseNatspecFromAST(internalFunctionSig, internalFunctionNode); + parseParamsAndReturnNatspecFromAST(internalFunctionNode, internalFunctionSig, 'internalMethods'); + }); + } + }; + + const libraryNodes = AST.filter((node: any) => node.contractKind === 'library'); + + // library do not have inheritance, so we can parse the Natspec directly + libraryNodes.forEach((library: any) => { + parseNatspecOfInternalFunctionsFromAST(library); + }); + + // contract have inheritance, so we need to search for all the internal functions + // through the linearized inheritance graph, + // from the most base (parent) to the most derived (child) contract + const contractsNode = AST.filter((node: any) => node.contractKind === 'contract'); + + contractsNode.forEach((contract: any) => { + const { linearizedBaseContracts } = contract; + + if (linearizedBaseContracts.length > 1) { + let ii = linearizedBaseContracts.length - 1; + + while (ii >= 0) { + const contractId = linearizedBaseContracts[ii]; + + for (const sourceFile in buildInfo?.output.sources) { + const matchingASTNode = buildInfo?.output.sources[sourceFile].ast.nodes.find( + (node: any) => node.contractKind === 'contract' && node.id === contractId, + ); + + if (matchingASTNode !== undefined) { + parseNatspecOfInternalFunctionsFromAST(matchingASTNode); + } + } + + ii--; + } + } else { + // parse directly if the contract does not inherit any other contract + parseNatspecOfInternalFunctionsFromAST(contract); + } + }); + for (const methodSig in info.devdoc?.methods) { const method = info.devdoc?.methods[methodSig]; diff --git a/src/template.sqrl b/src/template.sqrl index 15e3a20..3065469 100644 --- a/src/template.sqrl +++ b/src/template.sqrl @@ -39,6 +39,53 @@ {{@if (val['custom:info'])}}**Info:** *{{val['custom:info']}}*{{/if}} +{{@if (Object.keys(val.inputs).length > 0)}} +#### Parameters + +| Name | Type | Description | +|---|---|---| +{{@foreach(val.inputs) => key, val}} +| {{key}} | {{val.type}} | {{val.description}} | +{{/foreach}} +{{/if}} + +{{@if (Object.keys(val.outputs).length > 0)}} +#### Returns + +| Name | Type | Description | +|---|---|---| +{{@foreach(val.outputs) => key, val}} +| {{key}} | {{val.type}} | {{val.description}} | +{{/foreach}} + +{{/if}} +{{/foreach}} + +{{/if}} + +{{@if (Object.keys(it.internalMethods).length > 0)}} +## Internal Methods + +{{@foreach(it.internalMethods) => key, val}} +### {{key.split('(')[0]}} + + +```solidity +{{val.code}} + +``` + +{{@if (val.notice)}}{{val.notice}}{{/if}} + +{{@if (val.details)}}*{{val.details}}*{{/if}} + +{{@if (val['custom:requirement'])}}**Requirement:** *{{val['custom:requirement']}}*{{/if}} + +{{@if (val['custom:danger'])}}**Danger:** *{{val['custom:danger']}}*{{/if}} + +{{@if (val['custom:info'])}}**Info:** *{{val['custom:info']}}*{{/if}} + + {{@if (Object.keys(val.inputs).length > 0)}} #### Parameters From 4683558db175f682ab6d99d5565669d6c7bd4139 Mon Sep 17 00:00:00 2001 From: CJ42 Date: Fri, 14 Jul 2023 12:48:55 +0100 Subject: [PATCH 2/3] fix: do not parse from AST if no Natspec tags are specified --- src/index.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/index.ts b/src/index.ts index 5a32fa4..09b9c7b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -167,6 +167,8 @@ async function generateDocumentation(hre: HardhatRuntimeEnvironment): Promise { + if ('documentation' in functionASTNode === false) return; + const tags = functionASTNode.documentation.text.split('@'); const docEntry = doc.methods[functionSig] || doc.internalMethods[functionSig]; @@ -220,12 +222,18 @@ async function generateDocumentation(hre: HardhatRuntimeEnvironment): Promise { + if ('documentation' in astNode === false) return; + const paramDoc = astNode.documentation.text .match(/@.*/g) .filter((text: string) => text.match(/@param.*/)); - if (paramDoc.length !== 0) { + if (paramDoc.length > 0) { astNode.parameters.parameters.forEach((param: any, index: number) => { + // Check if there is not the same number of Natspec @param tags compared + // to the number of params for the function in the AST node. + if (paramDoc[index] === undefined) return; + const paramName = param.name; const paramType = param.typeName.name; @@ -241,8 +249,12 @@ async function generateDocumentation(hre: HardhatRuntimeEnvironment): Promise text.match(/@return.*/)); // custom errors and events do not have return parameters - if (returnDoc.length !== 0 && docEntry !== 'errors' && docEntry !== 'events') { + if (returnDoc.length > 0 && docEntry !== 'errors' && docEntry !== 'events') { astNode.returnParameters.parameters.forEach((returnParam: any, index: number) => { + // Check if there is not the same number of Natspec @return tags compared + // to the number of return params for the function in the AST node. + if (returnParam[index] === undefined) return; + const returnVariableName = returnParam.name === '' ? `_${index}` : returnParam.name; const returnParamType = returnParam.typeName.name; From cbfee1b9e7c04639790e3c5522297b227ded5584 Mon Sep 17 00:00:00 2001 From: CJ42 Date: Fri, 14 Jul 2023 16:09:51 +0100 Subject: [PATCH 3/3] fix: parsing and style for internal functions --- src/index.ts | 42 ++++++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/src/index.ts b/src/index.ts index 09b9c7b..bc492ab 100644 --- a/src/index.ts +++ b/src/index.ts @@ -71,14 +71,14 @@ async function generateDocumentation(hre: HardhatRuntimeEnvironment): Promise { // Check if there is not the same number of Natspec @return tags compared // to the number of return params for the function in the AST node. - if (returnParam[index] === undefined) return; + if (returnDoc[index] === undefined) return; const returnVariableName = returnParam.name === '' ? `_${index}` : returnParam.name; - const returnParamType = returnParam.typeName.name; + const returnParamType = returnParam.typeDescriptions.typeString; doc[docEntry][functionName].outputs[returnVariableName] = { type: returnParamType, @@ -393,22 +393,28 @@ async function generateDocumentation(hre: HardhatRuntimeEnvironment): Promise param.typeName.name) + .map((param: any) => param.typeDescriptions.typeString) .join(',')})`; let internalFunctionCode = `function ${functionName}(${params - .map((param: any) => `${param.typeName.name} ${param.name}`) - .join(',')}) internal`; + .map((param: any) => `${param.typeDescriptions.typeString} ${param.name}`) + .join(', ')}) internal`; internalFunctionCode += ` ${stateMutability}`; if (returnParams.length > 0) { internalFunctionCode += ` returns (${returnParams - .map((returnParam: any) => `${returnParam.typeName.name} ${returnParam.name}`) - .join(',')})`; - } + .map((returnParam: any) => { + let returnStatement = `${returnParam.typeDescriptions.typeString}`; + + if (returnParam.name !== '') { + returnStatement += ` ${returnParam.name}`; + } - internalFunctionCode += ';'; + return returnStatement; + }) + .join(', ')})`; + } if (!doc.internalMethods) { doc.internalMethods = {};