Skip to content

Commit

Permalink
feat: added inheritdoc support, workflows, and fixes
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 f8f1d03
Show file tree
Hide file tree
Showing 12 changed files with 236 additions and 190 deletions.
28 changes: 28 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
name: Node.js CI

on: push

permissions: write-all

jobs:
build:
runs-on: ubuntu-latest

strategy:
matrix:
node-version: [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
env:
SOL_AST_COMPILER_CACHE: /home/runner/work/_temp/.compiler_cache

3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,6 @@ yarn-error.log*

# Solidity
src/artifacts

# Build
lib
32 changes: 32 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
[![image](https://img.shields.io/npm/v/@defi-wonderland/natspec-smells.svg?style=flat-square)](https://www.npmjs.org/package/@defi-wonderland/natspec-smells)

# Natspec Smells

Some description will be written here.

## Usage

As simple as it gets, run:
```bash
npx @defi-wonderland/natspec-smells --contracts ./solidity
```


## Options

### `--contracts` (Required)
Relative path to your solidity files.

### `--base`
Base directory to be used.

Default: `./`


## Contributors

Natspec Smells was built with ❤️ by [Wonderland](https://defi.sucks).

Wonderland is a team of top Web3 researchers, developers, and operators who believe that the future needs to be open-source, permissionless, and decentralized.

[DeFi sucks](https://defi.sucks), but Wonderland is here to make it better.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"name": "@defi-wonderland/natspec-warnings",
"name": "@defi-wonderland/natspec-smells",
"version": "1.0.0",
"license": "MIT",
"main": "lib/main.js",
Expand Down
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
Loading

0 comments on commit f8f1d03

Please sign in to comment.