diff --git a/docs/parsing-and-hooks.md b/docs/parsing-and-hooks.md index 122cad6f7..2dbecff89 100644 --- a/docs/parsing-and-hooks.md +++ b/docs/parsing-and-hooks.md @@ -1,23 +1,30 @@ # Parsing life cycle and hooks -The processing starts with an array of args. Each command processes and removes the options it understands, and passes the remaining args to the next subcommand. -The final command calls the action handler. +The processing starts with an array of args. Each command processes and removes the options it understands, and passes the remaining args to the next subcommand. The final command calls the action handler. Starting with top-level command (program): -- parse options: parse recognised options (for this command) and remove from args -- parse env: look for environment variables (for this command) -- process implied: set any implied option values (for this command) -- if the first arg is a subcommand - - call `preSubcommand` hooks - - pass remaining arguments to subcommand, and process same way +* process options for this command (includes calling or chaining provided argParsers[^1]) + * recognised options from args + * options based on environment variables +* set implied option values (for this command) +* remove recognised options from args +* await thenable option values if parsing with `parseAsync()` +* if the first arg is a subcommand + * call or chain `preSubcommand` hooks[^1] + * pass remaining arguments to subcommand, and process same way Once reach final (leaf) command: -- check for missing mandatory options -- check for conflicting options -- check for unknown options -- process remaining args as command-arguments -- call `preAction` hooks -- call action handler -- call `postAction` hooks +* check for missing mandatory options +* check for conflicting options +* check for unknown options +* process remaining args as command-arguments (includes calling or chaining provided argParsers[^1]) +* await thenable argument values if parsing with `parseAsync()` +* if an action handler is provided + * call or chain `preAction` hooks[^1] + * call or chain action handler[^1] + * call or chain `postAction` hooks[^1] + + +[^1]: output a warning suggesting use of `parseAsync()` when parsing with `parse()` and a thenable is returned, unless `process.env.NODE_ENV === 'production'` diff --git a/lib/argument.js b/lib/argument.js index c16430250..7bea70ce4 100644 --- a/lib/argument.js +++ b/lib/argument.js @@ -16,6 +16,7 @@ class Argument { this.description = description || ''; this.variadic = false; this.parseArg = undefined; + this.chained = true; this.defaultValue = undefined; this.defaultValueDescription = undefined; this.argChoices = undefined; @@ -89,6 +90,17 @@ class Argument { return this; } + /** + * When set to `true` (the default), next call to the function provided via `.argParser()` will be chained to its return value if it is thenable. + * + * @param {boolean} [chained=true] + * @return {Argument} + */ + chainArgParserCalls(chained = true) { + this.chained = !!chained; + return this; + } + /** * Only allow argument value to be one of choices. * diff --git a/lib/command.js b/lib/command.js index 590a271dd..2c837851f 100644 --- a/lib/command.js +++ b/lib/command.js @@ -57,6 +57,10 @@ class Command extends EventEmitter { this._showHelpAfterError = false; this._showSuggestionAfterError = true; + /** @type {boolean | undefined} */ + this._asyncParsing = undefined; + this._handledNotChainedNonVariadicOptionValues = {}; + // see .configureOutput() for docs this._outputConfiguration = { writeOut: (str) => process.stdout.write(str), @@ -511,14 +515,14 @@ Expecting one of '${allowedValues.join("', '")}'`); const name = option.attributeName(); // store default value - if (option.negate) { + if (option.defaultValue !== undefined || !option.negate) { + this.setOptionValueWithSource(name, option.defaultValue, 'default'); + } else { // --no-foo is special and defaults foo to true, unless a --foo option is already defined const positiveLongFlag = option.long.replace(/^--no-/, '--'); if (!this._findOption(positiveLongFlag)) { - this.setOptionValueWithSource(name, option.defaultValue === undefined ? true : option.defaultValue, 'default'); + this.setOptionValueWithSource(name, true, 'default'); } - } else if (option.defaultValue !== undefined) { - this.setOptionValueWithSource(name, option.defaultValue, 'default'); } // register the option @@ -535,15 +539,22 @@ Expecting one of '${allowedValues.join("', '")}'`); // custom processing const oldValue = this.getOptionValue(name); if (val !== null && option.parseArg) { - try { - val = option.parseArg(val, oldValue); - } catch (err) { - if (err.code === 'commander.invalidArgument') { + const handleError = (err) => { + if (err?.code === 'commander.invalidArgument') { const message = `${invalidValueMessage} ${err.message}`; this.error(message, { exitCode: err.exitCode, code: err.code }); } throw err; - } + }; + const handledResultCallback = (handledResult) => { + if (!option.chained && !option.variadic) { + // For variadic options and arguments, it is well-justified to assume previous thenable is handled by parser, but this cannot be assumed for non-variadic options. + // End user might repeat an option that is not supposed to be repeated, i.e. an option whose parser ignores the second parameter, so special overwritten option handling is necessary. + this._handledNotChainedNonVariadicOptionValues[name] ??= []; + this._handledNotChainedNonVariadicOptionValues[name].push(handledResult); + } + }; + val = this._parseArg(option, val, oldValue, handleError, handledResultCallback); } else if (val !== null && option.variadic) { val = option._concatValue(val, oldValue); } @@ -793,6 +804,14 @@ Expecting one of '${allowedValues.join("', '")}'`); */ setOptionValueWithSource(key, value, source) { + if (!this._asyncParsing && isThenable(value)) { + console.warn(`.parse() is incompatible with ${ + ['cli', 'env'].includes(source) + ? 'async argParsers' + : 'thenable option values' + }. +Use .parseAsync() instead`); + } if (this._storeOptionsAsProperties) { this[key] = value; } else { @@ -906,7 +925,7 @@ Expecting one of '${allowedValues.join("', '")}'`); parse(argv, parseOptions) { const userArgs = this._prepareUserArgs(argv, parseOptions); - this._parseCommand([], userArgs); + this._parseCommand([], userArgs, false); return this; } @@ -932,7 +951,7 @@ Expecting one of '${allowedValues.join("', '")}'`); async parseAsync(argv, parseOptions) { const userArgs = this._prepareUserArgs(argv, parseOptions); - await this._parseCommand([], userArgs); + await this._parseCommand([], userArgs, true); return this; } @@ -1078,7 +1097,7 @@ Expecting one of '${allowedValues.join("', '")}'`); if (subCommand._executableHandler) { this._executeSubCommand(subCommand, operands.concat(unknown)); } else { - return subCommand._parseCommand(operands, unknown); + return subCommand._parseCommand(operands, unknown, this._asyncParsing); } }); return hookResult; @@ -1134,18 +1153,16 @@ Expecting one of '${allowedValues.join("', '")}'`); _processArguments() { const myParseArg = (argument, value, previous) => { - // Extra processing for nice error message on parsing failure. let parsedValue = value; if (value !== null && argument.parseArg) { - try { - parsedValue = argument.parseArg(value, previous); - } catch (err) { - if (err.code === 'commander.invalidArgument') { + const handleError = (err) => { + if (err?.code === 'commander.invalidArgument') { const message = `error: command-argument value '${value}' is invalid for argument '${argument.name()}'. ${err.message}`; this.error(message, { exitCode: err.exitCode, code: err.code }); } throw err; - } + }; + parsedValue = this._parseArg(argument, value, previous, handleError); } return parsedValue; }; @@ -1173,6 +1190,12 @@ Expecting one of '${allowedValues.join("', '")}'`); value = myParseArg(declaredArg, value, declaredArg.defaultValue); } } + if (!this._asyncParsing && isThenable(value)) { + console.warn(`.parse() is incompatible with ${ + declaredArg.parseArg ? 'async argParsers' : 'thenable argument values' + }. +Use .parseAsync() instead`); + } processedArgs[index] = value; }); this.processedArgs = processedArgs; @@ -1181,20 +1204,19 @@ Expecting one of '${allowedValues.join("', '")}'`); /** * Once we have a promise we chain, but call synchronously until then. * - * @param {Promise|undefined} promise + * @param {*} chain * @param {Function} fn - * @return {Promise|undefined} + * @return {*} * @api private */ - _chainOrCall(promise, fn) { - // thenable - if (promise && promise.then && typeof promise.then === 'function') { + _chainOrCall(chain, fn) { + if (isThenable(chain)) { // already have a promise, chain callback - return promise.then(() => fn()); + return chain.then(fn); } // callback might return a promise - return fn(); + return fn(chain); } /** @@ -1250,87 +1272,210 @@ Expecting one of '${allowedValues.join("', '")}'`); } /** - * Process arguments in context of this command. - * Returns action result, in case it is a promise. + * @param {Argument|Option} target + * @param {*} value + * @param {*} previous + * @param {Function} handleError + * @param {Function} [handledResultCallback] + * @returns {*} + * @api private + */ + + _parseArgSubroutine(target, value, previous, handleError, handledResultCallback) { + let result = target.parseArg(value, previous); + if (this._asyncParsing) { + let handledResult = result; + if (isThenable(result)) { + handledResult = result.then(x => x, handleError); + if (target.chained) { + result = handledResult; + } // otherwise pass original thenable to next parser call + } + handledResultCallback?.(handledResult); + } + return result; + } + + /** + * Internal implementation shared by ._processArguments() and option and optionEnv event listeners. * + * @param {Argument|Option} target + * @param {*} value + * @param {*} previous + * @param {Function} handleError + * @param {Function} [handledResultCallback] + * @return {*} * @api private */ - _parseCommand(operands, unknown) { - const parsed = this.parseOptions(unknown); - this._parseOptionsEnv(); // after cli, so parseArg not called on both cli and env - this._parseOptionsImplied(); - operands = operands.concat(parsed.operands); - unknown = parsed.unknown; - this.args = operands.concat(unknown); + _parseArg(target, value, previous, handleError, handledResultCallback) { + try { + if (target.chained) { + return this._chainOrCall(previous, (resolvedPrevious) => ( + this._parseArgSubroutine( + target, value, resolvedPrevious, handleError, handledResultCallback + ) + )); + } - if (operands && this._findCommand(operands[0])) { - return this._dispatchSubcommand(operands[0], operands.slice(1), unknown); - } - if (this._hasImplicitHelpCommand() && operands[0] === this._helpCommandName) { - return this._dispatchHelpCommand(operands[1]); - } - if (this._defaultCommandName) { - outputHelpIfRequested(this, unknown); // Run the help for default command from parent rather than passing to default command - return this._dispatchSubcommand(this._defaultCommandName, operands, unknown); + return this._parseArgSubroutine(...arguments); + } catch (err) { + handleError(err); } - if (this.commands.length && this.args.length === 0 && !this._actionHandler && !this._defaultCommandName) { - // probably missing subcommand and no handler, user needs help (and exit) - this.help({ error: true }); + } + + /** + * @api private + */ + + _optionalAwait(getPromises) { + if (this._asyncParsing) { + const toAwait = getPromises(); + if (toAwait.length) return Promise.all(toAwait); } + } - outputHelpIfRequested(this, parsed.unknown); - this._checkForMissingMandatoryOptions(); - this._checkForConflictingOptions(); + /** + * @api private + */ - // We do not always call this check to avoid masking a "better" error, like unknown command. - const checkForUnknownOptions = () => { - if (parsed.unknown.length > 0) { - this.unknownOption(parsed.unknown[0]); - } - }; + _awaitThenableOptions() { + return this._optionalAwait(() => { + const chainedPromises = Object.entries(this.opts()) + .filter(([_, value]) => isThenable(value)) + .map(([key, thenable]) => (async() => { + this.setOptionValueWithSource( + key, await thenable, this.getOptionValueSource(key) + ); + })()); + + const notChainedNonVariadicPromises = Object.entries( + this._handledNotChainedNonVariadicOptionValues + ) + .map(([_, values]) => values.slice(0, -1)) // only overwritten + .flat() + .filter(isThenable) + .map((thenable) => (async() => { + await thenable; + })()); + + return chainedPromises.concat(notChainedNonVariadicPromises); + }); + } - const commandEvent = `command:${this.name()}`; - if (this._actionHandler) { - checkForUnknownOptions(); - this._processArguments(); - - let actionResult; - actionResult = this._chainOrCallHooks(actionResult, 'preAction'); - actionResult = this._chainOrCall(actionResult, () => this._actionHandler(this.processedArgs)); - if (this.parent) { - actionResult = this._chainOrCall(actionResult, () => { - this.parent.emit(commandEvent, operands, unknown); // legacy - }); - } - actionResult = this._chainOrCallHooks(actionResult, 'postAction'); - return actionResult; - } - if (this.parent && this.parent.listenerCount(commandEvent)) { - checkForUnknownOptions(); - this._processArguments(); - this.parent.emit(commandEvent, operands, unknown); // legacy - } else if (operands.length) { - if (this._findCommand('*')) { // legacy default command - return this._dispatchSubcommand('*', operands, unknown); - } - if (this.listenerCount('command:*')) { - // skip option check, emit event for possible misspelling suggestion - this.emit('command:*', operands, unknown); - } else if (this.commands.length) { - this.unknownCommand(); + /** + * @api private + */ + + _awaitThenableArguments() { + return this._optionalAwait(() => { + return this.processedArgs + .filter(isThenable) + .map((thenable, i) => (async() => { + this.processedArgs[i] = await thenable; + })()); + }); + } + + /** + * Process arguments in context of this command. + * Returns action result, in case it is a promise. + * + * @api private + */ + + _parseCommand(operands, unknown, async) { + this._asyncParsing = async; + + try { + let chain; + + const parsed = this.parseOptions(unknown); + this._parseOptionsEnv(); // after cli, so parseArg not called on both cli and env + this._parseOptionsImplied(); + chain = this._chainOrCall(chain, () => this._awaitThenableOptions()); + + operands = operands.concat(parsed.operands); + unknown = parsed.unknown; + this.args = operands.concat(unknown); + + if (operands && this._findCommand(operands[0])) { + chain = this._chainOrCall(chain, () => this._dispatchSubcommand( + operands[0], operands.slice(1), unknown + )); + } else if (this._hasImplicitHelpCommand() && operands[0] === this._helpCommandName) { + chain = this._chainOrCall(chain, () => this._dispatchHelpCommand(operands[1])); + } else if (this._defaultCommandName) { + outputHelpIfRequested(this, unknown); // Run the help for default command from parent rather than passing to default command + chain = this._chainOrCall(chain, () => this._dispatchSubcommand( + this._defaultCommandName, operands, unknown + )); } else { - checkForUnknownOptions(); - this._processArguments(); + if (this.commands.length && this.args.length === 0 && !this._actionHandler && !this._defaultCommandName) { + // probably missing subcommand and no handler, user needs help (and exit) + this.help({ error: true }); + } + + outputHelpIfRequested(this, parsed.unknown); + this._checkForMissingMandatoryOptions(); + this._checkForConflictingOptions(); + + // We do not always call this check to avoid masking a "better" error, like unknown command. + const checkForUnknownOptions = () => { + if (parsed.unknown.length > 0) { + this.unknownOption(parsed.unknown[0]); + } + }; + const processAndAwaitArguments = () => { + this._processArguments(); + chain = this._chainOrCall(chain, () => this._awaitThenableArguments()); + }; + + const commandEvent = `command:${this.name()}`; + if (this._actionHandler) { + checkForUnknownOptions(); + processAndAwaitArguments(); + chain = this._chainOrCallHooks(chain, 'preAction'); + chain = this._chainOrCall(chain, () => this._actionHandler(this.processedArgs)); + if (this.parent) { + chain = this._chainOrCall(chain, () => { + this.parent.emit(commandEvent, operands, unknown); // legacy + }); + } + chain = this._chainOrCallHooks(chain, 'postAction'); + } else if (this.parent && this.parent.listenerCount(commandEvent)) { + checkForUnknownOptions(); + processAndAwaitArguments(); + chain = this._chainOrCall(chain, () => { + this.parent.emit(commandEvent, operands, unknown); // legacy + }); + } else if (operands.length) { + if (this._findCommand('*')) { // legacy default command + return this._dispatchSubcommand('*', operands, unknown); + } + if (this.listenerCount('command:*')) { + // skip option check, emit event for possible misspelling suggestion + this.emit('command:*', operands, unknown); + } else if (this.commands.length) { + this.unknownCommand(); + } else { + checkForUnknownOptions(); + processAndAwaitArguments(); + } + } else if (this.commands.length) { + checkForUnknownOptions(); + // This command has subcommands and nothing hooked up at this level, so display help (and exit). + this.help({ error: true }); + } else { + checkForUnknownOptions(); + processAndAwaitArguments(); + // fall through for caller to handle after calling .parse() + } } - } else if (this.commands.length) { - checkForUnknownOptions(); - // This command has subcommands and nothing hooked up at this level, so display help (and exit). - this.help({ error: true }); - } else { - checkForUnknownOptions(); - this._processArguments(); - // fall through for caller to handle after calling .parse() + + return chain; + } finally { + this._asyncParsing = undefined; } } @@ -2193,4 +2338,14 @@ function getCommandAndParents(startCommand) { return result; } +/** + * @param {*} value + * @returns {boolean} + * @api private + */ + +function isThenable(value) { + return typeof value?.then === 'function'; +} + exports.Command = Command; diff --git a/lib/error.js b/lib/error.js index e7cde9cc7..7f166a3bd 100644 --- a/lib/error.js +++ b/lib/error.js @@ -9,7 +9,7 @@ class CommanderError extends Error { * Constructs the CommanderError class * @param {number} exitCode suggested exit code which could be used with process.exit * @param {string} code an id string representing the error - * @param {string} message human-readable description of the error + * @param {string} [message] human-readable description of the error * @constructor */ constructor(exitCode, code, message) { diff --git a/lib/option.js b/lib/option.js index d61fc5f2f..78a83f92e 100644 --- a/lib/option.js +++ b/lib/option.js @@ -31,6 +31,7 @@ class Option { this.presetArg = undefined; this.envVar = undefined; this.parseArg = undefined; + this.chained = true; this.hidden = false; this.argChoices = undefined; this.conflictsWith = []; @@ -135,6 +136,17 @@ class Option { return this; } + /** + * When set to `true` (the default), next call to the function provided via `.argParser()` will be chained to its return value if it is thenable. + * + * @param {boolean} [chained=true] + * @return {Argument} + */ + chainArgParserCalls(chained = true) { + this.chained = !!chained; + return this; + } + /** * Whether the option is mandatory and must have a value after parsing. * diff --git a/tests/argument.chain.test.js b/tests/argument.chain.test.js index dbc7947c2..b8057f4ac 100644 --- a/tests/argument.chain.test.js +++ b/tests/argument.chain.test.js @@ -13,6 +13,12 @@ describe('Argument methods that should return this for chaining', () => { expect(result).toBe(argument); }); + test('when call .chainArgParserCalls() then returns this', () => { + const argument = new Argument(''); + const result = argument.chainArgParserCalls(); + expect(result).toBe(argument); + }); + test('when call .choices() then returns this', () => { const argument = new Argument(''); const result = argument.choices(['a']); diff --git a/tests/argument.custom-processing.test.js b/tests/argument.custom-processing.test.js index ef3d16822..52a5c098f 100644 --- a/tests/argument.custom-processing.test.js +++ b/tests/argument.custom-processing.test.js @@ -205,3 +205,22 @@ test('when custom processing for argument throws plain error then not CommanderE expect(caughtErr).toBeInstanceOf(Error); expect(caughtErr).not.toBeInstanceOf(commander.CommanderError); }); + +test('when async custom variadic then parsed to chain', async() => { + const args = ['1', '2']; + const resolvedValue = '12'; + const coercion = async(value, previousValue) => ( + previousValue === undefined ? value : previousValue + value + ); + const awaited = coercion(args[0], undefined); + const mockCoercion = jest.fn().mockImplementation(coercion); + + const program = new commander.Command(); + program + .argument('', 'desc', mockCoercion) + .action(() => {}); + + program.parseAsync(args, { from: 'user' }); + expect(program.processedArgs[0]).toEqual(awaited); + await expect(program.processedArgs[0]).resolves.toEqual(resolvedValue); +}); diff --git a/tests/command.argumentVariations.test.js b/tests/command.argumentVariations.test.js index a8a781625..0591f140d 100644 --- a/tests/command.argumentVariations.test.js +++ b/tests/command.argumentVariations.test.js @@ -9,7 +9,8 @@ test.each(getSingleArgCases(''))('when add "" using %s t _name: 'explicit-required', required: true, variadic: false, - description: '' + description: '', + chained: true }; expect(argument).toEqual(expectedShape); }); @@ -20,7 +21,8 @@ test.each(getSingleArgCases('implicit-required'))('when add "arg" using %s then _name: 'implicit-required', required: true, variadic: false, - description: '' + description: '', + chained: true }; expect(argument).toEqual(expectedShape); }); @@ -31,7 +33,8 @@ test.each(getSingleArgCases('[optional]'))('when add "[arg]" using %s then argum _name: 'optional', required: false, variadic: false, - description: '' + description: '', + chained: true }; expect(argument).toEqual(expectedShape); }); @@ -42,7 +45,8 @@ test.each(getSingleArgCases(''))('when add "" usin _name: 'explicit-required', required: true, variadic: true, - description: '' + description: '', + chained: true }; expect(argument).toEqual(expectedShape); }); @@ -53,7 +57,8 @@ test.each(getSingleArgCases('implicit-required...'))('when add "arg..." using %s _name: 'implicit-required', required: true, variadic: true, - description: '' + description: '', + chained: true }; expect(argument).toEqual(expectedShape); }); @@ -64,7 +69,8 @@ test.each(getSingleArgCases('[optional...]'))('when add "[arg...]" using %s then _name: 'optional', required: false, variadic: true, - description: '' + description: '', + chained: true }; expect(argument).toEqual(expectedShape); }); diff --git a/tests/command.await.test.js b/tests/command.await.test.js new file mode 100644 index 000000000..2d8178175 --- /dev/null +++ b/tests/command.await.test.js @@ -0,0 +1,255 @@ +/* eslint 'jest/expect-expect': [ + 'warn', + { + assertFunctionNames: ['expect', 'testWithArguments', 'testWithOptions'] + } +] */ + +const commander = require('../'); + +const makeThenable = (function() { + const cache = new Map(); + return (value) => { + if (cache.has(value)) { + return cache.get(value); + } + const thenable = { + then: (fn) => makeThenable(fn(value)) + }; + cache.set(value, thenable); + return thenable; + }; +})(); + +const chainedAwaited = async(coercion, args) => ( + args.reduce(async(promise, v) => { + const { thenable } = await promise; + return { thenable: makeThenable(coercion(v, await thenable)) }; + }, { thenable: makeThenable(undefined) }) +); + +describe('await arguments', () => { + async function testWithArguments(program, args, resolvedValues, awaited) { + let actionValues; + program + .action((...args) => { + actionValues = args.slice(0, resolvedValues.length); + }); + + const result = program.parseAsync(args, { from: 'user' }); + awaited.forEach((value, i) => { + expect(program.processedArgs[i]).toBe(value); + }); + await result; + expect(program.processedArgs).toEqual(resolvedValues); + expect(actionValues).toEqual(resolvedValues); + } + + test('when arguments with custom processing then .processedArgs and action arguments resolved from callback', async() => { + const args = ['1', '2']; + const resolvedValues = [3, 4]; + const awaited = [ + makeThenable(resolvedValues[0]), + resolvedValues[1] + ]; + const mockCoercions = awaited.map( + value => jest.fn().mockImplementation(() => value) + ); + + const program = new commander.Command(); + program + .argument('', 'desc', mockCoercions[0]) + .argument('[arg]', 'desc', mockCoercions[1]); + + await testWithArguments(program, args, resolvedValues, awaited); + }); + + test('when arguments not specified with default values then .processedArgs and action arguments resolved from default values', async() => { + const args = []; + const resolvedValues = [1, 2]; + const awaited = [ + makeThenable(resolvedValues[0]), + resolvedValues[1] + ]; + + const program = new commander.Command(); + program + .argument('[arg]', 'desc', awaited[0]) + .argument('[arg]', 'desc', awaited[1]); + + await testWithArguments(program, args, resolvedValues, awaited); + }); + + test('when variadic argument with asynchronous custom processing then .processedArgs and action arguments resolved from chain', async() => { + const args = ['1', '2']; + const resolvedValues = ['12']; + const coercion = (value, previousValue) => { + const coerced = previousValue === undefined + ? value + : previousValue + value; + return makeThenable(coerced); + }; + const awaited = [(await chainedAwaited(coercion, args)).thenable]; + const mockCoercion = jest.fn().mockImplementation(coercion); + + const program = new commander.Command(); + program + .argument('', 'desc', mockCoercion); + + await testWithArguments(program, args, resolvedValues, awaited); + }); +}); + +describe('await options', () => { + async function testWithOptions(program, args, resolvedValues, awaited) { + program + .action(() => {}); + + const result = program.parseAsync(args, { from: 'user' }); + Object.entries(awaited).forEach(([key, value]) => { + expect(program.opts()[key]).toBe(value); + }); + await result; + expect(program.opts()).toEqual(resolvedValues); + } + + test('when options with custom processing then .opts() resolved from callback', async() => { + const args = ['-a', '1', '-b', '2']; + const resolvedValues = { a: 3, b: 4 }; + const awaited = { + a: makeThenable(resolvedValues.a), + b: resolvedValues.b + }; + const mockCoercions = Object.entries(awaited).reduce((acc, [key, value]) => { + acc[key] = jest.fn().mockImplementation(() => value); + return acc; + }, {}); + + const program = new commander.Command(); + program + .option('-a ', 'desc', mockCoercions.a) + .option('-b [arg]', 'desc', mockCoercions.b); + + await testWithOptions(program, args, resolvedValues, awaited); + expect(program.getOptionValueSource('a')).toEqual('cli'); + expect(program.getOptionValueSource('b')).toEqual('cli'); + }); + + test('when options not specified with default values then .opts() resolved from default values', async() => { + const args = []; + const resolvedValues = { a: 1, b: 2 }; + const awaited = { + a: makeThenable(resolvedValues.a), + b: resolvedValues.b + }; + + const program = new commander.Command(); + program + .option('-a ', 'desc', awaited.a) + .option('-b [arg]', 'desc', awaited.b); + + await testWithOptions(program, args, resolvedValues, awaited); + expect(program.getOptionValueSource('a')).toEqual('default'); + expect(program.getOptionValueSource('b')).toEqual('default'); + }); + + test('when implied option values then .opts() resolved from implied values', async() => { + const args = ['-c']; + const resolvedValues = { a: 1, b: 2, c: true }; + const awaited = { + a: makeThenable(resolvedValues.a), + b: resolvedValues.b, + c: true + }; + + const option = new commander.Option('-c').implies(awaited); + const program = new commander.Command(); + program + .option('-a ') + .option('-b [arg]') + .addOption(option); + + await testWithOptions(program, args, resolvedValues, awaited); + expect(program.getOptionValueSource('a')).toEqual('implied'); + expect(program.getOptionValueSource('b')).toEqual('implied'); + }); + + test('when non-variadic repeated option with asynchronous custom processing then .opts() resolved from chain', async() => { + const args = ['-a', '1', '-a', '2']; + const resolvedValues = { a: '12' }; + const coercion = (value, previousValue) => { + const coerced = previousValue === undefined + ? value + : previousValue + value; + return makeThenable(coerced); + }; + const awaited = { + a: (await chainedAwaited( + coercion, args.filter((_, i) => i % 2)) + ).thenable + }; + const mockCoercion = jest.fn().mockImplementation(coercion); + + const program = new commander.Command(); + program + .option('-a [arg]', 'desc', mockCoercion); + + await testWithOptions(program, args, resolvedValues, awaited); + expect(program.getOptionValueSource('a')).toEqual('cli'); + }); + + test('when variadic option with asynchronous custom processing then .opts() resolved from chain', async() => { + const args = ['-a', '1', '2']; + const resolvedValues = { a: '12' }; + const coercion = (value, previousValue) => { + const coerced = previousValue === undefined + ? value + : previousValue + value; + return makeThenable(coerced); + }; + const awaited = { + a: (await chainedAwaited( + coercion, args.slice(1)) + ).thenable + }; + const mockCoercion = jest.fn().mockImplementation(coercion); + + const program = new commander.Command(); + program + .option('-a ', 'desc', mockCoercion); + + await testWithOptions(program, args, resolvedValues, awaited); + expect(program.getOptionValueSource('a')).toEqual('cli'); + }); + + test('when subcommand and options with custom processing then global .opts() resolved from callback before local option-argument parser is called', async() => { + const arr = []; + + const coercion = (value) => { + return new Promise((resolve) => { + setImmediate(() => { + arr.push(value); + resolve(value); + }); + }); + }; + const mockCoercion = jest.fn().mockImplementation(coercion); + + const syncCoercion = (value) => { + arr.push(value); + return value; + }; + const mockSyncCoercion = jest.fn().mockImplementation(syncCoercion); + + const program = new commander.Command(); + program + .option('-a ', 'desc', mockCoercion) + .command('subcommand') + .option('-b ', 'desc', mockSyncCoercion) + .action(() => {}); + + const args = ['-a', '1', 'subcommand', '-b', '2']; + await program.parseAsync(args, { from: 'user' }); + expect(arr).toEqual(['1', '2']); + }); +}); diff --git a/tests/command.parse.test.js b/tests/command.parse.test.js index b9903725f..6ef14330d 100644 --- a/tests/command.parse.test.js +++ b/tests/command.parse.test.js @@ -87,6 +87,101 @@ describe('return type', () => { const result = await program.parseAsync(['node', 'test']); expect(result).toBe(program); }); + + const makeMockCoercion = (errors, promises, condition) => ( + jest.fn().mockImplementation( + (value) => { + if (condition?.()) { + return value; + } + + const error = {}; // not Error so that unhandled promise rejections are properly reported + errors.push(error); + const promise = Promise.reject(error); + promises.push(promise); + return promise; + } + ) + ); + + test('when await .parseAsync and asynchronous custom processing for arguments fails then rejects', async() => { + const promises = []; + const errors = []; + const mockCoercion = makeMockCoercion(errors, promises); + + const program = new commander.Command(); + program + .argument('[arg]', 'desc', mockCoercion) + .argument('[arg]', 'desc', mockCoercion) + .action(() => { }); + + const result = program.parseAsync(['1', '2'], { from: 'user' }); + + let caught; + try { + await result; + } catch (value) { + caught = value; + } + + expect(errors).toContain(caught); + expect(mockCoercion).toHaveBeenCalledTimes(2); + }); + + test('when await .parseAsync and asynchronous custom processing for options fails then rejects', async() => { + const promises = []; + const errors = []; + const mockCoercion = makeMockCoercion(errors, promises); + + const program = new commander.Command(); + program + .option('-a [arg]', 'desc', mockCoercion) + .option('-b [arg]', 'desc', mockCoercion) + .action(() => { }); + + const result = program.parseAsync(['-a', '1', '-b', '2'], { from: 'user' }); + + let caught; + try { + await result; + } catch (value) { + caught = value; + } + + expect(errors).toContain(caught); + expect(mockCoercion).toHaveBeenCalledTimes(2); + }); + + test('when await .parseAsync and not chained asynchronous custom processing fails for overwritten non-variadic option then rejects', async() => { + const promises = []; + const errors = []; + const mockCoercion = makeMockCoercion( + errors, promises, () => promises.length + ); + + const option = new commander.Option('-a [arg]', 'desc') + .argParser(mockCoercion) + .chainArgParserCalls(false); + + const program = new commander.Command(); + program + .addOption(option) + .action(() => { }); + + const result = program.parseAsync( + ['-a', '1', '-a', '2', '-a', '3'], { from: 'user' } + ); + + let caught; + try { + await result; + } catch (value) { + caught = value; + } + + expect(caught).toBe(errors[0]); + expect(mockCoercion).toHaveBeenCalledTimes(3); + }); }); // Easy mistake to make when writing unit tests diff --git a/tests/option.chain.test.js b/tests/option.chain.test.js index 3ed59688d..6d23bce7b 100644 --- a/tests/option.chain.test.js +++ b/tests/option.chain.test.js @@ -13,6 +13,12 @@ describe('Option methods that should return this for chaining', () => { expect(result).toBe(option); }); + test('when call .chainArgParserCalls() then returns this', () => { + const option = new Option('-e,--example '); + const result = option.chainArgParserCalls(); + expect(result).toBe(option); + }); + test('when call .makeOptionMandatory() then returns this', () => { const option = new Option('-e,--example '); const result = option.makeOptionMandatory(); diff --git a/tests/options.custom-processing.test.js b/tests/options.custom-processing.test.js index aa1022fbb..08613c67a 100644 --- a/tests/options.custom-processing.test.js +++ b/tests/options.custom-processing.test.js @@ -139,3 +139,41 @@ test('when commaSeparatedList x,y,z then value is [x, y, z]', () => { program.parse(['node', 'test', '--list', 'x,y,z']); expect(program.opts().list).toEqual(['x', 'y', 'z']); }); + +test('when async custom non-variadic repeated then parsed to chain', async() => { + const args = ['-a', '1', '-a', '2']; + const resolvedValue = '12'; + const coercion = async(value, previousValue) => ( + previousValue === undefined ? value : previousValue + value + ); + const awaited = coercion(args[1], undefined); + const mockCoercion = jest.fn().mockImplementation(coercion); + + const program = new commander.Command(); + program + .option('-a ', 'desc', mockCoercion) + .action(() => {}); + + program.parseAsync(args, { from: 'user' }); + expect(program.opts()).toEqual({ a: awaited }); + await expect(program.opts().a).resolves.toEqual(resolvedValue); +}); + +test('when async custom variadic then parsed to chain', async() => { + const args = ['-a', '1', '2']; + const resolvedValue = '12'; + const coercion = async(value, previousValue) => ( + previousValue === undefined ? value : previousValue + value + ); + const awaited = coercion(args[1], undefined); + const mockCoercion = jest.fn().mockImplementation(coercion); + + const program = new commander.Command(); + program + .option('-a ', 'desc', mockCoercion) + .action(() => {}); + + program.parseAsync(args, { from: 'user' }); + expect(program.opts()).toEqual({ a: awaited }); + await expect(program.opts().a).resolves.toEqual(resolvedValue); +}); diff --git a/typings/index.d.ts b/typings/index.d.ts index 695c3bd25..1a065f5f8 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -65,6 +65,11 @@ export class Argument { */ argParser(fn: (value: string, previous: T) => T): this; + /** + * When set to `true` (the default), next call to the function provided via `.argParser()` will be chained to its return value if it is thenable. + */ + chainArgParserCalls(chained?: boolean): this; + /** * Only allow argument value to be one of choices. */ @@ -159,6 +164,11 @@ export class Option { */ argParser(fn: (value: string, previous: T) => T): this; + /** + * When set to `true` (the default), next call to the function provided via `.argParser()` will be chained to its return value if it is thenable. + */ + chainArgParserCalls(chained?: boolean): this; + /** * Whether the option is mandatory and must have a value after parsing. */ diff --git a/typings/index.test-d.ts b/typings/index.test-d.ts index 734036fad..7c1f5dd6c 100644 --- a/typings/index.test-d.ts +++ b/typings/index.test-d.ts @@ -418,6 +418,11 @@ expectType(baseOption.fullDescription()); expectType(baseOption.argParser((value: string) => parseInt(value))); expectType(baseOption.argParser((value: string, previous: string[]) => { return previous.concat(value); })); +// chainArgParserCalls +expectType(baseOption.chainArgParserCalls()); +expectType(baseOption.chainArgParserCalls(true)); +expectType(baseOption.chainArgParserCalls(false)); + // makeOptionMandatory expectType(baseOption.makeOptionMandatory()); expectType(baseOption.makeOptionMandatory(true)); @@ -466,6 +471,11 @@ expectType(baseArgument.default(60, 'one minute')); expectType(baseArgument.argParser((value: string) => parseInt(value))); expectType(baseArgument.argParser((value: string, previous: string[]) => { return previous.concat(value); })); +// chainArgParserCalls +expectType(baseArgument.chainArgParserCalls()); +expectType(baseArgument.chainArgParserCalls(true)); +expectType(baseArgument.chainArgParserCalls(false)); + // choices expectType(baseArgument.choices(['a', 'b'])); expectType(baseArgument.choices(['a', 'b'] as const));