diff --git a/package.json b/package.json index 391ae7f..5c949c5 100644 --- a/package.json +++ b/package.json @@ -28,6 +28,8 @@ "**/*": "prettier --write --ignore-unknown" }, "dependencies": { + "@sinclair/typebox": "0.32.14", + "ajv": "8.12.0", "fast-glob": "3.3.2", "solc-typed-ast": "18.1.2", "yargs": "17.7.2" diff --git a/src/constants.ts b/src/constants.ts new file mode 100644 index 0000000..cf8b9ad --- /dev/null +++ b/src/constants.ts @@ -0,0 +1,9 @@ +import { Functions } from './types'; + +export const defaultFunctions: Functions = { + internal: { tags: { dev: false, notice: true, return: true, param: true } }, + external: { tags: { dev: false, notice: true, return: true, param: true } }, + public: { tags: { dev: false, notice: true, return: true, param: true } }, + private: { tags: { dev: false, notice: true, return: true, param: true } }, + constructor: false as unknown as Function & boolean, +} as const; diff --git a/src/main.ts b/src/main.ts index 646e49c..d1b0490 100644 --- a/src/main.ts +++ b/src/main.ts @@ -1,18 +1,24 @@ #!/usr/bin/env node +import path from 'path'; import yargs from 'yargs'; +import fs from 'fs'; import { hideBin } from 'yargs/helpers'; import { glob } from 'fast-glob'; -import { getProjectCompiledSources } from './utils'; +import { getProjectCompiledSources, processConfig } from './utils'; import { Processor } from './processor'; import { Config } from './types'; import { Validator } from './validator'; +import { defaultFunctions } from './constants'; /** * Main function that processes the sources and prints the warnings */ (async () => { - const config: Config = getArguments(); + // Requires the config is in the root of the users directory + const configPath = path.join(process.cwd(), './natspec-smells.config.json'); + const config: Config = await getConfig(configPath); + // TODO: Add configuration logic to the linter const excludedPaths = config.exclude === '' ? [] : await glob(config.exclude, { cwd: config.root }); const includedPaths = await glob(config.include, { cwd: config.root, ignore: excludedPaths }); @@ -38,12 +44,17 @@ import { Validator } from './validator'; })().catch(console.error); /** - * Parses the command line arguments and returns the configuration - * @returns {Config} The config object + * Gets the config from the CLI or the config file + * @dev Prioritizes the config file over the CLI + * @param {string} configPath - The expected config path + * @returns {Config} - The config */ -function getArguments(): Config { - return yargs(hideBin(process.argv)) - .strict() +async function getConfig(configPath: string): Promise { + if (fs.existsSync(configPath)) { + return await processConfig(configPath); + } + + const config: Partial = yargs(hideBin(process.argv)) .options({ include: { type: 'string', @@ -60,18 +71,15 @@ function getArguments(): Config { description: 'Root directory of the project.', default: './', }, - enforceInheritdoc: { + inheritdoc: { type: 'boolean', description: 'If set to true, all external and public functions must have @inheritdoc.', default: true, }, - constructorNatspec: { - type: 'boolean', - description: 'True if constructor natspec is mandatory.', - default: false, - }, }) - .config() - .default('config', 'natspec-smells.config') .parseSync(); + + config.functions = defaultFunctions; + + return config as Config; } diff --git a/src/types.ts b/src/types.ts index ae47197..9e6f544 100644 --- a/src/types.ts +++ b/src/types.ts @@ -7,14 +7,63 @@ import { StructDefinition, VariableDeclaration, } from 'solc-typed-ast'; +import { Static, Type } from '@sinclair/typebox'; -export interface Config { - include: string; // Required: Glob pattern of files to process. - exclude: string; // Optional: Glob pattern of files to exclude. - root: string; // Optional: Project root directory. - enforceInheritdoc: boolean; // Optional: True if all external and public functions should have @inheritdoc. - constructorNatspec: boolean; // Optional: True if the constructor should have natspec. -} +// NOTE: For params like `return` if its set to true we will only force it if the function does return something +export const functionConfigSchema = Type.Object({ + internal: Type.Optional( + Type.Object({ + tags: Type.Object({ + dev: Type.Boolean({ default: false }), + notice: Type.Boolean({ default: true }), + return: Type.Boolean({ default: true }), + param: Type.Boolean({ default: true }), + }), + }) + ), + external: Type.Optional( + Type.Object({ + tags: Type.Object({ + dev: Type.Boolean({ default: false }), + notice: Type.Boolean({ default: true }), + return: Type.Boolean({ default: true }), + param: Type.Boolean({ default: true }), + }), + }) + ), + public: Type.Optional( + Type.Object({ + tags: Type.Object({ + dev: Type.Boolean({ default: false }), + notice: Type.Boolean({ default: true }), + return: Type.Boolean({ default: true }), + param: Type.Boolean({ default: true }), + }), + }) + ), + private: Type.Optional( + Type.Object({ + tags: Type.Object({ + dev: Type.Boolean({ default: false }), + notice: Type.Boolean({ default: true }), + return: Type.Boolean({ default: true }), + param: Type.Boolean({ default: true }), + }), + }) + ), + constructor: Type.Boolean({ default: false }), +}); + +export const configSchema = Type.Object({ + include: Type.String(), + exclude: Type.String({ default: '' }), + root: Type.String({ default: './' }), + functions: functionConfigSchema, + inheritdoc: Type.Boolean({ default: true }), +}); + +export type Config = Static; +export type Functions = Static; export interface NatspecDefinition { name?: string; diff --git a/src/utils.ts b/src/utils.ts index f2fc3cd..41bb24e 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,7 +1,9 @@ import fs from 'fs/promises'; import path from 'path'; -import { Natspec, NatspecDefinition, NodeToProcess } from './types'; +import Ajv from 'ajv'; +import { Natspec, NatspecDefinition, NodeToProcess, Config, configSchema, Functions } from './types'; import { ASTKind, ASTReader, SourceUnit, compileSol, FunctionDefinition } from 'solc-typed-ast'; +import { defaultFunctions } from './constants'; /** * Returns the absolute paths of the Solidity files @@ -207,3 +209,44 @@ export function getElementFrequency(array: any[]) { return acc; }, {}); } + +/** + * Processes a config file based on the given file path + * @param {string} filePath - The path to the config file + * @returns {Config} - The config + */ +export async function processConfig(filePath: string): Promise { + const file = await fs.readFile(filePath, 'utf8'); + const detectedConfig = JSON.parse(file); + + if (!detectedConfig.functions) { + detectedConfig.functions = defaultFunctions; + } else { + for (const key of Object.keys(defaultFunctions)) { + if (!detectedConfig.functions[key]) { + detectedConfig.functions[key] = defaultFunctions[key as keyof Functions]; + } + } + } + + // TODO: Deprecation logic will be defined here + // Set defaults if needed + const config: Config = { + include: detectedConfig.include, + exclude: detectedConfig.exclude ?? '', + root: detectedConfig.root ?? './', + functions: detectedConfig.functions, + inheritdoc: detectedConfig.inheritdoc ?? true, + }; + + // Validate the received config matches our expected type + const ajv = new Ajv(); + const validate = ajv.compile(configSchema); + const valid = validate(config); + + if (!valid) { + throw new Error(`Invalid config: ${ajv.errorsText(validate.errors)}`); + } + + return config; +} diff --git a/src/validator.ts b/src/validator.ts index 22589df..d3fe0f5 100644 --- a/src/validator.ts +++ b/src/validator.ts @@ -40,13 +40,13 @@ export class Validator { if (natspec.inheritdoc) return []; // Inheritdoc is enforced but not present, returning an error - if (this.config.enforceInheritdoc && this.requiresInheritdoc(node)) return [`@inheritdoc is missing`]; + if (this.config.inheritdoc && this.requiresInheritdoc(node)) return [`@inheritdoc is missing`]; const natspecParams = natspec.params.map((p) => p.name); // Validate natspec for the constructor only if configured if (matchesFunctionKind(node, 'constructor')) { - return this.config.constructorNatspec ? this.validateParameters(node as FunctionDefinition, natspecParams) : []; + return this.config.functions?.constructor ? this.validateParameters(node as FunctionDefinition, natspecParams) : []; } // Inheritdoc is not enforced nor present, and there is no other documentation, returning error diff --git a/test/parser.test.ts b/test/parser.test.ts index 7000b28..22ca00a 100644 --- a/test/parser.test.ts +++ b/test/parser.test.ts @@ -375,13 +375,6 @@ describe('Parser', () => { expect(result).toEqual(mockNatspec({})); }); - it('should parse block natspec with invalid formatting', async () => { - const node = findNode(contract.vFunctions, '_viewBlockLinterFail'); - const result = parseNodeNatspec(node); - - expect(result).toEqual(mockNatspec({})); - }); - it('should parse natspec with invalid formatting', async () => { const node = findNode(contract.vFunctions, '_viewLinterFail'); const result = parseNodeNatspec(node); diff --git a/test/utils.test.ts b/test/utils.test.ts index 270263f..5a03aa4 100644 --- a/test/utils.test.ts +++ b/test/utils.test.ts @@ -1,8 +1,10 @@ import fs from 'fs/promises'; +import fstest from 'fs'; import path from 'path'; import * as utils from '../src/utils'; import { mockFoundryConfig, mockFunctionDefinition } from './utils/mocks'; import { FunctionKind } from 'solc-typed-ast'; +import { defaultFunctions } from '../src/constants'; describe('Utils', () => { describe('getSolidityFilesAbsolutePaths', () => { @@ -186,4 +188,134 @@ describe('Utils', () => { }); }); }); + + describe('processConfig', () => { + it('should use a valid config', async () => { + fs.readFile = jest.fn().mockResolvedValueOnce( + JSON.stringify({ + include: './contracts/**/*.sol', + }) + ); + const config = await utils.processConfig(path.join(__dirname, './valid.config.json')); + + // The default settings should be applied + expect(config).toEqual({ + root: './', + include: './contracts/**/*.sol', + exclude: '', + inheritdoc: true, + functions: defaultFunctions, + }); + }); + + it('should revert with an invalid config', async () => { + fs.readFile = jest.fn().mockResolvedValueOnce( + JSON.stringify({ + include: './contracts/**/*.sol', + exclude: 123, + }) + ); + await expect(utils.processConfig(path.join(__dirname, './invalid.config.json'))).rejects.toThrow(); + }); + + it('should overwrite defaults if values are set', async () => { + fs.readFile = jest.fn().mockResolvedValueOnce( + JSON.stringify({ + include: './contracts/**/*.sol', + exclude: './contracts/ignored.sol', + root: './contracts', + inheritdoc: false, + }) + ); + const config = await utils.processConfig(path.join(__dirname, './valid.config.json')); + + expect(config).toEqual({ + root: './contracts', + include: './contracts/**/*.sol', + exclude: './contracts/ignored.sol', + inheritdoc: false, + functions: defaultFunctions, + }); + }); + + it('should set custom parameters for functions', async () => { + fs.readFile = jest.fn().mockResolvedValueOnce( + JSON.stringify({ + include: './contracts/**/*.sol', + functions: { + internal: { + tags: { + dev: true, + notice: true, + return: true, + param: true, + }, + }, + constructor: true, + }, + }) + ); + const config = await utils.processConfig(path.join(__dirname, './valid.config.json')); + + expect(config).toEqual({ + root: './', + include: './contracts/**/*.sol', + exclude: '', + inheritdoc: true, + functions: { + internal: { + tags: { + dev: true, + notice: true, + return: true, + param: true, + }, + }, + external: { + tags: { + dev: false, + notice: true, + return: true, + param: true, + }, + }, + public: { + tags: { + dev: false, + notice: true, + return: true, + param: true, + }, + }, + private: { + tags: { + dev: false, + notice: true, + return: true, + param: true, + }, + }, + constructor: true, + }, + }); + }); + + it('should revert if a function block is incomplete', async () => { + fs.readFile = jest.fn().mockResolvedValueOnce( + JSON.stringify({ + include: './contracts/**/*.sol', + functions: { + internal: { + tags: { + dev: true, + notice: true, + }, + }, + }, + }) + ); + + await expect(utils.processConfig(path.join(__dirname, './invalid.config.json'))).rejects.toThrow(); + }); + }); }); diff --git a/test/utils/helpers.ts b/test/utils/helpers.ts index 67edddb..1d56c37 100644 --- a/test/utils/helpers.ts +++ b/test/utils/helpers.ts @@ -1,4 +1,5 @@ import { ASTKind, ASTReader, SourceUnit, compileSol } from 'solc-typed-ast'; +import path from 'path'; import { NodeToProcess } from '../../src/types'; export async function getFileCompiledSource(filePath: string): Promise { diff --git a/test/validator.test.ts b/test/validator.test.ts index 772dbaa..b6f09d2 100644 --- a/test/validator.test.ts +++ b/test/validator.test.ts @@ -364,7 +364,7 @@ describe('Validator', () => { describe('with enforced constructor natspec', () => { beforeAll(async () => { - validator = new Validator(mockConfig({ constructorNatspec: true })); + validator = new Validator(mockConfig({ functions: { constructor: true as unknown as Function & boolean } })); }); it('should reveal missing constructor natspec', () => { @@ -376,7 +376,7 @@ describe('Validator', () => { describe('with disabled constructor natspec', () => { beforeAll(async () => { - validator = new Validator(mockConfig({ constructorNatspec: false })); + validator = new Validator(mockConfig({ functions: { constructor: false as unknown as Function & boolean } })); }); it('should ignore missing constructor natspec', () => { @@ -388,7 +388,7 @@ describe('Validator', () => { describe('with enforced inheritdoc', () => { beforeAll(async () => { - validator = new Validator(mockConfig({ enforceInheritdoc: true })); + validator = new Validator(mockConfig({ inheritdoc: true })); }); it('should return no warnings if inheritdoc is in place', () => { diff --git a/yarn.lock b/yarn.lock index f01d7bb..aaf98b9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -843,6 +843,11 @@ "@noble/hashes" "~1.3.0" "@scure/base" "~1.1.0" +"@sinclair/typebox@0.32.14": + version "0.32.14" + resolved "https://registry.yarnpkg.com/@sinclair/typebox/-/typebox-0.32.14.tgz#ef0a4ed981515fd430cadfb65cb6c2719a0b5539" + integrity sha512-EC77Mw8huT2z9YlYbWfpIQgN6shZE1tH4NP4/Trig8UBel9FZNMZRJ42ubJI8PLor2uIU+waLml1dce5ReCOPg== + "@sinclair/typebox@^0.27.8": version "0.27.8" resolved "https://registry.yarnpkg.com/@sinclair/typebox/-/typebox-0.27.8.tgz#6667fac16c436b5434a387a34dedb013198f6e6e" @@ -1081,17 +1086,7 @@ aggregate-error@^3.0.0: clean-stack "^2.0.0" indent-string "^4.0.0" -ajv@^6.12.4: - version "6.12.6" - resolved "https://registry.yarnpkg.com/ajv/-/ajv-6.12.6.tgz#baf5a62e802b07d977034586f8c3baf5adf26df4" - integrity sha512-j3fVLgvTo527anyYyJOGTYJbG+vnnQYvE0m5mmkc1TK+nxAppkCLMIL0aZ4dblVCNoGShhm+kzE4ZUykBoMg4g== - dependencies: - fast-deep-equal "^3.1.1" - fast-json-stable-stringify "^2.0.0" - json-schema-traverse "^0.4.1" - uri-js "^4.2.2" - -ajv@^8.11.0: +ajv@8.12.0, ajv@^8.11.0: version "8.12.0" resolved "https://registry.yarnpkg.com/ajv/-/ajv-8.12.0.tgz#d1a0527323e22f53562c567c00991577dfbe19d1" integrity sha512-sRu1kpcO9yLtYxBKvqfTeh9KzZEwO3STyX1HT+4CaDzC6HpTGYhIhPIzj9XuKU7KYDwnaeh5hcOwjy1QuJzBPA==