Throw error when add option with clashing flags (#2055)

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);

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 <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');
+  });
+});