fix(cli): fix bug with parsing --fooBar=baz type CLI flags (#3483)

This fixes a bug with how these flags were being parsed which was
messing with Jest. Basically, the issue related to how the `knownArgs`
array on the `ConfigFlags` object was being populated. For CLI key-value
args of the format `--argName=value` the whole string
`"--argName=value"` was being added _as well as_ the value string
`"value"`, which had the effect of passing duplicate args to Jest.

This refactors how such arguments are handled to always parse apart the
argument name and the value, adding only the argument name and then the
argument value, so that if you pass, for instance,
`--outputFile=output.json`, the args passed to Jest will look like

```ts
['--outputFile', 'output.json']
```

See #3471 and #3481 for more details

Author: alicewriteswrongs

src/cli/parse-flags.ts

@@ -51,9 +51,13 @@ export const parseFlags = (args: string[], sys?: CompilerSystem): ConfigFlags =>
     }
   }
 
-  flags.unknownArgs = flags.args.filter((arg: string) => {
-    return !flags.knownArgs.includes(arg);
-  });
+  // to find unknown / unrecognized arguments we filter `args`, including only
+  // arguments whose normalized form is not found in `knownArgs`. `knownArgs`
+  // is populated during the call to `parseArgs` above. For arguments like
+  // `--foobar` the string `"--foobar"` will be added, while for more
+  // complicated arguments like `--bizBoz=bop` or `--bizBoz bop` just the
+  // string `"--bizBoz"` will be added.
+  flags.unknownArgs = flags.args.filter((arg: string) => !flags.knownArgs.includes(parseEqualsArg(arg)[0]));
 
   return flags;
 };
