fix(cli): handle multiple --persist.outputDir arguments

Author: matejchalk

packages/cli/src/lib/implementation/core-config.int.test.ts

@@ -18,15 +18,15 @@ vi.mock('@code-pushup/core', async () => {
   return {
     ...(core as object),
     readRcByPath: vi.fn().mockImplementation((filepath: string): CoreConfig => {
-      const allPersistOptions = {
+      const allPersistOptions: CoreConfig = {
         ...CORE_CONFIG_MOCK,
         persist: {
           filename: 'rc-filename',
           format: ['json', 'md'],
           outputDir: 'rc-outputDir',
         },
       };
-      const persistOnlyFilename = {
+      const persistOnlyFilename: CoreConfig = {
         ...CORE_CONFIG_MOCK,
         persist: {
           filename: 'rc-filename',

packages/cli/src/lib/yargs-cli.int.test.ts

@@ -103,6 +103,14 @@ describe('yargsCli', () => {
     expect(parsedArgv.config).toBe('./config.b.ts');
   });
 
+  it('should use the last occurrence of an argument if persist.outputDir is passed multiple times', async () => {
+    const parsedArgv = await yargsCli<Pick<CoreConfig, 'persist'>>(
+      ['--persist.outputDir=output-a', '--persist.outputDir=output-b'],
+      { options },
+    ).parseAsync();
+    expect(parsedArgv.persist!.outputDir).toBe('output-b');
+  });
+
   it('should ignore unknown options', async () => {
     const parsedArgv = await yargsCli<GlobalOptions>(
       ['--no-progress', '--verbose'],

packages/cli/src/lib/yargs-cli.ts

@@ -13,7 +13,7 @@ import {
   formatSchema,
   validate,
 } from '@code-pushup/models';
-import { TERMINAL_WIDTH } from '@code-pushup/utils';
+import { TERMINAL_WIDTH, isRecord } from '@code-pushup/utils';
 import {
   descriptionStyle,
   formatNestedValues,
@@ -88,11 +88,11 @@ export function yargsCli<T = unknown>(
     .parserConfiguration({
       'strip-dashed': true,
     } satisfies Partial<ParserConfigurationOptions>)
-    .coerce('config', (config: string | string[]) =>
-      Array.isArray(config) ? config.at(-1) : config,
-    )
     .options(formatNestedValues(options, 'describe'));
 
+  // use last argument for non-array options
+  coerceArraysByOptionType(cli, options);
+
   // usage message
   if (usageMessage) {
     cli.usage(titleStyle(usageMessage));
@@ -166,3 +166,62 @@ function validatePersistFormat(persist: PersistConfig) {
     );
   }
 }
+
+function coerceArraysByOptionType(
+  cli: Argv,
+  options: Record<string, Options>,
+): void {
+  Object.entries(groupOptionsByKey(options)).forEach(([key, node]) => {
+    cli.coerce(key, (value: unknown) => coerceNode(node, value));
+  });
+}
+
+function coerceNode(
+  node: OptionsTreeNode | OptionsTreeLeaf,
+  value: unknown,
+): unknown {
+  if (node.isLeaf) {
+    if (node.options.type === 'array') {
+      return node.options.coerce?.(value) ?? value;
+    }
+    return Array.isArray(value) ? value.at(-1) : value;
+  }
+  return Object.entries(node.children).reduce(coerceChildNode, value);
+}
+
+function coerceChildNode(
+  value: unknown,
+  [key, node]: [string, OptionsTreeNode | OptionsTreeLeaf],
+): unknown {
+  if (!isRecord(value) || !(key in value)) {
+    return value;
+  }
+  return { ...value, [key]: coerceNode(node, value[key]) };
+}
+
+type OptionsTree = Record<string, OptionsTreeNode | OptionsTreeLeaf>;
+type OptionsTreeNode = { isLeaf: false; children: OptionsTree };
+type OptionsTreeLeaf = { isLeaf: true; options: Options };
+
+function groupOptionsByKey(options: Record<string, Options>): OptionsTree {
+  return Object.entries(options).reduce<OptionsTree>(addOptionToTree, {});
+}
+
+function addOptionToTree(
+  tree: OptionsTree,
+  [key, value]: [string, Options],
+): OptionsTree {
+  if (!key.includes('.')) {
+    return { ...tree, [key]: { isLeaf: true, options: value } };
+  }
+  const [parentKey, childKey] = key.split('.', 2) as [string, string];
+  const prevChildren =
+    tree[parentKey] && !tree[parentKey].isLeaf ? tree[parentKey].children : {};
+  return {
+    ...tree,
+    [parentKey]: {
+      isLeaf: false,
+      children: addOptionToTree(prevChildren, [childKey, value]),
+    },
+  };
+}

packages/utils/src/index.ts

@@ -84,6 +84,7 @@ export {
   hasNoNullableProps,
   isPromiseFulfilledResult,
   isPromiseRejectedResult,
+  isRecord,
 } from './lib/guards.js';
 export { interpolate } from './lib/interpolate.js';
 export { logMultipleResults } from './lib/log-results.js';

packages/utils/src/lib/guards.ts

@@ -17,3 +17,7 @@ export function hasNoNullableProps<T extends object>(
 ): obj is ExcludeNullableProps<T> {
   return Object.values(obj).every(value => value != null);
 }
+
+export function isRecord(value: unknown): value is Record<string, unknown> {
+  return typeof value === 'object' && value != null;
+}

packages/utils/src/lib/guards.unit.test.ts

@@ -3,6 +3,7 @@ import {
   hasNoNullableProps,
   isPromiseFulfilledResult,
   isPromiseRejectedResult,
+  isRecord,
 } from './guards.js';
 
 describe('promise-result', () => {
@@ -42,3 +43,29 @@ describe('hasNoNullableProps', () => {
     expect(hasNoNullableProps({})).toBeTrue();
   });
 });
+
+describe('isRecord', () => {
+  it('should return true for an object', () => {
+    expect(isRecord({ foo: 'bar' })).toBeTrue();
+  });
+
+  it('should return true for an empty object', () => {
+    expect(isRecord({})).toBeTrue();
+  });
+
+  it('should return true for an array', () => {
+    expect(isRecord([1, 2, 3])).toBeTrue();
+  });
+
+  it('should return false for a string', () => {
+    expect(isRecord('foo')).toBeFalse();
+  });
+
+  it('should return false for null', () => {
+    expect(isRecord(null)).toBeFalse();
+  });
+
+  it('should return false for undefined', () => {
+    expect(isRecord(undefined)).toBeFalse();
+  });
+});