From 671173e69473bdca3d84bcab619d6053989a08fe Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Sat, 22 Sep 2018 14:34:00 +0200
Subject: [PATCH 1/4] =?UTF-8?q?cli:=20normalize=20`=5F`=20=E2=86=92=20`-`?=
 =?UTF-8?q?=20when=20parsing=20options?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This allows for option syntax similar to V8’s one, e.g.
`--no_warnings` has the same effect as `--no-warnings`.
---
 doc/api/cli.md                                | 17 +++++---
 lib/internal/bootstrap/node.js                | 42 +++++--------------
 src/node_options-inl.h                        | 20 ++++-----
 src/node_options.cc                           | 12 +++---
 .../test-cli-node-options-disallowed.js       |  1 -
 test/parallel/test-cli-node-options.js        |  1 +
 test/parallel/test-pending-deprecation.js     |  8 ++++
 7 files changed, 43 insertions(+), 58 deletions(-)

diff --git a/doc/api/cli.md b/doc/api/cli.md
index 3f5441b82e7ce8..54f5cf4fd586b4 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -21,6 +21,18 @@ Execute without arguments to start the [REPL][].
 _For more info about `node inspect`, please see the [debugger][] documentation._
 
 ## Options
+<!-- YAML
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/REPLACEME
+    description: Underscores instead of dashes are now allowed for
+                 Node.js options as well, in addition to V8 options.
+-->
+
+All options, including V8 options, allow words to be separated by both
+dashes (`-`) or underscores (`_`).
+
+For example, `--pending-deprecation` is equivalent to `--pending_deprecation`.
 
 ### `-`
 <!-- YAML
@@ -390,11 +402,6 @@ added: v0.1.3
 
 Print V8 command line options.
 
-V8 options allow words to be separated by both dashes (`-`) or
-underscores (`_`).
-
-For example, `--stack-trace-limit` is equivalent to `--stack_trace_limit`.
-
 ### `--v8-pool-size=num`
 <!-- YAML
 added: v5.10.0
diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
index 8537131e8be9a7..56c88542e3858d 100644
--- a/lib/internal/bootstrap/node.js
+++ b/lib/internal/bootstrap/node.js
@@ -720,25 +720,18 @@
     // Save references so user code does not interfere
     const replace = Function.call.bind(String.prototype.replace);
     const has = Function.call.bind(Set.prototype.has);
