From ed359a72012b387c3d106be443be601f51a49225 Mon Sep 17 00:00:00 2001
From: Mike Donnalley <mdonnalley@salesforce.com>
Date: Wed, 2 Aug 2023 09:24:29 -0600
Subject: [PATCH] fix: add note to RequiredArgsError when there are flags with
 multiple=true (#754)

---
 package.json              |  2 +-
 src/interfaces/errors.ts  |  2 +-
 src/parser/errors.ts      |  8 ++++++-
 src/parser/validate.ts    |  6 ++++-
 test/parser/parse.test.ts | 47 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 61 insertions(+), 4 deletions(-)

diff --git a/package.json b/package.json
index e16d4530d..ac6045bb8 100644
--- a/package.json
+++ b/package.json
@@ -120,4 +120,4 @@
     "test:perf": "ts-node test/perf/parser.perf.ts"
   },
   "types": "lib/index.d.ts"
-}
\ No newline at end of file
+}
diff --git a/src/interfaces/errors.ts b/src/interfaces/errors.ts
index 70c61d5f8..f31c0d0a0 100644
--- a/src/interfaces/errors.ts
+++ b/src/interfaces/errors.ts
@@ -8,7 +8,7 @@ export interface OclifError {
 
 export interface PrettyPrintableError {
   /**
-   * messsage to display related to the error
+   * message to display related to the error
    */
   message?: string;
 
diff --git a/src/parser/errors.ts b/src/parser/errors.ts
index 976001c79..d6a832159 100644
--- a/src/parser/errors.ts
+++ b/src/parser/errors.ts
@@ -45,7 +45,7 @@ export class InvalidArgsSpecError extends CLIParseError {
 export class RequiredArgsError extends CLIParseError {
   public args: Arg<any>[]
 
-  constructor({args, parse}: CLIParseErrorOptions & { args: Arg<any>[] }) {
+  constructor({args, parse, flagsWithMultiple}: CLIParseErrorOptions & { args: Arg<any>[]; flagsWithMultiple?: string[] }) {
     let message = `Missing ${args.length} required arg${args.length === 1 ? '' : 's'}`
     const namedArgs = args.filter(a => a.name)
     if (namedArgs.length > 0) {
@@ -53,6 +53,12 @@ export class RequiredArgsError extends CLIParseError {
       message += `:\n${list}`
     }
 
+    if (flagsWithMultiple?.length) {
+      const flags = flagsWithMultiple.map(f => `--${f}`).join(', ')
+      message += `\n\nNote: ${flags} allow${flagsWithMultiple.length === 1 ? 's' : ''} multiple values. Because of this you need to provide all arguments before providing ${flagsWithMultiple.length === 1 ? 'that flag' : 'those flags'}.`
+      message += '\nAlternatively, you can use "--" to signify the end of the flags and the beginning of arguments.'
+    }
+
     super({parse, message})
     this.args = args
   }
diff --git a/src/parser/validate.ts b/src/parser/validate.ts
index 0d6e057b3..fe5c342f4 100644
--- a/src/parser/validate.ts
+++ b/src/parser/validate.ts
@@ -44,7 +44,11 @@ export async function validate(parse: {
     }
 
     if (missingRequiredArgs.length > 0) {
-      throw new RequiredArgsError({parse, args: missingRequiredArgs})
+      const flagsWithMultiple = Object.entries(parse.input.flags)
+      .filter(([_, flagDef]) => flagDef.type === 'option' && Boolean(flagDef.multiple))
+      .map(([name]) => name)
+
+      throw new RequiredArgsError({parse, args: missingRequiredArgs, flagsWithMultiple})
     }
   }
 
diff --git a/test/parser/parse.test.ts b/test/parser/parse.test.ts
index c1246938d..526663769 100644
--- a/test/parser/parse.test.ts
+++ b/test/parser/parse.test.ts
@@ -286,6 +286,53 @@ arg3  arg3 desc
 See more help with --help`)
       })
 
+      it('warns about having one flag with multiple values when missing an arg', async () => {
+        let message = ''
+        try {
+          await parse(['--flag1', 'val1', 'arg1'], {
+            args: {
+              arg1: Args.string({required: true, description: 'arg1 desc'}),
+            },
+            flags: {
+              flag1: Flags.string({multiple: true}),
+            },
+          })
+        } catch (error: any) {
+          message = error.message
+        }
+
+        expect(message).to.include(`Missing 1 required arg:
+arg1  arg1 desc
+
+Note: --flag1 allows multiple values. Because of this you need to provide all arguments before providing that flag.
+Alternatively, you can use "--" to signify the end of the flags and the beginning of arguments.
+See more help with --help`)
+      })
+
+      it('warns about having many flags with multiple values when missing an arg', async () => {
+        let message = ''
+        try {
+          await parse(['--flag1', 'val1', '--flag2', 'val1', 'val2', 'arg1'], {
+            args: {
+              arg1: Args.string({required: true, description: 'arg1 desc'}),
+            },
+            flags: {
+              flag1: Flags.string({multiple: true}),
+              flag2: Flags.string({multiple: true}),
+            },
+          })
+        } catch (error: any) {
+          message = error.message
+        }
+
+        expect(message).to.include(`Missing 1 required arg:
+arg1  arg1 desc
+
+Note: --flag1, --flag2 allow multiple values. Because of this you need to provide all arguments before providing those flags.
+Alternatively, you can use "--" to signify the end of the flags and the beginning of arguments.
+See more help with --help`)
+      })
+
       it('too many args', async () => {
         let message = ''
         try {