Repository: BrynCooke/buildstructor (language: rust)

Issue:
Bug when using generics in a constructor function
Example to reproduce:

```rust
#[derive(Debug)]
pub struct Request<T> {
    inner: http::Request<T>,
}

#[buildstructor::builder]
impl<T> Request<T> {
    pub fn fake_new<K, V>(
        headers: Vec<(K, V)>,
        uri: Option<http::Uri>,
        method: Option<http::Method>,
        body: T,
    ) -> http::Result<Request<T>>
    where
        HeaderName: TryFrom<K>,
        <HeaderName as TryFrom<K>>::Error: Into<http::Error>,
        HeaderValue: TryFrom<V>,
        <HeaderValue as TryFrom<V>>::Error: Into<http::Error>,
    {
        let mut builder = http::request::Builder::new().method(method).uri(uri);
        for (key, value) in headers {
            builder = builder.header(key, value);
        }
        let req = builder.body(body)?;

        Ok(Self { inner: req })
    }
}

// Use it

Request::builder()
      .header(("da".to_string(), "vda".to_string()))
      .header(("db".to_string(), "vdb".to_string()))
      .header(("db".to_string(), "vdb".to_string()))
      .header((HOST.to_string(), "host".to_string()))
      .header((CONTENT_LENGTH.to_string(), "2".to_string()))
      .header((CONTENT_TYPE.to_string(), "graphql".to_string()))
      .body(Request::builder().query("query").build())
      .build() // ERROR: Cannot find build
      .unwrap(),
```


Discussion / hints:

