Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resolveGlobalArguments implementation & testing #5414

Merged
merged 8 commits into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 55 additions & 8 deletions v-next/core/src/internal/global-options.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import type { ParameterTypeToValueType } from "../types/common.js";
import type {
ParameterTypeToValueType,
ParameterValue,
} from "../types/common.js";
import type {
GlobalOptions,
GlobalOption,
Expand All @@ -7,21 +10,24 @@ import type {
import type { HardhatPlugin } from "../types/plugins.js";

import { HardhatError } from "@nomicfoundation/hardhat-errors";
import { camelToSnakeCase } from "@nomicfoundation/hardhat-utils/string";

import { ParameterType } from "../types/common.js";

import {
RESERVED_PARAMETER_NAMES,
isParameterValueValid,
isValidParamNameCasing,
parseParameterValue,
} from "./parameters.js";

/**
* Builds a map of the global options, validating them.
* Builds a map of the global option definitions by going through all the
* plugins and validating the global options they define.
*
* Note: this function can be used before initializing the HRE, so the plugins
* shouldn't be consider validated. Hence, we should validate the global
* parameters.
* options.
*/
export function buildGlobalOptionsMap(
resolvedPlugins: HardhatPlugin[],
Expand Down Expand Up @@ -60,6 +66,10 @@ export function buildGlobalOptionsMap(
return globalOptionsMap;
}

/**
* Builds a global option definition, validating the name, type, and default
* value.
*/
export function buildGlobalOptionDefinition<T extends ParameterType>({
name,
description,
Expand Down Expand Up @@ -104,13 +114,50 @@ export function buildGlobalOptionDefinition<T extends ParameterType>({
};
}

/**
* Resolves global options by merging user-provided options with environment
* variables, adhering to predefined global option definitions. This function
* ensures that only options specified in the globalOptionsMap are considered.
* Each option is validated against its definition in the map, with
* user-provided options taking precedence over environment variables. If an
* option is not provided by the user or set as an environment variable, its
* default value (as specified in the globalOptionsMap) is used.
*
* @param userProvidedGlobalOptions The options explicitly provided by the
* user. These take precedence over equivalent environment variables.
* @param globalOptionsMap A map defining valid global options, their default
* values, and expected types. This map is used to validate and parse the options.
* @returns {GlobalOptions} An object containing the resolved global options,
* with each option adhering to its definition in the globalOptionsMap.
* @throws {HardhatError} with descriptor
* {@link HardhatError.ERRORS.ARGUMENTS.INVALID_VALUE_FOR_TYPE} if a user-provided
* option has an invalid value for its type.
*/
export function resolveGlobalOptions(
userProvidedGlobalOptions: Partial<GlobalOptions>,
_globalOptionsMap: GlobalOptionsMap,
globalOptionsMap: GlobalOptionsMap,
): GlobalOptions {
// TODO: Validate the userProvidedGlobalOptions and get the remaining ones
// from env variables
const globalOptions: GlobalOptions = {};
// iterate over the definitions to parse and validate the arguments
for (const [name, { option }] of globalOptionsMap) {
let value =
/* eslint-disable-next-line @typescript-eslint/consistent-type-assertions
-- GlobalOptions is empty for user extension, so we need to cast it to
assign the value. */
(userProvidedGlobalOptions as Record<string, string | undefined>)[name];
if (value === undefined) {
value = process.env[`HARDHAT_${camelToSnakeCase(name).toUpperCase()}`];
}

let parsedValue: ParameterValue;
if (value !== undefined) {
parsedValue = parseParameterValue(value, option.parameterType, name);
} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is trying to parse the userProvidedGlobalOptions's values.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if those are provided, they should already be parsed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 665a755

parsedValue = option.defaultValue;
}

globalOptions[name] = parsedValue;
}

// eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- TODO
return userProvidedGlobalOptions as GlobalOptions;
return globalOptions;
}
100 changes: 100 additions & 0 deletions v-next/core/src/internal/parameters.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
import type { ParameterValue } from "../types/common.js";

import { HardhatError } from "@nomicfoundation/hardhat-errors";

import { ParameterType } from "../types/common.js";

/**
Expand Down Expand Up @@ -54,3 +58,99 @@ const parameterTypeValidators: Record<
[ParameterType.FLOAT]: (value): value is number => typeof value === "number",
[ParameterType.FILE]: (value): value is string => typeof value === "string",
};

/**
* Parses a parameter value from a string to the corresponding type.
*/
// TODO: this code is duplicated in v-next/hardhat/src/internal/cli/main.ts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think exposing these things from core makes sense. We already expose functions like this. e.g. resolvePluginList.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I'll do it on a separate PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue created: #5431

// we should move it to a shared place and add tests
export function parseParameterValue(
value: string,
type: ParameterType,
name: string,
): ParameterValue {
switch (type) {
case ParameterType.STRING:
case ParameterType.FILE:
return value;
case ParameterType.INT:
return validateAndParseInt(name, value);
case ParameterType.FLOAT:
return validateAndParseFloat(name, value);
case ParameterType.BIGINT:
return validateAndParseBigInt(name, value);
case ParameterType.BOOLEAN:
return validateAndParseBoolean(name, value);
}
}

function validateAndParseInt(name: string, value: string): number {
const decimalPattern = /^\d+(?:[eE]\d+)?$/;
const hexPattern = /^0[xX][\dABCDEabcde]+$/;

if (!decimalPattern.test(value) && !hexPattern.test(value)) {
throw new HardhatError(
HardhatError.ERRORS.ARGUMENTS.INVALID_VALUE_FOR_TYPE,
{
value,
name,
type: ParameterType.INT,
},
);
}

return Number(value);
}

function validateAndParseFloat(name: string, value: string): number {
const decimalPattern = /^(?:\d+(?:\.\d*)?|\.\d+)(?:[eE]\d+)?$/;
const hexPattern = /^0[xX][\dABCDEabcde]+$/;

if (!decimalPattern.test(value) && !hexPattern.test(value)) {
throw new HardhatError(
HardhatError.ERRORS.ARGUMENTS.INVALID_VALUE_FOR_TYPE,
{
value,
name,
type: ParameterType.FLOAT,
},
);
}

return Number(value);
}

function validateAndParseBigInt(name: string, value: string): bigint {
const decimalPattern = /^\d+(?:n)?$/;
const hexPattern = /^0[xX][\dABCDEabcde]+$/;

if (!decimalPattern.test(value) && !hexPattern.test(value)) {
throw new HardhatError(
HardhatError.ERRORS.ARGUMENTS.INVALID_VALUE_FOR_TYPE,
{
value,
name,
type: ParameterType.BIGINT,
},
);
}

return BigInt(value.replace("n", ""));
}

function validateAndParseBoolean(name: string, value: string): boolean {
const normalizedValue = value.toLowerCase();

if (normalizedValue !== "true" && normalizedValue !== "false") {
throw new HardhatError(
HardhatError.ERRORS.ARGUMENTS.INVALID_VALUE_FOR_TYPE,
{
value,
name,
type: ParameterType.BOOLEAN,
},
);
}

return normalizedValue === "true";
}
8 changes: 6 additions & 2 deletions v-next/core/src/types/global-options.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import type { ParameterType, ParameterTypeToValueType } from "./common.js";
import type {
ParameterType,
ParameterTypeToValueType,
ParameterValue,
} from "./common.js";

/**
* A global option with an associated value and a default if not provided by
Expand All @@ -25,7 +29,7 @@ export interface GlobalOption<T extends ParameterType = ParameterType> {
* Runtime Environment.
*/
// eslint-disable-next-line @typescript-eslint/no-empty-interface -- To be used through module augmentation
export interface GlobalOptions {}
export interface GlobalOptions extends Record<string, ParameterValue> {}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will be a bit annoying, as you can do globalOptions.thisDoesntExist and you wouldn't get a type error

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What motivated it? We may find an alternative

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remember correctly, this was done to avoid an error when assigning the parsed value to the global options. As the interface is empty, this throws a TS error: globalOptions[name] = parsedValue;

Element implicitly has an 'any' type because expression of type 'string' can't be used to index type 'GlobalOptions'.
No index signature with a parameter of type 'string' was found on type 'GlobalOptions'.ts(7053)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 22abbd5

/**
* An entry in the global options map.
Expand Down
Loading
Loading