fix: unexpected commands and arguments should throw

This enables strict mode in our command line parser (yargs), so that unexpected commands and options uniformly throw errors. Fixes #706
cloudflare · Mar 26, 2022 · 2297a85 · 2297a85
1 parent 52ea60f
commit 2297a85
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 47 deletions.
diff --git a/.changeset/breezy-ladybugs-shout.md b/.changeset/breezy-ladybugs-shout.md
@@ -0,0 +1,9 @@
+---
+"wrangler": patch
+---
+
+fix: unexpected commands and arguments should throw
+
+This enables strict mode in our command line parser (yargs), so that unexpected commands and options uniformly throw errors.
+
+Fixes https://github.com/cloudflare/wrangler2/issues/706
diff --git a/packages/wrangler/src/__tests__/index.test.ts b/packages/wrangler/src/__tests__/index.test.ts
@@ -63,7 +63,7 @@ describe("wrangler", () => {
       await expect(
         runWrangler("invalid-command")
       ).rejects.toThrowErrorMatchingInlineSnapshot(
-        `"Unknown command: invalid-command."`
+        `"Unknown argument: invalid-command"`
       );
 
       expect(std.out).toMatchInlineSnapshot(`""`);
@@ -89,7 +89,7 @@ describe("wrangler", () => {
           -v, --version     Show version number  [boolean]
               --legacy-env  Use legacy environments  [boolean]
 
-        Unknown command: invalid-command."
+        Unknown argument: invalid-command"
       `);
     });
   });
@@ -812,14 +812,6 @@ describe("wrangler", () => {
       expect(fs.existsSync("./wrangler.toml")).toBe(true);
     });
 
-    it("should error if `--type` is used", async () => {
-      await expect(
-        runWrangler("init --type")
-      ).rejects.toThrowErrorMatchingInlineSnapshot(
-        `"The --type option is no longer supported."`
-      );
-    });
-
     it("should error if `--type javascript` is used", async () => {
       await expect(
         runWrangler("init --type javascript")
@@ -854,7 +846,7 @@ describe("wrangler", () => {
               Try using \`wrangler dev\` to to try out a worker during development.
               "
             `);
-      await expect(runWrangler("preview GET 'Some Body'")).rejects
+      await expect(runWrangler(`preview GET "SomeBody"`)).rejects
         .toThrowErrorMatchingInlineSnapshot(`
               "DEPRECATION WARNING:
               The \`wrangler preview\` command has been deprecated.

diff --git a/packages/wrangler/src/__tests__/kv.test.ts b/packages/wrangler/src/__tests__/kv.test.ts
@@ -72,7 +72,7 @@ describe("wrangler", () => {
         await expect(
           runWrangler("kv:namespace create abc def ghi")
         ).rejects.toThrowErrorMatchingInlineSnapshot(
-          `"Unexpected additional positional arguments \\"def ghi\\"."`
+          `"Unknown arguments: def, ghi"`
         );
         expect(std.out).toMatchInlineSnapshot(`""`);
         expect(std.err).toMatchInlineSnapshot(`
@@ -93,7 +93,7 @@ describe("wrangler", () => {
             -e, --env      Perform on a specific environment  [string]
                 --preview  Interact with a preview namespace  [boolean]
 
-          Unexpected additional positional arguments \\"def ghi\\"."
+          Unknown arguments: def, ghi"
         `);
       });
 
@@ -809,9 +809,7 @@ describe("wrangler", () => {
               100,
               blankCursorValue
             );
-            await runWrangler(
-              "kv:key list --namespace-id some-namespace-id --limit 100"
-            );
+            await runWrangler("kv:key list --namespace-id some-namespace-id");
             expect(std.err).toMatchInlineSnapshot(`""`);
             expect(JSON.parse(std.out)).toEqual(keys);
             expect(requests.count).toEqual(6);
@@ -889,7 +887,7 @@ describe("wrangler", () => {
           "my-value"
         );
         await runWrangler(
-          "kv:key get my-key my-value --binding someBinding --env some-environment --preview false"
+          "kv:key get my-key --binding someBinding --env some-environment --preview false"
         );
         expect(std.out).toMatchInlineSnapshot(`"my-value"`);
         expect(std.err).toMatchInlineSnapshot(`""`);

diff --git a/packages/wrangler/src/__tests__/r2.test.ts b/packages/wrangler/src/__tests__/r2.test.ts
@@ -90,7 +90,7 @@ describe("wrangler", () => {
           await expect(
             runWrangler("r2 bucket create abc def ghi")
           ).rejects.toThrowErrorMatchingInlineSnapshot(
-            `"Unexpected additional positional arguments \\"def ghi\\"."`
+            `"Unknown arguments: def, ghi"`
           );
           expect(std.out).toMatchInlineSnapshot(`""`);
           expect(std.err).toMatchInlineSnapshot(`
@@ -107,7 +107,7 @@ describe("wrangler", () => {
               -v, --version     Show version number  [boolean]
                   --legacy-env  Use legacy environments  [boolean]
 
-            Unexpected additional positional arguments \\"def ghi\\"."
+            Unknown arguments: def, ghi"
           `);
         });
 
@@ -166,7 +166,7 @@ describe("wrangler", () => {
           await expect(
             runWrangler("r2 bucket delete abc def ghi")
           ).rejects.toThrowErrorMatchingInlineSnapshot(
-            `"Unexpected additional positional arguments \\"def ghi\\"."`
+            `"Unknown arguments: def, ghi"`
           );
           expect(std.out).toMatchInlineSnapshot(`""`);
           expect(std.err).toMatchInlineSnapshot(`
@@ -183,7 +183,7 @@ describe("wrangler", () => {
               -v, --version     Show version number  [boolean]
                   --legacy-env  Use legacy environments  [boolean]
 
-            Unexpected additional positional arguments \\"def ghi\\"."
+            Unknown arguments: def, ghi"
           `);
         });
 

diff --git a/packages/wrangler/src/__tests__/tail.test.ts b/packages/wrangler/src/__tests__/tail.test.ts
@@ -134,17 +134,15 @@ describe("tail", () => {
 
     it("sends header filters without a query", async () => {
       const api = mockWebsocketAPIs();
-      await runWrangler("tail test-worker --header X-CUSTOM-HEADER ");
+      await runWrangler("tail test-worker --header X-CUSTOM-HEADER");
       await expect(api.nextMessageJson()).resolves.toHaveProperty("filters", [
         { header: { key: "X-CUSTOM-HEADER" } },
       ]);
     });
 
     it("sends header filters with a query", async () => {
       const api = mockWebsocketAPIs();
-      await runWrangler(
-        "tail test-worker --header X-CUSTOM-HEADER:some-value "
-      );
+      await runWrangler("tail test-worker --header X-CUSTOM-HEADER:some-value");
       await expect(api.nextMessageJson()).resolves.toHaveProperty("filters", [
         { header: { key: "X-CUSTOM-HEADER", query: "some-value" } },
       ]);

diff --git a/packages/wrangler/src/index.tsx b/packages/wrangler/src/index.tsx
@@ -219,6 +219,7 @@ class DeprecationError extends Error {
 
 export async function main(argv: string[]): Promise<void> {
   const wrangler = makeCLI(argv)
+    .strict()
     // We handle errors ourselves in a try-catch around `yargs.parse`.
     // If you want the "help info" to be displayed then throw an instance of `CommandLineArgsError`.
     // Otherwise we just log the error that was thrown without any "help info".
@@ -297,6 +298,12 @@ export async function main(argv: string[]): Promise<void> {
           describe: "The name of your worker",
           type: "string",
         })
+        .option("type", {
+          describe: "The type of worker to create",
+          type: "string",
+          choices: ["rust", "javascript", "webpack"],
+          deprecated: true,
+        })
         .option("yes", {
           describe: 'Answer "yes" to any prompts for new projects',
           type: "boolean",
@@ -305,7 +312,7 @@ export async function main(argv: string[]): Promise<void> {
     },
     async (args) => {
       printWranglerBanner();
-      if ("type" in args) {
+      if (args.type) {
         let message = "The --type option is no longer supported.";
         if (args.type === "webpack") {
           message +=
@@ -1407,7 +1414,7 @@ export async function main(argv: string[]): Promise<void> {
           }
         )
         .command(
-          "delete",
+          "delete [id]",
           "Delete a route associated with a zone",
           (yargs) => {
             return yargs
@@ -1717,12 +1724,6 @@ export async function main(argv: string[]): Promise<void> {
           },
           async (args) => {
             printWranglerBanner();
-            if (args._.length > 2) {
-              const extraArgs = args._.slice(2).join(" ");
-              throw new CommandLineArgsError(
-                `Unexpected additional positional arguments "${extraArgs}".`
-              );
-            }
 
             if (!isValidNamespaceBinding(args.namespace)) {
               throw new CommandLineArgsError(
@@ -2336,13 +2337,6 @@ export async function main(argv: string[]): Promise<void> {
           },
           async (args) => {
             printWranglerBanner();
-            // We expect three values in `_`: `r2`, `bucket`, `create`.
-            if (args._.length > 3) {
-              const extraArgs = args._.slice(3).join(" ");
-              throw new CommandLineArgsError(
-                `Unexpected additional positional arguments "${extraArgs}".`
-              );
-            }
 
             const config = readConfig(
               args.config as ConfigPath,
@@ -2378,13 +2372,6 @@ export async function main(argv: string[]): Promise<void> {
           },
           async (args) => {
             printWranglerBanner();
-            // We expect three values in `_`: `r2`, `bucket`, `delete`.
-            if (args._.length > 3) {
-              const extraArgs = args._.slice(3).join(" ");
-              throw new CommandLineArgsError(
-                `Unexpected additional positional arguments "${extraArgs}".`
-              );
-            }
 
             const config = readConfig(
               args.config as ConfigPath,