Skip to content

Commit

Permalink
feat: added inheritdoc support
Browse files Browse the repository at this point in the history
Co-authored-by: Gas <[email protected]>
  • Loading branch information
0xGorilla and gas1cent committed Jan 9, 2024
1 parent de26de2 commit 16cb8c4
Show file tree
Hide file tree
Showing 9 changed files with 195 additions and 189 deletions.
23 changes: 23 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
name: Node.js CI

on: push

jobs:
build:
runs-on: ubuntu-latest

strategy:
matrix:
node-version: [14.x, 16.x, 18.x]

steps:
- uses: actions/checkout@v3

- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v3
with:
node-version: ${{ matrix.node-version }}

- run: yarn install

- run: yarn test
2 changes: 1 addition & 1 deletion sample-data/sample.sol → sample-data/BasicSample.sol
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: MIT
pragma solidity =0.8.19;

contract Sample {
contract BasicSample {
/**
* @notice Empty string for revert checks
* @dev result of doing keccak256(bytes(''))
Expand Down
18 changes: 18 additions & 0 deletions sample-data/InterfacedSample.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// SPDX-License-Identifier: MIT
pragma solidity =0.8.19;

interface IInterfacedSample {
/**
* @notice Greets the caller
*
* @return _greeting The greeting
* @return _balance Current token balance of the caller
*/
function greet() external view returns (string memory _greeting, uint256 _balance);
}

contract InterfacedSample is IInterfacedSample {
/// @inheritdoc IInterfacedSample
/// @dev some dev thingy
function greet() external view returns (string memory _greeting, uint256 _balance) {}
}
29 changes: 17 additions & 12 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,26 @@ import { getRemappings, getSolidityFiles } from './utils';
});

for (const [fileName, source] of Object.entries(compiledFiles.data.sources)) {
const nodes = (source as any).ast.nodes.slice(-1)[0].nodes as SolcContractNode[];
if (fileName.startsWith('node_modules') || fileName.startsWith('lib')) continue;

nodes.forEach(node => {
const nodeNatspec = parseNodeNatspec(node);
const alerts = validate(node, nodeNatspec);

// print alerts
if (alerts.length) {
console.warn(`Natspec alerts in ${fileName}:${node.name}`);
alerts.forEach(alert => {
console.warn(' ' + alert.message);
});
}
const fileContracts = (source as any).ast.nodes.filter((node: any) => node.nodeType === 'ContractDefinition');
fileContracts.forEach((contract: any) => {
const nodes = contract.nodes as SolcContractNode[];

nodes.forEach(node => {
const nodeNatspec = parseNodeNatspec(node);
const warnings = validate(node, nodeNatspec);

// print warnings
if (warnings.length) {
console.warn(`Natspec warnings in ${fileName}:${contract.name}:${node.name}`);
warnings.forEach(warning => {
console.warn(' ' + warning);
});
}
});
});

}
})().catch(console.error);

Expand Down
26 changes: 14 additions & 12 deletions src/parser.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Natspec, NatspecParam, NatspecReturn, NatspecTag } from "./types/natspec.t";
import { Natspec, NatspecDefinition } from "./types/natspec.t";
import { SolcContractNode } from "./types/solc-typed-ast.t";

export function parseNodeNatspec(node: SolcContractNode): Natspec {
Expand All @@ -8,7 +8,7 @@ export function parseNodeNatspec(node: SolcContractNode): Natspec {

const docLines = node.documentation.text.split('\n');

let currentTag: null | NatspecTag | NatspecParam | NatspecReturn = null;
let currentTag: NatspecDefinition | null = null;
const result: Natspec = {
tags: [],
params: [],
Expand All @@ -20,25 +20,27 @@ export function parseNodeNatspec(node: SolcContractNode): Natspec {
if (tagTypeMatch) {
const tagName = tagTypeMatch[1];

if (tagName === 'param' || tagName === 'return') {
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], description: tagMatch[3].trim() };
if (tagName === 'param') {
result.params.push(currentTag as NatspecParam);
} else {
result.returns.push(currentTag as NatspecReturn);
}
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, description: tagMatch[2] };
result.tags.push(currentTag as NatspecTag);
currentTag = { name: tagName, content: tagMatch[2] };
result.tags.push(currentTag);
}
}
} else if (currentTag) {
currentTag.description += '\n' + line;
currentTag.content += '\n' + line;
}
});

Expand Down
23 changes: 7 additions & 16 deletions src/types/natspec.t.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,12 @@
export interface NatspecTag {
name: string;
description: string;
}

export interface NatspecParam {
name: string;
description: string;
}

