Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: add natspec #37

Merged
merged 6 commits into from
Feb 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"plugins": ["eslint-plugin-jsdoc"],
"extends": ["plugin:eslint-plugin-jsdoc/recommended"],
"ignorePatterns": ["dist", ".eslintrc.cjs"],
"parser": "@typescript-eslint/parser",
"parserOptions": {
"ecmaVersion": 9,
"sourceType": "module",
"project": "./tsconfig.json"
},
"env": {
"node": true,
"es6": true
}
}
4 changes: 4 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"scripts": {
"build": "tsc",
"lint:check": "prettier --check .",
"lint:docs": "eslint 'src/**/*.ts'",
"lint:fix": "sort-package-json && prettier --write .",
"prepack": "pinst --disable",
"postpack": "pinst --enable",
Expand All @@ -37,6 +38,9 @@
"@faker-js/faker": "8.3.1",
"@types/jest": "29.5.11",
"@types/node": "20.10.7",
"@typescript-eslint/parser": "6.2.0",
"eslint": "8.56.0",
"eslint-plugin-jsdoc": "48.1.0",
"husky": "8.0.3",
"jest": "29.7.0",
"lint-staged": "13.2.2",
Expand Down
7 changes: 7 additions & 0 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ import { Processor } from './processor';
import { Config } from './types';
import { Validator } from './validator';

/**
* Main function that processes the sources and prints the warnings
*/
(async () => {
const config: Config = getArguments();

Expand Down Expand Up @@ -34,6 +37,10 @@ import { Validator } from './validator';
});
})().catch(console.error);

