refactor: introducing classes (#11)

package.json

@@ -27,6 +27,7 @@
   "devDependencies": {
     "@commitlint/cli": "17.6.5",
     "@commitlint/config-conventional": "17.6.5",
+    "@faker-js/faker": "8.3.1",
     "@types/jest": "29.5.11",
     "@types/node": "20.10.7",
     "husky": "8.0.3",

sample-data/ParserTest.sol

@@ -0,0 +1,183 @@
+// SPDX-License-Identifier: MIT
+pragma solidity =0.8.19;
+
+// forgefmt: disable-start
+// This file is used for testing the parser
+
+interface IParserTest {
+  /// @notice Thrown whenever something goes wrong
+  error SimpleError(uint256 _param1, uint256 _param2);
+
+  /// @notice Emitted whenever something happens
+  event SimpleEvent(uint256 _param1, uint256 _param2);
+
+  /// @notice The enum description
+  enum SimpleEnum {
+    A,
+    B,
+    C
+  }
+
+  /// @notice View function with no parameters
+  /// @dev Natspec for the return value is missing
+  /// @return The returned value
+  function viewFunctionNoParams() external view returns (uint256);
+
+  /**
+   * @notice A function with different style of natspec
+   * @param _param1  The first parameter
+   * @param _param2  The second parameter
+   * @return The returned value
+   */
+   function viewFunctionWithParams(uint256 _param1, uint256 _param2) external view returns (uint256);
+
+   /// @notice A state variable
+   /// @return Some value
+   function someVariable() external view returns (uint256);
+
+  /// @notice A struct holding 2 variables of type uint256
+  /// @member a  The first variable
+  /// @member b  The second variable
+  /// @dev This is definitely a struct
+  struct SimplestStruct {
+    uint256 a;
+    uint256 b;
+  }
+
+  /// @notice A constant of type uint256
+  function SOME_CONSTANT() external view returns (uint256 _returned);
+}
+
+/// @notice A contract with correct natspec
+contract ParserTest is IParserTest {
+  /// @inheritdoc IParserTest
+  uint256 public someVariable;
+
+  /// @inheritdoc IParserTest
+  uint256 public constant SOME_CONSTANT = 123;
+
+  /// @notice The constructor
+  /// @param _struct The struct parameter
+  constructor(SimplestStruct memory _struct) {
+    someVariable = _struct.a + _struct.b;
+  }
+
+  /// @notice The description of the modifier
+  /// @param _param1 The only parameter
+  modifier someModifier(bool _param1) {
+    _;
+  }
+
+  // TODO: Fallback and receive functions
+  // fallback() {}
+  // receive () {}
+
+  /// @inheritdoc IParserTest
+  /// @dev Dev comment for the function
+  function viewFunctionNoParams() external pure returns (uint256){
+    return 1;
+  }
+
+  /// @inheritdoc IParserTest
+  function viewFunctionWithParams(uint256 _param1, uint256 _param2) external pure returns (uint256) {
+    return _param1 + _param2;
+  }
+
+  /// @notice Some private stuff
+  /// @dev Dev comment for the private function
+  /// @param _paramName The parameter name
+  /// @return _returned The returned value
+  function _viewPrivate(uint256 _paramName) private pure returns (uint256 _returned) {
+    return 1;
+  }
+
+  /// @notice Some internal stuff
+  /// @dev Dev comment for the internal function
+  /// @param _paramName The parameter name
+  /// @return _returned The returned value
+  function _viewInternal(uint256 _paramName) internal pure returns (uint256 _returned) {
+    return 1;
+  }
+
+  /// @notice Some internal stuff
+  ///         Separate line
+  ///         Third one
+  function _viewMultiline() internal pure {
+  }
+
+  /// @notice Some internal stuff
+  /// @notice Separate line
+  function _viewDuplicateTag() internal pure {
+  }
+}
+
+// This is a contract with invalid / missing natspec
+contract ParserTestFunny is IParserTest {
+  // no natspec, just a comment
+  struct SimpleStruct {
+    /// @notice The first variable
+    uint256 a;
+    /// @notice The first variable
+    uint256 b;
+  }
+
+  modifier someModifier() {
+    _;
+  }
+
+  /// @inheritdoc IParserTest
+  /// @dev Providing context
+  uint256 public someVariable;
+
+  // @inheritdoc IParserTest
+  uint256 public constant SOME_CONSTANT = 123;
+
+  /// @inheritdoc IParserTest
+  function viewFunctionNoParams() external view returns (uint256){
+    return 1;
+  }
+
+  // Forgot there is @inheritdoc and @notice
+  function viewFunctionWithParams(uint256 _param1, uint256 _param2) external view returns (uint256) {
+    return _param1 + _param2;
+  }
+
+  // @notice Some internal stuff
+  function _viewInternal() internal view returns (uint256) {
+    return 1;
+  }
+
+  /**
+   *
+   *
+   * 
+   * I met Obama once
+   * She's cool
+   */
+
+  /// @notice   Some private stuff
+  /// @param      _paramName The parameter name
+  /// @return     _returned     The returned value
+  function _viewPrivate(uint256 _paramName) private pure returns (uint256 _returned) {
+    return 1;
+  }
+
+  // @notice Forgot one slash and it's not natspec anymore
+  //// @dev Too many slashes is fine though
+  //// @return _returned The returned value
+  function _viewInternal(uint256 _paramName) internal pure returns (uint256 _returned) {
+    return 1;
+  }
+
+   /**** @notice Some text
+    ** */
+  function _viewBlockLinterFail() internal pure {
+  }
+
+ /// @notice Linter fail
+     /// @dev      What have I done
+  function _viewLinterFail() internal pure {
+
+  }
+}
+// forgefmt: disable-end

src/main.ts

@@ -1,17 +1,19 @@
 #!/usr/bin/env node
 import yargs from 'yargs';
 import { hideBin } from 'yargs/helpers';
+import { Config } from './types/config.t';
+import { getProjectCompiledSources } from './utils';
 import { globSync } from 'fast-glob';
-import { getProjectCompiledSources, Config } from './utils';
-import { processSources } from './processor';
+import { Processor } from './processor';
 
 (async () => {
   const config: Config = getArguments();
-  const ignoredPaths = config.ignore.map((path) => globSync(path, { cwd: config.root })).flat();
+  const ignoredPaths = config.ignore.map((path: string) => globSync(path, { cwd: config.root })).flat();
   const sourceUnits = await getProjectCompiledSources(config.root, config.contracts, ignoredPaths);
   if (!sourceUnits.length) return console.error('No solidity files found in the specified directory');
 
-  const warnings = await processSources(sourceUnits, config);
+  const processor = new Processor(config);
+  const warnings = processor.processSources(sourceUnits);
 
   warnings.forEach(({ location, messages }) => {
     console.warn(location);

src/processor.ts

@@ -1,63 +1,73 @@
-import { parseNodeNatspec } from './parser';
-import { Config } from './utils';
-import { validate } from './validator';
-import { SourceUnit, FunctionDefinition } from 'solc-typed-ast';
 import fs from 'fs';
+import { Config } from './types/config.t';
+import { Validator } from './validator';
+import { SourceUnit, FunctionDefinition, ContractDefinition } from 'solc-typed-ast';
+import { NodeToProcess } from './types/solc-typed-ast.t';
+import { parseNodeNatspec } from './utils';
 
 interface IWarning {
   location: string;
   messages: string[];
 }
 
-export async function processSources(sourceUnits: SourceUnit[], config: Config): Promise<IWarning[]> {
-  let warnings: IWarning[] = [];
-
-  sourceUnits.forEach((sourceUnit) => {
-    sourceUnit.vContracts.forEach((contract) => {
-      [
-        ...contract.vEnums,
-        ...contract.vErrors,
-        ...contract.vEvents,
-        ...contract.vFunctions,
-        ...contract.vModifiers,
-        ...contract.vStateVariables,
-        ...contract.vStructs,
-      ].forEach((node) => {
-        if (!node) return;
-
-        const nodeNatspec = parseNodeNatspec(node);
-        const validationMessages = validate(node, nodeNatspec, config);
-
-        // 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;
-        const sourceCode = fs.readFileSync(sourceUnit.absolutePath, 'utf8');
-        const line = lineNumber(nodeName as string, sourceCode);
-
-        if (validationMessages.length) {
-          warnings.push({
-            location: `${sourceUnit.absolutePath}:${line}\n${contract.name}:${nodeName}`,
-            messages: validationMessages,
-          });
-        }
-      });
-    });
-  });
-
-  return warnings;
-}
+export class Processor {
+  config: Config;
+  validator: Validator;
 
-function lineNumberByIndex(index: number, string: string): Number {
-  let line = 0;
-  let match;
-  let re = /(^)[\S\s]/gm;
+  constructor(config: Config) {
+    this.config = config;
+    this.validator = new Validator(config);
+  }
 
-  while ((match = re.exec(string))) {
-    if (match.index > index) break;
-    line++;
+  processSources(sourceUnits: SourceUnit[]): IWarning[] {
+    return sourceUnits.flatMap((sourceUnit) =>
+      sourceUnit.vContracts.flatMap((contract) =>
+        this.selectEligibleNodes(contract)
+          .map((node) => this.validateNodeNatspec(sourceUnit, node, contract))
+          .filter((warning) => warning.messages.length)
+      )
+    );
+  }
+
+  selectEligibleNodes(contract: ContractDefinition): NodeToProcess[] {
+    return [
+      ...contract.vEnums,
+      ...contract.vErrors,
+      ...contract.vEvents,
+      ...contract.vFunctions,
+      ...contract.vModifiers,
+      ...contract.vStateVariables,
+      ...contract.vStructs,
+    ];
+  }
+
+  validateNatspec(node: NodeToProcess): string[] {
+    if (!node) return [];
+    const nodeNatspec = parseNodeNatspec(node);
+    return this.validator.validate(node, nodeNatspec);
   }
-  return line;
-}
 
-function lineNumber(needle: string, haystack: string): Number {
-  return lineNumberByIndex(haystack.indexOf(needle), haystack);
+  validateNodeNatspec(sourceUnit: SourceUnit, node: NodeToProcess, contract: ContractDefinition): IWarning {
+    const validationMessages: string[] = this.validateNatspec(node);
+
+    if (validationMessages.length) {
+      return { location: this.formatLocation(node, sourceUnit, contract), messages: validationMessages };
+    } else {
+      return { location: '', messages: [] };
+    }
+  }
+
+  formatLocation(node: NodeToProcess, sourceUnit: SourceUnit, contract: ContractDefinition): 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;
+    const line = this.getLineNumberFromSrc(sourceUnit.absolutePath, node.src);
+    return `${sourceUnit.absolutePath}:${line}\n${contract.name}:${nodeName}`;
+  }
+
+  private getLineNumberFromSrc(filePath: string, src: string) {
+    const [start] = src.split(':').map(Number);
+    const fileContent = fs.readFileSync(filePath, 'utf8');
+    const lines = fileContent.substring(0, start).split('\n');
+    return lines.length; // Line number
+  }
 }

src/types/config.t.ts

@@ -0,0 +1,7 @@
+export interface Config {
+  root: string;
+  contracts: string;
+  enforceInheritdoc: boolean;
+  constructorNatspec: boolean;
+  ignore: string[];
+}