Check command-arguments even without action handler

index.js

@@ -1559,11 +1559,8 @@ class Command extends EventEmitter {
           this.unknownOption(parsed.unknown[0]);
         }
       };
-
-      const commandEvent = `command:${this.name()}`;
-      if (this._actionHandler) {
-        checkForUnknownOptions();
-        // Check expected arguments and collect variadic together.
+      // Check for missing or extra arguments, and refactor the arguments for passing to action handler.
+      const getCheckedArguments = () => {
         const args = this.args.slice();
         this._args.forEach((arg, i) => {
           if (arg.required && args[i] == null) {
@@ -1576,11 +1573,17 @@ class Command extends EventEmitter {
         if (args.length > this._args.length) {
           this._excessArguments(args);
         }
+        return args;
+      };
 
-        this._actionHandler(args);
+      const commandEvent = `command:${this.name()}`;
+      if (this._actionHandler) {
+        checkForUnknownOptions();
+        this._actionHandler(getCheckedArguments());
         if (this.parent) this.parent.emit(commandEvent, operands, unknown); // legacy
       } else if (this.parent && this.parent.listenerCount(commandEvent)) {
         checkForUnknownOptions();
+        getCheckedArguments();
         this.parent.emit(commandEvent, operands, unknown); // legacy
       } else if (operands.length) {
         if (this._findCommand('*')) { // legacy default command
@@ -1592,12 +1595,14 @@ class Command extends EventEmitter {
           this.unknownCommand();
         } else {
           checkForUnknownOptions();
+          getCheckedArguments();
         }
       } 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();
+        getCheckedArguments();
         // fall through for caller to handle after calling .parse()
       }
     }

tests/command.allowExcessArguments.test.js

@@ -2,27 +2,21 @@ const commander = require('../');
 
 // Not testing output, just testing whether an error is detected.
 
-describe('allowUnknownOption', () => {
-  // Optional. Use internal knowledge to suppress output to keep test output clean.
-  let writeErrorSpy;
-
-  beforeAll(() => {
-    writeErrorSpy = jest.spyOn(process.stderr, 'write').mockImplementation(() => { });
-  });
-
-  afterEach(() => {
-    writeErrorSpy.mockClear();
-  });
-
-  afterAll(() => {
-    writeErrorSpy.mockRestore();
-  });
+describe.each([true, false])('allowExcessArguments when action handler: %s', (hasActionHandler) => {
+  function configureCommand(cmd) {
+    cmd
+      .exitOverride()
+      .configureOutput({
+        writeErr: () => {}
+      });
+    if (hasActionHandler) {
+      cmd.action(() => {});
+    }
+  }
 
   test('when specify excess program argument then no error by default', () => {
     const program = new commander.Command();
-    program
-      .exitOverride()
-      .action(() => {});
+    configureCommand(program);
 
     expect(() => {
       program.parse(['excess'], { from: 'user' });
@@ -31,10 +25,9 @@ describe('allowUnknownOption', () => {
 
   test('when specify excess program argument and allowExcessArguments(false) then error', () => {
     const program = new commander.Command();
+    configureCommand(program);
     program
-      .exitOverride()
-      .allowExcessArguments(false)
-      .action(() => {});
+      .allowExcessArguments(false);
 
     expect(() => {
       program.parse(['excess'], { from: 'user' });
@@ -43,10 +36,9 @@ describe('allowUnknownOption', () => {
 
   test('when specify excess program argument and allowExcessArguments() then no error', () => {
     const program = new commander.Command();
+    configureCommand(program);
     program
-      .exitOverride()
-      .allowExcessArguments()
-      .action(() => {});
+      .allowExcessArguments();
 
     expect(() => {
       program.parse(['excess'], { from: 'user' });
@@ -55,10 +47,9 @@ describe('allowUnknownOption', () => {
 
   test('when specify excess program argument and allowExcessArguments(true) then no error', () => {
     const program = new commander.Command();
+    configureCommand(program);
     program
-      .exitOverride()
-      .allowExcessArguments(true)
-      .action(() => {});
+      .allowExcessArguments(true);
 
     expect(() => {
       program.parse(['excess'], { from: 'user' });
@@ -67,10 +58,9 @@ describe('allowUnknownOption', () => {
 
   test('when specify excess command argument then no error (by default)', () => {
     const program = new commander.Command();
-    program
-      .exitOverride()
-      .command('sub')
-      .action(() => { });
+    const sub = program
+      .command('sub');
+    configureCommand(sub);
 
     expect(() => {
       program.parse(['sub', 'excess'], { from: 'user' });
@@ -79,12 +69,10 @@ describe('allowUnknownOption', () => {
 
   test('when specify excess command argument and allowExcessArguments(false) then error', () => {
     const program = new commander.Command();
-    program
-      .exitOverride()
+    const sub = program
       .command('sub')
-      .allowUnknownOption()
-      .allowExcessArguments(false)
-      .action(() => { });
+      .allowExcessArguments(false);
+    configureCommand(sub);
 
     expect(() => {
       program.parse(['sub', 'excess'], { from: 'user' });
@@ -93,11 +81,10 @@ describe('allowUnknownOption', () => {
 
   test('when specify expected arg and allowExcessArguments(false) then no error', () => {
     const program = new commander.Command();
+    configureCommand(program);
     program
       .argument('<file>')
-      .exitOverride()
-      .allowExcessArguments(false)
-      .action(() => {});
+      .allowExcessArguments(false);
 
     expect(() => {
       program.parse(['file'], { from: 'user' });
@@ -106,11 +93,10 @@ describe('allowUnknownOption', () => {
 
   test('when specify excess after <arg> and allowExcessArguments(false) then error', () => {
     const program = new commander.Command();
+    configureCommand(program);
     program
       .argument('<file>')
-      .exitOverride()
-      .allowExcessArguments(false)
-      .action(() => {});
+      .allowExcessArguments(false);
 
     expect(() => {
       program.parse(['file', 'excess'], { from: 'user' });
@@ -119,11 +105,10 @@ describe('allowUnknownOption', () => {
 
   test('when specify excess after [arg] and allowExcessArguments(false) then error', () => {
     const program = new commander.Command();
+    configureCommand(program);
     program
       .argument('[file]')
-      .exitOverride()
-      .allowExcessArguments(false)
-      .action(() => {});
+      .allowExcessArguments(false);
 
     expect(() => {
       program.parse(['file', 'excess'], { from: 'user' });
@@ -132,11 +117,10 @@ describe('allowUnknownOption', () => {
 
   test('when specify args for [args...] and allowExcessArguments(false) then no error', () => {
     const program = new commander.Command();
+    configureCommand(program);
     program
       .argument('[files...]')
-      .exitOverride()
-      .allowExcessArguments(false)
-      .action(() => {});
+      .allowExcessArguments(false);
 
     expect(() => {
       program.parse(['file1', 'file2', 'file3'], { from: 'user' });

tests/command.exitOverride.test.js

@@ -120,6 +120,23 @@ describe('.exitOverride and error details', () => {
     expectCommanderError(caughtErr, 1, 'commander.missingArgument', "error: missing required argument 'arg-name'");
   });
 
+  test('when specify program without required argument and no action handler then throw CommanderError', () => {
+    const program = new commander.Command();
+    program
+      .exitOverride()
+      .argument('<arg-name>');
+
+    let caughtErr;
+    try {
+      program.parse(['node', 'test']);
+    } catch (err) {
+      caughtErr = err;
+    }
+
+    expect(stderrSpy).toHaveBeenCalled();
+    expectCommanderError(caughtErr, 1, 'commander.missingArgument', "error: missing required argument 'arg-name'");
+  });
+
   test('when specify excess argument then throw CommanderError', () => {
     const program = new commander.Command();
     program