From 4fcfff0393a90bef6313df2c8895cd285da29440 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Fri, 18 Sep 2020 18:09:11 +0100 Subject: [PATCH] fix(bundle, eval, repl): Add missing flags (#7414) Restructures flag helpers and applies them consistently. --- cli/flags.rs | 388 +++++++++++++++++++-------------------------------- 1 file changed, 147 insertions(+), 241 deletions(-) diff --git a/cli/flags.rs b/cli/flags.rs index 5d2e82619a..71a72b259e 100644 --- a/cli/flags.rs +++ b/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) { +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); + no_check_arg_parse(flags, matches); reload_arg_parse(flags, matches); lock_args_parse(flags, matches); - importmap_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); ca_file_arg_parse(flags, matches); +} + +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()) +} + +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); - 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::().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 run_parse(flags: &mut Flags, matches: &clap::ArgMatches) { - run_test_args_parse(flags, matches); + runtime_args_parse(flags, matches, true); let mut script: Vec = 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::() { - 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::() { + 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::().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 =