Ordering of generics bug that was causing issue with complex generic …

…types Fixes #14
BrynCooke · Apr 23, 2022 · 334a474 · 334a474
1 parent a08204d
commit 334a474
Show file tree

Hide file tree

Showing 7 changed files with 344 additions and 15 deletions.
diff --git a/.tool-versions b/.tool-versions
@@ -1 +1 @@
-rust 1.59.0
+rust 1.60.0
diff --git 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
@@ -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
@@ -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
@@ -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 })
+                }
+            }
+        )
+    }
 }