fix: fixup parsing negative numbers and quoted strings

lib/cli/parse-args.js

@@ -13,7 +13,9 @@ import {
   types,
 } from "./run-option-metadata.cjs";
 import { isNodeFlag } from "./node-flags.cjs";
+import debugModule from "debug";
 
+const debug = debugModule("mocha:cli:parse-args");
 const globOptions = ["spec", "ignore"];
 const aliasToCanonical = Object.entries(aliases).reduce(
   (acc, [canonical, optionAliases]) => {
@@ -186,6 +188,103 @@ const requiresValue = (name) => {
   );
 };
 
+/**
+ * Splits for downstream handling.
+ * Does not apply Mocha-specific logic.
+ * If arg is already string array, returns arg.
+ * Respects escaped chars and quoted strings.
+ * @param {string|string[]} args Input to be split, e.g. `"--grep -1"`
+ * @returns {string[]} One entry per CLI arg-equivalent
+ * @sample
+ * ```js
+ * const x = splitArgs("--timeout -1 --grep 'foo \' bar' --color");
+ * x === ["--timeout", "-1", "--grep", "foo ' bar", "--color"]; // true
+ * ```
+ */
+const splitArgs = (args) => {
+  debug(`starting splitArgs(${args})`);
+  if (!args) {
+    debug("splitArgs took in falsy value, returning empty array");
+    return [];
+  }
+
+  if (Array.isArray(args)) {
+    debug("splitArgs took in array, returning argument unchanged");
+    return args;
+  }
+
+  const result = [];
+  let current = "";
+  let tokenStarted = false;
+  let quote = null;
+  let escaped = false;
+
+  for (const char of args) {
+    if (escaped) {
+      current += char;
+      tokenStarted = true;
+      escaped = false;
+      continue;
+    }
+
+    if (char === "\\") {
+      tokenStarted = true;
+      escaped = true;
+      continue;
+    }
+
+    if (quote) {
+      if (char === quote) {
+        quote = null;
+      } else {
+        current += char;
+        tokenStarted = true;
+      }
+      continue;
+    }
+
+    if (char === '"' || char === "'") {
+      tokenStarted = true;
+      quote = char;
+      continue;
+    }
+    // At this point, we are not in quoted mode
+
+    // Using newlines in CLI in this way is difficult.
+    // We do not handle it for now to keep tests simple.
+    if (/\t| /.test(char)) {
+      if (tokenStarted) {
+        result.push(current);
+        current = "";
+        tokenStarted = false;
+      }
+      continue;
+    }
+
+    current += char;
+    tokenStarted = true;
+  }
+
+  if (escaped) {
+    current += "\\";
+  }
+
+  // Unterminated is OK, usually passed via MOCHA_OPTIONS
+  if (quote) {
+    debug("unterminated quoted token, prepending quote");
+    current = quote + current;
+    debug(`updated token: ${current}`);
+  }
+
+  if (tokenStarted) {
+    result.push(current);
+  }
+
+  debug(`finishing splitArgs(${args})`);
+  debug(`returning ${result}`);
+  return result;
+};
+
 /**
  * Disallows multiple arguments after a single option, such as `--grep abc def`.
  */
@@ -198,7 +297,10 @@ const validateArgsBeforeParse = (allArgs) => {
     const name = canonicalOptionName(stripLeadingDashes(arg));
     const next = allArgs[index + 1];
 
-    if (requiresValue(name) && (next === undefined || next.startsWith("-"))) {
+    if (
+      requiresValue(name) &&
+      (next === undefined || (next.startsWith("-") && !isNumeric(next)))
+    ) {
       throw new Error(`Not enough arguments following: ${name}`);
     }
   });
@@ -271,12 +373,25 @@ const createErrorForNumericPositionalArg = (
   );
 };
 
+/**
+ * Parse the args into a config object.
+ * Splits,
+ * handles Node args,
+ * validates general args,
+ * parses into config object,
+ * normalizes,
+ * validates numerical args,
+ * and returns resulting config object.
+ * @param {string|string[]} args
+ * @param {object} defaultValues
+ * @param  {...any} configObjects
+ */
 export const parseMochaArgs = (
   args = [],
   defaultValues = {},
   ...configObjects
 ) => {
-  const allArgs = Array.isArray(args) ? args : args ? args.split(" ") : [];
+  const allArgs = splitArgs(args);
 
   // Save Node-specific args for special handling. Equals-form args have values;
   // the rest are boolean flags.

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

@@ -466,6 +466,62 @@ describe("options", function () {
           color: true,
         });
       });
