Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for source forwarding in Error derive #293

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
([#279](https://github.com/JelteF/derive_more/pull/279))
- `derive_more::derive` module exporting only macros, without traits.
([#290](https://github.com/JelteF/derive_more/pull/290))
- Add support for source forwarding in `Error` derive.
([#293](https://github.com/JelteF/derive_more/pull/293))

### Changed

Expand Down
33 changes: 33 additions & 0 deletions impl/doc/error.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ often [`From` as well](crate::From).
called `Backtrace`. Then it would return that field as the `backtrace`.
3. One of the fields is annotated with `#[error(backtrace)]`. Then it would
return that field as the `backtrace`.
4. The source field is annotated with `#[error(backtrace)]`. Then it would
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy paste error?

Suggested change
4. The source field is annotated with `#[error(backtrace)]`. Then it would
4. The source field is annotated with `#[error(forward)]`. Then it would

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No that's correct, provide is forwarded only when the source is annotated with #[error(backtrace)].

That behaviour is unchanged in this PR.

The reasoning for keeping a separate attribute is to make source forwarding work on stable, since there's no good way of generating feature-gated code.

I also think this is somewhat unintuitive, but I haven't come up with a better idea.

Perhaps it could make sense to disallow #[error(backtrace)] on the source field if there's no #[error(forward)]?

forward implementation to the source.

If the source field is annotated with `#[error(backtrace)]`, and another field
is also annotated/was inferred, both backtraces will be provided.

### Ignoring fields for derives

Expand All @@ -41,6 +46,14 @@ detecting `backtrace` and `source`. It's also possible to mark a field only
ignored for one of these methods by using `#[error(not(backtrace))]` or
`#[error(not(source))]`.

### Source Forwarding

A struct, enum, or enum variant can be annotated with `#[error(forward)]` to forward
the `source()` implementation to the source field (inferred or explicitly annotated),
instead of returning the field itself.

In general this approach is only recommended if the error is intended to be fully
transparent, and forwards implementation of [`Display`](crate::Display) as well.

### What works in `no_std`?

Expand Down Expand Up @@ -127,6 +140,17 @@ enum CompoundError {
},
Tuple(WithExplicitSource),
WithoutSource(#[error(not(source))] Tuple),
#[error(forward)]
#[from(ignore)]
ForwardedImplicitSource {
source: WithSource,
},
#[error(forward)]
#[from(ignore)]
ForwardedExplicitSourceWithBacktrace {
#[error(source, backtrace)]
explicit_source: WithSourceAndBacktrace,
}
}

assert!(Simple.source().is_none());
Expand All @@ -147,5 +171,14 @@ assert!(CompoundError::from(Simple).source().is_some());
assert!(CompoundError::from(WithSource::default()).source().is_some());
assert!(CompoundError::from(WithExplicitSource::default()).source().is_some());
assert!(CompoundError::from(Tuple::default()).source().is_none());

let forwarded = CompoundError::ForwardedImplicitSource { source: WithSource::default() };
assert!(forwarded.source().is_some());
assert!(forwarded.source().unwrap().is::<Simple>());

let forwarded_with_backtrace = CompoundError::ForwardedExplicitSourceWithBacktrace { explicit_source: WithSourceAndBacktrace { source: Simple, backtrace: Backtrace::capture() } };
assert!(forwarded_with_backtrace.source().is_some());
assert!(forwarded_with_backtrace.source().unwrap().is::<Simple>());
assert!(request_ref::<Backtrace>(&forwarded_with_backtrace).is_some());
# }
```
38 changes: 30 additions & 8 deletions impl/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use proc_macro2::TokenStream;
use proc_macro2::{Span, TokenStream};
use quote::quote;
use syn::{spanned::Spanned as _, Error, Result};

Expand Down Expand Up @@ -110,9 +110,13 @@ fn render_enum(
let mut source_match_arms = Vec::new();
let mut provide_match_arms = Vec::new();

for variant in state.enabled_variant_data().variants {
let variant_data = state.enabled_variant_data();

for (variant, variant_info) in variant_data.variants.iter().zip(&variant_data.infos)
{
let default_info = FullMetaInfo {
enabled: true,
forward: variant_info.forward,
..FullMetaInfo::default()
};

Expand Down Expand Up @@ -160,9 +164,9 @@ fn render_enum(

fn allowed_attr_params() -> AttrParams {
AttrParams {
enum_: vec!["ignore"],
struct_: vec!["ignore"],
variant: vec!["ignore"],
enum_: vec!["ignore", "forward"],
struct_: vec!["ignore", "forward"],
variant: vec!["ignore", "forward"],
field: vec!["ignore", "source", "backtrace"],
}
}
Expand All @@ -189,13 +193,21 @@ impl<'input, 'state> ParsedFields<'input, 'state> {
fn render_source_as_struct(&self) -> Option<TokenStream> {
let source = self.source?;
let ident = &self.data.members[source];
Some(render_some(quote! { #ident }))
if self.data.infos[source].forward {
Some(quote! { ::derive_more::Error::source(&#ident) })
} else {
Some(render_some(quote! { #ident }))
}
}

fn render_source_as_enum_variant_match_arm(&self) -> Option<TokenStream> {
let source = self.source?;
let pattern = self.data.matcher(&[source], &[quote! { source }]);
let expr = render_some(quote! { source });
let expr = if self.data.infos[source].forward {
quote! { ::derive_more::Error::source(source) }
} else {
render_some(quote! { source })
};
Some(quote! { #pattern => #expr })
}

Expand Down Expand Up @@ -378,7 +390,12 @@ fn parse_fields_impl<'input, 'state, P>(
where
P: Fn(&str, &syn::Field, usize) -> bool,
{
let MultiFieldData { fields, infos, .. } = state.enabled_fields_data();
let MultiFieldData {
fields,
infos,
variant_info,
..
} = state.enabled_fields_data();

let iter = fields
.iter()
Expand Down Expand Up @@ -406,6 +423,11 @@ where

if let Some((index, _, _)) = source {
parsed_fields.source = Some(index);
} else if variant_info.forward {
return Err(syn::Error::new(
Span::call_site(),
"`#[error(forward)]` cannot be used when an error has no source",
));
}

if let Some((index, _, _)) = backtrace {
Expand Down
18 changes: 18 additions & 0 deletions tests/compile_fail/error/forward_no_source_enum.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
use derive_more::Error;

#[derive(Debug, Error)]
#[error(forward)]
enum Foo {
Bar,
Baz {
source: Box<dyn Error + Send + 'static>,
},
}

impl ::core::fmt::Display for Foo {
fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
write!(f, "")
}
}

fn main() {}
7 changes: 7 additions & 0 deletions tests/compile_fail/error/forward_no_source_enum.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
error: `#[error(forward)]` cannot be used when an error has no source
--> tests/compile_fail/error/forward_no_source_enum.rs:3:17
|
3 | #[derive(Debug, Error)]
| ^^^^^
|
= note: this error originates in the derive macro `Error` (in Nightly builds, run with -Z macro-backtrace for more info)
13 changes: 13 additions & 0 deletions tests/compile_fail/error/forward_no_source_struct.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
use derive_more::Error;

#[derive(Debug, Error)]
#[error(forward)]
struct Foo;

impl ::core::fmt::Display for Foo {
fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
write!(f, "")
}
}

fn main() {}
7 changes: 7 additions & 0 deletions tests/compile_fail/error/forward_no_source_struct.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
error: `#[error(forward)]` cannot be used when an error has no source
--> tests/compile_fail/error/forward_no_source_struct.rs:3:17
|
3 | #[derive(Debug, Error)]
| ^^^^^
|
= note: this error originates in the derive macro `Error` (in Nightly builds, run with -Z macro-backtrace for more info)
19 changes: 19 additions & 0 deletions tests/compile_fail/error/forward_no_source_variant.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
use derive_more::Error;

#[derive(Debug, Error)]
enum Foo {
#[error(forward)]
Bar,
#[error(forward)]
Baz {
source: Box<dyn Error + Send + 'static>,
},
}

impl ::core::fmt::Display for Foo {
fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
write!(f, "")
}
}

fn main() {}
7 changes: 7 additions & 0 deletions tests/compile_fail/error/forward_no_source_variant.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
error: `#[error(forward)]` cannot be used when an error has no source
--> tests/compile_fail/error/forward_no_source_variant.rs:3:17
|
3 | #[derive(Debug, Error)]
| ^^^^^
|
= note: this error originates in the derive macro `Error` (in Nightly builds, run with -Z macro-backtrace for more info)
83 changes: 83 additions & 0 deletions tests/error/derives_forward.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
use super::*;

derive_display!(Inner);
#[derive(Debug, Error)]
struct Inner {
source: SimpleErr,
}

derive_display!(StructAttr);
#[derive(Debug, Error)]
#[error(forward)]
struct StructAttr {
source: Inner,
}

#[test]
fn struct_attr() {
let err = StructAttr {
source: Inner { source: SimpleErr },
};

assert!(err.source().is_some());
assert!(err.source().unwrap().is::<SimpleErr>());
}

derive_display!(EnumAttr);
#[derive(Debug, Error)]
#[error(forward)]
enum EnumAttr {
A {
source: Inner,
},
B {
#[error(source)]
explicit_source: Inner,
},
}

#[test]
fn enum_attr() {
let err_a = EnumAttr::A {
source: Inner { source: SimpleErr },
};

let err_b = EnumAttr::B {
explicit_source: Inner { source: SimpleErr },
};

assert!(err_a.source().is_some());
assert!(err_a.source().unwrap().is::<SimpleErr>());

assert!(err_b.source().is_some());
assert!(err_b.source().unwrap().is::<SimpleErr>());
}

derive_display!(VariantAttr);
#[derive(Debug, Error)]
enum VariantAttr {
#[error(forward)]
A {
source: Inner,
},
B {
source: Inner,
},
}

#[test]
fn variant_attr() {
let err_a = VariantAttr::A {
source: Inner { source: SimpleErr },
};

let err_b = VariantAttr::B {
source: Inner { source: SimpleErr },
};

assert!(err_a.source().is_some());
assert!(err_a.source().unwrap().is::<SimpleErr>());

assert!(err_b.source().is_some());
assert!(err_b.source().unwrap().is::<Inner>());
}
1 change: 1 addition & 0 deletions tests/error/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ mod derives_for_enums_with_source;
mod derives_for_generic_enums_with_source;
mod derives_for_generic_structs_with_source;
mod derives_for_structs_with_source;
mod derives_forward;

#[cfg(all(feature = "std", nightly))]
mod nightly;
Expand Down
Loading