refactor: move code from fs.rs into ops/fs.rs (#4428)

This a complex boring PR that shifts around code (primarily) in cli/fs.rs and
cli/ops/fs.rs. The gain of this refactoring is to ease the way for #4188 and
#4017, and also to avoid some future development pain.

Mostly there is no change in functionality. Except:
* squashed bugs where op_utime and op_chown weren't using `resolve_from_cwd`
* eliminated the use of the external `remove_dir_all` crate.
* op_chmod now only queries metadata to verify file/dir exists on Windows (it
  will already fail on Unix if it doesn't)
* op_chown now verifies the file/dir's existence on Windows like chmod does.

1 files changed, 160 insertions, 84 deletions

diff --git a/cli/ops/fs.rs b/cli/ops/fs.rs
index d163c4306..31b22b3e6 100644
--- a/cli/ops/fs.rs
+++ b/cli/ops/fs.rs
@@ -2,23 +2,20 @@
 // Some deserializer fields are only used on Unix and Windows build fails without it
 use super::dispatch_json::{blocking_json, Deserialize, JsonOp, Value};
 use super::io::{FileMetadata, StreamResource, StreamResourceHolder};
-use crate::fs as deno_fs;
+use crate::fs::resolve_from_cwd;
 use crate::op_error::OpError;
 use crate::ops::dispatch_json::JsonResult;
 use crate::state::State;
 use deno_core::*;
 use futures::future::FutureExt;
-use remove_dir_all::remove_dir_all;
-use std;
 use std::convert::From;
-use std::fs;
+use std::env::{current_dir, set_current_dir, temp_dir};
 use std::io::SeekFrom;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::time::UNIX_EPOCH;
 use tokio;
 
-#[cfg(unix)]
-use std::os::unix::fs::{MetadataExt, OpenOptionsExt, PermissionsExt};
+use rand::{thread_rng, Rng};
 
 pub fn init(i: &mut Isolate, s: &State) {
   i.register_op("op_open", s.stateful_json_op(op_open));
@@ -72,16 +69,19 @@ fn op_open(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: OpenArgs = serde_json::from_value(args)?;
-  let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?;
+  let path = resolve_from_cwd(Path::new(&args.path))?;
   let state_ = state.clone();
 
   let mut open_options = if let Some(mode) = args.mode {
     #[allow(unused_mut)]
-    let mut std_options = fs::OpenOptions::new();
+    let mut std_options = std::fs::OpenOptions::new();
     // mode only used if creating the file on Unix
     // if not specified, defaults to 0o666
     #[cfg(unix)]
-    std_options.mode(mode & 0o777);
+    {
+      use std::os::unix::fs::OpenOptionsExt;
+      std_options.mode(mode & 0o777);
+    }
     #[cfg(not(unix))]
     let _ = mode; // avoid unused warning
     tokio::fs::OpenOptions::from(std_options)
@@ -287,7 +287,7 @@ fn op_chdir(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: ChdirArgs = serde_json::from_value(args)?;
-  std::env::set_current_dir(&args.directory)?;
+  set_current_dir(&args.directory)?;
   Ok(JsonOp::Sync(json!({})))
 }
 
@@ -306,15 +306,22 @@ fn op_mkdir(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: MkdirArgs = serde_json::from_value(args)?;
-  let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?;
-  let mode = args.mode.unwrap_or(0o777);
+  let path = resolve_from_cwd(Path::new(&args.path))?;
+  let mode = args.mode.unwrap_or(0o777) & 0o777;
 
   state.check_write(&path)?;
 
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
     debug!("op_mkdir {} {:o} {}", path.display(), mode, args.recursive);
-    deno_fs::mkdir(&path, mode, args.recursive)?;
+    let mut builder = std::fs::DirBuilder::new();
+    builder.recursive(args.recursive);
+    #[cfg(unix)]
+    {
+      use std::os::unix::fs::DirBuilderExt;
+      builder.mode(mode);
+    }
+    builder.create(path)?;
     Ok(json!({}))
   })
 }
@@ -324,7 +331,6 @@ fn op_mkdir(
 struct ChmodArgs {
   promise_id: Option<u64>,
   path: String,
-  #[allow(unused)]
   mode: u32,
 }
 
@@ -334,20 +340,25 @@ fn op_chmod(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: ChmodArgs = serde_json::from_value(args)?;
-  let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?;
+  let path = resolve_from_cwd(Path::new(&args.path))?;
+  let mode = args.mode & 0o777;
 
   state.check_write(&path)?;
 
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
-    debug!("op_chmod {}", path.display());
-    // Still check file/dir exists on windows
-    let _metadata = fs::metadata(&path)?;
+    debug!("op_chmod {} {:o}", path.display(), mode);
     #[cfg(unix)]
     {
-      let mut permissions = _metadata.permissions();
-      permissions.set_mode(args.mode);
-      fs::set_permissions(&path, permissions)?;
+      use std::os::unix::fs::PermissionsExt;
+      let permissions = PermissionsExt::from_mode(mode);
+      std::fs::set_permissions(&path, permissions)?;
+    }
+    // TODO Implement chmod for Windows (#4357)
+    #[cfg(not(unix))]
+    {
+      // Still check file/dir exists on Windows
+      let _metadata = std::fs::metadata(&path)?;
     }
     Ok(json!({}))
   })
@@ -368,15 +379,28 @@ fn op_chown(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: ChownArgs = serde_json::from_value(args)?;
-  let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?;
+  let path = resolve_from_cwd(Path::new(&args.path))?;
 
   state.check_write(&path)?;
 
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
-    debug!("op_chown {}", path.display());
-    deno_fs::chown(args.path.as_ref(), args.uid, args.gid)?;
-    Ok(json!({}))
+    debug!("op_chown {} {} {}", path.display(), args.uid, args.gid);
+    #[cfg(unix)]
+    {
+      use nix::unistd::{chown, Gid, Uid};
+      let nix_uid = Uid::from_raw(args.uid);
+      let nix_gid = Gid::from_raw(args.gid);
+      chown(&path, Option::Some(nix_uid), Option::Some(nix_gid))?;
+      Ok(json!({}))
+    }
+    // TODO Implement chown for Windows
+    #[cfg(not(unix))]
+    {
+      // Still check file/dir exists on Windows
+      let _metadata = std::fs::metadata(&path)?;
+      return Err(OpError::not_implemented());
+    }
   })
 }
 
@@ -394,22 +418,22 @@ fn op_remove(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: RemoveArgs = serde_json::from_value(args)?;
-  let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?;
+  let path = resolve_from_cwd(Path::new(&args.path))?;
   let recursive = args.recursive;
 
   state.check_write(&path)?;
 
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
-    debug!("op_remove {}", path.display());
-    let metadata = fs::symlink_metadata(&path)?;
+    let metadata = std::fs::symlink_metadata(&path)?;
+    debug!("op_remove {} {}", path.display(), recursive);
     let file_type = metadata.file_type();
     if file_type.is_file() || file_type.is_symlink() {
-      fs::remove_file(&path)?;
+      std::fs::remove_file(&path)?;
     } else if recursive {
-      remove_dir_all(&path)?;
+      std::fs::remove_dir_all(&path)?;
     } else {
-      fs::remove_dir(&path)?;
+      std::fs::remove_dir(&path)?;
     }
     Ok(json!({}))
   })
