fix(command): fix allowEmpty and disable it by default (#352)

command/command.ts

@@ -101,7 +101,7 @@ export class Command<
   private argsDefinition?: string;
   private isExecutable = false;
   private throwOnError = false;
-  private _allowEmpty = true;
+  private _allowEmpty = false;
   private _stopEarly = false;
   private defaultCommand?: string;
   private _useRawArgs = false;

command/test/command/allow_empty_test.ts

@@ -1,41 +1,35 @@
 import { assertEquals, assertRejects } from "../../../dev_deps.ts";
 import { Command } from "../../command.ts";
 
-Deno.test("flags allowEmpty enabled", async () => {
-  const { options, args } = await new Command()
+function cmd() {
+  return new Command()
     .throwErrors()
-    .allowEmpty(true)
-    .option("-f, --flag [value:boolean]", "description ...")
-    .action(() => {})
-    .parse(["-f"]);
+    .option("-f, --flag [value:boolean]", "description ...", {
+      required: true,
+    });
+}
 
-  assertEquals(options, { flag: true });
-  assertEquals(args, []);
+Deno.test("[flags] should not allow empty by default", async () => {
+  await assertRejects(
+    () => cmd().parse([]),
+    Error,
+    'Missing required option "--flag".',
+  );
 });
 
-Deno.test("flags allowEmpty disabledNoFlags", async () => {
-  const { options, args } = await new Command()
-    .throwErrors()
+Deno.test("[flags] should not allow empty if disabled", async () => {
+  await assertRejects(
+    () => cmd().allowEmpty(false).parse([]),
+    Error,
+    'Missing required option "--flag".',
+  );
+});
+
+Deno.test("[flags] should allow empty if enabled", async () => {
+  const { options, args } = await cmd()
     .allowEmpty(true)
-    .action(() => {})
     .parse([]);
 
   assertEquals(options, {});
   assertEquals(args, []);
 });
-
-Deno.test("flags allowEmpty disabled", async () => {
-  const cmd = new Command()
-    .throwErrors()
-    .allowEmpty(false)
-    .option("-f, --flag [value:boolean]", "description ...")
-    .action(() => {});
-
-  await assertRejects(
-    async () => {
-      await cmd.parse([]);
-    },
-    Error,
-    "No arguments.",
-  );
-});

command/test/command/sub_command_test.ts

@@ -16,11 +16,13 @@ function command(states: States = {}) {
     .version(version)
     .description(description)
     .arguments("[command]")
+    // sub-command
     .command("sub-command <input:string> <output:string>")
     .option("-f, --flag [value:string]", "description ...", { required: true })
     .action(() => {
       states.action1 = true;
     })
+    // sub-command2
     .command(
       "sub-command2 <input:string> <output:string>",
       new Command()
@@ -29,6 +31,7 @@ function command(states: States = {}) {
         .action(() => {
           states.action2 = true;
         })
+        // sub-command3
         .command("sub-command3 <input:string> <output:string>")
         .option("-e, --eee [value:string]", "description ...")
         .action(() => {
@@ -42,10 +45,10 @@ Deno.test("command - sub command - sub-command with arguments", async () => {
   // deno-lint-ignore no-explicit-any
   const cmd: Command<any> = command(stats);
   const { options, args } = await cmd.parse(
-    ["sub-command", "input-path", "output-path"],
+    ["sub-command", "input-path", "output-path", "-f"],
   );
 
-  assertEquals(options, {});
+  assertEquals(options, { flag: true });
   assertEquals(args[0], "input-path");
   assertEquals(args[1], "output-path");
   assertEquals(stats.action1, true);
@@ -85,17 +88,27 @@ Deno.test("command - sub command - nested child command with arguments", async (
   assertEquals(stats.action3, true);
 });
 
-Deno.test("command - sub command - sub-command with missing argument", async () => {
+Deno.test("[command] sub command - sub-command with missing argument", async () => {
   await assertRejects(
     async () => {
-      await command().parse(["sub-command", "input-path"]);
+      await command().parse(["sub-command", "input-path", "-f"]);
     },
     Error,
     "Missing argument: output",
   );
 });
 
-Deno.test("command - sub command - sub-command 2 with missing argument", async () => {
+Deno.test("[command] sub command - sub-command with missing flag", async () => {
+  await assertRejects(
+    async () => {
+      await command().parse(["sub-command", "input-path"]);
+    },
+    Error,
+    'Missing required option "--flag".',
+  );
+});
+
+Deno.test("[command] sub command - sub-command 2 with missing argument", async () => {
   await assertRejects(
     async () => {
       await command().parse(["sub-command2", "input-path"]);
@@ -105,7 +118,7 @@ Deno.test("command - sub command - sub-command 2 with missing argument", async (
   );
 });
 
-Deno.test("command - sub command - nested sub-command with missing argument", async () => {
+Deno.test("[command] sub command - nested sub-command with missing argument", async () => {
   await assertRejects(
     async () => {
       await command().parse(["sub-command2", "sub-command3", "input-path"]);
@@ -115,7 +128,7 @@ Deno.test("command - sub command - nested sub-command with missing argument", as
   );
 });
 
-Deno.test("command - sub command - command with empty name", async () => {
+Deno.test("[command] sub command - command with empty name", async () => {
   await assertRejects(
     async () => {
       await new Command()
@@ -127,14 +140,14 @@ Deno.test("command - sub command - command with empty name", async () => {
   );
 });
 
-Deno.test("command - sub command - override child command", async () => {
+Deno.test("[command] sub command - override child command", async () => {
   await new Command()
     .command("foo")
     .command("foo", "...", true)
     .parse(["foo"]);
 });
 
-Deno.test("command - sub command - duplicate command name", async () => {
+Deno.test("[command] sub command - duplicate command name", async () => {
   await assertRejects(
     async () => {
       await new Command()
@@ -147,7 +160,7 @@ Deno.test("command - sub command - duplicate command name", async () => {
   );
 });
 
-Deno.test("command - sub command - select sub-command", async () => {
+Deno.test("[command] sub command - select sub-command", async () => {
   const cmd = new Command()
     .command("foo")
     .command("bar");

command/test/option/depends_test.ts

@@ -19,14 +19,12 @@ Deno.test("command depends option with default value: should accept no arguments
   assertEquals(args, []);
 });
 
-// disabled because the -h flag call's Deno.exit() and stops the test's.
-// Deno.test( 'command depends option with default value: should accept -h', async () => {
-//
-//     const { options, args } = await cmd().parse( [ '-h' ] );
-//
-//     assertEquals( options, { flag2: 'example' } );
-//     assertEquals( args, [] );
-// } );
+Deno.test("command depends option with default value: should accept -h", async () => {
+  const { options, args } = await command().noExit().parse(["-h"]);
+
+  assertEquals(options, { flag2: "example", help: true });
+  assertEquals(args, []);
+});
 
 Deno.test("command depends option with default value: should accept --flag1", async () => {
   const { options, args } = await command().parse(["--flag1"]);

command/test/option/requires_test.ts

@@ -20,13 +20,6 @@ const cmd = new Command()
   )
   .action(() => {});
 
-Deno.test("command optionRequire noArguments", async () => {
-  const { options, args } = await cmd.parse([]);
-
-  assertEquals(options, {});
-  assertEquals(args, []);
-});
-
 Deno.test("command optionRequire videoAudioImageType", async () => {
   const { options, args } = await cmd.parse(
     ["-v", "value", "-a", "value", "--image-type", "value"],

flags/flags.ts

@@ -77,7 +77,8 @@ export function parseFlags<
   let negate = false;
 
   const flags: Record<string, unknown> = {};
-  const optionNames: Record<string, string> = {};
+  /** Option name mapping: propertyName -> option.name */
+  const optionNameMap: Record<string, string> = {};
   let literal: string[] = [];
   let unknown: string[] = [];
   let stopEarly: string | null = null;
@@ -209,7 +210,7 @@ export function parseFlags<
         flags[propName] = value;
       }
 
-      optionNames[propName] = option.name;
+      optionNameMap[propName] = option.name;
 
       opts.option?.(option as T, flags[propName]);
 
@@ -386,7 +387,7 @@ export function parseFlags<
     }
   }
 
-  validateFlags(opts, flags, optionNames);
+  validateFlags(opts, flags, optionNameMap);
 
   // convert dotted option keys into nested objects
   const result = Object.keys(flags)

flags/test/setting/allow_empty_test.ts

@@ -1,41 +1,44 @@
-import { assertThrows } from "../../../dev_deps.ts";
+import { assertEquals, assertThrows } from "../../../dev_deps.ts";
 import { parseFlags } from "../../flags.ts";
 
-Deno.test("flags allowEmpty enabled", () => {
-  parseFlags([], {
-    allowEmpty: true,
-    flags: [{
-      name: "flag",
-      aliases: ["f"],
-    }],
-  });
-});
-
-Deno.test("flags allowEmpty noFlags", () => {
-  parseFlags([], {
-    allowEmpty: true,
-    flags: [],
-  });
-});
-
-Deno.test("flags allowEmpty disabledNoFlags", () => {
-  parseFlags([], {
-    allowEmpty: false,
-    flags: [],
-  });
+Deno.test("[flags] should not allow empty by default", () => {
+  assertThrows(
+    () =>
+      parseFlags([], {
+        flags: [{
+          name: "flag",
+          required: true,
+        }],
+      }),
+    Error,
+    'Missing required option "--flag".',
+  );
 });
 
-Deno.test("flags allowEmpty disabled", () => {
+Deno.test("[flags] should not allow empty if disabled", () => {
   assertThrows(
     () =>
       parseFlags([], {
         allowEmpty: false,
         flags: [{
           name: "flag",
-          aliases: ["f"],
+          required: true,
         }],
       }),
     Error,
-    "No arguments.",
+    'Missing required option "--flag".',
   );
 });
+
+Deno.test("[flags] should allow empty if enabled", () => {
+  const { flags, unknown } = parseFlags([], {
+    allowEmpty: true,
+    flags: [{
+      name: "flag",
+      required: true,
+    }],
+  });
+
+  assertEquals(flags, {});
+  assertEquals(unknown, []);
+});