Add check for overlapping command names or aliases (#2059)

Co-authored-by: aweebit <[email protected]>

lib/command.js

@@ -165,7 +165,7 @@ class Command extends EventEmitter {
     cmd._hidden = !!(opts.noHelp || opts.hidden); // noHelp is deprecated old name for hidden
     cmd._executableFile = opts.executableFile || null; // Custom name for executable file, set missing to null to match constructor
     if (args) cmd.arguments(args);
-    this.commands.push(cmd);
+    this._registerCommand(cmd);
     cmd.parent = this;
     cmd.copyInheritedSettings(this);
 
@@ -282,7 +282,7 @@ class Command extends EventEmitter {
     if (opts.isDefault) this._defaultCommandName = cmd._name;
     if (opts.noHelp || opts.hidden) cmd._hidden = true; // modifying passed command due to existing implementation
 
-    this.commands.push(cmd);
+    this._registerCommand(cmd);
     cmd.parent = this;
     cmd._checkForBrokenPassThrough();
 
@@ -535,10 +535,10 @@ Expecting one of '${allowedValues.join("', '")}'`);
       throw err;
     }
   }
+
   /**
    * Check for option flag conflicts.
-   * Register option if no conflicts found.
-   * Throw otherwise.
+   * Register option if no conflicts found, or throw on conflict.
    *
    * @param {Option} option
    * @api private
@@ -552,9 +552,33 @@ Expecting one of '${allowedValues.join("', '")}'`);
       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);
   }
 
+  /**
+   * Check for command name and alias conflicts with existing commands.
+   * Register command if no conflicts found, or throw on conflict.
+   *
+   * @param {Command} command
+   * @api private
+   */
+
+  _registerCommand(command) {
+    const knownBy = (cmd) => {
+      return [cmd.name()].concat(cmd.aliases());
+    };
+
+    const alreadyUsed = knownBy(command).find((name) => this._findCommand(name));
+    if (alreadyUsed) {
+      const existingCmd = knownBy(this._findCommand(alreadyUsed)).join('|');
+      const newCmd = knownBy(command).join('|');
+      throw new Error(`cannot add command '${newCmd}' as already have command '${existingCmd}'`);
+    }
+
+    this.commands.push(command);
+  }
+
   /**
    * Add an option.
    *
@@ -1900,6 +1924,12 @@ Expecting one of '${allowedValues.join("', '")}'`);
     }
 
     if (alias === command._name) throw new Error('Command alias can\'t be the same as its name');
+    const matchingCommand = this.parent?._findCommand(alias);
+    if (matchingCommand) {
+      // c.f. _registerCommand
+      const existingCmd = [matchingCommand.name()].concat(matchingCommand.aliases()).join('|');
+      throw new Error(`cannot add alias '${alias}' to command '${this.name()}' as already have command '${existingCmd}'`);
+    }
 
     command._aliases.push(alias);
     return this;

tests/command.registerClash.test.js

@@ -0,0 +1,49 @@
+const { Command } = require('../');
+
+test('when command name conflicts with existing name then throw', () => {
+  expect(() => {
+    const program = new Command();
+    program.command('one');
+    program.command('one');
+  }).toThrow('cannot add command');
+});
+
+test('when command name conflicts with existing alias then throw', () => {
+  expect(() => {
+    const program = new Command();
+    program.command('one').alias('1');
+    program.command('1');
+  }).toThrow('cannot add command');
+});
+
+test('when command alias conflicts with existing name then throw', () => {
+  expect(() => {
+    const program = new Command();
+    program.command('one');
+    program.command('1').alias('one');
+  }).toThrow('cannot add alias');
+});
+
+test('when command alias conflicts with existing alias then throw', () => {
+  expect(() => {
+    const program = new Command();
+    program.command('one').alias('1');
+    program.command('unity').alias('1');
+  }).toThrow('cannot add alias');
+});
+
+test('when .addCommand name conflicts with existing name then throw', () => {
+  expect(() => {
+    const program = new Command();
+    program.command('one');
+    program.addCommand(new Command('one'));
+  }).toThrow('cannot add command');
+});
+
+test('when .addCommand alias conflicts with existing name then throw', () => {
+  expect(() => {
+    const program = new Command();
+    program.command('one');
+    program.addCommand(new Command('unity').alias('one'));
+  }).toThrow('cannot add command');
+});