diff --git a/compatibility-tests/compile-fail/tests/ui/attribute-unparseable.rs b/compatibility-tests/compile-fail/tests/ui/attribute-unparseable.rs index 57790c62..f0511290 100644 --- a/compatibility-tests/compile-fail/tests/ui/attribute-unparseable.rs +++ b/compatibility-tests/compile-fail/tests/ui/attribute-unparseable.rs @@ -1,9 +1,7 @@ use snafu::prelude::*; #[derive(Debug, Snafu)] -enum InnerError { - InnerVariant, -} +struct InnerError; #[derive(Debug, Snafu)] enum Error { @@ -14,4 +12,10 @@ enum Error { }, } +#[derive(Debug, Snafu)] +struct SourceFromTransformInvalidType { + #[snafu(source(from(Cow*, ?)))] + source: InnerError, +} + fn main() {} diff --git a/compatibility-tests/compile-fail/tests/ui/attribute-unparseable.stderr b/compatibility-tests/compile-fail/tests/ui/attribute-unparseable.stderr index 4fcd44ff..54fa910b 100644 --- a/compatibility-tests/compile-fail/tests/ui/attribute-unparseable.stderr +++ b/compatibility-tests/compile-fail/tests/ui/attribute-unparseable.stderr @@ -1,5 +1,17 @@ error: expected boolean literal or `from` - --> tests/ui/attribute-unparseable.rs:12:24 + --> tests/ui/attribute-unparseable.rs:10:24 | -12 | #[snafu(source(5))] +10 | #[snafu(source(5))] | ^ + +error: expected one of: `exact`, `generic` or a type followed by a comma and an expression + --> tests/ui/attribute-unparseable.rs:17:25 + | +17 | #[snafu(source(from(Cow*, ?)))] + | ^^^ + +error: expected `,` + --> tests/ui/attribute-unparseable.rs:17:28 + | +17 | #[snafu(source(from(Cow*, ?)))] + | ^ diff --git a/compatibility-tests/compile-fail/tests/ui/from-generic.rs b/compatibility-tests/compile-fail/tests/ui/from-generic.rs new file mode 100644 index 00000000..25d7d5f5 --- /dev/null +++ b/compatibility-tests/compile-fail/tests/ui/from-generic.rs @@ -0,0 +1,20 @@ +use snafu::prelude::*; + +#[derive(Debug, Snafu)] +struct DummyError; + +#[derive(Debug, Snafu)] +enum EnumErrorWithGenericSourceAndContextSelector { + TheVariant { + #[snafu(source(from(generic)))] + source: DummyError, + }, +} + +#[derive(Debug, Snafu)] +struct StructErrorWithGenericSourceAndContextSelector { + #[snafu(source(from(generic)))] + source: DummyError, +} + +fn main() {} diff --git a/compatibility-tests/compile-fail/tests/ui/from-generic.stderr b/compatibility-tests/compile-fail/tests/ui/from-generic.stderr new file mode 100644 index 00000000..0dbee877 --- /dev/null +++ b/compatibility-tests/compile-fail/tests/ui/from-generic.stderr @@ -0,0 +1,11 @@ +error: Cannot use `source(from(generic))` without disabling the context selector with `context(false)` or `transparent` + --> tests/ui/from-generic.rs:9:17 + | +9 | #[snafu(source(from(generic)))] + | ^^^^^^^^^^^^^^^^^^^^^ + +error: Cannot use `source(from(generic))` without disabling the context selector with `context(false)` or `transparent` + --> tests/ui/from-generic.rs:16:13 + | +16 | #[snafu(source(from(generic)))] + | ^^^^^^^^^^^^^^^^^^^^^ diff --git a/snafu-derive/src/lib.rs b/snafu-derive/src/lib.rs index 79cce53d..85400ec1 100644 --- a/snafu-derive/src/lib.rs +++ b/snafu-derive/src/lib.rs @@ -238,8 +238,10 @@ impl SourceField { enum Transformation { None { - ty: syn::Type, + target_ty: syn::Type, + from_is_generic: bool, }, + Transform { source_ty: syn::Type, target_ty: syn::Type, @@ -250,24 +252,37 @@ enum Transformation { impl Transformation { fn source_ty(&self) -> &syn::Type { match self { - Transformation::None { ty } => ty, + Transformation::None { target_ty, .. } => target_ty, Transformation::Transform { source_ty, .. } => source_ty, } } fn target_ty(&self) -> &syn::Type { match self { - Transformation::None { ty } => ty, + Transformation::None { target_ty, .. } => target_ty, Transformation::Transform { target_ty, .. } => target_ty, } } fn transformation(&self) -> proc_macro2::TokenStream { match self { - Transformation::None { .. } => quote! { |v| v }, + Transformation::None { + from_is_generic: false, + .. + } => quote! { |v| v }, + + Transformation::None { + from_is_generic: true, + .. + } => quote! { ::core::convert::Into::into }, + Transformation::Transform { expr, .. } => quote! { #expr }, } } + + fn is_generic(&self) -> bool { + matches!(self, Transformation::None { from_is_generic, .. } if *from_is_generic) + } } fn impl_snafu_macro(ty: syn::DeriveInput) -> TokenStream { @@ -831,8 +846,8 @@ impl TupleStructInfo { crate_root, generics, name, - transformation, provides, + transformation, } = self; let where_clauses: Vec<_> = generics diff --git a/snafu-derive/src/parse.rs b/snafu-derive/src/parse.rs index e53b6db5..514374a3 100644 --- a/snafu-derive/src/parse.rs +++ b/snafu-derive/src/parse.rs @@ -54,6 +54,8 @@ mod kw { custom_keyword!(whatever); custom_keyword!(from); + custom_keyword!(exact); + custom_keyword!(generic); custom_keyword!(name); custom_keyword!(suffix); @@ -1063,15 +1065,30 @@ struct SourceFrom { fn into_transformation(source_from: Option, target_ty: Type) -> Transformation { match source_from { - Some(SourceFrom { value, .. }) => { - let SourceFromArg { r#type, expr, .. } = value; - Transformation::Transform { - source_ty: r#type, + Some(SourceFrom { value, .. }) => match value.value { + SourceFromValue::Exact(_) => Transformation::None { target_ty, - expr, + from_is_generic: false, + }, + + SourceFromValue::Generic(_) => Transformation::None { + target_ty, + from_is_generic: true, + }, + + SourceFromValue::Transform(SourceFromTransform { r#type, expr, .. }) => { + Transformation::Transform { + source_ty: r#type, + target_ty, + expr, + } } - } - None => Transformation::None { ty: target_ty }, + }, + + None => Transformation::None { + target_ty, + from_is_generic: false, + }, } } @@ -1134,24 +1151,41 @@ impl Parse for NestedSource { } } +enum SourceArg { + Flag { value: LitBool }, + From(SourceFromArg), +} + +impl Parse for SourceArg { + fn parse(input: ParseStream) -> Result { + let lookahead = input.lookahead1(); + if lookahead.peek(LitBool) { + Ok(SourceArg::Flag { + value: input.parse()?, + }) + } else if lookahead.peek(kw::from) { + input.parse().map(SourceArg::From) + } else { + Err(lookahead.error()) + } + } +} + #[derive(Clone)] struct SourceFromArg { from_token: kw::from, paren_token: token::Paren, - r#type: Type, - comma_token: token::Comma, - expr: Expr, + value: SourceFromValue, } impl Parse for SourceFromArg { fn parse(input: ParseStream) -> Result { let content; + Ok(SourceFromArg { from_token: input.parse()?, paren_token: parenthesized!(content in input), - r#type: content.parse()?, - comma_token: content.parse()?, - expr: content.parse()?, + value: content.parse()?, }) } } @@ -1160,33 +1194,79 @@ impl ToTokens for SourceFromArg { fn to_tokens(&self, tokens: &mut TokenStream) { self.from_token.to_tokens(tokens); self.paren_token.surround(tokens, |tokens| { - self.r#type.to_tokens(tokens); - self.comma_token.to_tokens(tokens); - self.expr.to_tokens(tokens); + self.value.to_tokens(tokens); }); } } -enum SourceArg { - Flag { value: LitBool }, - From(SourceFromArg), +#[derive(Clone)] +enum SourceFromValue { + Exact(kw::exact), + + Generic(kw::generic), + + Transform(SourceFromTransform), } -impl Parse for SourceArg { +impl Parse for SourceFromValue { fn parse(input: ParseStream) -> Result { - let lookahead = input.lookahead1(); - if lookahead.peek(LitBool) { - Ok(SourceArg::Flag { - value: input.parse()?, - }) - } else if lookahead.peek(kw::from) { - input.parse().map(SourceArg::From) + if input.peek(kw::exact) { + input.parse().map(Self::Exact) + } else if input.peek(kw::generic) { + input.parse().map(Self::Generic) } else { - Err(lookahead.error()) + // We can't peek ahead for a type. If we fail, add our own + // error that mimics the lookahead error to tell the user + // that `exact` / `generic` are also possible here. + // + // FUTURE: Consider making transforms be keyword-prefixed (with a semver + // break?) e.g. `transform Type with Expr` + let span = input.span(); + let txt = "expected one of: `exact`, `generic` or a type followed by a comma and an expression"; + input.parse().map(Self::Transform).map_err(|e| { + let mut e1 = syn::Error::new(span, txt); + e1.combine(e); + e1 + }) + } + } +} + +impl ToTokens for SourceFromValue { + fn to_tokens(&self, tokens: &mut TokenStream) { + match self { + SourceFromValue::Exact(exact) => exact.to_tokens(tokens), + SourceFromValue::Generic(generic) => generic.to_tokens(tokens), + SourceFromValue::Transform(transform) => transform.to_tokens(tokens), } } } +#[derive(Clone)] +struct SourceFromTransform { + r#type: Type, + comma_token: token::Comma, + expr: Expr, +} + +impl Parse for SourceFromTransform { + fn parse(input: ParseStream) -> Result { + Ok(Self { + r#type: input.parse()?, + comma_token: input.parse()?, + expr: input.parse()?, + }) + } +} + +impl ToTokens for SourceFromTransform { + fn to_tokens(&self, tokens: &mut TokenStream) { + self.r#type.to_tokens(tokens); + self.comma_token.to_tokens(tokens); + self.expr.to_tokens(tokens); + } +} + struct Transparent { transparent_token: kw::transparent, arg: MaybeArg, diff --git a/snafu-derive/src/parse/field_container_impl.rs b/snafu-derive/src/parse/field_container_impl.rs index fd2ee3b8..28fa9799 100644 --- a/snafu-derive/src/parse/field_container_impl.rs +++ b/snafu-derive/src/parse/field_container_impl.rs @@ -221,6 +221,13 @@ pub(super) fn parse_field_container( _ => {} // no conflict } + if let Some(Sidecar(span, source_field)) = &source { + if source_field.transformation.is_generic() && !selector_kind.is_without_context() { + let txt = "Cannot use `source(from(generic))` without disabling the context selector with `context(false)` or `transparent`"; + errors.push_new(span, txt); + } + } + let source_field = source.map(|Sidecar(_, val)| val); let backtrace_field = backtrace.map(|Sidecar(_, val)| val); let is_transparent = selector_kind.is_transparent(); @@ -325,6 +332,10 @@ impl IntermediateSelectorKind { } ) } + + fn is_without_context(&self) -> bool { + matches!(self, IntermediateSelectorKind::WithoutContext { .. }) + } } enum WithoutContextSource { diff --git a/snafu-derive/src/shared.rs b/snafu-derive/src/shared.rs index 826cf937..d0db2f69 100644 --- a/snafu-derive/src/shared.rs +++ b/snafu-derive/src/shared.rs @@ -134,10 +134,13 @@ impl quote::ToTokens for GenericsWithoutDefaults<'_> { } } +const SOURCE_GENERIC_NAME: StaticIdent = StaticIdent("__SnafuSource"); + pub(crate) struct SourceInfo<'a> { pub source_field_type: &'a syn::Type, pub transform_source: proc_macro2::TokenStream, pub transfer_source_field: proc_macro2::TokenStream, + maybe_generic: Option, } impl<'a> SourceInfo<'a> { @@ -160,11 +163,17 @@ impl<'a> SourceInfo<'a> { let transform_source = quote! { let error: #target_field_type = (#source_transformation)(error) }; let transfer_source_field = quote! { #source_field_name: error, }; + let maybe_generic = if transformation.is_generic() { + Some(SOURCE_GENERIC_NAME) + } else { + None + }; Self { source_field_type, transform_source, transfer_source_field, + maybe_generic, } } } @@ -447,7 +456,11 @@ pub mod context_selector { source_field_type, transform_source, transfer_source_field, + maybe_generic, } = SourceInfo::from_source_field(source_field); + + assert!(maybe_generic.is_none(), "Internal error"); + ( quote! { #source_field_type }, Some(transform_source), @@ -594,15 +607,26 @@ pub mod no_context_selector { source_field_type, transform_source, transfer_source_field, + maybe_generic, } = source_info; + let maybe_generic = maybe_generic.as_ref().map(|m| m as &dyn ToTokens); + + let generics = match &maybe_generic { + Some(g) => generics.push(std::slice::from_ref(g)), + None => *generics, + }; + + let from_type = maybe_generic.unwrap_or(source_field_type); + let no_context_selector = quote! { - impl<#generics> ::core::convert::From<#source_field_type> for #parameterized_error_name + impl<#generics> ::core::convert::From<#from_type> for #parameterized_error_name where + #from_type: ::core::convert::Into<#source_field_type>, #(#where_clauses),* { #[track_caller] - fn from(error: #source_field_type) -> Self { + fn from(error: #from_type) -> Self { #transform_source; #error_constructor_name { #construct_implicit_fields_with_source diff --git a/src/Snafu.md b/src/Snafu.md index 8c49c321..78abd607 100644 --- a/src/Snafu.md +++ b/src/Snafu.md @@ -47,15 +47,17 @@ it is valid. Detailed information on each attribute is below. ### Context fields -| Option (inside `#[snafu(...)]`) | Description | -|---------------------------------|--------------------------------------------------------------------------------| -| `source` | Marks a field as the source error (even if not called `source`) | -| `source(from(type, transform))` | As above, plus converting from `type` to the field type by calling `transform` | -| `source(false)` | Marks a field that is named `source` as a regular field | -| `backtrace` | Marks a field as backtrace (even if not called `backtrace`) | -| `backtrace(false)` | Marks a field that is named `backtrace` as a regular field | -| `implicit` | Marks a field as implicit (Type needs to implement [`GenerateImplicitData`][]) | -| `provide` | Marks a field as providing a reference to the type | +| Option (inside `#[snafu(...)]`) | Description | +|---------------------------------|---------------------------------------------------------------------------------------------------------| +| `source` | Marks a field as the source error (even if not called `source`) | +| `source(from(type, transform))` | Marks a field as the source error and converts from `type` to the field type by calling `transform` | +| `source(from(generic))` | Marks a field as the source error and converts from any type to the field type by calling ``Into::into` | +| `source(from(exact))` | Marks a field as the source error and performs no conversion to the field type | +| `source(false)` | Marks a field that is named `source` as a regular field | +| `backtrace` | Marks a field as backtrace (even if not called `backtrace`) | +| `backtrace(false)` | Marks a field that is named `backtrace` as a regular field | +| `implicit` | Marks a field as implicit (Type needs to implement [`GenerateImplicitData`][]) | +| `provide` | Marks a field as providing a reference to the type | ## Controlling `Display` @@ -461,10 +463,62 @@ enum Error { struct ApiError(Box); ``` -Note: If you specify `#[snafu(source(from(...)))]` then the field +Note: If you specify `#[snafu(source(from(...)))]` then the field will +be treated as a source, even if it's not named "source" - in other +words, this option implies `#[snafu(source)]`. + +#### From an arbitrary type + +If you have an error that does not create a context selector, such as +an opaque error or `context(false)` error, you may use +`#[snafu(source(from(generic)))]` to create the wrapping error from +any type that can be converted [`into`](Into::into) the wrapped error. + +```rust +# use snafu::prelude::*; +#[derive(Debug, Snafu)] +struct InternalError { + code: i32, +} + +impl From for InternalError { + fn from(code: i32) -> Self { + Self { code } + } +} + +#[derive(Debug, Snafu)] +#[snafu(context(false))] +struct Error { + #[snafu(source(from(generic)))] + source: InternalError, +} + +pub fn usage() -> Result<(), Error> { + let code: Result<(), i32> = Err(42); + + // Converts the error type from `i32` to `InternalError` to `Error`. + code?; + + Ok(()) +} +``` + +Using this option with errors that create context selectors will +result in an error. + +Note: If you specify `#[snafu(source(from(generic)))]` then the field +will be treated as a source, even if it's not named "source" - in +other words, this option implies `#[snafu(source)]`. + +#### Disabling source transformation + +`#[snafu(source(from(exact)))]` can be used to explicitly disable +source transformation. + +Note: If you specify `#[snafu(source(from(exact)))]` then the field will be treated as a source, even if it's not named "source" - in -other words, `#[snafu(source(from(...)))]` implies -`#[snafu(source)]`. +other words, this option implies `#[snafu(source)]`. ## Controlling backtraces diff --git a/tests/no_context.rs b/tests/no_context.rs index 346c3d44..e0faf15b 100644 --- a/tests/no_context.rs +++ b/tests/no_context.rs @@ -1,14 +1,10 @@ use snafu::prelude::*; #[derive(Debug, Snafu)] -enum AlphaError { - _AlphaDummy, -} +struct AlphaError; #[derive(Debug, Snafu)] -enum BetaError { - _BetaDummy, -} +struct BetaError; #[derive(Debug, Snafu)] enum Error { @@ -84,3 +80,79 @@ mod with_bounds { check::>(); } } + +mod with_exact_source { + use super::*; + + #[derive(Debug, Snafu)] + #[snafu(context(false))] + struct Error { + #[snafu(source(from(exact)))] + source: AlphaError, + } + + #[test] + fn implements_error() { + check::(); + } + + trait LocalTrait {} + impl LocalTrait for i32 {} + + impl From for Error + where + T: LocalTrait, + { + fn from(_: T) -> Self { + Error { source: AlphaError } + } + } + + #[test] + fn custom_from_implementation() { + let _error: Error = 42.into(); + } +} + +mod with_generic_source { + use super::*; + + struct NotAlpha; + + impl From for AlphaError { + fn from(_: NotAlpha) -> Self { + AlphaError + } + } + + fn convertable_to_alpha() -> Result<(), NotAlpha> { + Ok(()) + } + + #[derive(Debug, Snafu)] + enum Error { + #[snafu(context(false))] + Alpha { + #[snafu(source(from(generic)))] + source: AlphaError, + }, + + #[snafu(context(false))] + Beta { source: BetaError }, + } + + #[test] + fn implements_error() { + check::(); + } + + #[test] + fn converted_to_alpha() { + fn converts_to_alpha() -> Result<(), Error> { + convertable_to_alpha()?; + Ok(()) + } + + converts_to_alpha().unwrap() + } +} diff --git a/tests/opaque.rs b/tests/opaque.rs index f0f1ad82..aca93726 100644 --- a/tests/opaque.rs +++ b/tests/opaque.rs @@ -1,3 +1,5 @@ +fn check() {} + mod inner { use snafu::prelude::*; @@ -35,7 +37,6 @@ mod inner { #[test] fn implements_error() { - fn check() {} check::(); let e = inner::api().unwrap_err(); assert!(e.to_string().contains("too big")); @@ -56,3 +57,69 @@ fn ensure_boxed() { let e = inner::boxed_inner(2).unwrap_err(); assert!(e.to_string().contains("too big")); } + +mod with_exact_source { + use super::*; + use snafu::prelude::*; + + #[derive(Debug, Snafu)] + #[snafu(display("The inner error"))] + struct InnerError; + + #[derive(Debug, Snafu)] + struct MiddleError(InnerError); + + #[derive(Debug, Snafu)] + #[snafu(source(from(exact)))] + struct OuterError(MiddleError); + + trait LocalTrait {} + impl LocalTrait for i32 {} + + impl From for OuterError + where + T: LocalTrait, + { + fn from(_: T) -> Self { + OuterError(MiddleError(InnerError)) + } + } + + #[test] + fn usage() { + check::(); + let e: OuterError = 42.into(); + assert_eq!(e.to_string(), "The inner error"); + } +} + +mod with_generic_source { + use super::*; + use snafu::prelude::*; + + #[derive(Debug, Snafu)] + #[snafu(display("The inner error"))] + struct InnerError; + + #[derive(Debug, Snafu)] + struct MiddleError(InnerError); + + #[derive(Debug, Snafu)] + #[snafu(source(from(generic)))] + struct OuterError(MiddleError); + + fn make_inner() -> Result<(), InnerError> { + InnerSnafu.fail() + } + + fn make_outer() -> Result<(), OuterError> { + Ok(make_inner()?) + } + + #[test] + fn usage() { + check::(); + let e: OuterError = make_outer().unwrap_err(); + assert_eq!(e.to_string(), "The inner error"); + } +} diff --git a/tests/source_attributes.rs b/tests/source_attributes.rs index a6dd30f4..047aaa56 100644 --- a/tests/source_attributes.rs +++ b/tests/source_attributes.rs @@ -1,14 +1,14 @@ use snafu::prelude::*; #[derive(Debug, Snafu)] -enum InnerError { - _Boom, -} +struct InnerError; fn inner() -> Result<(), InnerError> { Ok(()) } +fn check() {} + mod enabling { use super::*; @@ -45,12 +45,52 @@ mod enabling { #[test] fn implements_error() { - fn check() {} check::(); example().unwrap_err(); } } +// Corresponding `from(generic)` tests are in the compile-fail suite +mod exact { + use super::*; + + #[derive(Debug, Snafu)] + enum EnumError { + ExactMatch { + #[snafu(source(from(exact)))] + source: InnerError, + }, + } + + #[test] + fn enum_implements_error() { + fn example() -> Result<(), EnumError> { + inner().context(ExactMatchSnafu)?; + Ok(()) + } + + check::(); + example().unwrap(); + } + + #[derive(Debug, Snafu)] + struct StructError { + #[snafu(source(from(exact)))] + source: InnerError, + } + + #[test] + fn struct_implements_error() { + fn example() -> Result<(), StructError> { + inner().context(StructSnafu)?; + Ok(()) + } + + check::(); + example().unwrap(); + } +} + mod transformation { use super::*; use std::io; @@ -86,7 +126,6 @@ mod transformation { #[test] fn implements_error() { - fn check() {} check::(); example().unwrap(); } @@ -102,7 +141,6 @@ mod transformation { #[test] fn api_implements_error() { - fn check() {} check::(); api_example().unwrap(); } diff --git a/tests/transparent.rs b/tests/transparent.rs index 9d6604a4..154acdf5 100644 --- a/tests/transparent.rs +++ b/tests/transparent.rs @@ -91,3 +91,74 @@ mod with_bounds { check::>(); } } + +mod with_exact_source { + use super::*; + + #[derive(Debug, Snafu)] + #[snafu(transparent)] + struct Error { + #[snafu(source(from(exact)))] + source: AlphaError, + } + + trait LocalTrait {} + impl LocalTrait for i32 {} + + impl From for Error + where + T: LocalTrait, + { + fn from(_: T) -> Self { + Error { source: AlphaError } + } + } + + #[test] + fn implements_error() { + check::(); + } + + #[test] + fn custom_from_implementation() { + let _error: Error = 42.into(); + } +} + +mod with_generic_source { + use super::*; + + struct NotAlpha; + + impl From for AlphaError { + fn from(_: NotAlpha) -> Self { + AlphaError + } + } + + fn convertable_to_alpha() -> Result<(), NotAlpha> { + Ok(()) + } + + #[derive(Debug, Snafu)] + #[snafu(transparent)] + struct Error { + #[snafu(source(from(generic)))] + source: AlphaError, + } + + #[test] + fn implements_error() { + check::(); + } + + #[test] + fn converted_to_alpha() { + fn converts_to_alpha() -> Result<(), Error> { + convertable_to_alpha()?; + Ok(()) + } + + converts_to_alpha().unwrap() + } +}