Fix patch:
diff --git a/.tool-versions b/.tool-versions
index d64260b..06eb83d 100644
--- a/.tool-versions
+++ b/.tool-versions
@@ -1,1 +1,1 @@
-rust 1.59.0
+rust 1.60.0
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 439f534..a956ed3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,9 +4,13 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## 0.1.5 - unreleased
+## 0.1.6 - unreleased
+[#14](https://github.com/BrynCooke/buildstructor/issues/14) Generics ordering bug.
+Generics were not being consistently ordered, which caused issues if there were generics on the impl type and also in a where clause.
+
+## 0.1.5 - 2022-04-11
 ### Added
-[#9](https://github.com/BrynCooke/buildstructor/issues/9) Add `*_new` support
+[#9](https://github.com/BrynCooke/buildstructor/issues/9) Add `*_new` support.
 Any method named `new` or has a suffix `_new` will create a builder.
 Builders methods are named appropriately. e.g. `try_new` -> `try_build`.
 ### Fixed
diff --git a/Cargo.toml b/Cargo.toml
index 76e82ef..e7b0760 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -27,3 +27,4 @@ trybuild = "~1.0"
 tokio = { version = "1", features = ["rt-multi-thread", "macros"] }
 insta = "1.12.0"
 prettyplease = "0.1.7"
+http = "0.2.6"
diff --git a/src/buildstructor/codegen.rs b/src/buildstructor/codegen.rs
index 1fd8f58..d9b5218 100644
--- a/src/buildstructor/codegen.rs
+++ b/src/buildstructor/codegen.rs
@@ -5,7 +5,7 @@ use crate::lower::{FieldType, Ir};
 use proc_macro2::{Ident, TokenStream};
 use quote::{format_ident, quote};
 use syn::punctuated::Punctuated;
-use syn::{Expr, ExprCall, GenericArgument, Generics, Index, Result, Type};
+use syn::{Expr, ExprCall, GenericArgument, Generics, Index, Result, Type, WhereClause};
 extern crate inflector;
 use inflector::Inflector;
 
@@ -34,7 +34,7 @@ pub fn codegen(ir: Ir) -> Result<TokenStream> {
     let (_, all_ty_generics, _) = all_generics.split_for_impl();
 
     let method_generics = &ir.method_generics;
-    let builder_init_generics = Generics::combine(vec![&ir.method_generics, &ir.generics]);
+    let builder_init_generics = Generics::combine(vec![&ir.generics, &ir.method_generics]);
     let target_generics_raw: Vec<GenericArgument> = builder_init_generics
         .to_generic_args()
         .args
@@ -46,7 +46,7 @@ pub fn codegen(ir: Ir) -> Result<TokenStream> {
     let constructor_return = &ir.return_type;
     let builder_method_name = &ir.builder_method_name;
     let builder_name = &ir.builder_name;
-    let builder_methods = builder_methods(&ir)?;
+    let builder_methods = builder_methods(&ir, builder_where_clause)?;
     let builder_state_type_initial = ir.builder_state_type_initial();
     let builder_state_initial = ir.builder_state_initial();
 
@@ -123,8 +123,11 @@ pub fn codegen(ir: Ir) -> Result<TokenStream> {
     })
 }
 
-pub fn builder_methods(ir: &Ir) -> Result<Vec<TokenStream>> {
-    let builder_generics = Generics::combine(vec![&ir.method_generics, &ir.generics]);
+pub fn builder_methods(
+    ir: &Ir,
+    builder_where_clause: Option<&WhereClause>,
+) -> Result<Vec<TokenStream>> {
+    let builder_generics = Generics::combine(vec![&ir.generics, &ir.method_generics]);
     Ok(ir.builder_fields
         .iter()
         .enumerate()
@@ -172,13 +175,13 @@ pub fn builder_methods(ir: &Ir) -> Result<Vec<TokenStream>> {
                     let and_method_name = format_ident!("and_{}", f.name);
                     quote! {
                         impl #builder_type_generics #builder_name #before {
-                            pub fn #method_name (self, #field_name: #generic_param) -> #builder_name #after {
+                            pub fn #method_name (self, #field_name: #generic_param) -> #builder_name #after #builder_where_clause {
                                 #builder_name {
                                     fields: #new_state_option,
                                     phantom: core::default::Default::default()
                                 }
                             }
-                            pub fn #and_method_name (self, #field_name: #ty) -> #builder_name #after {
+                            pub fn #and_method_name (self, #field_name: #ty) -> #builder_name #after #builder_where_clause {
                                 #builder_name {
                                     fields: #new_state,
                                     phantom: core::default::Default::default()
@@ -194,12 +197,12 @@ pub fn builder_methods(ir: &Ir) -> Result<Vec<TokenStream>> {
                     quote! {
                         impl #builder_type_generics #builder_name #before {
 
-                            pub fn #plural (mut self, #field_name: #ty) -> #builder_name #before {
+                            pub fn #plural (mut self, #field_name: #ty) -> #builder_name #before #builder_where_clause {
                                 self.fields.#index.lazy.get_or_insert_with(||core::default::Default::default()).extend(#field_name.into_iter());
                                 self
                             }
 
-                            pub fn #singular (mut self, value: #field_collection_type) -> #builder_name #before {
+                            pub fn #singular (mut self, value: #field_collection_type) -> #builder_name #before #builder_where_clause{
                                 self.fields.#index.lazy.get_or_insert_with(||core::default::Default::default()).insert(value);
                                 self
                             }
@@ -214,12 +217,12 @@ pub fn builder_methods(ir: &Ir) -> Result<Vec<TokenStream>> {
                     quote! {
                         impl #builder_type_generics #builder_name #before {
 
-                            pub fn #plural (mut self, #field_name: #ty) -> #builder_name #before {
+                            pub fn #plural (mut self, #field_name: #ty) -> #builder_name #before #builder_where_clause{
                                 self.fields.#index.lazy.get_or_insert_with(||core::default::Default::default()).extend(#field_name.into_iter());
                                 self
                             }
 
-                            pub fn #singular (mut self, value: #field_collection_type) -> #builder_name #before {
+                            pub fn #singular (mut self, value: #field_collection_type) -> #builder_name #before #builder_where_clause{
                                 self.fields.#index.lazy.get_or_insert_with(||core::default::Default::default()).push(value);
                                 self
                             }
@@ -235,7 +238,7 @@ pub fn builder_methods(ir: &Ir) -> Result<Vec<TokenStream>> {
                     quote! {
                         impl #builder_type_generics #builder_name #before {
 
-                            pub fn #plural (mut self, #field_name: #ty) -> #builder_name #before {
+                            pub fn #plural (mut self, #field_name: #ty) -> #builder_name #before #builder_where_clause{
                                 self.fields.#index.lazy.get_or_insert_with(||core::default::Default::default()).extend(#field_name.into_iter());
                                 self
                             }
@@ -391,4 +394,9 @@ mod tests {
     fn returns_self_test() {
         assert_codegen!(returns_self_test_case());
     }
+
+    #[test]
+    fn multiple_generics_test() {
+        assert_codegen!(multiple_generics_test_case());
+    }
 }
diff --git a/src/buildstructor/mod.rs b/src/buildstructor/mod.rs
index 4182fe1..8d482e7 100644
--- a/src/buildstructor/mod.rs
+++ b/src/buildstructor/mod.rs
@@ -182,4 +182,32 @@ mod tests {
             }
         )
     }
+
+    pub fn multiple_generics_test_case() -> Ast {
+        parse_quote!(
+            #[builder]
+            impl<T> Request<T> {
+                pub fn fake_new<K, V>(
+                    headers: Vec<(K, V)>,
+                    uri: Option<http::Uri>,
+                    method: Option<http::Method>,
+                    body: T,
+                ) -> http::Result<Request<T>>
+                where
+                    HeaderName: TryFrom<K>,
+                    <HeaderName as TryFrom<K>>::Error: Into<http::Error>,
+                    HeaderValue: TryFrom<V>,
+                    <HeaderValue as TryFrom<V>>::Error: Into<http::Error>,
+                {
+                    let mut builder = http::request::Builder::new();
+                    for (key, value) in headers {
+                        builder = builder.header(key, value);
+                    }
+                    let req = builder.body(body)?;
+
+                    Ok(Self { inner: req })
+                }
+            }
+        )
+    }
 }


Test patch:
diff --git a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__multiple_generics_test.snap b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__multiple_generics_test.snap
new file mode 100644
index 0000000..917a130
--- /dev/null
+++ b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__multiple_generics_test.snap
@@ -0,0 +1,243 @@
+---
+source: src/buildstructor/codegen.rs
+assertion_line: 400
+expression: output
+---
+impl<T> Request<T> {
+    pub fn fake_builder<K, V>() -> __request_fake_new_builder::__RequestBuilder<
+            (
+                __request_fake_new_builder::__Optional<Vec<(K, V)>>,
+                __request_fake_new_builder::__Optional<Option<http::Uri>>,
+                __request_fake_new_builder::__Optional<Option<http::Method>>,
+                __request_fake_new_builder::__Required<T>,
+            ),
+            T,
+            K,
+            V,
+        > {
+        __request_fake_new_builder::new()
+    }
+}
+mod __request_fake_new_builder {
+    use super::*;
+    pub fn new<T, K, V>() -> __RequestBuilder<
+            (
+                __request_fake_new_builder::__Optional<Vec<(K, V)>>,
+                __request_fake_new_builder::__Optional<Option<http::Uri>>,
+                __request_fake_new_builder::__Optional<Option<http::Method>>,
+                __request_fake_new_builder::__Required<T>,
+            ),
+            T,
+            K,
+            V,
+        > {
+        __RequestBuilder {
+            fields: (__optional(), __optional(), __optional(), __required()),
+            phantom: core::default::Default::default(),
+        }
+    }
+    pub struct __Required<T> {
+        phantom: std::marker::PhantomData<T>,
+    }
+    pub struct __Optional<T> {
+        lazy: Option<T>,
+    }
+    pub struct __Set<T> {
+        value: T,
+    }
+    fn __set<T>(value: T) -> __Set<T> {
+        __Set { value }
+    }
+    fn __required<T>() -> __Required<T> {
+        __Required::<T> {
+            phantom: core::default::Default::default(),
+        }
+    }
+    fn __optional<T>() -> __Optional<T> {
+        __Optional::<T> { lazy: None }
+    }
+    impl<T: Default> From<__Optional<T>> for __Set<T> {
+        fn from(o: __Optional<T>) -> Self {
+            __Set {
+                value: o.lazy.unwrap_or_default(),
+            }
+        }
+    }
+    pub struct __RequestBuilder<__P, T, K, V> {
+        fields: __P,
+        phantom: (
+            core::marker::PhantomData<T>,
+            core::marker::PhantomData<K>,
+            core::marker::PhantomData<V>,
+        ),
+    }
+    impl<
+        __1,
+        __2,
+        __3,
+        T,
+        K,
+        V,
+    > __RequestBuilder<(__Optional<Vec<(K, V)>>, __1, __2, __3), T, K, V> {
+        pub fn headers(
+            mut self,
+            headers: Vec<(K, V)>,
+        ) -> __RequestBuilder<(__Optional<Vec<(K, V)>>, __1, __2, __3), T, K, V>
+        where
+            HeaderName: TryFrom<K>,
+            <HeaderName as TryFrom<K>>::Error: Into<http::Error>,
+            HeaderValue: TryFrom<V>,
+            <HeaderValue as TryFrom<V>>::Error: Into<http::Error>,
+        {
+            self.fields
+                .0
+                .lazy
+                .get_or_insert_with(|| core::default::Default::default())
+                .extend(headers.into_iter());
+            self
+        }
+        pub fn header(
+            mut self,
+            value: (K, V),
+        ) -> __RequestBuilder<(__Optional<Vec<(K, V)>>, __1, __2, __3), T, K, V>
+        where
+            HeaderName: TryFrom<K>,
+            <HeaderName as TryFrom<K>>::Error: Into<http::Error>,
+            HeaderValue: TryFrom<V>,
+            <HeaderValue as TryFrom<V>>::Error: Into<http::Error>,
+        {
+            self.fields
+                .0
+                .lazy
+                .get_or_insert_with(|| core::default::Default::default())
+                .push(value);
+            self
+        }
+    }
+    impl<
+        __0,
+        __2,
+        __3,
+        T,
+        K,
+        V,
+    > __RequestBuilder<(__0, __Optional<Option<http::Uri>>, __2, __3), T, K, V> {
+        pub fn uri(
+            self,
+            uri: http::Uri,
+        ) -> __RequestBuilder<(__0, __Set<Option<http::Uri>>, __2, __3), T, K, V>
+        where
+            HeaderName: TryFrom<K>,
+            <HeaderName as TryFrom<K>>::Error: Into<http::Error>,
+            HeaderValue: TryFrom<V>,
+            <HeaderValue as TryFrom<V>>::Error: Into<http::Error>,
+        {
+            __RequestBuilder {
+                fields: (self.fields.0, __set(Some(uri)), self.fields.2, self.fields.3),
+                phantom: core::default::Default::default(),
+            }
+        }
+        pub fn and_uri(
+            self,
+            uri: Option<http::Uri>,
+        ) -> __RequestBuilder<(__0, __Set<Option<http::Uri>>, __2, __3), T, K, V>
+        where
+            HeaderName: TryFrom<K>,
+            <HeaderName as TryFrom<K>>::Error: Into<http::Error>,
+            HeaderValue: TryFrom<V>,
+            <HeaderValue as TryFrom<V>>::Error: Into<http::Error>,
+        {
+            __RequestBuilder {
+                fields: (self.fields.0, __set(uri), self.fields.2, self.fields.3),
+                phantom: core::default::Default::default(),
+            }
+        }
+    }
+    impl<
+        __0,
+        __1,
+        __3,
+        T,
+        K,
+        V,
+    > __RequestBuilder<(__0, __1, __Optional<Option<http::Method>>, __3), T, K, V> {
+        pub fn method(
+            self,
+            method: http::Method,
+        ) -> __RequestBuilder<(__0, __1, __Set<Option<http::Method>>, __3), T, K, V>
+        where
+            HeaderName: TryFrom<K>,
+            <HeaderName as TryFrom<K>>::Error: Into<http::Error>,
+            HeaderValue: TryFrom<V>,
+            <HeaderValue as TryFrom<V>>::Error: Into<http::Error>,
+        {
+            __RequestBuilder {
+                fields: (
+                    self.fields.0,
+                    self.fields.1,
+                    __set(Some(method)),
+                    self.fields.3,
+                ),
+                phantom: core::default::Default::default(),
+            }
+        }
+        pub fn and_method(
+            self,
+            method: Option<http::Method>,
+        ) -> __RequestBuilder<(__0, __1, __Set<Option<http::Method>>, __3), T, K, V>
+        where
+            HeaderName: TryFrom<K>,
+            <HeaderName as TryFrom<K>>::Error: Into<http::Error>,
+            HeaderValue: TryFrom<V>,
+            <HeaderValue as TryFrom<V>>::Error: Into<http::Error>,
+        {
+            __RequestBuilder {
+                fields: (self.fields.0, self.fields.1, __set(method), self.fields.3),
+                phantom: core::default::Default::default(),
+            }
+        }
+    }
+    impl<
+        __0,
+        __1,
+        __2,
+        T,
+        K,
+        V,
+    > __RequestBuilder<(__0, __1, __2, __Required<T>), T, K, V> {
+        pub fn body(
+            self,
+            body: T,
+        ) -> __RequestBuilder<(__0, __1, __2, __Set<T>), T, K, V> {
+            __RequestBuilder {
+                fields: (self.fields.0, self.fields.1, self.fields.2, __set(body)),
+                phantom: core::default::Default::default(),
+            }
+        }
+    }
+    impl<
+        T,
+        K,
+        V,
+        __P0: Into<__Set<Vec<(K, V)>>>,
+        __P1: Into<__Set<Option<http::Uri>>>,
+        __P2: Into<__Set<Option<http::Method>>>,
+        __P3: Into<__Set<T>>,
+    > __RequestBuilder<(__P0, __P1, __P2, __P3), T, K, V>
+    where
+        HeaderName: TryFrom<K>,
+        <HeaderName as TryFrom<K>>::Error: Into<http::Error>,
+        HeaderValue: TryFrom<V>,
+        <HeaderValue as TryFrom<V>>::Error: Into<http::Error>,
+    {
+        pub fn build(self) -> http::Result<Request<T>> {
+            Request::fake_new(
+                self.fields.0.into().value,
+                self.fields.1.into().value,
+                self.fields.2.into().value,
+                self.fields.3.into().value,
+            )
+        }
+    }
+}
+
diff --git a/tests/buildstructor/pass/complex_generics.rs b/tests/buildstructor/pass/complex_generics.rs
new file mode 100644
index 0000000..f19af74
--- /dev/null
+++ b/tests/buildstructor/pass/complex_generics.rs
@@ -0,0 +1,45 @@
+use buildstructor::builder;
+use http::header::HeaderName;
+use http::HeaderValue;
+use std::error::Error;
+#[derive(Debug)]
+pub struct Request<T> {
+    inner: http::Request<T>,
+}
+
+#[builder]
+impl<T> Request<T> {
+    pub fn fake_new<K, V>(
+        headers: Vec<(K, V)>,
+        uri: Option<http::Uri>,
+        method: Option<http::Method>,
+        body: T,
+    ) -> http::Result<Request<T>>
+    where
+        HeaderName: TryFrom<K>,
+        <HeaderName as TryFrom<K>>::Error: Into<http::Error>,
+        HeaderValue: TryFrom<V>,
+        <HeaderValue as TryFrom<V>>::Error: Into<http::Error>,
+    {
+        let mut builder = http::request::Builder::new()
+            .method(method.unwrap_or_default())
+            .uri(uri.unwrap_or_default());
+        for (key, value) in headers {
+            builder = builder.header(key, value);
+        }
+        let req = builder.body(body)?;
+
+        Ok(Self { inner: req })
+    }
+}
+
+fn main() -> Result<(), Box<dyn Error>> {
+    let _ = Request::fake_builder()
+        .header(("a".to_string(), "b".to_string()))
+        .header(("a".to_string(), "b".to_string()))
+        .body("")
+        .build()
+        .unwrap();
+
+    Ok(())
+}