From 5fefe0c79d2ac612acb59688290c7911177cc14f Mon Sep 17 00:00:00 2001
From: ssube <seansube@gmail.com>
Date: Sat, 2 Nov 2019 14:30:37 -0500
Subject: [PATCH] fix(config): group yargs to fix result TS interface

---
 src/config/args.ts | 141 +++++++++++++++++++++++----------------------
 src/index.ts       |   4 +-
 src/utils/index.ts |   3 +-
 3 files changed, 75 insertions(+), 73 deletions(-)

diff --git a/src/config/args.ts b/src/config/args.ts
index bd16387..4131f18 100644
--- a/src/config/args.ts
+++ b/src/config/args.ts
@@ -36,7 +36,6 @@ export interface ParsedArgs extends RuleSelector, RuleSources {
   defaults: boolean;
   dest: string;
   mode: string;
-  rules: Array<string>;
   source: string;
 }
 
@@ -47,10 +46,8 @@ export interface ParseResults {
 
 /**
  * Wrap yargs to exit after completion.
- *
- * @TODO: fix it to use argv, not sure if yargs can do that
  */
-export function parseArgs(argv: Array<string>): ParseResults {
+export async function parseArgs(argv: Array<string>): Promise<ParseResults> {
   let mode: MODE = MODE.check;
 
   const parser = usage(`Usage: salty-dog <mode> [options]`)
@@ -64,13 +61,15 @@ export function parseArgs(argv: Array<string>): ParseResults {
     .command({
       builder: (yargs: any) => {
         return yargs
-          .option('coerce', {
-            default: false,
-            type: 'boolean',
-          })
-          .option('defaults', {
-            default: true,
-            type: 'boolean',
+          .options({
+            coerce: {
+              default: false,
+              type: 'boolean',
+            },
+            defaults: {
+              default: true,
+              type: 'boolean',
+            },
           });
       },
       command: ['fix'],
@@ -93,70 +92,72 @@ export function parseArgs(argv: Array<string>): ParseResults {
         mode = MODE.complete;
       },
     })
-    .option(CONFIG_ARGS_NAME, {
-      default: `.${VERSION_INFO.package.name}.yml`,
-      group: 'Config:',
-      type: 'string',
-    })
-    .option(CONFIG_ARGS_PATH, {
-      default: [],
-      group: 'Config:',
-      type: 'array',
-    })
-    .option('count', {
-      alias: ['c'],
-      default: false,
-      desc: 'Exit with error count',
-      type: 'boolean',
-    })
-    .option('dest', {
-      alias: ['d'],
-      default: '-',
-      type: 'string',
-    })
-    .option('format', {
-      alias: ['f'],
-      default: 'yaml',
-      type: 'string',
-    })
-    .option('rule-file', {
-      alias: ['r', 'rule', 'rules'],
-      default: [],
-      desc: 'Rules file',
-      type: 'array',
-    })
-    .option('rule-module', {
-      alias: ['m'],
-      default: [],
-      desc: 'Rules module',
-      type: 'array',
-    })
-    .option('rule-path', {
-      alias: ['p'],
-      default: [],
-      desc: 'Rules path',
-      type: 'array',
-    })
-    .option('source', {
-      alias: ['s'],
-      default: '-',
-      type: 'string',
-    })
-    .option('exclude-level', RULE_OPTION)
-    .option('exclude-name', RULE_OPTION)
-    .option('exclude-tag', RULE_OPTION)
-    .option('include-level', RULE_OPTION)
-    .option('include-name', RULE_OPTION)
-    .option('include-tag', {
-      ...RULE_OPTION,
-      alias: ['t', 'tag'],
+    .options({
+      [CONFIG_ARGS_NAME]: {
+        default: `.${VERSION_INFO.package.name}.yml`,
+        group: 'Config:',
+        type: 'string',
+      },
+      [CONFIG_ARGS_PATH]: {
+        default: [],
+        group: 'Config:',
+        type: 'array',
+      },
+      'count': {
+        alias: ['c'],
+        default: false,
+        desc: 'Exit with error count',
+        type: 'boolean',
+      },
+      'dest': {
+        alias: ['d'],
+        default: '-',
+        type: 'string',
+      },
+      'exclude-level': RULE_OPTION,
+      'exclude-name': RULE_OPTION,
+      'exclude-tag': RULE_OPTION,
+      'format': {
+        alias: ['f'],
+        default: 'yaml',
+        type: 'string',
+      },
+      'include-level': RULE_OPTION,
+      'include-name': RULE_OPTION,
+      'include-tag': {
+        ...RULE_OPTION,
+        alias: ['t', 'tag'],
+      },
+      'rule-file': {
+        alias: ['r', 'rule', 'rules'],
+        default: [],
+        desc: 'Rules file',
+        type: 'array',
+      },
+      'rule-module': {
+        alias: ['m'],
+        default: [],
+        desc: 'Rules module',
+        type: 'array',
+      },
+      'rule-path': {
+        alias: ['p'],
+        default: [],
+        desc: 'Rules path',
+        type: 'array',
+      },
+      'source': {
+        alias: ['s'],
+        default: '-',
+        type: 'string',
+      },
     })
     .help()
     .version(VERSION_INFO.package.version)
     .alias('version', 'v');
 
-  // @TODO: this should not need a cast but argv's type only has the last option (include-tag)
-  // @tslint:disable-next-line:no-any
+  // @TODO: this should not need a cast but the parser's type only has the last option (include-tag)
+  // tslint:disable-next-line:no-any
   const args = parser.argv as any;
   return {
     args,
diff --git a/src/index.ts b/src/index.ts
index b9f43a6..2a7ea15 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -14,7 +14,7 @@ const STATUS_ERROR = 1;
 const STATUS_MAX = 255;
 
 export async function main(argv: Array<string>): Promise<number> {
-  const { args, mode } = parseArgs(argv);
+  const { args, mode } = await parseArgs(argv);
   if (mode === MODE.complete) {
     showCompletionScript();
     return STATUS_SUCCESS;
@@ -24,7 +24,7 @@ export async function main(argv: Array<string>): Promise<number> {
 
   const logger = createLogger(config.data.logger);
   logger.info(VERSION_INFO, 'version info');
-  logger.info({ args }, 'main arguments');
+  logger.info({ args, mode }, 'main arguments');
 
   // check mode
   if (!VALID_MODES.has(mode)) {
diff --git a/src/utils/index.ts b/src/utils/index.ts
index dcf0d31..4213c03 100644
--- a/src/utils/index.ts
+++ b/src/utils/index.ts
@@ -9,7 +9,8 @@ export function doesExist<T>(val: T | null | undefined): val is T {
  *
  * This is not a general replacement for `.length > 0`, since it is also a typeguard:
  * `if (hasItems(val)) else { val }` will complain that `val` is `never` in the `else`
- * branch, since it was proven not to be an array by this function.
+ * branch, since it was proven not to be an array by this function, even if `val` is
+ * simply empty.
  */
 export function hasItems<T>(val: Array<T> | null | undefined): val is Array<T>;
 export function hasItems<T>(val: ReadonlyArray<T> | null | undefined): val is ReadonlyArray<T>;