From 6ebe72b8438f9108efca3f4f20124fab49e5b813 Mon Sep 17 00:00:00 2001 From: Freja Roberts Date: Wed, 24 Jan 2024 17:32:27 +0100 Subject: [PATCH] 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 trait had the same methods, except one was implemented on results, and one of options and results. Further, this did not align with our error model, wherein errors are wrapped with other errors in a chain now that options (which are no errors and have no message) could 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);