Skip to content

Commit

Permalink
Do not treat comma as delimiter for exclude
Browse files Browse the repository at this point in the history
  • Loading branch information
ericglau committed Aug 23, 2024
1 parent c2692a0 commit 8c84a64
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 10 deletions.
2 changes: 1 addition & 1 deletion docs/modules/ROOT/pages/api-core.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ If any errors are found, the command will exit with a non-zero exit code and pri
* `--reference <REFERENCE_CONTRACT>` - Can only be used when the `--contract` option is also provided. The name or fully qualified name of the reference contract to use for storage layout comparisons. If not specified, uses the `@custom:oz-upgrades-from` annotation if it is defined in the contract that is being validated.
* `--requireReference` - Can only be used when the `--contract` option is also provided. Not compatible with `--unsafeSkipStorageCheck`. If specified, requires either the `--reference` option to be provided or the contract to have a `@custom:oz-upgrades-from` annotation.
* `--referenceBuildInfoDirs "<BUILD_INFO_DIR>[,<BUILD_INFO_DIR>...]"` - Optional paths of additional build info directories from previous versions of the project to use for storage layout comparisons. When using this option, refer to one of these directories using prefix `<dirName>:` before the contract name or fully qualified name in the `--reference` option or `@custom:oz-upgrades-from` annotation, where `<dirName>` is the directory short name. Each directory short name must be unique, including compared to the main build info directory. If passing in multiple directories, separate them with commas or call the option multiple times, once for each directory.
* `--exclude "<GLOB_PATTERN>[,<GLOB_PATTERN>...]"` - Exclude validations for contracts in source file paths that match the given glob pattern. For example, `--exclude "contracts/mocks/\**/*.sol"`. Does not apply to reference contracts. If passing in multiple patterns, separate them with commas or call the option multiple times, once for each pattern.
* `--exclude "<GLOB_PATTERN>" [--exclude <GLOB_PATTERN>...]` - Exclude validations for contracts in source file paths that match the given glob pattern. For example, `--exclude "contracts/mocks/\**/*.sol"`. Does not apply to reference contracts. If passing in multiple patterns, call the option multiple times, once for each pattern.
* `--unsafeAllow "<VALIDATION_ERROR>[,<VALIDATION_ERROR>...]"` - Selectively disable one or more validation errors. Comma-separated list with one or more of the following: `state-variable-assignment, state-variable-immutable, external-library-linking, struct-definition, enum-definition, constructor, delegatecall, selfdestruct, missing-public-upgradeto, internal-function-storage`
* `--unsafeAllowRenames` - Configure storage layout check to allow variable renaming.
* `--unsafeSkipStorageCheck` - Skips checking for storage layout compatibility errors. This is a dangerous option meant to be used as a last resort.
Expand Down
9 changes: 9 additions & 0 deletions packages/core/src/cli/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,15 @@ test('validate - excludes by pattern - all match', async t => {
t.snapshot(output);
});

test('validate - excludes by pattern - all match using commas within glob', async t => {
const temp = await getTempDir(t);
const buildInfo = await artifacts.getBuildInfo(`contracts/test/cli/excludes/UsesAbstractUUPS.sol:UsesAbstractUUPS`);
await fs.writeFile(path.join(temp, 'validate.json'), JSON.stringify(buildInfo));

const output = await execAsync(`${CLI} validate ${temp} --exclude "**/excludes/{Abstract*,UsesAbstractUUPS}.sol"`);
t.snapshot(output);
});

