From 38c11902cbe67866158633906fe6bb5f6b703090 Mon Sep 17 00:00:00 2001
From: John Gee <john@ruru.gen.nz>
Date: Sat, 21 Sep 2019 16:29:28 +1200
Subject: [PATCH] Do not mutate program.args when calling action handler

---
 index.js                     | 16 ++++++++--------
 tests/command.action.test.js | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 8 deletions(-)
 create mode 100644 tests/command.action.test.js

diff --git a/index.js b/index.js
index ad5d82f4a..f1dd59eaf 100644
--- a/index.js
+++ b/index.js
@@ -346,16 +346,16 @@ Command.prototype.action = function(fn) {
       }
     });
 
-    // Always append ourselves to the end of the arguments,
-    // to make sure we match the number of arguments the user
-    // expects
-    if (self._args.length) {
-      args[self._args.length] = self;
-    } else {
-      args.push(self);
+    // The .action callback takes an extra parameter which is the command itself.
+    var expectedArgsCount = self._args.length;
+    var actionArgs = args.slice(0, expectedArgsCount);
+    actionArgs[expectedArgsCount] = self;
+    // Add the extra arguments so available too.
+    if (args.length > expectedArgsCount) {
+      actionArgs.push(args.slice(expectedArgsCount));
     }
 
-    fn.apply(self, args);
+    fn.apply(self, actionArgs);
   };
   var parent = this.parent || this;
   var name = parent === this ? '*' : this._name;
diff --git a/tests/command.action.test.js b/tests/command.action.test.js
new file mode 100644
index 000000000..cb3f69862
--- /dev/null
+++ b/tests/command.action.test.js
@@ -0,0 +1,36 @@
+const commander = require('../');
+
+// Test some behaviours of .action not covered in more specific tests.
+
+test('when .action called then command passed to action', () => {
+  const actionMock = jest.fn();
+  const program = new commander.Command();
+  const cmd = program
+    .command('info')
+    .action(actionMock);
+  program.parse(['node', 'test', 'info']);
+  expect(actionMock).toHaveBeenCalledWith(cmd);
+});
+
+test('when .action called then program.args only contains args', () => {
+  // At one time program.args was being modified to contain the same args as the call to .action
+  // and so included the command as an extra and unexpected complex item in array.
+  const program = new commander.Command();
+  program
+    .command('info <file>')
+    .action(() => {});
+  program.parse(['node', 'test', 'info', 'my-file']);
+  expect(program.args).toEqual(['my-file']);
+});
+
+test('when .action called with extra arguments then extras also passed to action', () => {
+  // This is a new and undocumented behaviour for now.
+  // Might make this an error by default in future.
+  const actionMock = jest.fn();
+  const program = new commander.Command();
+  const cmd = program
+    .command('info <file>')
+    .action(actionMock);
+  program.parse(['node', 'test', 'info', 'my-file', 'a']);
+  expect(actionMock).toHaveBeenCalledWith('my-file', cmd, ['a']);
+});