feat: finishing logic for remaining identifiers

defi-wonderland · Mar 6, 2024 · 8c3625a · 8c3625a
1 parent f87aca4
commit 8c3625a
Show file tree

Hide file tree

Showing 8 changed files with 554 additions and 29 deletions.
diff --git a/src/constants.ts b/src/constants.ts
@@ -6,3 +6,11 @@ export const defaultFunctions: Functions = {
   public: { tags: { dev: false, notice: true, return: true, param: true } },
   private: { tags: { dev: false, notice: true, return: true, param: true } },
 } as const;
+
+export const defaultTags = {
+  tags: {
+    dev: false,
+    notice: true,
+    param: true,
+  },
+} as const;
diff --git a/src/main.ts b/src/main.ts
@@ -8,7 +8,7 @@ import { getProjectCompiledSources, processConfig } from './utils';
 import { Processor } from './processor';
 import { Config } from './types';
 import { Validator } from './validator';
-import { defaultFunctions } from './constants';
+import { defaultFunctions, defaultTags } from './constants';
 
 /**
  * Main function that processes the sources and prints the warnings
@@ -85,6 +85,10 @@ async function getConfig(configPath: string): Promise<Config> {
     .parseSync();
 
   config.functions = defaultFunctions;
+  config.modifiers = defaultTags;
+  config.errors = defaultTags;
+  config.events = defaultTags;
+  config.structs = defaultTags;
 
   return config as Config;
 }
diff --git a/src/types.ts b/src/types.ts
@@ -11,12 +11,20 @@ import { Static, Type } from '@sinclair/typebox';
 
 // NOTE: For params like `return` if its set to true we will only force it if the function does return something
 
+export const tagSchema = Type.Object({
+  tags: Type.Object({
+    dev: Type.Boolean({ default: false }),
+    notice: Type.Boolean({ default: true }),
+    param: Type.Boolean({ default: true }),
+  }),
+});
+
 export const functionSchema = 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 }),
+    return: Type.Boolean({ default: true }),
   }),
 });
 
@@ -35,13 +43,19 @@ export const configSchema = Type.Object({
   exclude: Type.String({ default: '' }),
   root: Type.String({ default: './' }),
   functions: functionConfigSchema,
+  events: tagSchema,
+  errors: tagSchema,
+  modifiers: tagSchema,
+  structs: tagSchema,
   inheritdoc: Type.Boolean({ default: true }),
   constructorNatspec: Type.Boolean({ default: false }),
 });
 
+export type KeysForSupportedTags = 'events' | 'errors' | 'modifiers' | 'structs';
 export type FunctionConfig = Static<typeof functionSchema>;
 export type Config = Static<typeof configSchema>;
 export type Functions = Static<typeof functionConfigSchema>;
+export type Tags = Static<typeof tagSchema>;
 
 export interface NatspecDefinition {
   name?: string;
@@ -81,3 +95,9 @@ export interface IWarning {
   location: string;
   messages: string[];
 }
+
+export type HasVParameters = {
+  vParameters: {
+    vParameters: Array<{ name: string }>;
+  };
+};
diff --git a/src/utils.ts b/src/utils.ts
@@ -1,9 +1,9 @@
 import fs from 'fs/promises';
 import path from 'path';
 import Ajv from 'ajv';
-import { Natspec, NatspecDefinition, NodeToProcess, Config, configSchema, Functions } from './types';
+import { Natspec, NatspecDefinition, NodeToProcess, Config, configSchema, Functions, KeysForSupportedTags } from './types';
 import { ASTKind, ASTReader, SourceUnit, compileSol, FunctionDefinition } from 'solc-typed-ast';
-import { defaultFunctions } from './constants';
+import { defaultFunctions, defaultTags } from './constants';
 
 /**
  * Returns the absolute paths of the Solidity files
@@ -236,6 +236,10 @@ export async function processConfig(filePath: string): Promise<Config> {
     exclude: detectedConfig.exclude ?? '',
     root: detectedConfig.root ?? './',
     functions: detectedConfig.functions,
+    events: detectedConfig.events ?? defaultTags,
+    errors: detectedConfig.errors ?? defaultTags,
+    modifiers: detectedConfig.modifiers ?? defaultTags,
+    structs: detectedConfig.structs ?? defaultTags,
     inheritdoc: detectedConfig.inheritdoc ?? true,
     constructorNatspec: detectedConfig.constructorNatspec ?? false,
   };
@@ -251,3 +255,13 @@ export async function processConfig(filePath: string): Promise<Config> {
 
   return config;
 }
+
+/**
+ * Returns if the key being used is for a supported tag
+ * @dev A "supported tag" is a generalized term for events, errors, modifiers, and structs as they all share the same tag schema
+ * @param {string} key The key to check
+ * @returns {boolean} True if the key is for a supported tag
+ */
+export function isKeyForSupportedTags(key: string): key is KeysForSupportedTags {
+  return ['events', 'errors', 'modifiers', 'structs'].includes(key);
+}
diff --git a/src/validator.ts b/src/validator.ts
@@ -1,5 +1,15 @@
-import { Config, FunctionConfig, Functions, Natspec, NatspecDefinition, NodeToProcess } from './types';
-import { matchesFunctionKind, getElementFrequency } from './utils';
+import {
+  Config,
+  FunctionConfig,
+  Functions,
+  HasVParameters,
+  Natspec,
+  NatspecDefinition,
+  NodeToProcess,
+  KeysForSupportedTags,
+  Tags,
+} from './types';
+import { matchesFunctionKind, getElementFrequency, isKeyForSupportedTags } from './utils';
 import {
   EnumDefinition,
   ErrorDefinition,
@@ -51,46 +61,85 @@ export class Validator {
 
     // Inheritdoc is not enforced nor present, and there is no other documentation, returning error
     if (!natspec.tags.length) {
+      let needsWarning = false;
       // If node is a function, check the user defined config
       if (node instanceof FunctionDefinition) {
-        let needsWarning = false;
-
         Object.keys(this.config.functions).forEach((key) => {
           Object.keys(this.config.functions[key as keyof Functions].tags).forEach((tag) => {
             if (this.config.functions[key as keyof Functions][tag as keyof FunctionConfig]) {
               needsWarning = true;
             }
           });
         });
-
-        if (needsWarning) return [`Natspec is missing`];
       } else {
-        // TODO: Change this logic when we have more config options for events, structs, enums etc.
-        return [`Natspec is missing`];
+        // The other config rules use the same datatype so we can check them here
+        Object.keys(this.config).forEach((key) => {
+          if (isKeyForSupportedTags(key)) {
+            const tagsConfig = this.config[key]?.tags;
+            if (tagsConfig) {
+              Object.values(tagsConfig).forEach((value) => {
+                if (value) {
+                  needsWarning = true;
+                }
+              });
+            }
+          }
+        });
       }
+
+      if (needsWarning) return [`Natspec is missing`];
     }
 
     // Validate the completeness of the documentation
     let alerts: string[] = [];
+    let isDevTagForced: boolean;
+    let isNoticeTagForced: boolean;
 
     if (node instanceof EnumDefinition) {
       // TODO: Process enums
     } else if (node instanceof ErrorDefinition) {
-      alerts = [...alerts, ...this.validateParameters(node, natspecParams)];
+      isDevTagForced = this.config.errors.tags.dev;
+      isNoticeTagForced = this.config.errors.tags.notice;
+
+      alerts = [
+        ...alerts,
+        ...this.validateParameters(node, natspecParams, 'errors'),
+        ...this.validateTags(isDevTagForced, isNoticeTagForced, natspec.tags),
+      ];
     } else if (node instanceof EventDefinition) {
-      alerts = [...alerts, ...this.validateParameters(node, natspecParams)];
+      isDevTagForced = this.config.events.tags.dev;
+      isNoticeTagForced = this.config.events.tags.notice;
+
+      alerts = [
+        ...alerts,
+        ...this.validateParameters(node, natspecParams, 'events'),
+        ...this.validateTags(isDevTagForced, isNoticeTagForced, natspec.tags),
+      ];
     } else if (node instanceof FunctionDefinition) {
       const natspecReturns = natspec.returns.map((p) => p.name);
+      isDevTagForced = this.config.functions[node.visibility as keyof Functions]?.tags.dev;
+      isNoticeTagForced = this.config.functions[node.visibility as keyof Functions]?.tags.notice;
+
       alerts = [
         ...alerts,
         ...this.validateParameters(node, natspecParams),
         ...this.validateReturnParameters(node, natspecReturns),
-        ...this.validateTags(node, natspec.tags),
+        ...this.validateTags(isDevTagForced, isNoticeTagForced, natspec.tags),
       ];
     } else if (node instanceof ModifierDefinition) {
-      alerts = [...alerts, ...this.validateParameters(node, natspecParams)];
+      isDevTagForced = this.config.modifiers.tags.dev;
+      isNoticeTagForced = this.config.modifiers.tags.notice;
+
+      alerts = [
+        ...alerts,
+        ...this.validateParameters(node, natspecParams, 'modifiers'),
+        ...this.validateTags(isDevTagForced, isNoticeTagForced, natspec.tags),
+      ];
     } else if (node instanceof StructDefinition) {
-      alerts = [...alerts, ...this.validateMembers(node, natspecParams)];
+      isDevTagForced = this.config.structs.tags.dev;
+      isNoticeTagForced = this.config.structs.tags.notice;
+
+      alerts = [...alerts, ...this.validateMembers(node, natspecParams), ...this.validateTags(isDevTagForced, isNoticeTagForced, natspec.tags)];
     } else if (node instanceof VariableDeclaration) {
       // Only the presence of a notice is validated
     }
@@ -105,7 +154,11 @@ export class Validator {
    * @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[] {
+  private validateParameters<T extends HasVParameters>(
+    node: T,
+    natspecParams: (string | undefined)[],
+    key: KeysForSupportedTags | undefined = undefined
+  ): string[] {
     let definedParameters = node.vParameters.vParameters.map((p) => p.name);
     let alerts: string[] = [];
     const counter = getElementFrequency(natspecParams);
@@ -114,6 +167,10 @@ export class Validator {
       if (!this.config.functions[node.visibility as keyof Functions]?.tags.param) {
         return [];
       }
+    } else if (key !== undefined) {
+      if (!this.config[key]?.tags.param) {
+        return [];
+      }
     }
 
     for (let paramName of definedParameters) {
@@ -134,6 +191,10 @@ export class Validator {
    * @returns {string[]} - The list of alerts
    */
   private validateMembers(node: StructDefinition, natspecParams: (string | undefined)[]): string[] {
+    if (!this.config.structs.tags.param) {
+      return [];
+    }
+
     let members = node.vMembers.map((p) => p.name);
     let alerts: string[] = [];
     const counter = getElementFrequency(natspecParams);
@@ -179,10 +240,7 @@ export class Validator {
     return alerts;
   }
 
-  private validateTags(node: FunctionDefinition, natspecTags: NatspecDefinition[]): string[] {
-    const isDevTagForced = this.config.functions[node.visibility as keyof Functions]?.tags.dev;
-    const isNoticeTagForced = this.config.functions[node.visibility as keyof Functions]?.tags.notice;
-
+  private validateTags(isDevTagForced: boolean, isNoticeTagForced: boolean, natspecTags: NatspecDefinition[]): string[] {
     // If both are disabled no warnings should emit so we dont need to check anything
     if (!isDevTagForced && !isNoticeTagForced) {
       return [];

diff --git a/test/utils.test.ts b/test/utils.test.ts
@@ -205,6 +205,34 @@ describe('Utils', () => {
         exclude: '',
         inheritdoc: true,
         functions: defaultFunctions,
+        modifiers: {
+          tags: {
+            dev: false,
+            notice: true,
+            param: true,
+          },
+        },
+        events: {
+          tags: {
+            dev: false,
+            notice: true,
+            param: true,
+          },
+        },
+        errors: {
+          tags: {
+            dev: false,
+            notice: true,
+            param: true,
+          },
+        },
+        structs: {
+          tags: {
+            dev: false,
+            notice: true,
+            param: true,
+          },
+        },
         constructorNatspec: false,
       });
     });
@@ -236,6 +264,34 @@ describe('Utils', () => {
         exclude: './contracts/ignored.sol',
         inheritdoc: false,
         functions: defaultFunctions,
+        modifiers: {
+          tags: {
+            dev: false,
+            notice: true,
+            param: true,
+          },
+        },
+        events: {
+          tags: {
+            dev: false,
+            notice: true,
+            param: true,
+          },
+        },
+        errors: {
+          tags: {
+            dev: false,
+            notice: true,
+            param: true,
+          },
+        },
+        structs: {
+          tags: {
+            dev: false,
+            notice: true,
+            param: true,
+          },
+        },
         constructorNatspec: false,
       });
     });
@@ -298,6 +354,34 @@ describe('Utils', () => {
             },
           },
         },
+        modifiers: {
+          tags: {
+            dev: false,
+            notice: true,
+            param: true,
+          },
+        },
+        events: {
+          tags: {
+            dev: false,
+            notice: true,
+            param: true,
+          },
+        },
+        errors: {
+          tags: {
+            dev: false,
+            notice: true,
+            param: true,
+          },
+        },
+        structs: {
+          tags: {
+            dev: false,
+            notice: true,
+            param: true,
+          },
+        },
         constructorNatspec: true,
       });
     });