From 5403e4f06b2bb9da60c67b7c1909f4d412c20307 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Czerniawski?= <33061335+lczerniawski@users.noreply.github.com> Date: Fri, 15 Mar 2024 00:53:46 +0100 Subject: [PATCH] chore(cli): move away from PathBuf in clap (#22036) --- cli/args/flags.rs | 189 ++++++++++++++++++++-------------------- cli/args/mod.rs | 60 ++++++++++--- cli/tools/compile.rs | 32 +++---- cli/tools/installer.rs | 2 +- cli/tools/upgrade.rs | 11 ++- cli/tools/vendor/mod.rs | 2 +- cli/util/path.rs | 39 --------- 7 files changed, 168 insertions(+), 167 deletions(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 3065e130b9..07b60331ed 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -69,7 +69,7 @@ pub struct CheckFlags { #[derive(Clone, Debug, Eq, PartialEq)] pub struct CompileFlags { pub source_file: String, - pub output: Option, + pub output: Option, pub args: Vec, pub target: Option, pub no_terminal: bool, @@ -102,7 +102,7 @@ pub enum CoverageType { #[derive(Clone, Debug, Eq, PartialEq, Default)] pub struct CoverageFlags { pub files: FileFlags, - pub output: Option, + pub output: Option, pub include: Vec, pub exclude: Vec, pub r#type: CoverageType, @@ -123,7 +123,7 @@ impl Default for DocSourceFileFlag { #[derive(Clone, Debug, Eq, PartialEq)] pub struct DocHtmlFlag { pub name: String, - pub output: PathBuf, + pub output: String, } #[derive(Clone, Debug, Eq, PartialEq)] @@ -186,7 +186,7 @@ pub struct InstallFlags { pub struct JupyterFlags { pub install: bool, pub kernel: bool, - pub conn_file: Option, + pub conn_file: Option, } #[derive(Clone, Debug, Eq, PartialEq)] @@ -250,7 +250,7 @@ pub struct WatchFlags { #[derive(Clone, Default, Debug, Eq, PartialEq)] pub struct WatchFlagsWithPaths { pub hmr: bool, - pub paths: Vec, + pub paths: Vec, pub no_clear_screen: bool, } @@ -292,13 +292,13 @@ pub struct UpgradeFlags { pub force: bool, pub canary: bool, pub version: Option, - pub output: Option, + pub output: Option, } #[derive(Clone, Debug, Eq, PartialEq)] pub struct VendorFlags { pub specifiers: Vec, - pub output_path: Option, + pub output_path: Option, pub force: bool, } @@ -440,16 +440,16 @@ pub struct Flags { pub deny_hrtime: bool, pub allow_net: Option>, pub deny_net: Option>, - pub allow_ffi: Option>, - pub deny_ffi: Option>, - pub allow_read: Option>, - pub deny_read: Option>, + pub allow_ffi: Option>, + pub deny_ffi: Option>, + pub allow_read: Option>, + pub deny_read: Option>, pub allow_run: Option>, pub deny_run: Option>, pub allow_sys: Option>, pub deny_sys: Option>, - pub allow_write: Option>, - pub deny_write: Option>, + pub allow_write: Option>, + pub deny_write: Option>, pub ca_stores: Option>, pub ca_data: Option, pub cache_blocklist: Vec, @@ -464,7 +464,7 @@ pub struct Flags { pub enable_op_summary_metrics: bool, pub enable_testing_features: bool, pub ext: Option, - pub ignore: Vec, + pub ignore: Vec, pub import_map_path: Option, pub env_file: Option, pub inspect_brk: Option, @@ -472,7 +472,7 @@ pub struct Flags { pub inspect: Option, pub location: Option, pub lock_write: bool, - pub lock: Option, + pub lock: Option, pub log_level: Option, pub no_remote: bool, pub no_lock: bool, @@ -486,10 +486,10 @@ pub struct Flags { pub v8_flags: Vec, } -fn join_paths(allowlist: &[PathBuf], d: &str) -> String { +fn join_paths(allowlist: &[String], d: &str) -> String { allowlist .iter() - .map(|path| path.to_str().unwrap().to_string()) + .map(|path| path.to_string()) .collect::>() .join(d) } @@ -1348,8 +1348,7 @@ supported in canary. Arg::new("output") .long("output") .short('o') - // todo(dsherret): remove value_parser!(PathBuf) and instead parse as string - .value_parser(value_parser!(PathBuf)) + .value_parser(value_parser!(String)) .help("Output file (defaults to $PWD/)") .value_hint(ValueHint::FilePath), ) @@ -1473,8 +1472,7 @@ Generate html reports from lcov: Arg::new("output") .requires("lcov") .long("output") - // todo(dsherret): remove value_parser!(PathBuf) and instead parse as string - .value_parser(value_parser!(PathBuf)) + .value_parser(value_parser!(String)) .help("Output file (defaults to stdout) for lcov") .long_help( "Exports the coverage report in lcov format to the given file. @@ -1580,8 +1578,7 @@ Show documentation for runtime built-ins: .action(ArgAction::Set) .require_equals(true) .value_hint(ValueHint::DirPath) - // todo(dsherret): remove value_parser!(PathBuf) and instead parse as string - .value_parser(value_parser!(PathBuf)) + .value_parser(value_parser!(String)) ) .arg( Arg::new("private") @@ -1922,8 +1919,7 @@ fn jupyter_subcommand() -> Command { Arg::new("conn") .long("conn") .help("Path to JSON file describing connection parameters, provided by Jupyter") - // todo(dsherret): remove value_parser!(PathBuf) and instead parse as string - .value_parser(value_parser!(PathBuf)) + .value_parser(value_parser!(String)) .value_hint(ValueHint::FilePath) .conflicts_with("install")) .about("Deno kernel for Jupyter notebooks") @@ -2346,8 +2342,7 @@ update to a different location, use the --output flag Arg::new("output") .long("output") .help("The path to output the updated version to") - // todo(dsherret): remove value_parser!(PathBuf) and instead parse as string - .value_parser(value_parser!(PathBuf)) + .value_parser(value_parser!(String)) .value_hint(ValueHint::FilePath), ) .arg( @@ -2401,8 +2396,7 @@ Remote modules and multiple modules may also be specified: Arg::new("output") .long("output") .help("The directory to output the vendored modules to") - // todo(dsherret): remove value_parser!(PathBuf) and instead parse as string - .value_parser(value_parser!(PathBuf)) + .value_parser(value_parser!(String)) .value_hint(ValueHint::DirPath), ) .arg( @@ -2656,8 +2650,7 @@ fn permission_args(app: Command) -> Command { .require_equals(true) .value_name("PATH") .help(ALLOW_READ_HELP) - // todo(dsherret): remove value_parser!(PathBuf) and instead parse as string - .value_parser(value_parser!(PathBuf)) + .value_parser(value_parser!(String)) .value_hint(ValueHint::AnyPath), ) .arg( @@ -2668,8 +2661,7 @@ fn permission_args(app: Command) -> Command { .require_equals(true) .value_name("PATH") .help(DENY_READ_HELP) - // todo(dsherret): remove value_parser!(PathBuf) and instead parse as string - .value_parser(value_parser!(PathBuf)) + .value_parser(value_parser!(String)) .value_hint(ValueHint::AnyPath), ) .arg( @@ -2680,8 +2672,7 @@ fn permission_args(app: Command) -> Command { .require_equals(true) .value_name("PATH") .help(ALLOW_WRITE_HELP) - // todo(dsherret): remove value_parser!(PathBuf) and instead parse as string - .value_parser(value_parser!(PathBuf)) + .value_parser(value_parser!(String)) .value_hint(ValueHint::AnyPath), ) .arg( @@ -2692,8 +2683,7 @@ fn permission_args(app: Command) -> Command { .require_equals(true) .value_name("PATH") .help(DENY_WRITE_HELP) - // todo(dsherret): remove value_parser!(PathBuf) and instead parse as string - .value_parser(value_parser!(PathBuf)) + .value_parser(value_parser!(String)) .value_hint(ValueHint::AnyPath), ) .arg( @@ -2803,8 +2793,7 @@ fn permission_args(app: Command) -> Command { .require_equals(true) .value_name("PATH") .help(ALLOW_FFI_HELP) - // todo(dsherret): remove value_parser!(PathBuf) and instead parse as string - .value_parser(value_parser!(PathBuf)) + .value_parser(value_parser!(String)) .value_hint(ValueHint::AnyPath), ) .arg( @@ -2815,8 +2804,7 @@ fn permission_args(app: Command) -> Command { .require_equals(true) .value_name("PATH") .help(DENY_FFI_HELP) - // todo(dsherret): remove value_parser!(PathBuf) and instead parse as string - .value_parser(value_parser!(PathBuf)) + .value_parser(value_parser!(String)) .value_hint(ValueHint::AnyPath), ) .arg( @@ -3056,8 +3044,7 @@ fn hmr_arg(takes_files: bool) -> Arg { arg .value_name("FILES") .num_args(0..) - // todo(dsherret): remove value_parser!(PathBuf) and instead parse as string - .value_parser(value_parser!(PathBuf)) + .value_parser(value_parser!(String)) .use_value_delimiter(true) .require_equals(true) .long_help( @@ -3083,8 +3070,7 @@ fn watch_arg(takes_files: bool) -> Arg { arg .value_name("FILES") .num_args(0..) - // todo(dsherret): remove value_parser!(PathBuf) and instead parse as string - .value_parser(value_parser!(PathBuf)) + .value_parser(value_parser!(String)) .use_value_delimiter(true) .require_equals(true) .long_help( @@ -3175,8 +3161,7 @@ fn lock_arg() -> Arg { If value is not provided, defaults to \"deno.lock\" in the current working directory.") .num_args(0..=1) - // todo(dsherret): remove value_parser!(PathBuf) and instead parse as string - .value_parser(value_parser!(PathBuf)) + .value_parser(value_parser!(String)) .value_hint(ValueHint::FilePath) } @@ -3362,7 +3347,7 @@ fn compile_parse(flags: &mut Flags, matches: &mut ArgMatches) { let mut script = matches.remove_many::("script_arg").unwrap(); let source_file = script.next().unwrap(); let args = script.collect(); - let output = matches.remove_one::("output"); + let output = matches.remove_one::("output"); let target = matches.remove_one::("target"); let no_terminal = matches.get_flag("no-terminal"); let include = match matches.remove_many::("include") { @@ -3436,7 +3421,7 @@ fn coverage_parse(flags: &mut Flags, matches: &mut ArgMatches) { } else { CoverageType::Summary }; - let output = matches.remove_one::("output"); + let output = matches.remove_one::("output"); flags.subcommand = DenoSubcommand::Coverage(CoverageFlags { files: FileFlags { include: files, @@ -3482,8 +3467,8 @@ fn doc_parse(flags: &mut Flags, matches: &mut ArgMatches) { let html = if matches.get_flag("html") { let name = matches.remove_one::("name").unwrap(); let output = matches - .remove_one::("output") - .unwrap_or(PathBuf::from("./docs/")); + .remove_one::("output") + .unwrap_or(String::from("./docs/")); Some(DocHtmlFlag { name, output }) } else { None @@ -3605,7 +3590,7 @@ fn install_parse(flags: &mut Flags, matches: &mut ArgMatches) { } fn jupyter_parse(flags: &mut Flags, matches: &mut ArgMatches) { - let conn_file = matches.remove_one::("conn"); + let conn_file = matches.remove_one::("conn"); let kernel = matches.get_flag("kernel"); let install = matches.get_flag("install"); @@ -3871,7 +3856,7 @@ fn upgrade_parse(flags: &mut Flags, matches: &mut ArgMatches) { let force = matches.get_flag("force"); let canary = matches.get_flag("canary"); let version = matches.remove_one::("version"); - let output = matches.remove_one::("output"); + let output = matches.remove_one::("output"); flags.subcommand = DenoSubcommand::Upgrade(UpgradeFlags { dry_run, force, @@ -3894,7 +3879,7 @@ fn vendor_parse(flags: &mut Flags, matches: &mut ArgMatches) { .remove_many::("specifiers") .map(|p| p.collect()) .unwrap_or_default(), - output_path: matches.remove_one::("output"), + output_path: matches.remove_one::("output"), force: matches.get_flag("force"), }); } @@ -3936,19 +3921,19 @@ fn compile_args_without_check_parse( fn permission_args_parse(flags: &mut Flags, matches: &mut ArgMatches) { unsafely_ignore_certificate_errors_parse(flags, matches); - if let Some(read_wl) = matches.remove_many::("allow-read") { + if let Some(read_wl) = matches.remove_many::("allow-read") { flags.allow_read = Some(read_wl.collect()); } - if let Some(read_wl) = matches.remove_many::("deny-read") { + if let Some(read_wl) = matches.remove_many::("deny-read") { flags.deny_read = Some(read_wl.collect()); } - if let Some(write_wl) = matches.remove_many::("allow-write") { + if let Some(write_wl) = matches.remove_many::("allow-write") { flags.allow_write = Some(write_wl.collect()); } - if let Some(write_wl) = matches.remove_many::("deny-write") { + if let Some(write_wl) = matches.remove_many::("deny-write") { flags.deny_write = Some(write_wl.collect()); } @@ -3992,12 +3977,12 @@ fn permission_args_parse(flags: &mut Flags, matches: &mut ArgMatches) { debug!("sys info denylist: {:#?}", &flags.deny_sys); } - if let Some(ffi_wl) = matches.remove_many::("allow-ffi") { + if let Some(ffi_wl) = matches.remove_many::("allow-ffi") { flags.allow_ffi = Some(ffi_wl.collect()); debug!("ffi allowlist: {:#?}", &flags.allow_ffi); } - if let Some(ffi_wl) = matches.remove_many::("deny-ffi") { + if let Some(ffi_wl) = matches.remove_many::("deny-ffi") { flags.deny_ffi = Some(ffi_wl.collect()); debug!("ffi denylist: {:#?}", &flags.deny_ffi); } @@ -4191,8 +4176,8 @@ fn lock_args_parse(flags: &mut Flags, matches: &mut ArgMatches) { fn lock_arg_parse(flags: &mut Flags, matches: &mut ArgMatches) { if matches.contains_id("lock") { let lockfile = matches - .remove_one::("lock") - .unwrap_or_else(|| PathBuf::from("./deno.lock")); + .remove_one::("lock") + .unwrap_or_else(|| String::from("./deno.lock")); flags.lock = Some(lockfile); } } @@ -4257,7 +4242,7 @@ fn watch_arg_parse(matches: &mut ArgMatches) -> Option { fn watch_arg_parse_with_paths( matches: &mut ArgMatches, ) -> Option { - if let Some(paths) = matches.remove_many::("watch") { + if let Some(paths) = matches.remove_many::("watch") { return Some(WatchFlagsWithPaths { paths: paths.collect(), hmr: false, @@ -4266,7 +4251,7 @@ fn watch_arg_parse_with_paths( } matches - .remove_many::("hmr") + .remove_many::("hmr") .map(|paths| WatchFlagsWithPaths { paths: paths.collect(), hmr: true, @@ -4348,6 +4333,24 @@ mod tests { ); } + #[test] + fn upgrade_with_output_flag() { + let r = flags_from_vec(svec!["deno", "upgrade", "--output", "example.txt"]); + assert_eq!( + r.unwrap(), + Flags { + subcommand: DenoSubcommand::Upgrade(UpgradeFlags { + force: false, + dry_run: false, + canary: false, + version: None, + output: Some(String::from("example.txt")), + }), + ..Flags::default() + } + ); + } + #[test] fn version() { let r = flags_from_vec(svec!["deno", "--version"]); @@ -4458,7 +4461,7 @@ mod tests { script: "script.ts".to_string(), watch: Some(WatchFlagsWithPaths { hmr: true, - paths: vec![PathBuf::from("foo.txt")], + paths: vec![String::from("foo.txt")], no_clear_screen: true, }), }), @@ -4483,7 +4486,7 @@ mod tests { script: "script.ts".to_string(), watch: Some(WatchFlagsWithPaths { hmr: false, - paths: vec![PathBuf::from("file1"), PathBuf::from("file2")], + paths: vec![String::from("file1"), String::from("file2")], no_clear_screen: false, }), }), @@ -5517,7 +5520,7 @@ mod tests { config_flag: ConfigFlag::Path("tsconfig.json".to_owned()), type_check_mode: TypeCheckMode::None, reload: true, - lock: Some(PathBuf::from("lock.json")), + lock: Some(String::from("lock.json")), lock_write: true, ca_data: Some(CaData::File("example.crt".to_string())), cached_only: true, @@ -5632,7 +5635,7 @@ mod tests { config_flag: ConfigFlag::Path("tsconfig.json".to_owned()), type_check_mode: TypeCheckMode::None, reload: true, - lock: Some(PathBuf::from("lock.json")), + lock: Some(String::from("lock.json")), lock_write: true, ca_data: Some(CaData::File("example.crt".to_string())), cached_only: true, @@ -5701,18 +5704,18 @@ mod tests { fn allow_read_allowlist() { use test_util::TempDir; let temp_dir_guard = TempDir::new(); - let temp_dir = temp_dir_guard.path().to_path_buf(); + let temp_dir = temp_dir_guard.path().to_string(); let r = flags_from_vec(svec![ "deno", "run", - format!("--allow-read=.,{}", temp_dir.to_str().unwrap()), + format!("--allow-read=.,{}", temp_dir), "script.ts" ]); assert_eq!( r.unwrap(), Flags { - allow_read: Some(vec![PathBuf::from("."), temp_dir]), + allow_read: Some(vec![String::from("."), temp_dir]), subcommand: DenoSubcommand::Run(RunFlags::new_default( "script.ts".to_string(), )), @@ -5725,18 +5728,18 @@ mod tests { fn deny_read_denylist() { use test_util::TempDir; let temp_dir_guard = TempDir::new(); - let temp_dir = temp_dir_guard.path().to_path_buf(); + let temp_dir = temp_dir_guard.path().to_string(); let r = flags_from_vec(svec![ "deno", "run", - format!("--deny-read=.,{}", temp_dir.to_str().unwrap()), + format!("--deny-read=.,{}", temp_dir), "script.ts" ]); assert_eq!( r.unwrap(), Flags { - deny_read: Some(vec![PathBuf::from("."), temp_dir]), + deny_read: Some(vec![String::from("."), temp_dir]), subcommand: DenoSubcommand::Run(RunFlags::new_default( "script.ts".to_string(), )), @@ -5749,18 +5752,18 @@ mod tests { fn allow_write_allowlist() { use test_util::TempDir; let temp_dir_guard = TempDir::new(); - let temp_dir = temp_dir_guard.path().to_path_buf(); + let temp_dir = temp_dir_guard.path().to_string(); let r = flags_from_vec(svec![ "deno", "run", - format!("--allow-write=.,{}", temp_dir.to_str().unwrap()), + format!("--allow-write=.,{}", temp_dir), "script.ts" ]); assert_eq!( r.unwrap(), Flags { - allow_write: Some(vec![PathBuf::from("."), temp_dir]), + allow_write: Some(vec![String::from("."), temp_dir]), subcommand: DenoSubcommand::Run(RunFlags::new_default( "script.ts".to_string(), )), @@ -5773,18 +5776,18 @@ mod tests { fn deny_write_denylist() { use test_util::TempDir; let temp_dir_guard = TempDir::new(); - let temp_dir = temp_dir_guard.path().to_path_buf(); + let temp_dir = temp_dir_guard.path().to_string(); let r = flags_from_vec(svec![ "deno", "run", - format!("--deny-write=.,{}", temp_dir.to_str().unwrap()), + format!("--deny-write=.,{}", temp_dir), "script.ts" ]); assert_eq!( r.unwrap(), Flags { - deny_write: Some(vec![PathBuf::from("."), temp_dir]), + deny_write: Some(vec![String::from("."), temp_dir]), subcommand: DenoSubcommand::Run(RunFlags::new_default( "script.ts".to_string(), )), @@ -6226,7 +6229,7 @@ mod tests { }), type_check_mode: TypeCheckMode::Local, lock_write: true, - lock: Some(PathBuf::from("lock.json")), + lock: Some(String::from("lock.json")), ..Flags::default() } ); @@ -6523,7 +6526,7 @@ mod tests { config_flag: ConfigFlag::Path("tsconfig.json".to_owned()), type_check_mode: TypeCheckMode::None, reload: true, - lock: Some(PathBuf::from("lock.json")), + lock: Some(String::from("lock.json")), lock_write: true, ca_data: Some(CaData::File("example.crt".to_string())), cached_only: true, @@ -7054,7 +7057,7 @@ mod tests { "script.ts".to_string(), )), lock_write: true, - lock: Some(PathBuf::from("lock.json")), + lock: Some(String::from("lock.json")), ..Flags::default() } ); @@ -7085,7 +7088,7 @@ mod tests { "script.ts".to_string(), )), lock_write: true, - lock: Some(PathBuf::from("./deno.lock")), + lock: Some(String::from("./deno.lock")), ..Flags::default() } ); @@ -7105,7 +7108,7 @@ mod tests { "script.ts".to_string(), )), lock_write: true, - lock: Some(PathBuf::from("lock.json")), + lock: Some(String::from("lock.json")), ..Flags::default() } ); @@ -7692,7 +7695,7 @@ mod tests { lint: false, html: Some(DocHtmlFlag { name: "My library".to_string(), - output: PathBuf::from("./docs/"), + output: String::from("./docs/"), }), source_files: DocSourceFileFlag::Paths(svec!["path/to/module.ts"]), filter: None, @@ -7718,7 +7721,7 @@ mod tests { json: false, html: Some(DocHtmlFlag { name: "My library".to_string(), - output: PathBuf::from("./foo"), + output: String::from("./foo"), }), lint: true, source_files: DocSourceFileFlag::Paths(svec!["path/to/module.ts"]), @@ -7979,7 +7982,7 @@ mod tests { subcommand: DenoSubcommand::Compile(CompileFlags { source_file: "https://examples.deno.land/color-logging.ts" .to_string(), - output: Some(PathBuf::from("colors")), + output: Some(String::from("colors")), args: svec!["foo", "bar", "-p", "8080"], target: None, no_terminal: true, @@ -7990,7 +7993,7 @@ mod tests { config_flag: ConfigFlag::Path("tsconfig.json".to_owned()), type_check_mode: TypeCheckMode::None, reload: true, - lock: Some(PathBuf::from("lock.json")), + lock: Some(String::from("lock.json")), lock_write: true, ca_data: Some(CaData::File("example.crt".to_string())), cached_only: true, @@ -8046,7 +8049,7 @@ mod tests { include: vec![r"^file:".to_string()], exclude: vec![r"test\.(js|mjs|ts|jsx|tsx)$".to_string()], r#type: CoverageType::Lcov, - output: Some(PathBuf::from("foo.lcov")), + output: Some(String::from("foo.lcov")), }), ..Flags::default() } @@ -8163,11 +8166,11 @@ mod tests { subcommand: DenoSubcommand::Vendor(VendorFlags { specifiers: svec!["mod.ts", "deps.test.ts"], force: true, - output_path: Some(PathBuf::from("out_dir")), + output_path: Some(String::from("out_dir")), }), config_flag: ConfigFlag::Path("deno.json".to_owned()), import_map_path: Some("import_map.json".to_string()), - lock: Some(PathBuf::from("lock.json")), + lock: Some(String::from("lock.json")), reload: true, ..Flags::default() } @@ -8602,7 +8605,7 @@ mod tests { subcommand: DenoSubcommand::Jupyter(JupyterFlags { install: false, kernel: true, - conn_file: Some(PathBuf::from("path/to/conn/file")), + conn_file: Some(String::from("path/to/conn/file")), }), ..Flags::default() } diff --git a/cli/args/mod.rs b/cli/args/mod.rs index f22567d61a..e7913eddc8 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -1476,16 +1476,34 @@ impl CliOptions { deny_hrtime: self.flags.deny_hrtime, allow_net: self.flags.allow_net.clone(), deny_net: self.flags.deny_net.clone(), - allow_ffi: self.flags.allow_ffi.clone(), - deny_ffi: self.flags.deny_ffi.clone(), - allow_read: self.flags.allow_read.clone(), - deny_read: self.flags.deny_read.clone(), + allow_ffi: convert_option_str_to_path_buf( + &self.flags.allow_ffi, + self.initial_cwd(), + ), + deny_ffi: convert_option_str_to_path_buf( + &self.flags.deny_ffi, + self.initial_cwd(), + ), + allow_read: convert_option_str_to_path_buf( + &self.flags.allow_read, + self.initial_cwd(), + ), + deny_read: convert_option_str_to_path_buf( + &self.flags.deny_read, + self.initial_cwd(), + ), allow_run: self.flags.allow_run.clone(), deny_run: self.flags.deny_run.clone(), allow_sys: self.flags.allow_sys.clone(), deny_sys: self.flags.deny_sys.clone(), - allow_write: self.flags.allow_write.clone(), - deny_write: self.flags.deny_write.clone(), + allow_write: convert_option_str_to_path_buf( + &self.flags.allow_write, + self.initial_cwd(), + ), + deny_write: convert_option_str_to_path_buf( + &self.flags.deny_write, + self.initial_cwd(), + ), prompt: !self.no_prompt(), } } @@ -1584,29 +1602,29 @@ impl CliOptions { } pub fn watch_paths(&self) -> Vec { - let mut paths = if let DenoSubcommand::Run(RunFlags { + let mut full_paths = Vec::new(); + if let DenoSubcommand::Run(RunFlags { watch: Some(WatchFlagsWithPaths { paths, .. }), .. }) = &self.flags.subcommand { - paths.clone() - } else { - Vec::with_capacity(2) - }; + full_paths.extend(paths.iter().map(|path| self.initial_cwd.join(path))); + } + if let Ok(Some(import_map_path)) = self .resolve_specified_import_map_specifier() .map(|ms| ms.and_then(|ref s| s.to_file_path().ok())) { - paths.push(import_map_path); + full_paths.push(import_map_path); } if let Some(specifier) = self.maybe_config_file_specifier() { if specifier.scheme() == "file" { if let Ok(path) = specifier.to_file_path() { - paths.push(path); + full_paths.push(path); } } } - paths + full_paths } } @@ -1784,6 +1802,20 @@ pub fn npm_pkg_req_ref_to_binary_command( binary_name.to_string() } +fn convert_option_str_to_path_buf( + flag: &Option>, + initial_cwd: &Path, +) -> Option> { + if let Some(allow_ffi_paths) = &flag { + let mut full_paths = Vec::new(); + full_paths + .extend(allow_ffi_paths.iter().map(|path| initial_cwd.join(path))); + Some(full_paths) + } else { + None + } +} + #[cfg(test)] mod test { use crate::util::fs::FileCollector; diff --git a/cli/tools/compile.rs b/cli/tools/compile.rs index ed8f07542a..2825c92c7c 100644 --- a/cli/tools/compile.rs +++ b/cli/tools/compile.rs @@ -4,7 +4,6 @@ use crate::args::CompileFlags; use crate::args::Flags; use crate::factory::CliFactory; use crate::standalone::is_standalone_binary; -use crate::util::path::path_has_trailing_slash; use deno_core::anyhow::bail; use deno_core::anyhow::Context; use deno_core::error::generic_error; @@ -174,31 +173,34 @@ async fn resolve_compile_executable_output_path( let module_specifier = resolve_url_or_path(&compile_flags.source_file, current_dir)?; - let mut output = compile_flags.output.clone(); - - if let Some(out) = output.as_ref() { - if path_has_trailing_slash(out) { + let output_flag = compile_flags.output.clone(); + let mut output_path = if let Some(out) = output_flag.as_ref() { + let mut out_path = PathBuf::from(out); + if out.ends_with('/') || out.ends_with('\\') { if let Some(infer_file_name) = infer_name_from_url(&module_specifier) .await .map(PathBuf::from) { - output = Some(out.join(infer_file_name)); + out_path = out_path.join(infer_file_name); } } else { - output = Some(out.to_path_buf()); + out_path = out_path.to_path_buf(); } - } + Some(out_path) + } else { + None + }; - if output.is_none() { - output = infer_name_from_url(&module_specifier) + if output_flag.is_none() { + output_path = infer_name_from_url(&module_specifier) .await .map(PathBuf::from) } - output.ok_or_else(|| generic_error( + output_path.ok_or_else(|| generic_error( "An executable name was not provided. One could not be inferred from the URL. Aborting.", - )).map(|output| { - get_os_specific_filepath(output, &compile_flags.target) + )).map(|output_path| { + get_os_specific_filepath(output_path, &compile_flags.target) }) } @@ -231,7 +233,7 @@ mod test { let path = resolve_compile_executable_output_path( &CompileFlags { source_file: "mod.ts".to_string(), - output: Some(PathBuf::from("./file")), + output: Some(String::from("./file")), args: Vec::new(), target: Some("x86_64-unknown-linux-gnu".to_string()), no_terminal: false, @@ -253,7 +255,7 @@ mod test { let path = resolve_compile_executable_output_path( &CompileFlags { source_file: "mod.ts".to_string(), - output: Some(PathBuf::from("./file")), + output: Some(String::from("./file")), args: Vec::new(), target: Some("x86_64-pc-windows-msvc".to_string()), include: vec![], diff --git a/cli/tools/installer.rs b/cli/tools/installer.rs index 1b1b6f30c1..ea4b5ff489 100644 --- a/cli/tools/installer.rs +++ b/cli/tools/installer.rs @@ -455,7 +455,7 @@ async fn resolve_shim_data( extra_files.push(( copy_path, fs::read_to_string(lock_path) - .with_context(|| format!("error reading {}", lock_path.display()))?, + .with_context(|| format!("error reading {}", lock_path))?, )); } else { // Provide an empty lockfile so that this overwrites any existing lockfile diff --git a/cli/tools/upgrade.rs b/cli/tools/upgrade.rs index 86a271c1fc..6bb4606d3c 100644 --- a/cli/tools/upgrade.rs +++ b/cli/tools/upgrade.rs @@ -379,8 +379,11 @@ pub async fn upgrade( let factory = CliFactory::from_flags(flags)?; let client = factory.http_client(); let current_exe_path = std::env::current_exe()?; + let full_path_output_flag = upgrade_flags + .output + .map(|output| factory.cli_options().initial_cwd().join(output)); let output_exe_path = - upgrade_flags.output.as_ref().unwrap_or(¤t_exe_path); + full_path_output_flag.as_ref().unwrap_or(¤t_exe_path); let permissions = if let Ok(metadata) = fs::metadata(output_exe_path) { let permissions = metadata.permissions(); @@ -430,7 +433,7 @@ pub async fn upgrade( }; if !upgrade_flags.force - && upgrade_flags.output.is_none() + && full_path_output_flag.is_none() && current_is_passed { log::info!("Version {} is already installed", crate::version::deno()); @@ -464,7 +467,7 @@ pub async fn upgrade( }; if !upgrade_flags.force - && upgrade_flags.output.is_none() + && full_path_output_flag.is_none() && current_is_most_recent { log::info!( @@ -520,7 +523,7 @@ pub async fn upgrade( } } else { let output_exe_path = - upgrade_flags.output.as_ref().unwrap_or(¤t_exe_path); + full_path_output_flag.as_ref().unwrap_or(¤t_exe_path); let output_result = if *output_exe_path == current_exe_path { replace_exe(&new_exe_path, output_exe_path) } else { diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index 975cf08f12..2e01d19639 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -40,7 +40,7 @@ pub async fn vendor( ) -> Result<(), AnyError> { let mut cli_options = CliOptions::from_flags(flags)?; let raw_output_dir = match &vendor_flags.output_path { - Some(output_path) => output_path.to_owned(), + Some(output_path) => PathBuf::from(output_path).to_owned(), None => PathBuf::from("vendor/"), }; let output_dir = resolve_from_cwd(&raw_output_dir)?; diff --git a/cli/util/path.rs b/cli/util/path.rs index fed74cb066..b64fde6b9d 100644 --- a/cli/util/path.rs +++ b/cli/util/path.rs @@ -163,20 +163,6 @@ pub fn relative_specifier( }) } -/// This function checks if input path has trailing slash or not. If input path -/// has trailing slash it will return true else it will return false. -pub fn path_has_trailing_slash(path: &Path) -> bool { - if let Some(path_str) = path.to_str() { - if cfg!(windows) { - path_str.ends_with('\\') - } else { - path_str.ends_with('/') - } - } else { - false - } -} - /// Gets a path with the specified file stem suffix. /// /// Ex. `file.ts` with suffix `_2` returns `file_2.ts` @@ -432,31 +418,6 @@ mod test { } } - #[test] - fn test_path_has_trailing_slash() { - #[cfg(not(windows))] - { - run_test("/Users/johndoe/Desktop/deno-project/target/", true); - run_test(r"/Users/johndoe/deno-project/target//", true); - run_test("/Users/johndoe/Desktop/deno-project", false); - run_test(r"/Users/johndoe/deno-project\", false); - } - - #[cfg(windows)] - { - run_test(r"C:\test\deno-project\", true); - run_test(r"C:\test\deno-project\\", true); - run_test(r"C:\test\file.txt", false); - run_test(r"C:\test\file.txt/", false); - } - - fn run_test(path_str: &str, expected: bool) { - let path = Path::new(path_str); - let result = path_has_trailing_slash(path); - assert_eq!(result, expected); - } - } - #[test] fn test_path_with_stem_suffix() { assert_eq!(