diff --git a/doc/api/util.md b/doc/api/util.md index c30831019d0e..b8f83c648c09 100644 --- a/doc/api/util.md +++ b/doc/api/util.md @@ -2361,6 +2361,10 @@ changes: **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-`. + In strict mode, the option being negated must be a declared boolean option, + including when a `no-*` option is explicitly configured with a `short` alias. + An enumerable own boolean `no-foo` option cannot coexist with a string + `foo` option in strict mode, even if neither option appears in the arguments. **Default:** `false`. * `tokens` {boolean} Return the parsed tokens. This is useful for extending the built-in behavior, from adding additional checks through to reprocessing diff --git a/lib/internal/util/parse_args/parse_args.js b/lib/internal/util/parse_args/parse_args.js index e8751c6e2225..8e8c3c184f6b 100644 --- a/lib/internal/util/parse_args/parse_args.js +++ b/lib/internal/util/parse_args/parse_args.js @@ -1,6 +1,7 @@ 'use strict'; const { + ArrayIsArray, ArrayPrototypeForEach, ArrayPrototypeMap, ArrayPrototypePush, @@ -114,6 +115,16 @@ function checkOptionUsage(config, token) { if (type === 'boolean' && token.value != null) { throw new ERR_PARSE_ARGS_INVALID_OPTION_VALUE(`Option '${shortAndLong}' does not take an argument`); } + + // Implicit negative options already had their target checked above. + if (tokenName === token.name && config.allowNegative && + StringPrototypeStartsWith(token.name, 'no-') && token.value === undefined) { + const targetName = StringPrototypeSlice(token.name, 3); + if (optionsGetOwn(config.options, targetName, 'type') !== 'boolean') { + throw new ERR_PARSE_ARGS_UNKNOWN_OPTION( + token.rawName, config.allowPositionals); + } + } } @@ -357,6 +368,19 @@ const parseArgs = (config = kEmptyObject) => { } validator(defaultValue, `options.${longOption}.default`); } + + if (strict && allowNegative && optionType === 'boolean' && + StringPrototypeStartsWith(longOption, 'no-')) { + const targetName = StringPrototypeSlice(longOption, 3); + const target = objectGetOwn(options, targetName); + if (target !== null && typeof target === 'object' && !ArrayIsArray(target) && + objectGetOwn(target, 'type') === 'string') { + throw new ERR_INVALID_ARG_VALUE( + `options.${longOption}`, optionConfig, + `conflicts with string option ${targetName} when allowNegative is true`, + ); + } + } }, ); diff --git a/test/parallel/test-parse-args.mjs b/test/parallel/test-parse-args.mjs index 706a3b24f1d0..35c13e3a4498 100644 --- a/test/parallel/test-parse-args.mjs +++ b/test/parallel/test-parse-args.mjs @@ -1025,3 +1025,386 @@ test('auto-detect --no-foo as negated when strict:false and allowNegative', () = assert.deepStrictEqual(result, expected); process.argv = holdArgv; }); + +for (const flag of ['--no-logfile', '-L']) { + for (const args of [[flag], ['--logfile', 'out.log', flag]]) { + test(`strict mode rejects a conflicting negative boolean option: ${args.join(' ')}`, () => { + const options = { + 'logfile': { type: 'string' }, + 'no-logfile': { type: 'boolean', short: 'L' }, + }; + assert.throws(() => { + parseArgs({ args, options, allowNegative: true }); + }, { + code: 'ERR_INVALID_ARG_VALUE' + }); + }); + } +} + +for (const reverse of [false, true]) { + for (const args of [[], ['--logfile', 'out.log'], ['-l', 'out.log']]) { + test(`strict mode rejects a conflicting configuration before parsing: ${reverse} ${args.join(' ')}`, () => { + const entries = [ + ['logfile', { type: 'string', short: 'l' }], + ['no-logfile', { type: 'boolean', short: 'L' }], + ]; + const options = Object.fromEntries(reverse ? entries.reverse() : entries); + assert.throws(() => { + parseArgs({ args, options, allowNegative: true }); + }, { + code: 'ERR_INVALID_ARG_VALUE' + }); + }); + } +} + +for (const args of [[], ['--no-logfile']]) { + test(`strict mode detects a conflict with a non-enumerable target: ${args.join(' ')}`, () => { + const options = { + 'logfile': { type: 'string' }, + 'no-logfile': { type: 'boolean' }, + }; + Object.defineProperty(options, 'logfile', { enumerable: false }); + assert.throws(() => { + parseArgs({ args, options, allowNegative: true }); + }, { + code: 'ERR_INVALID_ARG_VALUE' + }); + }); +} + +test('strict mode accepts an unused non-enumerable conflicting negative option', () => { + const options = { + 'logfile': { type: 'string' }, + 'no-logfile': { type: 'boolean' }, + }; + Object.defineProperty(options, 'no-logfile', { enumerable: false }); + const expected = { values: { __proto__: null }, positionals: [] }; + assert.deepStrictEqual(parseArgs({ args: [], options, allowNegative: true }), expected); +}); + +test('strict mode rejects a used non-enumerable conflicting negative option', () => { + const options = { + 'logfile': { type: 'string' }, + 'no-logfile': { type: 'boolean' }, + }; + Object.defineProperty(options, 'no-logfile', { enumerable: false }); + assert.throws(() => { + parseArgs({ args: ['--no-logfile'], options, allowNegative: true }); + }, { code: 'ERR_PARSE_ARGS_UNKNOWN_OPTION' }); +}); + +for (const [description, prototype] of [ + ['absent', null], + ['inherited', { foo: { type: 'string' } }], +]) { + test(`strict mode does not read a non-enumerable negative getter without an own target: ${description}`, () => { + const options = { __proto__: prototype }; + let reads = 0; + Object.defineProperty(options, 'no-foo', { + get() { + reads++; + throw new Error('negative getter was read'); + }, + }); + const expected = { values: { __proto__: null }, positionals: [] }; + assert.deepStrictEqual(parseArgs({ args: [], options, allowNegative: true }), expected); + assert.strictEqual(reads, 0); + }); +} + +for (const [description, option, code, message] of [ + ['null', null, 'ERR_INVALID_ARG_TYPE', /options\.invalid/], + ['array', [], 'ERR_INVALID_ARG_TYPE', /options\.invalid/], + ['type', { type: 'number' }, 'ERR_INVALID_ARG_TYPE', /options\.invalid\.type/], + ['short', { type: 'boolean', short: 'xy' }, 'ERR_INVALID_ARG_VALUE', /options\.invalid\.short/], + ['default', { type: 'boolean', default: 'invalid' }, 'ERR_INVALID_ARG_TYPE', /options\.invalid\.default/], +]) { + test(`invalid ${description} is reported before a later configuration conflict`, () => { + const options = { + 'invalid': option, + 'logfile': { type: 'string' }, + 'no-logfile': { type: 'boolean' }, + }; + assert.throws(() => { + parseArgs({ args: [], options, allowNegative: true }); + }, { code, message }); + }); +} + +test('configuration conflict is reported before a later invalid option', () => { + const options = { + 'logfile': { type: 'string' }, + 'no-logfile': { type: 'boolean' }, + 'invalid': null, + }; + assert.throws(() => { + parseArgs({ args: [], options, allowNegative: true }); + }, { code: 'ERR_INVALID_ARG_VALUE', message: /options\.no-logfile.*conflicts with string option logfile/ }); +}); + +for (const [description, target] of [ + ['null', null], + ['undefined', undefined], + ['array', Object.assign([], { type: 'string' })], +]) { + for (const reverse of [false, true]) { + test(`invalid ${description} target is reported in either order: ${reverse}`, () => { + const entries = [ + ['alpha', target], + ['no-alpha', { type: 'boolean' }], + ]; + const options = Object.fromEntries(reverse ? entries.reverse() : entries); + assert.throws(() => { + parseArgs({ args: [], options, allowNegative: true }); + }, { code: 'ERR_INVALID_ARG_TYPE', message: /options\.alpha/ }); + }); + } +} + +test('a negative option is validated before checking its configuration conflict', () => { + const options = { + 'no-alpha': { type: 'boolean', default: 'invalid' }, + 'alpha': { type: 'string' }, + }; + assert.throws(() => { + parseArgs({ args: [], options, allowNegative: true }); + }, { code: 'ERR_INVALID_ARG_TYPE', message: /options\.no-alpha\.default/ }); +}); + +test('negative option type getter is read once during configuration validation', () => { + let reads = 0; + const options = { + 'alpha': { type: 'boolean' }, + 'no-alpha': { + get type() { + reads++; + if (reads > 1) throw new Error('negative type was read again'); + return 'boolean'; + }, + }, + }; + const expected = { values: { __proto__: null }, positionals: [] }; + assert.deepStrictEqual(parseArgs({ args: [], options, allowNegative: true }), expected); + assert.strictEqual(reads, 1); +}); + +test('strict mode accepts an explicit negative boolean without a target when unused', () => { + const options = { 'no-alpha': { type: 'boolean', short: 'A' } }; + const expected = { values: { __proto__: null }, positionals: [] }; + assert.deepStrictEqual(parseArgs({ args: [], options, allowNegative: true }), expected); +}); + +for (const flag of ['--no-alpha', '-A']) { + test(`strict mode rejects an explicit negative option without a target: ${flag}`, () => { + const options = { 'no-alpha': { type: 'boolean', short: 'A' } }; + assert.throws(() => { + parseArgs({ args: [flag], options, allowNegative: true }); + }, { + code: 'ERR_PARSE_ARGS_UNKNOWN_OPTION' + }); + }); +} + +test('explicit negative boolean options and short aliases retain negated tokens', () => { + const args = ['--alpha', '-A', '--no-alpha']; + const options = { + 'alpha': { type: 'boolean', short: 'a' }, + 'no-alpha': { type: 'boolean', short: 'A' }, + }; + const expected = { + values: { __proto__: null, alpha: false }, + positionals: [], + tokens: [ + { kind: 'option', name: 'alpha', rawName: '--alpha', + index: 0, value: undefined, inlineValue: undefined }, + { kind: 'option', name: 'alpha', rawName: '-A', + index: 1, value: undefined, inlineValue: undefined }, + { kind: 'option', name: 'alpha', rawName: '--no-alpha', + index: 2, value: undefined, inlineValue: undefined }, + ], + }; + assert.deepStrictEqual(parseArgs({ args, options, allowNegative: true, tokens: true }), expected); +}); + +test('explicit negative aliases use the target multiple setting and replace its default', () => { + const args = ['-A', '--alpha', '-A']; + const options = { + 'alpha': { type: 'boolean', multiple: true, default: [true] }, + 'no-alpha': { type: 'boolean', short: 'A' }, + }; + const expected = { values: { __proto__: null, alpha: [false, true, false] }, positionals: [] }; + assert.deepStrictEqual(parseArgs({ args, options, allowNegative: true }), expected); +}); + +test('explicit negative boolean options can themselves be negated', () => { + const args = ['--no-no-alpha']; + const options = { 'no-alpha': { type: 'boolean', default: true } }; + const expected = { values: { '__proto__': null, 'no-alpha': false }, positionals: [] }; + assert.deepStrictEqual(parseArgs({ args, options, allowNegative: true }), expected); +}); + +for (const args of [['--no-logfile=out.log'], ['--no-logfile', 'out.log'], ['-L', 'out.log']]) { + test(`explicit negative string options retain their values: ${args.join(' ')}`, () => { + const options = { + 'logfile': { type: 'string' }, + 'no-logfile': { type: 'string', short: 'L' }, + }; + const expected = { values: { '__proto__': null, 'no-logfile': 'out.log' }, positionals: [] }; + assert.deepStrictEqual(parseArgs({ args, options, allowNegative: true }), expected); + }); +} + +for (const flag of ['--no-logfile', '-L']) { + test(`explicit negative string options still require a value: ${flag}`, () => { + const options = { + 'logfile': { type: 'string' }, + 'no-logfile': { type: 'string', short: 'L' }, + }; + assert.throws(() => { + parseArgs({ args: [flag], options, allowNegative: true }); + }, { + code: 'ERR_PARSE_ARGS_INVALID_OPTION_VALUE' + }); + }); +} + +test('explicit negative boolean options still reject values in strict mode', () => { + const args = ['--no-alpha=value']; + const options = { + 'alpha': { type: 'boolean' }, + 'no-alpha': { type: 'boolean' }, + }; + assert.throws(() => { + parseArgs({ args, options, allowNegative: true }); + }, { + code: 'ERR_PARSE_ARGS_INVALID_OPTION_VALUE' + }); +}); + +test('allowNegative:false preserves explicitly configured negative options', () => { + const args = ['--no-alpha', '-A']; + const options = { + 'alpha': { type: 'string', default: 'value' }, + 'no-alpha': { type: 'boolean', short: 'A', default: false }, + }; + const expected = { values: { '__proto__': null, 'alpha': 'value', 'no-alpha': true }, positionals: [] }; + assert.deepStrictEqual(parseArgs({ args, options, allowNegative: false }), expected); +}); + +test('strict:false preserves negation of explicit string and unknown targets', () => { + const args = ['--logfile', 'out.log', '--no-logfile', '-L', '--no-warnings', '-W', '--no-other']; + const options = { + 'logfile': { type: 'string' }, + 'no-logfile': { type: 'boolean', short: 'L' }, + 'no-warnings': { type: 'boolean', short: 'W' }, + }; + const expected = { + values: { __proto__: null, logfile: false, warnings: false, other: false }, + positionals: [], + }; + assert.deepStrictEqual(parseArgs({ args, options, strict: false, allowNegative: true }), expected); +}); + +test('strict:false accepts a conflicting configuration with no arguments', () => { + const options = { + 'logfile': { type: 'string' }, + 'no-logfile': { type: 'boolean' }, + }; + const expected = { values: { __proto__: null }, positionals: [] }; + assert.deepStrictEqual(parseArgs({ args: [], options, strict: false, allowNegative: true }), expected); +}); + +test('strict:false preserves explicit values on negative boolean options', () => { + const args = ['--no-alpha=value']; + const options = { + 'alpha': { type: 'boolean' }, + 'no-alpha': { type: 'boolean' }, + }; + const expected = { values: { '__proto__': null, 'no-alpha': 'value' }, positionals: [] }; + assert.deepStrictEqual(parseArgs({ args, options, strict: false, allowNegative: true }), expected); +}); + +for (const flag of ['--no-no-alpha', '-N']) { + test(`explicit double negation uses the immediate boolean target: ${flag}`, () => { + const options = { + 'alpha': { type: 'boolean' }, + 'no-alpha': { type: 'boolean' }, + 'no-no-alpha': { type: 'boolean', short: 'N' }, + }; + const expected = { values: { '__proto__': null, 'no-alpha': false }, positionals: [] }; + assert.deepStrictEqual(parseArgs({ args: [flag], options, allowNegative: true }), expected); + }); +} + +for (const flag of ['--no-no-alpha', '-N']) { + test(`double negation rejects an independently conflicting configuration: ${flag}`, () => { + const options = { + 'alpha': { type: 'string' }, + 'no-alpha': { type: 'boolean' }, + 'no-no-alpha': { type: 'boolean', short: 'N' }, + }; + assert.throws(() => { + parseArgs({ args: [flag], options, allowNegative: true }); + }, { + code: 'ERR_INVALID_ARG_VALUE' + }); + }); + + test(`double negation accepts a boolean target without an alpha option: ${flag}`, () => { + const options = { + 'no-alpha': { type: 'boolean' }, + 'no-no-alpha': { type: 'boolean', short: 'N' }, + }; + const expected = { values: { '__proto__': null, 'no-alpha': false }, positionals: [] }; + assert.deepStrictEqual(parseArgs({ args: [flag], options, allowNegative: true }), expected); + }); +} + +test('inherited options do not cause a configuration conflict', () => { + const options = { + '__proto__': { 'no-alpha': { type: 'boolean' } }, + 'alpha': { type: 'string' }, + }; + const expected = { values: { __proto__: null }, positionals: [] }; + assert.deepStrictEqual(parseArgs({ args: [], options, allowNegative: true }), expected); +}); + +test('an inherited string target does not cause a configuration conflict', () => { + const options = { + '__proto__': { alpha: { type: 'string' } }, + 'no-alpha': { type: 'boolean' }, + }; + const expected = { values: { __proto__: null }, positionals: [] }; + assert.deepStrictEqual(parseArgs({ args: [], options, allowNegative: true }), expected); +}); + +for (const type of ['boolean', 'string']) { + const flags = type === 'boolean' ? ['--no-alpha'] : ['--no-alpha', '-A']; + for (const flag of flags) { + test(`explicit negative ${type} retains value errors without a target: ${flag}`, () => { + const options = { 'no-alpha': { type, short: 'A' } }; + const args = type === 'boolean' ? [`${flag}=value`] : [flag]; + assert.throws(() => { + parseArgs({ args, options, allowNegative: true }); + }, { + code: 'ERR_PARSE_ARGS_INVALID_OPTION_VALUE' + }); + }); + } +} + +for (const flag of ['--no-alpha', '-A']) { + test(`strict negation rejects an inherited boolean target: ${flag}`, () => { + const options = { + '__proto__': { alpha: { type: 'boolean' } }, + 'no-alpha': { type: 'boolean', short: 'A' }, + }; + assert.throws(() => { + parseArgs({ args: [flag], options, allowNegative: true }); + }, { + code: 'ERR_PARSE_ARGS_UNKNOWN_OPTION' + }); + }); +}