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

util: add default value option to parsearg #44631

Merged
merged 5 commits into from
Oct 7, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 6 additions & 0 deletions doc/api/util.md
Original file line number Diff line number Diff line change
Expand Up @@ -1031,6 +1031,9 @@ added:
- v18.3.0
- v16.17.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/44631
description: Add support for default values in input `config`.
- version:
- v18.7.0
- v16.17.0
Expand All @@ -1053,6 +1056,9 @@ changes:
times. If `true`, all values will be collected in an array. If
`false`, values for the option are last-wins. **Default:** `false`.
* `short` {string} A single character alias for the option.
* `default` {string | boolean | string\[] | boolean\[]} The default option
value when it is not set by args. It must be of the same type as the
the `type` property. When `multiple` is `true`, it must be an array.
* `strict` {boolean} Should an error be thrown when unknown arguments
are encountered, or when arguments are passed that do not match the
`type` configured in `options`.
Expand Down
56 changes: 54 additions & 2 deletions lib/internal/util/parse_args/parse_args.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ const {
const {
validateArray,
validateBoolean,
validateBooleanArray,
validateObject,
validateString,
validateStringArray,
validateUnion,
} = require('internal/validators');

Expand All @@ -34,6 +36,7 @@ const {
isOptionLikeValue,
isShortOptionAndValue,
isShortOptionGroup,
useDefaultValueOption,
objectGetOwn,
optionsGetOwn,
} = require('internal/util/parse_args/utils');
Expand Down Expand Up @@ -143,6 +146,24 @@ function storeOption(longOption, optionValue, options, values) {
}
}

/**
* Store the default option value in `values`.
*
* @param {string} longOption - long option name e.g. 'foo'
* @param {string
* | boolean
* | string[]
* | boolean[]} optionValue - default value from option config
* @param {object} values - option values returned in `values` by parseArgs
*/
function storeDefaultOption(longOption, optionValue, values) {
if (longOption === '__proto__') {
return; // No. Just no.
}

values[longOption] = optionValue;
}

/**
* Process args and turn into identified tokens:
* - option (along with value, if any)
Expand Down Expand Up @@ -290,7 +311,8 @@ const parseArgs = (config = kEmptyObject) => {
validateObject(optionConfig, `options.${longOption}`);

// type is required
validateUnion(objectGetOwn(optionConfig, 'type'), `options.${longOption}.type`, ['string', 'boolean']);
const optionType = objectGetOwn(optionConfig, 'type');
validateUnion(optionType, `options.${longOption}.type`, ['string', 'boolean']);

if (ObjectHasOwn(optionConfig, 'short')) {
const shortOption = optionConfig.short;
Expand All @@ -304,8 +326,24 @@ const parseArgs = (config = kEmptyObject) => {
}
}

const multipleOption = objectGetOwn(optionConfig, 'multiple');
if (ObjectHasOwn(optionConfig, 'multiple')) {
validateBoolean(optionConfig.multiple, `options.${longOption}.multiple`);
validateBoolean(multipleOption, `options.${longOption}.multiple`);
}

const defaultValue = objectGetOwn(optionConfig, 'default');
if (defaultValue !== undefined) {
let validator;
switch (optionType) {
case 'string':
validator = multipleOption ? validateStringArray : validateString;
break;

case 'boolean':
validator = multipleOption ? validateBooleanArray : validateBoolean;
break;
}
validator(defaultValue, `options.${longOption}.default`);
}
}
);
Expand Down Expand Up @@ -336,6 +374,20 @@ const parseArgs = (config = kEmptyObject) => {
}
});

// Phase 3: fill in default values for missing args
ArrayPrototypeForEach(ObjectEntries(options), ({ 0: longOption,
1: optionConfig }) => {
const mustSetDefault = useDefaultValueOption(longOption,
optionConfig,
result.values);
if (mustSetDefault) {
storeDefaultOption(longOption,
objectGetOwn(optionConfig, 'default'),
result.values);
}
});


return result;
};

Expand Down
14 changes: 14 additions & 0 deletions lib/internal/util/parse_args/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,19 @@ function findLongOptionForShort(shortOption, options) {
return longOptionEntry?.[0] ?? shortOption;
}

/**
* Check if the given option includes a default value
* and that option has not been set by the input args.
*
* @param {string} longOption - long option name e.g. 'foo'
* @param {object} optionConfig - the option configuration properties
* @param {object} values - option values returned in `values` by parseArgs
*/
function useDefaultValueOption(longOption, optionConfig, values) {
return objectGetOwn(optionConfig, 'default') !== undefined &&
values[longOption] === undefined;
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
}

