diff --git a/index.js b/index.js index 641946960..ec9887905 100644 --- a/index.js +++ b/index.js @@ -1475,12 +1475,17 @@ class Command extends EventEmitter { outputHelpIfRequested(this, parsed.unknown); this._checkForMissingMandatoryOptions(); - if (parsed.unknown.length > 0) { - this.unknownOption(parsed.unknown[0]); - } + + // We do not always call this check to avoid masking a "better" error, like unknown command. + const checkForUnknownOptions = () => { + if (parsed.unknown.length > 0) { + this.unknownOption(parsed.unknown[0]); + } + }; const commandEvent = `command:${this.name()}`; if (this._actionHandler) { + checkForUnknownOptions(); // Check expected arguments and collect variadic together. const args = this.args.slice(); this._args.forEach((arg, i) => { @@ -1498,19 +1503,24 @@ class Command extends EventEmitter { this._actionHandler(args); if (this.parent) this.parent.emit(commandEvent, operands, unknown); // legacy } else if (this.parent && this.parent.listenerCount(commandEvent)) { + checkForUnknownOptions(); this.parent.emit(commandEvent, operands, unknown); // legacy } else if (operands.length) { - if (this._findCommand('*')) { // legacy + if (this._findCommand('*')) { // legacy default command this._dispatchSubcommand('*', operands, unknown); } else if (this.listenerCount('command:*')) { + // skip option check, emit event for possible misspelling suggestion this.emit('command:*', operands, unknown); } else if (this.commands.length) { this.unknownCommand(); + } else { + checkForUnknownOptions(); } } else if (this.commands.length) { // This command has subcommands and nothing hooked up at this level, so display help. this.help({ error: true }); } else { + checkForUnknownOptions(); // fall through for caller to handle after calling .parse() } } diff --git a/tests/command.asterisk.test.js b/tests/command.asterisk.test.js index cf0e7f004..d975c868b 100644 --- a/tests/command.asterisk.test.js +++ b/tests/command.asterisk.test.js @@ -64,6 +64,45 @@ describe(".command('*')", () => { program.parse(['node', 'test', 'unrecognised-command']); expect(mockAction).toHaveBeenCalled(); }); + + test('when unrecognised argument and known option then asterisk action called', () => { + // This tests for a regression between v4 and v5. Known default option should not be rejected by program. + const mockAction = jest.fn(); + const program = new commander.Command(); + program + .command('install'); + const star = program + .command('*') + .arguments('[args...]') + .option('-d, --debug') + .action(mockAction); + program.parse(['node', 'test', 'unrecognised-command', '--debug']); + expect(mockAction).toHaveBeenCalled(); + expect(star.opts().debug).toEqual(true); + }); + + test('when non-command argument and unknown option then error for unknown option', () => { + // This is a change in behaviour from v2 which did not error, but is consistent with modern better detection of invalid options + const mockAction = jest.fn(); + const program = new commander.Command(); + program + .exitOverride() + .configureOutput({ + writeErr: () => {} + }) + .command('install'); + program + .command('*') + .arguments('[args...]') + .action(mockAction); + let caughtErr; + try { + program.parse(['node', 'test', 'some-argument', '--unknown']); + } catch (err) { + caughtErr = err; + } + expect(caughtErr.code).toEqual('commander.unknownOption'); + }); }); // Test .on explicitly rather than assuming covered by .command @@ -108,4 +147,18 @@ describe(".on('command:*')", () => { program.parse(['node', 'test', 'unrecognised-command']); expect(mockAction).toHaveBeenCalled(); }); + + test('when unrecognised command/argument and unknown option then listener called', () => { + // Give listener a chance to make a suggestion for misspelled command. The option + // could only be unknown because the command is not correct. + // Regression identified in https://github.com/tj/commander.js/issues/1460#issuecomment-772313494 + const mockAction = jest.fn(); + const program = new commander.Command(); + program + .command('install'); + program + .on('command:*', mockAction); + program.parse(['node', 'test', 'intsall', '--unknown']); + expect(mockAction).toHaveBeenCalled(); + }); }); diff --git a/tests/command.unknownCommand.test.js b/tests/command.unknownCommand.test.js index 844c8455a..7e8bfa0d9 100644 --- a/tests/command.unknownCommand.test.js +++ b/tests/command.unknownCommand.test.js @@ -1,6 +1,6 @@ const commander = require('../'); -describe('unknownOption', () => { +describe('unknownCommand', () => { // Optional. Use internal knowledge to suppress output to keep test output clean. let writeErrorSpy; @@ -63,6 +63,22 @@ describe('unknownOption', () => { expect(caughtErr.code).toBe('commander.unknownCommand'); }); + test('when unknown command and unknown option then error is for unknown command', () => { + // The unknown command is more useful since the option is for an unknown command (and might be + // ok if the command had been correctly spelled, say). + const program = new commander.Command(); + program + .exitOverride() + .command('sub'); + let caughtErr; + try { + program.parse('node test.js sbu --silly'.split(' ')); + } catch (err) { + caughtErr = err; + } + expect(caughtErr.code).toBe('commander.unknownCommand'); + }); + test('when unknown subcommand then help suggestion includes command path', () => { const program = new commander.Command(); program diff --git a/tests/command.unknownOption.test.js b/tests/command.unknownOption.test.js index 18ed188a5..4bceeeb8f 100644 --- a/tests/command.unknownOption.test.js +++ b/tests/command.unknownOption.test.js @@ -82,4 +82,17 @@ describe('unknownOption', () => { } expect(caughtErr.code).toBe('commander.unknownOption'); }); + + test('when specify unknown option with simple program then error', () => { + const program = new commander.Command(); + program + .exitOverride(); + let caughtErr; + try { + program.parse(['node', 'test', '--NONSENSE']); + } catch (err) { + caughtErr = err; + } + expect(caughtErr.code).toBe('commander.unknownOption'); + }); });