Fix error messages to distinguish arguments from flags (#858)

Author: kitlangton

packages/effect/src/unstable/cli/CliError.ts

@@ -272,7 +272,7 @@ export class MissingArgument extends Schema.ErrorClass(`${TypeId}/MissingArgumen
 }
 
 /**
- * Error thrown when an option value is invalid.
+ * Error thrown when an option or argument value is invalid.
  *
  * @example
  * ```ts
@@ -282,21 +282,23 @@ export class MissingArgument extends Schema.ErrorClass(`${TypeId}/MissingArgumen
  * const invalidValueError = new CliError.InvalidValue({
  *   option: "port",
  *   value: "abc123",
- *   expected: "integer between 1 and 65535"
+ *   expected: "integer between 1 and 65535",
+ *   kind: "flag"
  * })
  *
  * console.log(invalidValueError.message)
  * // "Invalid value for flag --port: "abc123". Expected: integer between 1 and 65535"
  *
- * // In value validation
- * const validatePortValue = (value: string) =>
- *   Effect.gen(function*() {
- *     const port = Number(value)
- *     if (isNaN(port) || port < 1 || port > 65535) {
- *       return yield* Effect.fail(invalidValueError)
- *     }
- *     return port
- *   })
+ * // For positional arguments
+ * const invalidArgError = new CliError.InvalidValue({
+ *   option: "count",
+ *   value: "abc",
+ *   expected: "integer",
+ *   kind: "argument"
+ * })
+ *
+ * console.log(invalidArgError.message)
+ * // "Invalid value for argument <count>: "abc". Expected: integer"
  * ```
  *
  * @since 4.0.0
@@ -306,7 +308,8 @@ export class InvalidValue extends Schema.ErrorClass(`${TypeId}/InvalidValue`)({
   _tag: Schema.tag("InvalidValue"),
   option: Schema.String,
   value: Schema.String,
-  expected: Schema.String
+  expected: Schema.String,
+  kind: Schema.Union([Schema.Literal("flag"), Schema.Literal("argument")])
 }) {
   /**
    * @since 4.0.0
@@ -317,6 +320,9 @@ export class InvalidValue extends Schema.ErrorClass(`${TypeId}/InvalidValue`)({
    * @since 4.0.0
    */
   override get message() {
+    if (this.kind === "argument") {
+      return `Invalid value for argument <${this.option}>: "${this.value}". Expected: ${this.expected}`
+    }
     return `Invalid value for flag --${this.option}: "${this.value}". Expected: ${this.expected}`
   }
 }

packages/effect/src/unstable/cli/CliOutput.ts

@@ -276,7 +276,8 @@ export const layer = (formatter: Formatter): Layer.Layer<never> => Layer.succeed
  *   const error = new CliError.InvalidValue({
  *     option: "foo",
  *     value: "bar",
- *     expected: "baz"
+ *     expected: "baz",
+ *     kind: "flag"
  *   })
  *   const errorText = formatter.formatError(error)
  *   console.log(errorText)

packages/effect/src/unstable/cli/Param.ts

@@ -952,7 +952,8 @@ export const map: {
  *         new CliError.InvalidValue({
  *           option: "email",
  *           value: email,
- *           expected: "valid email format"
+ *           expected: "valid email format",
+ *           kind: "flag"
  *         })
  *       )
  *   )
@@ -1037,7 +1038,8 @@ export const mapTryCatch: {
             new CliError.InvalidValue({
               option: single.name,
               value: String(a),
-              expected: error
+              expected: error,
+              kind: single.kind
             })
         ),
         Effect.map((b) => [leftover, b] as const)
@@ -1371,7 +1373,8 @@ export const filterMap: {
       return yield* new CliError.InvalidValue({
         option: single.name,
         value: String(a),
-        expected: onNone(a)
+        expected: onNone(a),
+        kind: single.kind
       })
     })
   ))
