From d8ec56618827b84d195d1e86a0700c792e355fe2 Mon Sep 17 00:00:00 2001 From: Steve Herzog Date: Tue, 14 Feb 2023 12:05:12 -0600 Subject: [PATCH] util: extend parseArgs to validate relationships between options Added 'conflicts', 'requires' and 'requiresOne' to parseArgs config. Fixes: https://github.com/nodejs/node/issues/45062 --- doc/api/errors.md | 7 + lib/internal/errors.js | 1 + lib/internal/util/parse_args/parse_args.js | 90 +++++++++++- .../text-parse-args-conflicts-requires.mjs | 133 ++++++++++++++++++ 4 files changed, 229 insertions(+), 2 deletions(-) create mode 100644 test/parallel/text-parse-args-conflicts-requires.mjs diff --git a/doc/api/errors.md b/doc/api/errors.md index a895dd5a5af..766071d6cc5 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -2429,6 +2429,13 @@ The `package.json` [`"exports"`][] field does not export the requested subpath. Because exports are encapsulated, private internal modules that are not exported cannot be imported through the package resolution, unless using an absolute URL. + + +### `ERR_PARSE_ARGS_INVALID_OPTION_CONFIG` + +Thrown by [`util.parseArgs()`][] if it receives incompatible configuration +settings for `conflicts`, `requires`, or `requiresOne`. + ### `ERR_PARSE_ARGS_INVALID_OPTION_VALUE` diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 4f833741cf9..afc874d5e4f 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1503,6 +1503,7 @@ E('ERR_PACKAGE_PATH_NOT_EXPORTED', (pkgPath, subpath, base = undefined) => { return `Package subpath '${subpath}' is not defined by "exports" in ${ pkgPath}package.json${base ? ` imported from ${base}` : ''}`; }, Error); +E('ERR_PARSE_ARGS_INVALID_OPTION_CONFIG', '%s', TypeError); E('ERR_PARSE_ARGS_INVALID_OPTION_VALUE', '%s', TypeError); E('ERR_PARSE_ARGS_UNEXPECTED_POSITIONAL', "Unexpected argument '%s'. This " + 'command does not take positional arguments', TypeError); diff --git a/lib/internal/util/parse_args/parse_args.js b/lib/internal/util/parse_args/parse_args.js index 68a9fddff50..07c64126166 100644 --- a/lib/internal/util/parse_args/parse_args.js +++ b/lib/internal/util/parse_args/parse_args.js @@ -9,8 +9,10 @@ const { ArrayPrototypeShift, ArrayPrototypeSlice, ArrayPrototypeUnshiftApply, + ArrayPrototypeSome, ObjectEntries, ObjectPrototypeHasOwnProperty: ObjectHasOwn, + String, StringPrototypeCharAt, StringPrototypeIndexOf, StringPrototypeSlice, @@ -44,6 +46,7 @@ const { const { codes: { ERR_INVALID_ARG_VALUE, + ERR_PARSE_ARGS_INVALID_OPTION_CONFIG, ERR_PARSE_ARGS_INVALID_OPTION_VALUE, ERR_PARSE_ARGS_UNKNOWN_OPTION, ERR_PARSE_ARGS_UNEXPECTED_POSITIONAL, @@ -113,6 +116,44 @@ function checkOptionUsage(config, token) { } } +/** + * Check for conflicting and required values. + * + * @param {object} conflicts - from config passed to parseArgs + * @param {object} options - from config passed to parseArgs + * @param {object} values - from values as available from parseArgs + */ +function checkConflictsAndRequirements(conflicts, options, values) { + ArrayPrototypeForEach(ObjectEntries(options), ({ 0: option, 1: optionConfig }) => { + const optionConflicts = objectGetOwn(conflicts, option); + if (optionConflicts !== undefined) { + ArrayPrototypeForEach(optionConflicts, (conflict) => { + if (objectGetOwn(values, conflict) !== undefined) { + throw new ERR_PARSE_ARGS_INVALID_OPTION_VALUE(`Options ${option} and ${conflict} ` + + 'are incompatible. You may only use one or the other.'); + } + }); + } + const optionRequires = objectGetOwn(optionConfig, 'requires'); + if (optionRequires !== undefined) { + ArrayPrototypeForEach(optionRequires, (requiredOption) => { + if (!ObjectHasOwn(values, requiredOption)) { + throw new ERR_PARSE_ARGS_INVALID_OPTION_VALUE(`Option ${option} requires ${requiredOption} ` + + 'to also be set'); + } + }); + } + const optionRequiresOne = objectGetOwn(optionConfig, 'requiresOne'); + if (optionRequiresOne !== undefined) { + if (!ArrayPrototypeSome(optionRequiresOne, (requiredOption) => + ObjectHasOwn(values, requiredOption))) { + throw new ERR_PARSE_ARGS_INVALID_OPTION_VALUE(`Option ${option} requires one of ` + + `${String(optionRequiresOne)} to also be set.`); + } + } + }); +} + /** * Store the option value in `values`. @@ -305,6 +346,7 @@ const parseArgs = (config = kEmptyObject) => { validateBoolean(allowPositionals, 'allowPositionals'); validateBoolean(returnTokens, 'tokens'); validateObject(options, 'options'); + const conflicts = {}; ArrayPrototypeForEach( ObjectEntries(options), ({ 0: longOption, 1: optionConfig }) => { @@ -313,7 +355,6 @@ const parseArgs = (config = kEmptyObject) => { // type is required const optionType = objectGetOwn(optionConfig, 'type'); validateUnion(optionType, `options.${longOption}.type`, ['string', 'boolean']); - if (ObjectHasOwn(optionConfig, 'short')) { const shortOption = optionConfig.short; validateString(shortOption, `options.${longOption}.short`); @@ -345,6 +386,48 @@ const parseArgs = (config = kEmptyObject) => { } validator(defaultValue, `options.${longOption}.default`); } + + const optionRequires = objectGetOwn(optionConfig, 'requires'); + const optionRequiresOne = objectGetOwn(optionConfig, 'requiresOne'); + if (optionRequires !== undefined) { + validateStringArray(optionRequires, `options.${longOption}.requires`); + } + if (optionRequiresOne !== undefined) { + validateStringArray(optionRequiresOne, `options.${longOption}.requiresOne`); + } + + const mergedRequires = [...(optionRequires || []), ...(optionRequiresOne || [])]; + ArrayPrototypeForEach(mergedRequires, (requiredOption) => { + const requiredOptionDef = objectGetOwn(options, requiredOption); + if (requiredOptionDef !== undefined) { + const requiredOptionConflicts = objectGetOwn(requiredOptionDef, 'conflicts'); + if (requiredOptionConflicts !== undefined && ArrayPrototypeIncludes(requiredOptionConflicts, longOption)) { + throw new ERR_PARSE_ARGS_INVALID_OPTION_CONFIG( + `Option ${requiredOption} conflicts with ${longOption}, but ${longOption} requires ${requiredOption}`, + ); + } + const defaultValue = objectGetOwn(requiredOptionDef, 'default'); + if (defaultValue !== undefined) { + throw new ERR_PARSE_ARGS_INVALID_OPTION_CONFIG( + `Option ${requiredOption} cannot have a default when it is ` + + `also required by ${longOption}`, + ); + } + } + }); + + const optionConflicts = objectGetOwn(optionConfig, 'conflicts'); + if (optionConflicts !== undefined) { + validateStringArray(optionConflicts, `options.${longOption}.conflict`); + ArrayPrototypeForEach(optionConflicts, (conflict) => { + if (ArrayPrototypeIncludes(mergedRequires, conflict)) { + throw new ERR_PARSE_ARGS_INVALID_OPTION_CONFIG( + `Option ${conflict} cannot be set as both required by and conflicting with ${longOption}`, + ); + } + }); + conflicts[longOption] = [...(objectGetOwn(conflicts, longOption) || []), ...optionConflicts]; + } }, ); @@ -374,7 +457,10 @@ const parseArgs = (config = kEmptyObject) => { } }); - // Phase 3: fill in default values for missing args + // Phase 3: check against required and incompatibile rules + checkConflictsAndRequirements(conflicts, options, result.values); + + // Phase 4: fill in default values for missing args ArrayPrototypeForEach(ObjectEntries(options), ({ 0: longOption, 1: optionConfig }) => { const mustSetDefault = useDefaultValueOption(longOption, diff --git a/test/parallel/text-parse-args-conflicts-requires.mjs b/test/parallel/text-parse-args-conflicts-requires.mjs new file mode 100644 index 00000000000..12bf227ce4c --- /dev/null +++ b/test/parallel/text-parse-args-conflicts-requires.mjs @@ -0,0 +1,133 @@ +import '../common/index.mjs'; +import assert from 'node:assert'; +import { test } from 'node:test'; +import { parseArgs } from 'node:util'; + +test('required option cannot have a default', () => { + const options = { + f: { + requires: ['g'], + type: 'string', + }, + g: { + default: 'b', + type: 'string', + }, + }; + assert.throws(() => { parseArgs({ options }); }, { + code: 'ERR_PARSE_ARGS_INVALID_OPTION_CONFIG' + }); +}); + +test('required option cannot also be a conflict', () => { + const options = { + f: { + requires: ['g'], + conflicts: ['g'], + type: 'string', + }, + }; + assert.throws(() => { parseArgs({ options }); }, { + code: 'ERR_PARSE_ARGS_INVALID_OPTION_CONFIG' + }); +}); + +test('required option cannot also be a conflict 2', () => { + const options = { + f: { + requires: ['g'], + type: 'string', + }, + g: { + type: 'string', + conflicts: ['f'], + } + }; + assert.throws(() => { parseArgs({ options }); }, { + code: 'ERR_PARSE_ARGS_INVALID_OPTION_CONFIG' + }); +}); + +test('requirements fulfilled', () => { + const options = { + f: { + requiresOne: ['g', 'h'], + type: 'string', + }, + g: { + type: 'string', + } + }; + const args = ['--f', '1', '--g', '2', '--h=3']; + const expected = { values: { __proto__: null, f: '1', g: '2', h: '3' }, positionals: [] }; + assert.deepStrictEqual(parseArgs({ args, options, strict: false }), expected); +}); + +test('requireOne fulfilled', () => { + const options = { + f: { + requiresOne: ['g', 'h'], + type: 'string', + }, + g: { + type: 'string', + } + }; + const args = ['--f', '1', '--h=2']; + const expected = { values: { __proto__: null, f: '1', h: '2' }, positionals: [] }; + assert.deepStrictEqual(parseArgs({ args, options, strict: false }), expected); +}); + +test('missing required option', () => { + const options = { + f: { + requires: ['g', 'h'], + type: 'string', + }, + g: { + type: 'string', + }, + h: { + type: 'string', + }, + }; + const args = ['--f', '1', '--g', '2']; + assert.throws(() => { parseArgs({ options, args }); }, { + code: 'ERR_PARSE_ARGS_INVALID_OPTION_VALUE' + }); +}); + +test('missing requiredOne option', () => { + const options = { + f: { + requiresOne: ['g', 'h'], + type: 'string', + }, + g: { + type: 'string', + }, + h: { + type: 'string', + }, + }; + const args = ['--f', '1']; + assert.throws(() => { parseArgs({ options, args }); }, { + code: 'ERR_PARSE_ARGS_INVALID_OPTION_VALUE' + }); +}); + +test('conflicting options', () => { + const options = { + f: { + conflicts: ['g'], + type: 'string', + }, + g: { + type: 'string', + }, + }; + const args = ['--f', '1', '--g', '2']; + assert.throws(() => { parseArgs({ options, args }); }, { + code: 'ERR_PARSE_ARGS_INVALID_OPTION_VALUE' + }); +});