From be2654b0ed3e4eb51bc3745b2329d6264588549f Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 16 Apr 2020 22:02:10 +0200 Subject: [PATCH 1/2] Decouple project loading from project discovery a bit --- crates/ra_project_model/src/lib.rs | 229 +++++++++++---------- crates/rust-analyzer/src/cli/load_cargo.rs | 8 +- crates/rust-analyzer/src/main_loop.rs | 58 +++--- 3 files changed, 165 insertions(+), 130 deletions(-) diff --git a/crates/ra_project_model/src/lib.rs b/crates/ra_project_model/src/lib.rs index 0ab64a1e052..df4be94dc4a 100644 --- a/crates/ra_project_model/src/lib.rs +++ b/crates/ra_project_model/src/lib.rs @@ -77,31 +77,131 @@ pub fn is_member(&self) -> bool { } } -impl ProjectWorkspace { - pub fn discover(path: &Path, cargo_features: &CargoConfig) -> Result { - ProjectWorkspace::discover_with_sysroot(path, true, cargo_features) +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum ProjectRoot { + ProjectJson(PathBuf), + CargoToml(PathBuf), +} + +impl ProjectRoot { + pub fn from_manifest_file(path: PathBuf) -> Result { + if path.ends_with("rust-project.json") { + return Ok(ProjectRoot::ProjectJson(path)); + } + if path.ends_with("Cargo.toml") { + return Ok(ProjectRoot::CargoToml(path)); + } + bail!("project root must point to Cargo.toml or rust-project.json: {}", path.display()) } - pub fn discover_with_sysroot( - path: &Path, - with_sysroot: bool, - cargo_features: &CargoConfig, - ) -> Result { - match find_rust_project_json(path) { - Some(json_path) => { - let file = File::open(&json_path) - .with_context(|| format!("Failed to open json file {}", json_path.display()))?; - let reader = BufReader::new(file); - Ok(ProjectWorkspace::Json { - project: from_reader(reader).with_context(|| { - format!("Failed to deserialize json file {}", json_path.display()) - })?, - }) + pub fn discover(path: &Path) -> Result { + if let Some(project_json) = find_rust_project_json(path) { + return Ok(ProjectRoot::ProjectJson(project_json)); + } + return find_cargo_toml(path).map(ProjectRoot::CargoToml); + + fn find_rust_project_json(path: &Path) -> Option { + if path.ends_with("rust-project.json") { + return Some(path.to_path_buf()); } - None => { - let cargo_toml = find_cargo_toml(path).with_context(|| { - format!("Failed to find Cargo.toml for path {}", path.display()) + + let mut curr = Some(path); + while let Some(path) = curr { + let candidate = path.join("rust-project.json"); + if candidate.exists() { + return Some(candidate); + } + curr = path.parent(); + } + + None + } + + fn find_cargo_toml(path: &Path) -> Result { + if path.ends_with("Cargo.toml") { + return Ok(path.to_path_buf()); + } + + if let Some(p) = find_cargo_toml_in_parent_dir(path) { + return Ok(p); + } + + let entities = match read_dir(path) { + Ok(entities) => entities, + Err(e) => { + return Err(CargoTomlNotFoundError { + searched_at: path.to_path_buf(), + reason: format!("file system error: {}", e), + } + .into()); + } + }; + + let mut valid_canditates = find_cargo_toml_in_child_dir(entities); + return match valid_canditates.len() { + 1 => Ok(valid_canditates.remove(0)), + 0 => Err(CargoTomlNotFoundError { + searched_at: path.to_path_buf(), + reason: "no Cargo.toml file found".to_string(), + } + .into()), + _ => Err(CargoTomlNotFoundError { + searched_at: path.to_path_buf(), + reason: format!( + "multiple equally valid Cargo.toml files found: {:?}", + valid_canditates + ), + } + .into()), + }; + } + + fn find_cargo_toml_in_parent_dir(path: &Path) -> Option { + let mut curr = Some(path); + while let Some(path) = curr { + let candidate = path.join("Cargo.toml"); + if candidate.exists() { + return Some(candidate); + } + curr = path.parent(); + } + + None + } + + fn find_cargo_toml_in_child_dir(entities: ReadDir) -> Vec { + // Only one level down to avoid cycles the easy way and stop a runaway scan with large projects + let mut valid_canditates = vec![]; + for entity in entities.filter_map(Result::ok) { + let candidate = entity.path().join("Cargo.toml"); + if candidate.exists() { + valid_canditates.push(candidate) + } + } + valid_canditates + } + } +} + +impl ProjectWorkspace { + pub fn load( + root: ProjectRoot, + cargo_features: &CargoConfig, + with_sysroot: bool, + ) -> Result { + let res = match root { + ProjectRoot::ProjectJson(project_json) => { + let file = File::open(&project_json).with_context(|| { + format!("Failed to open json file {}", project_json.display()) })?; + let reader = BufReader::new(file); + ProjectWorkspace::Json { + project: from_reader(reader).with_context(|| { + format!("Failed to deserialize json file {}", project_json.display()) + })?, + } + } + ProjectRoot::CargoToml(cargo_toml) => { let cargo = CargoWorkspace::from_cargo_metadata(&cargo_toml, cargo_features) .with_context(|| { format!( @@ -119,9 +219,11 @@ pub fn discover_with_sysroot( } else { Sysroot::default() }; - Ok(ProjectWorkspace::Cargo { cargo, sysroot }) + ProjectWorkspace::Cargo { cargo, sysroot } } - } + }; + + Ok(res) } /// Returns the roots for the current `ProjectWorkspace` @@ -469,87 +571,6 @@ pub fn workspace_root_for(&self, path: &Path) -> Option<&Path> { } } -fn find_rust_project_json(path: &Path) -> Option { - if path.ends_with("rust-project.json") { - return Some(path.to_path_buf()); - } - - let mut curr = Some(path); - while let Some(path) = curr { - let candidate = path.join("rust-project.json"); - if candidate.exists() { - return Some(candidate); - } - curr = path.parent(); - } - - None -} - -fn find_cargo_toml_in_parent_dir(path: &Path) -> Option { - let mut curr = Some(path); - while let Some(path) = curr { - let candidate = path.join("Cargo.toml"); - if candidate.exists() { - return Some(candidate); - } - curr = path.parent(); - } - - None -} - -fn find_cargo_toml_in_child_dir(entities: ReadDir) -> Vec { - // Only one level down to avoid cycles the easy way and stop a runaway scan with large projects - let mut valid_canditates = vec![]; - for entity in entities.filter_map(Result::ok) { - let candidate = entity.path().join("Cargo.toml"); - if candidate.exists() { - valid_canditates.push(candidate) - } - } - valid_canditates -} - -fn find_cargo_toml(path: &Path) -> Result { - if path.ends_with("Cargo.toml") { - return Ok(path.to_path_buf()); - } - - if let Some(p) = find_cargo_toml_in_parent_dir(path) { - return Ok(p); - } - - let entities = match read_dir(path) { - Ok(entities) => entities, - Err(e) => { - return Err(CargoTomlNotFoundError { - searched_at: path.to_path_buf(), - reason: format!("file system error: {}", e), - } - .into()); - } - }; - - let mut valid_canditates = find_cargo_toml_in_child_dir(entities); - match valid_canditates.len() { - 1 => Ok(valid_canditates.remove(0)), - 0 => Err(CargoTomlNotFoundError { - searched_at: path.to_path_buf(), - reason: "no Cargo.toml file found".to_string(), - } - .into()), - _ => Err(CargoTomlNotFoundError { - searched_at: path.to_path_buf(), - reason: format!( - "multiple equally valid Cargo.toml files found: {:?}", - valid_canditates - ), - } - .into()), - } -} - pub fn get_rustc_cfg_options() -> CfgOptions { let mut cfg_options = CfgOptions::default(); diff --git a/crates/rust-analyzer/src/cli/load_cargo.rs b/crates/rust-analyzer/src/cli/load_cargo.rs index 43062ea105b..f3f266ee444 100644 --- a/crates/rust-analyzer/src/cli/load_cargo.rs +++ b/crates/rust-analyzer/src/cli/load_cargo.rs @@ -8,7 +8,7 @@ use ra_db::{ExternSourceId, FileId, SourceRootId}; use ra_ide::{AnalysisChange, AnalysisHost}; use ra_project_model::{ - get_rustc_cfg_options, CargoConfig, PackageRoot, ProcMacroClient, ProjectWorkspace, + get_rustc_cfg_options, CargoConfig, PackageRoot, ProcMacroClient, ProjectRoot, ProjectWorkspace, }; use ra_vfs::{RootEntry, Vfs, VfsChange, VfsTask, Watch}; use rustc_hash::{FxHashMap, FxHashSet}; @@ -27,9 +27,11 @@ pub(crate) fn load_cargo( load_out_dirs_from_check: bool, ) -> Result<(AnalysisHost, FxHashMap)> { let root = std::env::current_dir()?.join(root); - let ws = ProjectWorkspace::discover( - root.as_ref(), + let root = ProjectRoot::discover(&root)?; + let ws = ProjectWorkspace::load( + root, &CargoConfig { load_out_dirs_from_check, ..Default::default() }, + true, )?; let mut extern_dirs = FxHashSet::default(); diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index 8d142919628..9c345601cb0 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -88,37 +88,49 @@ pub fn main_loop(ws_roots: Vec, config: Config, connection: Connection) let mut loop_state = LoopState::default(); let mut world_state = { - // FIXME: support dynamic workspace loading. let workspaces = { - let mut loaded_workspaces = Vec::new(); - for ws_root in &ws_roots { - let workspace = ra_project_model::ProjectWorkspace::discover_with_sysroot( - ws_root.as_path(), - config.with_sysroot, - &config.cargo, - ); - match workspace { - Ok(workspace) => loaded_workspaces.push(workspace), - Err(e) => { - log::error!("loading workspace failed: {:?}", e); + // FIXME: support dynamic workspace loading. + let mut visited = FxHashSet::default(); + let project_roots = ws_roots + .iter() + .map(|it| ra_project_model::ProjectRoot::discover(it)) + .filter_map(|dir| { + dir.map_err(|cargo_toml_not_found| { + log::error!("discovering workspace failed: {:?}", cargo_toml_not_found); - if let Some(ra_project_model::CargoTomlNotFoundError { .. }) = - e.downcast_ref() - { - if !config.notifications.cargo_toml_not_found { - continue; - } + if config.notifications.cargo_toml_not_found { + show_message( + req::MessageType::Error, + format!( + "rust-analyzer failed to discover workspace: {:?}", + cargo_toml_not_found + ), + &connection.sender, + ); } + }) + .ok() + }) + .filter(|it| visited.insert(it.clone())); + project_roots + .filter_map(|root| { + ra_project_model::ProjectWorkspace::load( + root, + &config.cargo, + config.with_sysroot, + ) + .map_err(|err| { + log::error!("failed to load workspace: {:#}", err); show_message( req::MessageType::Error, - format!("rust-analyzer failed to load workspace: {:?}", e), + format!("rust-analyzer failed to load workspace: {:#}", err), &connection.sender, ); - } - } - } - loaded_workspaces + }) + .ok() + }) + .collect::>() }; let globs = config From 422ae477ce2a52c8d004ce629318f0b7c1d89638 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 16 Apr 2020 22:19:38 +0200 Subject: [PATCH 2/2] Unmix error handling when discovering workspaces Hitting an io::Error is a legit problem. Finding more than one Cargo.toml is not. --- crates/ra_project_model/src/lib.rs | 80 +++++++--------------- crates/rust-analyzer/src/cli/load_cargo.rs | 2 +- crates/rust-analyzer/src/main_loop.rs | 34 +++++---- 3 files changed, 41 insertions(+), 75 deletions(-) diff --git a/crates/ra_project_model/src/lib.rs b/crates/ra_project_model/src/lib.rs index df4be94dc4a..03f2629dae0 100644 --- a/crates/ra_project_model/src/lib.rs +++ b/crates/ra_project_model/src/lib.rs @@ -5,9 +5,8 @@ mod sysroot; use std::{ - error::Error, fs::{read_dir, File, ReadDir}, - io::BufReader, + io::{self, BufReader}, path::{Path, PathBuf}, process::Command, }; @@ -25,25 +24,6 @@ }; pub use ra_proc_macro::ProcMacroClient; -#[derive(Clone, PartialEq, Eq, Hash, Debug)] -pub struct CargoTomlNotFoundError { - pub searched_at: PathBuf, - pub reason: String, -} - -impl std::fmt::Display for CargoTomlNotFoundError { - fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!( - fmt, - "can't find Cargo.toml at {}, due to {}", - self.searched_at.display(), - self.reason - ) - } -} - -impl Error for CargoTomlNotFoundError {} - #[derive(Debug, Clone)] pub enum ProjectWorkspace { /// Project workspace was discovered by running `cargo metadata` and `rustc --print sysroot`. @@ -94,11 +74,25 @@ pub fn from_manifest_file(path: PathBuf) -> Result { bail!("project root must point to Cargo.toml or rust-project.json: {}", path.display()) } - pub fn discover(path: &Path) -> Result { - if let Some(project_json) = find_rust_project_json(path) { - return Ok(ProjectRoot::ProjectJson(project_json)); + pub fn discover_single(path: &Path) -> Result { + let mut candidates = ProjectRoot::discover(path)?; + let res = match candidates.pop() { + None => bail!("no projects"), + Some(it) => it, + }; + + if !candidates.is_empty() { + bail!("more than one project") } - return find_cargo_toml(path).map(ProjectRoot::CargoToml); + Ok(res) + } + + pub fn discover(path: &Path) -> io::Result> { + if let Some(project_json) = find_rust_project_json(path) { + return Ok(vec![ProjectRoot::ProjectJson(project_json)]); + } + return find_cargo_toml(path) + .map(|paths| paths.into_iter().map(ProjectRoot::CargoToml).collect()); fn find_rust_project_json(path: &Path) -> Option { if path.ends_with("rust-project.json") { @@ -117,43 +111,17 @@ fn find_rust_project_json(path: &Path) -> Option { None } - fn find_cargo_toml(path: &Path) -> Result { + fn find_cargo_toml(path: &Path) -> io::Result> { if path.ends_with("Cargo.toml") { - return Ok(path.to_path_buf()); + return Ok(vec![path.to_path_buf()]); } if let Some(p) = find_cargo_toml_in_parent_dir(path) { - return Ok(p); + return Ok(vec![p]); } - let entities = match read_dir(path) { - Ok(entities) => entities, - Err(e) => { - return Err(CargoTomlNotFoundError { - searched_at: path.to_path_buf(), - reason: format!("file system error: {}", e), - } - .into()); - } - }; - - let mut valid_canditates = find_cargo_toml_in_child_dir(entities); - return match valid_canditates.len() { - 1 => Ok(valid_canditates.remove(0)), - 0 => Err(CargoTomlNotFoundError { - searched_at: path.to_path_buf(), - reason: "no Cargo.toml file found".to_string(), - } - .into()), - _ => Err(CargoTomlNotFoundError { - searched_at: path.to_path_buf(), - reason: format!( - "multiple equally valid Cargo.toml files found: {:?}", - valid_canditates - ), - } - .into()), - }; + let entities = read_dir(path)?; + Ok(find_cargo_toml_in_child_dir(entities)) } fn find_cargo_toml_in_parent_dir(path: &Path) -> Option { diff --git a/crates/rust-analyzer/src/cli/load_cargo.rs b/crates/rust-analyzer/src/cli/load_cargo.rs index f3f266ee444..e620712add6 100644 --- a/crates/rust-analyzer/src/cli/load_cargo.rs +++ b/crates/rust-analyzer/src/cli/load_cargo.rs @@ -27,7 +27,7 @@ pub(crate) fn load_cargo( load_out_dirs_from_check: bool, ) -> Result<(AnalysisHost, FxHashMap)> { let root = std::env::current_dir()?.join(root); - let root = ProjectRoot::discover(&root)?; + let root = ProjectRoot::discover_single(&root)?; let ws = ProjectWorkspace::load( root, &CargoConfig { load_out_dirs_from_check, ..Default::default() }, diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index 9c345601cb0..fc4c77f8aa5 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -15,6 +15,7 @@ }; use crossbeam_channel::{never, select, unbounded, RecvError, Sender}; +use itertools::Itertools; use lsp_server::{Connection, ErrorCode, Message, Notification, Request, RequestId, Response}; use lsp_types::{ NumberOrString, WorkDoneProgress, WorkDoneProgressBegin, WorkDoneProgressCreateParams, @@ -93,27 +94,24 @@ pub fn main_loop(ws_roots: Vec, config: Config, connection: Connection) let mut visited = FxHashSet::default(); let project_roots = ws_roots .iter() - .map(|it| ra_project_model::ProjectRoot::discover(it)) - .filter_map(|dir| { - dir.map_err(|cargo_toml_not_found| { - log::error!("discovering workspace failed: {:?}", cargo_toml_not_found); + .filter_map(|it| ra_project_model::ProjectRoot::discover(it).ok()) + .flatten() + .filter(|it| visited.insert(it.clone())) + .collect::>(); - if config.notifications.cargo_toml_not_found { - show_message( - req::MessageType::Error, - format!( - "rust-analyzer failed to discover workspace: {:?}", - cargo_toml_not_found - ), - &connection.sender, - ); - } - }) - .ok() - }) - .filter(|it| visited.insert(it.clone())); + if project_roots.is_empty() && config.notifications.cargo_toml_not_found { + show_message( + req::MessageType::Error, + format!( + "rust-analyzer failed to discover workspace, no Cargo.toml found, dirs searched: {}", + ws_roots.iter().format_with(", ", |it, f| f(&it.display())) + ), + &connection.sender, + ); + }; project_roots + .into_iter() .filter_map(|root| { ra_project_model::ProjectWorkspace::load( root,