Repository: mochajs/mocha

Problem statement:
cli option `--require trace-something` causes `bad option`-error
### Prerequisites

- [x] Checked that your issue hasn't already been filed by cross-referencing [issues with the `faq` label](https://github.com/mochajs/mocha/issues?utf8=%E2%9C%93&q=is%3Aissue%20label%3Afaq%20)
- [x] Checked next-gen ES issues and syntax problems by using the same environment and/or transpiler configuration without Mocha to ensure it isn't just a feature that actually isn't supported in the environment in question or a bug in your code.
- [x] 'Smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, your usage of Mocha, or Mocha itself
- [x] Ensured that there is no discrepancy between the locally and globally installed versions of Mocha. You can find them with: `node node_modules/.bin/mocha --version`(Local) and `mocha --version`(Global). We recommend avoiding the use of globally installed Mocha.

### Description

When executing mocha with the option `--require trace-something`, the error `bad-option --trace-something` is output. In my case. it is `trace-and-clarify-if-possible`, but it is reproducible with other package names.

This happens in `mocha@6.0.0` and `mocha@6.0.1`, but not in `mocha@6.0.0-1`, and only if the required package-name starts with "trace-"

### Steps to Reproduce

```
# Setup dependency with empty index.js-file
mkdir trace-dependency
cd trace-dependency/
npm init -y
touch index.js
cd ..

# Setup project and add dependency
mkdir test-project
cd test-project
npm init -y
npm install ../trace-dependency
mkdir test
touch test/dash-spec.js
```

Then install mocha and run the tests:

```
npm install mocha@6.0.0
npx mocha --require trace-dependency
# Error "bad option --trace-dependency"

npm install mocha@6.0.1
npx mocha --require trace-dependency
# Error "bad option --trace-dependency"

npm install mocha@6.0.0-1
npx mocha --require trace-dependency
# No error, "0 passing"
```

**Expected behavior:** [What you expect to happen]

I would have expected the same result for each mocha-version, i.e. the output "0 passing".

**Actual behavior:** [What actually happens]

* `mocha@6.0.0` and `mocha@6.0.1` have the output "bad option --trace-dependency"
* `mocha@6.0.0-1` has the expected output

**Reproduces how often:** [What percentage of the time does it reproduce?]

100%

### Versions

- The output of `mocha --version` and `node node_modules/.bin/mocha --version`: 6.0.0, 6.0.1
- The output of `node --version`: v10.15.1
- The version and architecture of your operating system: Xubuntu 18.04
- Your shell (bash, zsh, PowerShell, cmd, etc.): bash
- Any other third party Mocha related modules (with versions): none
- The code transpiler being used: none

### Additional Information

<!--
Any additional information, configuration or data that might be necessary to reproduce the issue.
-->



