feat!: positionals now opt-in when strict:true (#116)

Author: bakkot

README.md

@@ -30,6 +30,8 @@ added: REPLACEME
     are encountered, or when arguments are passed that do not match the
     `type` configured in `options`.
     **Default:** `true`.
+  * `allowPositionals`: {boolean} Whether this command accepts positional arguments.
+    **Default:** `false` if `strict` is `true`, otherwise `true`.
 
 * Returns: {Object} An {Object} representing the parsed command line
   arguments:
@@ -38,7 +40,7 @@ added: REPLACEME
   * `positionals` {string\[]}, containing positional arguments.
 
 Provides a higher level API for command-line argument parsing than interacting
-with `process.argv` directly.
+with `process.argv` directly. Takes a specification for the expected arguments and returns a structured object with the parsed options and positionals.
 
 ```mjs
 import { parseArgs } from 'util';
@@ -133,7 +135,7 @@ This package was implemented using [tape](https://www.npmjs.com/package/tape) as
 ## 💡 `process.mainArgs` Proposal
 
 > Note: This can be moved forward independently of the `util.parseArgs()` proposal/work.
- 
+
 ### Implementation:
 
 ```javascript
@@ -153,6 +155,7 @@ process.mainArgs = process.argv.slice(process._exec ? 1 : 2)
     * `multiple` {boolean} (Optional) If true, when appearing one or more times in `args`, results are collected in an `Array`
     * `short` {string} (Optional) A single character alias for an option; When appearing one or more times in `args`; Respects the `multiple` configuration
   * `strict` {Boolean} (Optional) A `Boolean` for whether or not to throw an error when unknown options are encountered, `type:'string'` options are missing an options-argument, or `type:'boolean'` options are passed an options-argument; defaults to `true`
+  * `allowPositionals` {Boolean} (Optional) Whether this command accepts positional arguments. Defaults `false` if `strict` is `true`, otherwise defaults to `true`.
 * Returns: {Object} An object having properties:
   * `values` {Object}, key:value for each option found. Value is a string for string options, or `true` for boolean options, or an array (of strings or booleans) for options configured as `multiple:true`.
   * `positionals` {string[]}, containing [Positionals][]
@@ -203,7 +206,7 @@ const options = {
   },
 };
 const args = ['-f', 'b'];
-const { values, positionals } = parseArgs({ args, options });
+const { values, positionals } = parseArgs({ args, options, allowPositionals: true });
 // values = { foo: true }
 // positionals = ['b']
 ```
@@ -213,7 +216,7 @@ const { parseArgs } = require('@pkgjs/parseargs');
 // unconfigured
 const options = {};
 const args = ['-f', '--foo=a', '--bar', 'b'];
-const { values, positionals } = parseArgs({ strict: false, args, options });
+const { values, positionals } = parseArgs({ strict: false, args, options, allowPositionals: true });
 // values = { f: true, foo: 'a', bar: true }
 // positionals = ['b']
 ```
@@ -273,7 +276,7 @@ const { values, positionals } = parseArgs({ strict: false, args, options });
   - no, `-bar` is a short option or options, with expansion logic that follows the
     [Utility Syntax Guidelines in POSIX.1-2017](https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html). `-bar` expands to `-b`, `-a`, `-r`.
 - Is `---foo` the same as `--foo`?
-  - no 
+  - no
   - the first is a long option named `'-foo'`
   - the second is a long option named `'foo'`
 - Is `-` a positional? ie, `bash some-test.sh | tap -`

errors.js

@@ -22,17 +22,26 @@ class ERR_PARSE_ARGS_INVALID_OPTION_VALUE extends Error {
 }
 
 class ERR_PARSE_ARGS_UNKNOWN_OPTION extends Error {
-  constructor(option) {
-    super(`Unknown option '${option}'`);
+  constructor(option, allowPositionals) {
+    const suggestDashDash = allowPositionals ? `. To specify a positional argument starting with a '-', place it at the end of the command after '--', as in '-- ${JSON.stringify(option)}` : '';
+    super(`Unknown option '${option}'${suggestDashDash}`);
     this.code = 'ERR_PARSE_ARGS_UNKNOWN_OPTION';
   }
 }
 
+class ERR_PARSE_ARGS_UNEXPECTED_POSITIONAL extends Error {
+  constructor(positional) {
+    super(`Unexpected argument '${positional}'. This command does not take positional arguments`);
+    this.code = 'ERR_PARSE_ARGS_UNEXPECTED_POSITIONAL';
+  }
+}
+
 module.exports = {
   codes: {
     ERR_INVALID_ARG_TYPE,
     ERR_INVALID_ARG_VALUE,
     ERR_PARSE_ARGS_INVALID_OPTION_VALUE,
     ERR_PARSE_ARGS_UNKNOWN_OPTION,
+    ERR_PARSE_ARGS_UNEXPECTED_POSITIONAL,
   }
 };

index.js

@@ -39,6 +39,7 @@ const {
     ERR_INVALID_ARG_VALUE,
     ERR_PARSE_ARGS_INVALID_OPTION_VALUE,
     ERR_PARSE_ARGS_UNKNOWN_OPTION,
+    ERR_PARSE_ARGS_UNEXPECTED_POSITIONAL,
   },
 } = require('./errors');
 
