From c740fece3f0190e3d9886855b7943f0d0834927c Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 4 Nov 2023 17:39:47 +1300 Subject: [PATCH] Add check for overlapping command names or aliases Co-authored-by: aweebit --- lib/command.js | 38 +++++++++++++++++++--- tests/command.registerClash.test.js | 49 +++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 4 deletions(-) create mode 100644 tests/command.registerClash.test.js diff --git a/lib/command.js b/lib/command.js index bc7d8dbcb..d8bfc9a3f 100644 --- a/lib/command.js +++ b/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; diff --git a/tests/command.registerClash.test.js b/tests/command.registerClash.test.js new file mode 100644 index 000000000..4930ecccc --- /dev/null +++ b/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'); +});