[dev/run] expose unexpected flags as more than just names (#54080)

Spencer · web-flow · commit 9eb0c77318c6 · 2020-01-07T11:56:08.000-07:00
diff --git a/packages/kbn-dev-utils/src/run/README.md b/packages/kbn-dev-utils/src/run/README.md
@@ -117,7 +117,7 @@ $ node scripts/my_task
 
     - *`flags.allowUnexpected: boolean`*
 
-      By default, any flag that is passed but not mentioned in `flags.string`, `flags.boolean`, `flags.alias` or `flags.default` will trigger an error, preventing the run function from calling its first argument. If you have a reason to disable this behavior set this option to `true`.
+      By default, any flag that is passed but not mentioned in `flags.string`, `flags.boolean`, `flags.alias` or `flags.default` will trigger an error, preventing the run function from calling its first argument. If you have a reason to disable this behavior set this option to `true`. Unexpected flags will be collected from argv into `flags.unexpected`. To parse these flags and guess at their types, you can additionally pass `flags.guessTypesForUnexpectedFlags` but that's not recommended.
 
 
 - ***`createFailError(reason: string, options: { exitCode: number, showHelp: boolean }): FailError`***
diff --git a/packages/kbn-dev-utils/src/run/flags.test.ts b/packages/kbn-dev-utils/src/run/flags.test.ts
@@ -0,0 +1,94 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { getFlags } from './flags';
+
+it('gets flags correctly', () => {
+  expect(
+    getFlags(['-a', '--abc=bcd', '--foo=bar', '--no-bar', '--foo=baz', '--box', 'yes', '-zxy'], {
+      flags: {
+        boolean: ['x'],
+        string: ['abc'],
+        alias: {
+          x: 'extra',
+        },
+        allowUnexpected: true,
+      },
+    })
+  ).toMatchInlineSnapshot(`
+    Object {
+      "_": Array [],
+      "abc": "bcd",
+      "debug": false,
+      "extra": true,
+      "help": false,
+      "quiet": false,
+      "silent": false,
+      "unexpected": Array [
+        "-a",
+        "--foo=bar",
+        "--foo=baz",
+        "--no-bar",
+        "--box",
+        "yes",
+        "-z",
+        "-y",
+      ],
+      "v": false,
+      "verbose": false,
+      "x": true,
+    }
+  `);
+});
+
+it('guesses types for unexpected flags', () => {
+  expect(
+    getFlags(['-abc', '--abc=bcd', '--no-foo', '--bar'], {
+      flags: {
+        allowUnexpected: true,
+        guessTypesForUnexpectedFlags: true,
+      },
+    })
+  ).toMatchInlineSnapshot(`
+    Object {
+      "_": Array [],
+      "a": true,
+      "abc": "bcd",
+      "b": true,
+      "bar": true,
+      "c": true,
+      "debug": false,
+      "foo": false,
+      "help": false,
+      "quiet": false,
+      "silent": false,
+      "unexpected": Array [
+        "-a",
+        "-b",
+        "-c",
+        "-abc",
+        "--abc=bcd",
+        "--no-foo",
+        "--bar",
+      ],
+      "v": false,
+      "verbose": false,
+    }
+  `);
+});
diff --git a/packages/kbn-dev-utils/src/run/flags.ts b/packages/kbn-dev-utils/src/run/flags.ts
@@ -37,7 +37,7 @@ export interface Flags {
 }
 
 export function getFlags(argv: string[], options: Options): Flags {
-  const unexpected: string[] = [];
+  const unexpectedNames = new Set<string>();
   const flagOpts = options.flags || {};
 
   const { verbose, quiet, silent, debug, help, _, ...others } = getopts(argv, {
@@ -49,15 +49,64 @@ export function getFlags(argv: string[], options: Options): Flags {
     },
     default: flagOpts.default,
     unknown: (name: string) => {
-      unexpected.push(name);
+      unexpectedNames.add(name);
+      return flagOpts.guessTypesForUnexpectedFlags;
+    },
+  } as any);
+
+  const unexpected: string[] = [];
+  for (const unexpectedName of unexpectedNames) {
+    const matchingArgv: string[] = [];
+
+    iterArgv: for (const [i, v] of argv.entries()) {
+      for (const prefix of ['--', '-']) {
+        if (v.startsWith(prefix)) {
+          // -/--name=value
+          if (v.startsWith(`${prefix}${unexpectedName}=`)) {
+            matchingArgv.push(v);
+            continue iterArgv;
+          }
+
+          // -/--name (value possibly follows)
+          if (v === `${prefix}${unexpectedName}`) {
+            matchingArgv.push(v);
 
-      if (options.flags && options.flags.allowUnexpected) {
-        return true;
+            // value follows -/--name
+            if (argv.length > i + 1 && !argv[i + 1].startsWith('-')) {
+              matchingArgv.push(argv[i + 1]);
+            }
+
+            continue iterArgv;
+          }
+        }
       }
 
-      return false;
-    },
-  } as any);
+      // special case for `--no-{flag}` disabling of boolean flags
+      if (v === `--no-${unexpectedName}`) {
+        matchingArgv.push(v);
+        continue iterArgv;
+      }
+
+      // special case for shortcut flags formatted as `-abc` where `a`, `b`,
+      // and `c` will be three separate unexpected flags
+      if (
+        unexpectedName.length === 1 &&
+        v[0] === '-' &&
+        v[1] !== '-' &&
+        !v.includes('=') &&
+        v.includes(unexpectedName)
+      ) {
+        matchingArgv.push(`-${unexpectedName}`);
+        continue iterArgv;
+      }
+    }
+
+    if (matchingArgv.length) {
+      unexpected.push(...matchingArgv);
+    } else {
+      throw new Error(`unable to find unexpected flag named "${unexpectedName}"`);
+    }
+  }
 
   return {
     verbose,
@@ -75,7 +124,7 @@ export function getHelp(options: Options) {
   const usage = options.usage || `node ${relative(process.cwd(), process.argv[1])}`;
 
   const optionHelp = (
-    dedent((options.flags && options.flags.help) || '') +
+    dedent(options?.flags?.help || '') +
     '\n' +
     dedent`
       --verbose, -v      Log verbosely
diff --git a/packages/kbn-dev-utils/src/run/run.ts b/packages/kbn-dev-utils/src/run/run.ts
@@ -36,6 +36,7 @@ export interface Options {
   description?: string;
   flags?: {
     allowUnexpected?: boolean;
+    guessTypesForUnexpectedFlags?: boolean;
     help?: string;
     alias?: { [key: string]: string | string[] };
     boolean?: string[];
@@ -46,7 +47,6 @@ export interface Options {
 
 export async function run(fn: RunFn, options: Options = {}) {
   const flags = getFlags(process.argv.slice(2), options);
-  const allowUnexpected = options.flags ? options.flags.allowUnexpected : false;
 
   if (flags.help) {
     process.stderr.write(getHelp(options));
@@ -97,7 +97,7 @@ export async function run(fn: RunFn, options: Options = {}) {
   const cleanupTasks: CleanupTask[] = [unhookExit];
 
   try {
-    if (!allowUnexpected && flags.unexpected.length) {
+    if (!options.flags?.allowUnexpected && flags.unexpected.length) {
       throw createFlagError(`Unknown flag(s) "${flags.unexpected.join('", "')}"`);
     }
 
diff --git a/packages/kbn-dev-utils/tsconfig.json b/packages/kbn-dev-utils/tsconfig.json
@@ -3,7 +3,8 @@
   "compilerOptions": {
     "outDir": "target",
     "target": "ES2019",
-    "declaration": true
+    "declaration": true,
+    "declarationMap": true
   },
   "include": [
     "src/**/*"
diff --git a/packages/kbn-pm/dist/index.js b/packages/kbn-pm/dist/index.js
@@ -37054,8 +37054,8 @@ const tooling_log_1 = __webpack_require__(415);
 const fail_1 = __webpack_require__(425);
 const flags_1 = __webpack_require__(426);
 async function run(fn, options = {}) {
+    var _a;
     const flags = flags_1.getFlags(process.argv.slice(2), options);
-    const allowUnexpected = options.flags ? options.flags.allowUnexpected : false;
     if (flags.help) {
         process.stderr.write(flags_1.getHelp(options));
         process.exit(1);
@@ -37098,7 +37098,7 @@ async function run(fn, options = {}) {
     const unhookExit = exit_hook_1.default(doCleanup);
     const cleanupTasks = [unhookExit];
     try {
-        if (!allowUnexpected && flags.unexpected.length) {
+        if (!((_a = options.flags) === null || _a === void 0 ? void 0 : _a.allowUnexpected) && flags.unexpected.length) {
             throw fail_1.createFlagError(`Unknown flag(s) "${flags.unexpected.join('", "')}"`);
         }
         try {
@@ -37218,7 +37218,7 @@ const path_1 = __webpack_require__(16);
 const dedent_1 = tslib_1.__importDefault(__webpack_require__(14));
 const getopts_1 = tslib_1.__importDefault(__webpack_require__(427));
 function getFlags(argv, options) {
-    const unexpected = [];
+    const unexpectedNames = new Set();
     const flagOpts = options.flags || {};
     const { verbose, quiet, silent, debug, help, _, ...others } = getopts_1.default(argv, {
         string: flagOpts.string,
@@ -37229,13 +37229,55 @@ function getFlags(argv, options) {
         },
         default: flagOpts.default,
         unknown: (name) => {
-            unexpected.push(name);
-            if (options.flags && options.flags.allowUnexpected) {
-                return true;
-            }
-            return false;
+            unexpectedNames.add(name);
+            return flagOpts.guessTypesForUnexpectedFlags;
         },
     });
+    const unexpected = [];
+    for (const unexpectedName of unexpectedNames) {
+        const matchingArgv = [];
+        iterArgv: for (const [i, v] of argv.entries()) {
+            for (const prefix of ['--', '-']) {
+                if (v.startsWith(prefix)) {
+                    // -/--name=value
+                    if (v.startsWith(`${prefix}${unexpectedName}=`)) {
+                        matchingArgv.push(v);
+                        continue iterArgv;
+                    }
+                    // -/--name (value possibly follows)
+                    if (v === `${prefix}${unexpectedName}`) {
+                        matchingArgv.push(v);
+                        // value follows -/--name
+                        if (argv.length > i + 1 && !argv[i + 1].startsWith('-')) {
+                            matchingArgv.push(argv[i + 1]);
+                        }
+                        continue iterArgv;
+                    }
+                }
+            }
+            // special case for `--no-{flag}` disabling of boolean flags
+            if (v === `--no-${unexpectedName}`) {
+                matchingArgv.push(v);
+                continue iterArgv;
+            }
+            // special case for shortcut flags formatted as `-abc` where `a`, `b`,
+            // and `c` will be three separate unexpected flags
+            if (unexpectedName.length === 1 &&
+                v[0] === '-' &&
+                v[1] !== '-' &&
+                !v.includes('=') &&
+                v.includes(unexpectedName)) {
+                matchingArgv.push(`-${unexpectedName}`);
+                continue iterArgv;
+            }
+        }
+        if (matchingArgv.length) {
+            unexpected.push(...matchingArgv);
+        }
+        else {
+            throw new Error(`unable to find unexpected flag named "${unexpectedName}"`);
+        }
+    }
     return {
         verbose,
         quiet,
@@ -37249,8 +37291,9 @@ function getFlags(argv, options) {
 }
 exports.getFlags = getFlags;
 function getHelp(options) {
+    var _a, _b;
     const usage = options.usage || `node ${path_1.relative(process.cwd(), process.argv[1])}`;
-    const optionHelp = (dedent_1.default((options.flags && options.flags.help) || '') +
+    const optionHelp = (dedent_1.default(((_b = (_a = options) === null || _a === void 0 ? void 0 : _a.flags) === null || _b === void 0 ? void 0 : _b.help) || '') +
         '\n' +
         dedent_1.default `
       --verbose, -v      Log verbosely
diff --git a/src/dev/run_i18n_check.ts b/src/dev/run_i18n_check.ts
@@ -133,6 +133,7 @@ run(
   {
     flags: {
       allowUnexpected: true,
+      guessTypesForUnexpectedFlags: true,
     },
   }
 );
diff --git a/src/dev/run_i18n_extract.ts b/src/dev/run_i18n_extract.ts
@@ -93,6 +93,7 @@ run(
   {
     flags: {
       allowUnexpected: true,
+      guessTypesForUnexpectedFlags: true,
     },
   }
 );
diff --git a/src/dev/run_i18n_integrate.ts b/src/dev/run_i18n_integrate.ts
@@ -121,6 +121,7 @@ run(
   {
     flags: {
       allowUnexpected: true,
+      guessTypesForUnexpectedFlags: true,
     },
   }
 );

Original file line number	Diff line number	Diff line change
`@@ -133,6 +133,7 @@ run(`
`133`	`133`	`{`
`134`	`134`	`flags: {`
`135`	`135`	`allowUnexpected: true,`
	`136`	`+ guessTypesForUnexpectedFlags: true,`
`136`	`137`	`},`
`137`	`138`	`}`
`138`	`139`	`);`
Original file line number	Diff line number	Diff line change
`@@ -93,6 +93,7 @@ run(`
`93`	`93`	`{`
`94`	`94`	`flags: {`
`95`	`95`	`allowUnexpected: true,`
	`96`	`+ guessTypesForUnexpectedFlags: true,`
`96`	`97`	`},`
`97`	`98`	`}`
`98`	`99`	`);`
Original file line number	Diff line number	Diff line change
`@@ -121,6 +121,7 @@ run(`
`121`	`121`	`{`
`122`	`122`	`flags: {`
`123`	`123`	`allowUnexpected: true,`
	`124`	`+ guessTypesForUnexpectedFlags: true,`
`124`	`125`	`},`
`125`	`126`	`}`
`126`	`127`	`);`