fix(cli): audit & normalize allowed transformers for each command

- rename mis-named `compatibleTransforms` to `compatibleTransformers` across API - transformer-specific options calculated from allowed transformers - added a missing type in `cli/src/console-utils.js`
IBM · Feb 5, 2020 · a1f106a · a1f106a
1 parent e804be0
commit a1f106a
Show file tree

Hide file tree

Showing 10 changed files with 101 additions and 67 deletions.
diff --git a/packages/cli/src/commands/common.js b/packages/cli/src/commands/common.js
@@ -13,7 +13,7 @@ import {terminalColumns} from '../console-utils.js';
 const debug = createDebugPipe('cli', 'commands', 'common');
 
 const {
-  compatibleTransforms,
+  compatibleTransformers,
   builtinTransformerIds,
   toReportFromObject
 } = observableAPI;
@@ -75,17 +75,6 @@ export const OPTIONS = {
       description: 'Live dangerously & do not automatically redact secrets',
       group: GROUPS.OUTPUT,
       type: 'boolean'
-    },
-    transform: {
-      // @todo list transform aliases
-      alias: 't',
-      choices: builtinTransformerIds,
-      coerce: toUniqueArray,
-      default: constants.DEFAULT_TRANSFORMER,
-      description: 'Transform(s) to apply',
-      group: GROUPS.OUTPUT,
-      nargs: 1,
-      type: 'string'
     }
   },
   TABLE_TRANSFORM: {
@@ -102,26 +91,66 @@ export const OPTIONS = {
       group: GROUPS.TABLE_TRANSFORM,
       type: 'boolean'
     }
+  },
+  TRANSFORM: {
+    // @todo list transform aliases
+    alias: 't',
+    choices: builtinTransformerIds,
+    coerce: toUniqueArray,
+    default: constants.DEFAULT_TRANSFORMER,
+    description: 'Transform(s) to apply',
+    group: GROUPS.OUTPUT,
+    nargs: 1,
+    type: 'string'
   }
 };
 
