From 4a72b2f92769753b48934665544d5facec9b2609 Mon Sep 17 00:00:00 2001 From: Zhenwei Jin <109658203+kylo5aby@users.noreply.github.com> Date: Wed, 19 Jun 2024 23:36:18 +0800 Subject: [PATCH] util: support `--no-` for argument with boolean type for parseArgs PR-URL: https://github.com/nodejs/node/pull/53107 Refs: https://github.com/nodejs/node/issues/53095 Reviewed-By: Matteo Collina Reviewed-By: Moshe Atlow Reviewed-By: James M Snell Reviewed-By: Chemi Atlow Reviewed-By: Chengzhong Wu --- doc/api/util.md | 12 +++- lib/internal/util/parse_args/parse_args.js | 43 +++++++++---- test/parallel/test-parse-args.mjs | 70 ++++++++++++++++++++++ 3 files changed, 111 insertions(+), 14 deletions(-) diff --git a/doc/api/util.md b/doc/api/util.md index f9da8d3bd97d58..b4230216869c2e 100644 --- a/doc/api/util.md +++ b/doc/api/util.md @@ -1390,6 +1390,9 @@ added: - v18.3.0 - v16.17.0 changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/53107 + description: add support for allowing negative options in input `config`. - version: - v20.0.0 pr-url: https://github.com/nodejs/node/pull/46718 @@ -1429,6 +1432,9 @@ changes: * `allowPositionals` {boolean} Whether this command accepts positional arguments. **Default:** `false` if `strict` is `true`, otherwise `true`. + * `allowNegative` {boolean} If `true`, allows explicitly setting boolean + options to `false` by prefixing the option name with `--no-`. + **Default:** `false`. * `tokens` {boolean} Return the parsed tokens. This is useful for extending the built-in behavior, from adding additional checks through to reprocessing the tokens in different ways. @@ -1511,9 +1517,9 @@ that appear more than once in args produce a token for each use. Short option groups like `-xy` expand to a token for each option. So `-xxx` produces three tokens. -For example to use the returned tokens to add support for a negated option -like `--no-color`, the tokens can be reprocessed to change the value stored -for the negated option. +For example, to add support for a negated option like `--no-color` (which +`allowNegative` supports when the option is of `boolean` type), the returned +tokens can be reprocessed to change the value stored for the negated option. ```mjs import { parseArgs } from 'node:util'; diff --git a/lib/internal/util/parse_args/parse_args.js b/lib/internal/util/parse_args/parse_args.js index d0a5a5569dc13b..09b3ff7316d3ef 100644 --- a/lib/internal/util/parse_args/parse_args.js +++ b/lib/internal/util/parse_args/parse_args.js @@ -94,14 +94,24 @@ To specify an option argument starting with a dash use ${example}.`; * @param {object} token - from tokens as available from parseArgs */ function checkOptionUsage(config, token) { - if (!ObjectHasOwn(config.options, token.name)) { - throw new ERR_PARSE_ARGS_UNKNOWN_OPTION( - token.rawName, config.allowPositionals); + let tokenName = token.name; + if (!ObjectHasOwn(config.options, tokenName)) { + // Check for negated boolean option. + if (config.allowNegative && StringPrototypeStartsWith(tokenName, 'no-')) { + tokenName = StringPrototypeSlice(tokenName, 3); + if (!ObjectHasOwn(config.options, tokenName) || optionsGetOwn(config.options, tokenName, 'type') !== 'boolean') { + throw new ERR_PARSE_ARGS_UNKNOWN_OPTION( + token.rawName, config.allowPositionals); + } + } else { + throw new ERR_PARSE_ARGS_UNKNOWN_OPTION( + token.rawName, config.allowPositionals); + } } - const short = optionsGetOwn(config.options, token.name, 'short'); - const shortAndLong = `${short ? `-${short}, ` : ''}--${token.name}`; - const type = optionsGetOwn(config.options, token.name, 'type'); + const short = optionsGetOwn(config.options, tokenName, 'short'); + const shortAndLong = `${short ? `-${short}, ` : ''}--${tokenName}`; + const type = optionsGetOwn(config.options, tokenName, 'type'); if (type === 'string' && typeof token.value !== 'string') { throw new ERR_PARSE_ARGS_INVALID_OPTION_VALUE(`Option '${shortAndLong} ' argument missing`); } @@ -114,16 +124,25 @@ function checkOptionUsage(config, token) { /** * Store the option value in `values`. - * @param {string} longOption - long option name e.g. 'foo' - * @param {string|undefined} optionValue - value from user args + * @param {object} token - from tokens as available from parseArgs * @param {object} options - option configs, from parseArgs({ options }) * @param {object} values - option values returned in `values` by parseArgs + * @param {boolean} allowNegative - allow negative optinons if true */ -function storeOption(longOption, optionValue, options, values) { +function storeOption(token, options, values, allowNegative) { + let longOption = token.name; + let optionValue = token.value; if (longOption === '__proto__') { return; // No. Just no. } + if (allowNegative && StringPrototypeStartsWith(longOption, 'no-') && optionValue === undefined) { + // Boolean option negation: --no-foo + longOption = StringPrototypeSlice(longOption, 3); + token.name = longOption; + optionValue = false; + } + // We store based on the option value rather than option type, // preserving the users intent for author to deal with. const newValue = optionValue ?? true; @@ -290,15 +309,17 @@ const parseArgs = (config = kEmptyObject) => { const strict = objectGetOwn(config, 'strict') ?? true; const allowPositionals = objectGetOwn(config, 'allowPositionals') ?? !strict; const returnTokens = objectGetOwn(config, 'tokens') ?? false; + const allowNegative = objectGetOwn(config, 'allowNegative') ?? false; const options = objectGetOwn(config, 'options') ?? { __proto__: null }; // Bundle these up for passing to strict-mode checks. - const parseConfig = { args, strict, options, allowPositionals }; + const parseConfig = { args, strict, options, allowPositionals, allowNegative }; // Validate input configuration. validateArray(args, 'args'); validateBoolean(strict, 'strict'); validateBoolean(allowPositionals, 'allowPositionals'); validateBoolean(returnTokens, 'tokens'); + validateBoolean(allowNegative, 'allowNegative'); validateObject(options, 'options'); ArrayPrototypeForEach( ObjectEntries(options), @@ -360,7 +381,7 @@ const parseArgs = (config = kEmptyObject) => { checkOptionUsage(parseConfig, token); checkOptionLikeValue(token); } - storeOption(token.name, token.value, options, result.values); + storeOption(token, options, result.values, parseConfig.allowNegative); } else if (token.kind === 'positional') { if (!allowPositionals) { throw new ERR_PARSE_ARGS_UNEXPECTED_POSITIONAL(token.value); diff --git a/test/parallel/test-parse-args.mjs b/test/parallel/test-parse-args.mjs index 41af0eba1c923d..e79434bdc6bbbf 100644 --- a/test/parallel/test-parse-args.mjs +++ b/test/parallel/test-parse-args.mjs @@ -992,3 +992,73 @@ test('multiple as false should expect a String', () => { }, /"options\.alpha\.default" property must be of type string/ ); }); + +// Test negative options +test('disable negative options and args are started with "--no-" prefix', () => { + const args = ['--no-alpha']; + const options = { alpha: { type: 'boolean' } }; + assert.throws(() => { + parseArgs({ args, options }); + }, { + code: 'ERR_PARSE_ARGS_UNKNOWN_OPTION' + }); +}); + +test('args are passed `type: "string"` and allow negative options', () => { + const args = ['--no-alpha', 'value']; + const options = { alpha: { type: 'string' } }; + assert.throws(() => { + parseArgs({ args, options, allowNegative: true }); + }, { + code: 'ERR_PARSE_ARGS_UNKNOWN_OPTION' + }); +}); + +test('args are passed `type: "boolean"` and allow negative options', () => { + const args = ['--no-alpha']; + const options = { alpha: { type: 'boolean' } }; + const expected = { values: { __proto__: null, alpha: false }, positionals: [] }; + assert.deepStrictEqual(parseArgs({ args, options, allowNegative: true }), expected); +}); + +test('args are passed `default: "true"` and allow negative options', () => { + const args = ['--no-alpha']; + const options = { alpha: { type: 'boolean', default: true } }; + const expected = { values: { __proto__: null, alpha: false }, positionals: [] }; + assert.deepStrictEqual(parseArgs({ args, options, allowNegative: true }), expected); +}); + +test('args are passed `default: "false" and allow negative options', () => { + const args = ['--no-alpha']; + const options = { alpha: { type: 'boolean', default: false } }; + const expected = { values: { __proto__: null, alpha: false }, positionals: [] }; + assert.deepStrictEqual(parseArgs({ args, options, allowNegative: true }), expected); +}); + +test('allow negative options and multiple as true', () => { + const args = ['--no-alpha', '--alpha', '--no-alpha']; + const options = { alpha: { type: 'boolean', multiple: true } }; + const expected = { values: { __proto__: null, alpha: [false, true, false] }, positionals: [] }; + assert.deepStrictEqual(parseArgs({ args, options, allowNegative: true }), expected); +}); + +test('allow negative options and passed multiple arguments', () => { + const args = ['--no-alpha', '--alpha']; + const options = { alpha: { type: 'boolean' } }; + const expected = { values: { __proto__: null, alpha: true }, positionals: [] }; + assert.deepStrictEqual(parseArgs({ args, options, allowNegative: true }), expected); +}); + +test('auto-detect --no-foo as negated when strict:false and allowNegative', () => { + const holdArgv = process.argv; + process.argv = [process.argv0, 'script.js', '--no-foo']; + const holdExecArgv = process.execArgv; + process.execArgv = []; + const result = parseArgs({ strict: false, allowNegative: true }); + + const expected = { values: { __proto__: null, foo: false }, + positionals: [] }; + assert.deepStrictEqual(result, expected); + process.argv = holdArgv; + process.execArgv = holdExecArgv; +});