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

Prevent commands being created with overlapping names or aliases #2059

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 34 additions & 4 deletions lib/command.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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
Expand All @@ -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.
*
Expand Down Expand Up @@ -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;
Expand Down
49 changes: 49 additions & 0 deletions tests/command.registerClash.test.js
Original file line number Diff line number Diff line change
@@ -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');
});