/**
* Parses the command line arguments and returns the configuration
* @returns {Config} The config object
*/
function getArguments(): Config {
return yargs(hideBin(process.argv))
.strict()
Expand Down
25 changes: 25 additions & 0 deletions src/processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ export interface IWarning {
export class Processor {
constructor(private validator: Validator) {}

/**
* Goes through all functions, modifiers, state variables, structs, enums, errors and events
* of the source files and validates their natspec
* @param {SourceUnit[]} sourceUnits - The list of source files
* @returns {Promise<IWarning[]>} - The list of resulting warnings
*/
async processSources(sourceUnits: SourceUnit[]): Promise<IWarning[]> {
const warnings: IWarning[] = [];

Expand Down Expand Up @@ -41,6 +47,12 @@ export class Processor {
return warnings;
}

/**
* Selects the nodes that are eligible for natspec validation:
* Enums, Errors, Events, Functions, Modifiers, State Variables, Structs
* @param {ContractDefinition} contract - The contract source
* @returns {NodeToProcess[]} - The list of nodes to process
*/
selectEligibleNodes(contract: ContractDefinition): NodeToProcess[] {
return [
...contract.vEnums,
Expand All @@ -53,11 +65,24 @@ export class Processor {
];
}

/**
* Validates the natspec of the node
* @param {NodeToProcess} node - The node to process
* @returns {string[]} - The list of warning messages
*/
validateNatspec(node: NodeToProcess): string[] {
const nodeNatspec = parseNodeNatspec(node);
return this.validator.validate(node, nodeNatspec);
}

/**
* Generates a warning location string
* @param {string} filePath - Path of the file with the warning
* @param {string} fileContent - The content of the file
* @param {string} contractName - The name of the contract
* @param {NodeToProcess} node - The node with the warning
* @returns {string} - The formatted location
*/
formatLocation(filePath: string, fileContent: string, contractName: string, node: NodeToProcess): 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;
Expand Down
5 changes: 5 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,8 @@ export type NodeToProcess =
| ModifierDefinition
| VariableDeclaration
| StructDefinition;

export interface IWarning {
location: string;
messages: string[];
}
60 changes: 59 additions & 1 deletion src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,21 @@ import path from 'path';
import { Natspec, NatspecDefinition, NodeToProcess } from './types';
import { ASTKind, ASTReader, SourceUnit, compileSol, FunctionDefinition } from 'solc-typed-ast';

/**
* Returns the absolute paths of the Solidity files
* @param {string[]} files - The list of files paths
* @returns {Promise<string[]>} - The list of absolute paths
*/
export async function getSolidityFilesAbsolutePaths(files: string[]): Promise<string[]> {
return files.filter((file) => file.endsWith('.sol')).map((file) => path.resolve(file));
}

/**
* Returns the list of source units of the compiled Solidity files
* @param {string} rootPath - The root path of the project
* @param {string[]} includedPaths - The list of included paths
* @returns {SourceUnit[]} - The list of source units extracted from the compiled files
*/
export async function getProjectCompiledSources(rootPath: string, includedPaths: string[]): Promise<SourceUnit[]> {
// Fetch Solidity files from the specified directory
const solidityFiles: string[] = await getSolidityFilesAbsolutePaths(includedPaths);
Expand All @@ -26,6 +37,12 @@ export async function getProjectCompiledSources(rootPath: string, includedPaths:
);
}

/**
* Checks if the file path is in the specified directory
* @param {string} directory - The directory path
* @param {string} filePath - The file path
* @returns {boolean} - True if the file is in the directory
*/
export function isFileInDirectory(directory: string, filePath: string): boolean {
// Convert both paths to absolute and normalize them
const absoluteDirectoryPath = path.resolve(directory) + path.sep;
Expand All @@ -35,6 +52,11 @@ export function isFileInDirectory(directory: string, filePath: string): boolean
return absoluteFilePath.startsWith(absoluteDirectoryPath);
}

/**
* Returns the remappings from the remappings.txt file or foundry.toml
* @param {string} rootPath - The root path of the project
* @returns {Promise<string[]>} - The list of remappings
*/
export async function getRemappings(rootPath: string): Promise<string[]> {
// First try the remappings.txt file
try {
Expand All @@ -49,6 +71,11 @@ export async function getRemappings(rootPath: string): Promise<string[]> {
}
}

/**
* Returns the remappings from the remappings.txt file
* @param {string} remappingsPath - The path of the remappings file
* @returns {Promise<string[]>} - The list of remappings
*/
export async function getRemappingsFromFile(remappingsPath: string): Promise<string[]> {
const remappingsContent = await fs.readFile(remappingsPath, 'utf8');

Expand All @@ -59,6 +86,11 @@ export async function getRemappingsFromFile(remappingsPath: string): Promise<str
.map((line) => sanitizeRemapping(line));
}

/**
* Returns the remappings from the foundry.toml file
* @param {string} foundryConfigPath - The path of the foundry.toml file
* @returns {Promise<string[]>} - The list of remappings
*/
export async function getRemappingsFromConfig(foundryConfigPath: string): Promise<string[]> {
const foundryConfigContent = await fs.readFile(foundryConfigPath, 'utf8');
const regex = /remappings[\s|\n]*\=[\s\n]*\[(?<remappings>[^\]]+)]/;
Expand All @@ -75,8 +107,12 @@ export async function getRemappingsFromConfig(foundryConfigPath: string): Promis
}
}

/**
* Makes sure both sides of a remapping either have or don't have a trailing slash
* @param {string} line - A line from the remappings array
* @returns {string} - The sanitized line
*/
export function sanitizeRemapping(line: string): string {
// Make sure the key and the value both either have or don't have a trailing slash
const [key, value] = line.split('=');
const slashNeeded = key.endsWith('/');

Expand All @@ -87,6 +123,11 @@ export function sanitizeRemapping(line: string): string {
}
}

/**
* Parses the natspec of the node
* @param {NodeToProcess} node - The node to process
* @returns {Natspec} - The parsed natspec
*/
export function parseNodeNatspec(node: NodeToProcess): Natspec {
if (!node.documentation) {
return { tags: [], params: [], returns: [] };
Expand Down Expand Up @@ -133,16 +174,33 @@ export function parseNodeNatspec(node: NodeToProcess): Natspec {
return result;
}

/**
* Returns the line number from the source code
* @param {string} fileContent - The content of the file
* @param {string} src - The node src location (e.g. "10:1:0")
* @returns {number} - The line number of the node
*/
export function getLineNumberFromSrc(fileContent: string, src: string): number {
const [start] = src.split(':').map(Number);
const lines = fileContent.substring(0, start).split('\n');
return lines.length; // Line number
}

/**
* Checks if the node matches the function kind
* @param {NodeToProcess} node - The node to process
* @param {string} kind - The function kind
* @returns {boolean} - True if the node matches the function kind
*/
export function matchesFunctionKind(node: NodeToProcess, kind: string): boolean {
return node instanceof FunctionDefinition && node.kind === kind;
}

/**
* Returns the frequency of the elements in the array
* @param {any[]} array - The array of elements
* @returns {Record<string, number>} - The frequency of the elements
*/
export function getElementFrequency(array: any[]) {
return array.reduce((acc, curr) => {
acc[curr] = (acc[curr] || 0) + 1;
Expand Down
41 changes: 38 additions & 3 deletions src/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,25 @@ import {
ContractDefinition,
} from 'solc-typed-ast';

/**
* Validator class that validates the natspec of the nodes
*/
export class Validator {
config: Config;

/**
* @param {Config} config - The configuration object
*/
constructor(config: Config) {
this.config = config;
}

/**
* Validates the natspec of the node
* @param {NodeToProcess} node - The node to validate (Enum, Function etc.)
* @param {Natspec} natspec - Parsed natspec of the node
* @returns {string[]} - The list of alerts
*/
validate(node: NodeToProcess, natspec: Natspec): string[] {
// Ignore fallback and receive
if (matchesFunctionKind(node, 'receive') || matchesFunctionKind(node, 'fallback')) {
Expand Down Expand Up @@ -63,7 +75,13 @@ export class Validator {
return alerts;
}

// All defined parameters should have natspec
/**
* Validates the natspec for parameters.
* All defined parameters should have natspec.
* @param {ErrorDefinition | FunctionDefinition | ModifierDefinition} node - The node to validate
* @param {string[]} natspecParams - The list of parameters from the natspec
* @returns {string[]} - The list of alerts
*/
private validateParameters(node: ErrorDefinition | FunctionDefinition | ModifierDefinition, natspecParams: (string | undefined)[]): string[] {
let definedParameters = node.vParameters.vParameters.map((p) => p.name);
let alerts: string[] = [];
Expand All @@ -79,7 +97,13 @@ export class Validator {
return alerts;
}

// All members of a struct should have natspec
/**
* Validates the natspec for members of a struct.
* All members of a struct should have natspec.
* @param {StructDefinition} node - The struct node
* @param {string[]} natspecParams - The list of parameters from the natspec
* @returns {string[]} - The list of alerts
*/
private validateMembers(node: StructDefinition, natspecParams: (string | undefined)[]): string[] {
let members = node.vMembers.map((p) => p.name);
let alerts: string[] = [];
Expand All @@ -96,7 +120,13 @@ export class Validator {
return alerts;
}

// All returned parameters should have natspec
/**
* Validates the natspec for return parameters.
* All returned parameters should have natspec
* @param {FunctionDefinition} node - The function node
* @param {(string | undefined)[]} natspecReturns - The list of `return` tags from the natspec
* @returns {string[]} - The list of alerts
*/
private validateReturnParameters(node: FunctionDefinition, natspecReturns: (string | undefined)[]): string[] {
let alerts: string[] = [];
let functionReturns = node.vReturnParameters.vParameters.map((p) => p.name);
Expand All @@ -115,6 +145,11 @@ export class Validator {
return alerts;
}

/**
* Checks if the node requires inheritdoc
* @param {NodeToProcess} node - The node to process
* @returns {boolean} - True if the node requires inheritdoc
*/
private requiresInheritdoc(node: NodeToProcess): boolean {
let _requiresInheritdoc: boolean = false;

Expand Down
Loading
Loading