-export const getOptions = (
-  group,
-  {
-    sourceType = 'report',
-    defaultTransformer = constants.DEFAULT_TRANSFORMER
-  } = {}
-) =>
-  group.transform
-    ? {
-        ...group,
+/**
+ * Yes, yes, I know.
+ * @typedef {Object} GetTransformerOptionsOptions
+ * @property {'report'|'object'} sourceType - Whether the transformer source is a report or an object
+ * @property {string} defaultTransformer - Name of default transformer
+ * @property {object} extra - Merge these into the result; use this to override specific values (e.g., default behavior of a transformer)
+ * @property {string[]} omit - List of transformers to explicitly omit, if any
+ */
+
+/**
+ * Get all transformer options for a command
+ * @param {Partial<GetTransformerOptionsOptions>} opts - Options
+ */
+export const getTransformerOptions = ({
+  sourceType = 'report',
+  defaultTransformer = constants.DEFAULT_TRANSFORMER,
+  omit = [],
+  extra = {}
+} = {}) => {
+  const transformerNames = _.filter(
+    transformerName => !omit.includes(transformerName),
+    compatibleTransformers(sourceType)
+  );
+
+  return _.defaultsDeep(
+    _.reduce(
+      (acc, transformName) => {
+        const transformSpecificOptions =
+          OPTIONS[`${transformName.toUpperCase()}_TRANSFORM`];
+        if (transformSpecificOptions) {
+          acc = {...acc, ...transformSpecificOptions};
+        }
+        return acc;
+      },
+      {
         transform: {
-          ...group.transform,
-          choices: compatibleTransforms(sourceType),
+          ...OPTIONS.TRANSFORM,
+          choices: transformerNames,
           default: defaultTransformer
         }
-      }
-    : group;
+      },
+      transformerNames
+    ),
+    extra
+  );
+};
 
 /**
  *

diff --git a/packages/cli/src/commands/diff.js b/packages/cli/src/commands/diff.js
@@ -4,7 +4,7 @@ import {observable as observableAPI} from '@report-toolkit/core';
 import {terminalColumns, toOutput} from '../console-utils.js';
 import {
   fromFilepathsToReports,
-  getOptions,
+  getTransformerOptions,
   GROUPS,
   mergeCommandConfig,
   OPTIONS
@@ -64,9 +64,8 @@ export const builder = yargs =>
       type: 'boolean',
       conflicts: ['i', 'x']
     },
-    ...getOptions(OPTIONS.OUTPUT, {sourceType: 'object'}),
-    ...OPTIONS.JSON_TRANSFORM,
-    ...OPTIONS.TABLE_TRANSFORM
+    ...OPTIONS.OUTPUT,
+    ...getTransformerOptions({sourceType: 'object'})
   });
 
 /**

diff --git a/packages/cli/src/commands/inspect.js b/packages/cli/src/commands/inspect.js
@@ -6,7 +6,8 @@ import {
   fromFilepathsToReports,
   GROUPS,
   mergeCommandConfig,
-  OPTIONS
+  OPTIONS,
+  getTransformerOptions
 } from './common.js';
 
 const {ERROR, WARNING, INFO, DEFAULT_SEVERITY} = constants;
@@ -50,7 +51,7 @@ const SEVERITY_COLOR_MAP = _.toFrozenMap({
 
 export const command = 'inspect <file..>';
 
-export const desc = 'Inspect diagnostic report JSON against rules';
+export const desc = 'Inspect Diagnostic Report file(s) for problems';
 
 export const builder = yargs =>
   yargs
@@ -60,15 +61,13 @@ export const builder = yargs =>
     })
     .options({
       severity: {
-        choices: [INFO, WARNING, ERROR],
+        choices: [ERROR, WARNING, INFO],
         default: DEFAULT_SEVERITY,
         description: 'Minimum threshold for message severity',
         group: GROUPS.FILTER
       },
       ...OPTIONS.OUTPUT,
-      ...OPTIONS.TABLE_TRANSFORM,
-      ...OPTIONS.JSON_TRANSFORM,
-      ...OPTIONS.FILTER_TRANSFORM
+      ...getTransformerOptions({sourceType: 'object'})
     });
 
 export const handler = argv => {

diff --git a/packages/cli/src/commands/list-rules.js b/packages/cli/src/commands/list-rules.js
@@ -2,7 +2,7 @@ import {_, observable} from '@report-toolkit/common';
 import {observable as observableAPI} from '@report-toolkit/core';
 
 import {toOutput} from '../console-utils.js';
-import {getOptions, mergeCommandConfig, OPTIONS} from './common.js';
+import {getTransformerOptions, mergeCommandConfig, OPTIONS} from './common.js';
 
 const {map, share} = observable;
 const {
@@ -35,9 +35,8 @@ export const desc = 'Lists built-in rules';
  */
 export const builder = yargs =>
   yargs.options({
-    ...getOptions(OPTIONS.OUTPUT, {sourceType: 'object'}),
-    ...OPTIONS.JSON_TRANSFORM,
-    ...OPTIONS.TABLE_TRANSFORM
+    ..._.omit(['show-secrets-unsafe'], OPTIONS.OUTPUT),
+    ...getTransformerOptions({sourceType: 'object'})
   });
 
 /**

diff --git a/packages/cli/src/commands/redact.js b/packages/cli/src/commands/redact.js
@@ -6,7 +6,8 @@ import {
   fromFilepathsToReports,
   GROUPS,
   mergeCommandConfig,
-  OPTIONS
+  OPTIONS,
+  getTransformerOptions
 } from './common.js';
 const {transform, fromTransformerChain} = api;
 const {of, iif, concatMap} = observable;
@@ -23,23 +24,22 @@ export const builder = yargs =>
       coerce: _.castArray
     })
     .options({
-      ..._.omit(['output', 'show-secrets-unsafe'], OPTIONS.OUTPUT),
-      transform: {
-        ...OPTIONS.OUTPUT.transform,
-        default: DEFAULT_TRANSFORMER
-      },
-      ...OPTIONS.TABLE_TRANSFORM,
-      ...OPTIONS.JSON_TRANSFORM,
-      pretty: {
-        ...OPTIONS.JSON_TRANSFORM.pretty,
-        default: true
-      },
-      ...OPTIONS.FILTER_TRANSFORM,
       replace: {
         description: 'Replace file(s) in-place',
         type: 'boolean',
         group: GROUPS.OUTPUT
-      }
+      },
+      ..._.omit(['output', 'show-secrets-unsafe'], OPTIONS.OUTPUT),
+      ...getTransformerOptions({
+        sourceType: 'report',
+        defaultTransformer: DEFAULT_TRANSFORMER,
+        omit: ['redact'],
+        extra: {
+          pretty: {
+            default: true
+          }
+        }
+      })
     });
 
 export const handler = argv => {

diff --git a/packages/cli/src/commands/transform.js b/packages/cli/src/commands/transform.js
@@ -2,7 +2,12 @@ import {_, createDebugger} from '@report-toolkit/common';
 import {observable} from '@report-toolkit/core';
 
 import {toOutput} from '../console-utils.js';
-import {fromFilepathsToReports, mergeCommandConfig, OPTIONS} from './common.js';
+import {
+  fromFilepathsToReports,
+  mergeCommandConfig,
+  OPTIONS,
+  getTransformerOptions
+} from './common.js';
 
 const {transform, fromTransformerChain} = observable;
 
@@ -23,7 +28,6 @@ export const command = 'transform <file..>';
 
 export const desc = 'Transform a report';
 
-// TODO: getOptions() should probably handle all of this merging
 export const builder = yargs =>
   yargs
     .positional('file', {
@@ -33,13 +37,10 @@ export const builder = yargs =>
     })
     .options({
       ...OPTIONS.OUTPUT,
-      transform: {
-        ...OPTIONS.OUTPUT.transform,
-        default: DEFAULT_TRANSFORMER
-      },
-      ...OPTIONS.JSON_TRANSFORM,
-      ...OPTIONS.FILTER_TRANSFORM,
-      ...OPTIONS.TABLE_TRANSFORM
+      ...getTransformerOptions({
+        sourceType: 'report',
+        defaultTransformer: DEFAULT_TRANSFORMER
+      })
     });
 
 export const handler = argv => {

diff --git a/packages/cli/src/console-utils.js b/packages/cli/src/console-utils.js
@@ -20,6 +20,7 @@ export const fail = text => colors.red(error) + ' ' + colors.red().bold(text);
  * @param {string} [filepath] - If present, will write to file
  * @param {Object} [opts]
  * @param {boolean} [opts.color=true]
+ * @returns {import('rxjs').OperatorFunction<any,string>}
  */
 export const toOutput = (filepath, {color = true} = {}) => observable =>
   observable.pipe(

diff --git a/packages/core/src/index.js b/packages/core/src/index.js
@@ -6,7 +6,7 @@ const {fromAny, of, toArray} = obs;
 
 export {
   registeredRuleDefinitions,
-  compatibleTransforms,
+  compatibleTransformers,
   builtinTransformerIds,
   isPluginRegistered,
   deregisterPlugins

diff --git a/packages/core/src/observable.js b/packages/core/src/observable.js
@@ -11,7 +11,7 @@ import {diff as diffReports} from '@report-toolkit/diff';
 import * as inspector from '@report-toolkit/inspector';
 import {
   builtinTransformerIds,
-  compatibleTransforms,
+  compatibleTransformers,
   runTransformer,
   toTransformer,
   validateTransformerChain
@@ -355,7 +355,7 @@ export function transform(source, opts = {}) {
     );
 }
 
-export {compatibleTransforms, builtinTransformerIds};
+export {compatibleTransformers, builtinTransformerIds};
 
 /**
  * Creates a target `Observable` of {@link @report-toolkit/common.report.Report|Report} objects from a source `Observable` of plain objects (usually parsed from a JSON report).

diff --git a/packages/transformers/src/index.js b/packages/transformers/src/index.js
@@ -165,7 +165,13 @@ export const runTransformer = source => /**
     )
   );
 
-export const compatibleTransforms = sourceType =>
+/**
+ * Returns a list of transformers which can accept data of type `sourceType`
+ * @todo memoize
+ * @todo constants for source types
+ * @param {string} sourceType
+ */
+export const compatibleTransformers = sourceType =>
   _.keys(
     _.fromPairs(
       _.filter(