@@ -1486,7 +1489,8 @@ export const withSchema: {
       return new CliError.InvalidValue({
         option: single.name,
         value: String(value),
-        expected: `Schema validation failed: ${error.message}`
+        expected: `Schema validation failed: ${error.message}`,
+        kind: single.kind
       })
     }))
 })
@@ -1597,7 +1601,8 @@ const parsePositional: <A>(
       new CliError.InvalidValue({
         option: name,
         value: arg,
-        expected: error
+        expected: error,
+        kind: "argument"
       })
   )
 
@@ -1633,7 +1638,8 @@ const parseFlag: <A>(
       new CliError.InvalidValue({
         option: name,
         value: arg,
-        expected: error
+        expected: error,
+        kind: "flag"
       })
   )
 
@@ -1675,7 +1681,8 @@ const parsePositionalVariadic: <Kind extends ParamKind, A>(
     return yield* new CliError.InvalidValue({
       option: single.name,
       value: `${count} values`,
-      expected: `at least ${minValue} value${minValue === 1 ? "" : "s"}`
+      expected: `at least ${minValue} value${minValue === 1 ? "" : "s"}`,
+      kind: single.kind
     })
   }
 
@@ -1709,15 +1716,17 @@ const parseOptionVariadic: <Kind extends ParamKind, A>(
       : new CliError.InvalidValue({
         option: single.name,
         value: `${count} occurrences`,
-        expected: `at least ${options.min} value${options.min === 1 ? "" : "s"}`
+        expected: `at least ${options.min} value${options.min === 1 ? "" : "s"}`,
+        kind: single.kind
       })
   }
 
   if (Predicate.isNotUndefined(options?.max) && count > options.max) {
     return yield* new CliError.InvalidValue({
       option: single.name,
       value: `${count} occurrences`,
-      expected: `at most ${options.max} value${options.max === 1 ? "" : "s"}`
+      expected: `at most ${options.max} value${options.max === 1 ? "" : "s"}`,
+      kind: single.kind
     })
   }
 

packages/effect/test/unstable/cli/Arguments.test.ts

@@ -148,7 +148,7 @@ describe("Command arguments", () => {
       expect(errorText).toMatchInlineSnapshot(`
         "
         ERROR
-          Invalid value for flag --count: "not-a-number". Expected: Failed to parse integer: Expected an integer, got NaN"
+          Invalid value for argument <count>: "not-a-number". Expected: Failed to parse integer: Expected an integer, got NaN"
       `)
     }).pipe(Effect.provide(TestLayer)))
 
@@ -355,4 +355,34 @@ describe("Command arguments", () => {
       const [_leftover, value] = result
       assert.isTrue(Option.isNone(value), "Should be Option.none()")
     }).pipe(Effect.provide(TestLayer)))
+
+  it.effect("should show correct error message for invalid argument (not 'flag')", () =>
+    Effect.gen(function*() {
+      // When a positional argument has an invalid value, the error should say "argument"
+      // not "flag" (which would be confusing)
+      const intArg = Argument.integer("count")
+
+      const result = yield* Effect.exit(
+        intArg.parse({ flags: {}, arguments: ["not-a-number"] })
+      )
+
+      assert.isTrue(result._tag === "Failure", "Should fail with invalid integer")
+      if (result._tag === "Failure") {
+        // Extract the error from the cause
+        const cause = result.cause
+        // The cause structure is { _id: 'Cause', failures: [...] } but we need to access it properly
+        const errorMessage = String(cause)
+
+        // Error message should NOT say "flag"
+        assert.isFalse(
+          errorMessage.includes("flag --"),
+          `Error message should not say 'flag --' for positional arguments: ${errorMessage}`
+        )
+        // Error message SHOULD say "argument"
+        assert.isTrue(
+          errorMessage.includes("argument"),
+          `Error message should say 'argument': ${errorMessage}`
+        )
+      }
+    }).pipe(Effect.provide(TestLayer)))
 })