From ca9ba87d9956e3f940e0116866e19461f008390b Mon Sep 17 00:00:00 2001
From: Kira <cyra@cyra.pink>
Date: Sun, 13 Aug 2023 04:04:17 +0200
Subject: [PATCH] fix(cli) error gracefully when script arg is not present and
 `--v8-flags` is present in `deno run` (#20145)

Fix #20022, fix #19627 (duplicate)

#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.
---
 cli/args/flags.rs | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/cli/args/flags.rs b/cli/args/flags.rs
index fdfb65f6295a7..bd3740bdb54c3 100644
--- a/cli/args/flags.rs
+++ b/cli/args/flags.rs
@@ -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),
@@ -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);
@@ -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) {
@@ -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]