From 8267d02083a87b8b8a71fcce08348d1e031ea91c Mon Sep 17 00:00:00 2001 From: Aaron Casanova <32409546+aaronccasanova@users.noreply.github.com> Date: Wed, 13 Apr 2022 01:16:38 -0700 Subject: [PATCH] feat: Add strict mode to parser (#74) This PR introduces a strict mode parser config that is enabled by default. Errors on: - Unknown option encountered - Option of type:'string' used like a boolean option e.g. lone --string - Option of type:'boolean' used like a string option e.g. --boolean=foo --- README.md | 40 +++++++------- errors.js | 18 ++++++- index.js | 59 ++++++++++++++++++--- test/index.js | 102 ++++++++++++++++++++++++++++++------ test/prototype-pollution.js | 2 +- test/short-option-groups.js | 2 +- 6 files changed, 175 insertions(+), 48 deletions(-) diff --git a/README.md b/README.md index 77b6396..c0d31b5 100644 --- a/README.md +++ b/README.md @@ -84,7 +84,7 @@ process.mainArgs = process.argv.slice(process._exec ? 1 : 2) * `type` {'string'|'boolean'} (Required) Type of known option * `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` on wheather or not to throw an error when unknown args are encountered + * `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` * 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][] @@ -98,59 +98,59 @@ const { parseArgs } = require('@pkgjs/parseargs'); ``` ```js -// unconfigured const { parseArgs } = require('@pkgjs/parseargs'); -const args = ['-f', '--foo=a', '--bar', 'b']; -const options = {}; -const { values, positionals } = parseArgs({ args, options }); -// values = { f: true, foo: 'a', bar: true } -// positionals = ['b'] -``` - -```js -const { parseArgs } = require('@pkgjs/parseargs'); -// type:string -const args = ['-f', '--foo=a', '--bar', 'b']; +// specify the options that may be used const options = { - bar: { - type: 'string', - }, + foo: { type: 'string'}, + bar: { type: 'boolean' }, }; +const args = ['--foo=a', '--bar']; const { values, positionals } = parseArgs({ args, options }); -// values = { f: true, foo: 'a', bar: 'b' } +// values = { foo: 'a', bar: true } // positionals = [] ``` ```js const { parseArgs } = require('@pkgjs/parseargs'); // type:string & multiple -const args = ['-f', '--foo=a', '--foo', 'b']; const options = { foo: { type: 'string', multiple: true, }, }; +const args = ['--foo=a', '--foo', 'b']; const { values, positionals } = parseArgs({ args, options }); -// values = { f: true, foo: [ 'a', 'b' ] } +// values = { foo: [ 'a', 'b' ] } // positionals = [] ``` ```js const { parseArgs } = require('@pkgjs/parseargs'); // shorts -const args = ['-f', 'b']; const options = { foo: { short: 'f', type: 'boolean' }, }; +const args = ['-f', 'b']; const { values, positionals } = parseArgs({ args, options }); // values = { foo: true } // positionals = ['b'] ``` +```js +const { parseArgs } = require('@pkgjs/parseargs'); +// unconfigured +const options = {}; +const args = ['-f', '--foo=a', '--bar', 'b']; +const { values, positionals } = parseArgs({ strict: false, args, options }); +// values = { f: true, foo: 'a', bar: true } +// positionals = ['b'] +``` + + ### F.A.Qs - Is `cmd --foo=bar baz` the same as `cmd baz --foo=bar`? diff --git a/errors.js b/errors.js index 2f260d7..689011e 100644 --- a/errors.js +++ b/errors.js @@ -14,9 +14,25 @@ class ERR_INVALID_ARG_VALUE extends TypeError { } } +class ERR_PARSE_ARGS_INVALID_OPTION_VALUE extends Error { + constructor(message) { + super(message); + this.code = 'ERR_PARSE_ARGS_INVALID_OPTION_VALUE'; + } +} + +class ERR_PARSE_ARGS_UNKNOWN_OPTION extends Error { + constructor(option) { + super(`Unknown option '${option}'`); + this.code = 'ERR_PARSE_ARGS_UNKNOWN_OPTION'; + } +} + module.exports = { codes: { ERR_INVALID_ARG_TYPE, - ERR_INVALID_ARG_VALUE + ERR_INVALID_ARG_VALUE, + ERR_PARSE_ARGS_INVALID_OPTION_VALUE, + ERR_PARSE_ARGS_UNKNOWN_OPTION, } }; diff --git a/index.js b/index.js index 4cc0c4a..9e25e05 100644 --- a/index.js +++ b/index.js @@ -35,6 +35,8 @@ const { const { codes: { ERR_INVALID_ARG_VALUE, + ERR_PARSE_ARGS_INVALID_OPTION_VALUE, + ERR_PARSE_ARGS_UNKNOWN_OPTION, }, } = require('./errors'); @@ -73,16 +75,41 @@ function getMainArgs() { } const protoKey = '__proto__'; -function storeOptionValue(options, longOption, value, result) { - const optionConfig = options[longOption] || {}; + +function storeOption({ + strict, + options, + result, + longOption, + shortOption, + optionValue, +}) { + const hasOptionConfig = ObjectHasOwn(options, longOption); + const optionConfig = hasOptionConfig ? options[longOption] : {}; + + if (strict) { + if (!hasOptionConfig) { + throw new ERR_PARSE_ARGS_UNKNOWN_OPTION(shortOption == null ? `--${longOption}` : `-${shortOption}`); + } + + const shortOptionErr = ObjectHasOwn(optionConfig, 'short') ? `-${optionConfig.short}, ` : ''; + + if (options[longOption].type === 'string' && optionValue == null) { + throw new ERR_PARSE_ARGS_INVALID_OPTION_VALUE(`Option '${shortOptionErr}--${longOption} ' argument missing`); + } + + if (options[longOption].type === 'boolean' && optionValue != null) { + throw new ERR_PARSE_ARGS_INVALID_OPTION_VALUE(`Option '${shortOptionErr}--${longOption}' does not take an argument`); + } + } if (longOption === protoKey) { return; } // Values - const usedAsFlag = value === undefined; - const newValue = usedAsFlag ? true : value; + const usedAsFlag = optionValue === undefined; + const newValue = usedAsFlag ? true : optionValue; if (optionConfig.multiple) { // Always store value in array, including for flags. // result.values[longOption] starts out not present, @@ -99,9 +126,11 @@ function storeOptionValue(options, longOption, value, result) { const parseArgs = ({ args = getMainArgs(), + strict = true, options = {} } = {}) => { validateArray(args, 'args'); + validateBoolean(strict, 'strict'); validateObject(options, 'options'); ArrayPrototypeForEach( ObjectEntries(options), @@ -158,7 +187,14 @@ const parseArgs = ({ // e.g. '-f', 'bar' optionValue = ArrayPrototypeShift(remainingArgs); } - storeOptionValue(options, longOption, optionValue, result); + storeOption({ + strict, + options, + result, + longOption, + shortOption, + optionValue, + }); continue; } @@ -188,7 +224,14 @@ const parseArgs = ({ const shortOption = StringPrototypeCharAt(arg, 1); const longOption = findLongOptionForShort(shortOption, options); const optionValue = StringPrototypeSlice(arg, 2); - storeOptionValue(options, longOption, optionValue, result); + storeOption({ + strict, + options, + result, + longOption, + shortOption, + optionValue, + }); continue; } @@ -200,7 +243,7 @@ const parseArgs = ({ // e.g. '--foo', 'bar' optionValue = ArrayPrototypeShift(remainingArgs); } - storeOptionValue(options, longOption, optionValue, result); + storeOption({ strict, options, result, longOption, optionValue }); continue; } @@ -209,7 +252,7 @@ const parseArgs = ({ const index = StringPrototypeIndexOf(arg, '='); const longOption = StringPrototypeSlice(arg, 2, index); const optionValue = StringPrototypeSlice(arg, index + 1); - storeOptionValue(options, longOption, optionValue, result); + storeOption({ strict, options, result, longOption, optionValue }); continue; } diff --git a/test/index.js b/test/index.js index 73b6deb..5d7c08e 100644 --- a/test/index.js +++ b/test/index.js @@ -9,7 +9,7 @@ const { parseArgs } = require('../index.js'); test('when short option used as flag then stored as flag', (t) => { const passedArgs = ['-f']; const expected = { values: { f: true }, positionals: [] }; - const args = parseArgs({ args: passedArgs }); + const args = parseArgs({ strict: false, args: passedArgs }); t.deepEqual(args, expected); @@ -19,7 +19,7 @@ test('when short option used as flag then stored as flag', (t) => { test('when short option used as flag before positional then stored as flag and positional (and not value)', (t) => { const passedArgs = ['-f', 'bar']; const expected = { values: { f: true }, positionals: [ 'bar' ] }; - const args = parseArgs({ args: passedArgs }); + const args = parseArgs({ strict: false, args: passedArgs }); t.deepEqual(args, expected); @@ -64,7 +64,7 @@ test('when short option `type: "string"` used without value then stored as flag' const passedArgs = ['-f']; const passedOptions = { f: { type: 'string' } }; const expected = { values: { f: true }, positionals: [] }; - const args = parseArgs({ args: passedArgs, options: passedOptions }); + const args = parseArgs({ strict: false, args: passedArgs, options: passedOptions }); t.deepEqual(args, expected); @@ -75,7 +75,7 @@ test('short option group behaves like multiple short options', (t) => { const passedArgs = ['-rf']; const passedOptions = { }; const expected = { values: { r: true, f: true }, positionals: [] }; - const args = parseArgs({ args: passedArgs, options: passedOptions }); + const args = parseArgs({ strict: false, args: passedArgs, options: passedOptions }); t.deepEqual(args, expected); @@ -86,7 +86,7 @@ test('short option group does not consume subsequent positional', (t) => { const passedArgs = ['-rf', 'foo']; const passedOptions = { }; const expected = { values: { r: true, f: true }, positionals: ['foo'] }; - const args = parseArgs({ args: passedArgs, options: passedOptions }); + const args = parseArgs({ strict: false, args: passedArgs, options: passedOptions }); t.deepEqual(args, expected); t.end(); @@ -97,7 +97,7 @@ test('if terminal of short-option group configured `type: "string"`, subsequent const passedArgs = ['-rvf', 'foo']; const passedOptions = { f: { type: 'string' } }; const expected = { values: { r: true, v: true, f: 'foo' }, positionals: [] }; - const args = parseArgs({ args: passedArgs, options: passedOptions }); + const args = parseArgs({ strict: false, args: passedArgs, options: passedOptions }); t.deepEqual(args, expected); t.end(); @@ -107,7 +107,7 @@ test('handles short-option groups in conjunction with long-options', (t) => { const passedArgs = ['-rf', '--foo', 'foo']; const passedOptions = { foo: { type: 'string' } }; const expected = { values: { r: true, f: true, foo: 'foo' }, positionals: [] }; - const args = parseArgs({ args: passedArgs, options: passedOptions }); + const args = parseArgs({ strict: false, args: passedArgs, options: passedOptions }); t.deepEqual(args, expected); t.end(); @@ -117,7 +117,7 @@ test('handles short-option groups with "short" alias configured', (t) => { const passedArgs = ['-rf']; const passedOptions = { remove: { short: 'r', type: 'boolean' } }; const expected = { values: { remove: true, f: true }, positionals: [] }; - const args = parseArgs({ args: passedArgs, options: passedOptions }); + const args = parseArgs({ strict: false, args: passedArgs, options: passedOptions }); t.deepEqual(args, expected); t.end(); @@ -136,7 +136,7 @@ test('Everything after a bare `--` is considered a positional argument', (t) => test('args are true', (t) => { const passedArgs = ['--foo', '--bar']; const expected = { values: { foo: true, bar: true }, positionals: [] }; - const args = parseArgs({ args: passedArgs }); + const args = parseArgs({ strict: false, args: passedArgs }); t.deepEqual(args, expected, Error('args are true')); @@ -146,7 +146,7 @@ test('args are true', (t) => { test('arg is true and positional is identified', (t) => { const passedArgs = ['--foo=a', '--foo', 'b']; const expected = { values: { foo: true }, positionals: ['b'] }; - const args = parseArgs({ args: passedArgs }); + const args = parseArgs({ strict: false, args: passedArgs }); t.deepEqual(args, expected, Error('arg is true and positional is identified')); @@ -178,7 +178,7 @@ test('zero config args equals are parsed as if `type: "string"`', (t) => { const passedArgs = ['--so=wat']; const passedOptions = { }; const expected = { values: { so: 'wat' }, positionals: [] }; - const args = parseArgs({ args: passedArgs, options: passedOptions }); + const args = parseArgs({ strict: false, args: passedArgs, options: passedOptions }); t.deepEqual(args, expected, Error('arg value is passed')); @@ -267,7 +267,7 @@ test('correct default args when use node -p', (t) => { process.argv = [process.argv0, '--foo']; const holdExecArgv = process.execArgv; process.execArgv = ['-p', '0']; - const result = parseArgs(); + const result = parseArgs({ strict: false }); const expected = { values: { foo: true }, positionals: [] }; @@ -283,7 +283,7 @@ test('correct default args when use node --print', (t) => { process.argv = [process.argv0, '--foo']; const holdExecArgv = process.execArgv; process.execArgv = ['--print', '0']; - const result = parseArgs(); + const result = parseArgs({ strict: false }); const expected = { values: { foo: true }, positionals: [] }; @@ -299,7 +299,7 @@ test('correct default args when use node -e', (t) => { process.argv = [process.argv0, '--foo']; const holdExecArgv = process.execArgv; process.execArgv = ['-e', '0']; - const result = parseArgs(); + const result = parseArgs({ strict: false }); const expected = { values: { foo: true }, positionals: [] }; @@ -315,7 +315,7 @@ test('correct default args when use node --eval', (t) => { process.argv = [process.argv0, '--foo']; const holdExecArgv = process.execArgv; process.execArgv = ['--eval', '0']; - const result = parseArgs(); + const result = parseArgs({ strict: false }); const expected = { values: { foo: true }, positionals: [] }; @@ -331,7 +331,7 @@ test('correct default args when normal arguments', (t) => { process.argv = [process.argv0, 'script.js', '--foo']; const holdExecArgv = process.execArgv; process.execArgv = []; - const result = parseArgs(); + const result = parseArgs({ strict: false }); const expected = { values: { foo: true }, positionals: [] }; @@ -350,7 +350,7 @@ test('excess leading dashes on options are retained', (t) => { values: { '-triple': true }, positionals: [] }; - const result = parseArgs({ args: passedArgs, options: passedOptions }); + const result = parseArgs({ strict: false, args: passedArgs, options: passedOptions }); t.deepEqual(result, expected, Error('excess option dashes are retained')); @@ -402,6 +402,74 @@ test('invalid union value passed to "type" option', (t) => { t.end(); }); +// Test strict mode + +test('unknown long option --bar', (t) => { + const passedArgs = ['--foo', '--bar']; + const passedOptions = { foo: { type: 'boolean' } }; + + t.throws(() => { parseArgs({ args: passedArgs, options: passedOptions }); }, { + code: 'ERR_PARSE_ARGS_UNKNOWN_OPTION' + }); + + t.end(); +}); + +test('unknown short option -b', (t) => { + const passedArgs = ['--foo', '-b']; + const passedOptions = { foo: { type: 'boolean' } }; + + t.throws(() => { parseArgs({ args: passedArgs, options: passedOptions }); }, { + code: 'ERR_PARSE_ARGS_UNKNOWN_OPTION' + }); + + t.end(); +}); + +test('unknown option -r in short option group -bar', (t) => { + const passedArgs = ['-bar']; + const passedOptions = { b: { type: 'boolean' }, a: { type: 'boolean' } }; + + t.throws(() => { parseArgs({ args: passedArgs, options: passedOptions }); }, { + code: 'ERR_PARSE_ARGS_UNKNOWN_OPTION' + }); + + t.end(); +}); + +test('unknown option with explicit value', (t) => { + const passedArgs = ['--foo', '--bar=baz']; + const passedOptions = { foo: { type: 'boolean' } }; + + t.throws(() => { parseArgs({ args: passedArgs, options: passedOptions }); }, { + code: 'ERR_PARSE_ARGS_UNKNOWN_OPTION' + }); + + t.end(); +}); + +test('string option used as boolean', (t) => { + const passedArgs = ['--foo']; + const passedOptions = { foo: { type: 'string' } }; + + t.throws(() => { parseArgs({ args: passedArgs, options: passedOptions }); }, { + code: 'ERR_PARSE_ARGS_INVALID_OPTION_VALUE' + }); + + t.end(); +}); + +test('boolean option used with value', (t) => { + const passedArgs = ['--foo=bar']; + const passedOptions = { foo: { type: 'boolean' } }; + + t.throws(() => { parseArgs({ args: passedArgs, options: passedOptions }); }, { + code: 'ERR_PARSE_ARGS_INVALID_OPTION_VALUE' + }); + + t.end(); +}); + test('invalid short option length', (t) => { const passedArgs = []; const passedOptions = { foo: { short: 'fo', type: 'boolean' } }; diff --git a/test/prototype-pollution.js b/test/prototype-pollution.js index 0edf48d..3f28f59 100644 --- a/test/prototype-pollution.js +++ b/test/prototype-pollution.js @@ -8,7 +8,7 @@ test('should not allow __proto__ key to be set on object', (t) => { const passedArgs = ['--__proto__=hello']; const expected = { values: {}, positionals: [] }; - const result = parseArgs({ args: passedArgs }); + const result = parseArgs({ strict: false, args: passedArgs }); t.deepEqual(result, expected); t.end(); diff --git a/test/short-option-groups.js b/test/short-option-groups.js index 8c24e1a..f95b6fa 100644 --- a/test/short-option-groups.js +++ b/test/short-option-groups.js @@ -9,7 +9,7 @@ test('when pass zero-config group of booleans then parsed as booleans', (t) => { const passedOptions = { }; const expected = { values: { r: true, f: true }, positionals: ['p'] }; - const result = parseArgs({ args: passedArgs, options: passedOptions }); + const result = parseArgs({ strict: false, args: passedArgs, options: passedOptions }); t.deepEqual(result, expected); t.end();