Repository: rust-analyzer/rust-analyzer
Pull request #5076 (2020-06-26 14:30:21)

Make VFS join methods fallible


Patch:
diff --git a/crates/ra_hir_ty/src/method_resolution.rs b/crates/ra_hir_ty/src/method_resolution.rs
index c19519cf11..8b59a8bd6e 100644
--- a/crates/ra_hir_ty/src/method_resolution.rs
+++ b/crates/ra_hir_ty/src/method_resolution.rs
@@ -281,7 +281,6 @@ pub fn iterate_method_candidates<T>(
         name,
         mode,
         &mut |ty, item| {
-            assert!(slot.is_none());
             slot = callback(ty, item);
             slot.is_some()
         },
@@ -289,7 +288,7 @@ pub fn iterate_method_candidates<T>(
     slot
 }
 
-fn iterate_method_candidates_impl(
+pub fn iterate_method_candidates_impl(
     ty: &Canonical<Ty>,
     db: &dyn HirDatabase,
     env: Arc<TraitEnvironment>,
diff --git a/crates/rust-analyzer/src/diagnostics.rs b/crates/rust-analyzer/src/diagnostics.rs
index 1cf50b677f..f3cdb842bd 100644
--- a/crates/rust-analyzer/src/diagnostics.rs
+++ b/crates/rust-analyzer/src/diagnostics.rs
@@ -3,6 +3,7 @@ pub(crate) mod to_proto;
 
 use std::{collections::HashMap, mem, sync::Arc};
 
+use lsp_types::{Diagnostic, Range};
 use ra_ide::FileId;
 use rustc_hash::FxHashSet;
 
@@ -18,15 +19,15 @@ pub struct DiagnosticsConfig {
 
 #[derive(Debug, Default, Clone)]
 pub(crate) struct DiagnosticCollection {
-    pub(crate) native: HashMap<FileId, Vec<lsp_types::Diagnostic>>,
-    pub(crate) check: HashMap<FileId, Vec<lsp_types::Diagnostic>>,
+    pub(crate) native: HashMap<FileId, Vec<Diagnostic>>,
+    pub(crate) check: HashMap<FileId, Vec<Diagnostic>>,
     pub(crate) check_fixes: CheckFixes,
     changes: FxHashSet<FileId>,
 }
 
 #[derive(Debug, Clone)]
 pub(crate) struct Fix {
-    pub(crate) range: lsp_types::Range,
+    pub(crate) range: Range,
     pub(crate) action: lsp_ext::CodeAction,
 }
 
@@ -39,7 +40,7 @@ impl DiagnosticCollection {
     pub(crate) fn add_check_diagnostic(
         &mut self,
         file_id: FileId,
-        diagnostic: lsp_types::Diagnostic,
+        diagnostic: Diagnostic,
         fixes: Vec<lsp_ext::CodeAction>,
     ) {
         let diagnostics = self.check.entry(file_id).or_default();
@@ -58,19 +59,12 @@ impl DiagnosticCollection {
         self.changes.insert(file_id);
     }
 
-    pub(crate) fn set_native_diagnostics(
-        &mut self,
-        file_id: FileId,
-        diagnostics: Vec<lsp_types::Diagnostic>,
-    ) {
+    pub(crate) fn set_native_diagnostics(&mut self, file_id: FileId, diagnostics: Vec<Diagnostic>) {
         self.native.insert(file_id, diagnostics);
         self.changes.insert(file_id);
     }
 
-    pub(crate) fn diagnostics_for(
-        &self,
-        file_id: FileId,
-    ) -> impl Iterator<Item = &lsp_types::Diagnostic> {
+    pub(crate) fn diagnostics_for(&self, file_id: FileId) -> impl Iterator<Item = &Diagnostic> {
         let native = self.native.get(&file_id).into_iter().flatten();
         let check = self.check.get(&file_id).into_iter().flatten();
         native.chain(check)
@@ -84,7 +78,7 @@ impl DiagnosticCollection {
     }
 }
 
-fn are_diagnostics_equal(left: &lsp_types::Diagnostic, right: &lsp_types::Diagnostic) -> bool {
+fn are_diagnostics_equal(left: &Diagnostic, right: &Diagnostic) -> bool {
     left.source == right.source
         && left.severity == right.severity
         && left.range == right.range
diff --git a/crates/rust-analyzer/src/diagnostics/to_proto.rs b/crates/rust-analyzer/src/diagnostics/to_proto.rs
index 3eed118a9b..f379f5ed0a 100644
--- a/crates/rust-analyzer/src/diagnostics/to_proto.rs
+++ b/crates/rust-analyzer/src/diagnostics/to_proto.rs
@@ -167,9 +167,9 @@ fn map_rust_child_diagnostic(
 
 #[derive(Debug)]
 pub(crate) struct MappedRustDiagnostic {
-    pub(crate) location: Location,
-    pub(crate) diagnostic: Diagnostic,
-    pub(crate) fixes: Vec<lsp_ext::CodeAction>,
+    pub location: Location,
+    pub diagnostic: Diagnostic,
+    pub fixes: Vec<lsp_ext::CodeAction>,
 }
 
 /// Converts a Rust root diagnostic to LSP form
diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs
index 4da0940831..28daec5a3a 100644
--- a/crates/rust-analyzer/src/global_state.rs
+++ b/crates/rust-analyzer/src/global_state.rs
@@ -195,7 +195,11 @@ impl Drop for GlobalState {
 
 impl GlobalStateSnapshot {
     pub(crate) fn url_to_file_id(&self, url: &Url) -> Result<FileId> {
-        url_to_file_id(&self.vfs.read().0, url)
+        let path = from_proto::abs_path(url)?;
+        let path = path.into();
+        let res =
+            self.vfs.read().0.file_id(&path).ok_or_else(|| format!("file not found: {}", path))?;
+        Ok(res)
     }
 
     pub(crate) fn file_id_to_url(&self, id: FileId) -> Url {
@@ -209,7 +213,7 @@ impl GlobalStateSnapshot {
     pub(crate) fn anchored_path(&self, file_id: FileId, path: &str) -> Url {
         let mut base = self.vfs.read().0.file_path(file_id);
         base.pop();
-        let path = base.join(path);
+        let path = base.join(path).unwrap();
         let path = path.as_path().unwrap();
         url_from_abs_path(&path)
     }
@@ -235,9 +239,3 @@ pub(crate) fn file_id_to_url(vfs: &vfs::Vfs, id: FileId) -> Url {
     let path = path.as_path().unwrap();
     url_from_abs_path(&path)
 }
-
-pub(crate) fn url_to_file_id(vfs: &vfs::Vfs, url: &Url) -> Result<FileId> {
-    let path = from_proto::vfs_path(url)?;
-    let res = vfs.file_id(&path).ok_or_else(|| format!("file not found: {}", path))?;
-    Ok(res)
-}
diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs
index 8fc816cbd5..162c0057e9 100644
--- a/crates/rust-analyzer/src/main_loop.rs
+++ b/crates/rust-analyzer/src/main_loop.rs
@@ -16,7 +16,7 @@ use crate::{
     config::Config,
     dispatch::{NotificationDispatcher, RequestDispatcher},
     from_proto,
-    global_state::{file_id_to_url, url_to_file_id, GlobalState, Status},
+    global_state::{file_id_to_url, GlobalState, Status},
     handlers, lsp_ext,
     lsp_utils::{apply_document_changes, is_canceled, notification_is, notification_new},
     Result,
@@ -200,16 +200,18 @@ impl GlobalState {
                         &workspace_root,
                     );
                     for diag in diagnostics {
-                        match url_to_file_id(&self.vfs.read().0, &diag.location.uri) {
-                            Ok(file_id) => self.diagnostics.add_check_diagnostic(
-                                file_id,
-                                diag.diagnostic,
-                                diag.fixes,
-                            ),
-                            Err(err) => {
-                                log::error!("File with cargo diagnostic not found in VFS: {}", err);
+                        let path = from_proto::vfs_path(&diag.location.uri)?;
+                        let file_id = match self.vfs.read().0.file_id(&path) {
+                            Some(file) => FileId(file.0),
+                            None => {
+                                log::error!(
+                                    "File with cargo diagnostic not found in VFS: {}",
+                                    path
+                                );
+                                return Ok(());
                             }
                         };
+                        self.diagnostics.add_check_diagnostic(file_id, diag.diagnostic, diag.fixes)
                     }
                 }
 
diff --git a/crates/vfs/src/file_set.rs b/crates/vfs/src/file_set.rs
index 0173f74649..d0ddeafe7c 100644
--- a/crates/vfs/src/file_set.rs
+++ b/crates/vfs/src/file_set.rs
@@ -18,7 +18,7 @@ impl FileSet {
     pub fn resolve_path(&self, anchor: FileId, path: &str) -> Option<FileId> {
         let mut base = self.paths[&anchor].clone();
         base.pop();
-        let path = base.join(path);
+        let path = base.join(path)?;
         let res = self.files.get(&path).copied();
         res
     }
diff --git a/crates/vfs/src/vfs_path.rs b/crates/vfs/src/vfs_path.rs
index 940f91d0e5..dc3031ada4 100644
--- a/crates/vfs/src/vfs_path.rs
+++ b/crates/vfs/src/vfs_path.rs
@@ -22,15 +22,15 @@ impl VfsPath {
             VfsPathRepr::VirtualPath(_) => None,
         }
     }
-    pub fn join(&self, path: &str) -> VfsPath {
+    pub fn join(&self, path: &str) -> Option<VfsPath> {
         match &self.0 {
             VfsPathRepr::PathBuf(it) => {
                 let res = it.join(path).normalize();
-                VfsPath(VfsPathRepr::PathBuf(res))
+                Some(VfsPath(VfsPathRepr::PathBuf(res)))
             }
             VfsPathRepr::VirtualPath(it) => {
-                let res = it.join(path);
-                VfsPath(VfsPathRepr::VirtualPath(res))
+                let res = it.join(path)?;
+                Some(VfsPath(VfsPathRepr::VirtualPath(res)))
             }
         }
     }
@@ -101,13 +101,15 @@ impl VirtualPath {
         self.0 = self.0[..pos].to_string();
         true
     }
-    fn join(&self, mut path: &str) -> VirtualPath {
+    fn join(&self, mut path: &str) -> Option<VirtualPath> {
         let mut res = self.clone();
         while path.starts_with("../") {
-            assert!(res.pop());
+            if !res.pop() {
+                return None;
+            }
             path = &path["../".len()..]
         }
         res.0 = format!("{}/{}", res.0, path);
-        res
+        Some(res)
     }
 }


Test patch:
diff --git a/crates/ra_hir_def/src/nameres/tests/mod_resolution.rs b/crates/ra_hir_def/src/nameres/tests/mod_resolution.rs
index e9a5e4cba1..7536842011 100644
--- a/crates/ra_hir_def/src/nameres/tests/mod_resolution.rs
+++ b/crates/ra_hir_def/src/nameres/tests/mod_resolution.rs
@@ -334,6 +334,22 @@ fn module_resolution_relative_path_2() {
     "###);
 }
 
+#[test]
+fn module_resolution_relative_path_outside_root() {
+    let map = def_map(
+        r###"
+        //- /main.rs
+
+        #[path="../../../../../outside.rs"]
+        mod foo;
+        "###,
+    );
+
+    assert_snapshot!(map, @r###"
+        ⋮crate
+    "###);
+}
+
 #[test]
 fn module_resolution_explicit_path_mod_rs_2() {
     let map = def_map(