Prevent unusable options

- Check if new option is usable with current settings before registering
- Check if registered options remain usable when updating settings

_registerOption() has been borrowed from deb3bdf in tj#1923.

lib/command.js

@@ -502,13 +502,28 @@ Expecting one of '${allowedValues.join("', '")}'`);
     return new Option(flags, description);
   }
 
+  /**
+   * Register option if possible with current settings.
+   * Throw otherwise.
+   *
+   * @param {Option} option
+   * @api private
+   */
+
+  _registerOption(option) {
+    this._checkForBrokenOptionalOptionArguments(option);
+    this.options.push(option);
+  }
+
   /**
    * Add an option.
    *
    * @param {Option} option
    * @return {Command} `this` command for chaining
    */
   addOption(option) {
+    this._registerOption(option);
+
     const oname = option.name();
     const name = option.attributeName();
 
@@ -523,9 +538,6 @@ Expecting one of '${allowedValues.join("', '")}'`);
       this.setOptionValueWithSource(name, option.defaultValue, 'default');
     }
 
-    // register the option
-    this.options.push(option);
-
     // handler for cli and env supplied values
     const handleOptionValue = (val, invalidValueMessage, valueSource) => {
       // val is null for optional option used without an optional-argument.
@@ -677,6 +689,40 @@ Expecting one of '${allowedValues.join("', '")}'`);
     return this._optionEx({ mandatory: true }, flags, description, fn, defaultValue);
   }
 
+  /**
+   * Check if the passed option or any registered option is unusable due to the current `strictOptionalOptionArguments` setting.
+   * Throw if the check result is positive.
+   *
+   * An option is deemed unusable when
+   * - `strictOptionalOptionArguments` is on,
+   * - and the option has an optional option-argument,
+   * - and one of the following holds true:
+   *   - `combineFlagAndOptionalValue` is off,
+   *   - or the option is variadic.
+   *
+   * @param {Option} [option]
+   * @api private
+   */
+
+  _checkForBrokenOptionalOptionArguments(option) {
+    if (this._strictOptionalOptionArguments) {
+      const options = option ? [option] : this.options;
+      if (!this._combineFlagAndOptionalValue) {
+        options.forEach((option) => {
+          if (option.optional && !option.long) {
+            throw new Error(`Option '${option.flags}' is incompatible with strictOptionalOptionArguments${this._name ? ` enabled for '${this._name}'` : ''} when combineFlagAndOptionalValue is off
+- must have a long flag`);
+          }
+        });
+      }
+      options.forEach((option) => {
+        if (option.optional && option.variadic) {
+          throw new Error(`Variadic option '${option.flags}' is incompatible with strictOptionalOptionArguments${this._name ? ` enabled for '${this._name}'` : ''}`);
+        }
+      });
+    }
+  }
+
   /**
    * When set to `true`, handle options with optional option-arguments as prescribed by POSIX,
    * i.e. only allow providing values for such option-arguments by combining them with a flag
@@ -688,6 +734,7 @@ Expecting one of '${allowedValues.join("', '")}'`);
 
   strictOptionalOptionArguments(strict = true) {
     this._strictOptionalOptionArguments = !!strict;
+    this._checkForBrokenOptionalOptionArguments();
     return this;
   }
 
@@ -703,6 +750,7 @@ Expecting one of '${allowedValues.join("', '")}'`);
    */
   combineFlagAndOptionalValue(combine = true) {
     this._combineFlagAndOptionalValue = !!combine;
+    this._checkForBrokenOptionalOptionArguments();
     return this;
   }
 
@@ -1842,7 +1890,7 @@ Expecting one of '${allowedValues.join("', '")}'`);
     description = description || 'output the version number';
     const versionOption = this.createOption(flags, description);
     this._versionOptionName = versionOption.attributeName();
-    this.options.push(versionOption);
+    this._registerOption(versionOption);
     this.on('option:' + versionOption.name(), () => {
       this._outputConfiguration.writeOut(`${str}\n`);
       this._exit(0, 'commander.version', str);