From a03951740c34e14055fc75f1981dcbb6316fd3fb Mon Sep 17 00:00:00 2001 From: Freja Roberts Date: Wed, 24 Jan 2024 17:32:27 +0100 Subject: [PATCH 1/6] 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 2/6] 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 4d6e73e5a9de72c79dd9c2d20fda6e0a1a32106c Mon Sep 17 00:00:00 2001 From: Tei Leelo Roberts Date: Fri, 21 Jun 2024 20:05:30 +0200 Subject: [PATCH 3/6] 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 4/6] 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 5/6] 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 6/6] 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() {