internal: a bit more of cwd safety for flycheck

Cargo.lock

@@ -381,6 +381,7 @@ dependencies = [
  "crossbeam-channel",
  "jod-thread",
  "log",
+ "paths",
  "serde",
  "serde_json",
  "stdx",
@@ -1131,6 +1132,7 @@ dependencies = [
  "mbe",
  "memmap2",
  "object",
+ "paths",
  "proc_macro_api",
  "proc_macro_test",
  "test_utils",

crates/flycheck/Cargo.toml

@@ -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/src/lib.rs

@@ -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/paths/src/lib.rs

@@ -186,6 +186,9 @@ impl AbsPath {
     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
 

crates/proc_macro_api/src/rpc.rs

@@ -271,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(),
         };
 

crates/proc_macro_srv/Cargo.toml

@@ -15,6 +15,7 @@ memmap2 = "0.3.0"
 
 tt = { path = "../tt", version = "0.0.0" }
 mbe = { path = "../mbe", version = "0.0.0" }
+paths = { path = "../paths", version = "0.0.0" }
 proc_macro_api = { path = "../proc_macro_api", version = "0.0.0" }
 
 [dev-dependencies]

crates/proc_macro_srv/src/dylib.rs

@@ -1,6 +1,7 @@
 //! Handles dynamic library loading for proc macro
 
 use std::{
+    convert::TryInto,
     fmt,
     fs::File,
     io,
@@ -10,6 +11,7 @@ use std::{
 use libloading::Library;
 use memmap2::Mmap;
 use object::Object;
+use paths::AbsPath;
 use proc_macro_api::{read_dylib_info, ProcMacroKind};
 
 use super::abis::Abi;
@@ -116,7 +118,10 @@ impl ProcMacroLibraryLibloading {
             invalid_data_err(format!("Cannot find registrar symbol in file {}", file.display()))
         })?;
 
-        let version_info = read_dylib_info(file)?;
+        let abs_file: &AbsPath = file.try_into().map_err(|_| {
+            invalid_data_err(format!("expected an absolute path, got {}", file.display()))
+        })?;
+        let version_info = read_dylib_info(&abs_file)?;
 
         let lib = load_library(file).map_err(invalid_data_err)?;
         let abi = Abi::from_lib(&lib, symbol_name, version_info)?;
@@ -136,7 +141,7 @@ impl Expander {
 
         let lib = ensure_file_with_lock_free_access(&lib)?;
 
-        let library = ProcMacroLibraryLibloading::open(&lib)?;
+        let library = ProcMacroLibraryLibloading::open(lib.as_ref())?;
 
         Ok(Expander { inner: library })
     }

crates/proc_macro_srv/src/lib.rs

@@ -30,7 +30,7 @@ pub(crate) struct ProcMacroSrv {
 
 impl ProcMacroSrv {
     pub fn expand(&mut self, task: &ExpansionTask) -> Result<ExpansionResult, String> {
-        let expander = self.expander(&task.lib)?;
+        let expander = self.expander(task.lib.as_ref())?;
 
         let mut prev_env = HashMap::new();
         for (k, v) in &task.env {
@@ -54,7 +54,7 @@ impl ProcMacroSrv {
     }
 
     pub fn list_macros(&mut self, task: &ListMacrosTask) -> Result<ListMacrosResult, String> {
-        let expander = self.expander(&task.lib)?;
+        let expander = self.expander(task.lib.as_ref())?;
         Ok(ListMacrosResult { macros: expander.list_macros() })
     }
 

crates/proc_macro_srv/src/tests/mod.rs

@@ -3,6 +3,7 @@
 #[macro_use]
 mod utils;
 use expect_test::expect;
+use paths::AbsPathBuf;
 use utils::*;
 
 #[test]
@@ -95,7 +96,7 @@ fn list_test_macros() {
 
 #[test]
 fn test_version_check() {
-    let path = fixtures::proc_macro_test_dylib_path();
+    let path = AbsPathBuf::assert(fixtures::proc_macro_test_dylib_path());
     let info = proc_macro_api::read_dylib_info(&path).unwrap();
     assert!(info.version.1 >= 50);
 }

crates/proc_macro_srv/src/tests/utils.rs

@@ -3,6 +3,7 @@
 use crate::dylib;
 use crate::ProcMacroSrv;
 use expect_test::Expect;
+use paths::AbsPathBuf;
 use proc_macro_api::ListMacrosTask;
 use std::str::FromStr;
 
@@ -41,7 +42,7 @@ fn assert_expand_impl(macro_name: &str, input: &str, attr: Option<&str>, expect:
 }
 
 pub fn list() -> Vec<String> {
-    let path = fixtures::proc_macro_test_dylib_path();
+    let path = AbsPathBuf::assert(fixtures::proc_macro_test_dylib_path());
     let task = ListMacrosTask { lib: path };
     let mut srv = ProcMacroSrv::default();
     let res = srv.list_macros(&task).unwrap();

crates/rust-analyzer/src/cli/load_cargo.rs

@@ -28,7 +28,9 @@ pub(crate) fn load_workspace_at(
     progress: &dyn Fn(String),
 ) -> Result<(AnalysisHost, vfs::Vfs, Option<ProcMacroClient>)> {
     let root = AbsPathBuf::assert(std::env::current_dir()?.join(root));
+    eprintln!("root = {:?}", root);
     let root = ProjectManifest::discover_single(&root)?;
+    eprintln!("root = {:?}", root);
     let workspace = ProjectWorkspace::load(root, cargo_config, progress)?;
 
     load_workspace(workspace, load_config, progress)
@@ -48,7 +50,7 @@ fn load_workspace(
     };
 
     let proc_macro_client = if config.with_proc_macro {
-        let path = std::env::current_exe()?;
+        let path = AbsPathBuf::assert(std::env::current_exe()?);
         Some(ProcMacroClient::extern_process(path, &["proc-macro"]).unwrap())
     } else {
         None
@@ -142,7 +144,7 @@ mod tests {
     use hir::Crate;
 
     #[test]
-    fn test_loading_rust_analyzer() -> Result<()> {
+    fn test_loading_rust_analyzer() {
         let path = Path::new(env!("CARGO_MANIFEST_DIR")).parent().unwrap().parent().unwrap();
         let cargo_config = Default::default();
         let load_cargo_config = LoadCargoConfig {
@@ -152,12 +154,10 @@ mod tests {
             prefill_caches: false,
         };
         let (host, _vfs, _proc_macro) =
-            load_workspace_at(path, &cargo_config, &load_cargo_config, &|_| {})?;
+            load_workspace_at(path, &cargo_config, &load_cargo_config, &|_| {}).unwrap();
 
         let n_crates = Crate::all(host.raw_database()).len();
         // RA has quite a few crates, but the exact count doesn't matter
         assert!(n_crates > 20);
-
-        Ok(())
     }
 }

crates/rust-analyzer/src/config.rs

@@ -597,12 +597,14 @@ impl Config {
     pub fn lru_capacity(&self) -> Option<usize> {
         self.data.lruCapacity
     }
-    pub fn proc_macro_srv(&self) -> Option<(PathBuf, Vec<OsString>)> {
+    pub fn proc_macro_srv(&self) -> Option<(AbsPathBuf, Vec<OsString>)> {
         if !self.data.procMacro_enable {
             return None;
         }
-
+        let path = match &self.data.procMacro_server {
-        let path = self.data.procMacro_server.clone().or_else(|| std::env::current_exe().ok())?;
+            Some(it) => self.root_path.join(it),
+            None => AbsPathBuf::assert(std::env::current_exe().ok()?),
+        };
         Some((path, vec!["proc-macro".into()]))
     }
     pub fn expand_proc_attr_macros(&self) -> bool {

crates/rust-analyzer/src/diagnostics/to_proto.rs

@@ -1,12 +1,10 @@
 //! This module provides the functionality needed to convert diagnostics from
 //! `cargo check` json format to the LSP diagnostic format.
-use std::{
+use std::collections::HashMap;
-    collections::HashMap,
-    path::{Path, PathBuf},
-};
 
 use flycheck::{DiagnosticLevel, DiagnosticSpan};
 use stdx::format_to;
+use vfs::{AbsPath, AbsPathBuf};
 
 use crate::{lsp_ext, to_proto::url_from_abs_path};
 
@@ -46,7 +44,7 @@ fn is_dummy_macro_file(file_name: &str) -> bool {
 /// Converts a Rust span to a LSP location
 fn location(
     config: &DiagnosticsMapConfig,
-    workspace_root: &Path,
+    workspace_root: &AbsPath,
     span: &DiagnosticSpan,
 ) -> lsp_types::Location {
     let file_name = resolve_path(config, workspace_root, &span.file_name);
@@ -67,7 +65,7 @@ fn location(
 /// workspace into account and tries to avoid those, in case macros are involved.
 fn primary_location(
     config: &DiagnosticsMapConfig,
-    workspace_root: &Path,
+    workspace_root: &AbsPath,
     span: &DiagnosticSpan,
 ) -> lsp_types::Location {
     let span_stack = std::iter::successors(Some(span), |span| Some(&span.expansion.as_ref()?.span));
@@ -88,7 +86,7 @@ fn primary_location(
 /// If the span is unlabelled this will return `None`.
 fn diagnostic_related_information(
     config: &DiagnosticsMapConfig,
-    workspace_root: &Path,
+    workspace_root: &AbsPath,
     span: &DiagnosticSpan,
 ) -> Option<lsp_types::DiagnosticRelatedInformation> {
     let message = span.label.clone()?;
@@ -98,7 +96,11 @@ fn diagnostic_related_information(
 
 /// Resolves paths applying any matching path prefix remappings, and then
 /// joining the path to the workspace root.
-fn resolve_path(config: &DiagnosticsMapConfig, workspace_root: &Path, file_name: &str) -> PathBuf {
+fn resolve_path(
+    config: &DiagnosticsMapConfig,
+    workspace_root: &AbsPath,
+    file_name: &str,
+) -> AbsPathBuf {
     match config
         .remap_prefix
         .iter()
@@ -121,7 +123,7 @@ enum MappedRustChildDiagnostic {
 
 fn map_rust_child_diagnostic(
     config: &DiagnosticsMapConfig,
-    workspace_root: &Path,
+    workspace_root: &AbsPath,
     rd: &flycheck::Diagnostic,
 ) -> MappedRustChildDiagnostic {
     let spans: Vec<&DiagnosticSpan> = rd.spans.iter().filter(|s| s.is_primary).collect();
@@ -191,7 +193,7 @@ pub(crate) struct MappedRustDiagnostic {
 pub(crate) fn map_rust_diagnostic_to_lsp(
     config: &DiagnosticsMapConfig,
     rd: &flycheck::Diagnostic,
-    workspace_root: &Path,
+    workspace_root: &AbsPath,
 ) -> Vec<MappedRustDiagnostic> {
     let primary_spans: Vec<&DiagnosticSpan> = rd.spans.iter().filter(|s| s.is_primary).collect();
     if primary_spans.is_empty() {
@@ -426,6 +428,8 @@ fn clippy_code_description(code: Option<&str>) -> Option<lsp_types::CodeDescript
 #[cfg(test)]
 #[cfg(not(windows))]
 mod tests {
+    use std::{convert::TryInto, path::Path};
+
     use super::*;
 
     use expect_test::{expect_file, ExpectFile};
@@ -436,7 +440,7 @@ mod tests {
 
     fn check_with_config(config: DiagnosticsMapConfig, diagnostics_json: &str, expect: ExpectFile) {
         let diagnostic: flycheck::Diagnostic = serde_json::from_str(diagnostics_json).unwrap();
-        let workspace_root = Path::new("/test/");
+        let workspace_root: &AbsPath = Path::new("/test/").try_into().unwrap();
         let actual = map_rust_diagnostic_to_lsp(&config, &diagnostic, workspace_root);
         expect.assert_debug_eq(&actual)
     }

crates/rust-analyzer/src/reload.rs

@@ -73,8 +73,9 @@ impl GlobalState {
         fn is_interesting(path: &AbsPath, change_kind: ChangeKind) -> bool {
             const IMPLICIT_TARGET_FILES: &[&str] = &["build.rs", "src/main.rs", "src/lib.rs"];
             const IMPLICIT_TARGET_DIRS: &[&str] = &["src/bin", "examples", "tests", "benches"];
+            let file_name = path.file_name().unwrap_or_default();
 
-            if path.ends_with("Cargo.toml") || path.ends_with("Cargo.lock") {
+            if file_name == "Cargo.toml" || file_name == "Cargo.lock" {
                 return true;
             }
             if change_kind == ChangeKind::Modify {
@@ -83,22 +84,22 @@ impl GlobalState {
             if path.extension().unwrap_or_default() != "rs" {
                 return false;
             }
-            if IMPLICIT_TARGET_FILES.iter().any(|it| path.ends_with(it)) {
+            if IMPLICIT_TARGET_FILES.iter().any(|it| path.as_ref().ends_with(it)) {
                 return true;
             }
             let parent = match path.parent() {
                 Some(it) => it,
                 None => return false,
             };
-            if IMPLICIT_TARGET_DIRS.iter().any(|it| parent.ends_with(it)) {
+            if IMPLICIT_TARGET_DIRS.iter().any(|it| parent.as_ref().ends_with(it)) {
                 return true;
             }
-            if path.ends_with("main.rs") {
+            if file_name == "main.rs" {
                 let grand_parent = match parent.parent() {
                     Some(it) => it,
                     None => return false,
                 };
-                if IMPLICIT_TARGET_DIRS.iter().any(|it| grand_parent.ends_with(it)) {
+                if IMPLICIT_TARGET_DIRS.iter().any(|it| grand_parent.as_ref().ends_with(it)) {
                     return true;
                 }
             }

crates/rust-analyzer/src/to_proto.rs

@@ -1,7 +1,7 @@
 //! Conversion of rust-analyzer specific types to lsp_types equivalents.
 use std::{
     iter::once,
-    path::{self, Path},
+    path,
     sync::atomic::{AtomicU32, Ordering},
 };
 
@@ -14,6 +14,7 @@ use ide::{
 };
 use itertools::Itertools;
 use serde_json::to_value;
+use vfs::AbsPath;
 
 use crate::{
     cargo_target_spec::CargoTargetSpec,
@@ -622,10 +623,9 @@ pub(crate) fn url(snap: &GlobalStateSnapshot, file_id: FileId) -> lsp_types::Url
 /// This will only happen when processing windows paths.
 ///
 /// When processing non-windows path, this is essentially the same as `Url::from_file_path`.
-pub(crate) fn url_from_abs_path(path: &Path) -> lsp_types::Url {
+pub(crate) fn url_from_abs_path(path: &AbsPath) -> lsp_types::Url {
-    assert!(path.is_absolute());
     let url = lsp_types::Url::from_file_path(path).unwrap();
-    match path.components().next() {
+    match path.as_ref().components().next() {
         Some(path::Component::Prefix(prefix))
             if matches!(prefix.kind(), path::Prefix::Disk(_) | path::Prefix::VerbatimDisk(_)) =>
         {
@@ -1328,15 +1328,13 @@ fn main() {
     // `Url` is not able to parse windows paths on unix machines.
     #[test]
     #[cfg(target_os = "windows")]
-    fn test_lowercase_drive_letter_with_drive() {
+    fn test_lowercase_drive_letter() {
-        let url = url_from_abs_path(Path::new("C:\\Test"));
+        use std::{convert::TryInto, path::Path};
-        assert_eq!(url.to_string(), "file:///c:/Test");
-    }
 
-    #[test]
+        let url = url_from_abs_path(Path::new("C:\\Test").try_into().unwrap());
-    #[cfg(target_os = "windows")]
+        assert_eq!(url.to_string(), "file:///c:/Test");
-    fn test_drive_without_colon_passthrough() {
+
-        let url = url_from_abs_path(Path::new(r#"\\localhost\C$\my_dir"#));
+        let url = url_from_abs_path(Path::new(r#"\\localhost\C$\my_dir"#).try_into().unwrap());
         assert_eq!(url.to_string(), "file://localhost/C$/my_dir");
     }
 }

crates/vfs-notify/src/lib.rs

@@ -6,7 +6,7 @@
 //!
 //! Hopefully, one day a reliable file watching/walking crate appears on
 //! crates.io, and we can reduce this to trivial glue code.
-use std::convert::TryFrom;
+use std::{convert::TryFrom, fs};
 
 use crossbeam_channel::{never, select, unbounded, Receiver, Sender};
 use notify::{RecommendedWatcher, RecursiveMode, Watcher};
@@ -123,7 +123,8 @@ impl NotifyActor {
                             .into_iter()
                             .map(|path| AbsPathBuf::try_from(path).unwrap())
                             .filter_map(|path| {
-                                if path.is_dir()
+                                let meta = fs::metadata(&path).ok()?;
+                                if meta.file_type().is_dir()
                                     && self
                                         .watched_entries
                                         .iter()
@@ -133,7 +134,7 @@ impl NotifyActor {
                                     return None;
                                 }
 
-                                if !path.is_file() {
+                                if !meta.file_type().is_file() {
                                     return None;
                                 }
                                 if !self