refactor: json config structure (#42)

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"

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;

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<Config> {
+  if (fs.existsSync(configPath)) {
+    return await processConfig(configPath);
+  }
+
+  const config: Partial<Config> = 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;
 }

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<typeof configSchema>;
+export type Functions = Static<typeof functionConfigSchema>;
 
 export interface NatspecDefinition {
   name?: string;

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<Config> {
+  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;
+}

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

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);

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();
+    });
+  });
 });