Repository: shepmaster/snafu (rust)

Issue:
CompatChain returns `&'a dyn Error + 'a` which cannot be downcast
It should be `&'a dyn Error + 'static`. Doing this means that the public constructor needs to be modified, making it technically a breaking change.

Pull request:
ChainCompat now returns &'a (dyn Error + 'b) to allow downcasting
Previously it unconditionally returned &'a (dyn Error + 'a) which was never downcastable. The additional lifetime can be `'static`, resulting in `&'a (dyn Error + 'static)`, which may be downcast.

Closes #370

Patch:
diff --git a/snafu-derive/Cargo.toml b/snafu-derive/Cargo.toml
index e37e42a..9bdba5d 100644
--- a/snafu-derive/Cargo.toml
+++ b/snafu-derive/Cargo.toml
@@ -20,7 +20,7 @@ unstable-provider-api = []
 proc-macro = true
 
 [dependencies]
-syn = { version = "2.0", features = ["full"] }
-quote = "1.0.25"
-proc-macro2 = "1.0.52"
+syn = { version = "1.0.3", features = ["full"] }
+quote = "1.0"
+proc-macro2 = "1.0"
 heck = "0.4"
diff --git a/snafu-derive/src/lib.rs b/snafu-derive/src/lib.rs
index 004a710..65ff48f 100644
--- a/snafu-derive/src/lib.rs
+++ b/snafu-derive/src/lib.rs
@@ -1438,29 +1438,27 @@ trait GenericAwareNames {
     }
 
     fn provided_generic_lifetimes(&self) -> Vec<proc_macro2::TokenStream> {
-        use syn::{GenericParam, LifetimeParam};
+        use syn::{GenericParam, LifetimeDef};
 
         self.generics()
             .params
             .iter()
             .flat_map(|p| match p {
-                GenericParam::Lifetime(LifetimeParam { lifetime, .. }) => {
-                    Some(quote! { #lifetime })
-                }
+                GenericParam::Lifetime(LifetimeDef { lifetime, .. }) => Some(quote! { #lifetime }),
                 _ => None,
             })
             .collect()
     }
 
     fn provided_generic_names(&self) -> Vec<proc_macro2::TokenStream> {
-        use syn::{ConstParam, GenericParam, LifetimeParam, TypeParam};
+        use syn::{ConstParam, GenericParam, LifetimeDef, TypeParam};
 
         self.generics()
             .params
             .iter()
             .map(|p| match p {
                 GenericParam::Type(TypeParam { ident, .. }) => quote! { #ident },
-                GenericParam::Lifetime(LifetimeParam { lifetime, .. }) => quote! { #lifetime },
+                GenericParam::Lifetime(LifetimeDef { lifetime, .. }) => quote! { #lifetime },
                 GenericParam::Const(ConstParam { ident, .. }) => quote! { #ident },
             })
             .collect()
diff --git a/snafu-derive/src/parse.rs b/snafu-derive/src/parse.rs
index e2591f1..e67e9c5 100644
--- a/snafu-derive/src/parse.rs
+++ b/snafu-derive/src/parse.rs
@@ -40,7 +40,7 @@ pub(crate) fn attributes_from_syn(
     let mut errs = Vec::new();
 
     for attr in attrs {
-        if attr.path().is_ident("snafu") {
+        if attr.path.is_ident("snafu") {
             let attr_list = Punctuated::<Attribute, token::Comma>::parse_terminated;
 
             match attr.parse_args_with(attr_list) {
@@ -49,11 +49,11 @@ pub(crate) fn attributes_from_syn(
                 }
                 Err(e) => errs.push(e),
             }
-        } else if attr.path().is_ident("doc") {
+        } else if attr.path.is_ident("doc") {
             // Ignore any errors that occur while parsing the doc
             // comment. This isn't our attribute so we shouldn't
             // assume that we know what values are acceptable.
-            if let Ok(comment) = syn::parse2::<DocComment>(attr.meta.to_token_stream()) {
+            if let Ok(comment) = syn::parse2::<DocComment>(attr.tokens) {
                 ours.push(comment.into());
             }
         }
@@ -422,7 +422,6 @@ impl ToTokens for Display {
 }
 
 struct DocComment {
-    doc_ident: Ident,
     eq_token: token::Eq,
     str: LitStr,
 }
@@ -442,7 +441,6 @@ impl From<DocComment> for SnafuAttribute {
 impl Parse for DocComment {
     fn parse(input: ParseStream) -> Result<Self> {
         Ok(Self {
-            doc_ident: input.parse()?,
             eq_token: input.parse()?,
             str: input.parse()?,
         })
@@ -451,7 +449,6 @@ impl Parse for DocComment {
 
 impl ToTokens for DocComment {
     fn to_tokens(&self, tokens: &mut TokenStream) {
-        self.doc_ident.to_tokens(tokens);
         self.eq_token.to_tokens(tokens);
         self.str.to_tokens(tokens);
     }
diff --git a/src/error_chain.rs b/src/error_chain.rs
index f3a5dc7..1893caa 100644
--- a/src/error_chain.rs
+++ b/src/error_chain.rs
@@ -4,19 +4,19 @@
 ///
 /// Can be created via [`ErrorCompat::iter_chain`][crate::ErrorCompat::iter_chain].
 #[derive(Debug, Clone)]
-pub struct ChainCompat<'a> {
-    inner: Option<&'a dyn crate::Error>,
+pub struct ChainCompat<'a, 'b> {
+    inner: Option<&'a (dyn crate::Error + 'b)>,
 }
 
-impl<'a> ChainCompat<'a> {
+impl<'a, 'b> ChainCompat<'a, 'b> {
     /// Creates a new error chain iterator.
-    pub fn new(error: &'a dyn crate::Error) -> Self {
+    pub fn new(error: &'a (dyn crate::Error + 'b)) -> Self {
         ChainCompat { inner: Some(error) }
     }
 }
 
-impl<'a> Iterator for ChainCompat<'a> {
-    type Item = &'a dyn crate::Error;
+impl<'a, 'b> Iterator for ChainCompat<'a, 'b> {
+    type Item = &'a (dyn crate::Error + 'b);
 
     fn next(&mut self) -> Option<Self::Item> {
         match self.inner {


Test patch:
diff --git a/tests/error_chain.rs b/tests/error_chain.rs
index f05eb7b..c4e15af 100644
--- a/tests/error_chain.rs
+++ b/tests/error_chain.rs
@@ -1,7 +1,7 @@
 use snafu::prelude::*;
 use std::fmt::Debug;
 
-#[derive(Debug, Clone, Snafu)]
+#[derive(Debug, Clone, Snafu, PartialEq)]
 enum LeafError {
     #[snafu(display("User ID {} is invalid", user_id))]
     InvalidUser { user_id: i32 },
@@ -9,13 +9,13 @@ enum LeafError {
     MissingUser,
 }
 
-#[derive(Debug, Clone, Snafu)]
+#[derive(Debug, Clone, Snafu, PartialEq)]
 enum MiddleError {
     #[snafu(display("failed to check the user"))]
     CheckUser { source: LeafError },
 }
 
-#[derive(Debug, Clone, Snafu)]
+#[derive(Debug, Clone, Snafu, PartialEq)]
 enum Error {
     #[snafu(display("access control failure"))]
     AccessControl { source: MiddleError },
@@ -55,3 +55,18 @@ fn errorcompat_chain_iterates() {
     assert_eq_debug(&errors[1], &middle_error);
     assert_eq_debug(&errors[2], &bottom_error);
 }
+
+#[test]
+fn can_downcast_chain_values() {
+    use snafu::{ChainCompat, IntoError};
+
+    let bottom_error = InvalidUserSnafu { user_id: 12 }.build();
+    let middle_error = CheckUserSnafu.into_error(bottom_error.clone());
+    let error = AccessControlSnafu.into_error(middle_error.clone());
+
+    let errors: Vec<_> = ChainCompat::new(&error).collect();
+
+    assert_eq!(Some(&error), errors[0].downcast_ref::<Error>());
+    assert_eq!(Some(&middle_error), errors[1].downcast_ref::<MiddleError>());
+    assert_eq!(Some(&bottom_error), errors[2].downcast_ref::<LeafError>());
+}