@@ -259,17 +263,18 @@ const getValue = (
 
   let value: string | undefined;
   let matchingArg: string | undefined;
+
   args.forEach((arg, i) => {
     if (arg.startsWith(`--${dashCaseName}=`) || arg.startsWith(`--${configCaseName}=`)) {
-      value = getEqualsValue(arg);
-      matchingArg = arg;
+      // our argument was passed at the command-line in the format --argName=arg-value
+      [matchingArg, value] = parseEqualsArg(arg);
     } else if (arg === `--${dashCaseName}` || arg === `--${configCaseName}`) {
+      // the next value in the array is assumed to be a value for this argument
       value = args[i + 1];
       matchingArg = arg;
     } else if (alias) {
       if (arg.startsWith(`-${alias}=`)) {
-        value = getEqualsValue(arg);
-        matchingArg = arg;
+        [matchingArg, value] = parseEqualsArg(arg);
       } else if (arg === `-${alias}`) {
         value = args[i + 1];
         matchingArg = arg;
@@ -287,13 +292,43 @@ interface CLIArgValue {
 }
 
 /**
- * When a parameter is set in the format `--foobar=12` at the CLI (as opposed to
- * `--foobar 12`) we want to get the value after the `=` sign
+ * Parse an 'equals' argument, which is a CLI argument-value pair in the
+ * format `--foobar=12` (as opposed to a space-separated format like
+ * `--foobar 12`).
+ *
+ * To parse this we split on the `=`, returning the first part as the argument
+ * name and the second part as the value. We join the value on `"="` in case
+ * there is another `"="` in the argument.
+ *
+ * This function is safe to call with any arg, and can therefore be used as
+ * an argument 'normalizer'. If CLI argument is not an 'equals' argument then
+ * the return value will be a tuple of the original argument and an empty
+ * string `""` for the value.
+ *
+ * In code terms, if you do:
  *
- * @param commandArgument the arg in question
- * @returns the value after the `=`
+ * ```ts
+ * const [arg, value] = parseEqualsArg("--myArgument")
+ * ```
+ *
+ * Then `arg` will be `"--myArgument"` and `value` will be `""`, whereas if
+ * you do:
+ *
+ *
+ * ```ts
+ * const [arg, value] = parseEqualsArg("--myArgument=myValue")
+ * ```
+ *
+ * Then `arg` will be `"--myArgument"` and `value` will be `"myValue"`.
+ *
+ * @param arg the arg in question
+ * @returns a tuple containing the arg name and the value (if present)
  */
-const getEqualsValue = (commandArgument: string) => commandArgument.split('=').slice(1).join('=');
+export const parseEqualsArg = (arg: string): [string, string] => {
+  const [originalArg, ...value] = arg.split('=');
+
+  return [originalArg, value.join('=')];
+};
 
 /**
  * Small helper for getting type-system-level assurance that a `string` can be

src/cli/test/parse-flags.spec.ts

@@ -1,7 +1,7 @@
 import type * as d from '../../declarations';
 import { LogLevel } from '../../declarations';
 import { BOOLEAN_CLI_ARGS, STRING_CLI_ARGS, NUMBER_CLI_ARGS } from '../config-flags';
-import { parseFlags } from '../parse-flags';
+import { parseEqualsArg, parseFlags } from '../parse-flags';
 
 describe('parseFlags', () => {
   let args: string[] = [];
@@ -190,19 +190,29 @@ describe('parseFlags', () => {
   it.each(STRING_CLI_ARGS)('should parse string flag %s', (cliArg) => {
     const flags = parseFlags([`--${cliArg}`, 'test-value'], sys);
     expect(flags.knownArgs).toEqual([`--${cliArg}`, 'test-value']);
+    expect(flags.unknownArgs).toEqual([]);
     expect(flags[cliArg]).toBe('test-value');
   });
 
+  it.each(STRING_CLI_ARGS)('should parse string flag --%s=value', (cliArg) => {
+    const flags = parseFlags([`--${cliArg}=path/to/file.js`], sys);
+    expect(flags.knownArgs).toEqual([`--${cliArg}`, 'path/to/file.js']);
+    expect(flags.unknownArgs).toEqual([]);
+    expect(flags[cliArg]).toBe('path/to/file.js');
+  });
+
   it.each(NUMBER_CLI_ARGS)('should parse number flag %s', (cliArg) => {
     const flags = parseFlags([`--${cliArg}`, '42'], sys);
     expect(flags.knownArgs).toEqual([`--${cliArg}`, '42']);
+    expect(flags.unknownArgs).toEqual([]);
     expect(flags[cliArg]).toBe(42);
   });
 
   it('should parse --compare', () => {
     args[0] = '--compare';
     const flags = parseFlags(args, sys);
     expect(flags.knownArgs).toEqual(['--compare']);
+    expect(flags.unknownArgs).toEqual([]);
     expect(flags.compare).toBe(true);
   });
 
@@ -574,4 +584,18 @@ describe('parseFlags', () => {
     expect(flags.help).toBe(true);
     expect(flags.config).toBe('./myconfig.json');
   });
+
+  describe('parseEqualsArg', () => {
+    it.each([
+      ['--fooBar=baz', '--fooBar', 'baz'],
+      ['--foo-bar=4', '--foo-bar', '4'],
+      ['--fooBar=twenty=3*4', '--fooBar', 'twenty=3*4'],
+      ['--fooBar', '--fooBar', ''],
+      ['--foo-bar', '--foo-bar', ''],
+    ])('should parse %s correctly', (testArg, expectedArg, expectedValue) => {
+      const [arg, value] = parseEqualsArg(testArg);
+      expect(arg).toBe(expectedArg);
+      expect(value).toBe(expectedValue);
+    });
+  });
 });

src/testing/jest/test/jest-config.spec.ts

@@ -19,6 +19,17 @@ describe('jest-config', () => {
     expect(jestArgv.maxWorkers).toBe(2);
   });
 
+  it('marks outputFile as a Jest argument', () => {
+    const args = ['test', '--ci', '--outputFile=path/to/my-file'];
+    const config = mockValidatedConfig();
+    config.testing = {};
+    config.flags = parseFlags(args, config.sys);
+    expect(config.flags.args).toEqual(['--ci', '--outputFile=path/to/my-file']);
+    expect(config.flags.unknownArgs).toEqual([]);
+    const jestArgv = buildJestArgv(config);
+    expect(jestArgv.outputFile).toBe('path/to/my-file');
+  });
+
   it('pass --maxWorkers=2 arg when e2e test and --ci', () => {
     const args = ['test', '--ci', '--e2e', '--max-workers=2'];
     const config = mockValidatedConfig();