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

Throw error when add option with clashing flags #2055

Merged
merged 11 commits into from
Nov 1, 2023
29 changes: 24 additions & 5 deletions lib/command.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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();

Expand All @@ -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.
Expand Down Expand Up @@ -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);
Expand Down
59 changes: 59 additions & 0 deletions tests/options.registerClash.test.js
Original file line number Diff line number Diff line change
@@ -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 <type>', '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 <type>', '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 <type>', '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 <type>', 'cheese type')
.addOption(new Option('-H, --cheese'));
}).toThrow('Cannot add option');
});
});