Skip to content

Commit b96af40

Browse files
authored
Throw error when add option with clashing flags (#2055)
1 parent d90e81e commit b96af40

File tree

2 files changed

+83
-5
lines changed

2 files changed

+83
-5
lines changed

lib/command.js

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,25 @@ Expecting one of '${allowedValues.join("', '")}'`);
535535
throw err;
536536
}
537537
}
538+
/**
539+
* Check for option flag conflicts.
540+
* Register option if no conflicts found.
541+
* Throw otherwise.
542+
*
543+
* @param {Option} option
544+
* @api private
545+
*/
546+
547+
_registerOption(option) {
548+
const matchingOption = (option.short && this._findOption(option.short)) ||
549+
(option.long && this._findOption(option.long));
550+
if (matchingOption) {
551+
const matchingFlag = (option.long && this._findOption(option.long)) ? option.long : option.short;
552+
throw new Error(`Cannot add option '${option.flags}'${this._name && ` to command '${this._name}'`} due to conflicting flag '${matchingFlag}'
553+
- already used by option '${matchingOption.flags}'`);
554+
}
555+
this.options.push(option);
556+
}
538557

539558
/**
540559
* Add an option.
@@ -543,6 +562,8 @@ Expecting one of '${allowedValues.join("', '")}'`);
543562
* @return {Command} `this` command for chaining
544563
*/
545564
addOption(option) {
565+
this._registerOption(option);
566+
546567
const oname = option.name();
547568
const name = option.attributeName();
548569

@@ -557,9 +578,6 @@ Expecting one of '${allowedValues.join("', '")}'`);
557578
this.setOptionValueWithSource(name, option.defaultValue, 'default');
558579
}
559580

560-
// register the option
561-
this.options.push(option);
562-
563581
// handler for cli and env supplied values
564582
const handleOptionValue = (val, invalidValueMessage, valueSource) => {
565583
// val is null for optional option used without an optional-argument.
@@ -1824,8 +1842,9 @@ Expecting one of '${allowedValues.join("', '")}'`);
18241842
flags = flags || '-V, --version';
18251843
description = description || 'output the version number';
18261844
const versionOption = this.createOption(flags, description);
1827-
this._versionOptionName = versionOption.attributeName(); // [sic] not defined in constructor, partly legacy, partly only needed at root
1828-
this.options.push(versionOption);
1845+
this._versionOptionName = versionOption.attributeName();
1846+
this._registerOption(versionOption);
1847+
18291848
this.on('option:' + versionOption.name(), () => {
18301849
this._outputConfiguration.writeOut(`${str}\n`);
18311850
this._exit(0, 'commander.version', str);
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
const { Command, Option } = require('../');
2+
3+
describe('.option()', () => {
4+
test('when short option flag conflicts then throws', () => {
5+
expect(() => {
6+
const program = new Command();
7+
program
8+
.option('-c, --cheese <type>', 'cheese type')
9+
.option('-c, --conflict');
10+
}).toThrow('Cannot add option');
11+
});
12+
13+
test('when long option flag conflicts then throws', () => {
14+
expect(() => {
15+
const program = new Command();
16+
program
17+
.option('-c, --cheese <type>', 'cheese type')
18+
.option('-H, --cheese');
19+
}).toThrow('Cannot add option');
20+
});
21+
22+
test('when use help options separately then does not throw', () => {
23+
expect(() => {
24+
const program = new Command();
25+
program
26+
.option('-h, --help', 'display help');
27+
}).not.toThrow();
28+
});
29+
30+
test('when reuse flags in subcommand then does not throw', () => {
31+
expect(() => {
32+
const program = new Command();
33+
program
34+
.option('e, --example');
35+
program.command('sub')
36+
.option('e, --example');
37+
}).not.toThrow();
38+
});
39+
});
40+
41+
describe('.addOption()', () => {
42+
test('when short option flags conflicts then throws', () => {
43+
expect(() => {
44+
const program = new Command();
45+
program
46+
.option('-c, --cheese <type>', 'cheese type')
47+
.addOption(new Option('-c, --conflict'));
48+
}).toThrow('Cannot add option');
49+
});
50+
51+
test('when long option flags conflicts then throws', () => {
52+
expect(() => {
53+
const program = new Command();
54+
program
55+
.option('-c, --cheese <type>', 'cheese type')
56+
.addOption(new Option('-H, --cheese'));
57+
}).toThrow('Cannot add option');
58+
});
59+
});

0 commit comments

Comments
 (0)