fix(bundle, eval, repl): Add missing flags (#7414)

Restructures flag helpers and applies them consistently.

cli/flags.rs

@@ -400,12 +400,15 @@ fn install_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
 }
 
 fn bundle_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
-  ca_file_arg_parse(flags, matches);
+  // TODO(nayeemrmn): Replace the next couple lines with `compile_args_parse()`
+  // once `deno bundle --no-check` is supported.
+  unstable_arg_parse(flags, matches);
+  importmap_arg_parse(flags, matches);
+  no_remote_arg_parse(flags, matches);
   config_arg_parse(flags, matches);
   reload_arg_parse(flags, matches);
-  importmap_arg_parse(flags, matches);
-  unstable_arg_parse(flags, matches);
   lock_args_parse(flags, matches);
+  ca_file_arg_parse(flags, matches);
 
   let source_file = matches.value_of("source_file").unwrap().to_string();
 
@@ -438,10 +441,7 @@ fn completions_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
 }
 
 fn repl_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
-  v8_flags_arg_parse(flags, matches);
-  ca_file_arg_parse(flags, matches);
-  inspect_arg_parse(flags, matches);
-  unstable_arg_parse(flags, matches);
+  runtime_args_parse(flags, matches, false);
   flags.subcommand = DenoSubcommand::Repl;
   flags.allow_net = true;
   flags.allow_env = true;
@@ -453,10 +453,7 @@ fn repl_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
 }
 
 fn eval_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
-  v8_flags_arg_parse(flags, matches);
-  ca_file_arg_parse(flags, matches);
-  inspect_arg_parse(flags, matches);
-  unstable_arg_parse(flags, matches);
+  runtime_args_parse(flags, matches, false);
   flags.allow_net = true;
   flags.allow_env = true;
   flags.allow_run = true;
@@ -486,14 +483,7 @@ fn info_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
 }
 
 fn cache_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
-  reload_arg_parse(flags, matches);
-  lock_args_parse(flags, matches);
-  importmap_arg_parse(flags, matches);
-  config_arg_parse(flags, matches);
-  no_check_arg_parse(flags, matches);
-  no_remote_arg_parse(flags, matches);
-  ca_file_arg_parse(flags, matches);
-  unstable_arg_parse(flags, matches);
+  compile_args_parse(flags, matches);
   let files = matches
     .values_of("file")
     .unwrap()
@@ -512,44 +502,60 @@ fn lock_args_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
   }
 }
 
-// Shared between the run and test subcommands. They both take similar options.
-fn run_test_args_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
-  reload_arg_parse(flags, matches);
-  lock_args_parse(flags, matches);
+fn compile_args<'a, 'b>(app: App<'a, 'b>) -> App<'a, 'b> {
+  app
+    .arg(unstable_arg())
+    .arg(importmap_arg())
+    .arg(no_remote_arg())
+    .arg(config_arg())
+    .arg(no_check_arg())
+    .arg(reload_arg())
+    .arg(lock_arg())
+    .arg(lock_write_arg())
+    .arg(ca_file_arg())
+}
+
+fn compile_args_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
+  unstable_arg_parse(flags, matches);
   importmap_arg_parse(flags, matches);
+  no_remote_arg_parse(flags, matches);
   config_arg_parse(flags, matches);
-  v8_flags_arg_parse(flags, matches);
   no_check_arg_parse(flags, matches);
-  no_remote_arg_parse(flags, matches);
-  permission_args_parse(flags, matches);
+  reload_arg_parse(flags, matches);
+  lock_args_parse(flags, matches);
   ca_file_arg_parse(flags, matches);
-  inspect_arg_parse(flags, matches);
-  unstable_arg_parse(flags, matches);
-
-  if matches.is_present("cached-only") {
-    flags.cached_only = true;
-  }
-
-  if matches.is_present("seed") {
-    let seed_string = matches.value_of("seed").unwrap();
-    let seed = seed_string.parse::<u64>().unwrap();
-    flags.seed = Some(seed);
+}
 
