Unnamed repository; edit this file 'description' to name the repository.
Auto merge of #15087 - matklad:error-handling, r=matklad
internal: use consistent style for error handling
22 files changed, 128 insertions, 134 deletions
diff --git a/crates/paths/src/lib.rs b/crates/paths/src/lib.rs index e0c20a4143..88b8d0aee3 100644 --- a/crates/paths/src/lib.rs +++ b/crates/paths/src/lib.rs @@ -6,7 +6,7 @@ use std::{ borrow::Borrow, ffi::OsStr, - ops, + fmt, ops, path::{Component, Path, PathBuf}, }; @@ -95,6 +95,12 @@ impl AbsPathBuf { } } +impl fmt::Display for AbsPathBuf { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&self.0.display(), f) + } +} + /// Wrapper around an absolute [`Path`]. #[derive(Debug, Ord, PartialOrd, Eq, PartialEq, Hash)] #[repr(transparent)] @@ -217,6 +223,7 @@ impl AbsPath { pub fn as_os_str(&self) -> &OsStr { self.0.as_os_str() } + #[deprecated(note = "use Display instead")] pub fn display(&self) -> std::path::Display<'_> { self.0.display() } @@ -227,6 +234,12 @@ impl AbsPath { // endregion:delegate-methods } +impl fmt::Display for AbsPath { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&self.0.display(), f) + } +} + /// Wrapper around a relative [`PathBuf`]. #[derive(Debug, Clone, Ord, PartialOrd, Eq, PartialEq, Hash)] pub struct RelPathBuf(PathBuf); diff --git a/crates/project-model/src/build_scripts.rs b/crates/project-model/src/build_scripts.rs index 6cbf403cb2..fb0f3ab7d1 100644 --- a/crates/project-model/src/build_scripts.rs +++ b/crates/project-model/src/build_scripts.rs @@ -225,9 +225,8 @@ impl WorkspaceBuildScripts { let package_build_data = &mut res[idx].outputs[package]; if !package_build_data.is_unchanged() { tracing::info!( - "{}: {:?}", - workspace[package].manifest.parent().display(), - package_build_data, + "{}: {package_build_data:?}", + workspace[package].manifest.parent(), ); } } @@ -270,9 +269,8 @@ impl WorkspaceBuildScripts { let package_build_data = &outputs[package]; if !package_build_data.is_unchanged() { tracing::info!( - "{}: {:?}", - workspace[package].manifest.parent().display(), - package_build_data, + "{}: {package_build_data:?}", + workspace[package].manifest.parent(), ); } } @@ -424,7 +422,7 @@ impl WorkspaceBuildScripts { let target_libdir = AbsPathBuf::try_from(PathBuf::from(target_libdir)) .map_err(|_| anyhow::format_err!("target-libdir was not an absolute path"))?; - tracing::info!("Loading rustc proc-macro paths from {}", target_libdir.display()); + tracing::info!("Loading rustc proc-macro paths from {target_libdir}"); let proc_macro_dylibs: Vec<(String, AbsPathBuf)> = std::fs::read_dir(target_libdir)? .filter_map(|entry| { @@ -458,9 +456,8 @@ impl WorkspaceBuildScripts { let package_build_data = &bs.outputs[package]; if !package_build_data.is_unchanged() { tracing::info!( - "{}: {:?}", - rustc[package].manifest.parent().display(), - package_build_data, + "{}: {package_build_data:?}", + rustc[package].manifest.parent(), ); } } diff --git a/crates/project-model/src/cargo_workspace.rs b/crates/project-model/src/cargo_workspace.rs index 92b454150c..e1117ac464 100644 --- a/crates/project-model/src/cargo_workspace.rs +++ b/crates/project-model/src/cargo_workspace.rs @@ -4,7 +4,7 @@ use std::path::PathBuf; use std::str::from_utf8; use std::{ops, process::Command}; -use anyhow::{Context, Result}; +use anyhow::Context; use base_db::Edition; use cargo_metadata::{CargoOpt, MetadataCommand}; use la_arena::{Arena, Idx}; @@ -236,7 +236,7 @@ impl CargoWorkspace { current_dir: &AbsPath, config: &CargoConfig, progress: &dyn Fn(String), - ) -> Result<cargo_metadata::Metadata> { + ) -> anyhow::Result<cargo_metadata::Metadata> { let targets = find_list_of_build_targets(config, cargo_toml); let mut meta = MetadataCommand::new(); diff --git a/crates/project-model/src/lib.rs b/crates/project-model/src/lib.rs index 9685fb3f97..901dcfd2b1 100644 --- a/crates/project-model/src/lib.rs +++ b/crates/project-model/src/lib.rs @@ -37,7 +37,7 @@ use std::{ process::Command, }; -use anyhow::{bail, format_err, Context, Result}; +use anyhow::{bail, format_err, Context}; use paths::{AbsPath, AbsPathBuf}; use rustc_hash::FxHashSet; @@ -60,19 +60,19 @@ pub enum ProjectManifest { } impl ProjectManifest { - pub fn from_manifest_file(path: AbsPathBuf) -> Result<ProjectManifest> { + pub fn from_manifest_file(path: AbsPathBuf) -> anyhow::Result<ProjectManifest> { let path = ManifestPath::try_from(path) - .map_err(|path| format_err!("bad manifest path: {}", path.display()))?; + .map_err(|path| format_err!("bad manifest path: {path}"))?; if path.file_name().unwrap_or_default() == "rust-project.json" { return Ok(ProjectManifest::ProjectJson(path)); } if path.file_name().unwrap_or_default() == "Cargo.toml" { return Ok(ProjectManifest::CargoToml(path)); } - bail!("project root must point to Cargo.toml or rust-project.json: {}", path.display()); + bail!("project root must point to Cargo.toml or rust-project.json: {path}"); } - pub fn discover_single(path: &AbsPath) -> Result<ProjectManifest> { + pub fn discover_single(path: &AbsPath) -> anyhow::Result<ProjectManifest> { let mut candidates = ProjectManifest::discover(path)?; let res = match candidates.pop() { None => bail!("no projects"), @@ -156,7 +156,7 @@ impl fmt::Display for ProjectManifest { } } -fn utf8_stdout(mut cmd: Command) -> Result<String> { +fn utf8_stdout(mut cmd: Command) -> anyhow::Result<String> { let output = cmd.output().with_context(|| format!("{cmd:?} failed"))?; if !output.status.success() { match String::from_utf8(output.stderr) { diff --git a/crates/project-model/src/manifest_path.rs b/crates/project-model/src/manifest_path.rs index 5ef2086fda..490e1a4ea8 100644 --- a/crates/project-model/src/manifest_path.rs +++ b/crates/project-model/src/manifest_path.rs @@ -42,7 +42,7 @@ impl ManifestPath { impl fmt::Display for ManifestPath { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Display::fmt(&self.file.display(), f) + fmt::Display::fmt(&self.file, f) } } diff --git a/crates/project-model/src/rustc_cfg.rs b/crates/project-model/src/rustc_cfg.rs index 0066f6717e..7ec6948eff 100644 --- a/crates/project-model/src/rustc_cfg.rs +++ b/crates/project-model/src/rustc_cfg.rs @@ -2,7 +2,6 @@ use std::process::Command; -use anyhow::Result; use rustc_hash::FxHashMap; use crate::{cfg_flag::CfgFlag, utf8_stdout, ManifestPath}; @@ -44,7 +43,7 @@ fn get_rust_cfgs( cargo_toml: Option<&ManifestPath>, target: Option<&str>, extra_env: &FxHashMap<String, String>, -) -> Result<String> { +) -> anyhow::Result<String> { if let Some(cargo_toml) = cargo_toml { let mut cargo_config = Command::new(toolchain::cargo()); cargo_config.envs(extra_env); diff --git a/crates/project-model/src/sysroot.rs b/crates/project-model/src/sysroot.rs index e3a2de927c..da862c9e87 100644 --- a/crates/project-model/src/sysroot.rs +++ b/crates/project-model/src/sysroot.rs @@ -85,9 +85,8 @@ impl Sysroot { " try running `rustup component add rust-src` to possible fix this" }; Some(format!( - "could not find libcore in loaded sysroot at `{}`{}", - self.src_root.as_path().display(), - var_note, + "could not find libcore in loaded sysroot at `{}`{var_note}", + self.src_root.as_path(), )) } else { None @@ -99,7 +98,7 @@ impl Sysroot { impl Sysroot { /// Attempts to discover the toolchain's sysroot from the given `dir`. pub fn discover(dir: &AbsPath, extra_env: &FxHashMap<String, String>) -> Result<Sysroot> { - tracing::debug!("discovering sysroot for {}", dir.display()); + tracing::debug!("discovering sysroot for {dir}"); let sysroot_dir = discover_sysroot_dir(dir, extra_env)?; let sysroot_src_dir = discover_sysroot_src_dir_or_add_component(&sysroot_dir, dir, extra_env)?; @@ -111,7 +110,7 @@ impl Sysroot { extra_env: &FxHashMap<String, String>, src: AbsPathBuf, ) -> Result<Sysroot> { - tracing::debug!("discovering sysroot for {}", current_dir.display()); + tracing::debug!("discovering sysroot for {current_dir}"); let sysroot_dir = discover_sysroot_dir(current_dir, extra_env)?; Ok(Sysroot::load(sysroot_dir, src)) } @@ -122,7 +121,7 @@ impl Sysroot { pub fn with_sysroot_dir(sysroot_dir: AbsPathBuf) -> Result<Sysroot> { let sysroot_src_dir = discover_sysroot_src_dir(&sysroot_dir).ok_or_else(|| { - format_err!("can't load standard library from sysroot path {}", sysroot_dir.display()) + format_err!("can't load standard library from sysroot path {sysroot_dir}") })?; Ok(Sysroot::load(sysroot_dir, sysroot_src_dir)) } @@ -220,10 +219,10 @@ fn discover_sysroot_src_dir(sysroot_path: &AbsPathBuf) -> Option<AbsPathBuf> { if let Ok(path) = AbsPathBuf::try_from(path.as_str()) { let core = path.join("core"); if fs::metadata(&core).is_ok() { - tracing::debug!("Discovered sysroot by RUST_SRC_PATH: {}", path.display()); + tracing::debug!("Discovered sysroot by RUST_SRC_PATH: {path}"); return Some(path); } - tracing::debug!("RUST_SRC_PATH is set, but is invalid (no core: {:?}), ignoring", core); + tracing::debug!("RUST_SRC_PATH is set, but is invalid (no core: {core:?}), ignoring"); } else { tracing::debug!("RUST_SRC_PATH is set, but is invalid, ignoring"); } @@ -250,10 +249,9 @@ fn discover_sysroot_src_dir_or_add_component( format_err!( "\ can't load standard library from sysroot -{} +{sysroot_path} (discovered via `rustc --print sysroot`) try installing the Rust source the same way you installed rustc", - sysroot_path.display(), ) }) } @@ -261,7 +259,7 @@ try installing the Rust source the same way you installed rustc", fn get_rustc_src(sysroot_path: &AbsPath) -> Option<ManifestPath> { let rustc_src = sysroot_path.join("lib/rustlib/rustc-src/rust/compiler/rustc/Cargo.toml"); let rustc_src = ManifestPath::try_from(rustc_src).ok()?; - tracing::debug!("checking for rustc source code: {}", rustc_src.display()); + tracing::debug!("checking for rustc source code: {rustc_src}"); if fs::metadata(&rustc_src).is_ok() { Some(rustc_src) } else { @@ -271,7 +269,7 @@ fn get_rustc_src(sysroot_path: &AbsPath) -> Option<ManifestPath> { fn get_rust_src(sysroot_path: &AbsPath) -> Option<AbsPathBuf> { let rust_src = sysroot_path.join("lib/rustlib/src/rust/library"); - tracing::debug!("checking sysroot library: {}", rust_src.display()); + tracing::debug!("checking sysroot library: {rust_src}"); if fs::metadata(&rust_src).is_ok() { Some(rust_src) } else { diff --git a/crates/project-model/src/target_data_layout.rs b/crates/project-model/src/target_data_layout.rs index 30ca7b348e..cb995857ec 100644 --- a/crates/project-model/src/target_data_layout.rs +++ b/crates/project-model/src/target_data_layout.rs @@ -1,7 +1,6 @@ //! Runs `rustc --print target-spec-json` to get the target_data_layout. use std::process::Command; -use anyhow::Result; use rustc_hash::FxHashMap; use crate::{utf8_stdout, ManifestPath}; @@ -10,7 +9,7 @@ pub fn get( cargo_toml: Option<&ManifestPath>, target: Option<&str>, extra_env: &FxHashMap<String, String>, -) -> Result<String> { +) -> anyhow::Result<String> { let output = (|| { if let Some(cargo_toml) = cargo_toml { let mut cmd = Command::new(toolchain::rustc()); diff --git a/crates/project-model/src/workspace.rs b/crates/project-model/src/workspace.rs index 34e5c1a9e2..f51ea7eeb2 100644 --- a/crates/project-model/src/workspace.rs +++ b/crates/project-model/src/workspace.rs @@ -4,7 +4,7 @@ use std::{collections::VecDeque, fmt, fs, process::Command, sync}; -use anyhow::{format_err, Context, Result}; +use anyhow::{format_err, Context}; use base_db::{ CrateDisplayName, CrateGraph, CrateId, CrateName, CrateOrigin, Dependency, Edition, Env, FileId, LangCrateOrigin, ProcMacroPaths, ReleaseChannel, TargetLayoutLoadResult, @@ -151,7 +151,7 @@ impl ProjectWorkspace { manifest: ProjectManifest, config: &CargoConfig, progress: &dyn Fn(String), - ) -> Result<ProjectWorkspace> { + ) -> anyhow::Result<ProjectWorkspace> { ProjectWorkspace::load_inner(&manifest, config, progress) .with_context(|| format!("Failed to load the project at {manifest}")) } @@ -160,7 +160,7 @@ impl ProjectWorkspace { manifest: &ProjectManifest, config: &CargoConfig, progress: &dyn Fn(String), - ) -> Result<ProjectWorkspace> { + ) -> anyhow::Result<ProjectWorkspace> { let version = |current_dir, cmd_path, prefix: &str| { let cargo_version = utf8_stdout({ let mut cmd = Command::new(cmd_path); @@ -176,12 +176,10 @@ impl ProjectWorkspace { }; let res = match manifest { ProjectManifest::ProjectJson(project_json) => { - let file = fs::read_to_string(&project_json).with_context(|| { - format!("Failed to read json file {}", project_json.display()) - })?; - let data = serde_json::from_str(&file).with_context(|| { - format!("Failed to deserialize json file {}", project_json.display()) - })?; + let file = fs::read_to_string(&project_json) + .with_context(|| format!("Failed to read json file {project_json}"))?; + let data = serde_json::from_str(&file) + .with_context(|| format!("Failed to deserialize json file {project_json}"))?; let project_location = project_json.parent().to_path_buf(); let toolchain = version(&*project_location, toolchain::rustc(), "rustc ")?; let project_json = ProjectJson::new(&project_location, data); @@ -202,9 +200,7 @@ impl ProjectWorkspace { ) .with_context(|| { format!( - "Failed to read Cargo metadata from Cargo.toml file {}, {:?}", - cargo_toml.display(), - toolchain + "Failed to read Cargo metadata from Cargo.toml file {cargo_toml}, {toolchain:?}", ) })?; let cargo = CargoWorkspace::new(meta); @@ -212,12 +208,12 @@ impl ProjectWorkspace { let sysroot = match (&config.sysroot, &config.sysroot_src) { (Some(RustLibSource::Path(path)), None) => { Sysroot::with_sysroot_dir(path.clone()).map_err(|e| { - Some(format!("Failed to find sysroot at {}:{e}", path.display())) + Some(format!("Failed to find sysroot at {path}:{e}")) }) } (Some(RustLibSource::Discover), None) => { Sysroot::discover(cargo_toml.parent(), &config.extra_env).map_err(|e| { - Some(format!("Failed to find sysroot for Cargo.toml file {}. Is rust-src installed? {e}", cargo_toml.display())) + Some(format!("Failed to find sysroot for Cargo.toml file {cargo_toml}. Is rust-src installed? {e}")) }) } (Some(RustLibSource::Path(sysroot)), Some(sysroot_src)) => { @@ -229,21 +225,19 @@ impl ProjectWorkspace { &config.extra_env, sysroot_src.clone(), ).map_err(|e| { - Some(format!("Failed to find sysroot for Cargo.toml file {}. Is rust-src installed? {e}", cargo_toml.display())) + Some(format!("Failed to find sysroot for Cargo.toml file {cargo_toml}. Is rust-src installed? {e}")) }) } (None, _) => Err(None), }; if let Ok(sysroot) = &sysroot { - tracing::info!(workspace = %cargo_toml.display(), src_root = %sysroot.src_root().display(), root = %sysroot.root().display(), "Using sysroot"); + tracing::info!(workspace = %cargo_toml, src_root = %sysroot.src_root(), root = %sysroot.root(), "Using sysroot"); } let rustc_dir = match &config.rustc_source { Some(RustLibSource::Path(path)) => ManifestPath::try_from(path.clone()) - .map_err(|p| { - Some(format!("rustc source path is not absolute: {}", p.display())) - }), + .map_err(|p| Some(format!("rustc source path is not absolute: {p}"))), Some(RustLibSource::Discover) => { sysroot.as_ref().ok().and_then(Sysroot::discover_rustc).ok_or_else(|| { Some(format!("Failed to discover rustc source for sysroot.")) @@ -253,7 +247,7 @@ impl ProjectWorkspace { }; let rustc = rustc_dir.and_then(|rustc_dir| { - tracing::info!(workspace = %cargo_toml.display(), rustc_dir = %rustc_dir.display(), "Using rustc source"); + tracing::info!(workspace = %cargo_toml, rustc_dir = %rustc_dir, "Using rustc source"); match CargoWorkspace::fetch_metadata( &rustc_dir, cargo_toml.parent(), @@ -275,13 +269,11 @@ impl ProjectWorkspace { Err(e) => { tracing::error!( %e, - "Failed to read Cargo metadata from rustc source at {}", - rustc_dir.display() + "Failed to read Cargo metadata from rustc source at {rustc_dir}", ); Err(Some(format!( - "Failed to read Cargo metadata from rustc source at {}: {e}", - rustc_dir.display()) - )) + "Failed to read Cargo metadata from rustc source at {rustc_dir}: {e}" + ))) } } }); @@ -339,7 +331,7 @@ impl ProjectWorkspace { (None, None) => Err(None), }; if let Ok(sysroot) = &sysroot { - tracing::info!(src_root = %sysroot.src_root().display(), root = %sysroot.root().display(), "Using sysroot"); + tracing::info!(src_root = %sysroot.src_root(), root = %sysroot.root(), "Using sysroot"); } let rustc_cfg = rustc_cfg::get(None, target, extra_env); @@ -349,26 +341,23 @@ impl ProjectWorkspace { pub fn load_detached_files( detached_files: Vec<AbsPathBuf>, config: &CargoConfig, - ) -> Result<ProjectWorkspace> { + ) -> anyhow::Result<ProjectWorkspace> { let sysroot = match &config.sysroot { Some(RustLibSource::Path(path)) => Sysroot::with_sysroot_dir(path.clone()) - .map_err(|e| Some(format!("Failed to find sysroot at {}:{e}", path.display()))), + .map_err(|e| Some(format!("Failed to find sysroot at {path}:{e}"))), Some(RustLibSource::Discover) => { let dir = &detached_files .first() .and_then(|it| it.parent()) .ok_or_else(|| format_err!("No detached files to load"))?; Sysroot::discover(dir, &config.extra_env).map_err(|e| { - Some(format!( - "Failed to find sysroot for {}. Is rust-src installed? {e}", - dir.display() - )) + Some(format!("Failed to find sysroot for {dir}. Is rust-src installed? {e}")) }) } None => Err(None), }; if let Ok(sysroot) = &sysroot { - tracing::info!(src_root = %sysroot.src_root().display(), root = %sysroot.root().display(), "Using sysroot"); + tracing::info!(src_root = %sysroot.src_root(), root = %sysroot.root(), "Using sysroot"); } let rustc_cfg = rustc_cfg::get(None, None, &Default::default()); Ok(ProjectWorkspace::DetachedFiles { files: detached_files, sysroot, rustc_cfg }) @@ -379,15 +368,12 @@ impl ProjectWorkspace { &self, config: &CargoConfig, progress: &dyn Fn(String), - ) -> Result<WorkspaceBuildScripts> { + ) -> anyhow::Result<WorkspaceBuildScripts> { match self { ProjectWorkspace::Cargo { cargo, toolchain, .. } => { WorkspaceBuildScripts::run_for_workspace(config, cargo, progress, toolchain) .with_context(|| { - format!( - "Failed to run build scripts for {}", - &cargo.workspace_root().display() - ) + format!("Failed to run build scripts for {}", cargo.workspace_root()) }) } ProjectWorkspace::Json { .. } | ProjectWorkspace::DetachedFiles { .. } => { @@ -402,7 +388,7 @@ impl ProjectWorkspace { workspaces: &[ProjectWorkspace], config: &CargoConfig, progress: &dyn Fn(String), - ) -> Vec<Result<WorkspaceBuildScripts>> { + ) -> Vec<anyhow::Result<WorkspaceBuildScripts>> { if matches!(config.invocation_strategy, InvocationStrategy::PerWorkspace) || config.run_build_script_command.is_none() { @@ -428,10 +414,7 @@ impl ProjectWorkspace { ProjectWorkspace::Cargo { cargo, .. } => match outputs { Ok(outputs) => Ok(outputs.next().unwrap()), Err(e) => Err(e.clone()).with_context(|| { - format!( - "Failed to run build scripts for {}", - &cargo.workspace_root().display() - ) + format!("Failed to run build scripts for {}", cargo.workspace_root()) }), }, _ => Ok(WorkspaceBuildScripts::default()), @@ -456,7 +439,7 @@ impl ProjectWorkspace { } } - pub fn find_sysroot_proc_macro_srv(&self) -> Result<AbsPathBuf> { + pub fn find_sysroot_proc_macro_srv(&self) -> anyhow::Result<AbsPathBuf> { match self { ProjectWorkspace::Cargo { sysroot: Ok(sysroot), .. } | ProjectWorkspace::Json { sysroot: Ok(sysroot), .. } @@ -468,22 +451,22 @@ impl ProjectWorkspace { .map(|segment| sysroot.root().join(segment).join(&standalone_server_name)) .find(|server_path| std::fs::metadata(server_path).is_ok()) .ok_or_else(|| { - anyhow::anyhow!( + anyhow::format_err!( "cannot find proc-macro server in sysroot `{}`", - sysroot.root().display() + sysroot.root() ) }) } ProjectWorkspace::DetachedFiles { .. } => { - Err(anyhow::anyhow!("cannot find proc-macro server, no sysroot was found")) + Err(anyhow::format_err!("cannot find proc-macro server, no sysroot was found")) } - ProjectWorkspace::Cargo { cargo, .. } => Err(anyhow::anyhow!( + ProjectWorkspace::Cargo { cargo, .. } => Err(anyhow::format_err!( "cannot find proc-macro-srv, the workspace `{}` is missing a sysroot", - cargo.workspace_root().display() + cargo.workspace_root() )), - ProjectWorkspace::Json { project, .. } => Err(anyhow::anyhow!( + ProjectWorkspace::Json { project, .. } => Err(anyhow::format_err!( "cannot find proc-macro-srv, the workspace `{}` is missing a sysroot", - project.path().display() + project.path() )), } } diff --git a/crates/rust-analyzer/src/cli.rs b/crates/rust-analyzer/src/cli.rs index e352019211..34cd595634 100644 --- a/crates/rust-analyzer/src/cli.rs +++ b/crates/rust-analyzer/src/cli.rs @@ -15,7 +15,6 @@ mod progress_report; use std::io::Read; -use anyhow::Result; use ide::AnalysisHost; use vfs::Vfs; @@ -36,7 +35,7 @@ impl Verbosity { } } -fn read_stdin() -> Result<String> { +fn read_stdin() -> anyhow::Result<String> { let mut buff = String::new(); std::io::stdin().read_to_string(&mut buff)?; Ok(buff) diff --git a/crates/rust-analyzer/src/cli/analysis_stats.rs b/crates/rust-analyzer/src/cli/analysis_stats.rs index 4cb917ce29..8654f68e95 100644 --- a/crates/rust-analyzer/src/cli/analysis_stats.rs +++ b/crates/rust-analyzer/src/cli/analysis_stats.rs @@ -37,7 +37,7 @@ use crate::cli::{ load_cargo::{load_workspace, LoadCargoConfig, ProcMacroServerChoice}, print_memory_usage, progress_report::ProgressReport, - report_metric, Result, Verbosity, + report_metric, Verbosity, }; /// Need to wrap Snapshot to provide `Clone` impl for `map_with` @@ -49,7 +49,7 @@ impl<DB: ParallelDatabase> Clone for Snap<salsa::Snapshot<DB>> { } impl flags::AnalysisStats { - pub fn run(self, verbosity: Verbosity) -> Result<()> { + pub fn run(self, verbosity: Verbosity) -> anyhow::Result<()> { let mut rng = { let seed = SystemTime::now().duration_since(UNIX_EPOCH).unwrap().as_millis() as u64; Rand32::new(seed) diff --git a/crates/rust-analyzer/src/cli/load_cargo.rs b/crates/rust-analyzer/src/cli/load_cargo.rs index 4e8f999716..1b413029e0 100644 --- a/crates/rust-analyzer/src/cli/load_cargo.rs +++ b/crates/rust-analyzer/src/cli/load_cargo.rs @@ -2,7 +2,6 @@ //! for incorporating changes. use std::path::Path; -use anyhow::{anyhow, Result}; use crossbeam_channel::{unbounded, Receiver}; use ide::{AnalysisHost, Change}; use ide_db::{ @@ -38,7 +37,7 @@ pub fn load_workspace_at( cargo_config: &CargoConfig, load_config: &LoadCargoConfig, progress: &dyn Fn(String), -) -> Result<(AnalysisHost, vfs::Vfs, Option<ProcMacroServer>)> { +) -> anyhow::Result<(AnalysisHost, vfs::Vfs, Option<ProcMacroServer>)> { let root = AbsPathBuf::assert(std::env::current_dir()?.join(root)); let root = ProjectManifest::discover_single(&root)?; let mut workspace = ProjectWorkspace::load(root, cargo_config, progress)?; @@ -60,7 +59,7 @@ pub fn load_workspace( ws: ProjectWorkspace, extra_env: &FxHashMap<String, String>, load_config: &LoadCargoConfig, -) -> Result<(AnalysisHost, vfs::Vfs, Option<ProcMacroServer>)> { +) -> anyhow::Result<(AnalysisHost, vfs::Vfs, Option<ProcMacroServer>)> { let (sender, receiver) = unbounded(); let mut vfs = vfs::Vfs::default(); let mut loader = { @@ -76,7 +75,7 @@ pub fn load_workspace( ProcMacroServerChoice::Explicit(path) => { ProcMacroServer::spawn(path.clone()).map_err(Into::into) } - ProcMacroServerChoice::None => Err(anyhow!("proc macro server disabled")), + ProcMacroServerChoice::None => Err(anyhow::format_err!("proc macro server disabled")), }; let (crate_graph, proc_macros) = ws.to_crate_graph( diff --git a/crates/rust-analyzer/src/cli/lsif.rs b/crates/rust-analyzer/src/cli/lsif.rs index 7f5d084496..6506f4cc4f 100644 --- a/crates/rust-analyzer/src/cli/lsif.rs +++ b/crates/rust-analyzer/src/cli/lsif.rs @@ -20,7 +20,6 @@ use crate::cli::load_cargo::ProcMacroServerChoice; use crate::cli::{ flags, load_cargo::{load_workspace, LoadCargoConfig}, - Result, }; use crate::line_index::{LineEndings, LineIndex, PositionEncoding}; use crate::to_proto; @@ -286,7 +285,7 @@ impl LsifManager<'_> { } impl flags::Lsif { - pub fn run(self) -> Result<()> { + pub fn run(self) -> anyhow::Result<()> { eprintln!("Generating LSIF started..."); let now = Instant::now(); let mut cargo_config = CargoConfig::default(); diff --git a/crates/rust-analyzer/src/cli/scip.rs b/crates/rust-analyzer/src/cli/scip.rs index b0b724bdfe..7ad68527d2 100644 --- a/crates/rust-analyzer/src/cli/scip.rs +++ b/crates/rust-analyzer/src/cli/scip.rs @@ -22,11 +22,10 @@ use std::env; use crate::cli::{ flags, load_cargo::{load_workspace, LoadCargoConfig}, - Result, }; impl flags::Scip { - pub fn run(self) -> Result<()> { + pub fn run(self) -> anyhow::Result<()> { eprintln!("Generating SCIP start..."); let now = Instant::now(); let mut cargo_config = CargoConfig::default(); @@ -65,7 +64,7 @@ impl flags::Scip { path.normalize() .as_os_str() .to_str() - .ok_or(anyhow::anyhow!("Unable to normalize project_root path"))? + .ok_or(anyhow::format_err!("Unable to normalize project_root path"))? ), text_document_encoding: scip_types::TextEncoding::UTF8.into(), special_fields: Default::default(), @@ -168,7 +167,7 @@ impl flags::Scip { let out_path = self.output.unwrap_or_else(|| PathBuf::from(r"index.scip")); scip::write_message_to_file(out_path, index) - .map_err(|err| anyhow::anyhow!("Failed to write scip to file: {}", err))?; + .map_err(|err| anyhow::format_err!("Failed to write scip to file: {}", err))?; eprintln!("Generating SCIP finished {:?}", now.elapsed()); Ok(()) diff --git a/crates/rust-analyzer/src/cli/ssr.rs b/crates/rust-analyzer/src/cli/ssr.rs index 82a769347d..7ccb3e216a 100644 --- a/crates/rust-analyzer/src/cli/ssr.rs +++ b/crates/rust-analyzer/src/cli/ssr.rs @@ -1,16 +1,16 @@ //! Applies structured search replace rules from the command line. +use anyhow::Context; use ide_ssr::MatchFinder; use project_model::{CargoConfig, RustLibSource}; use crate::cli::{ flags, load_cargo::{load_workspace_at, LoadCargoConfig, ProcMacroServerChoice}, - Result, }; impl flags::Ssr { - pub fn run(self) -> Result<()> { + pub fn run(self) -> anyhow::Result<()> { use ide_db::base_db::SourceDatabaseExt; let mut cargo_config = CargoConfig::default(); cargo_config.sysroot = Some(RustLibSource::Discover); @@ -35,7 +35,8 @@ impl flags::Ssr { if let Some(path) = vfs.file_path(file_id).as_path() { let mut contents = db.file_text(file_id).to_string(); edit.apply(&mut contents); - std::fs::write(path, contents)?; + std::fs::write(path, contents) + .with_context(|| format!("failed to write {path}"))?; } } Ok(()) @@ -46,7 +47,7 @@ impl flags::Search { /// Searches for `patterns`, printing debug information for any nodes whose text exactly matches /// `debug_snippet`. This is intended for debugging and probably isn't in it's current form useful /// for much else. - pub fn run(self) -> Result<()> { + pub fn run(self) -> anyhow::Result<()> { use ide_db::base_db::SourceDatabaseExt; use ide_db::symbol_index::SymbolsDatabase; let cargo_config = CargoConfig::default(); diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index 52512b2197..19a931f65b 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -318,7 +318,7 @@ impl GlobalState { // crate see https://github.com/rust-lang/rust-analyzer/issues/13029 if let Some((path, force_crate_graph_reload)) = workspace_structure_change { self.fetch_workspaces_queue.request_op( - format!("workspace vfs file change: {}", path.display()), + format!("workspace vfs file change: {path}"), force_crate_graph_reload, ); } diff --git a/crates/rust-analyzer/src/handlers/notification.rs b/crates/rust-analyzer/src/handlers/notification.rs index 3f94f2720c..e830e5e9a6 100644 --- a/crates/rust-analyzer/src/handlers/notification.rs +++ b/crates/rust-analyzer/src/handlers/notification.rs @@ -127,7 +127,7 @@ pub(crate) fn handle_did_save_text_document( if reload::should_refresh_for_change(abs_path, ChangeKind::Modify) { state .fetch_workspaces_queue - .request_op(format!("DidSaveTextDocument {}", abs_path.display()), false); + .request_op(format!("DidSaveTextDocument {abs_path}"), false); } } diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index 07e284e346..f9e6152f5b 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs @@ -307,7 +307,7 @@ impl GlobalState { res.map_or_else( |_| Err("proc macro crate is missing dylib".to_owned()), |(crate_name, path)| { - progress(path.display().to_string()); + progress(path.to_string()); client.as_ref().map_err(Clone::clone).and_then(|client| { load_proc_macro( client, @@ -407,9 +407,9 @@ impl GlobalState { .flat_map(|root| { root.include.into_iter().flat_map(|it| { [ - format!("{}/**/*.rs", it.display()), - format!("{}/**/Cargo.toml", it.display()), - format!("{}/**/Cargo.lock", it.display()), + format!("{it}/**/*.rs"), + format!("{it}/**/Cargo.toml"), + format!("{it}/**/Cargo.lock"), ] }) }) @@ -447,17 +447,13 @@ impl GlobalState { None => ws.find_sysroot_proc_macro_srv()?, }; - tracing::info!("Using proc-macro server at {}", path.display(),); + tracing::info!("Using proc-macro server at {path}"); ProcMacroServer::spawn(path.clone()).map_err(|err| { tracing::error!( - "Failed to run proc-macro server from path {}, error: {:?}", - path.display(), - err + "Failed to run proc-macro server from path {path}, error: {err:?}", ); - anyhow::anyhow!( - "Failed to run proc-macro server from path {}, error: {:?}", - path.display(), - err + anyhow::format_err!( + "Failed to run proc-macro server from path {path}, error: {err:?}", ) }) }) @@ -787,14 +783,13 @@ pub(crate) fn load_proc_macro( return match res { Ok(proc_macros) => { tracing::info!( - "Loaded proc-macros for {}: {:?}", - path.display(), + "Loaded proc-macros for {path}: {:?}", proc_macros.iter().map(|it| it.name.clone()).collect::<Vec<_>>() ); Ok(proc_macros) } Err(e) => { - tracing::warn!("proc-macro loading for {} failed: {e}", path.display()); + tracing::warn!("proc-macro loading for {path} failed: {e}"); Err(e) } }; diff --git a/crates/vfs/src/vfs_path.rs b/crates/vfs/src/vfs_path.rs index d327f2edf1..f9248feb34 100644 --- a/crates/vfs/src/vfs_path.rs +++ b/crates/vfs/src/vfs_path.rs @@ -292,7 +292,7 @@ impl From<AbsPathBuf> for VfsPath { impl fmt::Display for VfsPath { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match &self.0 { - VfsPathRepr::PathBuf(it) => fmt::Display::fmt(&it.display(), f), + VfsPathRepr::PathBuf(it) => fmt::Debug::fmt(&it, f), VfsPathRepr::VirtualPath(VirtualPath(it)) => fmt::Display::fmt(it, f), } } @@ -307,7 +307,7 @@ impl fmt::Debug for VfsPath { impl fmt::Debug for VfsPathRepr { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match &self { - VfsPathRepr::PathBuf(it) => fmt::Debug::fmt(&it.display(), f), + VfsPathRepr::PathBuf(it) => fmt::Display::fmt(&it, f), VfsPathRepr::VirtualPath(VirtualPath(it)) => fmt::Debug::fmt(&it, f), } } diff --git a/docs/dev/style.md b/docs/dev/style.md index d2a03fba40..786127639c 100644 --- a/docs/dev/style.md +++ b/docs/dev/style.md @@ -869,6 +869,19 @@ type -> ty **Rationale:** consistency. +## Error Handling Trivia + +Use `anyhow::Result` rather than just `Result`. + +**Rationale:** makes it immediately clear what result that is. + +Use `anyhow::format_err!` rather than `anyhow::anyhow`. + +**Rationale:** consistent, boring, avoids stuttering. + +There's no specific guidance on the formatting of error messages, see [anyhow/#209](https://github.com/dtolnay/anyhow/issues/209). +Do not end error and context messages with `.` though. + ## Early Returns Do use early returns diff --git a/xtask/src/install.rs b/xtask/src/install.rs index 83223a551d..e8c00c72e0 100644 --- a/xtask/src/install.rs +++ b/xtask/src/install.rs @@ -2,13 +2,13 @@ use std::{env, path::PathBuf, str}; -use anyhow::{bail, format_err, Context, Result}; +use anyhow::{bail, format_err, Context}; use xshell::{cmd, Shell}; use crate::flags; impl flags::Install { - pub(crate) fn run(self, sh: &Shell) -> Result<()> { + pub(crate) fn run(self, sh: &Shell) -> anyhow::Result<()> { if cfg!(target_os = "macos") { fix_path_for_mac(sh).context("Fix path for mac")?; } @@ -39,7 +39,7 @@ pub(crate) enum Malloc { Jemalloc, } -fn fix_path_for_mac(sh: &Shell) -> Result<()> { +fn fix_path_for_mac(sh: &Shell) -> anyhow::Result<()> { let mut vscode_path: Vec<PathBuf> = { const COMMON_APP_PATH: &str = r"/Applications/Visual Studio Code.app/Contents/Resources/app/bin"; @@ -68,7 +68,7 @@ fn fix_path_for_mac(sh: &Shell) -> Result<()> { Ok(()) } -fn install_client(sh: &Shell, client_opt: ClientOpt) -> Result<()> { +fn install_client(sh: &Shell, client_opt: ClientOpt) -> anyhow::Result<()> { let _dir = sh.push_dir("./editors/code"); // Package extension. @@ -129,7 +129,7 @@ fn install_client(sh: &Shell, client_opt: ClientOpt) -> Result<()> { Ok(()) } -fn install_server(sh: &Shell, opts: ServerOpt) -> Result<()> { +fn install_server(sh: &Shell, opts: ServerOpt) -> anyhow::Result<()> { let features = match opts.malloc { Malloc::System => &[][..], Malloc::Mimalloc => &["--features", "mimalloc"], diff --git a/xtask/src/publish.rs b/xtask/src/publish.rs index cdb7d8fac8..42b384ead0 100644 --- a/xtask/src/publish.rs +++ b/xtask/src/publish.rs @@ -1,12 +1,12 @@ mod notes; use crate::flags; -use anyhow::{anyhow, bail, Result}; +use anyhow::bail; use std::env; use xshell::{cmd, Shell}; impl flags::PublishReleaseNotes { - pub(crate) fn run(self, sh: &Shell) -> Result<()> { + pub(crate) fn run(self, sh: &Shell) -> anyhow::Result<()> { let asciidoc = sh.read_file(&self.changelog)?; let mut markdown = notes::convert_asciidoc_to_markdown(std::io::Cursor::new(&asciidoc))?; let file_name = check_file_name(self.changelog)?; @@ -24,11 +24,11 @@ impl flags::PublishReleaseNotes { } } -fn check_file_name<P: AsRef<std::path::Path>>(path: P) -> Result<String> { +fn check_file_name<P: AsRef<std::path::Path>>(path: P) -> anyhow::Result<String> { let file_name = path .as_ref() .file_name() - .ok_or_else(|| anyhow!("file name is not specified as `changelog`"))? + .ok_or_else(|| anyhow::format_err!("file name is not specified as `changelog`"))? .to_string_lossy(); let mut chars = file_name.chars(); @@ -61,7 +61,7 @@ fn create_original_changelog_url(file_name: &str) -> String { format!("https://rust-analyzer.github.io/thisweek/{year}/{month}/{day}/{stem}.html") } -fn update_release(sh: &Shell, tag_name: &str, release_notes: &str) -> Result<()> { +fn update_release(sh: &Shell, tag_name: &str, release_notes: &str) -> anyhow::Result<()> { let token = match env::var("GITHUB_TOKEN") { Ok(token) => token, Err(_) => bail!("Please obtain a personal access token from https://github.com/settings/tokens and set the `GITHUB_TOKEN` environment variable."), |