refactor: extract stripLeadingDashes helper + cover CLI arg-parsing edge cases (#6171)

* refactor: replace yargs-parser in CLI options

Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019f1368-7b8d-775f-bf67-9d38a255d13f

* test: cover CLI arg-parsing edge cases

Adds unit tests for scenarios in the new parseArgs-based CLI option
loading that were previously unexercised:

- parse-args: bare numeric positional -> UNSUPPORTED error; string and
  empty-string args input; falsy config objects skipped while merging;
  positionals merged across multiple config objects; non-boolean value
  kept for a boolean option in equals form.
- options: non-Mocha parse errors are printed and exit with code 1.

Test-only change; no production code is modified.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* refactor: extract stripLeadingDashes helper

Replace seven copies of `flag.replace(/^--?/, "")` across parse-args.js,
run-option-metadata.cjs, and node-flags.cjs with a single exported
`stripLeadingDashes` helper in run-option-metadata.cjs.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Revert changes to package-lock

* Format

---------

Co-authored-by: Josh Goldberg <git@joshuakgoldberg.com>
Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

Author: 4 people

lib/cli/node-flags.cjs

@@ -7,7 +7,10 @@
  */
 
 const nodeFlags = process.allowedNodeEnvironmentFlags;
-const { isMochaFlag } = require("./run-option-metadata.cjs");
+const {
+  isMochaFlag,
+  stripLeadingDashes,
+} = require("./run-option-metadata.cjs");
 const { unparseNodeFlags } = require("./unparse-args.js");
 
 /**
@@ -41,7 +44,7 @@ function isNodeFlag(flag, bareword = true) {
       return false;
     }
     // strip the leading dashes to match against subsequent checks
-    flag = flag.replace(/^--?/, "");
+    flag = stripLeadingDashes(flag);
   }
   return (
     // check actual node flags from `process.allowedNodeEnvironmentFlags`,

lib/cli/parse-args.js

@@ -9,6 +9,7 @@ import {
   aliases,
   expectedTypeForFlag,
   isMochaFlag,
+  stripLeadingDashes,
   types,
 } from "./run-option-metadata.cjs";
 import { isNodeFlag } from "./node-flags.cjs";
@@ -161,7 +162,7 @@ const mergeConfigObjects = (defaultValues, configObjects) => {
 
 const preprocessArgs = (allArgs) => {
   return allArgs.map((arg) => {
-    const noPrefix = arg.replace(/^--?/, "");
+    const noPrefix = stripLeadingDashes(arg);
     const [name, ...rest] = noPrefix.split("=");
     const canonical = canonicalOptionName(name.replace(/^no-/, ""));
     const negated = name.startsWith("no-");
@@ -194,7 +195,7 @@ const validateArgsBeforeParse = (allArgs) => {
       return;
     }
 
-    const name = canonicalOptionName(arg.replace(/^--?/, ""));
+    const name = canonicalOptionName(stripLeadingDashes(arg));
     const next = allArgs[index + 1];
 
     if (requiresValue(name) && (next === undefined || next.startsWith("-"))) {
@@ -249,7 +250,7 @@ const createErrorForNumericPositionalArg = (
   // 2. parseArgs() could not assign the numericArg to that flag because the
   //    option has an incompatible datatype.
   const flag = allArgs.find((arg, index) => {
-    const normalizedArg = arg.replace(/^--?/, "");
+    const normalizedArg = stripLeadingDashes(arg);
     return (
       isMochaFlag(arg) &&
       allArgs[index + 1] === String(numericArg) &&
@@ -283,7 +284,7 @@ export const parseMochaArgs = (
     const pair = arg.split("=");
     let flag = pair[0];
     if (isNodeFlag(flag, false)) {
-      flag = flag.replace(/^--?/, "");
+      flag = stripLeadingDashes(flag);
       return acc.concat([[flag, arg.includes("=") ? pair[1] : true]]);
     }
     return acc;

lib/cli/run-option-metadata.cjs

@@ -169,14 +169,23 @@ const ALL_MOCHA_FLAGS = Object.keys(types).reduce((acc, key) => {
   return acc;
 }, new Set());
 
+/**
+ * Strips one or two leading dashes from a flag, e.g. `--grep` becomes `grep`
+ * and `-g` becomes `g`. Barewords are returned unchanged.
+ * @param {string} flag - Flag to normalize (with or without leading dashes)
+ * @returns {string} The flag without its leading dashes
+ * @private
+ */
+const stripLeadingDashes = (flag) => flag.replace(/^--?/, "");
+
 /**
  * Returns `true` if the provided `flag` is known to Mocha.
  * @param {string} flag - Flag to check
  * @returns {boolean} If `true`, this is a Mocha flag
  * @private
  */
 const isMochaFlag = (flag) => {
-  return ALL_MOCHA_FLAGS.has(flag.replace(/^--?/, ""));
+  return ALL_MOCHA_FLAGS.has(stripLeadingDashes(flag));
 };
 
 /**
@@ -186,7 +195,7 @@ const isMochaFlag = (flag) => {
  * @private
  */
 const expectedTypeForFlag = (flag) => {
-  const normalizedName = flag.replace(/^--?/, "");
+  const normalizedName = stripLeadingDashes(flag);
 
   // If flag is an alias, get it's full name.
   const fullFlagName =
@@ -204,3 +213,4 @@ exports.aliases = aliases;
 exports.getRunOptionDefinitions = getRunOptionDefinitions;
 exports.isMochaFlag = isMochaFlag;
 exports.expectedTypeForFlag = expectedTypeForFlag;
+exports.stripLeadingDashes = stripLeadingDashes;

test/node-unit/cli/options.spec.js

@@ -788,5 +788,44 @@ describe("options", function () {
         expect(() => loadOptions(`--spec ${numericArg}`), "not to throw");
       });
     });
