From a53c5de1fd970fed989eb6435ba6a8c67362c63b Mon Sep 17 00:00:00 2001 From: DJMcNab <36049421+DJMcNab@users.noreply.github.com> Date: Sun, 13 Jan 2019 10:31:37 +0000 Subject: Add an explanatory message when we use the Query fallback --- crates/ra_ide_api/src/goto_definition.rs | 31 +++++++++++++++++++++++++------ crates/ra_ide_api/src/hover.rs | 15 +++++++++++---- 2 files changed, 36 insertions(+), 10 deletions(-) (limited to 'crates') diff --git a/crates/ra_ide_api/src/goto_definition.rs b/crates/ra_ide_api/src/goto_definition.rs index 8d2ff561a..e2537758d 100644 --- a/crates/ra_ide_api/src/goto_definition.rs +++ b/crates/ra_ide_api/src/goto_definition.rs @@ -13,8 +13,11 @@ pub(crate) fn goto_definition( let file = db.source_file(position.file_id); let syntax = file.syntax(); if let Some(name_ref) = find_node_at_offset::(syntax, position.offset) { - let navs = reference_definition(db, position.file_id, name_ref)?; - return Ok(Some(RangeInfo::new(name_ref.syntax().range(), navs))); + let navs = reference_definition(db, position.file_id, name_ref)?.to_vec(); + return Ok(Some(RangeInfo::new( + name_ref.syntax().range(), + navs.to_vec(), + ))); } if let Some(name) = find_node_at_offset::(syntax, position.offset) { let navs = ctry!(name_definition(db, position.file_id, name)?); @@ -23,11 +26,27 @@ pub(crate) fn goto_definition( Ok(None) } +pub(crate) enum ReferenceResult { + Exact(NavigationTarget), + Approximate(Vec), +} + +impl ReferenceResult { + fn to_vec(self) -> Vec { + use self::ReferenceResult::*; + match self { + Exact(target) => vec![target], + Approximate(vec) => vec, + } + } +} + pub(crate) fn reference_definition( db: &RootDatabase, file_id: FileId, name_ref: &ast::NameRef, -) -> Cancelable> { +) -> Cancelable { + use self::ReferenceResult::*; if let Some(fn_descr) = hir::source_binder::function_from_child_node(db, file_id, name_ref.syntax())? { @@ -35,7 +54,7 @@ pub(crate) fn reference_definition( // First try to resolve the symbol locally if let Some(entry) = scope.resolve_local_name(name_ref) { let nav = NavigationTarget::from_scope_entry(file_id, &entry); - return Ok(vec![nav]); + return Ok(Exact(nav)); }; } // Then try module name resolution @@ -51,7 +70,7 @@ pub(crate) fn reference_definition( let resolved = module.resolve_path(db, &path)?; if let Some(def_id) = resolved.take_types().or(resolved.take_values()) { if let Some(target) = NavigationTarget::from_def(db, def_id.resolve(db)?)? { - return Ok(vec![target]); + return Ok(Exact(target)); } } } @@ -62,7 +81,7 @@ pub(crate) fn reference_definition( .into_iter() .map(NavigationTarget::from_symbol) .collect(); - Ok(navs) + Ok(Approximate(navs)) } fn name_definition( diff --git a/crates/ra_ide_api/src/hover.rs b/crates/ra_ide_api/src/hover.rs index b66774cdf..2968b807c 100644 --- a/crates/ra_ide_api/src/hover.rs +++ b/crates/ra_ide_api/src/hover.rs @@ -16,9 +16,16 @@ pub(crate) fn hover( let mut range = None; if let Some(name_ref) = find_node_at_offset::(file.syntax(), position.offset) { - let navs = crate::goto_definition::reference_definition(db, position.file_id, name_ref)?; - for nav in navs { - res.extend(doc_text_for(db, nav)?) + use crate::goto_definition::{ReferenceResult::*, reference_definition}; + let ref_result = reference_definition(db, position.file_id, name_ref)?; + match ref_result { + Exact(nav) => res.extend(doc_text_for(db, nav)?), + Approximate(navs) => { + res.push("Failed to exactly resolve the symbol. This is probably because rust_analyzer does not yet support glob imports or traits. \nThese methods were found instead:".to_string()); + for nav in navs { + res.extend(doc_text_for(db, nav)?) + } + } } if !res.is_empty() { range = Some(name_ref.syntax().range()) @@ -34,7 +41,7 @@ pub(crate) fn hover( file_id: position.file_id, range: node.range(), }; - res.extend(type_of(db, frange)?); + res.extend(type_of(db, frange)?.map(Into::into)); range = Some(node.range()); }; -- cgit v1.2.3 From 77f67ca7e2caac6d94215834981ae3f6fb908443 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 13 Jan 2019 12:27:26 +0300 Subject: gracefully handle cycles in crate graph rust-lang/rust has absolutely weird setup with rustc-workspace-shim, which leads to real cycles. --- crates/ra_db/src/input.rs | 82 +++++++++++++++++++------------- crates/ra_hir/src/nameres/tests.rs | 12 +++-- crates/ra_lsp_server/src/server_world.rs | 25 ++++++++-- 3 files changed, 78 insertions(+), 41 deletions(-) (limited to 'crates') diff --git a/crates/ra_db/src/input.rs b/crates/ra_db/src/input.rs index 023183e29..2b761ea0c 100644 --- a/crates/ra_db/src/input.rs +++ b/crates/ra_db/src/input.rs @@ -53,6 +53,9 @@ pub struct CrateGraph { arena: FxHashMap, } +#[derive(Debug)] +pub struct CyclicDependencies; + #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct CrateId(pub u32); @@ -94,12 +97,16 @@ impl CrateGraph { assert!(prev.is_none()); crate_id } - pub fn add_dep(&mut self, from: CrateId, name: SmolStr, to: CrateId) { - let mut visited = FxHashSet::default(); - if self.dfs_find(from, to, &mut visited) { - panic!("Cycle dependencies found.") + pub fn add_dep( + &mut self, + from: CrateId, + name: SmolStr, + to: CrateId, + ) -> Result<(), CyclicDependencies> { + if self.dfs_find(from, to, &mut FxHashSet::default()) { + return Err(CyclicDependencies); } - self.arena.get_mut(&from).unwrap().add_dep(name, to) + Ok(self.arena.get_mut(&from).unwrap().add_dep(name, to)) } pub fn is_empty(&self) -> bool { self.arena.is_empty() @@ -139,35 +146,6 @@ impl CrateGraph { } } -#[cfg(test)] -mod tests { - use super::{CrateGraph, FxHashMap, FileId, SmolStr}; - - #[test] - #[should_panic] - fn it_should_painc_because_of_cycle_dependencies() { - let mut graph = CrateGraph::default(); - let crate1 = graph.add_crate_root(FileId(1u32)); - let crate2 = graph.add_crate_root(FileId(2u32)); - let crate3 = graph.add_crate_root(FileId(3u32)); - graph.add_dep(crate1, SmolStr::new("crate2"), crate2); - graph.add_dep(crate2, SmolStr::new("crate3"), crate3); - graph.add_dep(crate3, SmolStr::new("crate1"), crate1); - } - - #[test] - fn it_works() { - let mut graph = CrateGraph { - arena: FxHashMap::default(), - }; - let crate1 = graph.add_crate_root(FileId(1u32)); - let crate2 = graph.add_crate_root(FileId(2u32)); - let crate3 = graph.add_crate_root(FileId(3u32)); - graph.add_dep(crate1, SmolStr::new("crate2"), crate2); - graph.add_dep(crate2, SmolStr::new("crate3"), crate3); - } -} - salsa::query_group! { pub trait FilesDatabase: salsa::Database { /// Text of the file. @@ -209,3 +187,39 @@ salsa::query_group! { } } } + +#[cfg(test)] +mod tests { + use super::{CrateGraph, FileId, SmolStr}; + + #[test] + fn it_should_painc_because_of_cycle_dependencies() { + let mut graph = CrateGraph::default(); + let crate1 = graph.add_crate_root(FileId(1u32)); + let crate2 = graph.add_crate_root(FileId(2u32)); + let crate3 = graph.add_crate_root(FileId(3u32)); + assert!(graph + .add_dep(crate1, SmolStr::new("crate2"), crate2) + .is_ok()); + assert!(graph + .add_dep(crate2, SmolStr::new("crate3"), crate3) + .is_ok()); + assert!(graph + .add_dep(crate3, SmolStr::new("crate1"), crate1) + .is_err()); + } + + #[test] + fn it_works() { + let mut graph = CrateGraph::default(); + let crate1 = graph.add_crate_root(FileId(1u32)); + let crate2 = graph.add_crate_root(FileId(2u32)); + let crate3 = graph.add_crate_root(FileId(3u32)); + assert!(graph + .add_dep(crate1, SmolStr::new("crate2"), crate2) + .is_ok()); + assert!(graph + .add_dep(crate2, SmolStr::new("crate3"), crate3) + .is_ok()); + } +} diff --git a/crates/ra_hir/src/nameres/tests.rs b/crates/ra_hir/src/nameres/tests.rs index ba9fcb3d1..647fd92aa 100644 --- a/crates/ra_hir/src/nameres/tests.rs +++ b/crates/ra_hir/src/nameres/tests.rs @@ -235,7 +235,9 @@ fn item_map_across_crates() { let mut crate_graph = CrateGraph::default(); let main_crate = crate_graph.add_crate_root(main_id); let lib_crate = crate_graph.add_crate_root(lib_id); - crate_graph.add_dep(main_crate, "test_crate".into(), lib_crate); + crate_graph + .add_dep(main_crate, "test_crate".into(), lib_crate) + .unwrap(); db.set_crate_graph(crate_graph); @@ -288,7 +290,9 @@ fn import_across_source_roots() { let mut crate_graph = CrateGraph::default(); let main_crate = crate_graph.add_crate_root(main_id); let lib_crate = crate_graph.add_crate_root(lib_id); - crate_graph.add_dep(main_crate, "test_crate".into(), lib_crate); + crate_graph + .add_dep(main_crate, "test_crate".into(), lib_crate) + .unwrap(); db.set_crate_graph(crate_graph); @@ -330,7 +334,9 @@ fn reexport_across_crates() { let mut crate_graph = CrateGraph::default(); let main_crate = crate_graph.add_crate_root(main_id); let lib_crate = crate_graph.add_crate_root(lib_id); - crate_graph.add_dep(main_crate, "test_crate".into(), lib_crate); + crate_graph + .add_dep(main_crate, "test_crate".into(), lib_crate) + .unwrap(); db.set_crate_graph(crate_graph); diff --git a/crates/ra_lsp_server/src/server_world.rs b/crates/ra_lsp_server/src/server_world.rs index 4f3c231d3..d5dbf999f 100644 --- a/crates/ra_lsp_server/src/server_world.rs +++ b/crates/ra_lsp_server/src/server_world.rs @@ -73,7 +73,9 @@ impl ServerWorldState { if let (Some(&from), Some(&to)) = (sysroot_crates.get(&from), sysroot_crates.get(&to)) { - crate_graph.add_dep(from, name.clone(), to); + if let Err(_) = crate_graph.add_dep(from, name.clone(), to) { + log::error!("cyclic dependency between sysroot crates") + } } } } @@ -108,11 +110,20 @@ impl ServerWorldState { for &from in pkg_crates.get(&pkg).into_iter().flatten() { if let Some(to) = lib_tgt { if to != from { - crate_graph.add_dep(from, pkg.name(&ws.cargo).into(), to); + if let Err(_) = + crate_graph.add_dep(from, pkg.name(&ws.cargo).into(), to) + { + log::error!( + "cyclic dependency between targets of {}", + pkg.name(&ws.cargo) + ) + } } } if let Some(std) = libstd { - crate_graph.add_dep(from, "std".into(), std); + if let Err(_) = crate_graph.add_dep(from, "std".into(), std) { + log::error!("cyclic dependency on std for {}", pkg.name(&ws.cargo)) + } } } } @@ -123,7 +134,13 @@ impl ServerWorldState { for dep in pkg.dependencies(&ws.cargo) { if let Some(&to) = pkg_to_lib_crate.get(&dep.pkg) { for &from in pkg_crates.get(&pkg).into_iter().flatten() { - crate_graph.add_dep(from, dep.name.clone(), to); + if let Err(_) = crate_graph.add_dep(from, dep.name.clone(), to) { + log::error!( + "cyclic dependency {} -> {}", + pkg.name(&ws.cargo), + dep.pkg.name(&ws.cargo) + ) + } } } } -- cgit v1.2.3