@@ -429,8 +453,8 @@ fn op_copy_file(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: CopyFileArgs = serde_json::from_value(args)?;
-  let from = deno_fs::resolve_from_cwd(Path::new(&args.from))?;
-  let to = deno_fs::resolve_from_cwd(Path::new(&args.to))?;
+  let from = resolve_from_cwd(Path::new(&args.from))?;
+  let to = resolve_from_cwd(Path::new(&args.to))?;
 
   state.check_read(&from)?;
   state.check_write(&to)?;
@@ -446,7 +470,7 @@ fn op_copy_file(
     }
 
     // returns size of from as u64 (we ignore)
-    fs::copy(&from, &to)?;
+    std::fs::copy(&from, &to)?;
     Ok(json!({}))
   })
 }
@@ -463,7 +487,7 @@ macro_rules! to_seconds {
 
 #[inline(always)]
 fn get_stat_json(
-  metadata: fs::Metadata,
+  metadata: std::fs::Metadata,
   maybe_name: Option<String>,
 ) -> JsonResult {
   // Unix stat member (number types only). 0 if not on unix.
@@ -480,6 +504,8 @@ fn get_stat_json(
     }};
   }
 
+  #[cfg(unix)]
+  use std::os::unix::fs::MetadataExt;
   let mut json_val = json!({
     "isFile": metadata.is_file(),
     "isSymlink": metadata.file_type().is_symlink(),
@@ -526,7 +552,7 @@ fn op_stat(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: StatArgs = serde_json::from_value(args)?;
-  let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?;
+  let path = resolve_from_cwd(Path::new(&args.path))?;
   let lstat = args.lstat;
 
   state.check_read(&path)?;
@@ -535,9 +561,9 @@ fn op_stat(
   blocking_json(is_sync, move || {
     debug!("op_stat {} {}", path.display(), lstat);
     let metadata = if lstat {
-      fs::symlink_metadata(&path)?
+      std::fs::symlink_metadata(&path)?
     } else {
-      fs::metadata(&path)?
+      std::fs::metadata(&path)?
     };
     get_stat_json(metadata, None)
   })
@@ -556,7 +582,7 @@ fn op_realpath(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: RealpathArgs = serde_json::from_value(args)?;
-  let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?;
+  let path = resolve_from_cwd(Path::new(&args.path))?;
 
   state.check_read(&path)?;
 
@@ -565,7 +591,7 @@ fn op_realpath(
     debug!("op_realpath {}", path.display());
     // corresponds to the realpath on Unix and
     // CreateFile and GetFinalPathNameByHandle on Windows
-    let realpath = fs::canonicalize(&path)?;
+    let realpath = std::fs::canonicalize(&path)?;
     let mut realpath_str =
       realpath.to_str().unwrap().to_owned().replace("\\", "/");
     if cfg!(windows) {
@@ -588,14 +614,14 @@ fn op_read_dir(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: ReadDirArgs = serde_json::from_value(args)?;
-  let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?;
+  let path = resolve_from_cwd(Path::new(&args.path))?;
 
   state.check_read(&path)?;
 
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
     debug!("op_read_dir {}", path.display());
-    let entries: Vec<_> = fs::read_dir(path)?
+    let entries: Vec<_> = std::fs::read_dir(path)?
       .filter_map(|entry| {
         let entry = entry.unwrap();
         let metadata = entry.metadata().unwrap();
@@ -627,8 +653,8 @@ fn op_rename(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: RenameArgs = serde_json::from_value(args)?;
-  let oldpath = deno_fs::resolve_from_cwd(Path::new(&args.oldpath))?;
-  let newpath = deno_fs::resolve_from_cwd(Path::new(&args.newpath))?;
+  let oldpath = resolve_from_cwd(Path::new(&args.oldpath))?;
+  let newpath = resolve_from_cwd(Path::new(&args.newpath))?;
 
   state.check_read(&oldpath)?;
   state.check_write(&oldpath)?;
@@ -637,7 +663,7 @@ fn op_rename(
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
     debug!("op_rename {} {}", oldpath.display(), newpath.display());
-    fs::rename(&oldpath, &newpath)?;
+    std::fs::rename(&oldpath, &newpath)?;
     Ok(json!({}))
   })
 }
@@ -646,8 +672,8 @@ fn op_rename(
 #[serde(rename_all = "camelCase")]
 struct LinkArgs {
   promise_id: Option<u64>,
-  oldname: String,
-  newname: String,
+  oldpath: String,
+  newpath: String,
 }
 
 fn op_link(
@@ -656,16 +682,16 @@ fn op_link(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: LinkArgs = serde_json::from_value(args)?;
-  let oldname = deno_fs::resolve_from_cwd(Path::new(&args.oldname))?;
-  let newname = deno_fs::resolve_from_cwd(Path::new(&args.newname))?;
+  let oldpath = resolve_from_cwd(Path::new(&args.oldpath))?;
+  let newpath = resolve_from_cwd(Path::new(&args.newpath))?;
 
-  state.check_read(&oldname)?;
-  state.check_write(&newname)?;
+  state.check_read(&oldpath)?;
+  state.check_write(&newpath)?;
 
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
-    debug!("op_link {} {}", oldname.display(), newname.display());
-    std::fs::hard_link(&oldname, &newname)?;
+    debug!("op_link {} {}", oldpath.display(), newpath.display());
+    std::fs::hard_link(&oldpath, &newpath)?;
     Ok(json!({}))
   })
 }
@@ -674,8 +700,8 @@ fn op_link(
 #[serde(rename_all = "camelCase")]
 struct SymlinkArgs {
   promise_id: Option<u64>,
-  oldname: String,
-  newname: String,
+  oldpath: String,
+  newpath: String,
 }
 
 fn op_symlink(
@@ -684,20 +710,28 @@ fn op_symlink(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: SymlinkArgs = serde_json::from_value(args)?;
-  let oldname = deno_fs::resolve_from_cwd(Path::new(&args.oldname))?;
-  let newname = deno_fs::resolve_from_cwd(Path::new(&args.newname))?;
+  let oldpath = resolve_from_cwd(Path::new(&args.oldpath))?;
+  let newpath = resolve_from_cwd(Path::new(&args.newpath))?;
+
+  state.check_write(&newpath)?;
 
-  state.check_write(&newname)?;
-  // TODO Use type for Windows.
-  if cfg!(not(unix)) {
-    return Err(OpError::other("Not implemented".to_string()));
-  }
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
-    debug!("op_symlink {} {}", oldname.display(), newname.display());
+    debug!("op_symlink {} {}", oldpath.display(), newpath.display());
     #[cfg(unix)]
-    std::os::unix::fs::symlink(&oldname, &newname)?;
-    Ok(json!({}))
+    {
+      use std::os::unix::fs::symlink;
+      symlink(&oldpath, &newpath)?;
+      Ok(json!({}))
+    }
+    // TODO Implement symlink, use type for Windows
+    #[cfg(not(unix))]
+    {
+      // Unlike with chmod/chown, here we don't
+      // require `oldpath` to exist on Windows
+      let _ = oldpath; // avoid unused warning
+      return Err(OpError::other("Not implemented".to_string()));
+    }
   })
 }
 
@@ -714,14 +748,14 @@ fn op_read_link(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: ReadLinkArgs = serde_json::from_value(args)?;
-  let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?;
+  let path = resolve_from_cwd(Path::new(&args.path))?;
 
   state.check_read(&path)?;
 
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
     debug!("op_read_link {}", path.display());
-    let path = fs::read_link(&path)?;
+    let path = std::fs::read_link(&path)?;
     let path_str = path.to_str().unwrap();
 
     Ok(json!(path_str))
@@ -742,7 +776,7 @@ fn op_truncate(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: TruncateArgs = serde_json::from_value(args)?;
-  let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?;
+  let path = resolve_from_cwd(Path::new(&args.path))?;
   let len = args.len;
 
   state.check_write(&path)?;
@@ -750,12 +784,57 @@ fn op_truncate(
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
     debug!("op_truncate {} {}", path.display(), len);
-    let f = fs::OpenOptions::new().write(true).open(&path)?;
+    let f = std::fs::OpenOptions::new().write(true).open(&path)?;
     f.set_len(len)?;
     Ok(json!({}))
   })
 }
 
+fn make_temp(
+  dir: Option<&Path>,
+  prefix: Option<&str>,
+  suffix: Option<&str>,
+  is_dir: bool,
+) -> std::io::Result<PathBuf> {
+  let prefix_ = prefix.unwrap_or("");
+  let suffix_ = suffix.unwrap_or("");
+  let mut buf: PathBuf = match dir {
+    Some(ref p) => p.to_path_buf(),
+    None => temp_dir(),
+  }
+  .join("_");
+  let mut rng = thread_rng();
+  loop {
+    let unique = rng.gen::<u32>();
+    buf.set_file_name(format!("{}{:08x}{}", prefix_, unique, suffix_));
+    let r = if is_dir {
+      #[allow(unused_mut)]
+      let mut builder = std::fs::DirBuilder::new();
+      #[cfg(unix)]
+      {
+        use std::os::unix::fs::DirBuilderExt;
+        builder.mode(0o700);
+      }
+      builder.create(buf.as_path())
+    } else {
+      let mut open_options = std::fs::OpenOptions::new();
+      open_options.write(true).create_new(true);
+      #[cfg(unix)]
+      {
+        use std::os::unix::fs::OpenOptionsExt;
+        open_options.mode(0o600);
+      }
+      open_options.open(buf.as_path())?;
+      Ok(())
+    };
+    match r {
+      Err(ref e) if e.kind() == std::io::ErrorKind::AlreadyExists => continue,
+      Ok(_) => return Ok(buf),
+      Err(e) => return Err(e),
+    }
+  }
+}
+
 #[derive(Deserialize)]
 #[serde(rename_all = "camelCase")]
 struct MakeTempArgs {
@@ -772,21 +851,18 @@ fn op_make_temp_dir(
 ) -> Result<JsonOp, OpError> {
   let args: MakeTempArgs = serde_json::from_value(args)?;
 
-  let dir = args
-    .dir
-    .map(|s| deno_fs::resolve_from_cwd(Path::new(&s)).unwrap());
+  let dir = args.dir.map(|s| resolve_from_cwd(Path::new(&s)).unwrap());
   let prefix = args.prefix.map(String::from);
   let suffix = args.suffix.map(String::from);
 
-  state
-    .check_write(dir.clone().unwrap_or_else(std::env::temp_dir).as_path())?;
+  state.check_write(dir.clone().unwrap_or_else(temp_dir).as_path())?;
 
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
     // TODO(piscisaureus): use byte vector for paths, not a string.
     // See https://github.com/denoland/deno/issues/627.
     // We can't assume that paths are always valid utf8 strings.
-    let path = deno_fs::make_temp(
+    let path = make_temp(
       // Converting Option<String> to Option<&str>
       dir.as_ref().map(|x| &**x),
       prefix.as_ref().map(|x| &**x),
@@ -806,21 +882,18 @@ fn op_make_temp_file(
 ) -> Result<JsonOp, OpError> {
   let args: MakeTempArgs = serde_json::from_value(args)?;
 
-  let dir = args
-    .dir
-    .map(|s| deno_fs::resolve_from_cwd(Path::new(&s)).unwrap());
+  let dir = args.dir.map(|s| resolve_from_cwd(Path::new(&s)).unwrap());
   let prefix = args.prefix.map(String::from);
   let suffix = args.suffix.map(String::from);
 
-  state
-    .check_write(dir.clone().unwrap_or_else(std::env::temp_dir).as_path())?;
+  state.check_write(dir.clone().unwrap_or_else(temp_dir).as_path())?;
 
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
     // TODO(piscisaureus): use byte vector for paths, not a string.
     // See https://github.com/denoland/deno/issues/627.
     // We can't assume that paths are always valid utf8 strings.
-    let path = deno_fs::make_temp(
+    let path = make_temp(
       // Converting Option<String> to Option<&str>
       dir.as_ref().map(|x| &**x),
       prefix.as_ref().map(|x| &**x),
@@ -848,7 +921,10 @@ fn op_utime(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: UtimeArgs = serde_json::from_value(args)?;
-  state.check_write(Path::new(&args.path))?;
+  let path = resolve_from_cwd(Path::new(&args.path))?;
+
+  state.check_write(&path)?;
+
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
     debug!("op_utime {} {} {}", args.path, args.atime, args.mtime);
@@ -862,7 +938,7 @@ fn op_cwd(
   _args: Value,
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
-  let path = std::env::current_dir()?;
+  let path = current_dir()?;
   let path_str = path.into_os_string().into_string().unwrap();
   Ok(JsonOp::Sync(json!(path_str)))
 }