diff --git a/lib/command.js b/lib/command.js index cd6614f0c..1a1424c5e 100644 --- a/lib/command.js +++ b/lib/command.js @@ -946,6 +946,7 @@ Expecting one of '${allowedValues.join("', '")}'`); // Not checking for help first. Unlikely to have mandatory and executable, and can't robustly test for help flags in external command. this._checkForMissingMandatoryOptions(); + this._checkForConflictingOptions(); // executableFile and executableDir might be full path, or just a name let executableFile = subcommand._executableFile || `${this._name}-${subcommand._name}`; @@ -1294,7 +1295,7 @@ Expecting one of '${allowedValues.join("', '")}'`); /** * Display an error message if a mandatory option does not have a value. - * Lazy calling after checking for help flags from leaf subcommand. + * Called after checking for help flags in leaf subcommand. * * @api private */ @@ -1311,11 +1312,11 @@ Expecting one of '${allowedValues.join("', '")}'`); } /** - * Display an error message if conflicting options are used together. + * Display an error message if conflicting options are used together in this. * * @api private */ - _checkForConflictingOptions() { + _checkForConflictingLocalOptions() { const definedNonDefaultOptions = this.options.filter( (option) => { const optionKey = option.attributeName(); @@ -1340,6 +1341,19 @@ Expecting one of '${allowedValues.join("', '")}'`); }); } + /** + * Display an error message if conflicting options are used together. + * Called after checking for help flags in leaf subcommand. + * + * @api private + */ + _checkForConflictingOptions() { + // Walk up hierarchy so can call in subcommand after checking for displaying help. + for (let cmd = this; cmd; cmd = cmd.parent) { + cmd._checkForConflictingLocalOptions(); + } + } + /** * Parse options from `argv` removing known options, * and return argv split into operands and unknown arguments. diff --git a/tests/command.conflicts.test.js b/tests/options.conflicts.test.js similarity index 83% rename from tests/command.conflicts.test.js rename to tests/options.conflicts.test.js index 8e2ce9860..72884b802 100644 --- a/tests/command.conflicts.test.js +++ b/tests/options.conflicts.test.js @@ -1,3 +1,4 @@ +const path = require('path'); const commander = require('../'); describe('command with conflicting options', () => { @@ -7,7 +8,8 @@ describe('command with conflicting options', () => { program .exitOverride() .configureOutput({ - writeErr: () => {} + writeErr: () => {}, + writeOut: () => {} }) .command('foo') .addOption(new commander.Option('-s, --silent', "Don't print anything").env('SILENT')) @@ -253,4 +255,57 @@ describe('command with conflicting options', () => { program.parse('node test.js bar --a --b'.split(' ')); }).toThrow("error: option '--b' cannot be used with option '--a'"); }); + + test('when conflict on program calling action subcommand then throw conflict', () => { + const { program } = makeProgram(); + let exception; + + program + .addOption(new commander.Option('--black')) + .addOption(new commander.Option('--white').conflicts('black')); + + try { + program.parse('--white --black foo'.split(' '), { from: 'user' }); + } catch (err) { + exception = err; + } + expect(exception).not.toBeUndefined(); + expect(exception.code).toBe('commander.conflictingOption'); + }); + + test('when conflict on program calling action subcommand with help then show help', () => { + const { program } = makeProgram(); + let exception; + + program + .addOption(new commander.Option('--black')) + .addOption(new commander.Option('--white').conflicts('black')); + + try { + program.parse('--white --black foo --help'.split(' '), { from: 'user' }); + } catch (err) { + exception = err; + } + expect(exception).not.toBeUndefined(); + expect(exception.code).toBe('commander.helpDisplayed'); + }); + + test('when conflict on program calling external subcommand then throw conflict', () => { + const { program } = makeProgram(); + let exception; + + program + .addOption(new commander.Option('--black')) + .addOption(new commander.Option('--white').conflicts('black')); + const pm = path.join(__dirname, './fixtures/pm'); + program.command('ext', 'external command', { executableFile: pm }); + + try { + program.parse('--white --black ext'.split(' '), { from: 'user' }); + } catch (err) { + exception = err; + } + expect(exception).not.toBeUndefined(); + expect(exception.code).toBe('commander.conflictingOption'); + }); });