From 6425807df4d27e0478770598d619104e9226c87b Mon Sep 17 00:00:00 2001 From: islandryu Date: Thu, 19 Dec 2024 18:07:36 +0900 Subject: [PATCH] cli: fix negated boolean options not removing implied options --- src/node_options-inl.h | 15 ++- .../test-negative-and-implies-options.js | 117 ++++++++++++++++++ 2 files changed, 124 insertions(+), 8 deletions(-) create mode 100644 test/parallel/test-negative-and-implies-options.js diff --git a/src/node_options-inl.h b/src/node_options-inl.h index 24954e0b583834..0b0f9bb150b59f 100644 --- a/src/node_options-inl.h +++ b/src/node_options-inl.h @@ -388,18 +388,17 @@ void OptionsParser::Parse( } { - std::string implied_name = name; - if (is_negation) { - // Implications for negated options are defined with "--no-". - implied_name.insert(2, "no-"); - } - auto implications = implications_.equal_range(implied_name); + auto implications = implications_.equal_range(name); for (auto imp = implications.first; imp != implications.second; ++imp) { if (imp->second.type == kV8Option) { v8_args->push_back(imp->second.name); } else { - *imp->second.target_field->template Lookup(options) = - imp->second.target_value; + auto p = imp->second.target_field->template Lookup(options); + if (is_negation) { + if (it->second.type == kBoolean) *p = !imp->second.target_value; + } else { + *p = imp->second.target_value; + } } } } diff --git a/test/parallel/test-negative-and-implies-options.js b/test/parallel/test-negative-and-implies-options.js new file mode 100644 index 00000000000000..1577e47c52829e --- /dev/null +++ b/test/parallel/test-negative-and-implies-options.js @@ -0,0 +1,117 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const { describe, it } = require('node:test'); + +describe('negative and implies options', () => { + it('should remove --inspect option when --no-inspect-brk is specified', async () => { + const { stdout, stderr } = await common.spawnPromisified( + process.execPath, + ['--inspect-brk=0', '--no-inspect-brk', '-e', "console.log('inspect-brk')"]); + assert.strictEqual(stdout, 'inspect-brk\n'); + assert.strictEqual(stderr, ''); + }); + + it('should remove --inspect option when --no-inspect-wait is specified', async () => { + const { stdout, stderr } = await common.spawnPromisified( + process.execPath, + ['--inspect-wait=0', '--no-inspect-wait', '-e', "console.log('inspect-wait')"]); + assert.strictEqual(stdout, 'inspect-wait\n'); + assert.strictEqual(stderr, ''); + }); + + it('should remove --trace-env option when --no-trace-env-js-stack is specified', async () => { + const { stdout, stderr } = await common.spawnPromisified( + process.execPath, + ['--trace-env-js-stack', '--no-trace-env-js-stack', '-e', "console.log('trace-env-js-stack')"]); + assert.strictEqual(stdout, 'trace-env-js-stack\n'); + assert.strictEqual(stderr, ''); + }); + + it('should remove --trace-env option when --no-trace-env-native-stack is specified', async () => { + const { stdout, stderr } = await common.spawnPromisified( + process.execPath, + ['--trace-env-native-stack', '--no-trace-env-native-stack', '-e', "console.log('trace-env-native-stack')"]); + assert.strictEqual(stdout, 'trace-env-native-stack\n'); + assert.strictEqual(stderr, ''); + }); + + it('should remove implies options when --no-experimental-transform-types is specified', + async () => { + { + const { stdout, stderr } = await common.spawnPromisified( + process.execPath, + ['--experimental-transform-types', + '--no-experimental-transform-types', + '-e', + "console.log('experimental-transform-types')"]); + assert.strictEqual(stdout, 'experimental-transform-types\n'); + assert.strictEqual(stderr, ''); + } + { + const { stderr } = await common.spawnPromisified( + process.execPath, + ['--experimental-transform-types', + '--no-experimental-transform-types', + '../fixtures/source-map/throw-async.mjs']); + assert.doesNotMatch( + stderr, + /at Throw \([^)]+throw-async\.ts:4:9\)/ + ); + } + }); + it('should remove shadow-realm option when negate shadow-realm options are specified', async () => { + { + const { stdout, stderr } = await common.spawnPromisified( + process.execPath, + ['--experimental-shadow-realm', + '--no-experimental-shadow-realm', + '-e', + "new ShadowRealm().eval('console.log(1)')", + ]); + assert.strictEqual(stdout, ''); + assert.match(stderr, /Error: Not supported/); + } + { + const { stdout, stderr } = await common.spawnPromisified( + process.execPath, + ['--harmony-shadow-realm', '--no-harmony-shadow-realm', '-e', "new ShadowRealm().eval('console.log(1)')"]); + assert.strictEqual(stdout, ''); + assert.match(stderr, /ReferenceError: ShadowRealm is not defined/); + } + { + const { stdout, stderr } = await common.spawnPromisified( + process.execPath, + ['--harmony-shadow-realm', '--no-experimental-shadow-realm', '-e', "new ShadowRealm().eval('console.log(1)')"]); + assert.strictEqual(stdout, ''); + assert.match(stderr, /Error: Not supported/); + } + { + const { stdout, stderr } = await common.spawnPromisified( + process.execPath, + ['--experimental-shadow-realm', '--no-harmony-shadow-realm', '-e', "new ShadowRealm().eval('console.log(1)')"]); + assert.strictEqual(stdout, ''); + assert.match(stderr, /ReferenceError: ShadowRealm is not defined/); + } + }); + + + it('should not affect with only negation option', async () => { + const { stdout, stderr } = await common.spawnPromisified( + process.execPath, + ['--no-inspect-brk', '-e', "console.log('inspect-brk')"]); + assert.strictEqual(stdout, 'inspect-brk\n'); + assert.strictEqual(stderr, ''); + }); + + it('should throw no boolean option error', async () => { + const { stdout, stderr } = await common.spawnPromisified( + process.execPath, + [`--env-file=../fixtures/dotenv/.env`, '--no-env-file', '-e', 'const foo = 1'], + { cwd: __dirname }); + + assert.strictEqual(stdout, ''); + assert.match(stderr, /--no-env-file is an invalid negation because it is not a boolean option/); + }); +});