src: allow -- value form for option arguments

The CLI option parser rejected any value beginning with `-` when
consuming the argument for a value-taking option such as
`--eval`, including the conventional `--` end-of-options marker.
As a result `node -pe -- -0` failed with `--eval requires an
argument`, and there was no way to pass a leading-dash value
(e.g. negative numeric literals) through `-e` / `-p` / `-pe`.

Recognize a literal `--` as an end-of-options marker when reading
the value for a value-taking option: pop the `--` and consume the
following argv element verbatim. Bare `node -pe -0` continues to
error so short-flag stacking and parsing of legitimate options
after `-e` are preserved.

Fixes: #43397
Signed-off-by: Maruthan G <[email protected]>

Author: maruthang

src/node_options-inl.h

@@ -465,14 +465,28 @@ void OptionsParser<Options>::Parse(
           break;
         }
 
-        value = args.pop_first();
-
-        if (!value.empty() && value[0] == '-') {
-          missing_argument();
-          break;
+        // Treat a literal `--` as an end-of-options marker for the value
+        // of an option that takes an argument. This allows passing values
+        // that themselves start with `-`, e.g. `node -e -- -0` or
+        // `node --eval -- -42`, mirroring the convention that arguments
+        // following `--` are positional.
+        if (args.first() == "--") {
+          args.pop_first();
+          if (args.empty()) {
+            missing_argument();
+            break;
+          }
+          value = args.pop_first();
         } else {
-          if (!value.empty() && value[0] == '\\' && value[1] == '-')
-            value = value.substr(1);  // Treat \- as escaping an -.
+          value = args.pop_first();
+
+          if (!value.empty() && value[0] == '-') {
+            missing_argument();
+            break;
+          } else {
+            if (!value.empty() && value[0] == '\\' && value[1] == '-')
+              value = value.substr(1);  // Treat \- as escaping an -.
+          }
         }
       }
     }

test/parallel/test-cli-eval-unary-minus.js

@@ -0,0 +1,89 @@
+'use strict';
+
+// Regression test for https://github.com/nodejs/node/issues/43397
+//
+// `node -pe '-0'` (and other expressions whose first character is `-`) used to
+// fail with "--eval requires an argument" because the option parser treated
+// the value as another flag. The supported way to pass such values is to
+// terminate the option list with `--`, e.g. `node -pe -- -0`.
+
+require('../common');
+const assert = require('assert');
+const { spawnSync } = require('child_process');
+
+function run(...args) {
+  const result = spawnSync(process.execPath, args, { encoding: 'utf8' });
+  return { stdout: result.stdout, stderr: result.stderr, status: result.status };
+}
+
+// `--` should let the next argv entry be consumed verbatim as the value of
+// `--eval`, even when it starts with `-`.
+{
+  const { stdout, stderr, status } = run('-pe', '--', '-0');
+  assert.strictEqual(status, 0, `stderr: ${stderr}`);
+  assert.strictEqual(stderr, '');
+  assert.strictEqual(stdout, '0\n');
+}
+
+{
+  const { stdout, stderr, status } = run('-pe', '--', '-1.5');
+  assert.strictEqual(status, 0, `stderr: ${stderr}`);
+  assert.strictEqual(stderr, '');
+  assert.strictEqual(stdout, '-1.5\n');
+}
+
+{
+  const { stdout, stderr, status } = run('-pe', '--', '-1+0');
+  assert.strictEqual(status, 0, `stderr: ${stderr}`);
+  assert.strictEqual(stderr, '');
+  assert.strictEqual(stdout, '-1\n');
+}
+
+// `-e` (no print) should also accept a leading-dash value via `--`.
+{
+  const { stdout, stderr, status } =
+    run('-e', '--', '-42; console.log("ok")');
+  assert.strictEqual(status, 0, `stderr: ${stderr}`);
+  assert.strictEqual(stderr, '');
+  assert.strictEqual(stdout, 'ok\n');
+}
+
+// The long-form `--eval` should behave the same way.
+{
+  const { stdout, stderr, status } = run('--print', '--eval', '--', '-7');
+  assert.strictEqual(status, 0, `stderr: ${stderr}`);
+  assert.strictEqual(stderr, '');
+  assert.strictEqual(stdout, '-7\n');
+}
+
+// `--eval=-42` already worked and must keep working.
+{
+  const { stdout, stderr, status } = run('--print', '--eval=-42');
+  assert.strictEqual(status, 0, `stderr: ${stderr}`);
+  assert.strictEqual(stderr, '');
+  assert.strictEqual(stdout, '-42\n');
+}
+
+// The pre-existing `\-` escape must keep working.
+{
+  const { stdout, stderr, status } = run('-pe', '\\-0');
+  assert.strictEqual(status, 0, `stderr: ${stderr}`);
+  assert.strictEqual(stderr, '');
+  assert.strictEqual(stdout, '0\n');
+}
+
+// Without `--`, a leading-dash value still produces the existing diagnostic;
+// that behavior is intentional so unrelated stacked flags keep being detected.
+{
+  const { stderr, status } = run('-pe', '-0');
+  assert.notStrictEqual(status, 0);
+  assert.match(stderr, /requires an argument/);
+}
+
+// Sanity: a positional value with no leading dash works without `--`.
+{
+  const { stdout, stderr, status } = run('-pe', '42');
+  assert.strictEqual(status, 0, `stderr: ${stderr}`);
+  assert.strictEqual(stderr, '');
+  assert.strictEqual(stdout, '42\n');
+}