+
+      it("should parse quoted flags from MOCHA_OPTIONS", function () {
+        readFileSync = sinon.stub().onFirstCall().returns("{}");
+        findConfig = sinon.stub().returns("/some/.mocharc.json");
+        loadConfig = sinon.stub().returns({});
+        findupSync = sinon.stub().returns("/some/package.json");
+        sinon
+          .stub(process, "env")
+          .value({ MOCHA_OPTIONS: "--grep 'foo bar' --color" });
+
+        loadOptions = proxyLoadOptions({
+          readFileSync,
+          findConfig,
+          loadConfig,
+          findupSync,
+        });
+
+        expect(loadOptions(), "to satisfy", {
+          grep: "foo bar",
+          color: true,
+        });
+      });
+
+      it("should allow negative numeric values in MOCHA_OPTIONS", function () {
+        readFileSync = sinon.stub().onFirstCall().returns("{}");
+        findConfig = sinon.stub().returns("/some/.mocharc.json");
+        loadConfig = sinon.stub().returns({});
+        findupSync = sinon.stub().returns("/some/package.json");
+        sinon.stub(process, "env").value({ MOCHA_OPTIONS: "--timeout -1" });
+
+        loadOptions = proxyLoadOptions({
+          readFileSync,
+          findConfig,
+          loadConfig,
+          findupSync,
+        });
+
+        expect(loadOptions(), "to have property", "timeout", "-1");
+      });
+
+      it("should allow dash-prefixed option values in MOCHA_OPTIONS", function () {
+        readFileSync = sinon.stub().onFirstCall().returns("{}");
+        findConfig = sinon.stub().returns("/some/.mocharc.json");
+        loadConfig = sinon.stub().returns({});
+        findupSync = sinon.stub().returns("/some/package.json");
+        sinon.stub(process, "env").value({ MOCHA_OPTIONS: "--grep -1" });
+
+        loadOptions = proxyLoadOptions({
+          readFileSync,
+          findConfig,
+          loadConfig,
+          findupSync,
+        });
+
+        expect(loadOptions(), "to have property", "grep", "-1");
+      });
     });
 
     describe("config priority", function () {

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

@@ -247,6 +247,85 @@ describe("parse-args", function () {
     );
   });
 
+  it("accepts negative numeric option values in separated form", function () {
+    expect(parseMochaArgs(["--timeout", "-1"], defaults), "to satisfy", {
+      timeout: "-1",
+    });
+  });
+
+  it("accepts negative numeric option values from strings", function () {
+    expect(parseMochaArgs("--timeout -1", defaults), "to satisfy", {
+      timeout: "-1",
+    });
+  });
+
+  it("accepts decimal and exponential negative numeric option values", function () {
+    ["-1.5", "-.5", "-1e3"].forEach((timeout) => {
+      expect(parseMochaArgs(["--timeout", timeout], defaults), "to satisfy", {
+        timeout,
+      });
+    });
+  });
+
+  it("parses double-quoted arguments from strings", function () {
+    expect(
+      parseMochaArgs('--grep "foo bar" --retries 3 --color', defaults),
+      "to satisfy",
+      {
+        grep: "foo bar",
+        retries: 3,
+        color: true,
+      },
+    );
+  });
+
+  it("parses single-quoted arguments from strings", function () {
+    expect(parseMochaArgs("--grep 'foo bar' --color", defaults), "to satisfy", {
+      grep: "foo bar",
+      color: true,
+    });
+  });
+
+  it("parses quoted values attached to options from strings", function () {
+    expect(parseMochaArgs('--grep="foo bar"', defaults), "to satisfy", {
+      grep: "foo bar",
+    });
+  });
+
+  it("preserves empty quoted option values from strings", function () {
+    expect(parseMochaArgs('--grep ""', defaults), "to satisfy", {
+      grep: "",
+    });
+  });
+
+  it("parses escaped spaces and quotes from strings", function () {
+    expect(
+      parseMochaArgs('--grep foo\\ bar --fgrep "a \\"quote\\""', defaults),
+      "to satisfy",
+      {
+        grep: "foo bar",
+        fgrep: 'a "quote"',
+      },
+    );
+  });
+
+  it("splits arguments on all spaces and tabs", function () {
+    expect(
+      parseMochaArgs("\t--grep  \t foo   --color", defaults),
+      "to satisfy",
+      {
+        grep: "foo",
+        color: true,
+      },
+    );
+  });
+
+  it("accepts unterminated quoted arguments", function () {
+    expect(parseMochaArgs('--grep "foo', defaults), "to satisfy", {
+      grep: '"foo',
+    });
+  });
+
   it("preserves directly-passed Node and V8 flags", function () {
     expect(
       parseMochaArgs(