Merge #9620

9620: internal: don't depend on cwd r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <[email protected]>

Author: bors[bot]

Cargo.lock

@@ -18,3 +18,4 @@ jod-thread = "0.1.1"
 
 toolchain = { path = "../toolchain", version = "0.0.0" }
 stdx = { path = "../stdx", version = "0.0.0" }
+paths = { path = "../paths", version = "0.0.0" }

crates/flycheck/Cargo.toml

@@ -5,12 +5,12 @@
 use std::{
     fmt,
     io::{self, BufRead, BufReader},
-    path::PathBuf,
     process::{self, Command, Stdio},
     time::Duration,
 };
 
 use crossbeam_channel::{never, select, unbounded, Receiver, Sender};
+use paths::AbsPathBuf;
 use serde::Deserialize;
 use stdx::JodChild;
 
@@ -63,7 +63,7 @@ impl FlycheckHandle {
         id: usize,
         sender: Box<dyn Fn(Message) + Send>,
         config: FlycheckConfig,
-        workspace_root: PathBuf,
+        workspace_root: AbsPathBuf,
     ) -> FlycheckHandle {
         let actor = FlycheckActor::new(id, sender, config, workspace_root);
         let (sender, receiver) = unbounded::<Restart>();
@@ -82,7 +82,7 @@ impl FlycheckHandle {
 
 pub enum Message {
     /// Request adding a diagnostic with fixes included to a file
-    AddDiagnostic { workspace_root: PathBuf, diagnostic: Diagnostic },
+    AddDiagnostic { workspace_root: AbsPathBuf, diagnostic: Diagnostic },
 
     /// Request check progress notification to client
     Progress {
@@ -121,7 +121,7 @@ struct FlycheckActor {
     id: usize,
     sender: Box<dyn Fn(Message) + Send>,
     config: FlycheckConfig,
-    workspace_root: PathBuf,
+    workspace_root: AbsPathBuf,
     /// WatchThread exists to wrap around the communication needed to be able to
     /// run `cargo check` without blocking. Currently the Rust standard library
     /// doesn't provide a way to read sub-process output without blocking, so we
@@ -140,7 +140,7 @@ impl FlycheckActor {
         id: usize,
         sender: Box<dyn Fn(Message) + Send>,
         config: FlycheckConfig,
-        workspace_root: PathBuf,
+        workspace_root: AbsPathBuf,
     ) -> FlycheckActor {
         FlycheckActor { id, sender, config, workspace_root, cargo_handle: None }
     }
@@ -220,7 +220,7 @@ impl FlycheckActor {
                 cmd.arg(command);
                 cmd.current_dir(&self.workspace_root);
                 cmd.args(&["--workspace", "--message-format=json", "--manifest-path"])
-                    .arg(self.workspace_root.join("Cargo.toml"));
+                    .arg(self.workspace_root.join("Cargo.toml").as_os_str());
 
                 if let Some(target) = target_triple {
                     cmd.args(&["--target", target.as_str()]);

crates/flycheck/src/lib.rs

@@ -7,3 +7,6 @@ edition = "2018"
 
 [lib]
 doctest = false
+
+[dependencies]
+serde = "1"

crates/paths/Cargo.toml

@@ -3,6 +3,7 @@
 use std::{
     borrow::Borrow,
     convert::{TryFrom, TryInto},
+    ffi::OsStr,
     ops,
     path::{Component, Path, PathBuf},
 };
@@ -65,6 +66,27 @@ impl PartialEq<AbsPath> for AbsPathBuf {
     }
 }
 
+impl serde::Serialize for AbsPathBuf {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        self.0.serialize(serializer)
+    }
+}
+
+impl<'de> serde::Deserialize<'de> for AbsPathBuf {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        let path = PathBuf::deserialize(deserializer)?;
+        AbsPathBuf::try_from(path).map_err(|path| {
+            serde::de::Error::custom(format!("expected absolute path, got {}", path.display()))
+        })
+    }
+}
+
 impl AbsPathBuf {
     /// Wrap the given absolute path in `AbsPathBuf`
     ///
@@ -97,13 +119,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 +183,43 @@ 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)
+    }
+    pub fn ends_with(&self, suffix: &RelPath) -> bool {
+        self.0.starts_with(&suffix.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 +276,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/paths/src/lib.rs

@@ -18,6 +18,7 @@ memmap2 = "0.3.0"
 object = { version = "0.25.3", default-features = false, features = ["std", "read_core", "elf", "macho", "pe"] }
 snap = "1.0"
 
+paths = { path = "../paths", version = "0.0.0" }
 tt = { path = "../tt", version = "0.0.0" }
 base_db = { path = "../base_db", version = "0.0.0" }
 stdx = { path = "../stdx", version = "0.0.0" }

crates/proc_macro_api/Cargo.toml

@@ -11,10 +11,10 @@ mod rpc;
 mod version;
 
 use base_db::{Env, ProcMacro};
+use paths::{AbsPath, AbsPathBuf};
 use std::{
     ffi::OsStr,
     io,
-    path::{Path, PathBuf},
     sync::{Arc, Mutex},
 };
 
@@ -28,7 +28,7 @@ pub use version::{read_dylib_info, RustCInfo};
 #[derive(Debug, Clone)]
 struct ProcMacroProcessExpander {
     process: Arc<Mutex<ProcMacroProcessSrv>>,
-    dylib_path: PathBuf,
+    dylib_path: AbsPathBuf,
     name: SmolStr,
 }
 
@@ -79,25 +79,25 @@ pub struct ProcMacroClient {
 impl ProcMacroClient {
     /// Spawns an external process as the proc macro server and returns a client connected to it.
     pub fn extern_process(
-        process_path: PathBuf,
+        process_path: AbsPathBuf,
         args: impl IntoIterator<Item = impl AsRef<OsStr>>,
     ) -> io::Result<ProcMacroClient> {
         let process = ProcMacroProcessSrv::run(process_path, args)?;
         Ok(ProcMacroClient { process: Arc::new(Mutex::new(process)) })
     }
 
-    pub fn by_dylib_path(&self, dylib_path: &Path) -> Vec<ProcMacro> {
+    pub fn by_dylib_path(&self, dylib_path: &AbsPath) -> Vec<ProcMacro> {
         let _p = profile::span("ProcMacroClient::by_dylib_path");
         match version::read_dylib_info(dylib_path) {
             Ok(info) => {
                 if info.version.0 < 1 || info.version.1 < 47 {
-                    eprintln!("proc-macro {} built by {:#?} is not supported by Rust Analyzer, please update your rust version.", dylib_path.to_string_lossy(), info);
+                    eprintln!("proc-macro {} built by {:#?} is not supported by Rust Analyzer, please update your rust version.", dylib_path.display(), info);
                 }
             }
             Err(err) => {
                 eprintln!(
                     "proc-macro {} failed to find the given version. Reason: {}",
-                    dylib_path.to_string_lossy(),
+                    dylib_path.display(),
                     err
                 );
             }
@@ -128,7 +128,7 @@ impl ProcMacroClient {
                 let expander = Arc::new(ProcMacroProcessExpander {
                     process: self.process.clone(),
                     name: name.clone(),
-                    dylib_path: dylib_path.into(),
+                    dylib_path: dylib_path.to_path_buf(),
                 });
 
                 ProcMacro { name, kind, expander }

crates/proc_macro_api/src/lib.rs

@@ -4,10 +4,10 @@ use std::{
     convert::{TryFrom, TryInto},
     ffi::{OsStr, OsString},
     io::{self, BufRead, BufReader, Write},
-    path::{Path, PathBuf},
     process::{Child, ChildStdin, ChildStdout, Command, Stdio},
 };
 
+use paths::{AbsPath, AbsPathBuf};
 use stdx::JodChild;
 
 use crate::{
@@ -24,7 +24,7 @@ pub(crate) struct ProcMacroProcessSrv {
 
 impl ProcMacroProcessSrv {
     pub(crate) fn run(
-        process_path: PathBuf,
+        process_path: AbsPathBuf,
         args: impl IntoIterator<Item = impl AsRef<OsStr>>,
     ) -> io::Result<ProcMacroProcessSrv> {
         let mut process = Process::run(process_path, args)?;
@@ -37,7 +37,7 @@ impl ProcMacroProcessSrv {
 
     pub(crate) fn find_proc_macros(
         &mut self,
-        dylib_path: &Path,
+        dylib_path: &AbsPath,
     ) -> Result<Vec<(String, ProcMacroKind)>, tt::ExpansionError> {
         let task = ListMacrosTask { lib: dylib_path.to_path_buf() };
 
@@ -84,7 +84,7 @@ struct Process {
 
 impl Process {
     fn run(
-        path: PathBuf,
+        path: AbsPathBuf,
         args: impl IntoIterator<Item = impl AsRef<OsStr>>,
     ) -> io::Result<Process> {
         let args: Vec<OsString> = args.into_iter().map(|s| s.as_ref().into()).collect();
@@ -101,8 +101,11 @@ impl Process {
     }
 }
 
-fn mk_child(path: &Path, args: impl IntoIterator<Item = impl AsRef<OsStr>>) -> io::Result<Child> {
-    Command::new(&path)
+fn mk_child(
+    path: &AbsPath,
+    args: impl IntoIterator<Item = impl AsRef<OsStr>>,
+) -> io::Result<Child> {
+    Command::new(path.as_os_str())
         .args(args)
         .stdin(Stdio::piped())
         .stdout(Stdio::piped())

crates/proc_macro_api/src/process.rs

@@ -6,8 +6,7 @@
 //! to be much easier, we deliberately duplicate `tt` structs with `#[serde(with = "XXDef")]`
 //! for separation of code responsibility.
 
-use std::path::PathBuf;
-
+use paths::AbsPathBuf;
 use serde::{Deserialize, Serialize};
 use tt::{
     Delimiter, DelimiterKind, Ident, Leaf, Literal, Punct, SmolStr, Spacing, Subtree, TokenId,
@@ -16,7 +15,7 @@ use tt::{
 
 #[derive(Clone, Eq, PartialEq, Debug, Serialize, Deserialize)]
 pub struct ListMacrosTask {
-    pub lib: PathBuf,
+    pub lib: AbsPathBuf,
 }
 
 #[derive(Copy, Clone, Eq, PartialEq, Debug, Serialize, Deserialize)]
@@ -50,7 +49,7 @@ pub struct ExpansionTask {
     #[serde(with = "opt_subtree_def")]
     pub attributes: Option<Subtree>,
 
-    pub lib: PathBuf,
+    pub lib: AbsPathBuf,
 
     /// Environment variables to set during macro expansion.
     pub env: Vec<(String, String)>,
@@ -272,7 +271,7 @@ mod tests {
             macro_body: tt.clone(),
             macro_name: Default::default(),
             attributes: None,
-            lib: Default::default(),
+            lib: AbsPathBuf::assert(std::env::current_dir().unwrap()),
             env: Default::default(),
         };