From 7f9edba0280be090c5207b1fd13fc47220b4ffdf Mon Sep 17 00:00:00 2001 From: John Gee Date: Tue, 8 Oct 2019 14:15:29 +1300 Subject: [PATCH] Add .requiredOption() for mandatory options (#1071) * Add declaration of requiredOption and mandatory property * Add initial implementation for error detection for mandatory options * Add unit tests for mandatory options * Add detailed test for exitOverride intercept for missing mandatory option * Add .requiredOption to README * Reword mandatory description. * Lookup option value using attributeName, not name * Expand tests - option name different than property name - both of option/requiredOption for negated option - add exitOverride to allow Jest to catch unexpected errors * Add .requiredOption calls to TypeScript dummy program * Tweak requiredOption example file --- Readme.md | 19 +++ examples/options-required.js | 19 +++ index.js | 160 ++++++++++++++------ tests/command.exitOverride.test.js | 17 +++ tests/options.mandatory.test.js | 226 +++++++++++++++++++++++++++++ typings/commander-tests.ts | 10 ++ typings/index.d.ts | 14 +- 7 files changed, 416 insertions(+), 49 deletions(-) create mode 100644 examples/options-required.js create mode 100644 tests/options.mandatory.test.js diff --git a/Readme.md b/Readme.md index c1ba24672..9375afe26 100644 --- a/Readme.md +++ b/Readme.md @@ -15,6 +15,7 @@ The complete solution for [node.js](http://nodejs.org) command-line interfaces, - [Default option value](#default-option-value) - [Other option types, negatable boolean and flag|value](#other-option-types-negatable-boolean-and-flagvalue) - [Custom option processing](#custom-option-processing) + - [Required option](#required-option) - [Version option](#version-option) - [Commands](#commands) - [Specify the argument syntax](#specify-the-argument-syntax) @@ -242,6 +243,24 @@ $ custom --list x,y,z [ 'x', 'y', 'z' ] ``` +### Required option + +You may specify a required (mandatory) option using `.requiredOption`. The option must be specified on the command line, or by having a default value. The method is otherwise the same as `.option` in format, taking flags and description, and optional default value or custom processing. + +```js +const program = require('commander'); + +program + .requiredOption('-c, --cheese ', 'pizza must have cheese'); + +program.parse(process.argv); +``` + +``` +$ pizza +error: required option '-c, --cheese ' not specified +``` + ### Version option The optional `version` method adds handling for displaying the command version. The default option flags are `-V` and `--version`, and when present the command prints the version number and exits. diff --git a/examples/options-required.js b/examples/options-required.js new file mode 100644 index 000000000..9963ad6ab --- /dev/null +++ b/examples/options-required.js @@ -0,0 +1,19 @@ +#!/usr/bin/env node + +// This is used as an example in the README for: +// Required option +// You may specify a required (mandatory) option using `.requiredOption`. +// The option must be specified on the command line, or by having a default value. +// +// Example output pretending command called pizza (or try directly with `node options-required.js`) +// +// $ pizza +// error: required option '-c, --cheese ' not specified + +const commander = require('..'); // For running direct from git clone of commander repo +const program = new commander.Command(); + +program + .requiredOption('-c, --cheese ', 'pizza must have cheese'); + +program.parse(process.argv); diff --git a/index.js b/index.js index 554fa434c..3efcf38e2 100644 --- a/index.js +++ b/index.js @@ -43,8 +43,9 @@ exports.Option = Option; function Option(flags, description) { this.flags = flags; - this.required = flags.indexOf('<') >= 0; - this.optional = flags.indexOf('[') >= 0; + this.required = flags.indexOf('<') >= 0; // A value must be supplied when the option is specified. + this.optional = flags.indexOf('[') >= 0; // A value is optional when the option is specified. + this.mandatory = false; // The option must have a value after parsing, which usually means it must be specified on command line. this.negate = flags.indexOf('-no-') !== -1; flags = flags.split(/[ ,|]+/); if (flags.length > 1 && !/^[[<]/.test(flags[1])) this.short = flags.shift(); @@ -365,62 +366,23 @@ Command.prototype.action = function(fn) { }; /** - * Define option with `flags`, `description` and optional - * coercion `fn`. - * - * The `flags` string should contain both the short and long flags, - * separated by comma, a pipe or space. The following are all valid - * all will output this way when `--help` is used. - * - * "-p, --pepper" - * "-p|--pepper" - * "-p --pepper" - * - * Examples: - * - * // simple boolean defaulting to undefined - * program.option('-p, --pepper', 'add pepper'); - * - * program.pepper - * // => undefined - * - * --pepper - * program.pepper - * // => true - * - * // simple boolean defaulting to true (unless non-negated option is also defined) - * program.option('-C, --no-cheese', 'remove cheese'); - * - * program.cheese - * // => true - * - * --no-cheese - * program.cheese - * // => false - * - * // required argument - * program.option('-C, --chdir ', 'change the working directory'); - * - * --chdir /tmp - * program.chdir - * // => "/tmp" - * - * // optional argument - * program.option('-c, --cheese [type]', 'add cheese [marble]'); + * Internal implementation shared by .option() and .requiredOption() * + * @param {Object} config * @param {String} flags * @param {String} description - * @param {Function|*} [fn] or default + * @param {Function|*} [fn] - custom option processing function or default vaue * @param {*} [defaultValue] * @return {Command} for chaining - * @api public + * @api private */ -Command.prototype.option = function(flags, description, fn, defaultValue) { +Command.prototype._optionEx = function(config, flags, description, fn, defaultValue) { var self = this, option = new Option(flags, description), oname = option.name(), name = option.attributeName(); + option.mandatory = !!config.mandatory; // default as 3rd arg if (typeof fn !== 'function') { @@ -482,6 +444,80 @@ Command.prototype.option = function(flags, description, fn, defaultValue) { return this; }; +/** + * Define option with `flags`, `description` and optional + * coercion `fn`. + * + * The `flags` string should contain both the short and long flags, + * separated by comma, a pipe or space. The following are all valid + * all will output this way when `--help` is used. + * + * "-p, --pepper" + * "-p|--pepper" + * "-p --pepper" + * + * Examples: + * + * // simple boolean defaulting to undefined + * program.option('-p, --pepper', 'add pepper'); + * + * program.pepper + * // => undefined + * + * --pepper + * program.pepper + * // => true + * + * // simple boolean defaulting to true (unless non-negated option is also defined) + * program.option('-C, --no-cheese', 'remove cheese'); + * + * program.cheese + * // => true + * + * --no-cheese + * program.cheese + * // => false + * + * // required argument + * program.option('-C, --chdir ', 'change the working directory'); + * + * --chdir /tmp + * program.chdir + * // => "/tmp" + * + * // optional argument + * program.option('-c, --cheese [type]', 'add cheese [marble]'); + * + * @param {String} flags + * @param {String} description + * @param {Function|*} [fn] - custom option processing function or default vaue + * @param {*} [defaultValue] + * @return {Command} for chaining + * @api public + */ + +Command.prototype.option = function(flags, description, fn, defaultValue) { + return this._optionEx({}, flags, description, fn, defaultValue); +}; + +/* + * Add a required option which must have a value after parsing. This usually means + * the option must be specified on the command line. (Otherwise the same as .option().) + * + * The `flags` string should contain both the short and long flags, separated by comma, a pipe or space. + * + * @param {String} flags + * @param {String} description + * @param {Function|*} [fn] - custom option processing function or default vaue + * @param {*} [defaultValue] + * @return {Command} for chaining + * @api public + */ + +Command.prototype.requiredOption = function(flags, description, fn, defaultValue) { + return this._optionEx({ mandatory: true }, flags, description, fn, defaultValue); +}; + /** * Allow unknown options on the command line. * @@ -789,6 +825,21 @@ Command.prototype.optionFor = function(arg) { } }; +/** + * Display an error message if a mandatory option does not have a value. + * + * @api private + */ + +Command.prototype._checkForMissingMandatoryOptions = function() { + const self = this; + this.options.forEach((anOption) => { + if (anOption.mandatory && (self[anOption.attributeName()] === undefined)) { + self.missingMandatoryOptionValue(anOption); + } + }); +}; + /** * Parse options from `argv` returning `argv` * void of these options. @@ -865,6 +916,8 @@ Command.prototype.parseOptions = function(argv) { args.push(arg); } + this._checkForMissingMandatoryOptions(); + return { args: args, unknown: unknownOptions }; }; @@ -917,6 +970,19 @@ Command.prototype.optionMissingArgument = function(option, flag) { this._exit(1, 'commander.optionMissingArgument', message); }; +/** + * `Option` does not have a value, and is a mandatory option. + * + * @param {String} option + * @api private + */ + +Command.prototype.missingMandatoryOptionValue = function(option) { + const message = `error: required option '${option.flags}' not specified`; + console.error(message); + this._exit(1, 'commander.missingMandatoryOptionValue', message); +}; + /** * Unknown option `flag`. * diff --git a/tests/command.exitOverride.test.js b/tests/command.exitOverride.test.js index ab2596c2b..db5fd148c 100644 --- a/tests/command.exitOverride.test.js +++ b/tests/command.exitOverride.test.js @@ -210,4 +210,21 @@ describe('.exitOverride and error details', () => { program.parse(['node', pm, 'does-not-exist']); }); + + test('when mandatory program option missing then throw CommanderError', () => { + const optionFlags = '-p, --pepper '; + const program = new commander.Command(); + program + .exitOverride() + .requiredOption(optionFlags, 'add pepper'); + + let caughtErr; + try { + program.parse(['node', 'test']); + } catch (err) { + caughtErr = err; + } + + expectCommanderError(caughtErr, 1, 'commander.missingMandatoryOptionValue', `error: required option '${optionFlags}' not specified`); + }); }); diff --git a/tests/options.mandatory.test.js b/tests/options.mandatory.test.js new file mode 100644 index 000000000..0fe633592 --- /dev/null +++ b/tests/options.mandatory.test.js @@ -0,0 +1,226 @@ +const commander = require('../'); + +// Assuming mandatory options behave as normal options apart from the mandatory aspect, not retesting all behaviour. + +describe('required program option with mandatory value specified', () => { + test('when program has required value specified then value as specified', () => { + const program = new commander.Command(); + program + .exitOverride() + .requiredOption('--cheese ', 'cheese type'); + program.parse(['node', 'test', '--cheese', 'blue']); + expect(program.cheese).toBe('blue'); + }); + + test('when program has option with name different than property then still recognised', () => { + const program = new commander.Command(); + program + .exitOverride() + .requiredOption('--cheese-type ', 'cheese type'); + program.parse(['node', 'test', '--cheese-type', 'blue']); + expect(program.cheeseType).toBe('blue'); + }); + + test('when program has required value default then default value', () => { + const program = new commander.Command(); + program + .exitOverride() + .requiredOption('--cheese ', 'cheese type', 'default'); + program.parse(['node', 'test']); + expect(program.cheese).toBe('default'); + }); + + test('when program has optional value flag specified then true', () => { + const program = new commander.Command(); + program + .exitOverride() + .requiredOption('--cheese [type]', 'cheese type'); + program.parse(['node', 'test', '--cheese']); + expect(program.cheese).toBe(true); + }); + + test('when program has optional value default then default value', () => { + const program = new commander.Command(); + program + .exitOverride() + .requiredOption('--cheese [type]', 'cheese type', 'default'); + program.parse(['node', 'test']); + expect(program.cheese).toBe('default'); + }); + + test('when program has value/no flag specified with value then specified value', () => { + const program = new commander.Command(); + program + .exitOverride() + .requiredOption('--cheese ', 'cheese type') + .requiredOption('--no-cheese', 'no cheese thanks'); + program.parse(['node', 'test', '--cheese', 'blue']); + expect(program.cheese).toBe('blue'); + }); + + test('when program has mandatory-yes/no flag specified with flag then true', () => { + const program = new commander.Command(); + program + .exitOverride() + .requiredOption('--cheese', 'cheese type') + .option('--no-cheese', 'no cheese thanks'); + program.parse(['node', 'test', '--cheese']); + expect(program.cheese).toBe(true); + }); + + test('when program has mandatory-yes/mandatory-no flag specified with flag then true', () => { + const program = new commander.Command(); + program + .exitOverride() + .requiredOption('--cheese', 'cheese type') + .requiredOption('--no-cheese', 'no cheese thanks'); + program.parse(['node', 'test', '--cheese']); + expect(program.cheese).toBe(true); + }); + + test('when program has yes/no flag specified negated then false', () => { + const program = new commander.Command(); + program + .exitOverride() + .requiredOption('--cheese ', 'cheese type') + .option('--no-cheese', 'no cheese thanks'); + program.parse(['node', 'test', '--no-cheese']); + expect(program.cheese).toBe(false); + }); + + test('when program has required value specified and subcommand then specified value', () => { + const program = new commander.Command(); + program + .exitOverride() + .requiredOption('--cheese ', 'cheese type') + .command('sub') + .action(() => {}); + program.parse(['node', 'test', '--cheese', 'blue', 'sub']); + expect(program.cheese).toBe('blue'); + }); +}); + +describe('required program option with mandatory value not specified', () => { + // Optional. Use internal knowledge to suppress output to keep test output clean. + let consoleErrorSpy; + + beforeAll(() => { + consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => { }); + }); + + afterEach(() => { + consoleErrorSpy.mockClear(); + }); + + afterAll(() => { + consoleErrorSpy.mockRestore(); + }); + + test('when program has required option not specified then error', () => { + const program = new commander.Command(); + program + .exitOverride() + .requiredOption('--cheese ', 'cheese type'); + + expect(() => { + program.parse(['node', 'test']); + }).toThrow(); + }); + + test('when program has optional option not specified then error', () => { + const program = new commander.Command(); + program + .exitOverride() + .requiredOption('--cheese [type]', 'cheese type'); + + expect(() => { + program.parse(['node', 'test']); + }).toThrow(); + }); + + test('when program has yes/no not specified then error', () => { + const program = new commander.Command(); + program + .exitOverride() + .requiredOption('--cheese', 'cheese type') + .option('--no-cheese', 'no cheese thanks'); + + expect(() => { + program.parse(['node', 'test']); + }).toThrow(); + }); + + test('when program has required value not specified and subcommand then error', () => { + const program = new commander.Command(); + program + .exitOverride() + .requiredOption('--cheese ', 'cheese type') + .command('sub') + .action(() => {}); + + expect(() => { + program.parse(['node', 'test', 'sub']); + }).toThrow(); + }); +}); + +describe('required command option with mandatory value specified', () => { + test('when command has required value specified then specified value', () => { + const program = new commander.Command(); + let cmdOptions; + program + .exitOverride() + .command('sub') + .requiredOption('--subby ', 'description') + .action((cmd) => { + cmdOptions = cmd; + }); + + program.parse(['node', 'test', 'sub', '--subby', 'blue']); + + expect(cmdOptions.subby).toBe('blue'); + }); +}); + +describe('required command option with mandatory value not specified', () => { + // Optional. Use internal knowledge to suppress output to keep test output clean. + let consoleErrorSpy; + + beforeAll(() => { + consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => { }); + }); + + afterEach(() => { + consoleErrorSpy.mockClear(); + }); + + afterAll(() => { + consoleErrorSpy.mockRestore(); + }); + + test('when command has required value not specified then eror', () => { + const program = new commander.Command(); + program + .exitOverride() + .command('sub') + .requiredOption('--subby ', 'description') + .action((cmd) => {}); + + expect(() => { + program.parse(['node', 'test', 'sub']); + }).toThrow(); + }); + + test('when command has required value but not called then no error', () => { + const program = new commander.Command(); + program + .exitOverride() + .command('sub') + .requiredOption('--subby ', 'description') + .action((cmd) => {}); + + expect(() => { + program.parse(['node', 'test']); + }).not.toThrow(); + }); +}); diff --git a/typings/commander-tests.ts b/typings/commander-tests.ts index 9b6f55147..25c00e9e5 100644 --- a/typings/commander-tests.ts +++ b/typings/commander-tests.ts @@ -89,6 +89,16 @@ program console.log('unknown option is allowed'); }); +program + .requiredOption('-a,--aaa', 'description') + .requiredOption('-b,--bbb ', 'description') + .requiredOption('-c,--ccc [value]', 'description') + .requiredOption('-d,--ddd ', 'description', 'default value') + .requiredOption('-e,--eee ', 'description', (value, memo) => { return value; }) + .requiredOption('-f,--fff ', 'description', (value, memo) => { return value; }, 'starting value') + .requiredOption('-g,--ggg ', 'description') + .requiredOption('-G,--no-ggg ', 'description for negation'); + program .version('0.0.1') .arguments(' [env]') diff --git a/typings/index.d.ts b/typings/index.d.ts index 2d94eb14e..7b9509603 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -18,8 +18,9 @@ declare namespace local { class Option { flags: string; - required: boolean; - optional: boolean; + required: boolean; // A value must be supplied when the option is specified. + optional: boolean; // A value is optional when the option is specified. + mandatory: boolean; // The option must have a value after parsing, which usually means it must be specified on command line. bool: boolean; short?: string; long: string; @@ -186,6 +187,15 @@ declare namespace local { option(flags: string, description?: string, fn?: ((arg1: any, arg2: any) => void) | RegExp, defaultValue?: any): Command; option(flags: string, description?: string, defaultValue?: any): Command; + /** + * Define a required option, which must have a value after parsing. This usually means + * the option must be specified on the command line. (Otherwise the same as .option().) + * + * The `flags` string should contain both the short and long flags, separated by comma, a pipe or space. + */ + requiredOption(flags: string, description?: string, fn?: ((arg1: any, arg2: any) => void) | RegExp, defaultValue?: any): Command; + requiredOption(flags: string, description?: string, defaultValue?: any): Command; + /** * Allow unknown options on the command line. *