Skip to content

Commit

Permalink
fix: remove ambiguity between options and results trait methods
Browse files Browse the repository at this point in the history
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 compatiblity methods (`context` and `with_context`) where
introduced as conditional required methods for the trait `WrapErr`, as
well as another separate trait `ContextCompat`, implemented for options.

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.

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
  • Loading branch information
ten3roberts committed Mar 12, 2024
1 parent 34bd1d9 commit 0cc07ae
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 49 deletions.
33 changes: 13 additions & 20 deletions eyre/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,42 +61,34 @@ where
Err(e) => Err(e.ext_report(msg())),
}
}
}

#[cfg(feature = "anyhow")]
fn context<D>(self, msg: D) -> Result<T, Report>
#[cfg(feature = "anyhow")]
impl<T, E> crate::ContextCompat<T> for Result<T, E>
where
Self: WrapErr<T, E>,
{
#[track_caller]
fn context<D>(self, msg: D) -> crate::Result<T, Report>
where
D: Display + Send + Sync + 'static,
{
self.wrap_err(msg)
}

#[cfg(feature = "anyhow")]
fn with_context<D, F>(self, msg: F) -> Result<T, Report>
#[track_caller]
fn with_context<D, F>(self, f: F) -> crate::Result<T, Report>
where
D: Display + Send + Sync + 'static,
F: FnOnce() -> D,
{
self.wrap_err_with(msg)
self.wrap_err_with(f)
}
}

#[cfg(feature = "anyhow")]
impl<T> crate::ContextCompat<T> for Option<T> {
fn wrap_err<D>(self, msg: D) -> Result<T, Report>
where
D: Display + Send + Sync + 'static,
{
self.context(msg)
}

fn wrap_err_with<D, F>(self, msg: F) -> Result<T, Report>
where
D: Display + Send + Sync + 'static,
F: FnOnce() -> D,
{
self.with_context(msg)
}

#[track_caller]
fn context<D>(self, msg: D) -> Result<T, Report>
where
D: Display + Send + Sync + 'static,
Expand All @@ -107,6 +99,7 @@ impl<T> crate::ContextCompat<T> for Option<T> {
}
}

#[track_caller]
fn with_context<D, F>(self, msg: F) -> Result<T, Report>
where
D: Display + Send + Sync + 'static,
Expand Down
40 changes: 13 additions & 27 deletions eyre/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1125,21 +1125,6 @@ pub trait WrapErr<T, E>: 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<D>(self, msg: D) -> Result<T, Report>
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<D, F>(self, f: F) -> Result<T, Report>
where
D: Display + Send + Sync + 'static,
F: FnOnce() -> D;
}

/// Provides the [`ok_or_eyre`][OptionExt::ok_or_eyre] method for [`Option`].
Expand Down Expand Up @@ -1197,7 +1182,8 @@ pub trait OptionExt<T>: 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`.
Expand Down Expand Up @@ -1257,18 +1243,18 @@ pub trait ContextCompat<T>: context::private::Sealed {
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<D>(self, msg: D) -> Result<T, Report>
where
D: Display + Send + Sync + 'static;
// /// Compatibility re-export of `context` for porting from `anyhow` to `eyre`
// #[cfg_attr(track_caller, track_caller)]
// fn wrap_err<D>(self, msg: D) -> Result<T, Report>
// 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<D, F>(self, f: F) -> Result<T, Report>
where
D: Display + Send + Sync + 'static,
F: FnOnce() -> D;
// /// Compatibility re-export of `with_context` for porting from `anyhow` to `eyre`
// #[cfg_attr(track_caller, track_caller)]
// fn wrap_err_with<D, F>(self, f: F) -> Result<T, Report>
// where
// D: Display + Send + Sync + 'static,
// F: FnOnce() -> D;
}

/// Equivalent to Ok::<_, eyre::Error>(value).
Expand Down
8 changes: 6 additions & 2 deletions eyre/tests/test_location.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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))
Expand All @@ -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);
Expand All @@ -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);
Expand Down

0 comments on commit 0cc07ae

Please sign in to comment.