feat!: switch type:string option arguments to greedy, but with error for suspect cases in strict mode

index.js

@@ -28,6 +28,7 @@ const {
   isLoneShortOption,
   isLongOptionAndValue,
   isOptionValue,
+  isSafeOptionValue,
   isShortOptionAndValue,
   isShortOptionGroup
 } = require('./utils');
@@ -102,6 +103,7 @@ function storeOptionValue(options, longOption, value, result) {
 
 const parseArgs = ({
   args = getMainArgs(),
+  strict = false,
   options = {}
 } = {}) => {
   validateArray(args, 'args');
@@ -129,6 +131,14 @@ const parseArgs = ({
     }
   );
 
+  const checkSafeOptionValue = (dashedOption, longOption, value) => {
+    if (strict && !isSafeOptionValue(value)) {
+      const errorMessage = `Did you forget to specify the option argument for '${dashedOption}'?
+To specify an option argument starting with a dash use '--${longOption}=-EXAMPLE'.`;
+      throw new Error(errorMessage);
+    }
+  };
+
   const result = {
     flags: {},
     values: {},
@@ -159,6 +169,7 @@ const parseArgs = ({
       if (options[longOption]?.type === 'string' && isOptionValue(nextArg)) {
         // e.g. '-f', 'bar'
         optionValue = ArrayPrototypeShift(remainingArgs);
+        checkSafeOptionValue(arg, longOption, optionValue);
       }
       storeOptionValue(options, longOption, optionValue, result);
       continue;
@@ -202,6 +213,7 @@ const parseArgs = ({
       if (options[longOption]?.type === 'string' && isOptionValue(nextArg)) {
         // e.g. '--foo', 'bar'
         optionValue = ArrayPrototypeShift(remainingArgs);
+        checkSafeOptionValue(arg, longOption, optionValue);
       }
       storeOptionValue(options, longOption, optionValue, result);
       continue;

test/is-option-value.js

@@ -4,6 +4,8 @@
 const test = require('tape');
 const { isOptionValue } = require('../utils.js');
 
+// Options are greedy so simple behaviour, but run through the interesting possibilities.
+
 test('isOptionValue: when passed plain text then returns true', (t) => {
   t.true(isOptionValue('abc'));
   t.end();
@@ -25,28 +27,43 @@ test('isOptionValue: when passed dash then returns true', (t) => {
   t.end();
 });
 
-// Supporting undefined so can pass element off end of array without checking
+test('isOptionValue: when passed -- then returns true', (t) => {
+  t.true(isOptionValue('--'));
+  t.end();
+});
+
+// Checking undefined so can pass element off end of array.
 test('isOptionValue: when passed undefined then returns false', (t) => {
   t.false(isOptionValue(undefined));
   t.end();
 });
 
-test('isOptionValue: when passed short option then returns false', (t) => {
-  t.false(isOptionValue('-a'));
+test('isOptionValue: when passed short option then returns true', (t) => {
+  t.true(isOptionValue('-a'));
+  t.end();
+});
+
+test('isOptionValue: when passed short option digit then returns true', (t) => {
+  t.true(isOptionValue('-1'));
+  t.end();
+});
+
+test('isOptionValue: when passed negative number then returns true', (t) => {
+  t.true(isOptionValue('-123'));
   t.end();
 });
 
-test('isOptionValue: when passed short option group of short option with value then returns false', (t) => {
-  t.false(isOptionValue('-abd'));
+test('isOptionValue: when passed short option group of short option with value then returns true', (t) => {
+  t.true(isOptionValue('-abd'));
   t.end();
 });
 
-test('isOptionValue: when passed long option then returns false', (t) => {
-  t.false(isOptionValue('--foo'));
+test('isOptionValue: when passed long option then returns true', (t) => {
+  t.true(isOptionValue('--foo'));
   t.end();
 });
 
-test('isOptionValue: when passed long option with value then returns false', (t) => {
-  t.false(isOptionValue('--foo=bar'));
+test('isOptionValue: when passed long option with value then returns true', (t) => {
+  t.true(isOptionValue('--foo=bar'));
   t.end();
 });

test/is-safe-option-value.js

@@ -0,0 +1,69 @@
+'use strict';
+/* eslint max-len: 0 */
+
+const test = require('tape');
+const { isSafeOptionValue } = require('../utils.js');
+
+// Basically rejecting values starting with a dash, but run through the interesting possibilities.
+
+test('isSafeOptionValue: when passed plain text then returns true', (t) => {
+  t.true(isSafeOptionValue('abc'));
+  t.end();
+});
+
+test('isSafeOptionValue: when passed digits then returns true', (t) => {
+  t.true(isSafeOptionValue(123));
+  t.end();
+});
+
+test('isSafeOptionValue: when passed empty string then returns true', (t) => {
+  t.true(isSafeOptionValue(''));
+  t.end();
+});
+
+// Special case, used as stdin/stdout et al and not reason to reject
+test('isSafeOptionValue: when passed dash then returns true', (t) => {
+  t.true(isSafeOptionValue('-'));
+  t.end();
+});
+
+test('isSafeOptionValue: when passed -- then returns false', (t) => {
+  t.false(isSafeOptionValue('--'));
+  t.end();
+});
+
+// Supporting undefined so can pass element off end of array without checking
+test('isSafeOptionValue: when passed undefined then returns false', (t) => {
+  t.false(isSafeOptionValue(undefined));
+  t.end();
+});
+
+test('isSafeOptionValue: when passed short option then returns false', (t) => {
+  t.false(isSafeOptionValue('-a'));
+  t.end();
+});
+
+test('isSafeOptionValue: when passed short option digit then returns false', (t) => {
+  t.false(isSafeOptionValue('-1'));
+  t.end();
+});
+
+test('isSafeOptionValue: when passed negative number then returns false', (t) => {
+  t.false(isSafeOptionValue('-123'));
+  t.end();
+});
+
+test('isSafeOptionValue: when passed short option group of short option with value then returns false', (t) => {
+  t.false(isSafeOptionValue('-abd'));
+  t.end();
+});
+
+test('isSafeOptionValue: when passed long option then returns false', (t) => {
+  t.false(isSafeOptionValue('--foo'));
+  t.end();
+});
+
+test('isSafeOptionValue: when passed long option with value then returns false', (t) => {
+  t.false(isSafeOptionValue('--foo=bar'));
+  t.end();
+});

test/strict-value-check.js

@@ -0,0 +1,95 @@
+'use strict';
+/* eslint max-len: 0 */
+
+const test = require('tape');
+const { parseArgs } = require('../index.js');
+
+test('strict: when candidate option value is plain text then does not throw', (t) => {
+  const passedArgs = ['--with', 'abc'];
+  const knownOptions = { with: { type: 'string' } };
+
+  t.doesNotThrow(() => {
+    parseArgs({ args: passedArgs, options: knownOptions, strict: true });
+  });
+  t.end();
+});
+
+test("strict: when candidate option value is '-' then does not throw", (t) => {
+  const passedArgs = ['--with', '-'];
+  const knownOptions = { with: { type: 'string' } };
+
+  t.doesNotThrow(() => {
+    parseArgs({ args: passedArgs, options: knownOptions, strict: true });
+  });
+  t.end();
+});
+
+test("strict: when candidate option value is '--' then throws", (t) => {
+  const passedArgs = ['--with', '--'];
+  const knownOptions = { with: { type: 'string' } };
+
+  t.throws(() => {
+    parseArgs({ args: passedArgs, options: knownOptions, strict: true });
+  });
+  t.end();
+});
+
+test('strict: when candidate option value is short option then throws', (t) => {
+  const passedArgs = ['--with', '-a'];
+  const knownOptions = { with: { type: 'string' } };
+
+  t.throws(() => {
+    parseArgs({ args: passedArgs, options: knownOptions, strict: true });
+  });
+  t.end();
+});
+
+test('strict: when candidate option value is short option digit then throws', (t) => {
+  const passedArgs = ['--with', '-1'];
+  const knownOptions = { with: { type: 'string' } };
+
+  t.throws(() => {
+    parseArgs({ args: passedArgs, options: knownOptions, strict: true });
+  });
+  t.end();
+});
+
+test('strict: when candidate option value is long option then throws', (t) => {
+  const passedArgs = ['--with', '--foo'];
+  const knownOptions = { with: { type: 'string' } };
+
+  t.throws(() => {
+    parseArgs({ args: passedArgs, options: knownOptions, strict: true });
+  });
+  t.end();
+});
+
+test('strict: when short option and suspect value then throws with short option in error message', (t) => {
+  const passedArgs = ['-w', '--foo'];
+  const knownOptions = { with: { type: 'string', short: 'w' } };
+
+  t.throws(() => {
+    parseArgs({ args: passedArgs, options: knownOptions, strict: true });
+  }, /for '-w'/);
+  t.end();
+});
+
+test('strict: when long option and suspect value then throws with long option in error message', (t) => {
+  const passedArgs = ['--with', '--foo'];
+  const knownOptions = { with: { type: 'string' } };
+
+  t.throws(() => {
+    parseArgs({ args: passedArgs, options: knownOptions, strict: true });
+  }, /for '--with'/);
+  t.end();
+});
+
+test('strict: when long option and suspect value then throws with whole expected message', (t) => {
+  const passedArgs = ['--with', '--foo'];
+  const knownOptions = { with: { type: 'string' } };
+
+  t.throws(() => {
+    parseArgs({ args: passedArgs, options: knownOptions, strict: true });
+  }, /Did you forget to specify the option argument for '--with'\?\nTo specify an option argument starting with a dash use '--with=-EXAMPLE'\./);
+  t.end();
+});

utils.js

@@ -22,24 +22,31 @@ const {
 
 /**
  * Determines if the argument may be used as an option value.
- * NB: We are choosing not to accept option-ish arguments.
  * @example
- * isOptionValue('V']) // returns true
- * isOptionValue('-v') // returns false
- * isOptionValue('--foo') // returns false
+ * isOptionValue('V') // returns true
+ * isOptionValue('-v') // returns true (greedy)
+ * isOptionValue('--foo') // returns true (greedy)
  * isOptionValue(undefined) // returns false
  */
 function isOptionValue(value) {
   if (value == null) return false;
-  if (value === '-') return true; // e.g. representing stdin/stdout for file
 
   // Open Group Utility Conventions are that an option-argument
   // is the argument after the option, and may start with a dash.
-  // However, we are currently rejecting these and prioritising the
-  // option-like appearance of the argument. Rejection allows more error
-  // detection for strict:true, but comes at the cost of rejecting intended
-  // values starting with a dash, especially negative numbers.
-  return !StringPrototypeStartsWith(value, '-');
+  return true; // greedy!
+}
+
+/**
+ * Detect whether there is possible confusion and user may have omitted
+ * the option argument, like `--port --verbose` when `port` of type:string.
+ * In strict mode we throw errors if value is not safe.
+ */
+function isSafeOptionValue(value) {
+  if (value == null) return false;
+  if (value === '-') return true;
+  if (value === '--') return false;
+
+  return StringPrototypeCharAt(value, 0) !== '-';
 }
 
 /**
@@ -155,6 +162,7 @@ module.exports = {
   isLoneShortOption,
   isLongOptionAndValue,
   isOptionValue,
+  isSafeOptionValue,
   isShortOptionAndValue,
   isShortOptionGroup
 };