@@ -86,12 +87,12 @@ function getMainArgs() {
  * @param {boolean} strict - show errors, from parseArgs({ strict })
  */
 function checkOptionUsage(longOption, optionValue, options,
-                          shortOrLong, strict) {
+                          shortOrLong, strict, allowPositionals) {
   // Strict and options are used from local context.
   if (!strict) return;
 
   if (!ObjectHasOwn(options, longOption)) {
-    throw new ERR_PARSE_ARGS_UNKNOWN_OPTION(shortOrLong);
+    throw new ERR_PARSE_ARGS_UNKNOWN_OPTION(shortOrLong, allowPositionals);
   }
 
   const short = optionsGetOwn(options, longOption, 'short');
@@ -141,11 +142,13 @@ function storeOption(longOption, optionValue, options, values) {
 const parseArgs = (config = { __proto__: null }) => {
   const args = objectGetOwn(config, 'args') ?? getMainArgs();
   const strict = objectGetOwn(config, 'strict') ?? true;
+  const allowPositionals = objectGetOwn(config, 'allowPositionals') ?? !strict;
   const options = objectGetOwn(config, 'options') ?? { __proto__: null };
 
   // Validate input configuration.
   validateArray(args, 'args');
   validateBoolean(strict, 'strict');
+  validateBoolean(allowPositionals, 'allowPositionals');
   validateObject(options, 'options');
   ArrayPrototypeForEach(
     ObjectEntries(options),
@@ -186,6 +189,10 @@ const parseArgs = (config = { __proto__: null }) => {
     // Check if `arg` is an options terminator.
     // Guideline 10 in https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html
     if (arg === '--') {
+      if (!allowPositionals && remainingArgs.length > 0) {
+        throw new ERR_PARSE_ARGS_UNEXPECTED_POSITIONAL(nextArg);
+      }
+
       // Everything after a bare '--' is considered a positional argument.
       result.positionals = ArrayPrototypeConcat(
         result.positionals,
@@ -204,7 +211,8 @@ const parseArgs = (config = { __proto__: null }) => {
         // e.g. '-f', 'bar'
         optionValue = ArrayPrototypeShift(remainingArgs);
       }
-      checkOptionUsage(longOption, optionValue, options, arg, strict);
+      checkOptionUsage(longOption, optionValue, options,
+                       arg, strict, allowPositionals);
       storeOption(longOption, optionValue, options, result.values);
       continue;
     }
@@ -235,7 +243,7 @@ const parseArgs = (config = { __proto__: null }) => {
       const shortOption = StringPrototypeCharAt(arg, 1);
       const longOption = findLongOptionForShort(shortOption, options);
       const optionValue = StringPrototypeSlice(arg, 2);
-      checkOptionUsage(longOption, optionValue, options, `-${shortOption}`, strict);
+      checkOptionUsage(longOption, optionValue, options, `-${shortOption}`, strict, allowPositionals);
       storeOption(longOption, optionValue, options, result.values);
       continue;
     }
@@ -249,7 +257,8 @@ const parseArgs = (config = { __proto__: null }) => {
         // e.g. '--foo', 'bar'
         optionValue = ArrayPrototypeShift(remainingArgs);
       }
-      checkOptionUsage(longOption, optionValue, options, arg, strict);
+      checkOptionUsage(longOption, optionValue, options,
+                       arg, strict, allowPositionals);
       storeOption(longOption, optionValue, options, result.values);
       continue;
     }
@@ -259,12 +268,16 @@ const parseArgs = (config = { __proto__: null }) => {
       const index = StringPrototypeIndexOf(arg, '=');
       const longOption = StringPrototypeSlice(arg, 2, index);
       const optionValue = StringPrototypeSlice(arg, index + 1);
-      checkOptionUsage(longOption, optionValue, options, `--${longOption}`, strict);
+      checkOptionUsage(longOption, optionValue, options, `--${longOption}`, strict, allowPositionals);
       storeOption(longOption, optionValue, options, result.values);
       continue;
     }
 
     // Anything left is a positional
+    if (!allowPositionals) {
+      throw new ERR_PARSE_ARGS_UNEXPECTED_POSITIONAL(arg);
+    }
+
     ArrayPrototypePush(result.positionals, arg);
   }
 

test/dash.js

@@ -15,7 +15,7 @@ test("dash: when args include '-' used as positional then result has '-' in posi
   const args = ['-'];
   const expected = { values: { __proto__: null }, positionals: ['-'] };
 
-  const result = parseArgs({ args });
+  const result = parseArgs({ allowPositionals: true, args });
 
   t.deepEqual(result, expected);
   t.end();

test/index.js

@@ -98,7 +98,7 @@ test('handles short-option groups with "short" alias configured', () => {
 test('Everything after a bare `--` is considered a positional argument', () => {
   const args = ['--', 'barepositionals', 'mopositionals'];
   const expected = { values: { __proto__: null }, positionals: ['barepositionals', 'mopositionals'] };
-  const result = parseArgs({ args });
+  const result = parseArgs({ allowPositionals: true, args });
   assert.deepStrictEqual(result, expected, Error('testing bare positionals'));
 });
 
@@ -127,7 +127,7 @@ test('args equals are passed `type: "string"`', () => {
 test('when args include single dash then result stores dash as positional', () => {
   const args = ['-'];
   const expected = { values: { __proto__: null }, positionals: ['-'] };
-  const result = parseArgs({ args });
+  const result = parseArgs({ allowPositionals: true, args });
   assert.deepStrictEqual(result, expected);
 });
 
@@ -196,12 +196,12 @@ test('order of option and positional does not matter (per README)', () => {
   const options = { foo: { type: 'string' } };
   const expected = { values: { __proto__: null, foo: 'bar' }, positionals: ['baz'] };
   assert.deepStrictEqual(
-    parseArgs({ args: args1, options }),
+    parseArgs({ allowPositionals: true, args: args1, options }),
     expected,
     Error('option then positional')
   );
   assert.deepStrictEqual(
-    parseArgs({ args: args2, options }),
+    parseArgs({ allowPositionals: true, args: args2, options }),
     expected,
     Error('positional then option')
   );
@@ -288,6 +288,25 @@ test('excess leading dashes on options are retained', () => {
   assert.deepStrictEqual(result, expected, Error('excess option dashes are retained'));
 });
 
+test('positional arguments are allowed by default in strict:false', () => {
+  const args = ['foo'];
+  const options = { };
+  const expected = {
+    values: { __proto__: null },
+    positionals: ['foo']
+  };
+  const result = parseArgs({ strict: false, args, options });
+  assert.deepStrictEqual(result, expected);
+});
+
+test('positional arguments may be explicitly disallowed in strict:false', () => {
+  const args = ['foo'];
+  const options = { };
+  assert.throws(() => { parseArgs({ strict: false, allowPositionals: false, args, options }); }, {
+    code: 'ERR_PARSE_ARGS_UNEXPECTED_POSITIONAL'
+  });
+});
+
 // Test bad inputs
 
 test('invalid argument passed for options', () => {
@@ -355,6 +374,31 @@ test('unknown option with explicit value', () => {
   });
 });
 
+test('unexpected positional', () => {
+  const args = ['foo'];
+  const options = { foo: { type: 'boolean' } };
+  assert.throws(() => { parseArgs({ args, options }); }, {
+    code: 'ERR_PARSE_ARGS_UNEXPECTED_POSITIONAL'
+  });
+});
+
+test('unexpected positional after --', () => {
+  const args = ['--', 'foo'];
+  const options = { foo: { type: 'boolean' } };
+  assert.throws(() => { parseArgs({ args, options }); }, {
+    code: 'ERR_PARSE_ARGS_UNEXPECTED_POSITIONAL'
+  });
+});
+
+test('-- by itself is not a positional', () => {
+  const args = ['--foo', '--'];
+  const options = { foo: { type: 'boolean' } };
+  const result = parseArgs({ args, options });
+  const expected = { values: { __proto__: null, foo: true },
+                     positionals: [] };
+  assert.deepStrictEqual(result, expected);
+});
+
 test('string option used as boolean', () => {
   const args = ['--foo'];
   const options = { foo: { type: 'string' } };

test/short-option-groups.js

@@ -20,7 +20,7 @@ test('when pass full-config group of booleans then parsed as booleans', (t) => {
   const options = { r: { type: 'boolean' }, f: { type: 'boolean' } };
   const expected = { values: { __proto__: null, r: true, f: true }, positionals: ['p'] };
 
-  const result = parseArgs({ args, options });
+  const result = parseArgs({ allowPositionals: true, args, options });
 
   t.deepEqual(result, expected);
   t.end();