Merge pull request #5532 from NomicFoundation/refactor-validations

schaable · web-flow · commit 02730b048982 · 2024-07-18T12:16:56.000-03:00
Refactor argument validations
diff --git a/v-next/core/src/internal/arguments.ts b/v-next/core/src/internal/arguments.ts
@@ -12,13 +12,66 @@ export const RESERVED_ARGUMENT_NAMES: Set<string> = new Set([]);
 
 const VALID_ARGUMENT_NAME_PATTERN = /^[a-z][a-zA-Z0-9]*$/;
 
+/**
+ * Validates an argument name, throwing an error if it is invalid.
+ *
+ * @param name The name of the argument.
+ * @throws {HardhatError} with descriptor:
+ * - {@link HardhatError.ERRORS.ARGUMENTS.INVALID_NAME} if the name is invalid.
+ * A valid name must start with a lowercase letter and contain only
+ * alphanumeric characters.
+ * - {@link HardhatError.ERRORS.ARGUMENTS.RESERVED_NAME} if the name is
+ * reserved. See {@link RESERVED_ARGUMENT_NAMES}.
+ */
+export function validateArgumentName(name: string): void {
+  if (!isArgumentNameValid(name)) {
+    throw new HardhatError(HardhatError.ERRORS.ARGUMENTS.INVALID_NAME, {
+      name,
+    });
+  }
+
+  if (RESERVED_ARGUMENT_NAMES.has(name)) {
+    throw new HardhatError(HardhatError.ERRORS.ARGUMENTS.RESERVED_NAME, {
+      name,
+    });
+  }
+}
+
 /**
  * Returns true if the given name is a valid argument name.
  */
 export function isArgumentNameValid(name: string): boolean {
   return VALID_ARGUMENT_NAME_PATTERN.test(name);
 }
 
