From 0df74531ad1c9c34088e60f50ae0f2405fa26195 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 2 Feb 2020 10:02:15 +1300 Subject: [PATCH 1/2] Throw errors for author errors. Detect bad variadic when added. --- tests/args.variadic.test.js | 16 ++----- ...ommand.executableSubcommand.lookup.test.js | 4 +- tests/command.exitOverride.test.js | 42 ------------------- 3 files changed, 6 insertions(+), 56 deletions(-) diff --git a/tests/args.variadic.test.js b/tests/args.variadic.test.js index 8be9f7a7c..83c3dae82 100644 --- a/tests/args.variadic.test.js +++ b/tests/args.variadic.test.js @@ -70,25 +70,17 @@ describe('variadic argument', () => { test('when program variadic argument not last then error', () => { const program = new commander.Command(); - program - .exitOverride() - .arguments(' [optionalArg]') - .action(jest.fn); expect(() => { - program.parse(['node', 'test', 'a']); - }).toThrow("error: variadic arguments must be last 'variadicArg'"); + program.arguments(' [optionalArg]'); + }).toThrow("only the last argument can be variadic 'variadicArg'"); }); test('when command variadic argument not last then error', () => { const program = new commander.Command(); - program - .exitOverride() - .command('sub [optionalArg]') - .action(jest.fn); expect(() => { - program.parse(['node', 'test', 'sub', 'a']); - }).toThrow("error: variadic arguments must be last 'variadicArg'"); + program.command('sub [optionalArg]'); + }).toThrow("only the last argument can be variadic 'variadicArg'"); }); }); diff --git a/tests/command.executableSubcommand.lookup.test.js b/tests/command.executableSubcommand.lookup.test.js index f18bfb694..2cad054c8 100644 --- a/tests/command.executableSubcommand.lookup.test.js +++ b/tests/command.executableSubcommand.lookup.test.js @@ -12,7 +12,7 @@ test('when subcommand file missing then error', (done) => { // Get uncaught thrown error on Windows expect(stderr.length).toBeGreaterThan(0); } else { - expect(stderr).toBe('error: pm-list(1) does not exist\n'); + expect(stderr).toMatch(new RegExp(/Error: 'pm-list' does not exist/)); } done(); }); @@ -24,7 +24,7 @@ test('when alias subcommand file missing then error', (done) => { // Get uncaught thrown error on Windows expect(stderr.length).toBeGreaterThan(0); } else { - expect(stderr).toBe('error: pm-list(1) does not exist\n'); + expect(stderr).toMatch(new RegExp(/Error: 'pm-list' does not exist/)); } done(); }); diff --git a/tests/command.exitOverride.test.js b/tests/command.exitOverride.test.js index fd75d5e3f..5160c3687 100644 --- a/tests/command.exitOverride.test.js +++ b/tests/command.exitOverride.test.js @@ -13,8 +13,6 @@ function expectCommanderError(err, exitCode, code, message) { expect(err.message).toBe(message); } -const testOrSkipOnWindows = (process.platform === 'win32') ? test.skip : test; - describe('.exitOverride and error details', () => { // Use internal knowledge to suppress output to keep test output clean. let consoleErrorSpy; @@ -174,25 +172,6 @@ describe('.exitOverride and error details', () => { expectCommanderError(caughtErr, 0, 'commander.version', myVersion); }); - test('when program variadic argument not last then throw CommanderError', () => { - // Note: this error is notified during parse, although could have been detected at declaration. - const program = new commander.Command(); - program - .exitOverride() - .arguments(' [optionalArg]') - .action(jest.fn); - - let caughtErr; - try { - program.parse(['node', 'test', 'a']); - } catch (err) { - caughtErr = err; - } - - expect(consoleErrorSpy).toHaveBeenCalled(); - expectCommanderError(caughtErr, 1, 'commander.variadicArgNotLast', "error: variadic arguments must be last 'myVariadicArg'"); - }); - test('when executableSubcommand succeeds then call exitOverride', (done) => { const pm = path.join(__dirname, 'fixtures/pm'); const program = new commander.Command(); @@ -206,27 +185,6 @@ describe('.exitOverride and error details', () => { program.parse(['node', pm, 'silent']); }); - // Throws directly on Windows - testOrSkipOnWindows('when executableSubcommand fails then call exitOverride', (done) => { - // Tricky for override, get called for `error` event then `exit` event. - const exitCallback = jest.fn() - .mockImplementationOnce((err) => { - expectCommanderError(err, 1, 'commander.executeSubCommandAsync', '(error)'); - expect(err.nestedError.code).toBe('ENOENT'); - }) - .mockImplementation((err) => { - expectCommanderError(err, 0, 'commander.executeSubCommandAsync', '(close)'); - done(); - }); - const pm = path.join(__dirname, 'fixtures/pm'); - const program = new commander.Command(); - program - .exitOverride(exitCallback) - .command('does-not-exist', 'fail'); - - program.parse(['node', pm, 'does-not-exist']); - }); - test('when mandatory program option missing then throw CommanderError', () => { const optionFlags = '-p, --pepper '; const program = new commander.Command(); From 367980f06142875f28a36d5f9b290ee8a6018c42 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 2 Feb 2020 10:03:31 +1300 Subject: [PATCH 2/2] Throw errors for author errors. Detect bad variadic when added. --- index.js | 34 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/index.js b/index.js index 311781ff0..fe354d19f 100644 --- a/index.js +++ b/index.js @@ -196,7 +196,7 @@ class Command extends EventEmitter { */ addCommand(cmd) { - if (!cmd._name) throw new Error('Command passed to .AddCommand must have a name'); + if (!cmd._name) throw new Error('Command passed to .addCommand() must have a name'); // To keep things simple, block automatic name generation for deeply nested executables. // Fail fast and detect when adding rather than later when parsing. @@ -300,6 +300,11 @@ class Command extends EventEmitter { self._args.push(argDetails); } }); + this._args.forEach(function(arg, i) { + if (arg.variadic && i < self._args.length - 1) { + throw new Error(`only the last argument can be variadic '${arg.name}'`); + } + }); return this; }; @@ -566,8 +571,7 @@ class Command extends EventEmitter { storeOptionsAsProperties(value) { this._storeOptionsAsProperties = (value === undefined) || value; if (this.options.length) { - // This is for programmer, not end user. - console.error('Commander usage error: call storeOptionsAsProperties before adding options'); + throw new Error('call .storeOptionsAsProperties() before adding options'); } return this; }; @@ -736,9 +740,12 @@ class Command extends EventEmitter { } proc.on('error', function(err) { if (err.code === 'ENOENT') { - console.error('error: %s(1) does not exist', bin); + const executableMissing = `'${bin}' does not exist + - if '${subcommand._name}' is not meant to be an executable command, remove description parameter from '.command()' and use '.description()' instead + - if the default executable name is not suitable, use the executableFile option to supply a custom name`; + throw new Error(executableMissing); } else if (err.code === 'EACCES') { - console.error('error: %s(1) not executable', bin); + throw new Error(`'${bin}' not executable`); } if (!exitCallback) { process.exit(1); @@ -809,10 +816,6 @@ class Command extends EventEmitter { if (arg.required && args[i] == null) { self.missingArgument(arg.name); } else if (arg.variadic) { - if (i !== self._args.length - 1) { - self.variadicArgNotLast(arg.name); - } - args[i] = args.splice(i); } }); @@ -1068,19 +1071,6 @@ class Command extends EventEmitter { this._exit(1, 'commander.unknownCommand', message); }; - /** - * Variadic argument with `name` is not the last argument as required. - * - * @param {String} name - * @api private - */ - - variadicArgNotLast(name) { - const message = `error: variadic arguments must be last '${name}'`; - console.error(message); - this._exit(1, 'commander.variadicArgNotLast', message); - }; - /** * Set the program version to `str`. *