Task fixes

v-next/core/src/internal/tasks/builders.ts

@@ -18,13 +18,14 @@ import { HardhatError } from "@ignored/hardhat-vnext-errors";
 
 import { ArgumentType } from "../../types/arguments.js";
 import { TaskDefinitionType } from "../../types/tasks.js";
-import {
-  RESERVED_ARGUMENT_NAMES,
-  isArgumentValueValid,
-  isArgumentNameValid,
-} from "../arguments.js";
 
-import { formatTaskId, isValidActionUrl } from "./utils.js";
+import { formatTaskId } from "./utils.js";
+import {
+  validateAction,
+  validateId,
+  validateOption,
+  validatePositionalArgument,
+} from "./validations.js";
 
 export class EmptyTaskDefinitionBuilderImplementation
   implements EmptyTaskDefinitionBuilder
@@ -34,11 +35,7 @@ export class EmptyTaskDefinitionBuilderImplementation
   #description: string;
 
   constructor(id: string | string[], description: string = "") {
-    if (id.length === 0) {
-      throw new HardhatError(
-        HardhatError.ERRORS.TASK_DEFINITIONS.EMPTY_TASK_ID,
-      );
-    }
+    validateId(id);
 
     this.#id = Array.isArray(id) ? id : [id];
     this.#description = description;
@@ -72,11 +69,7 @@ export class NewTaskDefinitionBuilderImplementation
   #action?: NewTaskActionFunction | string;
 
   constructor(id: string | string[], description: string = "") {
-    if (id.length === 0) {
-      throw new HardhatError(
-        HardhatError.ERRORS.TASK_DEFINITIONS.EMPTY_TASK_ID,
-      );
-    }
+    validateId(id);
 
     this.#id = Array.isArray(id) ? id : [id];
     this.#description = description;
@@ -88,14 +81,7 @@ export class NewTaskDefinitionBuilderImplementation
   }
 
   public setAction(action: NewTaskActionFunction | string): this {
-    if (typeof action === "string" && !isValidActionUrl(action)) {
-      throw new HardhatError(
-        HardhatError.ERRORS.TASK_DEFINITIONS.INVALID_FILE_ACTION,
-        {
-          action,
-        },
-      );
-    }
+    validateAction(action);
 
     this.#action = action;
 
@@ -115,45 +101,17 @@ export class NewTaskDefinitionBuilderImplementation
   }): this {
     const argumentType = type ?? ArgumentType.STRING;
 
-    if (!isArgumentNameValid(name)) {
-      throw new HardhatError(HardhatError.ERRORS.ARGUMENTS.INVALID_NAME, {
-        name,
-      });
-    }
-
-    if (this.#usedNames.has(name)) {
-      throw new HardhatError(HardhatError.ERRORS.ARGUMENTS.DUPLICATED_NAME, {
-        name,
-      });
-    }
-
-    if (RESERVED_ARGUMENT_NAMES.has(name)) {
-      throw new HardhatError(HardhatError.ERRORS.ARGUMENTS.RESERVED_NAME, {
-        name,
-      });
-    }
-
-    if (!isArgumentValueValid(argumentType, defaultValue)) {
-      throw new HardhatError(
-        HardhatError.ERRORS.TASK_DEFINITIONS.INVALID_VALUE_FOR_TYPE,
-        {
-          value: defaultValue,
-          name: "defaultValue",
-          type: argumentType,
-          task: formatTaskId(this.#id),
-        },
-      );
-    }
-
-    this.#usedNames.add(name);
-
-    this.#options[name] = {
+    const optionDefinition = {
       name,
       description,
       type: argumentType,
       defaultValue,
     };
 
+    validateOption(optionDefinition, this.#usedNames, this.#id);
+
+    this.#options[name] = optionDefinition;
+
     return this;
   }
 
@@ -223,69 +181,23 @@ export class NewTaskDefinitionBuilderImplementation
   }): this {
     const argumentType = type ?? ArgumentType.STRING;
 
-    if (!isArgumentNameValid(name)) {
-      throw new HardhatError(HardhatError.ERRORS.ARGUMENTS.INVALID_NAME, {
-        name,
-      });
-    }
-
-    if (this.#usedNames.has(name)) {
-      throw new HardhatError(HardhatError.ERRORS.ARGUMENTS.DUPLICATED_NAME, {
-        name,
-      });
-    }
-
-    if (RESERVED_ARGUMENT_NAMES.has(name)) {
-      throw new HardhatError(HardhatError.ERRORS.ARGUMENTS.RESERVED_NAME, {
-        name,
-      });
-    }
-
-    if (defaultValue !== undefined) {
-      if (!isArgumentValueValid(argumentType, defaultValue, isVariadic)) {
-        throw new HardhatError(
-          HardhatError.ERRORS.TASK_DEFINITIONS.INVALID_VALUE_FOR_TYPE,
-          {
-            value: defaultValue,
-            name: "defaultValue",
-            type: argumentType,
-            task: formatTaskId(this.#id),
-          },
-        );
-      }
-    }
-
-    if (this.#positionalArgs.length > 0) {
-      const lastArg = this.#positionalArgs[this.#positionalArgs.length - 1];
-
-      if (lastArg.isVariadic) {
-        throw new HardhatError(
-          HardhatError.ERRORS.TASK_DEFINITIONS.POSITIONAL_ARG_AFTER_VARIADIC,
-          {
-            name,
-          },
-        );
-      }
-
-      if (lastArg.defaultValue !== undefined && defaultValue === undefined) {
-        throw new HardhatError(
-          HardhatError.ERRORS.TASK_DEFINITIONS.REQUIRED_ARG_AFTER_OPTIONAL,
-          {
-            name,
-          },
-        );
-      }
-    }
-
-    this.#usedNames.add(name);
-
-    this.#positionalArgs.push({
+    const positionalArgDef = {
       name,
       description,
       type: argumentType,
       defaultValue,
       isVariadic,
-    });
+    };
+
+    const lastArg = this.#positionalArgs.at(-1);
+    validatePositionalArgument(
+      positionalArgDef,
+      this.#usedNames,
+      this.#id,
+      lastArg,
+    );
+
+    this.#positionalArgs.push(positionalArgDef);
 
     return this;
   }