+/**
+ * Validates an argument value, throwing an error if it is invalid.
+ *
+ * @param name The name of the argument.
+ * @param expectedType The expected type of the argument. One of {@link ArgumentType}.
+ * @param value The value of the argument.
+ * @param isVariadic Whether the argument is variadic.
+ * @throws {HardhatError} with descriptor {@link HardhatError.ERRORS.ARGUMENTS.INVALID_VALUE_FOR_TYPE}
+ * if the value is invalid for the expected type.
+ */
+export function validateArgumentValue(
+  name: string,
+  expectedType: ArgumentType,
+  value: ArgumentValue | ArgumentValue[],
+  isVariadic: boolean = false,
+): void {
+  if (!isArgumentValueValid(expectedType, value, isVariadic)) {
+    throw new HardhatError(
+      HardhatError.ERRORS.ARGUMENTS.INVALID_VALUE_FOR_TYPE,
+      {
+        name,
+        type: expectedType,
+        value,
+      },
+    );
+  }
+}
+
 /**
  * Checks if an argument value is valid for a given argument type.
  *
diff --git a/v-next/core/src/internal/global-options.ts b/v-next/core/src/internal/global-options.ts
@@ -15,10 +15,9 @@ import { camelToSnakeCase } from "@ignored/hardhat-vnext-utils/string";
 import { ArgumentType } from "../types/arguments.js";
 
 import {
-  RESERVED_ARGUMENT_NAMES,
-  isArgumentValueValid,
-  isArgumentNameValid,
   parseArgumentValue,
+  validateArgumentValue,
+  validateArgumentName,
 } from "./arguments.js";
 
 /**
@@ -83,28 +82,9 @@ export function buildGlobalOptionDefinition<T extends ArgumentType>({
 }): OptionDefinition {
   const argumentType = type ?? ArgumentType.STRING;
 
-  if (!isArgumentNameValid(name)) {
-    throw new HardhatError(HardhatError.ERRORS.ARGUMENTS.INVALID_NAME, {
-      name,
-    });
-  }
-
-  if (RESERVED_ARGUMENT_NAMES.has(name)) {
-    throw new HardhatError(HardhatError.ERRORS.ARGUMENTS.RESERVED_NAME, {
-      name,
-    });
-  }
+  validateArgumentName(name);
 
-  if (!isArgumentValueValid(argumentType, defaultValue)) {
-    throw new HardhatError(
-      HardhatError.ERRORS.ARGUMENTS.INVALID_VALUE_FOR_TYPE,
-      {
-        value: defaultValue,
-        name: "defaultValue",
-        type,
-      },
-    );
-  }
+  validateArgumentValue("defaultValue", argumentType, defaultValue);
 
   return {
     name,
diff --git a/v-next/core/src/internal/tasks/resolved-task.ts b/v-next/core/src/internal/tasks/resolved-task.ts
@@ -18,10 +18,10 @@ import {
 } from "@ignored/hardhat-vnext-errors";
 import { ensureError } from "@ignored/hardhat-vnext-utils/error";
 
-import { isArgumentValueValid } from "../arguments.js";
 import { detectPluginNpmDependencyProblems } from "../plugins/detect-plugin-npm-dependency-problems.js";
 
 import { formatTaskId } from "./utils.js";
+import { validateTaskArgumentValue } from "./validations.js";
 
 export class ResolvedTask implements Task {
   readonly #hre: HardhatRuntimeEnvironment;
@@ -112,11 +112,16 @@ export class ResolvedTask implements Task {
       if (isPositional) {
         this.#validateRequiredArgument(argumentDefinition, value);
       }
-      this.#validateArgumentType(
-        argumentDefinition,
-        value,
-        isPositional && argumentDefinition.isVariadic,
-      );
+
+      if (value !== undefined) {
+        validateTaskArgumentValue(
+          argumentDefinition.name,
+          argumentDefinition.type,
+          value,
+          isPositional && argumentDefinition.isVariadic,
+          this.id,
+        );
+      }
 
       // resolve defaults for optional arguments
       validatedTaskArguments[argumentDefinition.name] =
@@ -185,37 +190,6 @@ export class ResolvedTask implements Task {
     }
   }
 
-  /**
-   * Validates that a argument has the correct type. If the argument is optional
-   * and doesn't have a value, the type is not validated as it will be resolved
-   * to the default value.
-   *
-   * @throws HardhatError if the argument has an invalid type.
-   */
-  #validateArgumentType(
-    argument: OptionDefinition | PositionalArgumentDefinition,
-    value: ArgumentValue | ArgumentValue[],
-    isVariadic: boolean = false,
-  ) {
-    // skip type validation for optional arguments with undefined value
-    if (value === undefined && argument.defaultValue !== undefined) {
-      return;
-    }
-
-    // check if the value is valid for the argument type
-    if (!isArgumentValueValid(argument.type, value, isVariadic)) {
-      throw new HardhatError(
-        HardhatError.ERRORS.TASK_DEFINITIONS.INVALID_VALUE_FOR_TYPE,
-        {
-          value,
-          name: argument.name,
-          type: argument.type,
-          task: formatTaskId(this.id),
-        },
-      );
-    }
-  }
-
   /**
    * Validates that no extra arguments were provided in the task arguments.
    *
diff --git a/v-next/core/src/internal/tasks/validations.ts b/v-next/core/src/internal/tasks/validations.ts
@@ -7,11 +7,7 @@ import type {
 
 import { HardhatError } from "@ignored/hardhat-vnext-errors";
 
-import {
-  isArgumentNameValid,
-  isArgumentValueValid,
-  RESERVED_ARGUMENT_NAMES,
-} from "../arguments.js";
+import { validateArgumentName, validateArgumentValue } from "../arguments.js";
 
 import { formatTaskId } from "./utils.js";
 
@@ -33,75 +29,70 @@ export function validateAction(action: unknown): void {
 }
 
 export function validateOption(
-  optionDefinition: OptionDefinition,
+  { name, type, defaultValue }: OptionDefinition,
   usedNames: Set<string>,
   taskId: string | string[],
 ): void {
-  validateArgumentName(optionDefinition.name);
+  validateArgumentName(name);
 
-  if (usedNames.has(optionDefinition.name)) {
+  if (usedNames.has(name)) {
     throw new HardhatError(HardhatError.ERRORS.ARGUMENTS.DUPLICATED_NAME, {
-      name: optionDefinition.name,
+      name,
     });
   }
 
-  validateArgumentValue({
-    name: "defaultValue",
-    value: optionDefinition.defaultValue,
-    expectedType: optionDefinition.type,
-    taskId: formatTaskId(taskId),
-  });
+  validateTaskArgumentValue("defaultValue", type, defaultValue, false, taskId);
 
-  usedNames.add(optionDefinition.name);
+  usedNames.add(name);
 }
 
 export function validatePositionalArgument(
-  positionalArgDef: PositionalArgumentDefinition,
+  { name, type, defaultValue, isVariadic }: PositionalArgumentDefinition,
   usedNames: Set<string>,
   taskId: string | string[],
   lastArg?: PositionalArgumentDefinition,
 ): void {
-  validateArgumentName(positionalArgDef.name);
+  validateArgumentName(name);
 
-  if (usedNames.has(positionalArgDef.name)) {
+  if (usedNames.has(name)) {
     throw new HardhatError(HardhatError.ERRORS.ARGUMENTS.DUPLICATED_NAME, {
-      name: positionalArgDef.name,
+      name,
     });
   }
 
-  if (positionalArgDef.defaultValue !== undefined) {
-    validateArgumentValue({
-      name: "defaultValue",
-      value: positionalArgDef.defaultValue,
-      isVariadic: positionalArgDef.isVariadic,
-      expectedType: positionalArgDef.type,
-      taskId: formatTaskId(taskId),
-    });
+  if (defaultValue !== undefined) {
+    validateTaskArgumentValue(
+      "defaultValue",
+      type,
+      defaultValue,
+      isVariadic,
+      taskId,
+    );
   }
 
   if (lastArg !== undefined && lastArg.isVariadic) {
     throw new HardhatError(
       HardhatError.ERRORS.TASK_DEFINITIONS.POSITIONAL_ARG_AFTER_VARIADIC,
       {
-        name: positionalArgDef.name,
+        name,
       },
     );
   }
 
   if (
     lastArg !== undefined &&
     lastArg.defaultValue !== undefined &&
-    positionalArgDef.defaultValue === undefined
+    defaultValue === undefined
   ) {
     throw new HardhatError(
       HardhatError.ERRORS.TASK_DEFINITIONS.REQUIRED_ARG_AFTER_OPTIONAL,
       {
-        name: positionalArgDef.name,
+        name,
       },
     );
   }
 
-  usedNames.add(positionalArgDef.name);
+  usedNames.add(name);
 }
 
 const FILE_PROTOCOL_PATTERN = /^file:\/\/.+/;
@@ -110,42 +101,33 @@ function isValidActionUrl(action: string): boolean {
   return FILE_PROTOCOL_PATTERN.test(action);
 }
 
-function validateArgumentName(name: string): void {
-  if (!isArgumentNameValid(name)) {
-    throw new HardhatError(HardhatError.ERRORS.ARGUMENTS.INVALID_NAME, {
-      name,
-    });
-  }
-
-  if (RESERVED_ARGUMENT_NAMES.has(name)) {
-    throw new HardhatError(HardhatError.ERRORS.ARGUMENTS.RESERVED_NAME, {
-      name,
-    });
-  }
-}
-
-function validateArgumentValue({
-  name,
-  expectedType,
-  isVariadic = false,
-  value,
-  taskId,
-}: {
-  name: string;
-  expectedType: ArgumentType;
-  isVariadic?: boolean;
-  value: ArgumentValue | ArgumentValue[];
-  taskId: string | string[];
-}): void {
-  if (!isArgumentValueValid(expectedType, value, isVariadic)) {
-    throw new HardhatError(
-      HardhatError.ERRORS.TASK_DEFINITIONS.INVALID_VALUE_FOR_TYPE,
-      {
-        value,
-        name,
-        type: expectedType,
-        task: formatTaskId(taskId),
-      },
-    );
+export function validateTaskArgumentValue(
+  name: string,
+  expectedType: ArgumentType,
+  value: ArgumentValue | ArgumentValue[],
+  isVariadic: boolean,
+  taskId: string | string[],
+): void {
+  try {
+    validateArgumentValue(name, expectedType, value, isVariadic);
+  } catch (error) {
+    if (
+      HardhatError.isHardhatError(
+        error,
+        HardhatError.ERRORS.ARGUMENTS.INVALID_VALUE_FOR_TYPE,
+      )
+    ) {
+      throw new HardhatError(
+        HardhatError.ERRORS.TASK_DEFINITIONS.INVALID_VALUE_FOR_TYPE,
+        {
+          name,
+          type: expectedType,
+          value,
+          task: formatTaskId(taskId),
+        },
+      );
+    }
+
+    throw error;
   }
 }
