From 4559f7024f5ded23d3711c3ceb3f9fda57a9c055 Mon Sep 17 00:00:00 2001 From: islandryu Date: Thu, 8 Jan 2026 14:55:07 +0900 Subject: [PATCH 1/4] cli: fix negated boolean options not removing implied options --- src/node_options-inl.h | 14 +-- .../test-negative-and-implies-options.js | 117 ++++++++++++++++++ 2 files changed, 124 insertions(+), 7 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 877e8ce4ded92b..b2408203fa226f 100644 --- a/src/node_options-inl.h +++ b/src/node_options-inl.h @@ -421,20 +421,20 @@ 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 [f, l] = implications_.equal_range(implied_name); + auto [f, l] = implications_.equal_range(name); std::ranges::for_each(std::ranges::subrange(f, l) | std::views::values, [&](const auto& value) { if (value.type == kV8Option) { v8_args->push_back(value.name); } else { + bool target_value = value.target_value; + if (is_negation && value.type == kBoolean) { + target_value = !value.target_value; + } *value.target_field->template Lookup( - options) = value.target_value; + options) = 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/); + }); +}); From 2c90317a8b0d6924e7b1641b772fb186c6f21f5b Mon Sep 17 00:00:00 2001 From: islandryu Date: Thu, 8 Jan 2026 15:52:20 +0900 Subject: [PATCH 2/4] fix lint --- src/node_options-inl.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/node_options-inl.h b/src/node_options-inl.h index b2408203fa226f..8af10fe05360a6 100644 --- a/src/node_options-inl.h +++ b/src/node_options-inl.h @@ -434,7 +434,6 @@ void OptionsParser::Parse( *value.target_field->template Lookup( options) = target_value; } - }); } From 2231fb588200e05d7d4fc754b22797cb51aaa818 Mon Sep 17 00:00:00 2001 From: islandryu Date: Thu, 8 Jan 2026 23:33:55 +0900 Subject: [PATCH 3/4] set default value when negated --- src/node_options-inl.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/node_options-inl.h b/src/node_options-inl.h index 8af10fe05360a6..ca9e7846711011 100644 --- a/src/node_options-inl.h +++ b/src/node_options-inl.h @@ -6,6 +6,7 @@ #include #include #include +#include #include "node_options.h" #include "util.h" @@ -334,6 +335,8 @@ void OptionsParser::Parse( if (v8_args->empty()) v8_args->push_back(args.program_name()); + std::unordered_map default_field_map = {}; + while (!args.empty() && errors->empty()) { if (args.first().size() <= 1 || args.first()[0] != '-') break; @@ -428,8 +431,13 @@ void OptionsParser::Parse( v8_args->push_back(value.name); } else { bool target_value = value.target_value; + if (!default_field_map.contains(value.name)) { + default_field_map[value.name] = + *value.target_field + ->template Lookup(options); + } if (is_negation && value.type == kBoolean) { - target_value = !value.target_value; + target_value = default_field_map[value.name]; } *value.target_field->template Lookup( options) = target_value; From 7ff865b79963c9f4267ccb8da0fc592f72fd2d7f Mon Sep 17 00:00:00 2001 From: islandryu Date: Fri, 9 Jan 2026 09:24:51 +0900 Subject: [PATCH 4/4] skip test if inspector disabled --- test/parallel/test-negative-and-implies-options.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/parallel/test-negative-and-implies-options.js b/test/parallel/test-negative-and-implies-options.js index 1577e47c52829e..1617eacc08df7b 100644 --- a/test/parallel/test-negative-and-implies-options.js +++ b/test/parallel/test-negative-and-implies-options.js @@ -1,6 +1,9 @@ 'use strict'; const common = require('../common'); + +common.skipIfInspectorDisabled(); + const assert = require('assert'); const { describe, it } = require('node:test');