-    const test = Function.call.bind(RegExp.prototype.test);
 
     const get = () => {
       const {
         getOptions,
-        types: { kV8Option },
         envSettings: { kAllowedInEnvironment }
       } = internalBinding('options');
       const { options, aliases } = getOptions();
 
-      const allowedV8EnvironmentFlags = [];
       const allowedNodeEnvironmentFlags = [];
       for (const [name, info] of options) {
         if (info.envVarSettings === kAllowedInEnvironment) {
-          if (info.type === kV8Option) {
-            allowedV8EnvironmentFlags.push(name);
-          } else {
-            allowedNodeEnvironmentFlags.push(name);
-          }
+          allowedNodeEnvironmentFlags.push(name);
         }
       }
 
@@ -772,11 +765,9 @@
       // process.allowedNodeEnvironmentFlags.has() which lack leading dashes.
       // Avoid interference w/ user code by flattening `Set.prototype` into
       // each object.
-      const [nodeFlags, v8Flags] = [
-        allowedNodeEnvironmentFlags, allowedV8EnvironmentFlags
-      ].map((flags) => Object.defineProperties(
-        new Set(flags.map(trimLeadingDashes)),
-        Object.getOwnPropertyDescriptors(Set.prototype))
+      const nodeFlags = Object.defineProperties(
+        new Set(allowedNodeEnvironmentFlags.map(trimLeadingDashes)),
+        Object.getOwnPropertyDescriptors(Set.prototype)
       );
 
       class NodeEnvironmentFlagsSet extends Set {
@@ -800,29 +791,16 @@
         has(key) {
           // This will return `true` based on various possible
           // permutations of a flag, including present/missing leading
-          // dash(es) and/or underscores-for-dashes in the case of V8-specific
-          // flags.  Strips any values after `=`, inclusive.
+          // dash(es) and/or underscores-for-dashes.
+          // Strips any values after `=`, inclusive.
           // TODO(addaleax): It might be more flexible to run the option parser
           // on a dummy option set and see whether it rejects the argument or
           // not.
           if (typeof key === 'string') {
             key = replace(key, trailingValuesRegex, '');
-            if (test(leadingDashesRegex, key)) {
-              return has(this, key) ||
-                    has(v8Flags,
-                        replace(
-                          replace(
-                            key,
-                            leadingDashesRegex,
-                            ''
-                          ),
-                          replaceDashesRegex,
-                          '_'
-                        )
-                    );
-            }
-            return has(nodeFlags, key) ||
-                  has(v8Flags, replace(key, replaceDashesRegex, '_'));
+            key = replace(key, leadingDashesRegex, '');
+            key = replace(key, replaceDashesRegex, '_');
+            return has(nodeFlags, key);
           }
           return false;
         }
@@ -833,7 +811,7 @@
 
       return process.allowedNodeEnvironmentFlags = Object.freeze(
         new NodeEnvironmentFlagsSet(
-          allowedNodeEnvironmentFlags.concat(allowedV8EnvironmentFlags)
+          allowedNodeEnvironmentFlags
         ));
     };
 
diff --git a/src/node_options-inl.h b/src/node_options-inl.h
index 1d1eda54751a8f..9642b59a2368f8 100644
--- a/src/node_options-inl.h
+++ b/src/node_options-inl.h
@@ -307,6 +307,13 @@ void OptionsParser<Options>::Parse(
     if (equals_index != std::string::npos)
       original_name += '=';
 
+    {
+      // Normalize by replacing `_` with `-` in options.
+      std::string::size_type index = 2;  // Start after initial '--'.
+      while ((index = name.find('_', index + 1)) != std::string::npos)
+        name[index] = '-';
+    }
+
     {
       auto it = aliases_.end();
       // Expand aliases:
@@ -341,19 +348,6 @@ void OptionsParser<Options>::Parse(
 
     auto it = options_.find(name);
 
-    if (it == options_.end()) {
-      // We would assume that this is a V8 option if neither we nor any child
-      // parser knows about it, so we convert - to _ for
-      // canonicalization (since V8 accepts both) and look up again in order
-      // to find a match.
-      // TODO(addaleax): Make the canonicalization unconditional, i.e. allow
-      // both - and _ in Node's own options as well.
-      std::string::size_type index = 2;  // Start after initial '--'.
-      while ((index = name.find('-', index + 1)) != std::string::npos)
-        name[index] = '_';
-      it = options_.find(name);
-    }
-
     if ((it == options_.end() ||
          it->second.env_setting == kDisallowedInEnvironment) &&
         required_env_settings == kAllowedInEnvironment) {
diff --git a/src/node_options.cc b/src/node_options.cc
index 156ea11fe91ef0..4951679994a243 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -102,8 +102,6 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             &EnvironmentOptions::experimental_worker,
             kAllowedInEnvironment);
   AddOption("--expose-internals", "", &EnvironmentOptions::expose_internals);
-  // TODO(addaleax): Remove this when adding -/_ canonicalization to the parser.
-  AddAlias("--expose_internals", "--expose-internals");
   AddOption("--loader",
             "(with --experimental-modules) use the specified file as a "
             "custom loader",
@@ -204,15 +202,15 @@ PerIsolateOptionsParser::PerIsolateOptionsParser() {
             kAllowedInEnvironment);
 
   // Explicitly add some V8 flags to mark them as allowed in NODE_OPTIONS.
-  AddOption("--abort_on_uncaught_exception",
+  AddOption("--abort-on-uncaught-exception",
             "aborting instead of exiting causes a core file to be generated "
             "for analysis",
             V8Option{},
             kAllowedInEnvironment);
-  AddOption("--max_old_space_size", "", V8Option{}, kAllowedInEnvironment);
-  AddOption("--perf_basic_prof", "", V8Option{}, kAllowedInEnvironment);
-  AddOption("--perf_prof", "", V8Option{}, kAllowedInEnvironment);
-  AddOption("--stack_trace_limit", "", V8Option{}, kAllowedInEnvironment);
+  AddOption("--max-old-space-size", "", V8Option{}, kAllowedInEnvironment);
+  AddOption("--perf-basic-prof", "", V8Option{}, kAllowedInEnvironment);
+  AddOption("--perf-prof", "", V8Option{}, kAllowedInEnvironment);
+  AddOption("--stack-trace-limit", "", V8Option{}, kAllowedInEnvironment);
 
   Insert(&EnvironmentOptionsParser::instance,
          &PerIsolateOptions::get_per_env_options);
diff --git a/test/parallel/test-cli-node-options-disallowed.js b/test/parallel/test-cli-node-options-disallowed.js
index 0351f83c52b50b..733e5cd7c7b977 100644
--- a/test/parallel/test-cli-node-options-disallowed.js
+++ b/test/parallel/test-cli-node-options-disallowed.js
@@ -29,7 +29,6 @@ disallow('--interactive');
 disallow('-i');
 disallow('--v8-options');
 disallow('--');
-disallow('--no_warnings'); // Node options don't allow '_' instead of '-'.
 
 function disallow(opt) {
   const env = Object.assign({}, process.env, { NODE_OPTIONS: opt });
diff --git a/test/parallel/test-cli-node-options.js b/test/parallel/test-cli-node-options.js
index c7096a3acb55f7..4ff63009a7a1e3 100644
--- a/test/parallel/test-cli-node-options.js
+++ b/test/parallel/test-cli-node-options.js
@@ -19,6 +19,7 @@ expect(`-r ${printA}`, 'A\nB\n');
 expect(`-r ${printA} -r ${printA}`, 'A\nB\n');
 expect('--no-deprecation', 'B\n');
 expect('--no-warnings', 'B\n');
+expect('--no_warnings', 'B\n');
 expect('--trace-warnings', 'B\n');
 expect('--redirect-warnings=_', 'B\n');
 expect('--trace-deprecation', 'B\n');
diff --git a/test/parallel/test-pending-deprecation.js b/test/parallel/test-pending-deprecation.js
index f8b4ec8e5b7b94..bb11d84bcc0aec 100644
--- a/test/parallel/test-pending-deprecation.js
+++ b/test/parallel/test-pending-deprecation.js
@@ -36,6 +36,14 @@ switch (process.argv[2]) {
       assert.strictEqual(code, 0, message('--pending-deprecation'));
     }));
 
+    // Test the --pending_deprecation command line switch.
+    fork(__filename, ['switch'], {
+      execArgv: ['--pending_deprecation'],
+      silent: true
+    }).on('exit', common.mustCall((code) => {
+      assert.strictEqual(code, 0, message('--pending_deprecation'));
+    }));
+
     // Test the NODE_PENDING_DEPRECATION environment var.
     fork(__filename, ['env'], {
       env: Object.assign({}, process.env, { NODE_PENDING_DEPRECATION: 1 }),

From 841cf934dcfe5b64e37eb3c66b45d7a303c2e712 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Sat, 22 Sep 2018 15:03:32 +0200
Subject: [PATCH 2/4] =?UTF-8?q?fixup!=20cli:=20normalize=20`=5F`=20?=
 =?UTF-8?q?=E2=86=92=20`-`=20when=20parsing=20options?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 lib/internal/bootstrap/node.js                  | 11 +++++++----
 test/parallel/test-process-env-allowed-flags.js |  2 +-
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
index 56c88542e3858d..6eeb57fe45d0c0 100644
--- a/lib/internal/bootstrap/node.js
+++ b/lib/internal/bootstrap/node.js
@@ -713,13 +713,14 @@
     // This builds process.allowedNodeEnvironmentFlags
     // from data in the config binding
 
-    const replaceDashesRegex = /-/g;
+    const replaceUnderscoresRegex = /_/g;
     const leadingDashesRegex = /^--?/;
     const trailingValuesRegex = /=.*$/;
 
     // Save references so user code does not interfere
     const replace = Function.call.bind(String.prototype.replace);
     const has = Function.call.bind(Set.prototype.has);
+    const test = Function.call.bind(RegExp.prototype.test);
 
     const get = () => {
       const {
@@ -797,9 +798,11 @@
           // on a dummy option set and see whether it rejects the argument or
           // not.
           if (typeof key === 'string') {
-            key = replace(key, trailingValuesRegex, '');
-            key = replace(key, leadingDashesRegex, '');
-            key = replace(key, replaceDashesRegex, '_');
+            key = replace(key, replaceUnderscoresRegex, '-');
+            if (test(leadingDashesRegex, key)) {
+              key = replace(key, trailingValuesRegex, '');
+              return has(this, key);
+            }
             return has(nodeFlags, key);
           }
           return false;
diff --git a/test/parallel/test-process-env-allowed-flags.js b/test/parallel/test-process-env-allowed-flags.js
index b853bdd539852e..ddd8d894eabcb1 100644
--- a/test/parallel/test-process-env-allowed-flags.js
+++ b/test/parallel/test-process-env-allowed-flags.js
@@ -19,10 +19,10 @@ require('../common');
   ].concat(process.config.variables.v8_enable_inspector ? [
     '--inspect-brk',
     'inspect-brk',
+    '--inspect_brk',
   ] : []);
 
   const badFlags = [
-    '--inspect_brk',
     'INSPECT-BRK',
     '--INSPECT-BRK',
     '--r',

From a389c08cfd73e20cc9b8c3541f5a3e47ca9d1255 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Sat, 22 Sep 2018 16:16:05 +0200
Subject: [PATCH 3/4] =?UTF-8?q?fixup!=20cli:=20normalize=20`=5F`=20?=
 =?UTF-8?q?=E2=86=92=20`-`=20when=20parsing=20options?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 doc/api/cli.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/api/cli.md b/doc/api/cli.md
index 54f5cf4fd586b4..9c6195aa4e2f9e 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -24,7 +24,7 @@ _For more info about `node inspect`, please see the [debugger][] documentation._
 <!-- YAML
 changes:
   - version: REPLACEME
-    pr-url: https://github.com/nodejs/node/pull/REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/23020
     description: Underscores instead of dashes are now allowed for
                  Node.js options as well, in addition to V8 options.
 -->

From 7ca87b0100d84eddf6f92f3996c2650c18111657 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Sat, 22 Sep 2018 20:34:06 +0200
Subject: [PATCH 4/4] =?UTF-8?q?fixup!=20cli:=20normalize=20`=5F`=20?=
 =?UTF-8?q?=E2=86=92=20`-`=20when=20parsing=20options?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 src/node_options-inl.h | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/node_options-inl.h b/src/node_options-inl.h
index 9642b59a2368f8..277121036e519d 100644
--- a/src/node_options-inl.h
+++ b/src/node_options-inl.h
@@ -307,11 +307,10 @@ void OptionsParser<Options>::Parse(
     if (equals_index != std::string::npos)
       original_name += '=';
 
-    {
-      // Normalize by replacing `_` with `-` in options.
-      std::string::size_type index = 2;  // Start after initial '--'.
-      while ((index = name.find('_', index + 1)) != std::string::npos)
-        name[index] = '-';
+    // Normalize by replacing `_` with `-` in options.
+    for (std::string::size_type i = 2; i < name.size(); ++i) {
+      if (name[i] == '_')
+        name[i] = '-';
     }
 
     {