From a03951740c34e14055fc75f1981dcbb6316fd3fb Mon Sep 17 00:00:00 2001 From: Freja Roberts Date: Wed, 24 Jan 2024 17:32:27 +0100 Subject: [PATCH 01/10] fix: remove ambiguity between options and results trait methods 1. Remove the conditional trait methods `context` and `with_context` from `WrapErr` 2. Remove ambiguity between "wrapping errors" with another, and converting a `None` value to an error. The anyhow compatibility methods (`context` and `with_context`) were introduced as conditional required methods for the trait `WrapErr`, as well as another separate trait `ContextCompat`, implemented for options *and* results. The latter trait also provided methods `wrap_err` and `wrap_err_with`. This led to confusion as the two distinct traits had the same methods, one was implemented on results, and the other on both options and results. Furthermore, wrapping options does not align with our error model where errors are wrapped around other errors; an error wrapping `None` should just be a single error and should not be wrapped. See: #149 --- eyre/src/context.rs | 33 +++++++++++++-------------------- eyre/src/lib.rs | 31 ++----------------------------- eyre/tests/test_location.rs | 8 ++++++-- 3 files changed, 21 insertions(+), 51 deletions(-) diff --git a/eyre/src/context.rs b/eyre/src/context.rs index aaba5bd..44ef6fe 100644 --- a/eyre/src/context.rs +++ b/eyre/src/context.rs @@ -61,42 +61,34 @@ where Err(e) => Err(e.ext_report(msg())), } } +} - #[cfg(feature = "anyhow")] - fn context(self, msg: D) -> Result +#[cfg(feature = "anyhow")] +impl crate::ContextCompat for Result +where + Self: WrapErr, +{ + #[track_caller] + fn context(self, msg: D) -> crate::Result where D: Display + Send + Sync + 'static, { self.wrap_err(msg) } - #[cfg(feature = "anyhow")] - fn with_context(self, msg: F) -> Result + #[track_caller] + fn with_context(self, f: F) -> crate::Result where D: Display + Send + Sync + 'static, F: FnOnce() -> D, { - self.wrap_err_with(msg) + self.wrap_err_with(f) } } #[cfg(feature = "anyhow")] impl crate::ContextCompat for Option { - fn wrap_err(self, msg: D) -> Result - where - D: Display + Send + Sync + 'static, - { - self.context(msg) - } - - fn wrap_err_with(self, msg: F) -> Result - where - D: Display + Send + Sync + 'static, - F: FnOnce() -> D, - { - self.with_context(msg) - } - + #[track_caller] fn context(self, msg: D) -> Result where D: Display + Send + Sync + 'static, @@ -107,6 +99,7 @@ impl crate::ContextCompat for Option { } } + #[track_caller] fn with_context(self, msg: F) -> Result where D: Display + Send + Sync + 'static, diff --git a/eyre/src/lib.rs b/eyre/src/lib.rs index 33c60d3..65b3980 100644 --- a/eyre/src/lib.rs +++ b/eyre/src/lib.rs @@ -1125,21 +1125,6 @@ pub trait WrapErr: context::private::Sealed { where D: Display + Send + Sync + 'static, F: FnOnce() -> D; - - /// Compatibility re-export of wrap_err for interop with `anyhow` - #[cfg(feature = "anyhow")] - #[cfg_attr(track_caller, track_caller)] - fn context(self, msg: D) -> Result - where - D: Display + Send + Sync + 'static; - - /// Compatibility re-export of wrap_err_with for interop with `anyhow` - #[cfg(feature = "anyhow")] - #[cfg_attr(track_caller, track_caller)] - fn with_context(self, f: F) -> Result - where - D: Display + Send + Sync + 'static, - F: FnOnce() -> D; } /// Provides the [`ok_or_eyre`][OptionExt::ok_or_eyre] method for [`Option`]. @@ -1197,7 +1182,8 @@ pub trait OptionExt: context::private::Sealed { M: Debug + Display + Send + Sync + 'static; } -/// Provides the `context` method for `Option` when porting from `anyhow` +/// Provides the `context` and `with_context` methods for `Result` and `Option` to enhance +/// compatibility when porting from anyhow. /// /// This trait is sealed and cannot be implemented for types outside of /// `eyre`. @@ -1256,19 +1242,6 @@ pub trait ContextCompat: context::private::Sealed { where D: Display + Send + Sync + 'static, F: FnOnce() -> D; - - /// Compatibility re-export of `context` for porting from `anyhow` to `eyre` - #[cfg_attr(track_caller, track_caller)] - fn wrap_err(self, msg: D) -> Result - where - D: Display + Send + Sync + 'static; - - /// Compatibility re-export of `with_context` for porting from `anyhow` to `eyre` - #[cfg_attr(track_caller, track_caller)] - fn wrap_err_with(self, f: F) -> Result - where - D: Display + Send + Sync + 'static, - F: FnOnce() -> D; } /// Equivalent to Ok::<_, eyre::Error>(value). diff --git a/eyre/tests/test_location.rs b/eyre/tests/test_location.rs index 53c181f..6a6ccfd 100644 --- a/eyre/tests/test_location.rs +++ b/eyre/tests/test_location.rs @@ -101,6 +101,8 @@ fn test_option_ok_or_eyre() { #[cfg(feature = "anyhow")] #[test] fn test_context() { + use eyre::ContextCompat; + let _ = eyre::set_hook(Box::new(|_e| { let expected_location = file!(); Box::new(LocationHandler::new(expected_location)) @@ -117,6 +119,8 @@ fn test_context() { #[cfg(feature = "anyhow")] #[test] fn test_with_context() { + use eyre::ContextCompat; + let _ = eyre::set_hook(Box::new(|_e| { let expected_location = file!(); Box::new(LocationHandler::new(expected_location)) @@ -139,7 +143,7 @@ fn test_option_compat_wrap_err() { })); use eyre::ContextCompat; - let err = None::<()>.wrap_err("oopsie").unwrap_err(); + let err = None::<()>.context("oopsie").unwrap_err(); // should panic if the location isn't in our crate println!("{:?}", err); @@ -154,7 +158,7 @@ fn test_option_compat_wrap_err_with() { })); use eyre::ContextCompat; - let err = None::<()>.wrap_err_with(|| "oopsie").unwrap_err(); + let err = None::<()>.with_context(|| "oopsie").unwrap_err(); // should panic if the location isn't in our crate println!("{:?}", err); From 90645efddcefa475a37608184cb3a699685ffc65 Mon Sep 17 00:00:00 2001 From: Freja Roberts Date: Fri, 29 Mar 2024 10:51:15 +0100 Subject: [PATCH 02/10] fix: warnings from new clippy lints --- eyre/src/lib.rs | 6 +++--- eyre/tests/test_location.rs | 2 -- eyre/tests/test_repr.rs | 1 - 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/eyre/src/lib.rs b/eyre/src/lib.rs index 65b3980..1c5231f 100644 --- a/eyre/src/lib.rs +++ b/eyre/src/lib.rs @@ -624,7 +624,7 @@ fn capture_handler(error: &(dyn StdError + 'static)) -> Box { } impl dyn EyreHandler { - /// + /// Check if the handler is of type `T` pub fn is(&self) -> bool { // Get `TypeId` of the type this function is instantiated with. let t = core::any::TypeId::of::(); @@ -636,7 +636,7 @@ impl dyn EyreHandler { t == concrete } - /// + /// Downcast the handler to a contcrete type `T` pub fn downcast_ref(&self) -> Option<&T> { if self.is::() { unsafe { Some(&*(self as *const dyn EyreHandler as *const T)) } @@ -645,7 +645,7 @@ impl dyn EyreHandler { } } - /// + /// Downcast the handler to a contcrete type `T` pub fn downcast_mut(&mut self) -> Option<&mut T> { if self.is::() { unsafe { Some(&mut *(self as *mut dyn EyreHandler as *mut T)) } diff --git a/eyre/tests/test_location.rs b/eyre/tests/test_location.rs index 63f8709..2d199d9 100644 --- a/eyre/tests/test_location.rs +++ b/eyre/tests/test_location.rs @@ -107,7 +107,6 @@ fn test_context() { Box::new(LocationHandler::new(expected_location)) })); - use eyre::WrapErr; let err = read_path("totally_fake_path") .context("oopsie") .unwrap_err(); @@ -126,7 +125,6 @@ fn test_with_context() { Box::new(LocationHandler::new(expected_location)) })); - use eyre::WrapErr; let err = read_path("totally_fake_path") .with_context(|| "oopsie") .unwrap_err(); diff --git a/eyre/tests/test_repr.rs b/eyre/tests/test_repr.rs index 71a5663..5a1b5bf 100644 --- a/eyre/tests/test_repr.rs +++ b/eyre/tests/test_repr.rs @@ -4,7 +4,6 @@ mod drop; use self::common::maybe_install_handler; use self::drop::{DetectDrop, Flag}; use eyre::Report; -use std::marker::Unpin; use std::mem; #[test] From a0f3f641dfd19920a82737b2daae2ed853f9757e Mon Sep 17 00:00:00 2001 From: Freja Roberts Date: Thu, 16 May 2024 14:43:04 +0200 Subject: [PATCH 03/10] Fix overly strict clippy lint (#172) Deny style, suspicious and complexity categories, rather than all warnings If we find issues that could have been caught by a clippy lint we can add that lint on as *deny* too, as for now, they still appear as warnings but won't prevent CI from running. This most notably fixes random CI breakages when clippy/rust updates and introduces new warn level lints --- .github/workflows/ci.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9719f86..c2f107b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -135,8 +135,7 @@ jobs: - uses: actions-rs/cargo@v1 with: command: clippy - args: --all-targets --all-features -- -D warnings - + args: --all-targets --all-features -- -D clippy::style -D clippy::suspiscious -D clippy::complexity miri: name: Miri runs-on: ubuntu-latest From f21bfc253ceba07b9be8e4f5db5064827219361f Mon Sep 17 00:00:00 2001 From: Malloc Voidstar <1284317+AlyoshaVasilieva@users.noreply.github.com> Date: Wed, 29 May 2024 10:55:52 -0700 Subject: [PATCH 04/10] Fix typo in CI --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c2f107b..014e490 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -135,7 +135,7 @@ jobs: - uses: actions-rs/cargo@v1 with: command: clippy - args: --all-targets --all-features -- -D clippy::style -D clippy::suspiscious -D clippy::complexity + args: --all-targets --all-features -- -D clippy::style -D clippy::suspicious -D clippy::complexity miri: name: Miri runs-on: ubuntu-latest From dbf32d9cbe5994bbf5dea33998e50cfcca746a5f Mon Sep 17 00:00:00 2001 From: Malloc Voidstar <1284317+AlyoshaVasilieva@users.noreply.github.com> Date: Wed, 29 May 2024 15:49:13 -0700 Subject: [PATCH 05/10] Remove empty doc comments --- eyre/src/lib.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/eyre/src/lib.rs b/eyre/src/lib.rs index 52828b0..ea00ac1 100644 --- a/eyre/src/lib.rs +++ b/eyre/src/lib.rs @@ -624,7 +624,6 @@ fn capture_handler(error: &(dyn StdError + 'static)) -> Box { } impl dyn EyreHandler { - /// pub fn is(&self) -> bool { // Get `TypeId` of the type this function is instantiated with. let t = core::any::TypeId::of::(); @@ -636,7 +635,6 @@ impl dyn EyreHandler { t == concrete } - /// pub fn downcast_ref(&self) -> Option<&T> { if self.is::() { unsafe { Some(&*(self as *const dyn EyreHandler as *const T)) } @@ -645,7 +643,6 @@ impl dyn EyreHandler { } } - /// pub fn downcast_mut(&mut self) -> Option<&mut T> { if self.is::() { unsafe { Some(&mut *(self as *mut dyn EyreHandler as *mut T)) } From 4d6e73e5a9de72c79dd9c2d20fda6e0a1a32106c Mon Sep 17 00:00:00 2001 From: Tei Leelo Roberts Date: Fri, 21 Jun 2024 20:05:30 +0200 Subject: [PATCH 06/10] fix: different call machinery between linux and windows This filters out diverging backtraces caused by different before-main and test calling machinery on windows and linux --- color-eyre/tests/theme.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/color-eyre/tests/theme.rs b/color-eyre/tests/theme.rs index 716aaef..e0666e1 100644 --- a/color-eyre/tests/theme.rs +++ b/color-eyre/tests/theme.rs @@ -170,7 +170,12 @@ fn test_backwards_compatibility(target: String, file_name: &str) { fn normalize_backtrace(input: &str) -> String { input .lines() - .take_while(|v| !v.contains("core::panic") && !v.contains("theme_test_helper::main")) + .take_while(|v| { + !v.contains("core::panic") + && !v.contains("theme_test_helper::main") + && !v.contains("theme::test_error_backwards_compatibility::closure") + && !v.contains("theme::test_error_backwards_compatibility::{{closure}}") + }) .collect::>() .join("\n") } From 231090d5f664d73da0691a4f154a24b75bb1d74e Mon Sep 17 00:00:00 2001 From: Freja Roberts Date: Fri, 28 Jun 2024 10:58:09 +0200 Subject: [PATCH 07/10] fix: typo in doc comment --- eyre/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/eyre/src/lib.rs b/eyre/src/lib.rs index b3167f8..93ec747 100644 --- a/eyre/src/lib.rs +++ b/eyre/src/lib.rs @@ -636,7 +636,7 @@ impl dyn EyreHandler { t == concrete } - /// Downcast the handler to a contcrete type `T` + /// Downcast the handler to a concrete type `T` pub fn downcast_ref(&self) -> Option<&T> { if self.is::() { unsafe { Some(&*(self as *const dyn EyreHandler as *const T)) } @@ -645,7 +645,7 @@ impl dyn EyreHandler { } } - /// Downcast the handler to a contcrete type `T` + /// Downcast the handler to a concrete type `T` pub fn downcast_mut(&mut self) -> Option<&mut T> { if self.is::() { unsafe { Some(&mut *(self as *mut dyn EyreHandler as *mut T)) } From 0eabf81eef73e665bb48a08b383f6d13446f600d Mon Sep 17 00:00:00 2001 From: Tei Leelo Roberts Date: Fri, 28 Jun 2024 17:31:40 +0200 Subject: [PATCH 08/10] fix: import consistency --- eyre/tests/test_location.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/eyre/tests/test_location.rs b/eyre/tests/test_location.rs index 2d199d9..21f4716 100644 --- a/eyre/tests/test_location.rs +++ b/eyre/tests/test_location.rs @@ -100,13 +100,12 @@ fn test_option_ok_or_eyre() { #[cfg(feature = "anyhow")] #[test] fn test_context() { - use eyre::ContextCompat; - let _ = eyre::set_hook(Box::new(|_e| { let expected_location = file!(); Box::new(LocationHandler::new(expected_location)) })); + use eyre::ContextCompat; let err = read_path("totally_fake_path") .context("oopsie") .unwrap_err(); @@ -118,13 +117,12 @@ fn test_context() { #[cfg(feature = "anyhow")] #[test] fn test_with_context() { - use eyre::ContextCompat; - let _ = eyre::set_hook(Box::new(|_e| { let expected_location = file!(); Box::new(LocationHandler::new(expected_location)) })); + use eyre::ContextCompat; let err = read_path("totally_fake_path") .with_context(|| "oopsie") .unwrap_err(); From c98bc8cc1cef5eef28badbd8d19d6ab1a4b5a324 Mon Sep 17 00:00:00 2001 From: Tei Leelo Roberts Date: Fri, 28 Jun 2024 17:39:22 +0200 Subject: [PATCH 09/10] fix: consistent naming and duplicated tests --- eyre/tests/test_location.rs | 30 ------------------------------ 1 file changed, 30 deletions(-) diff --git a/eyre/tests/test_location.rs b/eyre/tests/test_location.rs index 21f4716..ad1941c 100644 --- a/eyre/tests/test_location.rs +++ b/eyre/tests/test_location.rs @@ -131,36 +131,6 @@ fn test_with_context() { println!("{:?}", err); } -#[cfg(feature = "anyhow")] -#[test] -fn test_option_compat_wrap_err() { - let _ = eyre::set_hook(Box::new(|_e| { - let expected_location = file!(); - Box::new(LocationHandler::new(expected_location)) - })); - - use eyre::ContextCompat; - let err = None::<()>.context("oopsie").unwrap_err(); - - // should panic if the location isn't in our crate - println!("{:?}", err); -} - -#[cfg(feature = "anyhow")] -#[test] -fn test_option_compat_wrap_err_with() { - let _ = eyre::set_hook(Box::new(|_e| { - let expected_location = file!(); - Box::new(LocationHandler::new(expected_location)) - })); - - use eyre::ContextCompat; - let err = None::<()>.with_context(|| "oopsie").unwrap_err(); - - // should panic if the location isn't in our crate - println!("{:?}", err); -} - #[cfg(feature = "anyhow")] #[test] fn test_option_compat_context() { From d717cf9cac6f7ca6cad3e28885ab54ad2888a5fc Mon Sep 17 00:00:00 2001 From: Josh McKinney Date: Sat, 29 Jun 2024 07:42:39 -0700 Subject: [PATCH 10/10] fix(color_eyre): build warnings Remove structs that are unused and have been migrated to use the eyre versions of the same: - color_eyre::config::InstallError -> eyre::InstallError - color_eyre::config::InstallThemeError -> eyre::InstallThemeError - color_eyre::config::InstallColorSpantraceThemeError -> no equivalent Add cfg guards to the DisplayExt, FooterWriter, Footer, and Header to prevent unused warnings when the issue-url feature is not enabled. --- color-eyre/src/config.rs | 33 --------------------------------- color-eyre/src/writers.rs | 8 ++++++++ 2 files changed, 8 insertions(+), 33 deletions(-) diff --git a/color-eyre/src/config.rs b/color-eyre/src/config.rs index 22b4e60..f8f19b4 100644 --- a/color-eyre/src/config.rs +++ b/color-eyre/src/config.rs @@ -11,39 +11,6 @@ use std::env; use std::fmt::Write as _; use std::{fmt, path::PathBuf, sync::Arc}; -#[derive(Debug)] -struct InstallError; - -impl fmt::Display for InstallError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str("could not install the BacktracePrinter as another was already installed") - } -} - -impl std::error::Error for InstallError {} - -#[derive(Debug)] -struct InstallThemeError; - -impl fmt::Display for InstallThemeError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str("could not set the provided `Theme` globally as another was already set") - } -} - -impl std::error::Error for InstallThemeError {} - -#[derive(Debug)] -struct InstallColorSpantraceThemeError; - -impl fmt::Display for InstallColorSpantraceThemeError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str("could not set the provided `Theme` via `color_spantrace::set_theme` globally as another was already set") - } -} - -impl std::error::Error for InstallColorSpantraceThemeError {} - /// A struct that represents a theme that is used by `color_eyre` #[derive(Debug, Copy, Clone, Default)] pub struct Theme { diff --git a/color-eyre/src/writers.rs b/color-eyre/src/writers.rs index b5bb344..ce828af 100644 --- a/color-eyre/src/writers.rs +++ b/color-eyre/src/writers.rs @@ -28,11 +28,13 @@ impl WriterExt for W { } } +#[cfg(feature = "issue-url")] pub(crate) trait DisplayExt: Sized + Display { fn with_header(self, header: H) -> Header; fn with_footer(self, footer: F) -> Footer; } +#[cfg(feature = "issue-url")] impl DisplayExt for T where T: Display, @@ -80,11 +82,13 @@ where } } +#[cfg(feature = "issue-url")] pub(crate) struct FooterWriter { inner: W, had_output: bool, } +#[cfg(feature = "issue-url")] impl fmt::Write for FooterWriter where W: fmt::Write, @@ -98,6 +102,7 @@ where } } +#[cfg(feature = "issue-url")] #[allow(explicit_outlives_requirements)] pub(crate) struct Footer where @@ -108,6 +113,7 @@ where footer: H, } +#[cfg(feature = "issue-url")] impl fmt::Display for Footer where B: Display, @@ -129,6 +135,7 @@ where } } +#[cfg(feature = "issue-url")] #[allow(explicit_outlives_requirements)] pub(crate) struct Header where @@ -139,6 +146,7 @@ where h: H, } +#[cfg(feature = "issue-url")] impl fmt::Display for Header where B: Display,