export interface NatspecReturn {
name: string | undefined;
description: string;
export interface NatspecDefinition {
name?: string;
content: string;
}

export interface Natspec {
tags: NatspecTag[];
params: NatspecParam[];
returns: NatspecReturn[];
inheritdoc?: NatspecDefinition;
tags: NatspecDefinition[];
params: NatspecDefinition[];
returns: NatspecDefinition[];
}

63 changes: 21 additions & 42 deletions src/validator.ts
Original file line number Diff line number Diff line change
@@ -1,64 +1,43 @@
import { Natspec } from '../src/types/natspec.t';
import { SolcContractNode } from "./types/solc-typed-ast.t";

interface IAlert {
severity: string;
message: string;
}
export function validate(contractNode: SolcContractNode, natspec: Natspec): string[] {
let alerts: string[] = [];

export function validate(contractNode: SolcContractNode, natspec: Natspec): IAlert[] {
let alerts: IAlert[] = [];
if (natspec.inheritdoc) {
return alerts;
}

if(natspec.tags.length === 0) {
alerts.push({
severity: 'error',
message: `Natspec is missing`,
});
if(!natspec.tags.length) {
alerts.push(`Natspec is missing`);
return alerts;
};

// Make sure all defined function parameters have natspec
let functionParameters = contractNode.parameters?.parameters.map(p => p.name) ?? [];
let natspecParameters = natspec.params.map(p => p.name);

for(let param of functionParameters) {
if(!natspecParameters.includes(param)) {
alerts.push({
severity: 'error',
message: `Natspec for ${param} is missing`,
});
}
}

// Make sure there is no natspec defined for non-existing parameters
for(let param of natspecParameters) {
if(!functionParameters.includes(param)) {
alerts.push({
severity: 'error',
message: `Found natspec for undefined function parameter ${param}`,
});

// Make sure all defined function parameters have natspec
for(let paramName of functionParameters) {
if(!natspecParameters.includes(paramName)) {
alerts.push(`@param ${paramName} is missing`);
}
}

let functionReturns = contractNode.returnParameters?.parameters.map(p => p.name) ?? [];
let natspecReturns = natspec.returns.map(p => p.name);

for(let param of functionReturns) {
if(!natspecReturns.includes(param)) {
let message = param === '' ? 'Natspec for a return parameter is missing' : `Natspec for ${param} is missing`;
alerts.push({
severity: 'error',
message: message,
});
// 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);
}
}

// Make sure there is no natspec defined for non-existing returns
for(let param of natspecReturns) {
if(param && !functionReturns.includes(param)) {
alerts.push({
severity: 'error',
message: `Found natspec for undefined returned value ${param}`,
});
for(let paramName of natspecReturns) {
if(paramName && !functionReturns.includes(paramName)) {
alerts.push(`Missing named return for: @return ${paramName}`);
}
}

Expand Down
55 changes: 41 additions & 14 deletions test/parser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ import { parseSolidityFile } from './test-utils';

describe('parseNodeNatspec', () => {

describe('sample.sol', () => {
describe('BasicSample.sol', () => {
let nodes: SolcContractNode[];

beforeAll(async () => {
const file = 'sample-data/sample.sol';
const file = 'sample-data/BasicSample.sol';
const compileResult = await parseSolidityFile(file);
nodes = compileResult.data.sources[file].ast.nodes[1].nodes as SolcContractNode[];
});
Expand All @@ -20,10 +20,10 @@ describe('parseNodeNatspec', () => {
expect(result).toEqual({
tags: [{
name: 'notice',
description: 'Empty string for revert checks',
content: 'Empty string for revert checks',
}, {
name: 'dev',
description: `result of doing keccak256(bytes(''))`,
content: `result of doing keccak256(bytes(''))`,
}],
params: [],
returns: [],
Expand All @@ -39,21 +39,21 @@ describe('parseNodeNatspec', () => {
expect(result).toEqual({
tags: [{
name: 'notice',
description: 'External function that returns a bool',
content: 'External function that returns a bool',
}, {
name: 'dev',
description: 'A dev comment',
content: 'A dev comment',
}],
params: [{
name: '_magicNumber',
description: 'A parameter description',
content: 'A parameter description',
}, {
name: '_name',
description: 'Another parameter description',
content: 'Another parameter description',
}],
returns: [{
name: '_isMagic',
description: 'Some return data',
content: 'Some return data',
}],
});
});
Expand All @@ -65,11 +65,11 @@ describe('parseNodeNatspec', () => {
expect(result).toEqual({
tags: [{
name: 'notice',
description: 'Private test function',
content: 'Private test function',
}],
params: [{
name: '_magicNumber',
description: 'A parameter description',
content: 'A parameter description',
}],
returns: [],
});
Expand All @@ -83,7 +83,7 @@ describe('parseNodeNatspec', () => {
expect(result).toEqual({
tags: [{
name: 'notice',
description: 'Private test function\n with multiple\n lines',
content: 'Private test function\n with multiple\n lines',
}],
params: [],
returns: [],
Expand All @@ -97,10 +97,37 @@ describe('parseNodeNatspec', () => {
expect(result).toEqual({
tags: [{
name: 'notice',
description: 'Private test function',
content: 'Private test function',
}, {
name: 'notice',
description: 'Another notice',
content: 'Another notice',
}],
params: [],
returns: [],
});
});
});

describe('InterfacedSample.sol', () => {
let nodes: SolcContractNode[];

beforeAll(async () => {
const file = 'sample-data/InterfacedSample.sol';
const compileResult = await parseSolidityFile(file);
nodes = compileResult.data.sources[file].ast.nodes[2].nodes as SolcContractNode[];
});

it('should parse the inheritdoc tag', async () => {
const functionNode = nodes[0];
const result = parseNodeNatspec(functionNode);

expect(result).toEqual({
inheritdoc: {
content: 'IInterfacedSample',
},
tags: [{
name: 'dev',
content: 'some dev thingy',
}],
params: [],
returns: [],
Expand Down
Loading

0 comments on commit 16cb8c4

Please sign in to comment.