diff --git a/package.json b/package.json index 874005e..62a81b3 100644 --- a/package.json +++ b/package.json @@ -27,6 +27,7 @@ "devDependencies": { "@commitlint/cli": "17.6.5", "@commitlint/config-conventional": "17.6.5", + "@faker-js/faker": "8.3.1", "@types/jest": "29.5.11", "@types/node": "20.10.7", "husky": "8.0.3", diff --git a/sample-data/ParserTest.sol b/sample-data/ParserTest.sol new file mode 100644 index 0000000..d33084c --- /dev/null +++ b/sample-data/ParserTest.sol @@ -0,0 +1,183 @@ +// SPDX-License-Identifier: MIT +pragma solidity =0.8.19; + +// forgefmt: disable-start +// This file is used for testing the parser + +interface IParserTest { + /// @notice Thrown whenever something goes wrong + error SimpleError(uint256 _param1, uint256 _param2); + + /// @notice Emitted whenever something happens + event SimpleEvent(uint256 _param1, uint256 _param2); + + /// @notice The enum description + enum SimpleEnum { + A, + B, + C + } + + /// @notice View function with no parameters + /// @dev Natspec for the return value is missing + /// @return The returned value + function viewFunctionNoParams() external view returns (uint256); + + /** + * @notice A function with different style of natspec + * @param _param1 The first parameter + * @param _param2 The second parameter + * @return The returned value + */ + function viewFunctionWithParams(uint256 _param1, uint256 _param2) external view returns (uint256); + + /// @notice A state variable + /// @return Some value + function someVariable() external view returns (uint256); + + /// @notice A struct holding 2 variables of type uint256 + /// @member a The first variable + /// @member b The second variable + /// @dev This is definitely a struct + struct SimplestStruct { + uint256 a; + uint256 b; + } + + /// @notice A constant of type uint256 + function SOME_CONSTANT() external view returns (uint256 _returned); +} + +/// @notice A contract with correct natspec +contract ParserTest is IParserTest { + /// @inheritdoc IParserTest + uint256 public someVariable; + + /// @inheritdoc IParserTest + uint256 public constant SOME_CONSTANT = 123; + + /// @notice The constructor + /// @param _struct The struct parameter + constructor(SimplestStruct memory _struct) { + someVariable = _struct.a + _struct.b; + } + + /// @notice The description of the modifier + /// @param _param1 The only parameter + modifier someModifier(bool _param1) { + _; + } + + // TODO: Fallback and receive functions + // fallback() {} + // receive () {} + + /// @inheritdoc IParserTest + /// @dev Dev comment for the function + function viewFunctionNoParams() external pure returns (uint256){ + return 1; + } + + /// @inheritdoc IParserTest + function viewFunctionWithParams(uint256 _param1, uint256 _param2) external pure returns (uint256) { + return _param1 + _param2; + } + + /// @notice Some private stuff + /// @dev Dev comment for the private function + /// @param _paramName The parameter name + /// @return _returned The returned value + function _viewPrivate(uint256 _paramName) private pure returns (uint256 _returned) { + return 1; + } + + /// @notice Some internal stuff + /// @dev Dev comment for the internal function + /// @param _paramName The parameter name + /// @return _returned The returned value + function _viewInternal(uint256 _paramName) internal pure returns (uint256 _returned) { + return 1; + } + + /// @notice Some internal stuff + /// Separate line + /// Third one + function _viewMultiline() internal pure { + } + + /// @notice Some internal stuff + /// @notice Separate line + function _viewDuplicateTag() internal pure { + } +} + +// This is a contract with invalid / missing natspec +contract ParserTestFunny is IParserTest { + // no natspec, just a comment + struct SimpleStruct { + /// @notice The first variable + uint256 a; + /// @notice The first variable + uint256 b; + } + + modifier someModifier() { + _; + } + + /// @inheritdoc IParserTest + /// @dev Providing context + uint256 public someVariable; + + // @inheritdoc IParserTest + uint256 public constant SOME_CONSTANT = 123; + + /// @inheritdoc IParserTest + function viewFunctionNoParams() external view returns (uint256){ + return 1; + } + + // Forgot there is @inheritdoc and @notice + function viewFunctionWithParams(uint256 _param1, uint256 _param2) external view returns (uint256) { + return _param1 + _param2; + } + + // @notice Some internal stuff + function _viewInternal() internal view returns (uint256) { + return 1; + } + + /** + * + * + * + * I met Obama once + * She's cool + */ + + /// @notice Some private stuff + /// @param _paramName The parameter name + /// @return _returned The returned value + function _viewPrivate(uint256 _paramName) private pure returns (uint256 _returned) { + return 1; + } + + // @notice Forgot one slash and it's not natspec anymore + //// @dev Too many slashes is fine though + //// @return _returned The returned value + function _viewInternal(uint256 _paramName) internal pure returns (uint256 _returned) { + return 1; + } + + /**** @notice Some text + ** */ + function _viewBlockLinterFail() internal pure { + } + + /// @notice Linter fail + /// @dev What have I done + function _viewLinterFail() internal pure { + + } +} +// forgefmt: disable-end diff --git a/src/main.ts b/src/main.ts index a017019..625e1bd 100644 --- a/src/main.ts +++ b/src/main.ts @@ -3,7 +3,7 @@ import yargs from 'yargs'; import { hideBin } from 'yargs/helpers'; import { globSync } from 'fast-glob'; import { getProjectCompiledSources } from './utils'; -import { processSources } from './processor'; +import { Processor } from './processor'; import { getConfig } from './config'; (async () => { @@ -14,7 +14,8 @@ import { getConfig } from './config'; const sourceUnits = await getProjectCompiledSources(config.root, config.include, excludedPaths); if (!sourceUnits.length) return console.error('No solidity files found in the specified directory'); - const warnings = await processSources(sourceUnits, config); + const processor = new Processor(config); + const warnings = processor.processSources(sourceUnits); warnings.forEach(({ location, messages }) => { console.warn(location); diff --git a/src/parser.ts b/src/parser.ts deleted file mode 100644 index 554c96d..0000000 --- a/src/parser.ts +++ /dev/null @@ -1,48 +0,0 @@ -import { Natspec, NatspecDefinition } from './types/natspec'; -import { NodeToProcess } from './types/solc-typed-ast'; - -export function parseNodeNatspec(node: NodeToProcess): Natspec { - if (!node.documentation) { - return { tags: [], params: [], returns: [] }; - } - - const docText: string = typeof node.documentation === 'string' ? node.documentation : node.documentation.text; - - let currentTag: NatspecDefinition | null = null; - const result: Natspec = { - tags: [], - params: [], - returns: [], - }; - - docText.split('\n').forEach((line) => { - const tagTypeMatch = line.match(/^\s*@(\w+)/); - if (tagTypeMatch) { - const tagName = tagTypeMatch[1]; - - if (tagName === 'inheritdoc') { - const tagMatch = line.match(/^\s*@(\w+) (.*)$/); - if (tagMatch) { - currentTag = null; - result.inheritdoc = { content: tagMatch[2] }; - } - } else if (tagName === 'param' || tagName === 'return') { - const tagMatch = line.match(/^\s*@(\w+) *(\w+) (.*)$/); - if (tagMatch) { - currentTag = { name: tagMatch[2], content: tagMatch[3].trim() }; - result[tagName === 'param' ? 'params' : 'returns'].push(currentTag); - } - } else { - const tagMatch = line.match(/^\s*@(\w+) *(.*)$/); - if (tagMatch) { - currentTag = { name: tagName, content: tagMatch[2] }; - result.tags.push(currentTag); - } - } - } else if (currentTag) { - currentTag.content += '\n' + line; - } - }); - - return result; -} diff --git a/src/processor.ts b/src/processor.ts index 76ad405..cf7c4b2 100644 --- a/src/processor.ts +++ b/src/processor.ts @@ -1,63 +1,73 @@ -import { parseNodeNatspec } from './parser'; -import { validate } from './validator'; -import { SourceUnit, FunctionDefinition } from 'solc-typed-ast'; import fs from 'fs'; import { InternalConfig } from './types/config'; +import { Validator } from './validator'; +import { SourceUnit, FunctionDefinition, ContractDefinition } from 'solc-typed-ast'; +import { NodeToProcess } from './types/solc-typed-ast.d'; +import { parseNodeNatspec } from './utils'; interface IWarning { location: string; messages: string[]; } -export async function processSources(sourceUnits: SourceUnit[], config: InternalConfig): Promise { - let warnings: IWarning[] = []; - - sourceUnits.forEach((sourceUnit) => { - sourceUnit.vContracts.forEach((contract) => { - [ - ...contract.vEnums, - ...contract.vErrors, - ...contract.vEvents, - ...contract.vFunctions, - ...contract.vModifiers, - ...contract.vStateVariables, - ...contract.vStructs, - ].forEach((node) => { - if (!node) return; - - const nodeNatspec = parseNodeNatspec(node); - const validationMessages = validate(node, nodeNatspec, config); - - // the constructor function definition does not have a name, but it has kind: 'constructor' - const nodeName = node instanceof FunctionDefinition ? node.name || node.kind : node.name; - const sourceCode = fs.readFileSync(sourceUnit.absolutePath, 'utf8'); - const line = lineNumber(nodeName as string, sourceCode); - - if (validationMessages.length) { - warnings.push({ - location: `${sourceUnit.absolutePath}:${line}\n${contract.name}:${nodeName}`, - messages: validationMessages, - }); - } - }); - }); - }); - - return warnings; -} +export class Processor { + config: InternalConfig; + validator: Validator; -function lineNumberByIndex(index: number, string: string): Number { - let line = 0; - let match; - let re = /(^)[\S\s]/gm; + constructor(config: InternalConfig) { + this.config = config; + this.validator = new Validator(config); + } - while ((match = re.exec(string))) { - if (match.index > index) break; - line++; + processSources(sourceUnits: SourceUnit[]): IWarning[] { + return sourceUnits.flatMap((sourceUnit) => + sourceUnit.vContracts.flatMap((contract) => + this.selectEligibleNodes(contract) + .map((node) => this.validateNodeNatspec(sourceUnit, node, contract)) + .filter((warning) => warning.messages.length) + ) + ); + } + + selectEligibleNodes(contract: ContractDefinition): NodeToProcess[] { + return [ + ...contract.vEnums, + ...contract.vErrors, + ...contract.vEvents, + ...contract.vFunctions, + ...contract.vModifiers, + ...contract.vStateVariables, + ...contract.vStructs, + ]; + } + + validateNatspec(node: NodeToProcess): string[] { + if (!node) return []; + const nodeNatspec = parseNodeNatspec(node); + return this.validator.validate(node, nodeNatspec); } - return line; -} -function lineNumber(needle: string, haystack: string): Number { - return lineNumberByIndex(haystack.indexOf(needle), haystack); + validateNodeNatspec(sourceUnit: SourceUnit, node: NodeToProcess, contract: ContractDefinition): IWarning { + const validationMessages: string[] = this.validateNatspec(node); + + if (validationMessages.length) { + return { location: this.formatLocation(node, sourceUnit, contract), messages: validationMessages }; + } else { + return { location: '', messages: [] }; + } + } + + formatLocation(node: NodeToProcess, sourceUnit: SourceUnit, contract: ContractDefinition): string { + // the constructor function definition does not have a name, but it has kind: 'constructor' + const nodeName = node instanceof FunctionDefinition ? node.name || node.kind : node.name; + const line = this.getLineNumberFromSrc(sourceUnit.absolutePath, node.src); + return `${sourceUnit.absolutePath}:${line}\n${contract.name}:${nodeName}`; + } + + private getLineNumberFromSrc(filePath: string, src: string) { + const [start] = src.split(':').map(Number); + const fileContent = fs.readFileSync(filePath, 'utf8'); + const lines = fileContent.substring(0, start).split('\n'); + return lines.length; // Line number + } } diff --git a/src/types/config.t.ts b/src/types/config.t.ts new file mode 100644 index 0000000..7f12f79 --- /dev/null +++ b/src/types/config.t.ts @@ -0,0 +1,7 @@ +export interface Config { + root: string; + contracts: string; + enforceInheritdoc: boolean; + constructorNatspec: boolean; + ignore: string[]; +} diff --git a/src/utils.ts b/src/utils.ts index 170408f..e5044b4 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,6 +1,8 @@ import fs from 'fs/promises'; import path from 'path'; import { ASTKind, ASTReader, SourceUnit, compileSol } from 'solc-typed-ast'; +import { Natspec, NatspecDefinition } from './types/natspec.d'; +import { NodeToProcess } from './types/solc-typed-ast.d'; export async function getSolidityFiles(dir: string): Promise { let files = await fs.readdir(dir, { withFileTypes: true }); @@ -18,21 +20,6 @@ export async function getSolidityFiles(dir: string): Promise { return solidityFiles; } -export async function getRemappings(rootPath: string): Promise { - try { - const filePath = path.join(rootPath, 'remappings.txt'); - const fileContent = await fs.readFile(filePath, 'utf8'); - - return fileContent - .split('\n') - .map((line) => line.trim()) - .filter((line) => line.length) - .map((line) => (line.slice(-1) === '/' ? line : line + '/')); - } catch (e) { - return []; - } -} - export async function getProjectCompiledSources(rootPath: string, includedPath: string, excludedPaths: string[]): Promise { // Fetch Solidity files from the specified directory const solidityFiles: string[] = await getSolidityFiles(includedPath); @@ -67,3 +54,64 @@ export function isFileInDirectory(directory: string, filePath: string): boolean // Check if the file path starts with the directory path return absoluteFilePath.startsWith(absoluteDirectoryPath); } + +export async function getRemappings(rootPath: string): Promise { + try { + const filePath = path.join(rootPath, 'remappings.txt'); + const fileContent = await fs.readFile(filePath, 'utf8'); + + return fileContent + .split('\n') + .map((line) => line.trim()) + .filter((line) => line.length) + .map((line) => (line.slice(-1) === '/' ? line : line + '/')); + } catch (e) { + return []; + } +} + +export function parseNodeNatspec(node: NodeToProcess): Natspec { + if (!node.documentation) { + return { tags: [], params: [], returns: [] }; + } + + let currentTag: NatspecDefinition | null = null; + const result: Natspec = { + tags: [], + params: [], + returns: [], + }; + + const docText: string = typeof node.documentation === 'string' ? node.documentation : node.documentation.text; + + docText.split('\n').forEach((line) => { + const tagTypeMatch = line.match(/^\s*@(\w+)/); + if (tagTypeMatch) { + const tagName = tagTypeMatch[1]; + + if (tagName === 'inheritdoc') { + const tagMatch = line.match(/^\s*@(\w+) (.*)$/); + if (tagMatch) { + currentTag = null; + result.inheritdoc = { content: tagMatch[2] }; + } + } else if (tagName === 'param' || tagName === 'return') { + const tagMatch = line.match(/^\s*@(\w+) *(\w+) (.*)$/); + if (tagMatch) { + currentTag = { name: tagMatch[2], content: tagMatch[3].trim() }; + result[tagName === 'param' ? 'params' : 'returns'].push(currentTag); + } + } else { + const tagMatch = line.match(/^\s*@(\w+) *(.*)$/); + if (tagMatch) { + currentTag = { name: tagName, content: tagMatch[2] }; + result.tags.push(currentTag); + } + } + } else if (currentTag) { + currentTag.content += '\n' + line; + } + }); + + return result; +} diff --git a/src/validator.ts b/src/validator.ts index 1b54de4..c5cc02d 100644 --- a/src/validator.ts +++ b/src/validator.ts @@ -11,106 +11,100 @@ import { VariableDeclaration, } from 'solc-typed-ast'; -export function validate(node: NodeToProcess, natspec: Natspec, config: InternalConfig): string[] { - // There is inheritdoc, no other validation is needed - if (natspec.inheritdoc) return []; +export class Validator { + config: InternalConfig; - // Inheritdoc is enforced but not present, returning an error - if (config.enforceInheritdoc && requiresInheritdoc(node)) return [`@inheritdoc is missing`]; - - // Validate natspec for the constructor only if configured - if (node instanceof FunctionDefinition && node.kind === 'constructor') { - return config.constructorNatspec ? validateParameters(node, natspec) : []; + constructor(config: InternalConfig) { + this.config = config; } - // Inheritdoc is not enforced nor present, and there is no other documentation, returning error - if (!natspec.tags.length) return [`Natspec is missing`]; - - // Validate the completeness of the documentation - let alerts: string[] = []; - if (node instanceof EnumDefinition) { - // TODO: Process enums - } else if (node instanceof ErrorDefinition) { - alerts = [...alerts, ...validateParameters(node, natspec)]; - } else if (node instanceof EventDefinition) { - alerts = [...alerts, ...validateParameters(node, natspec)]; - } else if (node instanceof FunctionDefinition) { - alerts = [...alerts, ...validateParameters(node, natspec), ...validateReturnParameters(node, natspec)]; - } else if (node instanceof ModifierDefinition) { - alerts = [...alerts, ...validateParameters(node, natspec)]; - } else if (node instanceof StructDefinition) { - alerts = [...alerts, ...validateMembers(node, natspec)]; - } else if (node instanceof VariableDeclaration) { - // Only the presence of a notice is validated - } + validate(node: NodeToProcess, natspec: Natspec): string[] { + // There is inheritdoc, no other validation is needed + if (natspec.inheritdoc) return []; - return alerts; -} + // Inheritdoc is enforced but not present, returning an error + if (this.config.enforceInheritdoc && this.requiresInheritdoc(node)) return [`@inheritdoc is missing`]; -function validateParameters(node: ErrorDefinition | FunctionDefinition | ModifierDefinition, natspec: Natspec): string[] { - // Make sure all defined parameters have natspec - let alerts: string[] = []; + const natspecParams = natspec.params.map((p) => p.name); - let definedParameters = node.vParameters.vParameters.map((p) => p.name); - let natspecParameters = natspec.params.map((p) => p.name); - - for (let paramName of definedParameters) { - if (!natspecParameters.includes(paramName)) { - alerts.push(`@param ${paramName} is missing`); + // Validate natspec for the constructor only if configured + if (node instanceof FunctionDefinition && node.kind === 'constructor') { + return this.config.constructorNatspec ? this.validateParameters(node, natspecParams) : []; } - } - return alerts; -} - -function validateReturnParameters(node: FunctionDefinition, natspec: Natspec): string[] { - let alerts: string[] = []; - let functionReturns = node.vReturnParameters.vParameters.map((p) => p.name); - let natspecReturns = natspec.returns.map((p) => p.name); - - // Make sure all defined returns have natspec - for (let paramName of functionReturns) { - if (!natspecReturns.includes(paramName)) { - let message = paramName === '' ? '@return missing for unnamed return' : `@return ${paramName} is missing`; - alerts.push(message); + // Inheritdoc is not enforced nor present, and there is no other documentation, returning error + if (!natspec.tags.length) return [`Natspec is missing`]; + + // Validate the completeness of the documentation + let alerts: string[] = []; + + if (node instanceof EnumDefinition) { + // TODO: Process enums + } else if (node instanceof ErrorDefinition) { + alerts = [...alerts, ...this.validateParameters(node, natspecParams)]; + } else if (node instanceof EventDefinition) { + alerts = [...alerts, ...this.validateParameters(node, natspecParams)]; + } else if (node instanceof FunctionDefinition) { + const natspecReturns = natspec.returns.map((p) => p.name); + alerts = [...alerts, ...this.validateParameters(node, natspecParams), ...this.validateReturnParameters(node, natspecReturns)]; + } else if (node instanceof ModifierDefinition) { + alerts = [...alerts, ...this.validateParameters(node, natspecParams)]; + } else if (node instanceof StructDefinition) { + alerts = [...alerts, ...this.validateMembers(node, natspecParams)]; + } else if (node instanceof VariableDeclaration) { + // Only the presence of a notice is validated } + + return alerts; } - // Make sure there is no natspec defined for non-existing returns - for (let paramName of natspecReturns) { - if (paramName && !functionReturns.includes(paramName)) { - alerts.push(`Missing named return for: @return ${paramName}`); - } + // All defined parameters should have natspec + private validateParameters(node: ErrorDefinition | FunctionDefinition | ModifierDefinition, natspecParams: (string | undefined)[]): string[] { + let definedParameters = node.vParameters.vParameters.map((p) => p.name); + return definedParameters.filter((p) => !natspecParams.includes(p)).map((p) => `@param ${p} is missing`); } - return alerts; -} + // All members of a struct should have natspec + private validateMembers(node: StructDefinition, natspecParams: (string | undefined)[]): string[] { + let members = node.vMembers.map((p) => p.name); + return members.filter((m) => !natspecParams.includes(m)).map((m) => `@param ${m} is missing`); + } -function validateMembers(node: StructDefinition, natspec: Natspec): string[] { - let alerts: string[] = []; - let members = node.vMembers.map((p) => p.name); - let natspecMembers = natspec.params.map((p) => p.name); + // All returned parameters should have natspec + private validateReturnParameters(node: FunctionDefinition, natspecReturns: (string | undefined)[]): string[] { + let alerts: string[] = []; + let functionReturns = node.vReturnParameters.vParameters.map((p) => p.name); + + // Make sure all defined returns have natspec + for (let paramName of functionReturns) { + if (!natspecReturns.includes(paramName)) { + let message = paramName === '' ? '@return missing for unnamed return' : `@return ${paramName} is missing`; + alerts.push(message); + } + } - for (let memberName of members) { - if (!natspecMembers.includes(memberName)) { - alerts.push(`@param ${memberName} is missing`); + // Make sure there is no natspec defined for non-existing returns + for (let paramName of natspecReturns) { + if (paramName && !functionReturns.includes(paramName)) { + alerts.push(`Missing named return for: @return ${paramName}`); + } } - } - return alerts; -} + return alerts; + } -function requiresInheritdoc(node: NodeToProcess): boolean { - let _requiresInheritdoc: boolean = false; + private requiresInheritdoc(node: NodeToProcess): boolean { + let _requiresInheritdoc: boolean = false; - // External or public function - _requiresInheritdoc ||= node instanceof FunctionDefinition && (node.visibility === 'external' || node.visibility === 'public'); + // External or public function + _requiresInheritdoc ||= node instanceof FunctionDefinition && (node.visibility === 'external' || node.visibility === 'public'); - // Internal virtual function - _requiresInheritdoc ||= node instanceof FunctionDefinition && node.visibility === 'internal' && node.virtual; + // Internal virtual function + _requiresInheritdoc ||= node instanceof FunctionDefinition && node.visibility === 'internal' && node.virtual; - // Public variable - _requiresInheritdoc ||= node instanceof VariableDeclaration && node.visibility === 'public'; + // Public variable + _requiresInheritdoc ||= node instanceof VariableDeclaration && node.visibility === 'public'; - return _requiresInheritdoc; + return _requiresInheritdoc; + } } diff --git a/test/mocks.ts b/test/mocks.ts new file mode 100644 index 0000000..f3c85bf --- /dev/null +++ b/test/mocks.ts @@ -0,0 +1,34 @@ +import { Natspec } from '../src/types/natspec.d'; +import { SourceUnit, ContractDefinition, FunctionDefinition } from 'solc-typed-ast'; +import { NodeToProcess } from '../src/types/solc-typed-ast.d'; + +export function mockNatspec(mockNatspec: Partial): Natspec { + const natspec: Natspec = { + tags: mockNatspec.tags || [], + params: mockNatspec.params || [], + returns: mockNatspec.returns || [], + }; + + if (mockNatspec.inheritdoc) natspec.inheritdoc = mockNatspec.inheritdoc; + + return natspec; +} + +export function mockNodeToProcess(mockNodeToProcess: Partial): NodeToProcess { + return mockNodeToProcess as NodeToProcess; +} + +export function mockFunctionDefinition(mockFunctionDefinition: Partial): FunctionDefinition { + // This is a hack to trick `instanceof` + let functionDefinition: FunctionDefinition = Object.create(FunctionDefinition.prototype); + Object.assign(functionDefinition, mockFunctionDefinition); + return functionDefinition; +} + +export function mockSourceUnit(mockSourceUnit: Partial): SourceUnit { + return mockSourceUnit as SourceUnit; +} + +export function mockContractDefinition(mockContractDefinition: Partial): ContractDefinition { + return mockContractDefinition as ContractDefinition; +} diff --git a/test/parser.test.ts b/test/parser.test.ts index fd9c681..5cdbb8d 100644 --- a/test/parser.test.ts +++ b/test/parser.test.ts @@ -1,169 +1,424 @@ import { ContractDefinition } from 'solc-typed-ast'; -import { parseNodeNatspec } from '../src/parser'; -import { getFileCompiledSource } from '../src/utils'; +import { getFileCompiledSource } from './utils'; +import { parseNodeNatspec } from '../src/utils'; +import { mockNatspec } from './mocks'; -describe('parseNodeNatspec', () => { - describe('BasicSample.sol', () => { - let contract: ContractDefinition; +describe('Parser', () => { + let contract: ContractDefinition; + describe('Contract', () => { beforeAll(async () => { - const compileResult = await getFileCompiledSource('sample-data/BasicSample.sol'); - contract = compileResult.vContracts[0]; + const compileResult = await getFileCompiledSource('sample-data/ParserTest.sol'); + contract = compileResult.vContracts.find(({ name }) => name === 'ParserTest')!; }); - it('should parse struct', async () => { - const structNode = contract.vStructs.find(({ name }) => name === 'TestStruct')!; - const result = parseNodeNatspec(structNode); - - expect(result).toEqual({ - tags: [ - { - name: 'notice', - content: 'Some notice of the struct', - }, - ], - params: [], - returns: [], - }); + it('should parse the inheritdoc tag', async () => { + const node = contract.vFunctions.find(({ name }) => name === 'viewFunctionNoParams')!; + const result = parseNodeNatspec(node); + + expect(result).toEqual( + mockNatspec({ + inheritdoc: { content: 'IParserTest' }, + tags: [ + { + name: 'dev', + content: 'Dev comment for the function', + }, + ], + }) + ); }); it('should parse constant', async () => { - const emptyStringNode = contract.vStateVariables.find(({ name }) => name === '_EMPTY_STRING')!; - const result = parseNodeNatspec(emptyStringNode); - - expect(result).toEqual({ - tags: [ - { - name: 'notice', - content: 'Empty string for revert checks', - }, - { - name: 'dev', - content: `result of doing keccak256(bytes(''))`, + const node = contract.vStateVariables.find(({ name }) => name === 'SOME_CONSTANT')!; + const result = parseNodeNatspec(node); + + expect(result).toEqual( + mockNatspec({ + inheritdoc: { + content: 'IParserTest', }, - ], - params: [], - returns: [], - }); + }) + ); }); - it('should parse a fully natspeced external function', async () => { - const functionNode = contract.vFunctions.find(({ name }) => name === 'externalSimple')!; - const result = parseNodeNatspec(functionNode); + it('should parse variable', async () => { + const node = contract.vStateVariables.find(({ name }) => name === 'someVariable')!; + const result = parseNodeNatspec(node); - expect(result).toEqual({ - tags: [ - { - name: 'notice', - content: 'External function that returns a bool', - }, - { - name: 'dev', - content: 'A dev comment', - }, - ], - params: [ - { - name: '_magicNumber', - content: 'A parameter description', - }, - { - name: '_name', - content: 'Another parameter description', + expect(result).toEqual( + mockNatspec({ + inheritdoc: { + content: 'IParserTest', }, - ], - returns: [ - { - name: '_isMagic', - content: 'Some return data', - }, - ], - }); + }) + ); }); - it('should parse a fully natspeced private function', async () => { - const functionNode = contract.vFunctions.find(({ name }) => name === 'privateSimple')!; - const result = parseNodeNatspec(functionNode); + it('should parse modifier', async () => { + const node = contract.vModifiers.find(({ name }) => name === 'someModifier')!; + const result = parseNodeNatspec(node); - expect(result).toEqual({ - tags: [ - { - name: 'notice', - content: 'Private test function', - }, - ], - params: [ - { - name: '_magicNumber', - content: 'A parameter description', + expect(result).toEqual( + mockNatspec({ + tags: [ + { + name: 'notice', + content: 'The description of the modifier', + }, + ], + params: [ + { + name: '_param1', + content: 'The only parameter', + }, + ], + }) + ); + }); + + it('should parse external function', async () => { + const node = contract.vFunctions.find(({ name }) => name === 'viewFunctionNoParams')!; + const result = parseNodeNatspec(node); + + expect(result).toEqual( + mockNatspec({ + inheritdoc: { + content: 'IParserTest', }, - ], - returns: [], - }); + tags: [ + { + name: 'dev', + content: 'Dev comment for the function', + }, + ], + }) + ); + }); + + it('should parse private function', async () => { + const node = contract.vFunctions.find(({ name }) => name === '_viewPrivate')!; + const result = parseNodeNatspec(node); + + expect(result).toEqual( + mockNatspec({ + tags: [ + { + name: 'notice', + content: 'Some private stuff', + }, + { + name: 'dev', + content: 'Dev comment for the private function', + }, + ], + params: [ + { + name: '_paramName', + content: 'The parameter name', + }, + ], + returns: [ + { + name: '_returned', + content: 'The returned value', + }, + ], + }) + ); }); it('should parse multiline descriptions', async () => { - const functionNode = contract.vFunctions.find(({ name }) => name === 'multiline')!; - const result = parseNodeNatspec(functionNode); - - expect(result).toEqual({ - tags: [ - { - name: 'notice', - content: 'Private test function\n with multiple\n lines', - }, - ], - params: [], - returns: [], - }); + const node = contract.vFunctions.find(({ name }) => name === '_viewMultiline')!; + const result = parseNodeNatspec(node); + + expect(result).toEqual( + mockNatspec({ + tags: [ + { + name: 'notice', + content: 'Some internal stuff\n Separate line\n Third one', + }, + ], + }) + ); }); it('should parse multiple of the same tag', async () => { - const functionNode = contract.vFunctions.find(({ name }) => name === 'multitag')!; - const result = parseNodeNatspec(functionNode); - - expect(result).toEqual({ - tags: [ - { - name: 'notice', - content: 'Private test function', - }, - { - name: 'notice', - content: 'Another notice', - }, - ], - params: [], - returns: [], - }); + const node = contract.vFunctions.find(({ name }) => name === '_viewDuplicateTag')!; + const result = parseNodeNatspec(node); + + expect(result).toEqual( + mockNatspec({ + tags: [ + { + name: 'notice', + content: 'Some internal stuff', + }, + { + name: 'notice', + content: 'Separate line', + }, + ], + }) + ); }); }); - describe('InterfacedSample.sol', () => { - let contract: ContractDefinition; + describe('Interface', () => { + beforeAll(async () => { + const compileResult = await getFileCompiledSource('sample-data/ParserTest.sol'); + contract = compileResult.vContracts.find(({ name }) => name === 'IParserTest')!; + }); + + it('should parse error', async () => { + const node = contract.vErrors.find(({ name }) => name === 'SimpleError')!; + const result = parseNodeNatspec(node); + + expect(result).toEqual( + mockNatspec({ + tags: [ + { + name: 'notice', + content: 'Thrown whenever something goes wrong', + }, + ], + }) + ); + }); + + it('should parse event', async () => { + const node = contract.vEvents.find(({ name }) => name === 'SimpleEvent')!; + const result = parseNodeNatspec(node); + + expect(result).toEqual( + mockNatspec({ + tags: [ + { + name: 'notice', + content: 'Emitted whenever something happens', + }, + ], + }) + ); + }); + + it('should parse struct', async () => { + const node = contract.vStructs.find(({ name }) => name === 'SimplestStruct')!; + const result = parseNodeNatspec(node); + + expect(result).toEqual( + mockNatspec({ + tags: [ + { + name: 'notice', + content: 'A struct holding 2 variables of type uint256', + }, + { + content: 'a The first variable', + name: 'member', + }, + { + content: 'b The second variable', + name: 'member', + }, + { + content: 'This is definitely a struct', + name: 'dev', + }, + ], + }) + ); + }); + // TODO: Parse natspec for enums + // it('should parse enum', async () => { + // const node = contract.vEnums.find(({ name }) => name === 'SimpleEnum')!; + // const result = parseNodeNatspec(node); + + // expect(result).toEqual(mockNatspec({ + // tags: [], + // })); + // }); + + it('should parse external function without parameters', async () => { + const node = contract.vFunctions.find(({ name }) => name === 'viewFunctionNoParams')!; + const result = parseNodeNatspec(node); + + expect(result).toEqual( + mockNatspec({ + tags: [ + { + name: 'notice', + content: 'View function with no parameters', + }, + { + name: 'dev', + content: 'Natspec for the return value is missing', + }, + ], + returns: [ + { + name: 'The', + content: 'returned value', + }, + ], + }) + ); + }); + + it('should parse external function with parameters', async () => { + const node = contract.vFunctions.find(({ name }) => name === 'viewFunctionWithParams')!; + const result = parseNodeNatspec(node); + + expect(result).toEqual( + mockNatspec({ + tags: [ + { + name: 'notice', + content: 'A function with different style of natspec', + }, + ], + params: [ + { + name: '_param1', + content: 'The first parameter', + }, + { + name: '_param2', + content: 'The second parameter', + }, + ], + returns: [ + { + name: 'The', + content: 'returned value', + }, + ], + }) + ); + }); + }); + + describe('Contract with invalid natspec', () => { beforeAll(async () => { - const compileResult = await getFileCompiledSource('sample-data/InterfacedSample.sol'); - contract = compileResult.vContracts[1]; + const compileResult = await getFileCompiledSource('sample-data/ParserTest.sol'); + contract = compileResult.vContracts.find(({ name }) => name === 'ParserTestFunny')!; }); - it('should parse the inheritdoc tag', async () => { - const functionNode = contract.vFunctions.find(({ name }) => name === 'greet')!; - const result = parseNodeNatspec(functionNode); - - expect(result).toEqual({ - inheritdoc: { - content: 'IInterfacedSample', - }, - tags: [ - { - name: 'dev', - content: 'some dev thingy', + it('should parse struct', async () => { + const node = contract.vStructs.find(({ name }) => name === 'SimpleStruct')!; + const result = parseNodeNatspec(node); + + expect(result).toEqual( + mockNatspec({ + tags: [], + }) + ); + }); + + it('should parse inheritdoc + natspec', async () => { + const node = contract.vStateVariables.find(({ name }) => name === 'someVariable')!; + const result = parseNodeNatspec(node); + + expect(result).toEqual( + mockNatspec({ + inheritdoc: { + content: 'IParserTest', }, - ], - params: [], - returns: [], - }); + tags: [ + { + name: 'dev', + content: 'Providing context', + }, + ], + }) + ); + }); + + it('should not parse the inheritdoc tag with just 2 slashes', async () => { + const node = contract.vStateVariables.find(({ name }) => name === 'SOME_CONSTANT')!; + const result = parseNodeNatspec(node); + + expect(result).toEqual(mockNatspec({})); + }); + + it('should not parse regular comments as natspec', async () => { + const node = contract.vFunctions.find(({ name }) => name === 'viewFunctionWithParams')!; + const result = parseNodeNatspec(node); + + expect(result).toEqual(mockNatspec({})); + }); + + it('should parse natspec with multiple spaces', async () => { + const node = contract.vFunctions.find(({ name }) => name === '_viewPrivate')!; + const result = parseNodeNatspec(node); + + expect(result).toEqual( + mockNatspec({ + tags: [ + { + name: 'notice', + content: 'Some private stuff', + }, + ], + params: [ + { + name: '_paramName', + content: 'The parameter name', + }, + ], + returns: [ + { + name: '_returned', + content: 'The returned value', + }, + ], + }) + ); + }); + + it('should not parse natspec with invalid number of slashes', async () => { + const node = contract.vFunctions.find(({ name }) => name === '_viewInternal')!; + const result = parseNodeNatspec(node); + + expect(result).toEqual(mockNatspec({})); + }); + + it('should parse block natspec with invalid formatting', async () => { + const node = contract.vFunctions.find(({ name }) => name === '_viewBlockLinterFail')!; + const result = parseNodeNatspec(node); + + expect(result).toEqual( + mockNatspec({ + tags: [ + { + name: 'notice', + content: 'Some text', + }, + ], + }) + ); + }); + + it('should parse block natspec with invalid formatting', async () => { + const node = contract.vFunctions.find(({ name }) => name === '_viewLinterFail')!; + const result = parseNodeNatspec(node); + + expect(result).toEqual( + mockNatspec({ + tags: [ + { + name: 'notice', + content: 'Linter fail', + }, + { + name: 'dev', + content: 'What have I done', + }, + ], + }) + ); }); }); }); diff --git a/test/processor.test.ts b/test/processor.test.ts index 34bf110..680d0ae 100644 --- a/test/processor.test.ts +++ b/test/processor.test.ts @@ -1,8 +1,9 @@ -import { processSources } from '../src/processor'; -import { getFileCompiledSource } from '../src/utils'; import { InternalConfig } from '../src/types/config'; +import { ContractDefinition, FunctionDefinition, UserDefinedType, UsingForDirective } from 'solc-typed-ast'; +import { getFileCompiledSource } from './utils'; +import { Processor } from '../src/processor'; -describe('processSources', () => { +describe('Processor', () => { const config: InternalConfig = { root: '.', include: './sample-data', @@ -11,16 +12,116 @@ describe('processSources', () => { constructorNatspec: false, }; - describe('LibrarySample.sol', () => { - it('should return warnings only for the library method empty natspec', async () => { - const source = await getFileCompiledSource('sample-data/LibrarySample.sol'); - const warnings = await processSources([source], config); - expect(warnings).toEqual([ - { - location: 'sample-data/LibrarySample.sol:5\nStringUtils:nothing', - messages: ['Natspec is missing'], - }, - ]); + const processor: Processor = new Processor(config); + + // TODO: Fix these tests + // describe('formatLocation', () => { + // const absolutePath = faker.system.filePath(); + // const contractName = faker.lorem.word(); + // const nodeName = faker.lorem.word(); + + // const sourceUnit = mockSourceUnit({ + // absolutePath: absolutePath, + // }); + + // const contract = mockContractDefinition({ + // name: contractName, + // }); + + // it('should format the location of the node', () => { + // const node = mockNodeToProcess({ + // name: nodeName, + // }); + + // expect(processor.formatLocation(node, sourceUnit, contract)).toEqual(`${absolutePath}\n${contractName}:${nodeName}`); + // }); + + // it('should format the location of a constructor', () => { + // const node = mockFunctionDefinition({ + // kind: FunctionKind.Constructor, + // }); + + // expect(processor.formatLocation(node, sourceUnit, contract)).toEqual(`${absolutePath}\n${contractName}:constructor`); + // }); + // }); + + describe('selectEligibleNodes', () => { + let contract: ContractDefinition; + + beforeAll(async () => { + const compileResult = await getFileCompiledSource('sample-data/ParserTest.sol'); + contract = compileResult.vContracts.find(({ name }) => name === 'ParserTest')!; + }); + + it('should select enums', () => { + const eligibleNodes = processor.selectEligibleNodes(contract); + + contract.vEnums.forEach((enumNode) => { + expect(eligibleNodes.find(({ name }) => name === enumNode.name)).toBeDefined(); + }); + }); + + it('should select errors', () => { + const eligibleNodes = processor.selectEligibleNodes(contract); + + contract.vErrors.forEach((errorNode) => { + expect(eligibleNodes.find(({ name }) => name === errorNode.name)).toBeDefined(); + }); + }); + + it('should select events', () => { + const eligibleNodes = processor.selectEligibleNodes(contract); + + contract.vEvents.forEach((eventNode) => { + expect(eligibleNodes.find(({ name }) => name === eventNode.name)).toBeDefined(); + }); + }); + + it('should select functions', () => { + const eligibleNodes = processor.selectEligibleNodes(contract); + + contract.vFunctions.forEach((functionNode) => { + expect(eligibleNodes.find(({ name }) => name === functionNode.name)).toBeDefined(); + }); + }); + + it('should select modifiers', () => { + const eligibleNodes = processor.selectEligibleNodes(contract); + + contract.vModifiers.forEach((modifierNode) => { + expect(eligibleNodes.find(({ name }) => name === modifierNode.name)).toBeDefined(); + }); + }); + + it('should select state variables', () => { + const eligibleNodes = processor.selectEligibleNodes(contract); + + contract.vStateVariables.forEach((variableNode) => { + expect(eligibleNodes.find(({ name }) => name === variableNode.name)).toBeDefined(); + }); + }); + + it('should select structs', () => { + const eligibleNodes = processor.selectEligibleNodes(contract); + + contract.vStructs.forEach((structNode) => { + expect(eligibleNodes.find(({ name }) => name === structNode.name)).toBeDefined(); + }); + }); + + it('should not select using directives', () => { + const eligibleNodes = processor.selectEligibleNodes(contract); + expect(eligibleNodes.some((node) => node instanceof UsingForDirective)).toBeFalsy(); + }); + + it('should not select user defined value types', () => { + const eligibleNodes = processor.selectEligibleNodes(contract); + expect(eligibleNodes.some((node) => node instanceof UserDefinedType)).toBeFalsy(); + }); + + it('should select the constructor only once', () => { + const eligibleNodes = processor.selectEligibleNodes(contract); + expect(eligibleNodes.filter((node) => node instanceof FunctionDefinition && node.isConstructor).length).toEqual(1); }); }); }); diff --git a/test/utils.ts b/test/utils.ts new file mode 100644 index 0000000..8bd6662 --- /dev/null +++ b/test/utils.ts @@ -0,0 +1,6 @@ +import { ASTKind, ASTReader, SourceUnit, compileSol } from 'solc-typed-ast'; + +export async function getFileCompiledSource(filePath: string): Promise { + const compiledFile = await compileSol(filePath, 'auto'); + return new ASTReader().read(compiledFile.data, ASTKind.Any, compiledFile.files)[0]; +} diff --git a/test/validator.test.ts b/test/validator.test.ts index 5531bbe..024b157 100644 --- a/test/validator.test.ts +++ b/test/validator.test.ts @@ -1,10 +1,10 @@ -import { validate } from '../src/validator'; -import { getFileCompiledSource } from '../src/utils'; -import { NodeToProcess } from '../src/types/solc-typed-ast'; +import { Validator } from '../src/validator'; +import { getFileCompiledSource } from './utils'; +import { NodeToProcess } from '../src/types/solc-typed-ast.d'; import { ContractDefinition } from 'solc-typed-ast'; import { InternalConfig } from '../src/types/config'; -describe('validator function', () => { +describe('Validator', () => { let contract: ContractDefinition; let node: NodeToProcess; @@ -16,6 +16,8 @@ describe('validator function', () => { constructorNatspec: false, }; + const validator: Validator = new Validator(config); + beforeAll(async () => { const compileResult = await getFileCompiledSource('sample-data/BasicSample.sol'); contract = compileResult.vContracts[0]; @@ -56,7 +58,7 @@ describe('validator function', () => { }; it('should validate proper natspec', () => { - const result = validate(node, natspec, config); + const result = validator.validate(node, natspec); expect(result).toEqual([]); }); @@ -83,7 +85,7 @@ describe('validator function', () => { ], }; - const result = validate(node, natspec, config); + const result = validator.validate(node, natspec); expect(result).toContainEqual(`@param ${paramName} is missing`); }); @@ -113,7 +115,7 @@ describe('validator function', () => { returns: [], }; - const result = validate(node, natspec, config); + const result = validator.validate(node, natspec); expect(result).toContainEqual(`@return ${paramName} is missing`); }); @@ -148,7 +150,7 @@ describe('validator function', () => { ], }; - const result = validate(node, natspec, config); + const result = validator.validate(node, natspec); expect(result).toContainEqual(`@return missing for unnamed return`); }); @@ -165,7 +167,7 @@ describe('validator function', () => { params: [], returns: [], }; - const result = validate(node, natspec, config); + const result = validator.validate(node, natspec); expect(result).toContainEqual(`Natspec is missing`); }); @@ -182,7 +184,7 @@ describe('validator function', () => { params: [], returns: [], }; - const result = validate(node, natspec, config); + const result = validator.validate(node, natspec); expect(result).toContainEqual(`@param ${paramName} is missing`); }); @@ -199,7 +201,7 @@ describe('validator function', () => { params: [], returns: [], }; - const result = validate(node, natspec, config); + const result = validator.validate(node, natspec); expect(result).toContainEqual(`@param ${paramName} is missing`); }); @@ -216,7 +218,7 @@ describe('validator function', () => { params: [], returns: [], }; - const result = validate(node, natspec, config); + const result = validator.validate(node, natspec); expect(result).toContainEqual(`@param ${paramName} is missing`); }); @@ -234,7 +236,7 @@ describe('validator function', () => { params: [], returns: [], }; - const result = validate(node, natspec, config); + const result = validator.validate(node, natspec); expect(result).toContainEqual(`@param ${paramName1} is missing`); expect(result).toContainEqual(`@param ${paramName2} is missing`); }); diff --git a/yarn.lock b/yarn.lock index add15f1..1edb98a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -465,6 +465,11 @@ dependencies: "@jridgewell/trace-mapping" "0.3.9" +"@faker-js/faker@8.3.1": + version "8.3.1" + resolved "https://registry.yarnpkg.com/@faker-js/faker/-/faker-8.3.1.tgz#7753df0cb88d7649becf984a96dd1bd0a26f43e3" + integrity sha512-FdgpFxY6V6rLZE9mmIBb9hM0xpfvQOSNOLnzolzKwsE1DH+gC7lEKV1p1IbR0lAYyvYd5a4u3qWJzowUkw1bIw== + "@istanbuljs/load-nyc-config@^1.0.0": version "1.1.0" resolved "https://registry.yarnpkg.com/@istanbuljs/load-nyc-config/-/load-nyc-config-1.1.0.tgz#fd3db1d59ecf7cf121e80650bb86712f9b55eced"