Skip to content

Commit f93776e

Browse files
authored
Support Option as Error::source() in derive(Error) (#459, #426)
## Synopsis See #426 (comment): > I’d like to be able to use `derive_more` with optional sources, like: > ```rust > #[derive(derive_more::Error, derive_more::Display, Debug)] > #[display("it’s an error")] > struct MyErr { > source: Option<Box<dyn std::error::Error + Send + Sync + 'static>>, > } > ``` ## Solution The way, the `thiserror` does it, is by [checking](https://docs.rs/thiserror-impl/2.0.12/src/thiserror_impl/expand.rs.html#50) the [type name to contain `Option`](https://docs.rs/thiserror-impl/2.0.12/src/thiserror_impl/expand.rs.html#559-583). Relying on type could be fragile, as the call site could introduce its own type named `Option` in the scope where macro is called. Ideally, we need to aid this case with type machinery. For supporting this via type machinery, we obviously need some sort of specialization to specialize case `Option<E: Error>` over `E: Error`, since vanilla Rust coherence naturally hits the wall of "upstream crates may add a new impl of trait `std::error::Error` for type `Option<_>` in future versions". The usual solution is to use [autoref-based specialization](https://lukaskalbertodt.github.io/2019/12/05/generalized-autoref-based-specialization.html) as [we do for `AsRef`](https://docs.rs/derive_more/2.0.1/src/derive_more/as.rs.html). However, autoref-based specialization doesn't work in a generic context (i.e. `MyError<E1, E2>`), since it requires trait bounds to work, and we cannot describe conditional trait bounds in Rust, which leads to the situation that this case could not be supported at all: ```rust struct MyError<E> { source: Option<E>, } ``` So, choosing between checking the field's type syntactically (and so loosing support for renamed types) and not supporting optional `source` in generic contexts, I would definitely choose to support generics despite having naming issues in edge cases. As the result, this PR implements the similar syntactically-based solution as the `thiserror` crate does. The implications of this are warned and described in the documentation. ## Future possibilities Additionally, we may support `#[error(source(optional))]` attribute, which will allow using renamed types naturally: ```rust use derive_more::{Display, Error}; #[derive(Debug, Display, Error)] struct Simple; #[derive(Debug, Display, Error)] #[display("Oops!")] struct Generic(#[error(source(optional))] RenamedOption<Simple>); // froces recognizing as `Option` type RenamedOption<T> = Option<T>; ``` However, it's better to be implemented as a separate PR, to not overcomplicate this one.
1 parent f324e0f commit f93776e

7 files changed

+553
-10
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
1212
- Support `#[display(rename_all = "<casing>")]` attribute to change output for
1313
implicit naming of unit enum variants or unit structs when deriving `Display`.
1414
([#443](https://github.com/JelteF/derive_more/pull/443))
15+
- Support `Option` fields for `Error::source()` in `Error` derive.
16+
([#459](https://github.com/JelteF/derive_more/pull/459))
1517

1618
### Fixed
1719

impl/doc/error.md

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,38 @@ Backtraces don't work though, because the `Backtrace` type is only available in
5252
`std`.
5353

5454

55+
### `Option`al fields
56+
57+
Deriving `source()` is supported naturally for `Option<_>`-typed fields.
58+
59+
> **WARNING**: Field type is **considered as `Option<_>` only syntactically**,
60+
> meaning that:
61+
> - Neither renaming `std::option::Option` would work:
62+
> ```rust,compile_fail
63+
> # use derive_more::{Display, Error};
64+
> #
65+
> #[derive(Debug, Display, Error)]
66+
> struct Simple;
67+
>
68+
> #[derive(Debug, Display, Error)]
69+
> #[display("Oops!")]
70+
> struct Generic(RenamedOption<Simple>); // not recognized as `Option`
71+
> type RenamedOption<T> = Option<T>;
72+
> ```
73+
> - Nor using any custom type named as `Option` would:
74+
> ```rust,compile_fail
75+
> # use derive_more::{Display, Error};
76+
> #
77+
> # #[derive(Debug, Display, Error)]
78+
> # struct Simple;
79+
> #
80+
> #[derive(Debug, Display, Error)]
81+
> #[display("No way!")]
82+
> struct Generic(Option<Simple>); // cannot use `?` syntax
83+
> struct Option<T>(T);
84+
> ```
85+
86+
5587
5688
5789
## Example usage
@@ -79,10 +111,21 @@ struct WithSource {
79111
source: Simple,
80112
}
81113
#[derive(Default, Debug, Display, Error)]
114+
#[display("WithOptionalSource")]
115+
struct WithOptionalSource {
116+
source: Option<Simple>,
117+
}
118+
#[derive(Default, Debug, Display, Error)]
82119
struct WithExplicitSource {
83120
#[error(source)]
84121
explicit_source: Simple,
85122
}
123+
#[derive(Default, Debug, Display, Error)]
124+
#[display("WithExplicitOptionalSource")]
125+
struct WithExplicitOptionalSource {
126+
#[error(source)]
127+
explicit_source: Option<Simple>,
128+
}
86129
87130
#[derive(Default, Debug, Display, Error)]
88131
struct Tuple(Simple);
@@ -104,6 +147,10 @@ enum CompoundError {
104147
WithSource {
105148
source: Simple,
106149
},
150+
#[display("WithOptionalSource")]
151+
WithOptionalSource {
152+
source: Option<Simple>,
153+
},
107154
#[from(ignore)]
108155
WithBacktraceFromSource {
109156
#[error(backtrace)]
@@ -118,21 +165,35 @@ enum CompoundError {
118165
#[error(source)]
119166
explicit_source: WithSource,
120167
},
168+
#[display("WithExplicitOptionalSource")]
169+
WithExplicitOptionalSource {
170+
#[error(source)]
171+
explicit_source: Option<WithSource>,
172+
},
121173
#[from(ignore)]
122174
WithBacktraceFromExplicitSource {
123175
#[error(backtrace, source)]
124176
explicit_source: WithSource,
125177
},
126178
Tuple(WithExplicitSource),
179+
#[display("TupleOptional")]
180+
TupleOptional(Option<WithExplicitSource>),
127181
WithoutSource(#[error(not(source))] Tuple),
128182
}
129183
130184
assert!(Simple.source().is_none());
131185
assert!(request_ref::<Backtrace>(&Simple).is_none());
186+
132187
assert!(WithSource::default().source().is_some());
188+
assert!(WithOptionalSource { source: Some(Simple) }.source().is_some());
189+
133190
assert!(WithExplicitSource::default().source().is_some());
191+
assert!(WithExplicitOptionalSource { explicit_source: Some(Simple) }.source().is_some());
192+
134193
assert!(Tuple::default().source().is_some());
194+
135195
assert!(WithoutSource::default().source().is_none());
196+
136197
let with_source_and_backtrace = WithSourceAndBacktrace {
137198
source: Simple,
138199
backtrace: Backtrace::capture(),
@@ -141,9 +202,16 @@ assert!(with_source_and_backtrace.source().is_some());
141202
assert!(request_ref::<Backtrace>(&with_source_and_backtrace).is_some());
142203
143204
assert!(CompoundError::Simple.source().is_none());
205+
144206
assert!(CompoundError::from(Simple).source().is_some());
207+
assert!(CompoundError::from(Some(Simple)).source().is_some());
208+
145209
assert!(CompoundError::from(WithSource::default()).source().is_some());
210+
assert!(CompoundError::from(Some(WithSource::default())).source().is_some());
211+
146212
assert!(CompoundError::from(WithExplicitSource::default()).source().is_some());
213+
assert!(CompoundError::from(Some(WithExplicitSource::default())).source().is_some());
214+
147215
assert!(CompoundError::from(Tuple::default()).source().is_none());
148216
# }
149217
```

impl/src/error.rs

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ pub fn expand(
4343
// TODO: Use `derive_more::core::error::Error` once `error_in_core` Rust feature is
4444
// stabilized.
4545
fn source(&self) -> Option<&(dyn derive_more::with_trait::Error + 'static)> {
46-
use derive_more::__private::AsDynError;
46+
use derive_more::__private::AsDynError as _;
4747
#source
4848
}
4949
}
@@ -207,13 +207,17 @@ impl ParsedFields<'_, '_> {
207207
fn render_source_as_struct(&self) -> Option<TokenStream> {
208208
let source = self.source?;
209209
let ident = &self.data.members[source];
210-
Some(render_some(quote! { #ident }))
210+
Some(render_some(
211+
quote! { (&#ident) },
212+
self.data.field_types[source].is_option(),
213+
))
211214
}
212215

213216
fn render_source_as_enum_variant_match_arm(&self) -> Option<TokenStream> {
214217
let source = self.source?;
215218
let pattern = self.data.matcher(&[source], &[quote! { source }]);
216-
let expr = render_some(quote! { source });
219+
let expr =
220+
render_some(quote! { source }, self.data.field_types[source].is_option());
217221
Some(quote! { #pattern => #expr })
218222
}
219223

@@ -287,10 +291,10 @@ impl ParsedFields<'_, '_> {
287291
}
288292
}
289293

290-
fn render_some<T>(expr: T) -> TokenStream
291-
where
292-
T: quote::ToTokens,
293-
{
294+
fn render_some(mut expr: TokenStream, unpack: bool) -> TokenStream {
295+
if unpack {
296+
expr = quote! { derive_more::core::option::Option::as_ref(#expr)? }
297+
}
294298
quote! { Some(#expr.as_dyn_error()) }
295299
}
296300

@@ -500,6 +504,42 @@ fn add_bound_if_type_parameter_used_in_type(
500504
ty: &syn::Type,
501505
) {
502506
if let Some(ty) = utils::get_if_type_parameter_used_in_type(type_params, ty) {
503-
bounds.insert(ty);
507+
bounds.insert(ty.get_option_inner().cloned().unwrap_or(ty));
508+
}
509+
}
510+
511+
/// Extension of a [`syn::Type`] used by this expansion.
512+
trait TypeExt {
513+
/// Checks syntactically whether this [`syn::Type`] represents an [`Option`].
514+
fn is_option(&self) -> bool;
515+
516+
/// Returns the inner [`syn::Type`] if this one represents an [`Option`].
517+
fn get_option_inner(&self) -> Option<&Self>;
518+
}
519+
520+
impl TypeExt for syn::Type {
521+
fn is_option(&self) -> bool {
522+
self.get_option_inner().is_some()
523+
}
524+
525+
fn get_option_inner(&self) -> Option<&Self> {
526+
match self {
527+
Self::Group(g) => g.elem.get_option_inner(),
528+
Self::Paren(p) => p.elem.get_option_inner(),
529+
Self::Path(p) => p
530+
.path
531+
.segments
532+
.last()
533+
.filter(|s| s.ident == "Option")
534+
.and_then(|s| {
535+
if let syn::PathArguments::AngleBracketed(a) = &s.arguments {
536+
if let Some(syn::GenericArgument::Type(ty)) = a.args.first() {
537+
return Some(ty);
538+
}
539+
}
540+
None
541+
}),
542+
_ => None,
543+
}
504544
}
505545
}

0 commit comments

Comments
 (0)