-    let v8_seed_flag = format!("--random-seed={}", seed);
+fn runtime_args<'a, 'b>(app: App<'a, 'b>, include_perms: bool) -> App<'a, 'b> {
+  let app = inspect_args(compile_args(app));
+  let app = if include_perms {
+    permission_args(app)
+  } else {
+    app
+  };
+  app
+    .arg(cached_only_arg())
+    .arg(v8_flags_arg())
+    .arg(seed_arg())
+}
 
-    match flags.v8_flags {
-      Some(ref mut v8_flags) => {
-        v8_flags.push(v8_seed_flag);
-      }
-      None => {
-        flags.v8_flags = Some(svec![v8_seed_flag]);
-      }
-    }
+fn runtime_args_parse(
+  flags: &mut Flags,
+  matches: &clap::ArgMatches,
+  include_perms: bool,
+) {
+  compile_args_parse(flags, matches);
+  cached_only_arg_parse(flags, matches);
+  if include_perms {
+    permission_args_parse(flags, matches);
   }
+  v8_flags_arg_parse(flags, matches);
+  seed_arg_parse(flags, matches);
+  inspect_arg_parse(flags, matches);
 }
 
 fn run_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
-  run_test_args_parse(flags, matches);
+  runtime_args_parse(flags, matches, true);
 
   let mut script: Vec<String> = matches
     .values_of("script_arg")
@@ -568,7 +574,7 @@ fn run_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
 }
 
 fn test_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
-  run_test_args_parse(flags, matches);
+  runtime_args_parse(flags, matches, true);
 
   let failfast = matches.is_present("failfast");
   let allow_none = matches.is_present("allow_none");
