Repository: BrynCooke/buildstructor (language: rust)

Issue:
Visibility of builders should match the visibility of the build method.
Otherwise the compile will complain of leakage of types.


Discussion / hints:

Fix patch:
diff --git a/src/buildstructor/codegen.rs b/src/buildstructor/codegen.rs
index 6f121f1..c80c736 100644
--- a/src/buildstructor/codegen.rs
+++ b/src/buildstructor/codegen.rs
@@ -53,6 +53,7 @@ pub fn codegen(ir: Ir) -> Result<TokenStream> {
     let async_token = ir.is_async.then(|| quote! {async});
     let await_token = ir.is_async.then(|| quote! {.await});
     let vis = &ir.vis;
+    let builder_vis = &ir.builder_vis;
 
     Ok(quote! {
         impl #impl_generics #target_name #ty_generics #where_clause {
@@ -115,7 +116,7 @@ pub fn codegen(ir: Ir) -> Result<TokenStream> {
             #(#builder_methods)*
 
             impl #builder_impl_generics #builder_name #builder_tuple_ty_generics #builder_where_clause {
-                pub #async_token fn build(self) #constructor_return {
+                #builder_vis #async_token fn build(self) #constructor_return {
                     #target_name::#constructor_method_name(#(#constructor_args),*) #await_token
                 }
             }
diff --git a/src/buildstructor/lower.rs b/src/buildstructor/lower.rs
index 15b23ae..3218c35 100644
--- a/src/buildstructor/lower.rs
+++ b/src/buildstructor/lower.rs
@@ -6,7 +6,7 @@ use std::default::Default;
 use syn::punctuated::Punctuated;
 use syn::{
     Expr, ExprField, FnArg, GenericArgument, GenericParam, Generics, Index, Member, Pat, Result,
-    ReturnType, Type, TypeParam, TypeTuple, Visibility,
+    ReturnType, Type, TypeParam, TypeTuple, VisRestricted, Visibility,
 };
 use try_match::try_match;
 
@@ -20,6 +20,7 @@ pub struct Ir {
     pub return_type: ReturnType,
     pub is_async: bool,
     pub vis: Visibility,
+    pub builder_vis: Visibility,
     pub generics: Generics,
     pub builder_generics: Generics,
     pub method_generics: Generics,
@@ -49,8 +50,13 @@ pub enum FieldType {
 }
 
 pub fn lower(model: ConstrutorModel) -> Result<Ir> {
+    // Either visibility is set explicitly or we default to super.
+    let vis = model.vis.clone();
+    let builder_vis = builder_vilibility(&vis);
+
     Ok(Ir {
-        vis: model.vis.clone(),
+        vis,
+        builder_vis,
         module_name: format_ident!(
             "__{}_{}_builder",
             model.ident.to_string().to_lowercase(),
@@ -70,6 +76,19 @@ pub fn lower(model: ConstrutorModel) -> Result<Ir> {
     })
 }
 
+fn builder_vilibility(vis: &Visibility) -> Visibility {
+    if let Visibility::Inherited = vis {
+        Visibility::Restricted(VisRestricted {
+            pub_token: Default::default(),
+            paren_token: Default::default(),
+            in_token: None,
+            path: Box::new(format_ident!("super").to_path()),
+        })
+    } else {
+        vis.clone()
+    }
+}
+
 fn builder_return_type(mut return_type: ReturnType, target: Ident) -> ReturnType {
     if let ReturnType::Type(_, ty) = &mut return_type {
         let self_type = Box::new(Type::Path(format_ident!("Self").to_type_path()));


Test patch:
diff --git a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__async_test.snap b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__async_test.snap
index e3b3f55..abb90bc 100644
--- a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__async_test.snap
+++ b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__async_test.snap
@@ -58,7 +58,7 @@ mod __foo_new_builder {
         }
     }
     impl<__P0: Into<__Set<usize>>> __FooBuilder<(__P0,)> {
-        pub async fn build(self) -> Foo {
+        pub(super) async fn build(self) -> Foo {
             Foo::new(self.fields.0.into().value).await
         }
     }
diff --git a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__collection_generics_test.snap b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__collection_generics_test.snap
index e32642b..e8cf185 100644
--- a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__collection_generics_test.snap
+++ b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__collection_generics_test.snap
@@ -87,7 +87,7 @@ mod __foo_new_builder {
         V: Into<String>,
         __P0: Into<__Set<HashMap<K, V>>>,
     > __FooBuilder<(__P0,), K, V> {
-        pub fn build(self) -> Foo {
+        pub(super) fn build(self) -> Foo {
             Foo::new(self.fields.0.into().value)
         }
     }
diff --git a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__collection_generics_test2.snap b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__collection_generics_test2.snap
index bb34014..9a680aa 100644
--- a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__collection_generics_test2.snap
+++ b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__collection_generics_test2.snap
@@ -123,7 +123,7 @@ mod __collections_new_builder {
         __P0: Into<__Set<HashMap<K, V>>>,
         __P1: Into<__Set<HashSet<K>>>,
     > __CollectionsBuilder<(__P0, __P1), K, V> {
-        pub fn build(self) -> Collections {
+        pub(super) fn build(self) -> Collections {
             Collections::new(self.fields.0.into().value, self.fields.1.into().value)
         }
     }
diff --git a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__collection_option_test.snap b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__collection_option_test.snap
index 8b05cfd..ec2b7b1 100644
--- a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__collection_option_test.snap
+++ b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__collection_option_test.snap
@@ -78,7 +78,7 @@ mod __foo_new_builder {
     impl<
         __P0: Into<__Set<HashMap<Option<String>, Option<String>>>>,
     > __FooBuilder<(__P0,)> {
-        pub fn build(self) -> Foo {
+        pub(super) fn build(self) -> Foo {
             Foo::new(self.fields.0.into().value)
         }
     }
diff --git a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__collection_test.snap b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__collection_test.snap
index a89de53..d65423e 100644
--- a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__collection_test.snap
+++ b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__collection_test.snap
@@ -264,7 +264,7 @@ mod __foo_new_builder {
         __P4: Into<__Set<BTreeMap<String, String>>>,
         __P5: Into<__Set<BTreeSet<String>>>,
     > __FooBuilder<(__P0, __P1, __P2, __P3, __P4, __P5)> {
-        pub fn build(self) -> Foo {
+        pub(super) fn build(self) -> Foo {
             Foo::new(
                 self.fields.0.into().value,
                 self.fields.1.into().value,
diff --git a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__fallible_test.snap b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__fallible_test.snap
index 598c60e..8126415 100644
--- a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__fallible_test.snap
+++ b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__fallible_test.snap
@@ -58,7 +58,7 @@ mod __foo_new_builder {
         }
     }
     impl<__P0: Into<__Set<usize>>> __FooBuilder<(__P0,)> {
-        pub fn build(self) -> Result<Foo, String> {
+        pub(super) fn build(self) -> Result<Foo, String> {
             Foo::new(self.fields.0.into().value)
         }
     }
diff --git a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__generic_test.snap b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__generic_test.snap
index 8d4e959..1932589 100644
--- a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__generic_test.snap
+++ b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__generic_test.snap
@@ -59,7 +59,7 @@ mod __foo_new_builder {
         }
     }
     impl<T, __P0: Into<__Set<T>>> __FooBuilder<(__P0,), T> {
-        pub fn build(self) -> Foo<T> {
+        pub(super) fn build(self) -> Foo<T> {
             Foo::new(self.fields.0.into().value)
         }
     }
diff --git a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__into_test.snap b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__into_test.snap
index 629e63c..5675232 100644
--- a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__into_test.snap
+++ b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__into_test.snap
@@ -62,7 +62,7 @@ mod __foo_new_builder {
         }
     }
     impl<T: Into<String>, __P0: Into<__Set<T>>> __FooBuilder<(__P0,), T> {
-        pub fn build(self) -> Foo {
+        pub(super) fn build(self) -> Foo {
             Foo::new(self.fields.0.into().value)
         }
     }
diff --git a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__into_where_test.snap b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__into_where_test.snap
index 273a94d..5254b4e 100644
--- a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__into_where_test.snap
+++ b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__into_where_test.snap
@@ -62,7 +62,7 @@ mod __foo_new_builder {
     where
         T: Into<String>,
     {
-        pub fn build(self) -> Foo {
+        pub(super) fn build(self) -> Foo {
             Foo::new(self.fields.0.into().value)
         }
     }
diff --git a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__multi_field_test.snap b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__multi_field_test.snap
index 2b07fd4..d11e253 100644
--- a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__multi_field_test.snap
+++ b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__multi_field_test.snap
@@ -69,7 +69,7 @@ mod __foo_new_builder {
         }
     }
     impl<__P0: Into<__Set<usize>>, __P1: Into<__Set<usize>>> __FooBuilder<(__P0, __P1)> {
-        pub fn build(self) -> Foo {
+        pub(super) fn build(self) -> Foo {
             Foo::new(self.fields.0.into().value, self.fields.1.into().value)
         }
     }
diff --git a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__option_test.snap b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__option_test.snap
index d4d0f18..85cf574 100644
--- a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__option_test.snap
+++ b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__option_test.snap
@@ -68,7 +68,7 @@ mod __foo_new_builder {
         }
     }
     impl<__P0: Into<__Set<Option<usize>>>> __FooBuilder<(__P0,)> {
-        pub fn build(self) -> Foo {
+        pub(super) fn build(self) -> Foo {
             Foo::new(self.fields.0.into().value)
         }
     }
diff --git a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__returns_self_test.snap b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__returns_self_test.snap
index 923cdf4..c1e86f1 100644
--- a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__returns_self_test.snap
+++ b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__returns_self_test.snap
@@ -58,7 +58,7 @@ mod __foo_new_builder {
         }
     }
     impl<__P0: Into<__Set<usize>>> __FooBuilder<(__P0,)> {
-        pub fn build(self) -> Foo {
+        pub(super) fn build(self) -> Foo {
             Foo::new(self.fields.0.into().value)
         }
     }
diff --git a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__single_field_test.snap b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__single_field_test.snap
index 923cdf4..c1e86f1 100644
--- a/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__single_field_test.snap
+++ b/src/buildstructor/snapshots/buildstructor__buildstructor__codegen__tests__single_field_test.snap
@@ -58,7 +58,7 @@ mod __foo_new_builder {
         }
     }
     impl<__P0: Into<__Set<usize>>> __FooBuilder<(__P0,)> {
-        pub fn build(self) -> Foo {
+        pub(super) fn build(self) -> Foo {
             Foo::new(self.fields.0.into().value)
         }
     }
diff --git a/tests/buildstructor/pass/crate_visibility.rs b/tests/buildstructor/pass/crate_visibility.rs
new file mode 100644
index 0000000..8befaef
--- /dev/null
+++ b/tests/buildstructor/pass/crate_visibility.rs
@@ -0,0 +1,48 @@
+mod sub1 {
+    pub(crate) struct Foo {
+        simple: usize,
+    }
+
+    #[buildstructor::builder]
+    impl Foo {
+        pub(crate) fn new(simple: usize) -> Self {
+            Self { simple }
+        }
+    }
+}
+
+mod sub2 {
+    pub struct Foo {
+        simple: usize,
+    }
+
+    #[buildstructor::builder]
+    impl Foo {
+        pub fn new(simple: usize) -> Self {
+            Self { simple }
+        }
+    }
+}
+
+mod sub3 {
+    struct Foo {
+        simple: usize,
+    }
+
+    #[buildstructor::builder]
+    impl Foo {
+        fn new(simple: usize) -> Self {
+            Self { simple }
+        }
+    }
+
+    pub fn foo() {
+        let _ = Foo::builder().simple(3).build();
+    }
+}
+
+fn main() {
+    let _ = sub1::Foo::builder().simple(3).build();
+    let _ = sub2::Foo::builder().simple(3).build();
+    sub3::foo();
+}