Skip to content

Commit

Permalink
cli: fix negated boolean options not removing implied options
Browse files Browse the repository at this point in the history
  • Loading branch information
islandryu committed Dec 19, 2024
1 parent 7560778 commit 4801938
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 8 deletions.
16 changes: 8 additions & 8 deletions src/node_options-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -388,18 +388,18 @@ void OptionsParser<Options>::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<bool>(options) =
imp->second.target_value;
auto p = imp->second.target_field->template Lookup<bool>(options);
if (is_negation) {
if (it->second.type == kBoolean)
*p = !imp->second.target_value;
} else {
*p = imp->second.target_value;
}
}
}
}
Expand Down
117 changes: 117 additions & 0 deletions test/parallel/test-negative-and-implies-options.js
Original file line number Diff line number Diff line change
@@ -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/);
});
});

0 comments on commit 4801938

Please sign in to comment.