@@ -715,11 +721,8 @@ Ignore formatting a file by adding an ignore comment at the top of the file:
 }
 
 fn repl_subcommand<'a, 'b>() -> App<'a, 'b> {
-  inspect_args(SubCommand::with_name("repl"))
+  runtime_args(SubCommand::with_name("repl"), false)
     .about("Read Eval Print Loop")
-    .arg(v8_flags_arg())
-    .arg(ca_file_arg())
-    .arg(unstable_arg())
 }
 
 fn install_subcommand<'a, 'b>() -> App<'a, 'b> {
@@ -783,19 +786,22 @@ These must be added to the path manually if required.")
 
 fn bundle_subcommand<'a, 'b>() -> App<'a, 'b> {
   SubCommand::with_name("bundle")
+    // TODO(nayeemrmn): Replace the next couple lines with `compile_args()` once
+    // `deno bundle --no-check` is supported.
+    .arg(unstable_arg())
+    .arg(importmap_arg())
+    .arg(no_remote_arg())
+    .arg(config_arg())
+    .arg(reload_arg())
     .arg(lock_arg())
     .arg(lock_write_arg())
+    .arg(ca_file_arg())
     .arg(
       Arg::with_name("source_file")
         .takes_value(true)
         .required(true),
     )
     .arg(Arg::with_name("out_file").takes_value(true).required(false))
-    .arg(ca_file_arg())
-    .arg(reload_arg())
-    .arg(importmap_arg())
-    .arg(unstable_arg())
-    .arg(config_arg())
     .about("Bundle module and dependencies into single file")
     .long_about(
       "Output a single JavaScript file with all dependencies.
@@ -823,9 +829,7 @@ fn completions_subcommand<'a, 'b>() -> App<'a, 'b> {
 }
 
 fn eval_subcommand<'a, 'b>() -> App<'a, 'b> {
-  inspect_args(SubCommand::with_name("eval"))
-    .arg(ca_file_arg())
-    .arg(unstable_arg())
+  runtime_args(SubCommand::with_name("eval"), false)
     .about("Eval script")
     .long_about(
       "Evaluate JavaScript from the command line.
@@ -853,7 +857,6 @@ This command has implicit access to all permissions (--allow-all).",
         .multiple(false),
     )
     .arg(Arg::with_name("code").takes_value(true).required(true))
-    .arg(v8_flags_arg())
 }
 
 fn info_subcommand<'a, 'b>() -> App<'a, 'b> {
@@ -894,22 +897,13 @@ TypeScript compiler cache: Subdirectory containing TS compiler output.",
 }
 
 fn cache_subcommand<'a, 'b>() -> App<'a, 'b> {
-  SubCommand::with_name("cache")
-    .arg(reload_arg())
-    .arg(lock_arg())
-    .arg(lock_write_arg())
-    .arg(importmap_arg())
-    .arg(unstable_arg())
-    .arg(config_arg())
-    .arg(no_check_arg())
-    .arg(no_remote_arg())
+  compile_args(SubCommand::with_name("cache"))
     .arg(
       Arg::with_name("file")
         .takes_value(true)
         .required(true)
         .min_values(1),
     )
-    .arg(ca_file_arg())
     .about("Cache the dependencies")
     .long_about(
       "Cache and compile remote dependencies recursively.
@@ -1136,38 +1130,8 @@ fn permission_args<'a, 'b>(app: App<'a, 'b>) -> App<'a, 'b> {
     )
 }
 
-fn run_test_args<'a, 'b>(app: App<'a, 'b>) -> App<'a, 'b> {
-  permission_args(inspect_args(app))
-    .arg(importmap_arg())
-    .arg(unstable_arg())
-    .arg(reload_arg())
-    .arg(config_arg())
-    .arg(lock_arg())
-    .arg(lock_write_arg())
-    .arg(no_check_arg())
-    .arg(no_remote_arg())
-    .arg(v8_flags_arg())
-    .arg(ca_file_arg())
-    .arg(
-      Arg::with_name("cached-only")
-        .long("cached-only")
-        .help("Require that remote dependencies are already cached"),
-    )
-    .arg(
-      Arg::with_name("seed")
-        .long("seed")
-        .value_name("NUMBER")
-        .help("Seed Math.random()")
-        .takes_value(true)
-        .validator(|val: String| match val.parse::<u64>() {
-          Ok(_) => Ok(()),
-          Err(_) => Err("Seed should be a number".to_string()),
-        }),
-    )
-}
-
 fn run_subcommand<'a, 'b>() -> App<'a, 'b> {
-  run_test_args(SubCommand::with_name("run"))
+  runtime_args(SubCommand::with_name("run"), true)
     .arg(watch_arg())
     .setting(AppSettings::TrailingVarArg)
     .arg(script_arg())
@@ -1194,7 +1158,7 @@ Deno allows specifying the filename '-' to read the file from stdin.
 }
 
 fn test_subcommand<'a, 'b>() -> App<'a, 'b> {
-  run_test_args(SubCommand::with_name("test"))
+  runtime_args(SubCommand::with_name("test"), true)
     .arg(
       Arg::with_name("failfast")
         .long("failfast")
@@ -1441,6 +1405,49 @@ Only local files from entry point module graph are watched.",
     )
 }
 
+fn seed_arg<'a, 'b>() -> Arg<'a, 'b> {
+  Arg::with_name("seed")
+    .long("seed")
+    .value_name("NUMBER")
+    .help("Seed Math.random()")
+    .takes_value(true)
+    .validator(|val: String| match val.parse::<u64>() {
+      Ok(_) => Ok(()),
+      Err(_) => Err("Seed should be a number".to_string()),
+    })
+}
+
+fn seed_arg_parse(flags: &mut Flags, matches: &ArgMatches) {
+  if matches.is_present("seed") {
+    let seed_string = matches.value_of("seed").unwrap();
+    let seed = seed_string.parse::<u64>().unwrap();
+    flags.seed = Some(seed);
+
+    let v8_seed_flag = format!("--random-seed={}", seed);
+
+    match flags.v8_flags {
+      Some(ref mut v8_flags) => {
+        v8_flags.push(v8_seed_flag);
+      }
+      None => {
+        flags.v8_flags = Some(svec![v8_seed_flag]);
+      }
+    }
+  }
+}
+
+fn cached_only_arg<'a, 'b>() -> Arg<'a, 'b> {
+  Arg::with_name("cached-only")
+    .long("cached-only")
+    .help("Require that remote dependencies are already cached")
+}
+
+fn cached_only_arg_parse(flags: &mut Flags, matches: &ArgMatches) {
+  if matches.is_present("cached-only") {
+    flags.cached_only = true;
+  }
+}
+
 fn no_check_arg<'a, 'b>() -> Arg<'a, 'b> {
   Arg::with_name("no-check")
     .long("no-check")
@@ -2087,35 +2094,6 @@ mod tests {
     );
   }
 
-  #[test]
-  fn eval_unstable() {
-    let r = flags_from_vec_safe(svec![
-      "deno",
-      "eval",
-      "--unstable",
-      "'console.log(\"hello\")'"
-    ]);
-    assert_eq!(
-      r.unwrap(),
-      Flags {
-        unstable: true,
-        subcommand: DenoSubcommand::Eval {
-          print: false,
-          code: "'console.log(\"hello\")'".to_string(),
-          as_typescript: false,
-        },
-        allow_net: true,
-        allow_env: true,
-        allow_run: true,
-        allow_read: true,
-        allow_write: true,
-        allow_plugin: true,
-        allow_hrtime: true,
-        ..Flags::default()
-      }
-    );
-  }
-
   #[test]
   fn eval_typescript() {
     let r = flags_from_vec_safe(svec![
@@ -2145,9 +2123,9 @@ mod tests {
   }
 
   #[test]
-  fn eval_with_v8_flags() {
-    let r =
-      flags_from_vec_safe(svec!["deno", "eval", "--v8-flags=--help", "42"]);
+  fn eval_with_flags() {
+    #[rustfmt::skip]
+    let r = flags_from_vec_safe(svec!["deno", "eval", "--unstable", "--importmap", "import_map.json", "--no-remote", "--config", "tsconfig.json", "--no-check", "--reload", "--lock", "lock.json", "--lock-write", "--cert", "example.crt", "--cached-only", "--v8-flags=--help", "--seed", "1", "--inspect=127.0.0.1:9229", "42"]);
     assert_eq!(
       r.unwrap(),
       Flags {
@@ -2156,7 +2134,19 @@ mod tests {
           code: "42".to_string(),
           as_typescript: false,
         },
-        v8_flags: Some(svec!["--help"]),
+        unstable: true,
+        import_map_path: Some("import_map.json".to_string()),
+        no_remote: true,
+        config_path: Some("tsconfig.json".to_string()),
+        no_check: true,
+        reload: true,
+        lock: Some("lock.json".to_string()),
+        lock_write: true,
+        ca_file: Some("example.crt".to_string()),
+        cached_only: true,
+        v8_flags: Some(svec!["--help", "--random-seed=1"]),
+        seed: Some(1),
+        inspect: Some("127.0.0.1:9229".parse().unwrap()),
         allow_net: true,
         allow_env: true,
         allow_run: true,
@@ -2189,13 +2179,26 @@ mod tests {
   }
 
   #[test]
-  fn repl_unstable() {
-    let r = flags_from_vec_safe(svec!["deno", "repl", "--unstable"]);
+  fn repl_with_flags() {
+    #[rustfmt::skip]
+    let r = flags_from_vec_safe(svec!["deno", "repl", "--unstable", "--importmap", "import_map.json", "--no-remote", "--config", "tsconfig.json", "--no-check", "--reload", "--lock", "lock.json", "--lock-write", "--cert", "example.crt", "--cached-only", "--v8-flags=--help", "--seed", "1", "--inspect=127.0.0.1:9229"]);
     assert_eq!(
       r.unwrap(),
       Flags {
-        unstable: true,
         subcommand: DenoSubcommand::Repl,
+        unstable: true,
+        import_map_path: Some("import_map.json".to_string()),
+        no_remote: true,
+        config_path: Some("tsconfig.json".to_string()),
+        no_check: true,
+        reload: true,
+        lock: Some("lock.json".to_string()),
+        lock_write: true,
+        ca_file: Some("example.crt".to_string()),
+        cached_only: true,
+        v8_flags: Some(svec!["--help", "--random-seed=1"]),
+        seed: Some(1),
+        inspect: Some("127.0.0.1:9229".parse().unwrap()),
         allow_net: true,
         allow_env: true,
         allow_run: true,
@@ -2314,6 +2317,7 @@ mod tests {
     let r = flags_from_vec_safe(svec![
       "deno",
       "bundle",
+      "--no-remote",
       "--config",
       "tsconfig.json",
       "source.ts",
@@ -2327,6 +2331,7 @@ mod tests {
           out_file: Some(PathBuf::from("bundle.js")),
         },
         allow_write: true,
+        no_remote: true,
         config_path: Some("tsconfig.json".to_owned()),
         ..Flags::default()
       }
@@ -3008,36 +3013,6 @@ mod tests {
     );
   }
 
-  #[test]
-  fn eval_with_cafile() {
-    let r = flags_from_vec_safe(svec![
-      "deno",
-      "eval",
-      "--cert",
-      "example.crt",
-      "console.log('hello world')"
-    ]);
-    assert_eq!(
-      r.unwrap(),
-      Flags {
-        subcommand: DenoSubcommand::Eval {
-          print: false,
-          code: "console.log('hello world')".to_string(),
-          as_typescript: false,
-        },
-        ca_file: Some("example.crt".to_owned()),
-        allow_net: true,
-        allow_env: true,
-        allow_run: true,
-        allow_read: true,
-        allow_write: true,
-        allow_plugin: true,
-        allow_hrtime: true,
-        ..Flags::default()
-      }
-    );
-  }
-
   #[test]
   fn upgrade_with_ca_file() {
     let r =
@@ -3058,35 +3033,6 @@ mod tests {
     );
   }
 
-  #[test]
-  fn eval_with_inspect() {
-    let r = flags_from_vec_safe(svec![
-      "deno",
-      "eval",
-      "--inspect",
-      "const foo = 'bar'"
-    ]);
-    assert_eq!(
-      r.unwrap(),
-      Flags {
-        subcommand: DenoSubcommand::Eval {
-          print: false,
-          code: "const foo = 'bar'".to_string(),
-          as_typescript: false,
-        },
-        inspect: Some("127.0.0.1:9229".parse().unwrap()),
-        allow_net: true,
-        allow_env: true,
-        allow_run: true,
-        allow_read: true,
-        allow_write: true,
-        allow_plugin: true,
-        allow_hrtime: true,
-        ..Flags::default()
-      }
-    );
-  }
-
   #[test]
   fn cache_with_cafile() {
     let r = flags_from_vec_safe(svec![
@@ -3158,46 +3104,6 @@ mod tests {
     );
   }
 
-  #[test]
-  fn repl_with_cafile() {
-    let r = flags_from_vec_safe(svec!["deno", "repl", "--cert", "example.crt"]);
-    assert_eq!(
-      r.unwrap(),
-      Flags {
-        subcommand: DenoSubcommand::Repl {},
-        ca_file: Some("example.crt".to_owned()),
-        allow_read: true,
-        allow_write: true,
-        allow_net: true,
-        allow_env: true,
-        allow_run: true,
-        allow_plugin: true,
-        allow_hrtime: true,
-        ..Flags::default()
-      }
-    );
-  }
-
-  #[test]
-  fn repl_with_inspect() {
-    let r = flags_from_vec_safe(svec!["deno", "repl", "--inspect"]);
-    assert_eq!(
-      r.unwrap(),
-      Flags {
-        subcommand: DenoSubcommand::Repl {},
-        inspect: Some("127.0.0.1:9229".parse().unwrap()),
-        allow_read: true,
-        allow_write: true,
-        allow_net: true,
-        allow_env: true,
-        allow_run: true,
-        allow_plugin: true,
-        allow_hrtime: true,
-        ..Flags::default()
-      }
-    );
-  }
-
   #[test]
   fn doc() {
     let r =