Skip to content

fix: improve unrecognized arg handling in parser #24984

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aaron-ang wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: improve unrecognized arg handling in parser #24984

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 6 additions & 13 deletions src/deps/zig-clap/clap/streaming.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,22 +87,15 @@ pub fn StreamingClap(comptime Id: type, comptime ArgIterator: type) type {
}

// unrecognized command
// if flag else arg
if (arg_info.kind == .long or arg_info.kind == .short) {
if (warn_on_unrecognized_flag) {
Output.warn("unrecognized flag: {s}{s}\n", .{ if (arg_info.kind == .long) "--" else "-", name });
Output.flush();
}

// continue parsing after unrecognized flag
return parser.next();
}

if (warn_on_unrecognized_flag) {
Output.warn("unrecognized argument: {s}\n", .{name});
if (maybe_value) |v| {
Output.warn("unrecognized argument: --{s}={s}\n", .{ name, v });
} else {
Output.warn("unrecognized flag: --{s}\n", .{name});
}
Output.flush();
}
return null;
return parser.next();
},
Comment on lines 89 to 99
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Unrecognized long options now silently drop InvalidArgument errors

With this change, an unrecognized long argument (e.g. --q or --q=1) never raises error.InvalidArgument anymore:

  • When warn_on_unrecognized_flag is false (the default, and what test "errors" uses), the code just falls through the if and then return parser.next();, effectively skipping the unknown flag with no diagnostic.
  • This contradicts the expectations in test "errors" for --q / --q=1 and diverges from short/positional handling which still report InvalidArgument.

To keep the new, more precise warning text while preserving prior error semantics, consider gating the “warn & skip” behavior behind the flag and otherwise returning an error, e.g.:

-                    // unrecognized command
-                    if (warn_on_unrecognized_flag) {
-                        if (maybe_value) |v| {
-                            Output.warn("unrecognized argument: --{s}={s}\n", .{ name, v });
-                        } else {
-                            Output.warn("unrecognized flag: --{s}\n", .{name});
-                        }
-                        Output.flush();
-                    }
-                    return parser.next();
+                    // unrecognized command
+                    if (warn_on_unrecognized_flag) {
+                        if (maybe_value) |v| {
+                            Output.warn("unrecognized argument: --{s}={s}\n", .{ name, v });
+                        } else {
+                            Output.warn("unrecognized flag: --{s}\n", .{ name });
+                        }
+                        Output.flush();
+                        // Skip unknown flag and keep parsing further args.
+                        return parser.next();
+                    }
+                    // Preserve existing error semantics when warnings are disabled.
+                    return parser.err(arg, .{ .long = name }, error.InvalidArgument);

That keeps the improved warning messages for the Bun CLI use‑case, while ensuring the StreamingClap tests and existing callers relying on InvalidArgument continue to behave as before.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// unrecognized command
// if flag else arg
if (arg_info.kind == .long or arg_info.kind == .short) {
if (warn_on_unrecognized_flag) {
Output.warn("unrecognized flag: {s}{s}\n", .{ if (arg_info.kind == .long) "--" else "-", name });
Output.flush();
}
// continue parsing after unrecognized flag
return parser.next();
}
if (warn_on_unrecognized_flag) {
Output.warn("unrecognized argument: {s}\n", .{name});
if (maybe_value) |v| {
Output.warn("unrecognized argument: --{s}={s}\n", .{ name, v });
} else {
Output.warn("unrecognized flag: --{s}\n", .{name});
}
Output.flush();
}
return null;
return parser.next();
},
// unrecognized command
if (warn_on_unrecognized_flag) {
if (maybe_value) |v| {
Output.warn("unrecognized argument: --{s}={s}\n", .{ name, v });
} else {
Output.warn("unrecognized flag: --{s}\n", .{ name });
}
Output.flush();
// Skip unknown flag and keep parsing further args.
return parser.next();
}
// Preserve existing error semantics when warnings are disabled.
return parser.err(arg, .{ .long = name }, error.InvalidArgument);
🤖 Prompt for AI Agents 
In src/deps/zig-clap/clap/streaming.zig around lines 89 to 99, unrecognized long
options currently always fall through to parser.next() and silently drop
error.InvalidArgument; change the logic so that if warn_on_unrecognized_flag is
true you keep the existing warn/flush/return parser.next() behavior, but if it
is false you return error.InvalidArgument (preserving the prior error semantics
used by tests and short/positional handling); ensure the returned
InvalidArgument includes the option name (and value variant when present)
consistent with existing error construction elsewhere in this file.

.short => return try parser.chainging(.{
.arg = arg,
Expand Down