Skip to content

Commit

Permalink
refactor: Use PathBuf for paths in flag parsing and whitelists (#3955)
Browse files Browse the repository at this point in the history 
* Use PathBuf for DenoSubcommand::Bundle's out_file
* Use PathBuf for DenoSubcommand::Format's files
* Use PathBuf for DenoSubcommand::Install's dir
* Use PathBuf for read/write whitelists
  • Loading branch information
@nayeemrmn
nayeemrmn authored Feb 11, 2020
1 parent 79b3bc0 commit 701ce9b
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 67 deletions.
6 changes: 3 additions & 3 deletions cli/compilers/ts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ fn req(
request_type: msg::CompilerRequestType,
root_names: Vec<String>,
compiler_config: CompilerConfig,
out_file: Option<String>,
out_file: Option<PathBuf>,
target: &str,
bundle: bool,
) -> Buf {
Expand Down Expand Up @@ -275,7 +275,7 @@ impl TsCompiler {
&self,
global_state: GlobalState,
module_name: String,
out_file: Option<String>,
out_file: Option<PathBuf>,
) -> Result<(), ErrBox> {
debug!(
"Invoking the compiler to bundle. module_name: {}",
Expand Down Expand Up @@ -743,7 +743,7 @@ mod tests {
.bundle_async(
state.clone(),
module_name,
Some(String::from("$deno$/bundle.js")),
Some(PathBuf::from("$deno$/bundle.js")),
)
.await;
assert!(result.is_ok());
Expand Down
81 changes: 39 additions & 42 deletions cli/flags.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use clap::ArgMatches;
use clap::SubCommand;
use log::Level;
use std::collections::HashSet;
use std::path::Path;
use std::path::{Path, PathBuf};

/// Creates vector of strings, Vec<String>
macro_rules! svec {
Expand Down Expand Up @@ -35,7 +35,7 @@ const TEST_RUNNER_URL: &str = std_url!("testing/runner.ts");
pub enum DenoSubcommand {
Bundle {
source_file: String,
out_file: Option<String>,
out_file: Option<PathBuf>,
},
Completions {
buf: Box<[u8]>,
Expand All @@ -48,14 +48,14 @@ pub enum DenoSubcommand {
},
Format {
check: bool,
files: Option<Vec<String>>,
files: Option<Vec<PathBuf>>,
},
Help,
Info {
file: Option<String>,
},
Install {
dir: Option<String>,
dir: Option<PathBuf>,
exe_name: String,
module_url: String,
args: Vec<String>,
Expand Down Expand Up @@ -87,10 +87,10 @@ pub struct DenoFlags {
pub config_path: Option<String>,
pub import_map_path: Option<String>,
pub allow_read: bool,
pub read_whitelist: Vec<String>,
pub read_whitelist: Vec<PathBuf>,
pub cache_blacklist: Vec<String>,
pub allow_write: bool,
pub write_whitelist: Vec<String>,
pub write_whitelist: Vec<PathBuf>,
pub allow_net: bool,
pub net_whitelist: Vec<String>,
pub allow_env: bool,
Expand All @@ -107,14 +107,22 @@ pub struct DenoFlags {
pub lock_write: bool,
}

fn join_paths(whitelist: &[PathBuf], d: &str) -> String {
whitelist
.iter()
.map(|path| path.to_str().unwrap().to_string())
.collect::<Vec<String>>()
.join(d)
}

impl DenoFlags {
/// Return list of permission arguments that are equivalent
/// to the ones used to create `self`.
pub fn to_permission_args(&self) -> Vec<String> {
let mut args = vec![];

if !self.read_whitelist.is_empty() {
let s = format!("--allow-read={}", self.read_whitelist.join(","));
let s = format!("--allow-read={}", join_paths(&self.read_whitelist, ","));
args.push(s);
}

Expand All @@ -123,7 +131,8 @@ impl DenoFlags {
}

if !self.write_whitelist.is_empty() {
let s = format!("--allow-write={}", self.write_whitelist.join(","));
let s =
format!("--allow-write={}", join_paths(&self.write_whitelist, ","));
args.push(s);
}

Expand Down Expand Up @@ -297,7 +306,7 @@ fn types_parse(flags: &mut DenoFlags, _matches: &clap::ArgMatches) {
fn fmt_parse(flags: &mut DenoFlags, matches: &clap::ArgMatches) {
let maybe_files = match matches.values_of("files") {
Some(f) => {
let files: Vec<String> = f.map(String::from).collect();
let files: Vec<PathBuf> = f.map(PathBuf::from).collect();
Some(files)
}
None => None,
Expand All @@ -316,7 +325,7 @@ fn install_parse(flags: &mut DenoFlags, matches: &clap::ArgMatches) {

let dir = if matches.is_present("dir") {
let install_dir = matches.value_of("dir").unwrap();
Some(install_dir.to_string())
Some(PathBuf::from(install_dir))
} else {
None
};
Expand Down Expand Up @@ -347,7 +356,7 @@ fn bundle_parse(flags: &mut DenoFlags, matches: &clap::ArgMatches) {

let out_file = if let Some(out_file) = matches.value_of("out_file") {
flags.allow_write = true;
Some(out_file.to_string())
Some(PathBuf::from(out_file))
} else {
None
};
Expand Down Expand Up @@ -428,16 +437,10 @@ fn lock_args_parse(flags: &mut DenoFlags, matches: &clap::ArgMatches) {
}
}

fn resolve_fs_whitelist(whitelist: &[String]) -> Vec<String> {
fn resolve_fs_whitelist(whitelist: &[PathBuf]) -> Vec<PathBuf> {
whitelist
.iter()
.map(|raw_path| {
resolve_from_cwd(Path::new(&raw_path))
.unwrap()
.to_str()
.unwrap()
.to_owned()
})
.map(|raw_path| resolve_from_cwd(Path::new(&raw_path)).unwrap())
.collect()
}

Expand Down Expand Up @@ -998,8 +1001,8 @@ fn permission_args_parse(flags: &mut DenoFlags, matches: &clap::ArgMatches) {
if matches.is_present("allow-read") {
if matches.value_of("allow-read").is_some() {
let read_wl = matches.values_of("allow-read").unwrap();
let raw_read_whitelist: Vec<String> =
read_wl.map(std::string::ToString::to_string).collect();
let raw_read_whitelist: Vec<PathBuf> =
read_wl.map(PathBuf::from).collect();
flags.read_whitelist = resolve_fs_whitelist(&raw_read_whitelist);
debug!("read whitelist: {:#?}", &flags.read_whitelist);
} else {
Expand All @@ -1009,10 +1012,9 @@ fn permission_args_parse(flags: &mut DenoFlags, matches: &clap::ArgMatches) {
if matches.is_present("allow-write") {
if matches.value_of("allow-write").is_some() {
let write_wl = matches.values_of("allow-write").unwrap();
let raw_write_whitelist: Vec<String> =
write_wl.map(std::string::ToString::to_string).collect();
flags.write_whitelist =
resolve_fs_whitelist(raw_write_whitelist.as_slice());
let raw_write_whitelist: Vec<PathBuf> =
write_wl.map(PathBuf::from).collect();
flags.write_whitelist = resolve_fs_whitelist(&raw_write_whitelist);
debug!("write whitelist: {:#?}", &flags.write_whitelist);
} else {
flags.allow_write = true;
Expand Down Expand Up @@ -1376,7 +1378,10 @@ mod tests {
DenoFlags {
subcommand: DenoSubcommand::Format {
check: false,
files: Some(svec!["script_1.ts", "script_2.ts"])
files: Some(vec![
PathBuf::from("script_1.ts"),
PathBuf::from("script_2.ts")
])
},
..DenoFlags::default()
}
Expand Down Expand Up @@ -1544,23 +1549,19 @@ mod tests {
#[test]
fn allow_read_whitelist() {
use tempfile::TempDir;
let temp_dir = TempDir::new().expect("tempdir fail");
let temp_dir_path = temp_dir.path().to_str().unwrap();
let temp_dir = TempDir::new().expect("tempdir fail").path().to_path_buf();

let r = flags_from_vec_safe(svec![
"deno",
"run",
format!("--allow-read=.,{}", &temp_dir_path),
format!("--allow-read=.,{}", temp_dir.to_str().unwrap()),
"script.ts"
]);
assert_eq!(
r.unwrap(),
DenoFlags {
allow_read: false,
read_whitelist: svec![
current_dir().unwrap().to_str().unwrap().to_owned(),
&temp_dir_path
],
read_whitelist: vec![current_dir().unwrap(), temp_dir],
subcommand: DenoSubcommand::Run {
script: "script.ts".to_string(),
},
Expand All @@ -1572,23 +1573,19 @@ mod tests {
#[test]
fn allow_write_whitelist() {
use tempfile::TempDir;
let temp_dir = TempDir::new().expect("tempdir fail");
let temp_dir_path = temp_dir.path().to_str().unwrap();
let temp_dir = TempDir::new().expect("tempdir fail").path().to_path_buf();

let r = flags_from_vec_safe(svec![
"deno",
"run",
format!("--allow-write=.,{}", &temp_dir_path),
format!("--allow-write=.,{}", temp_dir.to_str().unwrap()),
"script.ts"
]);
assert_eq!(
r.unwrap(),
DenoFlags {
allow_write: false,
write_whitelist: svec![
current_dir().unwrap().to_str().unwrap().to_owned(),
&temp_dir_path
],
write_whitelist: vec![current_dir().unwrap(), temp_dir],
subcommand: DenoSubcommand::Run {
script: "script.ts".to_string(),
},
Expand Down Expand Up @@ -1677,7 +1674,7 @@ mod tests {
DenoFlags {
subcommand: DenoSubcommand::Bundle {
source_file: "source.ts".to_string(),
out_file: Some("bundle.js".to_string()),
out_file: Some(PathBuf::from("bundle.js")),
},
allow_write: true,
..DenoFlags::default()
Expand Down Expand Up @@ -1868,7 +1865,7 @@ mod tests {
r.unwrap(),
DenoFlags {
subcommand: DenoSubcommand::Install {
dir: Some("/usr/local/bin".to_string()),
dir: Some(PathBuf::from("/usr/local/bin")),
exe_name: "file_server".to_string(),
module_url: "https://deno.land/std/http/file_server.ts".to_string(),
args: svec!["arg1", "arg2"],
Expand Down
10 changes: 5 additions & 5 deletions cli/fmt.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,11 @@ fn format_source_files(
);
}

fn get_matching_files(glob_paths: Vec<String>) -> Vec<PathBuf> {
fn get_matching_files(glob_paths: Vec<PathBuf>) -> Vec<PathBuf> {
let mut target_files = Vec::with_capacity(128);

for path in glob_paths {
let files = glob::glob(&path)
let files = glob::glob(&path.to_str().unwrap())
.expect("Failed to execute glob.")
.filter_map(Result::ok);
target_files.extend(files);
Expand All @@ -165,14 +165,14 @@ fn get_matching_files(glob_paths: Vec<String>) -> Vec<PathBuf> {
/// First argument supports globs, and if it is `None`
/// then the current directory is recursively walked.
pub fn format_files(
maybe_files: Option<Vec<String>>,
maybe_files: Option<Vec<PathBuf>>,
check: bool,
) -> Result<(), ErrBox> {
// TODO: improve glob to look for tsx?/jsx? files only
let glob_paths = maybe_files.unwrap_or_else(|| vec!["**/*".to_string()]);
let glob_paths = maybe_files.unwrap_or_else(|| vec![PathBuf::from("**/*")]);

for glob_path in glob_paths.iter() {
if glob_path == "-" {
if glob_path.to_str().unwrap() == "-" {
// If the only given path is '-', format stdin.
if glob_paths.len() == 1 {
format_stdin(check);
Expand Down
10 changes: 5 additions & 5 deletions cli/installer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,14 @@ fn get_installer_dir() -> Result<PathBuf, Error> {

pub fn install(
flags: DenoFlags,
installation_dir: Option<String>,
installation_dir: Option<PathBuf>,
exec_name: &str,
module_url: &str,
args: Vec<String>,
force: bool,
) -> Result<(), Error> {
let installation_dir = if let Some(dir) = installation_dir {
PathBuf::from(dir).canonicalize()?
dir.canonicalize()?
} else {
get_installer_dir()?
};
Expand Down Expand Up @@ -245,7 +245,7 @@ mod tests {
let temp_dir = TempDir::new().expect("tempdir fail");
install(
DenoFlags::default(),
Some(temp_dir.path().to_string_lossy().to_string()),
Some(temp_dir.path().to_path_buf()),
"echo_test",
"http://localhost:4545/cli/tests/echo_server.ts",
vec![],
Expand Down Expand Up @@ -274,7 +274,7 @@ mod tests {
allow_read: true,
..DenoFlags::default()
},
Some(temp_dir.path().to_string_lossy().to_string()),
Some(temp_dir.path().to_path_buf()),
"echo_test",
"http://localhost:4545/cli/tests/echo_server.ts",
vec!["--foobar".to_string()],
Expand All @@ -301,7 +301,7 @@ mod tests {

install(
DenoFlags::default(),
Some(temp_dir.path().to_string_lossy().to_string()),
Some(temp_dir.path().to_path_buf()),
"echo_test",
&local_module_str,
vec![],
Expand Down
7 changes: 4 additions & 3 deletions cli/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ use log::Level;
use log::Metadata;
use log::Record;
use std::env;
use std::path::PathBuf;

static LOGGER: Logger = Logger;

Expand Down Expand Up @@ -258,7 +259,7 @@ async fn info_command(flags: DenoFlags, file: Option<String>) {

async fn install_command(
flags: DenoFlags,
dir: Option<String>,
dir: Option<PathBuf>,
exe_name: String,
module_url: String,
args: Vec<String>,
Expand Down Expand Up @@ -331,7 +332,7 @@ async fn eval_command(flags: DenoFlags, code: String) {
async fn bundle_command(
flags: DenoFlags,
source_file: String,
out_file: Option<String>,
out_file: Option<PathBuf>,
) {
debug!(">>>>> bundle_async START");
let source_file_specifier =
Expand Down Expand Up @@ -404,7 +405,7 @@ async fn run_script(flags: DenoFlags, script: String) {
js_check(worker.execute("window.dispatchEvent(new Event('unload'))"));
}

async fn fmt_command(files: Option<Vec<String>>, check: bool) {
async fn fmt_command(files: Option<Vec<PathBuf>>, check: bool) {
if let Err(err) = fmt::format_files(files, check) {
print_err_and_exit(err);
}
Expand Down
Loading

0 comments on commit 701ce9b

Please sign in to comment.