From ff7cefc3a3780558a36f4ff7a09a1b8ac31730c0 Mon Sep 17 00:00:00 2001 From: Kevin Gibbons Date: Sun, 17 Apr 2022 12:01:13 -0700 Subject: [PATCH 1/4] Make positionals opt-in when strict:true --- README.md | 6 +++-- errors.js | 8 ++++++ index.js | 11 ++++++++ test/dash.js | 2 +- test/index.js | 52 ++++++++++++++++++++++++++++++++++--- test/short-option-groups.js | 2 +- 6 files changed, 73 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 8037541..0c0abcc 100644 --- a/README.md +++ b/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: @@ -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 @@ -273,7 +275,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 -` diff --git a/errors.js b/errors.js index 689011e..8d876ff 100644 --- a/errors.js +++ b/errors.js @@ -28,11 +28,19 @@ class ERR_PARSE_ARGS_UNKNOWN_OPTION extends Error { } } +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, } }; diff --git a/index.js b/index.js index 87f12bd..9162fb1 100644 --- a/index.js +++ b/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'); @@ -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, @@ -265,6 +272,10 @@ const parseArgs = (config = { __proto__: null }) => { } // Anything left is a positional + if (!allowPositionals) { + throw new ERR_PARSE_ARGS_UNEXPECTED_POSITIONAL(arg); + } + ArrayPrototypePush(result.positionals, arg); } diff --git a/test/dash.js b/test/dash.js index dab82b2..0ab661b 100644 --- a/test/dash.js +++ b/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(); diff --git a/test/index.js b/test/index.js index 0d2be1b..68c0de0 100644 --- a/test/index.js +++ b/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' } }; diff --git a/test/short-option-groups.js b/test/short-option-groups.js index 7b528f0..c790a7d 100644 --- a/test/short-option-groups.js +++ b/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(); From 832c9fcd018d8a0ad6094f362925eefa65d06a0d Mon Sep 17 00:00:00 2001 From: Kevin Gibbons Date: Sun, 17 Apr 2022 12:08:11 -0700 Subject: [PATCH 2/4] Suggest -- for positional arguments starting with - --- errors.js | 5 +++-- index.js | 14 ++++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/errors.js b/errors.js index 8d876ff..e1b237b 100644 --- a/errors.js +++ b/errors.js @@ -22,8 +22,9 @@ 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'; } } diff --git a/index.js b/index.js index 9162fb1..1d19365 100644 --- a/index.js +++ b/index.js @@ -87,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'); @@ -211,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; } @@ -242,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; } @@ -256,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; } @@ -266,7 +268,7 @@ 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; } From e851584212603577d8d057be1bf6e5f3c7066a74 Mon Sep 17 00:00:00 2001 From: Kevin Gibbons Date: Sun, 17 Apr 2022 13:52:26 -0700 Subject: [PATCH 3/4] fixup readme, add sentence mentioning config --- README.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 0c0abcc..bc04a7a 100644 --- a/README.md +++ b/README.md @@ -40,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 options and returns a structured object corresponding to passed arguments. ```mjs import { parseArgs } from 'util'; @@ -155,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][] @@ -205,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'] ``` @@ -215,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'] ``` From a16332a3eef7a6510cd56ba60ef1dbe7a52121f6 Mon Sep 17 00:00:00 2001 From: Kevin Gibbons Date: Sun, 17 Apr 2022 14:26:12 -0700 Subject: [PATCH 4/4] tweak wording Co-authored-by: John Gee --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index bc04a7a..5f70344 100644 --- a/README.md +++ b/README.md @@ -40,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. Takes a specification for the expected options and returns a structured object corresponding to passed arguments. +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';