diff --git a/lib/command.js b/lib/command.js index 8cca02598..bc7d8dbcb 100644 --- a/lib/command.js +++ b/lib/command.js @@ -535,6 +535,25 @@ Expecting one of '${allowedValues.join("', '")}'`); throw err; } } + /** + * Check for option flag conflicts. + * Register option if no conflicts found. + * Throw otherwise. + * + * @param {Option} option + * @api private + */ + + _registerOption(option) { + const matchingOption = (option.short && this._findOption(option.short)) || + (option.long && this._findOption(option.long)); + if (matchingOption) { + const matchingFlag = (option.long && this._findOption(option.long)) ? option.long : option.short; + throw new Error(`Cannot add option '${option.flags}'${this._name && ` to command '${this._name}'`} due to conflicting flag '${matchingFlag}' +- already used by option '${matchingOption.flags}'`); + } + this.options.push(option); + } /** * Add an option. @@ -543,6 +562,8 @@ Expecting one of '${allowedValues.join("', '")}'`); * @return {Command} `this` command for chaining */ addOption(option) { + this._registerOption(option); + const oname = option.name(); const name = option.attributeName(); @@ -557,9 +578,6 @@ Expecting one of '${allowedValues.join("', '")}'`); this.setOptionValueWithSource(name, option.defaultValue, 'default'); } - // register the option - this.options.push(option); - // handler for cli and env supplied values const handleOptionValue = (val, invalidValueMessage, valueSource) => { // val is null for optional option used without an optional-argument. @@ -1824,8 +1842,9 @@ Expecting one of '${allowedValues.join("', '")}'`); flags = flags || '-V, --version'; description = description || 'output the version number'; const versionOption = this.createOption(flags, description); - this._versionOptionName = versionOption.attributeName(); // [sic] not defined in constructor, partly legacy, partly only needed at root - this.options.push(versionOption); + this._versionOptionName = versionOption.attributeName(); + this._registerOption(versionOption); + this.on('option:' + versionOption.name(), () => { this._outputConfiguration.writeOut(`${str}\n`); this._exit(0, 'commander.version', str); diff --git a/tests/options.registerClash.test.js b/tests/options.registerClash.test.js new file mode 100644 index 000000000..b5d531379 --- /dev/null +++ b/tests/options.registerClash.test.js @@ -0,0 +1,59 @@ +const { Command, Option } = require('../'); + +describe('.option()', () => { + test('when short option flag conflicts then throws', () => { + expect(() => { + const program = new Command(); + program + .option('-c, --cheese ', 'cheese type') + .option('-c, --conflict'); + }).toThrow('Cannot add option'); + }); + + test('when long option flag conflicts then throws', () => { + expect(() => { + const program = new Command(); + program + .option('-c, --cheese ', 'cheese type') + .option('-H, --cheese'); + }).toThrow('Cannot add option'); + }); + + test('when use help options separately then does not throw', () => { + expect(() => { + const program = new Command(); + program + .option('-h, --help', 'display help'); + }).not.toThrow(); + }); + + test('when reuse flags in subcommand then does not throw', () => { + expect(() => { + const program = new Command(); + program + .option('e, --example'); + program.command('sub') + .option('e, --example'); + }).not.toThrow(); + }); +}); + +describe('.addOption()', () => { + test('when short option flags conflicts then throws', () => { + expect(() => { + const program = new Command(); + program + .option('-c, --cheese ', 'cheese type') + .addOption(new Option('-c, --conflict')); + }).toThrow('Cannot add option'); + }); + + test('when long option flags conflicts then throws', () => { + expect(() => { + const program = new Command(); + program + .option('-c, --cheese ', 'cheese type') + .addOption(new Option('-H, --cheese')); + }).toThrow('Cannot add option'); + }); +});