@@ -303,11 +215,7 @@ export class TaskOverrideDefinitionBuilderImplementation
   #action?: TaskOverrideActionFunction | string;
 
   constructor(id: string | string[]) {
-    if (id.length === 0) {
-      throw new HardhatError(
-        HardhatError.ERRORS.TASK_DEFINITIONS.EMPTY_TASK_ID,
-      );
-    }
+    validateId(id);
 
     this.#id = Array.isArray(id) ? id : [id];
   }
@@ -318,14 +226,7 @@ export class TaskOverrideDefinitionBuilderImplementation
   }
 
   public setAction(action: TaskOverrideActionFunction | string): this {
-    if (typeof action === "string" && !isValidActionUrl(action)) {
-      throw new HardhatError(
-        HardhatError.ERRORS.TASK_DEFINITIONS.INVALID_FILE_ACTION,
-        {
-          action,
-        },
-      );
-    }
+    validateAction(action);
 
     this.#action = action;
 
@@ -345,43 +246,21 @@ export class TaskOverrideDefinitionBuilderImplementation
   }): this {
     const argumentType = type ?? ArgumentType.STRING;
 
-    if (!isArgumentNameValid(name)) {
-      throw new HardhatError(HardhatError.ERRORS.ARGUMENTS.INVALID_NAME, {
-        name,
-      });
-    }
-
-    if (name in this.#options) {
-      throw new HardhatError(HardhatError.ERRORS.ARGUMENTS.DUPLICATED_NAME, {
-        name,
-      });
-    }
-
-    if (RESERVED_ARGUMENT_NAMES.has(name)) {
-      throw new HardhatError(HardhatError.ERRORS.ARGUMENTS.RESERVED_NAME, {
-        name,
-      });
-    }
-
-    if (!isArgumentValueValid(argumentType, defaultValue)) {
-      throw new HardhatError(
-        HardhatError.ERRORS.TASK_DEFINITIONS.INVALID_VALUE_FOR_TYPE,
-        {
-          value: defaultValue,
-          name: "defaultValue",
-          type: argumentType,
-          task: formatTaskId(this.#id),
-        },
-      );
-    }
-
-    this.#options[name] = {
+    const optionDefinition = {
       name,
       description,
       type: argumentType,
       defaultValue,
     };
 
+    validateOption(
+      optionDefinition,
+      new Set(Object.keys(this.#options)),
+      this.#id,
+    );
+
+    this.#options[name] = optionDefinition;
+
     return this;
   }
 

v-next/core/src/internal/tasks/resolved-task.ts

@@ -100,34 +100,29 @@ export class ResolvedTask implements Task {
     }
 
     const providedArgumentNames = new Set(Object.keys(taskArguments));
-
-    // Validate and resolve the task options
-    for (const option of this.options.values()) {
-      const value = taskArguments[option.name];
-
-      this.#validateArgumentType(option, value);
-
-      // resolve defaults for optional arguments
-      if (value === undefined) {
-        taskArguments[option.name] = option.defaultValue;
+    const argumentDefinitions = [
+      ...this.options.values(),
+      ...this.positionalArguments,
+    ];
+    const validatedTaskArguments: TaskArguments = {};
+    for (const argumentDefinition of argumentDefinitions) {
+      const value = taskArguments[argumentDefinition.name];
+      const isPositional = "isVariadic" in argumentDefinition;
+
+      if (isPositional) {
+        this.#validateRequiredArgument(argumentDefinition, value);
       }
-
-      providedArgumentNames.delete(option.name);
-    }
-
-    // Validate and resolve the task positional arguments
-    for (const argument of this.positionalArguments) {
-      const value = taskArguments[argument.name];
-
-      this.#validateRequiredArgument(argument, value);
-      this.#validateArgumentType(argument, value, argument.isVariadic);
+      this.#validateArgumentType(
+        argumentDefinition,
+        value,
+        isPositional && argumentDefinition.isVariadic,
+      );
 
       // resolve defaults for optional arguments
-      if (value === undefined && argument.defaultValue !== undefined) {
-        taskArguments[argument.name] = argument.defaultValue;
-      }
+      validatedTaskArguments[argumentDefinition.name] =
+        value ?? argumentDefinition.defaultValue;
 
-      providedArgumentNames.delete(argument.name);
+      providedArgumentNames.delete(argumentDefinition.name);
     }
 
     // At this point, the set should be empty as all the task arguments have
@@ -166,7 +161,7 @@ export class ResolvedTask implements Task {
       );
     };
 
-    return next(taskArguments);
+    return next(validatedTaskArguments);
   }
 
   /**

v-next/core/src/internal/tasks/task-manager.ts

@@ -1,3 +1,4 @@
+import type { PositionalArgumentDefinition } from "../../types/arguments.js";
 import type { GlobalOptionDefinitions } from "../../types/global-options.js";
 import type { HardhatRuntimeEnvironment } from "../../types/hre.js";
 import type {
@@ -17,6 +18,12 @@ import { TaskDefinitionType } from "../../types/tasks.js";
 
 import { ResolvedTask } from "./resolved-task.js";
 import { formatTaskId, getActorFragment } from "./utils.js";
+import {
+  validateAction,
+  validateId,
+  validateOption,
+  validatePositionalArgument,
+} from "./validations.js";
 
 export class TaskManagerImplementation implements TaskManager {
   readonly #hre: HardhatRuntimeEnvironment;
@@ -35,6 +42,7 @@ export class TaskManagerImplementation implements TaskManager {
       }
 
       for (const taskDefinition of plugin.tasks) {
+        this.#validateTaskDefinition(taskDefinition);
         this.#reduceTaskDefinition(
           globalOptionDefinitions,
           taskDefinition,
@@ -45,6 +53,7 @@ export class TaskManagerImplementation implements TaskManager {
 
     // reduce global user defined tasks
     for (const taskDefinition of this.#hre.config.tasks) {
+      this.#validateTaskDefinition(taskDefinition);
       this.#reduceTaskDefinition(globalOptionDefinitions, taskDefinition);
     }
   }
@@ -243,4 +252,37 @@ export class TaskManagerImplementation implements TaskManager {
 
     task.actions.push({ pluginId, action: taskDefinition.action });
   }
+
+  #validateTaskDefinition(taskDefinition: TaskDefinition): void {
+    validateId(taskDefinition.id);
+
+    // Empty tasks don't have actions, options, or positional arguments
+    if (taskDefinition.type === TaskDefinitionType.EMPTY_TASK) {
+      return;
+    }
+
+    const usedNames = new Set<string>();
+
+    validateAction(taskDefinition.action);
+
+    Object.values(taskDefinition.options).forEach((optionDefinition) =>
+      validateOption(optionDefinition, usedNames, taskDefinition.id),
+    );
+
+    // Override tasks don't have positional arguments
+    if (taskDefinition.type === TaskDefinitionType.TASK_OVERRIDE) {
+      return;
+    }
+
+    let lastArg: PositionalArgumentDefinition;
+    taskDefinition.positionalArguments.forEach((posArgDefinition) => {
+      validatePositionalArgument(
+        posArgDefinition,
+        usedNames,
+        taskDefinition.id,
+        lastArg,
+      );
+      lastArg = posArgDefinition;
+    });
+  }
 }

v-next/core/src/internal/tasks/utils.ts

@@ -9,9 +9,3 @@ export function formatTaskId(taskId: string | string[]): string {
 export function getActorFragment(pluginId: string | undefined): string {
   return pluginId !== undefined ? `Plugin ${pluginId} is` : "You are";
 }
-
-const FILE_PROTOCOL_PATTERN = /^file:\/\/.+/;
-
-export function isValidActionUrl(action: string): boolean {
-  return FILE_PROTOCOL_PATTERN.test(action);
-}

v-next/core/src/internal/tasks/validations.ts

@@ -0,0 +1,151 @@
+import type {
+  ArgumentType,
+  ArgumentValue,
+  OptionDefinition,
+  PositionalArgumentDefinition,
+} from "../../types/arguments.js";
+
+import { HardhatError } from "@ignored/hardhat-vnext-errors";
+
+import {
+  isArgumentNameValid,
+  isArgumentValueValid,
+  RESERVED_ARGUMENT_NAMES,
+} from "../arguments.js";
+
+import { formatTaskId } from "./utils.js";
+
+export function validateId(id: string | string[]): void {
+  if (id.length === 0) {
+    throw new HardhatError(HardhatError.ERRORS.TASK_DEFINITIONS.EMPTY_TASK_ID);
+  }
+}
+
+export function validateAction(action: unknown): void {
+  if (typeof action === "string" && !isValidActionUrl(action)) {
+    throw new HardhatError(
+      HardhatError.ERRORS.TASK_DEFINITIONS.INVALID_FILE_ACTION,
+      {
+        action,
+      },
+    );
+  }
+}
+
+export function validateOption(
+  optionDefinition: OptionDefinition,
+  usedNames: Set<string>,
+  taskId: string | string[],
+): void {
+  validateArgumentName(optionDefinition.name);
+
+  if (usedNames.has(optionDefinition.name)) {
+    throw new HardhatError(HardhatError.ERRORS.ARGUMENTS.DUPLICATED_NAME, {
+      name: optionDefinition.name,
+    });
+  }
+
+  validateArgumentValue({
+    name: "defaultValue",
+    value: optionDefinition.defaultValue,
+    expectedType: optionDefinition.type,
+    taskId: formatTaskId(taskId),
+  });
+
+  usedNames.add(optionDefinition.name);
+}
+
+export function validatePositionalArgument(
+  positionalArgDef: PositionalArgumentDefinition,
+  usedNames: Set<string>,
+  taskId: string | string[],
+  lastArg?: PositionalArgumentDefinition,
+): void {
+  validateArgumentName(positionalArgDef.name);
+
+  if (usedNames.has(positionalArgDef.name)) {
+    throw new HardhatError(HardhatError.ERRORS.ARGUMENTS.DUPLICATED_NAME, {
+      name: positionalArgDef.name,
+    });
+  }
+
+  if (positionalArgDef.defaultValue !== undefined) {
+    validateArgumentValue({
+      name: "defaultValue",
+      value: positionalArgDef.defaultValue,
+      isVariadic: positionalArgDef.isVariadic,
+      expectedType: positionalArgDef.type,
+      taskId: formatTaskId(taskId),
+    });
+  }
+
+  if (lastArg !== undefined && lastArg.isVariadic) {
+    throw new HardhatError(
+      HardhatError.ERRORS.TASK_DEFINITIONS.POSITIONAL_ARG_AFTER_VARIADIC,
+      {
+        name: positionalArgDef.name,
+      },
+    );
+  }
+
+  if (
+    lastArg !== undefined &&
+    lastArg.defaultValue !== undefined &&
+    positionalArgDef.defaultValue === undefined
+  ) {
+    throw new HardhatError(
+      HardhatError.ERRORS.TASK_DEFINITIONS.REQUIRED_ARG_AFTER_OPTIONAL,
+      {
+        name: positionalArgDef.name,
+      },
+    );
+  }
+
+  usedNames.add(positionalArgDef.name);
+}
+
+const FILE_PROTOCOL_PATTERN = /^file:\/\/.+/;
+
+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),
+      },
+    );
+  }
+}

v-next/core/src/types/arguments.ts

@@ -57,7 +57,7 @@ export type ArgumentTypeToValueType<T extends ArgumentType> =
 export interface OptionDefinition<T extends ArgumentType = ArgumentType> {
   name: string;
   description: string;
-  type: ArgumentType;
+  type: T;
   defaultValue: ArgumentTypeToValueType<T>;
 }
 

v-next/core/test/internal/tasks/utils.ts

@@ -1,10 +1,7 @@
 import assert from "node:assert/strict";
 import { describe, it } from "node:test";
 
-import {
-  formatTaskId,
-  isValidActionUrl,
-} from "../../../src/internal/tasks/utils.js";
+import { formatTaskId } from "../../../src/internal/tasks/utils.js";
 
 describe("Task utils", () => {
   describe("formatTaskId", () => {
@@ -16,16 +13,4 @@ describe("Task utils", () => {
       assert.equal(formatTaskId(["foo", "bar"]), "foo bar");
     });
   });
-
-  describe("isValidActionUrl", () => {
-    it("should return true if the action is a file URL", () => {
-      assert.equal(isValidActionUrl("file://foo"), true);
-    });
-
-    it("should return false if the action is not a file URL", () => {
-      assert.equal(isValidActionUrl("http://foo"), false);
-      assert.equal(isValidActionUrl("file://"), false);
-      assert.equal(isValidActionUrl("missing-protocol"), false);
-    });
-  });
 });