Check for missing required options after check for displaying help

index.js

@@ -324,6 +324,7 @@ Command.prototype.action = function(fn) {
 
     // Output help if necessary
     outputHelpIfNecessary(self, parsed.unknown);
+    self._checkForMissingMandatoryOptions();
 
     // If there are still any unknown options, then we simply
     // die, unless someone asked for help, in which case we give it
@@ -563,10 +564,17 @@ Command.prototype.parse = function(argv) {
 
   if (args[0] === 'help' && args.length === 1) this.help();
 
+  // Note for future: we could return early if we found an action handler in parseArgs, as none of following code needed?
+
   // <cmd> --help
   if (args[0] === 'help') {
     args[0] = args[1];
     args[1] = this._helpLongFlag;
+  } else {
+    // If calling through to executable subcommand we could check for help flags before failing,
+    // but a somewhat unlikely case since program options not passed to executable subcommands.
+    // Wait for reports to see if check needed and what usage pattern is.
+    this._checkForMissingMandatoryOptions();
   }
 
   // executable sub-commands
@@ -832,12 +840,14 @@ Command.prototype.optionFor = function(arg) {
  */
 
 Command.prototype._checkForMissingMandatoryOptions = function() {
-  const self = this;
-  this.options.forEach((anOption) => {
-    if (anOption.mandatory && (self[anOption.attributeName()] === undefined)) {
-      self.missingMandatoryOptionValue(anOption);
-    }
-  });
+  // Walk up hierarchy so can call from action handler after checking for displaying help.
+  for (var cmd = this; cmd; cmd = cmd.parent) {
+    cmd.options.forEach((anOption) => {
+      if (anOption.mandatory && (cmd[anOption.attributeName()] === undefined)) {
+        cmd.missingMandatoryOptionValue(anOption);
+      }
+    });
+  }
 };
 
 /**
@@ -916,8 +926,6 @@ Command.prototype.parseOptions = function(argv) {
     args.push(arg);
   }
 
-  this._checkForMissingMandatoryOptions();
-
   return { args: args, unknown: unknownOptions };
 };
 

tests/options.mandatory.test.js

@@ -1,6 +1,7 @@
 const commander = require('../');
 
 // Assuming mandatory options behave as normal options apart from the mandatory aspect, not retesting all behaviour.
+// Likewise, not redoing all tests on subcommand after testing on program.
 
 describe('required program option with mandatory value specified', () => {
   test('when program has required value specified then value as specified', () => {
@@ -224,3 +225,54 @@ describe('required command option with mandatory value not specified', () => {
     }).not.toThrow();
   });
 });
+
+describe('missing mandatory option but help requested', () => {
+  // Optional. Use internal knowledge to suppress output to keep test output clean.
+  let writeSpy;
+
+  beforeAll(() => {
+    writeSpy = jest.spyOn(process.stdout, 'write').mockImplementation(() => { });
+  });
+
+  afterEach(() => {
+    writeSpy.mockClear();
+  });
+
+  afterAll(() => {
+    writeSpy.mockRestore();
+  });
+
+  test('when program has required option not specified and --help then help', () => {
+    const program = new commander.Command();
+    program
+      .exitOverride()
+      .requiredOption('--cheese <type>', 'cheese type');
+
+    let caughtErr;
+    try {
+      program.parse(['node', 'test', '--help']);
+    } catch (err) {
+      caughtErr = err;
+    }
+
+    expect(caughtErr.code).toEqual('commander.helpDisplayed');
+  });
+
+  test('when program has required option not specified and subcommand --help then help', () => {
+    const program = new commander.Command();
+    program
+      .exitOverride()
+      .requiredOption('--cheese <type>', 'cheese type')
+      .command('sub')
+      .action(() => {});
+
+    let caughtErr;
+    try {
+      program.parse(['node', 'test', 'sub', '--help']);
+    } catch (err) {
+      caughtErr = err;
+    }
+
+    expect(caughtErr.code).toEqual('commander.helpDisplayed');
+  });
+});