From 9ec5e6e4fdbe893f38d10dbdc609284368efdb64 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 6 Apr 2021 16:22:26 +0300 Subject: Clearer naming --- crates/paths/src/lib.rs | 7 ++++ crates/project_model/src/build_data.rs | 59 ++++++++++++++++++++-------------- crates/project_model/src/workspace.rs | 10 +++--- 3 files changed, 47 insertions(+), 29 deletions(-) diff --git a/crates/paths/src/lib.rs b/crates/paths/src/lib.rs index 22011cb33..f09ad37e3 100644 --- a/crates/paths/src/lib.rs +++ b/crates/paths/src/lib.rs @@ -1,6 +1,7 @@ //! Thin wrappers around `std::path`, distinguishing between absolute and //! relative paths. use std::{ + borrow::Borrow, convert::{TryFrom, TryInto}, ops, path::{Component, Path, PathBuf}, @@ -35,6 +36,12 @@ impl AsRef for AbsPathBuf { } } +impl Borrow for AbsPathBuf { + fn borrow(&self) -> &AbsPath { + self.as_path() + } +} + impl TryFrom for AbsPathBuf { type Error = PathBuf; fn try_from(path_buf: PathBuf) -> Result { diff --git a/crates/project_model/src/build_data.rs b/crates/project_model/src/build_data.rs index c2c87b207..fdf4b2d55 100644 --- a/crates/project_model/src/build_data.rs +++ b/crates/project_model/src/build_data.rs @@ -18,7 +18,7 @@ use stdx::JodChild; use crate::{cfg_flag::CfgFlag, CargoConfig}; #[derive(Debug, Clone, Default, PartialEq, Eq)] -pub(crate) struct BuildData { +pub(crate) struct PackageBuildData { /// List of config flags defined by this package's build script pub(crate) cfgs: Vec, /// List of cargo-related environment variables with their value @@ -32,6 +32,16 @@ pub(crate) struct BuildData { pub(crate) proc_macro_dylib_path: Option, } +#[derive(Debug, Default, PartialEq, Eq, Clone)] +pub(crate) struct WorkspaceBuildData { + per_package: FxHashMap, +} + +#[derive(Debug, Default, PartialEq, Eq, Clone)] +pub struct BuildDataResult { + per_workspace: FxHashMap, +} + #[derive(Clone, Debug)] pub(crate) struct BuildDataConfig { cargo_toml: AbsPathBuf, @@ -52,13 +62,6 @@ pub struct BuildDataCollector { configs: FxHashMap, } -#[derive(Debug, Default, PartialEq, Eq, Clone)] -pub struct BuildDataResult { - data: FxHashMap, -} - -pub(crate) type BuildDataMap = FxHashMap; - impl BuildDataCollector { pub(crate) fn add_config(&mut self, workspace_root: &AbsPath, config: BuildDataConfig) { self.configs.insert(workspace_root.to_path_buf(), config); @@ -67,7 +70,7 @@ impl BuildDataCollector { pub fn collect(&mut self, progress: &dyn Fn(String)) -> Result { let mut res = BuildDataResult::default(); for (path, config) in self.configs.iter() { - res.data.insert( + res.per_workspace.insert( path.clone(), collect_from_workspace( &config.cargo_toml, @@ -81,9 +84,15 @@ impl BuildDataCollector { } } +impl WorkspaceBuildData { + pub(crate) fn get(&self, package_id: &str) -> Option<&PackageBuildData> { + self.per_package.get(package_id) + } +} + impl BuildDataResult { - pub(crate) fn get(&self, root: &AbsPath) -> Option<&BuildDataMap> { - self.data.get(&root.to_path_buf()) + pub(crate) fn get(&self, workspace_root: &AbsPath) -> Option<&WorkspaceBuildData> { + self.per_workspace.get(workspace_root) } } @@ -102,7 +111,7 @@ fn collect_from_workspace( cargo_features: &CargoConfig, packages: &Vec, progress: &dyn Fn(String), -) -> Result { +) -> Result { let mut cmd = Command::new(toolchain::cargo()); cmd.args(&["check", "--workspace", "--message-format=json", "--manifest-path"]) .arg(cargo_toml.as_ref()); @@ -136,7 +145,7 @@ fn collect_from_workspace( let child_stdout = child.stdout.take().unwrap(); let stdout = BufReader::new(child_stdout); - let mut res = BuildDataMap::default(); + let mut res = WorkspaceBuildData::default(); for message in cargo_metadata::Message::parse_stream(stdout).flatten() { match message { Message::BuildScriptExecuted(BuildScript { @@ -154,16 +163,17 @@ fn collect_from_workspace( } acc }; - let res = res.entry(package_id.repr.clone()).or_default(); + let package_build_data = + res.per_package.entry(package_id.repr.clone()).or_default(); // cargo_metadata crate returns default (empty) path for // older cargos, which is not absolute, so work around that. if !out_dir.as_str().is_empty() { let out_dir = AbsPathBuf::assert(PathBuf::from(out_dir.into_os_string())); - res.out_dir = Some(out_dir); - res.cfgs = cfgs; + package_build_data.out_dir = Some(out_dir); + package_build_data.cfgs = cfgs; } - res.envs = env; + package_build_data.envs = env; } Message::CompilerArtifact(message) => { progress(format!("metadata {}", message.target.name)); @@ -173,8 +183,9 @@ fn collect_from_workspace( // Skip rmeta file if let Some(filename) = message.filenames.iter().find(|name| is_dylib(name)) { let filename = AbsPathBuf::assert(PathBuf::from(&filename)); - let res = res.entry(package_id.repr.clone()).or_default(); - res.proc_macro_dylib_path = Some(filename); + let package_build_data = + res.per_package.entry(package_id.repr.clone()).or_default(); + package_build_data.proc_macro_dylib_path = Some(filename); } } } @@ -188,12 +199,12 @@ fn collect_from_workspace( } for package in packages { - let build_data = res.entry(package.id.repr.clone()).or_default(); - inject_cargo_env(package, build_data); - if let Some(out_dir) = &build_data.out_dir { + let package_build_data = res.per_package.entry(package.id.repr.clone()).or_default(); + inject_cargo_env(package, package_build_data); + if let Some(out_dir) = &package_build_data.out_dir { // NOTE: cargo and rustc seem to hide non-UTF-8 strings from env! and option_env!() if let Some(out_dir) = out_dir.to_str().map(|s| s.to_owned()) { - build_data.envs.push(("OUT_DIR".to_string(), out_dir)); + package_build_data.envs.push(("OUT_DIR".to_string(), out_dir)); } } } @@ -212,7 +223,7 @@ fn is_dylib(path: &Utf8Path) -> bool { /// Recreates the compile-time environment variables that Cargo sets. /// /// Should be synced with -fn inject_cargo_env(package: &cargo_metadata::Package, build_data: &mut BuildData) { +fn inject_cargo_env(package: &cargo_metadata::Package, build_data: &mut PackageBuildData) { let env = &mut build_data.envs; // FIXME: Missing variables: diff --git a/crates/project_model/src/workspace.rs b/crates/project_model/src/workspace.rs index 1b53fcc30..2fcd0f8fa 100644 --- a/crates/project_model/src/workspace.rs +++ b/crates/project_model/src/workspace.rs @@ -12,7 +12,7 @@ use proc_macro_api::ProcMacroClient; use rustc_hash::{FxHashMap, FxHashSet}; use crate::{ - build_data::{BuildData, BuildDataMap, BuildDataResult}, + build_data::{BuildDataResult, PackageBuildData, WorkspaceBuildData}, cargo_workspace, cfg_flag::CfgFlag, rustc_cfg, @@ -354,10 +354,10 @@ fn cargo_to_crate_graph( proc_macro_loader: &dyn Fn(&Path) -> Vec, load: &mut dyn FnMut(&AbsPath) -> Option, cargo: &CargoWorkspace, - build_data_map: Option<&BuildDataMap>, + build_data_map: Option<&WorkspaceBuildData>, sysroot: &Sysroot, rustc: &Option, - rustc_build_data_map: Option<&BuildDataMap>, + rustc_build_data_map: Option<&WorkspaceBuildData>, ) -> CrateGraph { let _p = profile::span("cargo_to_crate_graph"); let mut crate_graph = CrateGraph::default(); @@ -464,7 +464,7 @@ fn handle_rustc_crates( rustc_workspace: &CargoWorkspace, load: &mut dyn FnMut(&AbsPath) -> Option, crate_graph: &mut CrateGraph, - rustc_build_data_map: Option<&FxHashMap>, + rustc_build_data_map: Option<&WorkspaceBuildData>, cfg_options: &CfgOptions, proc_macro_loader: &dyn Fn(&Path) -> Vec, pkg_to_lib_crate: &mut FxHashMap, CrateId>, @@ -555,7 +555,7 @@ fn handle_rustc_crates( fn add_target_crate_root( crate_graph: &mut CrateGraph, pkg: &cargo_workspace::PackageData, - build_data: Option<&BuildData>, + build_data: Option<&PackageBuildData>, cfg_options: &CfgOptions, proc_macro_loader: &dyn Fn(&Path) -> Vec, file_id: FileId, -- cgit v1.2.3 From de3370278468e5135e4990fc14562e5ce523ef37 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 6 Apr 2021 18:08:05 +0300 Subject: feat: show errors from `cargo metadata` and initial `cargo check` in the status bar closes #3155 --- crates/project_model/src/build_data.rs | 27 ++++++++++++++++++++-- crates/rust-analyzer/src/reload.rs | 41 +++++++++++++++++++++++++--------- crates/stdx/src/lib.rs | 8 +++++++ 3 files changed, 63 insertions(+), 13 deletions(-) diff --git a/crates/project_model/src/build_data.rs b/crates/project_model/src/build_data.rs index fdf4b2d55..0d4d39fef 100644 --- a/crates/project_model/src/build_data.rs +++ b/crates/project_model/src/build_data.rs @@ -13,7 +13,7 @@ use cargo_metadata::{BuildScript, Message}; use itertools::Itertools; use paths::{AbsPath, AbsPathBuf}; use rustc_hash::FxHashMap; -use stdx::JodChild; +use stdx::{format_to, JodChild}; use crate::{cfg_flag::CfgFlag, CargoConfig}; @@ -35,6 +35,7 @@ pub(crate) struct PackageBuildData { #[derive(Debug, Default, PartialEq, Eq, Clone)] pub(crate) struct WorkspaceBuildData { per_package: FxHashMap, + error: Option, } #[derive(Debug, Default, PartialEq, Eq, Clone)] @@ -94,6 +95,19 @@ impl BuildDataResult { pub(crate) fn get(&self, workspace_root: &AbsPath) -> Option<&WorkspaceBuildData> { self.per_workspace.get(workspace_root) } + pub fn error(&self) -> Option { + let mut buf = String::new(); + for (_workspace_root, build_data) in &self.per_workspace { + if let Some(err) = &build_data.error { + format_to!(buf, "cargo check failed:\n{}", err); + } + } + if buf.is_empty() { + return None; + } + + Some(buf) + } } impl BuildDataConfig { @@ -139,7 +153,7 @@ fn collect_from_workspace( } } - cmd.stdout(Stdio::piped()).stderr(Stdio::null()).stdin(Stdio::null()); + cmd.stdout(Stdio::piped()).stderr(Stdio::piped()).stdin(Stdio::null()); let mut child = cmd.spawn().map(JodChild)?; let child_stdout = child.stdout.take().unwrap(); @@ -209,6 +223,15 @@ fn collect_from_workspace( } } + let output = child.into_inner().wait_with_output()?; + if !output.status.success() { + let mut stderr = String::from_utf8(output.stderr).unwrap_or_default(); + if stderr.is_empty() { + stderr = "cargo check failed".to_string(); + } + res.error = Some(stderr) + } + Ok(res) } diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index 301c7003b..d0cc1b61a 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs @@ -109,6 +109,11 @@ impl GlobalState { quiescent: self.is_quiescent(), message: None, }; + + if let Some(error) = self.build_data_error() { + status.health = lsp_ext::Health::Warning; + status.message = Some(error) + } if !self.config.cargo_autoreload() && self.is_quiescent() && self.fetch_workspaces_queue.op_requested() @@ -116,9 +121,10 @@ impl GlobalState { status.health = lsp_ext::Health::Warning; status.message = Some("Workspace reload required".to_string()) } - if let Some(error) = self.loading_error() { + + if let Some(error) = self.fetch_workspace_error() { status.health = lsp_ext::Health::Error; - status.message = Some(format!("Workspace reload failed: {}", error)) + status.message = Some(error) } if self.last_reported_status.as_ref() != Some(&status) { @@ -217,7 +223,7 @@ impl GlobalState { let _p = profile::span("GlobalState::switch_workspaces"); log::info!("will switch workspaces"); - if let Some(error_message) = self.loading_error() { + if let Some(error_message) = self.fetch_workspace_error() { log::error!("failed to switch workspaces: {}", error_message); self.show_message(lsp_types::MessageType::Error, error_message); if !self.workspaces.is_empty() { @@ -225,6 +231,11 @@ impl GlobalState { } } + if let Some(error_message) = self.build_data_error() { + log::error!("failed to switch build data: {}", error_message); + self.show_message(lsp_types::MessageType::Error, error_message); + } + let workspaces = self .fetch_workspaces_queue .last_op_result() @@ -343,22 +354,30 @@ impl GlobalState { log::info!("did switch workspaces"); } - fn loading_error(&self) -> Option { - let mut message = None; + fn fetch_workspace_error(&self) -> Option { + let mut buf = String::new(); for ws in self.fetch_workspaces_queue.last_op_result() { if let Err(err) = ws { - let message = message.get_or_insert_with(String::new); - stdx::format_to!(message, "rust-analyzer failed to load workspace: {:#}\n", err); + stdx::format_to!(buf, "rust-analyzer failed to load workspace: {:#}\n", err); } } - if let Some(Err(err)) = self.fetch_build_data_queue.last_op_result() { - let message = message.get_or_insert_with(String::new); - stdx::format_to!(message, "rust-analyzer failed to fetch build data: {:#}\n", err); + if buf.is_empty() { + return None; } - message + Some(buf) + } + + fn build_data_error(&self) -> Option { + match self.fetch_build_data_queue.last_op_result() { + Some(Err(err)) => { + Some(format!("rust-analyzer failed to fetch build data: {:#}\n", err)) + } + Some(Ok(data)) => data.error(), + None => None, + } } fn reload_flycheck(&mut self) { diff --git a/crates/stdx/src/lib.rs b/crates/stdx/src/lib.rs index d26be4853..b0a18d58d 100644 --- a/crates/stdx/src/lib.rs +++ b/crates/stdx/src/lib.rs @@ -178,6 +178,7 @@ where start..start + len } +#[repr(transparent)] pub struct JodChild(pub process::Child); impl ops::Deref for JodChild { @@ -200,6 +201,13 @@ impl Drop for JodChild { } } +impl JodChild { + pub fn into_inner(self) -> process::Child { + // SAFETY: repr transparent + unsafe { std::mem::transmute::(self) } + } +} + #[cfg(test)] mod tests { use super::*; -- cgit v1.2.3