test('validate - exclude passed multiple times', async t => {
const temp = await getTempDir(t);
const buildInfo = await artifacts.getBuildInfo(`contracts/test/cli/excludes/UsesAbstractUUPS.sol:UsesAbstractUUPS`);
Expand Down
14 changes: 12 additions & 2 deletions packages/core/src/cli/cli.test.ts.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ Generated by [AVA](https://avajs.dev).
--reference <REFERENCE_CONTRACT> Can only be used when the --contract option is also provided. The name or fully qualified name of the reference contract to use for storage layout comparisons. If not specified, uses the @custom:oz-upgrades-from annotation if it is defined in the contract that is being validated.␊
--requireReference Can only be used when the --contract option is also provided. Not compatible with --unsafeSkipStorageCheck. If specified, requires either the --reference option to be provided or the contract to have a @custom:oz-upgrades-from annotation.␊
--referenceBuildInfoDirs "<BUILD_INFO_DIR>[,<BUILD_INFO_DIR>...]" Optional paths of additional build info directories from previous versions of the project to use for storage layout comparisons. When using this option, refer to one of these directories using prefix '<dirName>:' before the contract name or fully qualified name in the --reference option or @custom:oz-upgrades-from annotation, where <dirName> is the directory short name. Each directory short name must be unique, including compared to the main build info directory. If passing in multiple directories, separate them with commas or call the option multiple times, once for each directory.␊
--exclude "<GLOB_PATTERN>[,<GLOB_PATTERN>...]" Exclude validations for contracts in source file paths that match the given glob patterns. For example, --exclude "contracts/mocks/**/*.sol". Does not apply to reference contracts. If passing in multiple patterns, separate them with commas or call the option multiple times, once for each pattern.␊
--exclude "<GLOB_PATTERN>" [--exclude <GLOB_PATTERN>...] Exclude validations for contracts in source file paths that match the given glob patterns. For example, --exclude "contracts/mocks/**/*.sol". Does not apply to reference contracts. If passing in multiple patterns, call the option multiple times, once for each pattern.␊
--unsafeAllow "<VALIDATION_ERROR>[,<VALIDATION_ERROR>...]" Selectively disable one or more validation errors. Comma-separated list with one or more of the following: state-variable-assignment, state-variable-immutable, external-library-linking, struct-definition, enum-definition, constructor, delegatecall, selfdestruct, missing-public-upgradeto, internal-function-storage␊
--unsafeAllowRenames Configure storage layout check to allow variable renaming.␊
--unsafeSkipStorageCheck Skips checking for storage layout compatibility errors. This is a dangerous option meant to be used as a last resort.␊
Expand All @@ -42,7 +42,7 @@ Generated by [AVA](https://avajs.dev).
--reference <REFERENCE_CONTRACT> Can only be used when the --contract option is also provided. The name or fully qualified name of the reference contract to use for storage layout comparisons. If not specified, uses the @custom:oz-upgrades-from annotation if it is defined in the contract that is being validated.␊
--requireReference Can only be used when the --contract option is also provided. Not compatible with --unsafeSkipStorageCheck. If specified, requires either the --reference option to be provided or the contract to have a @custom:oz-upgrades-from annotation.␊
--referenceBuildInfoDirs "<BUILD_INFO_DIR>[,<BUILD_INFO_DIR>...]" Optional paths of additional build info directories from previous versions of the project to use for storage layout comparisons. When using this option, refer to one of these directories using prefix '<dirName>:' before the contract name or fully qualified name in the --reference option or @custom:oz-upgrades-from annotation, where <dirName> is the directory short name. Each directory short name must be unique, including compared to the main build info directory. If passing in multiple directories, separate them with commas or call the option multiple times, once for each directory.␊
--exclude "<GLOB_PATTERN>[,<GLOB_PATTERN>...]" Exclude validations for contracts in source file paths that match the given glob patterns. For example, --exclude "contracts/mocks/**/*.sol". Does not apply to reference contracts. If passing in multiple patterns, separate them with commas or call the option multiple times, once for each pattern.␊
--exclude "<GLOB_PATTERN>" [--exclude <GLOB_PATTERN>...] Exclude validations for contracts in source file paths that match the given glob patterns. For example, --exclude "contracts/mocks/**/*.sol". Does not apply to reference contracts. If passing in multiple patterns, call the option multiple times, once for each pattern.␊
--unsafeAllow "<VALIDATION_ERROR>[,<VALIDATION_ERROR>...]" Selectively disable one or more validation errors. Comma-separated list with one or more of the following: state-variable-assignment, state-variable-immutable, external-library-linking, struct-definition, enum-definition, constructor, delegatecall, selfdestruct, missing-public-upgradeto, internal-function-storage␊
--unsafeAllowRenames Configure storage layout check to allow variable renaming.␊
--unsafeSkipStorageCheck Skips checking for storage layout compatibility errors. This is a dangerous option meant to be used as a last resort.␊
Expand Down Expand Up @@ -497,6 +497,16 @@ Generated by [AVA](https://avajs.dev).

## validate - excludes by pattern - all match

> Snapshot 1
{
stderr: '',
stdout: `No upgradeable contracts detected.␊
`,
}

## validate - excludes by pattern - all match using commas within glob

> Snapshot 1
{
Expand Down
Binary file modified packages/core/src/cli/cli.test.ts.snap
Binary file not shown.
22 changes: 15 additions & 7 deletions packages/core/src/cli/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Options:
--reference <REFERENCE_CONTRACT> Can only be used when the --contract option is also provided. The name or fully qualified name of the reference contract to use for storage layout comparisons. If not specified, uses the @custom:oz-upgrades-from annotation if it is defined in the contract that is being validated.
--requireReference Can only be used when the --contract option is also provided. Not compatible with --unsafeSkipStorageCheck. If specified, requires either the --reference option to be provided or the contract to have a @custom:oz-upgrades-from annotation.
--referenceBuildInfoDirs "<BUILD_INFO_DIR>[,<BUILD_INFO_DIR>...]" Optional paths of additional build info directories from previous versions of the project to use for storage layout comparisons. When using this option, refer to one of these directories using prefix '<dirName>:' before the contract name or fully qualified name in the --reference option or @custom:oz-upgrades-from annotation, where <dirName> is the directory short name. Each directory short name must be unique, including compared to the main build info directory. If passing in multiple directories, separate them with commas or call the option multiple times, once for each directory.
--exclude "<GLOB_PATTERN>[,<GLOB_PATTERN>...]" Exclude validations for contracts in source file paths that match the given glob patterns. For example, --exclude "contracts/mocks/**/*.sol". Does not apply to reference contracts. If passing in multiple patterns, separate them with commas or call the option multiple times, once for each pattern.
--exclude "<GLOB_PATTERN>" [--exclude <GLOB_PATTERN>...] Exclude validations for contracts in source file paths that match the given glob patterns. For example, --exclude "contracts/mocks/**/*.sol". Does not apply to reference contracts. If passing in multiple patterns, call the option multiple times, once for each pattern.
--unsafeAllow "<VALIDATION_ERROR>[,<VALIDATION_ERROR>...]" Selectively disable one or more validation errors. Comma-separated list with one or more of the following: ${errorKinds.join(
', ',
)}
Expand Down Expand Up @@ -96,8 +96,8 @@ export function getFunctionArgs(parsedArgs: minimist.ParsedArgs, extraArgs: stri
const contract = getAndValidateString(parsedArgs, 'contract');
const reference = getAndValidateString(parsedArgs, 'reference');
const opts = withDefaults(parsedArgs);
const referenceBuildInfoDirs = getAndValidateStringArray(parsedArgs, 'referenceBuildInfoDirs');
const exclude = getAndValidateStringArray(parsedArgs, 'exclude');
const referenceBuildInfoDirs = getAndValidateStringArray(parsedArgs, 'referenceBuildInfoDirs', true);
const exclude = getAndValidateStringArray(parsedArgs, 'exclude', false);

if (contract === undefined) {
if (reference !== undefined) {
Expand All @@ -118,20 +118,28 @@ function getAndValidateString(parsedArgs: minimist.ParsedArgs, option: string):
return value;
}

function getAndValidateStringArray(parsedArgs: minimist.ParsedArgs, option: string): string[] | undefined {
function getAndValidateStringArray(
parsedArgs: minimist.ParsedArgs,
option: string,
useCommaDelimiter: boolean,
): string[] | undefined {
const value = parsedArgs[option];
if (value !== undefined) {
if (Array.isArray(value)) {
return value;
} else {
assertNonEmptyString(value, option);
return value.split(/[\s,]+/);
if (useCommaDelimiter) {
return value.split(/[\s,]+/);
} else {
return [value];
}
}
}
return value;
}

function assertNonEmptyString(value: string, option: string) {
function assertNonEmptyString(value: unknown, option: string) {
if (typeof value !== 'string' || value.trim().length === 0) {
throw new Error(`Invalid option: --${option} cannot be empty`);
}
Expand Down Expand Up @@ -164,7 +172,7 @@ function validateOptions(parsedArgs: minimist.ParsedArgs) {
function getUnsafeAllowKinds(parseArgs: minimist.ParsedArgs): ValidationError['kind'][] {
type errorKindsType = (typeof errorKinds)[number];

const unsafeAllowTokens: string[] = getAndValidateStringArray(parseArgs, 'unsafeAllow') ?? [];
const unsafeAllowTokens: string[] = getAndValidateStringArray(parseArgs, 'unsafeAllow', true) ?? [];
if (unsafeAllowTokens.some(token => !errorKinds.includes(token as errorKindsType))) {
throw new Error(
`Invalid option: --unsafeAllow "${parseArgs['unsafeAllow']}". Supported values for the --unsafeAllow option are: ${errorKinds.join(
Expand Down

0 comments on commit 8c84a64

Please sign in to comment.