+
+    describe("when parsing throws a non-Mocha error", function () {
+      let exit;
+      let consoleError;
+
+      beforeEach(function () {
+        readFileSync = sinon.stub();
+        findConfig = sinon.stub();
+        loadConfig = sinon.stub();
+        findupSync = sinon.stub();
+        loadOptions = proxyLoadOptions({
+          readFileSync,
+          findConfig,
+          loadConfig,
+          findupSync,
+        });
+        exit = sinon.stub(process, "exit");
+        consoleError = sinon.stub(console, "error");
+      });
+
+      it("prints the error message and exits with code 1", function () {
+        try {
+          // `--grep` requires a value, so a dash-prefixed follower throws a
+          // plain (non-Mocha) Error rather than a Mocha error.
+          loadOptions(["--grep", "-foo"]);
+        } catch {
+          // `process.exit` is stubbed, so execution continues past it and may
+          // throw later; we only assert that the error was reported and exit
+          // was requested.
+        }
+        expect(exit, "to have a call satisfying", [1]);
+        expect(consoleError, "was called");
+        expect(
+          consoleError.firstCall.args[0],
+          "to contain",
+          "Not enough arguments following: grep",
+        );
+      });
+    });
   });
 });

test/node-unit/cli/parse-args.spec.js

@@ -289,4 +289,53 @@ describe("parse-args", function () {
       expected: "boolean",
     });
   });
+
+  it("throws an unsupported error for a bare numeric positional argument", function () {
+    expect(() => parseMochaArgs(["123"], defaults), "to throw", {
+      message: "Option 123 is unsupported by the mocha cli",
+      code: constants.UNSUPPORTED,
+    });
+  });
+
+  it("accepts a space-delimited string of arguments", function () {
+    expect(parseMochaArgs("--bail --grep foo", defaults), "to satisfy", {
+      bail: true,
+      grep: "foo",
+    });
+  });
+
+  it("treats an empty string of arguments as no arguments", function () {
+    expect(parseMochaArgs("", defaults), "to satisfy", {
+      _: [],
+      timeout: 1000,
+      extension: ["js"],
+    });
+  });
+
+  it("ignores falsy config objects while merging the rest", function () {
+    expect(
+      parseMochaArgs(["--bail"], defaults, null, undefined, { grep: "x" }),
+      "to satisfy",
+      {
+        bail: true,
+        grep: "x",
+      },
+    );
+  });
+
+  it("merges positional arguments across multiple config objects", function () {
+    expect(
+      parseMochaArgs([], defaults, { _: ["a.js"] }, { _: ["b.js"] }),
+      "to satisfy",
+      {
+        _: ["a.js", "b.js"],
+      },
+    );
+  });
+
+  it("keeps a non-boolean value for a boolean option given in equals form", function () {
+    expect(parseMochaArgs(["--bail=maybe"], defaults), "to satisfy", {
+      bail: "maybe",
+    });
+  });
 });