internal: make it easier to isolate IO

Author: matklad

crates/paths/src/lib.rs

@@ -3,6 +3,7 @@
 use std::{
     borrow::Borrow,
     convert::{TryFrom, TryInto},
+    ffi::OsStr,
     ops,
     path::{Component, Path, PathBuf},
 };
@@ -97,13 +98,6 @@ impl AbsPathBuf {
 #[repr(transparent)]
 pub struct AbsPath(Path);
 
-impl ops::Deref for AbsPath {
-    type Target = Path;
-    fn deref(&self) -> &Path {
-        &self.0
-    }
-}
-
 impl AsRef<Path> for AbsPath {
     fn as_ref(&self) -> &Path {
         &self.0
@@ -168,6 +162,40 @@ impl AbsPath {
     pub fn strip_prefix(&self, base: &AbsPath) -> Option<&RelPath> {
         self.0.strip_prefix(base).ok().map(RelPath::new_unchecked)
     }
+    pub fn starts_with(&self, base: &AbsPath) -> bool {
+        self.0.starts_with(&base.0)
+    }
+
+    // region:delegate-methods
+
+    // Note that we deliberately don't implement `Deref<Target = Path>` here.
+    //
+    // The problem with `Path` is that it directly exposes convenience IO-ing
+    // methods. For example, `Path::exists` delegates to `fs::metadata`.
+    //
+    // For `AbsPath`, we want to make sure that this is a POD type, and that all
+    // IO goes via `fs`. That way, it becomes easier to mock IO when we need it.
+
+    pub fn file_name(&self) -> Option<&OsStr> {
+        self.0.file_name()
+    }
+    pub fn extension(&self) -> Option<&OsStr> {
+        self.0.extension()
+    }
+    pub fn file_stem(&self) -> Option<&OsStr> {
+        self.0.file_stem()
+    }
+    pub fn as_os_str(&self) -> &OsStr {
+        self.0.as_os_str()
+    }
+    pub fn display(&self) -> std::path::Display<'_> {
+        self.0.display()
+    }
+    #[deprecated(note = "use std::fs::metadata().is_ok() instead")]
+    pub fn exists(&self) -> bool {
+        self.0.exists()
+    }
+    // endregion:delegate-methods
 }
 
 /// Wrapper around a relative [`PathBuf`].
@@ -224,13 +252,6 @@ impl RelPathBuf {
 #[repr(transparent)]
 pub struct RelPath(Path);
 
-impl ops::Deref for RelPath {
-    type Target = Path;
-    fn deref(&self) -> &Path {
-        &self.0
-    }
-}
-
 impl AsRef<Path> for RelPath {
     fn as_ref(&self) -> &Path {
         &self.0

crates/proc_macro_api/src/lib.rs

@@ -86,6 +86,7 @@ impl ProcMacroClient {
         Ok(ProcMacroClient { process: Arc::new(Mutex::new(process)) })
     }
 
+    // TODO: use paths::AbsPath here
     pub fn by_dylib_path(&self, dylib_path: &Path) -> Vec<ProcMacro> {
         let _p = profile::span("ProcMacroClient::by_dylib_path");
         match version::read_dylib_info(dylib_path) {

crates/project_model/src/build_data.rs

@@ -258,7 +258,7 @@ impl WorkspaceBuildData {
             inject_cargo_env(package, package_build_data);
             if let Some(out_dir) = &package_build_data.out_dir {
                 // NOTE: cargo and rustc seem to hide non-UTF-8 strings from env! and option_env!()
-                if let Some(out_dir) = out_dir.to_str().map(|s| s.to_owned()) {
+                if let Some(out_dir) = out_dir.as_os_str().to_str().map(|s| s.to_owned()) {
                     package_build_data.envs.push(("OUT_DIR".to_string(), out_dir));
                 }
             }

crates/project_model/src/cargo_workspace.rs

@@ -273,11 +273,11 @@ impl CargoWorkspace {
                 .parent()
                 .map(|p| p.to_path_buf())
                 .or(cwd)
-                .map(|dir| dir.to_string_lossy().to_string())
-                .unwrap_or_else(|| "<failed to get path>".into());
+                .map(|dir| format!(" in `{}`", dir.display()))
+                .unwrap_or_default();
 
             format!(
-                "Failed to run `cargo metadata --manifest-path {}` in `{}`",
+                "Failed to run `cargo metadata --manifest-path {}`{}",
                 cargo_toml.display(),
                 workdir
             )

crates/project_model/src/lib.rs

@@ -24,7 +24,7 @@ mod rustc_cfg;
 mod build_data;
 
 use std::{
-    fs::{read_dir, ReadDir},
+    fs::{self, read_dir, ReadDir},
     io,
     process::Command,
 };
@@ -54,10 +54,10 @@ pub enum ProjectManifest {
 
 impl ProjectManifest {
     pub fn from_manifest_file(path: AbsPathBuf) -> Result<ProjectManifest> {
-        if path.ends_with("rust-project.json") {
+        if path.file_name().unwrap_or_default() == "rust-project.json" {
             return Ok(ProjectManifest::ProjectJson(path));
         }
-        if path.ends_with("Cargo.toml") {
+        if path.file_name().unwrap_or_default() == "Cargo.toml" {
             return Ok(ProjectManifest::CargoToml(path));
         }
         bail!("project root must point to Cargo.toml or rust-project.json: {}", path.display())
@@ -91,15 +91,15 @@ impl ProjectManifest {
         }
 
         fn find_in_parent_dirs(path: &AbsPath, target_file_name: &str) -> Option<AbsPathBuf> {
-            if path.ends_with(target_file_name) {
+            if path.file_name().unwrap_or_default() == target_file_name {
                 return Some(path.to_path_buf());
             }
 
             let mut curr = Some(path);
 
             while let Some(path) = curr {
                 let candidate = path.join(target_file_name);
-                if candidate.exists() {
+                if fs::metadata(&candidate).is_ok() {
                     return Some(candidate);
                 }
                 curr = path.parent();

crates/project_model/src/sysroot.rs

@@ -4,7 +4,7 @@
 //! but we can't process `.rlib` and need source code instead. The source code
 //! is typically installed with `rustup component add rust-src` command.
 
-use std::{convert::TryFrom, env, ops, path::PathBuf, process::Command};
+use std::{convert::TryFrom, env, fs, ops, path::PathBuf, process::Command};
 
 use anyhow::{format_err, Result};
 use la_arena::{Arena, Idx};
@@ -73,7 +73,7 @@ impl Sysroot {
             let root = [format!("{}/src/lib.rs", path), format!("lib{}/lib.rs", path)]
                 .iter()
                 .map(|it| sysroot_src_dir.join(it))
-                .find(|it| it.exists());
+                .find(|it| fs::metadata(it).is_ok());
 
             if let Some(root) = root {
                 sysroot.crates.alloc(SysrootCrateData {
@@ -142,7 +142,7 @@ fn discover_sysroot_src_dir(
         let path = AbsPathBuf::try_from(path.as_str())
             .map_err(|path| format_err!("RUST_SRC_PATH must be absolute: {}", path.display()))?;
         let core = path.join("core");
-        if core.exists() {
+        if fs::metadata(&core).is_ok() {
             log::debug!("Discovered sysroot by RUST_SRC_PATH: {}", path.display());
             return Ok(path);
         }
@@ -171,7 +171,7 @@ try installing the Rust source the same way you installed rustc",
 fn get_rustc_src(sysroot_path: &AbsPath) -> Option<AbsPathBuf> {
     let rustc_src = sysroot_path.join("lib/rustlib/rustc-src/rust/compiler/rustc/Cargo.toml");
     log::debug!("Checking for rustc source code: {}", rustc_src.display());
-    if rustc_src.exists() {
+    if fs::metadata(&rustc_src).is_ok() {
         Some(rustc_src)
     } else {
         None
@@ -182,7 +182,7 @@ fn get_rust_src(sysroot_path: &AbsPath) -> Option<AbsPathBuf> {
     // Try the new path first since the old one still exists.
     let rust_src = sysroot_path.join("lib/rustlib/src/rust");
     log::debug!("Checking sysroot (looking for `library` and `src` dirs): {}", rust_src.display());
-    ["library", "src"].iter().map(|it| rust_src.join(it)).find(|it| it.exists())
+    ["library", "src"].iter().map(|it| rust_src.join(it)).find(|it| fs::metadata(it).is_ok())
 }
 
 impl SysrootCrateData {

crates/project_model/src/workspace.rs

@@ -2,7 +2,7 @@
 //! metadata` or `rust-project.json`) into representation stored in the salsa
 //! database -- `CrateGraph`.
 
-use std::{collections::VecDeque, fmt, fs, path::Path, process::Command};
+use std::{collections::VecDeque, fmt, fs, process::Command};
 
 use anyhow::{format_err, Context, Result};
 use base_db::{CrateDisplayName, CrateGraph, CrateId, CrateName, Edition, Env, FileId, ProcMacro};
@@ -311,8 +311,8 @@ impl ProjectWorkspace {
         load: &mut dyn FnMut(&AbsPath) -> Option<FileId>,
     ) -> CrateGraph {
         let _p = profile::span("ProjectWorkspace::to_crate_graph");
-        let proc_macro_loader = |path: &Path| match proc_macro_client {
-            Some(client) => client.by_dylib_path(path),
+        let proc_macro_loader = |path: &AbsPath| match proc_macro_client {
+            Some(client) => client.by_dylib_path(path.as_ref()), // TODO
             None => Vec::new(),
         };
 
@@ -364,7 +364,7 @@ impl ProjectWorkspace {
 
 fn project_json_to_crate_graph(
     rustc_cfg: Vec<CfgFlag>,
-    proc_macro_loader: &dyn Fn(&Path) -> Vec<ProcMacro>,
+    proc_macro_loader: &dyn Fn(&AbsPath) -> Vec<ProcMacro>,
     load: &mut dyn FnMut(&AbsPath) -> Option<FileId>,
     project: &ProjectJson,
     sysroot: &Option<Sysroot>,
@@ -431,7 +431,7 @@ fn project_json_to_crate_graph(
 fn cargo_to_crate_graph(
     rustc_cfg: Vec<CfgFlag>,
     override_cfg: &CfgOverrides,
-    proc_macro_loader: &dyn Fn(&Path) -> Vec<ProcMacro>,
+    proc_macro_loader: &dyn Fn(&AbsPath) -> Vec<ProcMacro>,
     load: &mut dyn FnMut(&AbsPath) -> Option<FileId>,
     cargo: &CargoWorkspace,
     build_data_map: Option<&WorkspaceBuildData>,
@@ -616,7 +616,7 @@ fn handle_rustc_crates(
     crate_graph: &mut CrateGraph,
     rustc_build_data_map: Option<&WorkspaceBuildData>,
     cfg_options: &CfgOptions,
-    proc_macro_loader: &dyn Fn(&Path) -> Vec<ProcMacro>,
+    proc_macro_loader: &dyn Fn(&AbsPath) -> Vec<ProcMacro>,
     pkg_to_lib_crate: &mut FxHashMap<la_arena::Idx<crate::PackageData>, CrateId>,
     public_deps: &[(CrateName, CrateId)],
     cargo: &CargoWorkspace,
@@ -708,7 +708,7 @@ fn add_target_crate_root(
     pkg: &cargo_workspace::PackageData,
     build_data: Option<&PackageBuildData>,
     cfg_options: &CfgOptions,
-    proc_macro_loader: &dyn Fn(&Path) -> Vec<ProcMacro>,
+    proc_macro_loader: &dyn Fn(&AbsPath) -> Vec<ProcMacro>,
     file_id: FileId,
     cargo_name: &str,
 ) -> CrateId {

crates/vfs/src/vfs_path.rs

@@ -121,6 +121,7 @@ impl VfsPath {
                 #[cfg(windows)]
                 {
                     use windows_paths::Encode;
+                    let path: &std::path::Path = path.as_ref();
                     let components = path.components();
                     let mut add_sep = false;
                     for component in components {