Skip to content

Commit

Permalink
util: extend parseArgs to validate relationships between options
Browse files Browse the repository at this point in the history
Added 'conflicts', 'requires' and 'requiresOne' to parseArgs config.

Fixes: nodejs/node#45062
  • Loading branch information
SRHerzog committed Feb 14, 2023
1 parent fe514bf commit d8ec566
Show file tree
Hide file tree
Showing 4 changed files with 229 additions and 2 deletions.
7 changes: 7 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

<a id="ERR_PARSE_ARGS_INVALID_OPTION_CONFIG"></a>

### `ERR_PARSE_ARGS_INVALID_OPTION_CONFIG`

Thrown by [`util.parseArgs()`][] if it receives incompatible configuration
settings for `conflicts`, `requires`, or `requiresOne`.

<a id="ERR_PARSE_ARGS_INVALID_OPTION_VALUE"></a>

### `ERR_PARSE_ARGS_INVALID_OPTION_VALUE`
Expand Down
1 change: 1 addition & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
90 changes: 88 additions & 2 deletions lib/internal/util/parse_args/parse_args.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ const {
ArrayPrototypeShift,
ArrayPrototypeSlice,
ArrayPrototypeUnshiftApply,
ArrayPrototypeSome,
ObjectEntries,
ObjectPrototypeHasOwnProperty: ObjectHasOwn,
String,
StringPrototypeCharAt,
StringPrototypeIndexOf,
StringPrototypeSlice,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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`.
Expand Down Expand Up @@ -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 }) => {
Expand All @@ -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`);
Expand Down Expand Up @@ -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];
}
},
);

Expand Down Expand Up @@ -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,
Expand Down
133 changes: 133 additions & 0 deletions test/parallel/text-parse-args-conflicts-requires.mjs
Original file line number Diff line number Diff line change
@@ -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'
});
});

0 comments on commit d8ec566

Please sign in to comment.