module.exports = {
findLongOptionForShort,
isLoneLongOption,
Expand All @@ -179,6 +192,7 @@ module.exports = {
isOptionLikeValue,
isShortOptionAndValue,
isShortOptionGroup,
useDefaultValueOption,
objectGetOwn,
optionsGetOwn,
};
32 changes: 32 additions & 0 deletions lib/internal/validators.js
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,36 @@ const validateArray = hideStackFrames((value, name, minLength = 0) => {
}
});

/**
* @callback validateStringArray
* @param {*} value
* @param {string} name
* @returns {asserts value is string[]}
*/

/** @type {validateStringArray} */
function validateStringArray(value, name) {
validateArray(value, name);
for (let i = 0; i < value.length; i++) {
validateString(value[i], `${name}[${i}]`);
}
}

/**
* @callback validateBooleanArray
* @param {*} value
* @param {string} name
* @returns {asserts value is boolean[]}
*/

/** @type {validateBooleanArray} */
function validateBooleanArray(value, name) {
validateArray(value, name);
for (let i = 0; i < value.length; i++) {
validateBoolean(value[i], `${name}[${i}]`);
}
}

// eslint-disable-next-line jsdoc/require-returns-check
/**
* @param {*} signal
Expand Down Expand Up @@ -423,6 +453,8 @@ module.exports = {
isUint32,
parseFileMode,
validateArray,
validateStringArray,
validateBooleanArray,
validateBoolean,
validateBuffer,
validateEncoding,
Expand Down
169 changes: 169 additions & 0 deletions test/parallel/test-parse-args.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -823,3 +823,172 @@ test('tokens: strict:false with -- --', () => {
const { tokens } = parseArgs({ strict: false, args, tokens: true });
assert.deepStrictEqual(tokens, expectedTokens);
});

test('default must be a boolean when option type is boolean', () => {
const args = [];
const options = { alpha: { type: 'boolean', default: 'not a boolean' } };
assert.throws(() => {
parseArgs({ args, options });
}, /"options\.alpha\.default" property must be of type boolean/
);
});

test('default must accept undefined value', () => {
const args = [];
const options = { alpha: { type: 'boolean', default: undefined } };
const result = parseArgs({ args, options });
const expected = {
values: {
__proto__: null,
},
positionals: []
};
assert.deepStrictEqual(result, expected);
});

test('default must be a boolean array when option type is boolean and multiple', () => {
const args = [];
const options = { alpha: { type: 'boolean', multiple: true, default: 'not an array' } };
assert.throws(() => {
parseArgs({ args, options });
}, /"options\.alpha\.default" property must be an instance of Array/
);
});

test('default must be a boolean array when option type is string and multiple is true', () => {
const args = [];
const options = { alpha: { type: 'boolean', multiple: true, default: [true, true, 42] } };
assert.throws(() => {
parseArgs({ args, options });
}, /"options\.alpha\.default\[2\]" property must be of type boolean/
);
});

test('default must be a string when option type is string', () => {
const args = [];
const options = { alpha: { type: 'string', default: true } };
assert.throws(() => {
parseArgs({ args, options });
}, /"options\.alpha\.default" property must be of type string/
);
});

test('default must be an array when option type is string and multiple is true', () => {
const args = [];
const options = { alpha: { type: 'string', multiple: true, default: 'not an array' } };
assert.throws(() => {
parseArgs({ args, options });
}, /"options\.alpha\.default" property must be an instance of Array/
);
});

test('default must be a string array when option type is string and multiple is true', () => {
const args = [];
const options = { alpha: { type: 'string', multiple: true, default: ['str', 42] } };
assert.throws(() => {
parseArgs({ args, options });
}, /"options\.alpha\.default\[1\]" property must be of type string/
);
});

test('default accepted input when multiple is true', () => {
const args = ['--inputStringArr', 'c', '--inputStringArr', 'd', '--inputBoolArr', '--inputBoolArr'];
const options = {
inputStringArr: { type: 'string', multiple: true, default: ['a', 'b'] },
emptyStringArr: { type: 'string', multiple: true, default: [] },
fullStringArr: { type: 'string', multiple: true, default: ['a', 'b'] },
inputBoolArr: { type: 'boolean', multiple: true, default: [false, true, false] },
emptyBoolArr: { type: 'boolean', multiple: true, default: [] },
fullBoolArr: { type: 'boolean', multiple: true, default: [false, true, false] },
};
const expected = { values: { __proto__: null,
inputStringArr: ['c', 'd'],
inputBoolArr: [true, true],
emptyStringArr: [],
fullStringArr: ['a', 'b'],
emptyBoolArr: [],
fullBoolArr: [false, true, false] },
positionals: [] };
const result = parseArgs({ args, options });
assert.deepStrictEqual(result, expected);
});

test('when default is set, the option must be added as result', () => {
const args = [];
const options = {
a: { type: 'string', default: 'HELLO' },
b: { type: 'boolean', default: false },
c: { type: 'boolean', default: true }
};
const expected = { values: { __proto__: null, a: 'HELLO', b: false, c: true }, positionals: [] };

const result = parseArgs({ args, options });
assert.deepStrictEqual(result, expected);
});

test('when default is set, the args value takes precedence', () => {
const args = ['--a', 'WORLD', '--b', '-c'];
const options = {
a: { type: 'string', default: 'HELLO' },
b: { type: 'boolean', default: false },
c: { type: 'boolean', default: true }
};
const expected = { values: { __proto__: null, a: 'WORLD', b: true, c: true }, positionals: [] };

const result = parseArgs({ args, options });
assert.deepStrictEqual(result, expected);
});

test('tokens should not include the default options', () => {
const args = [];
const options = {
a: { type: 'string', default: 'HELLO' },
b: { type: 'boolean', default: false },
c: { type: 'boolean', default: true }
};

const expectedTokens = [];

const { tokens } = parseArgs({ args, options, tokens: true });
assert.deepStrictEqual(tokens, expectedTokens);
});

test('tokens:true should not include the default options after the args input', () => {
const args = ['--z', 'zero', 'positional-item'];
const options = {
z: { type: 'string' },
a: { type: 'string', default: 'HELLO' },
b: { type: 'boolean', default: false },
c: { type: 'boolean', default: true }
};

const expectedTokens = [
{ kind: 'option', name: 'z', rawName: '--z', index: 0, value: 'zero', inlineValue: false },
{ kind: 'positional', index: 2, value: 'positional-item' },
];

const { tokens } = parseArgs({ args, options, tokens: true, allowPositionals: true });
assert.deepStrictEqual(tokens, expectedTokens);
});

test('proto as default value must be ignored', () => {
const args = [];
const options = Object.create(null);

// eslint-disable-next-line no-proto
options.__proto__ = { type: 'string', default: 'HELLO' };

const result = parseArgs({ args, options, allowPositionals: true });
const expected = { values: { __proto__: null }, positionals: [] };
assert.deepStrictEqual(result, expected);
});


test('multiple as false should expect a String', () => {
const args = [];
const options = { alpha: { type: 'string', multiple: false, default: ['array'] } };
assert.throws(() => {
parseArgs({ args, options });
}, /"options\.alpha\.default" property must be of type string/
);
});