Hints:
also happens when
```
mocha --trace-warnings --inspect
/usr/local/bin/node: bad option: --trace-warnings --inspect
```
but not when
```
mocha --inspect
```
or
```
mocha --trace-warnings
```
Reasonably sure this has to do with the Node flag processing going on [here](https://github.com/mochajs/mocha/blob/6535965e8655a66de54fc0ad9465c2eb825f13f8/lib/cli/node-flags.js#L19-L41).
The problem here is that its checking for node-flags, when the original `--` or `-` is already removed by yargs or [this little line](https://github.com/mochajs/mocha/blob/6535965e8655a66de54fc0ad9465c2eb825f13f8/lib/cli/options.js#L88).

A workaround for my problem is to use `--require=trace-and-clarify-if-possible`, but for @Janpot this will not work.

I don't have a good solution to this, other than doing the whole preprocessing before `yargs` gets to parse the arguments...

Patch:
diff --git a/lib/cli/node-flags.js b/lib/cli/node-flags.js
--- a/lib/cli/node-flags.js
+++ b/lib/cli/node-flags.js
@@ -29,16 +29,32 @@ const debugFlags = new Set(['debug', 'debug-brk', 'inspect', 'inspect-brk']);
  *   - `--v8-*` (but *not* `--v8-options`)
  * @summary Whether or not to pass a flag along to the `node` executable.
  * @param {string} flag - Flag to test
- * @returns {boolean}
+ * @param {boolean} [bareword=true] - If `false`, we expect `flag` to have one or two leading dashes.
+ * @returns {boolean} If the flag is considered a "Node" flag.
  * @private
  */
-exports.isNodeFlag = flag =>
-  !/^(?:require|r)$/.test(flag) &&
-  (nodeFlags.has(flag) ||
-    debugFlags.has(flag) ||
-    /(?:preserve-symlinks(?:-main)?|harmony(?:[_-]|$)|(?:trace[_-].+$)|gc(?:[_-]global)?$|es[_-]staging$|use[_-]strict$|v8[_-](?!options).+?$)/.test(
-      flag
-    ));
+exports.isNodeFlag = (flag, bareword = true) => {
+  if (!bareword) {
+    // check if the flag begins with dashes; if not, not a node flag.
+    if (!/^--?/.test(flag)) {
+      return false;
+    }
+    // strip the leading dashes to match against subsequent checks
+    flag = flag.replace(/^--?/, '');
+  }
+  return (
+    // treat --require/-r as Mocha flag even though it's also a node flag
+    !(flag === 'require' || flag === 'r') &&
+    // check actual node flags from `process.allowedNodeEnvironmentFlags`,
+    // then historical support for various V8 and non-`NODE_OPTIONS` flags
+    // and also any V8 flags with `--v8-` prefix
+    (nodeFlags.has(flag) ||
+      debugFlags.has(flag) ||
+      /(?:preserve-symlinks(?:-main)?|harmony(?:[_-]|$)|(?:trace[_-].+$)|gc(?:[_-]global)?$|es[_-]staging$|use[_-]strict$|v8[_-](?!options).+?$)/.test(
+        flag
+      ))
+  );
+};
 
 /**
  * Returns `true` if the flag is a "debug-like" flag.  These require timeouts
diff --git a/lib/cli/options.js b/lib/cli/options.js
--- a/lib/cli/options.js
+++ b/lib/cli/options.js
@@ -85,8 +85,9 @@ const parse = (args = [], ...configObjects) => {
   const nodeArgs = (Array.isArray(args) ? args : args.split(' ')).reduce(
     (acc, arg) => {
       const pair = arg.split('=');
-      const flag = pair[0].replace(/^--?/, '');
-      if (isNodeFlag(flag)) {
+      let flag = pair[0];
+      if (isNodeFlag(flag, false)) {
+        flag = flag.replace(/^--?/, '');
         return arg.includes('=')
           ? acc.concat([[flag, pair[1]]])
           : acc.concat([[flag, true]]);


Test patch:
diff --git a/test/integration/options/node-flags.spec.js b/test/integration/options/node-flags.spec.js
new file mode 100644
--- /dev/null
+++ b/test/integration/options/node-flags.spec.js
@@ -0,0 +1,19 @@
+'use strict';
+
+var invokeMocha = require('../helpers').invokeMocha;
+
+describe('node flags', function() {
+  it('should not consider argument values to be node flags', function(done) {
+    invokeMocha(
+      ['--require', 'trace-dependency'],
+      function(err, res) {
+        if (err) {
+          return done(err);
+        }
+        expect(res, 'not to have failed with output', /bad option/i);
+        done();
+      },
+      'pipe'
+    );
+  });
+});
diff --git a/test/node-unit/cli/node-flags.spec.js b/test/node-unit/cli/node-flags.spec.js
--- a/test/node-unit/cli/node-flags.spec.js
+++ b/test/node-unit/cli/node-flags.spec.js
@@ -15,6 +15,18 @@ describe('node-flags', function() {
       });
     });
 
+    describe('when expecting leading dashes', function() {
+      it('should require leading dashes', function() {
+        expect(isNodeFlag('throw-deprecation', false), 'to be false');
+        expect(isNodeFlag('--throw-deprecation', false), 'to be true');
+      });
+
+      it('should return false for --require/-r', function() {
+        expect(isNodeFlag('--require', false), 'to be false');
+        expect(isNodeFlag('-r', false), 'to be false');
+      });
+    });
+
     describe('special cases', function() {
       it('should return true for flags starting with "preserve-symlinks"', function() {
         expect(isNodeFlag('preserve-symlinks'), 'to be true');