diff --git a/bon-macros/src/builder/builder_gen/generic_setters.rs b/bon-macros/src/builder/builder_gen/generic_setters.rs index c1e21613..7b7f04d3 100644 --- a/bon-macros/src/builder/builder_gen/generic_setters.rs +++ b/bon-macros/src/builder/builder_gen/generic_setters.rs @@ -1,8 +1,10 @@ use super::models::BuilderGenCtx; use crate::parsing::ItemSigConfig; use crate::util::prelude::*; +use std::collections::BTreeSet; use syn::punctuated::Punctuated; use syn::token::Where; +use syn::visit::Visit; pub(super) struct GenericSettersCtx<'a> { base: &'a BuilderGenCtx, @@ -28,19 +30,24 @@ impl<'a> GenericSettersCtx<'a> { // Check for interdependent type parameters in generic bounds for param in generics { if let syn::GenericParam::Type(type_param) = param { - let params_in_bounds = - find_type_params_in_bounds(&type_param.bounds, &type_param_idents); - if params_in_bounds.len() > 1 - || (params_in_bounds.len() == 1 - && params_in_bounds.first() != Some(&&type_param.ident)) - { - let params_str = params_in_bounds + let mut params = TypeParamFinder::new(&type_param_idents); + + for bound in &type_param.bounds { + params.visit_type_param_bound(bound); + } + + // Self-referential type params are fine + params.found.remove(&type_param.ident); + + if let Some(first_param) = params.found.iter().next() { + let params_str = params + .found .iter() .map(|p| format!("`{p}`")) .collect::>() .join(", "); bail!( - &type_param.bounds, + first_param, "generic conversion methods cannot be generated for interdependent type parameters; \ the bounds on generic parameter `{}` reference other type parameters: {}\n\ \n\ @@ -55,10 +62,11 @@ impl<'a> GenericSettersCtx<'a> { // Check for interdependent type parameters in where clauses if let Some(where_clause) = &self.base.generics.where_clause { for predicate in &where_clause.predicates { - let params_in_predicate = - find_type_params_in_predicate(predicate, &type_param_idents); - if params_in_predicate.len() > 1 { - let params_str = params_in_predicate + let mut params = TypeParamFinder::new(&type_param_idents); + params.visit_where_predicate(predicate); + if params.found.len() > 1 { + let params_str = params + .found .iter() .map(|p| format!("`{p}`")) .collect::>() @@ -85,8 +93,9 @@ impl<'a> GenericSettersCtx<'a> { syn::GenericParam::Const(const_param) => { bail!( &const_param.ident, - "const generic parameters are not supported in `generics(setters(...))`; \ - only type parameters can be converted" + "const generic parameters are not yet supported with `generics(setters(...))`; \ + only type parameters can be overridden, feel free to open an issue if you need \ + this feature" ); } syn::GenericParam::Lifetime(_) => { @@ -122,8 +131,12 @@ impl<'a> GenericSettersCtx<'a> { let docs = self.method_docs(param_ident); // Build the generic arguments for the output type, where the current parameter - // is replaced with a new type variable - let new_type_var = self.base.namespace.unique_ident(param_ident.to_string()); + // is replaced with a new type variable. Even though the `GenericsNamespace` + let new_type_var = self + .base + .namespace + // Add `New` prefix to make the type variable more readable in the docs and IDE hints + .unique_ident(format!("New{param_ident}")); // Copy the bounds from the original type parameter to the new one let bounds = &type_param.bounds; @@ -168,7 +181,10 @@ impl<'a> GenericSettersCtx<'a> { // Add runtime assert that this field is None let field_ident = &member.name.orig; - let message = format!("BUG: field `{field_ident}` should be None when converting generic parameter `{param_ident}`"); + let message = format!( + "BUG: field `{field_ident}` should be None \ + when converting generic parameter `{param_ident}`" + ); runtime_asserts.push(quote! { ::core::assert!(named.#index.is_none(), #message); }); @@ -211,11 +227,7 @@ impl<'a> GenericSettersCtx<'a> { clause.predicates.push(syn::parse_quote!(#bound)); } - if clause.predicates.is_empty() { - None - } else { - Some(clause) - } + (!clause.predicates.is_empty()).then(|| clause) }; quote! { @@ -280,82 +292,34 @@ impl<'a> GenericSettersCtx<'a> { } } -fn find_type_params_in_bounds<'b>( - bounds: &Punctuated, - type_params: &'b [&'b syn::Ident], -) -> Vec<&'b syn::Ident> { - use syn::visit::Visit; +struct TypeParamFinder<'ty, 'ast> { + type_params: &'ty [&'ty syn::Ident], - struct TypeParamFinder<'a> { - type_params: &'a [&'a syn::Ident], - found: std::collections::HashSet<&'a syn::Ident>, - } + // Use a `BTreeSet` for deterministic ordering + found: BTreeSet<&'ast syn::Ident>, +} - impl<'ast> Visit<'ast> for TypeParamFinder<'_> { - fn visit_path(&mut self, path: &'ast syn::Path) { - // Check if this path is one of our type parameters - for ¶m in self.type_params { - if path.is_ident(param) { - self.found.insert(param); - } - } - // Continue visiting nested paths - syn::visit::visit_path(self, path); +impl<'ty> TypeParamFinder<'ty, '_> { + fn new(type_params: &'ty [&'ty syn::Ident]) -> Self { + Self { + type_params, + found: BTreeSet::new(), } } - - let mut finder = TypeParamFinder { - type_params, - found: std::collections::HashSet::new(), - }; - - for bound in bounds { - finder.visit_type_param_bound(bound); - } - - // Preserve the original order of type parameters for deterministic output - type_params - .iter() - .filter(|param| finder.found.contains(*param)) - .copied() - .collect() } -fn find_type_params_in_predicate<'b>( - predicate: &syn::WherePredicate, - type_params: &'b [&'b syn::Ident], -) -> Vec<&'b syn::Ident> { - use syn::visit::Visit; - - struct TypeParamFinder<'a> { - type_params: &'a [&'a syn::Ident], - found: std::collections::HashSet<&'a syn::Ident>, - } - - impl<'ast> Visit<'ast> for TypeParamFinder<'_> { - fn visit_path(&mut self, path: &'ast syn::Path) { - // Check if this path is one of our type parameters - for ¶m in self.type_params { - if path.is_ident(param) { - self.found.insert(param); - } +impl<'ast> Visit<'ast> for TypeParamFinder<'_, 'ast> { + fn visit_path(&mut self, path: &'ast syn::Path) { + // Check if this path is one of our type parameters + if let Some(param) = path.get_ident() { + if self.type_params.contains(¶m) { + self.found.insert(param); } - // Continue visiting nested paths - syn::visit::visit_path(self, path); } - } - let mut finder = TypeParamFinder { - type_params, - found: std::collections::HashSet::new(), - }; - finder.visit_where_predicate(predicate); - // Preserve the original order of type parameters for deterministic output - type_params - .iter() - .filter(|param| finder.found.contains(*param)) - .copied() - .collect() + // Continue visiting nested paths + syn::visit::visit_path(self, path); + } } fn replace_type_param_in_predicate( @@ -406,8 +370,6 @@ fn member_uses_generic_param(member: &super::NamedMember, param_ident: &syn::Ide /// Recursively check if a type uses a specific generic parameter fn type_uses_generic_param(ty: &syn::Type, param_ident: &syn::Ident) -> bool { - use syn::visit::Visit; - struct GenericParamVisitor<'a> { param_ident: &'a syn::Ident, found: bool, diff --git a/bon-macros/src/builder/builder_gen/top_level_config/mod.rs b/bon-macros/src/builder/builder_gen/top_level_config/mod.rs index a31b214c..5fc29675 100644 --- a/bon-macros/src/builder/builder_gen/top_level_config/mod.rs +++ b/bon-macros/src/builder/builder_gen/top_level_config/mod.rs @@ -183,6 +183,19 @@ impl TopLevelConfig { ..Self::from_list(&configs)? }; + if let Some(generics) = &me.generics { + if generics.setters.is_some() { + if let Some(const_) = &me.const_ { + bail!( + const_, + "`generics(setters(...))` cannot be used together with `const` \ + functions; if you have a use case for this, consider opening an \ + issue to discuss it!" + ); + } + } + } + if let Some(on) = me.on.iter().skip(1).find(|on| on.required.is_present()) { bail!( &on.required.span(), diff --git a/bon/tests/integration/ui/compile_fail/generics_setters/fn_pointer.rs b/bon/tests/integration/ui/compile_fail/generics_setters/fn_pointer.rs index 125d293d..dcff395c 100644 --- a/bon/tests/integration/ui/compile_fail/generics_setters/fn_pointer.rs +++ b/bon/tests/integration/ui/compile_fail/generics_setters/fn_pointer.rs @@ -5,14 +5,35 @@ use bon::Builder; #[derive(Builder)] #[builder(generics(setters(name = "conv_{}")))] -struct FnPointerField { +struct FnPointerFieldInOut { value: fn(T) -> T, } +#[derive(Builder)] +#[builder(generics(setters(name = "conv_{}")))] +struct FnPointerFieldIn { + value: fn(T), +} + +#[derive(Builder)] +#[builder(generics(setters(name = "conv_{}")))] +struct FnPointerFieldOut { + value: fn() -> T, +} + fn main() { - // Test fn(T) -> T - can't change type after setting field - FnPointerField::<()>::builder() - .value(|x| x) + FnPointerFieldInOut::<()>::builder() + .value(|()| ()) + .conv_t::() + .build(); + + FnPointerFieldIn::<()>::builder() + .value(|()| ()) + .conv_t::() + .build(); + + FnPointerFieldOut::<()>::builder() + .value(|| ()) .conv_t::() .build(); } diff --git a/bon/tests/integration/ui/compile_fail/generics_setters/fn_pointer.stderr b/bon/tests/integration/ui/compile_fail/generics_setters/fn_pointer.stderr index 171d9b44..df83b094 100644 --- a/bon/tests/integration/ui/compile_fail/generics_setters/fn_pointer.stderr +++ b/bon/tests/integration/ui/compile_fail/generics_setters/fn_pointer.stderr @@ -1,16 +1,50 @@ -error[E0277]: the member `Set` was already set, but this method requires it to be unset - --> tests/integration/ui/compile_fail/generics_setters/fn_pointer.rs:16:10 +error[E0277]: the member `Set` was already set, but this method requires it to be unset + --> tests/integration/ui/compile_fail/generics_setters/fn_pointer.rs:27:10 | -16 | .conv_t::() - | ^^^^^^ the member `Set` was already set, but this method requires it to be unset +27 | .conv_t::() + | ^^^^^^ the member `Set` was already set, but this method requires it to be unset | - = help: the trait `IsUnset` is not implemented for `Set` -note: required by a bound in `FnPointerFieldBuilder::::conv_t` + = help: the trait `IsUnset` is not implemented for `Set` +note: required by a bound in `FnPointerFieldInOutBuilder::::conv_t` --> tests/integration/ui/compile_fail/generics_setters/fn_pointer.rs:6:10 | 6 | #[derive(Builder)] - | ^^^^^^^ required by this bound in `FnPointerFieldBuilder::::conv_t` + | ^^^^^^^ required by this bound in `FnPointerFieldInOutBuilder::::conv_t` 7 | #[builder(generics(setters(name = "conv_{}")))] - 8 | struct FnPointerField { - | - required by a bound in this associated function + 8 | struct FnPointerFieldInOut { + | - required by a bound in this associated function + = note: this error originates in the derive macro `Builder` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0277]: the member `Set` was already set, but this method requires it to be unset + --> tests/integration/ui/compile_fail/generics_setters/fn_pointer.rs:32:10 + | +32 | .conv_t::() + | ^^^^^^ the member `Set` was already set, but this method requires it to be unset + | + = help: the trait `IsUnset` is not implemented for `Set` +note: required by a bound in `FnPointerFieldInBuilder::::conv_t` + --> tests/integration/ui/compile_fail/generics_setters/fn_pointer.rs:12:10 + | +12 | #[derive(Builder)] + | ^^^^^^^ required by this bound in `FnPointerFieldInBuilder::::conv_t` +13 | #[builder(generics(setters(name = "conv_{}")))] +14 | struct FnPointerFieldIn { + | - required by a bound in this associated function + = note: this error originates in the derive macro `Builder` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0277]: the member `Set` was already set, but this method requires it to be unset + --> tests/integration/ui/compile_fail/generics_setters/fn_pointer.rs:37:10 + | +37 | .conv_t::() + | ^^^^^^ the member `Set` was already set, but this method requires it to be unset + | + = help: the trait `IsUnset` is not implemented for `Set` +note: required by a bound in `FnPointerFieldOutBuilder::::conv_t` + --> tests/integration/ui/compile_fail/generics_setters/fn_pointer.rs:18:10 + | +18 | #[derive(Builder)] + | ^^^^^^^ required by this bound in `FnPointerFieldOutBuilder::::conv_t` +19 | #[builder(generics(setters(name = "conv_{}")))] +20 | struct FnPointerFieldOut { + | - required by a bound in this associated function = note: this error originates in the derive macro `Builder` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/bon/tests/integration/ui/compile_fail/generics_setters/interdependent_param_bounds.stderr b/bon/tests/integration/ui/compile_fail/generics_setters/interdependent_param_bounds.stderr index b514575d..2d4f239b 100644 --- a/bon/tests/integration/ui/compile_fail/generics_setters/interdependent_param_bounds.stderr +++ b/bon/tests/integration/ui/compile_fail/generics_setters/interdependent_param_bounds.stderr @@ -1,7 +1,7 @@ error: generic conversion methods cannot be generated for interdependent type parameters; the bounds on generic parameter `Iter` reference other type parameters: `Item` Consider removing `generics(setters(...))` or restructuring your types to avoid interdependencies - --> tests/integration/ui/compile_fail/generics_setters/interdependent_param_bounds.rs:7:18 + --> tests/integration/ui/compile_fail/generics_setters/interdependent_param_bounds.rs:7:34 | 7 | struct Sut, Item> { - | ^^^^^^^^ + | ^^^^ diff --git a/scripts/test-msrv.sh b/scripts/test-msrv.sh index d9808a7c..83d0ade1 100755 --- a/scripts/test-msrv.sh +++ b/scripts/test-msrv.sh @@ -35,6 +35,7 @@ with_log cd bon step echo '[workspace]' >> Cargo.toml step cargo update --precise 0.21.3 -p darling +step cargo update --precise 1.0.22 -p unicode-ident step cargo update --precise 1.0.15 -p itoa step cargo update --precise 1.0.101 -p proc-macro2 step cargo update --precise 1.0.40 -p quote