Skip to content

Commit

Permalink
fix(cli) error gracefully when script arg is not present and `--v8-fl…
Browse files Browse the repository at this point in the history
…ags` is present in `deno run` (denoland#20145)

Fix denoland#20022, fix denoland#19627 (duplicate)

denoland#17333 upgraded clap from version 3.1 to version 4. clap version 3.2.0
(intentionally) broke a behavior that deno was relying on to make `deno
run --v8-flags=--help` work without specifying a file, see
clap-rs/clap#3793. The workaround was to make the script argument
required _unless_ `--v8-flags` is present. This broke the expectation
that all successfully parsed `run` commands have the script argument
set, leading to the panic on
`matches.remove_many::<String>("script_arg").unwrap()`.

Clap, as far as I was able to find out, does not currently offer a neat
solution to this problem. This PR adds logic to create and return a
custom clap error when a parsed run command does not have the script
argument.

I added an appropriate test.
  • Loading branch information
Kira authored and littledivy committed Aug 21, 2023
1 parent 52729ed commit 29a3475
Showing 1 changed file with 23 additions and 3 deletions.
26 changes: 23 additions & 3 deletions cli/args/flags.rs
Expand Up @@ -831,7 +831,7 @@ pub fn flags_from_vec(args: Vec<String>) -> clap::error::Result<Flags> {
"lint" => lint_parse(&mut flags, &mut m),
"lsp" => lsp_parse(&mut flags, &mut m),
"repl" => repl_parse(&mut flags, &mut m),
"run" => run_parse(&mut flags, &mut m),
"run" => run_parse(&mut flags, &mut m, app)?,
"task" => task_parse(&mut flags, &mut m),
"test" => test_parse(&mut flags, &mut m),
"types" => types_parse(&mut flags, &mut m),
Expand Down Expand Up @@ -3245,10 +3245,22 @@ fn repl_parse(flags: &mut Flags, matches: &mut ArgMatches) {
);
}

fn run_parse(flags: &mut Flags, matches: &mut ArgMatches) {
fn run_parse(
flags: &mut Flags,
matches: &mut ArgMatches,
app: Command,
) -> clap::error::Result<()> {
runtime_args_parse(flags, matches, true, true);

let mut script_arg = matches.remove_many::<String>("script_arg").unwrap();
let mut script_arg =
matches.remove_many::<String>("script_arg").ok_or_else(|| {
let mut app = app;
let subcommand = &mut app.find_subcommand_mut("run").unwrap();
subcommand.error(
clap::error::ErrorKind::MissingRequiredArgument,
"[SCRIPT_ARG] may only be omitted with --v8-flags=--help",
)
})?;

let script = script_arg.next().unwrap();
flags.argv.extend(script_arg);
Expand All @@ -3259,6 +3271,8 @@ fn run_parse(flags: &mut Flags, matches: &mut ArgMatches) {
script,
watch: watch_arg_parse_with_paths(matches),
});

Ok(())
}

fn task_parse(flags: &mut Flags, matches: &mut ArgMatches) {
Expand Down Expand Up @@ -3996,6 +4010,12 @@ mod tests {
..Flags::default()
}
);

let r = flags_from_vec(svec!["deno", "run", "--v8-flags=--expose-gc"]);
assert!(r
.unwrap_err()
.to_string()
.contains("[SCRIPT_ARG] may only be omitted with --v8-flags=--help"));
}

#[test]
Expand Down

0 comments on commit 29a3475

Please sign in to comment.