Repository: shepmaster/snafu (rust)

Issue:
Generic parameters with default values fail because they are placed before generated generic parameters in context selectors
I have a trouble using generic error type with default value, as in snafu-generated context selectors original generic parameters are placed before generated ones and if original generic parameter has default value it triggers compilation error, because defaults must be trailing. Error is very hard to understand as it points to original type where the generic parameter is the only, so is trailing, and you need to know how snafu works to understand what is true cause of that compilation error. I suppose the corresponding source is [https://github.com/shepmaster/snafu/blob/e09e9478bcc37968f711b0d5d22f04ea60963c77/snafu-derive/src/lib.rs#L774](https://github.com/shepmaster/snafu/blob/e09e9478bcc37968f711b0d5d22f04ea60963c77/snafu-derive/src/lib.rs#L774) 

Pull request:
Allow default generic parameters on error types
Fixes #116 but I'm not sure about some quote macro usages and test completeness

Patch:
diff --git a/snafu-derive/src/lib.rs b/snafu-derive/src/lib.rs
index fe0ee56..4830fbd 100644
--- a/snafu-derive/src/lib.rs
+++ b/snafu-derive/src/lib.rs
@@ -674,6 +674,50 @@ trait GenericAwareNames {
         Box::new(quote! { #enum_name<#(#original_generics,)*> })
     }
 
+    fn provided_generic_types_without_defaults(&self) -> Vec<proc_macro2::TokenStream> {
+        use syn::TypeParam;
+        self.generics()
+            .type_params()
+            .map(|t: &TypeParam| {
+                let &TypeParam {
+                    ref attrs,
+                    ref ident,
+                    ref colon_token,
+                    ref bounds,
+                    ..
+                } = t;
+                quote! {
+                    #(#attrs)*
+                    #ident
+                    #colon_token
+                    #bounds
+                }
+            })
+            .collect()
+    }
+
+    fn provided_generics_without_defaults(&self) -> Vec<proc_macro2::TokenStream> {
+        self.provided_generic_lifetimes()
+            .into_iter()
+            .chain(self.provided_generic_types_without_defaults().into_iter())
+            .collect()
+    }
+
+    fn provided_generic_lifetimes(&self) -> Vec<proc_macro2::TokenStream> {
+        use syn::{GenericParam, LifetimeDef};
+
+        self.generics()
+            .params
+            .iter()
+            .flat_map(|p| match *p {
+                GenericParam::Lifetime(LifetimeDef { ref lifetime, .. }) => {
+                    Some(quote! { #lifetime })
+                }
+                _ => None,
+            })
+            .collect()
+    }
+
     fn provided_generic_names(&self) -> Vec<proc_macro2::TokenStream> {
         use syn::{ConstParam, GenericParam, LifetimeDef, TypeParam};
 
@@ -749,7 +793,10 @@ impl<'a> quote::ToTokens for ContextSelector<'a> {
         use syn::Ident;
 
         let enum_name = &self.0.name;
-        let original_generics: &Vec<_> = &self.0.generics.params.iter().collect();
+        let original_lifetimes = self.0.provided_generic_lifetimes();
+        let original_generic_types_without_defaults =
+            self.0.provided_generic_types_without_defaults();
+        let original_generics_without_defaults = self.0.provided_generics_without_defaults();
 
         let parameterized_enum_name = &self.0.parameterized_name();
 
@@ -771,7 +818,7 @@ impl<'a> quote::ToTokens for ContextSelector<'a> {
             .as_ref()
             .unwrap_or(&self.0.default_visibility);
 
-        let generics_list = quote! { <#(#original_generics,)* #(#generic_names,)*> };
+        let generics_list = quote! { <#(#original_lifetimes,)* #(#generic_names,)* #(#original_generic_types_without_defaults,)*> };
         let selector_name = quote! { #variant_name<#(#generic_names,)*> };
 
         let names: &Vec<_> = &user_fields.iter().map(|f| f.name.clone()).collect();
@@ -815,10 +862,11 @@ impl<'a> quote::ToTokens for ContextSelector<'a> {
 
         let inherent_impl = if source_field.is_none() {
             let names2 = names;
+
             quote! {
                 impl<#(#generic_names,)*> #selector_name
                 {
-                    #visibility fn fail<#(#original_generics,)* __T>(self) -> std::result::Result<__T, #parameterized_enum_name>
+                    #visibility fn fail<#(#original_generics_without_defaults,)* __T>(self) -> std::result::Result<__T, #parameterized_enum_name>
                     where
                         #(#where_clauses),*
                     {
@@ -945,7 +993,7 @@ impl<'a> DisplayImpl<'a> {
 
 impl<'a> quote::ToTokens for DisplayImpl<'a> {
     fn to_tokens(&self, stream: &mut proc_macro2::TokenStream) {
-        let original_generics = &self.0.generics;
+        let original_generics = self.0.provided_generics_without_defaults();
         let parameterized_enum_name = &self.0.parameterized_name();
         let where_clauses = &self.0.provided_where_clauses();
 
@@ -953,7 +1001,7 @@ impl<'a> quote::ToTokens for DisplayImpl<'a> {
 
         stream.extend({
             quote! {
-                impl#original_generics std::fmt::Display for #parameterized_enum_name
+                impl<#(#original_generics),*> std::fmt::Display for #parameterized_enum_name
                 where
                     #(#where_clauses),*
                 {
@@ -1026,7 +1074,7 @@ impl<'a> ErrorImpl<'a> {
 
 impl<'a> quote::ToTokens for ErrorImpl<'a> {
     fn to_tokens(&self, stream: &mut proc_macro2::TokenStream) {
-        let original_generics = &self.0.generics;
+        let original_generics = self.0.provided_generics_without_defaults();
         let parameterized_enum_name = &self.0.parameterized_name();
         let where_clauses: &Vec<_> = &self.0.provided_where_clauses();
 
@@ -1066,7 +1114,7 @@ impl<'a> quote::ToTokens for ErrorImpl<'a> {
 
         stream.extend({
             quote! {
-                impl#original_generics std::error::Error for #parameterized_enum_name
+                impl<#(#original_generics),*> std::error::Error for #parameterized_enum_name
                 where
                     Self: std::fmt::Debug + std::fmt::Display,
                     #(#where_clauses),*
@@ -1122,7 +1170,7 @@ impl<'a> ErrorCompatImpl<'a> {
 
 impl<'a> quote::ToTokens for ErrorCompatImpl<'a> {
     fn to_tokens(&self, stream: &mut proc_macro2::TokenStream) {
-        let original_generics = &self.0.generics;
+        let original_generics = self.0.provided_generics_without_defaults();
         let parameterized_enum_name = &self.0.parameterized_name();
         let where_clauses = &self.0.provided_where_clauses();
 
@@ -1142,7 +1190,7 @@ impl<'a> quote::ToTokens for ErrorCompatImpl<'a> {
 
         stream.extend({
             quote! {
-                impl#original_generics snafu::ErrorCompat for #parameterized_enum_name
+                impl<#(#original_generics),*> snafu::ErrorCompat for #parameterized_enum_name
                 where
                     #(#where_clauses),*
                 {


Test patch:
diff --git a/tests/generics_with_default.rs b/tests/generics_with_default.rs
new file mode 100644
index 0000000..cd27fa0
--- /dev/null
+++ b/tests/generics_with_default.rs
@@ -0,0 +1,36 @@
+extern crate snafu;
+
+mod default_with_lifetime {
+    use snafu::{AsErrorSource, Snafu};
+    use std::fmt::{Debug, Display};
+
+    #[derive(Debug, Snafu)]
+    pub struct ApiError<'a, S, T>(Error<'a, S, T>)
+    where
+        T: Debug + Display,
+        S: std::error::Error + AsErrorSource;
+
+    #[derive(Debug, Snafu)]
+    enum Error<'a, S = std::io::Error, T = String>
+    where
+        T: Display,
+        S: std::error::Error + AsErrorSource,
+    {
+        #[snafu(display("Boom: {}", value))]
+        Boom {
+            value: T,
+            name: &'a str,
+        },
+        WithSource {
+            source: S,
+        },
+        Empty,
+    }
+
+    #[test]
+    fn implements_error() {
+        fn check_bounds<T: std::error::Error>() {}
+        check_bounds::<Error<std::io::Error>>();
+        check_bounds::<ApiError<std::io::Error, i32>>();
+    }
+}