From f9a3b76ea45ca5ffa3d4cadfccc156bca7c76450 Mon Sep 17 00:00:00 2001 From: Veetaha Date: Fri, 25 Oct 2024 18:33:50 +0000 Subject: [PATCH] `#[builder(on(_, transparent))]` attribute --- .../src/builder/builder_gen/input_struct.rs | 3 +- .../src/builder/builder_gen/member/named.rs | 9 + .../builder_gen/top_level_config/mod.rs | 59 +++- .../builder_gen/top_level_config/on.rs | 21 +- .../integration/builder/attr_transparent.rs | 334 ++++++++++++------ .../integration/ui/compile_fail/attr_on.rs | 30 ++ .../ui/compile_fail/attr_on.stderr | 32 +- 7 files changed, 379 insertions(+), 109 deletions(-) diff --git a/bon-macros/src/builder/builder_gen/input_struct.rs b/bon-macros/src/builder/builder_gen/input_struct.rs index de458114..95174c1c 100644 --- a/bon-macros/src/builder/builder_gen/input_struct.rs +++ b/bon-macros/src/builder/builder_gen/input_struct.rs @@ -7,7 +7,6 @@ use crate::builder::builder_gen::models::{BuilderGenCtxParams, BuilderTypeParams use crate::normalization::{GenericsNamespace, SyntaxVariant}; use crate::parsing::{ItemSigConfig, SpannedKey}; use crate::util::prelude::*; -use darling::FromMeta; use std::borrow::Cow; use syn::visit::Visit; use syn::visit_mut::VisitMut; @@ -40,7 +39,7 @@ fn parse_top_level_config(item_struct: &syn::ItemStruct) -> Result Result { + // This is a temporary hack. We only allow `on(_, transparent)` as the + // first `on(...)` clause. Instead we should implement the extended design: + // https://github.com/elastio/bon/issues/152 + if let Some(on) = on.first().filter(|on| on.transparent.is_present()) { + if self.is_special_option_ty() { + self.config.transparent = on.transparent; + } + } + self.merge_config_into(on)?; // FIXME: refactor this to make it more consistent with `into` 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 17c612e1..84f1a47f 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 @@ -68,7 +68,7 @@ pub(crate) struct TopLevelConfig { impl TopLevelConfig { pub(crate) fn parse_for_fn(meta_list: &[darling::ast::NestedMeta]) -> Result { - let me = Self::from_list(meta_list)?; + let me = Self::parse_for_any(meta_list)?; if me.start_fn.name.is_none() { let ItemSigConfig { name: _, vis, docs } = &me.start_fn; @@ -95,6 +95,63 @@ impl TopLevelConfig { Ok(me) } + + pub(crate) fn parse_for_struct(meta_list: &[darling::ast::NestedMeta]) -> Result { + Self::parse_for_any(meta_list) + } + + fn parse_for_any(meta_list: &[darling::ast::NestedMeta]) -> Result { + // This is a temporary hack. We only allow `on(_, transparent)` as the + // first `on(...)` clause. Instead we should implement an extended design: + // https://github.com/elastio/bon/issues/152 + let mut on_configs = meta_list + .iter() + .enumerate() + .filter_map(|(i, meta)| match meta { + darling::ast::NestedMeta::Meta(syn::Meta::List(meta)) + if meta.path.is_ident("on") => + { + Some((i, meta)) + } + _ => None, + }) + .peekable(); + + while let Some((i, _)) = on_configs.next() { + if let Some((j, next_on)) = on_configs.peek() { + if *j != i + 1 { + bail!( + next_on, + "this `on(...)` clause is out of order; all `on(...)` \ + clauses must be consecutive; there shouldn't be any \ + other parameters between them", + ) + } + } + } + + let me = Self::from_list(meta_list)?; + + if let Some(on) = me.on.iter().skip(1).find(|on| on.transparent.is_present()) { + bail!( + &on.transparent.span(), + "`transparent` can only be specified in the first `on(...)` clause; \ + this restriction may be lifted in the future", + ); + } + + if let Some(first_on) = me.on.first().filter(|on| on.transparent.is_present()) { + if !matches!(first_on.type_pattern, syn::Type::Infer(_)) { + bail!( + &first_on.type_pattern, + "`transparent` can only be used with the wildcard type pattern \ + i.e. `on(_, transparent)`; this restriction may be lifted in the future", + ); + } + } + + Ok(me) + } } #[derive(Debug, Clone, Default, FromMeta)] diff --git a/bon-macros/src/builder/builder_gen/top_level_config/on.rs b/bon-macros/src/builder/builder_gen/top_level_config/on.rs index 6a105dd1..f3e624a8 100644 --- a/bon-macros/src/builder/builder_gen/top_level_config/on.rs +++ b/bon-macros/src/builder/builder_gen/top_level_config/on.rs @@ -9,6 +9,7 @@ pub(crate) struct OnConfig { pub(crate) type_pattern: syn::Type, pub(crate) into: darling::util::Flag, pub(crate) overwritable: darling::util::Flag, + pub(crate) transparent: darling::util::Flag, } impl Parse for OnConfig { @@ -22,6 +23,7 @@ impl Parse for OnConfig { struct Parsed { into: darling::util::Flag, overwritable: darling::util::Flag, + transparent: darling::util::Flag, } let parsed = Parsed::from_meta(&syn::parse_quote!(on(#rest)))?; @@ -31,8 +33,16 @@ impl Parse for OnConfig { // This lives in a separate block to make sure that if a new // field is added to `Parsed` and unused here, then a compiler // warning is emitted. - let Parsed { into, overwritable } = &parsed; - let flags = [("into", into), ("overwritable", overwritable)]; + let Parsed { + into, + overwritable, + transparent, + } = &parsed; + let flags = [ + ("into", into), + ("overwritable", overwritable), + ("transparent", transparent), + ]; if flags.iter().all(|(_, flag)| !flag.is_present()) { let flags = flags.iter().map(|(name, _)| format!("`{name}`")).join(", "); @@ -76,12 +86,17 @@ impl Parse for OnConfig { "BUG: the type pattern does not match itself: {type_pattern:#?}" ); - let Parsed { into, overwritable } = parsed; + let Parsed { + into, + overwritable, + transparent, + } = parsed; Ok(Self { type_pattern, into, overwritable, + transparent, }) } } diff --git a/bon/tests/integration/builder/attr_transparent.rs b/bon/tests/integration/builder/attr_transparent.rs index b372003a..aae84f53 100644 --- a/bon/tests/integration/builder/attr_transparent.rs +++ b/bon/tests/integration/builder/attr_transparent.rs @@ -1,85 +1,58 @@ -use crate::prelude::*; -use core::fmt; +mod member_level { + use crate::prelude::*; + use core::fmt; -#[test] -fn test_struct() { - #[derive(Debug, Builder)] - #[allow(dead_code)] - struct Sut { - #[builder(transparent)] - regular: Option, + #[test] + fn test_struct() { + #[derive(Debug, Builder)] + #[allow(dead_code)] + struct Sut { + #[builder(transparent)] + regular: Option, - #[builder(transparent)] - generic: Option, + #[builder(transparent)] + generic: Option, - #[builder(transparent, into)] - with_into: Option, + #[builder(transparent, into)] + with_into: Option, - #[builder(transparent, default = Some(99))] - with_default: Option, + #[builder(transparent, default = Some(99))] + with_default: Option, - #[builder(transparent, default = Some(10))] - with_default_2: Option, - } - - assert_debug_eq( - Sut::builder() - .regular(Some(1)) - .generic(Some(false)) - .with_into(2) - .maybe_with_default_2(Some(Some(3))) - .build(), - expect![[r#" - Sut { - regular: Some( - 1, - ), - generic: Some( - false, - ), - with_into: Some( - 2, - ), - with_default: Some( - 99, - ), - with_default_2: Some( - 3, - ), - }"#]], - ); -} + #[builder(transparent, default = Some(10))] + with_default_2: Option, + } -#[test] -fn test_free_fn() { - #[builder] - fn sut( - #[builder(transparent)] regular: Option, - #[builder(transparent)] generic: Option, - #[builder(transparent, into)] with_into: Option, - #[builder(transparent, default = Some(99))] with_default: Option, - #[builder(transparent, default = Some(10))] with_default_2: Option, - ) -> impl fmt::Debug { - (regular, generic, with_into, with_default, with_default_2) + assert_debug_eq( + Sut::builder() + .regular(Some(1)) + .generic(Some(false)) + .with_into(2) + .maybe_with_default_2(Some(Some(3))) + .build(), + expect![[r#" + Sut { + regular: Some( + 1, + ), + generic: Some( + false, + ), + with_into: Some( + 2, + ), + with_default: Some( + 99, + ), + with_default_2: Some( + 3, + ), + }"#]], + ); } - assert_debug_eq( - sut() - .regular(Some(1)) - .generic(Some(false)) - .with_into(2) - .maybe_with_default_2(Some(Some(3))) - .call(), - expect!["(Some(1), Some(false), Some(2), Some(99), Some(3))"], - ); -} - -#[test] -fn test_assoc_method() { - struct Sut; - - #[bon] - impl Sut { + #[test] + fn test_free_fn() { #[builder] fn sut( #[builder(transparent)] regular: Option, @@ -91,37 +64,194 @@ fn test_assoc_method() { (regular, generic, with_into, with_default, with_default_2) } - #[builder] - fn with_self( - &self, - #[builder(transparent)] regular: Option, - #[builder(transparent)] generic: Option, - #[builder(transparent, into)] with_into: Option, - #[builder(transparent, default = Some(99))] with_default: Option, - #[builder(transparent, default = Some(10))] with_default_2: Option, + assert_debug_eq( + sut() + .regular(Some(1)) + .generic(Some(false)) + .with_into(2) + .maybe_with_default_2(Some(Some(3))) + .call(), + expect!["(Some(1), Some(false), Some(2), Some(99), Some(3))"], + ); + } + + #[test] + fn test_assoc_method() { + struct Sut; + + #[bon] + impl Sut { + #[builder] + fn sut( + #[builder(transparent)] regular: Option, + #[builder(transparent)] generic: Option, + #[builder(transparent, into)] with_into: Option, + #[builder(transparent, default = Some(99))] with_default: Option, + #[builder(transparent, default = Some(10))] with_default_2: Option, + ) -> impl fmt::Debug { + (regular, generic, with_into, with_default, with_default_2) + } + + #[builder] + fn with_self( + &self, + #[builder(transparent)] regular: Option, + #[builder(transparent)] generic: Option, + #[builder(transparent, into)] with_into: Option, + #[builder(transparent, default = Some(99))] with_default: Option, + #[builder(transparent, default = Some(10))] with_default_2: Option, + ) -> impl fmt::Debug { + let _ = self; + (regular, generic, with_into, with_default, with_default_2) + } + } + + assert_debug_eq( + Sut::sut() + .regular(Some(1)) + .generic(Some(false)) + .with_into(2) + .maybe_with_default_2(Some(Some(3))) + .call(), + expect!["(Some(1), Some(false), Some(2), Some(99), Some(3))"], + ); + + assert_debug_eq( + Sut.with_self() + .regular(Some(1)) + .generic(Some(false)) + .with_into(2) + .maybe_with_default_2(Some(Some(3))) + .call(), + expect!["(Some(1), Some(false), Some(2), Some(99), Some(3))"], + ); + } +} + +mod attr_on { + use crate::prelude::*; + use core::fmt; + + #[test] + fn test_struct() { + #[derive(Debug, Builder)] + #[builder(on(_, transparent))] + #[allow(dead_code)] + struct Sut { + regular: Option, + generic: Option, + + #[builder(into)] + with_into: Option, + + #[builder(default = Some(99))] + with_default: Option, + + #[builder(default = Some(10))] + with_default_2: Option, + } + + assert_debug_eq( + Sut::builder() + .regular(Some(1)) + .generic(Some(false)) + .with_into(2) + .maybe_with_default_2(Some(Some(3))) + .build(), + expect![[r#" + Sut { + regular: Some( + 1, + ), + generic: Some( + false, + ), + with_into: Some( + 2, + ), + with_default: Some( + 99, + ), + with_default_2: Some( + 3, + ), + }"#]], + ); + } + + #[test] + fn test_free_fn() { + #[builder(on(_, transparent))] + fn sut( + regular: Option, + generic: Option, + #[builder(into)] with_into: Option, + #[builder(default = Some(99))] with_default: Option, + #[builder(default = Some(10))] with_default_2: Option, ) -> impl fmt::Debug { - let _ = self; (regular, generic, with_into, with_default, with_default_2) } + + assert_debug_eq( + sut() + .regular(Some(1)) + .generic(Some(false)) + .with_into(2) + .maybe_with_default_2(Some(Some(3))) + .call(), + expect!["(Some(1), Some(false), Some(2), Some(99), Some(3))"], + ); } - assert_debug_eq( - Sut::sut() - .regular(Some(1)) - .generic(Some(false)) - .with_into(2) - .maybe_with_default_2(Some(Some(3))) - .call(), - expect!["(Some(1), Some(false), Some(2), Some(99), Some(3))"], - ); - - assert_debug_eq( - Sut.with_self() - .regular(Some(1)) - .generic(Some(false)) - .with_into(2) - .maybe_with_default_2(Some(Some(3))) - .call(), - expect!["(Some(1), Some(false), Some(2), Some(99), Some(3))"], - ); + #[test] + fn test_assoc_method() { + struct Sut; + + #[bon] + impl Sut { + #[builder(on(_, transparent))] + fn sut( + regular: Option, + generic: Option, + #[builder(into)] with_into: Option, + #[builder(default = Some(99))] with_default: Option, + #[builder(default = Some(10))] with_default_2: Option, + ) -> impl fmt::Debug { + (regular, generic, with_into, with_default, with_default_2) + } + + #[builder(on(_, transparent))] + fn with_self( + &self, + regular: Option, + generic: Option, + #[builder(into)] with_into: Option, + #[builder(default = Some(99))] with_default: Option, + #[builder(default = Some(10))] with_default_2: Option, + ) -> impl fmt::Debug { + let _ = self; + (regular, generic, with_into, with_default, with_default_2) + } + } + + assert_debug_eq( + Sut::sut() + .regular(Some(1)) + .generic(Some(false)) + .with_into(2) + .maybe_with_default_2(Some(Some(3))) + .call(), + expect!["(Some(1), Some(false), Some(2), Some(99), Some(3))"], + ); + + assert_debug_eq( + Sut.with_self() + .regular(Some(1)) + .generic(Some(false)) + .with_into(2) + .maybe_with_default_2(Some(Some(3))) + .call(), + expect!["(Some(1), Some(false), Some(2), Some(99), Some(3))"], + ); + } } diff --git a/bon/tests/integration/ui/compile_fail/attr_on.rs b/bon/tests/integration/ui/compile_fail/attr_on.rs index 7fcafb2b..d66e6da3 100644 --- a/bon/tests/integration/ui/compile_fail/attr_on.rs +++ b/bon/tests/integration/ui/compile_fail/attr_on.rs @@ -24,4 +24,34 @@ fn incomplete_on3() {} #[builder(on(_,))] fn incomplete_on4() {} +#[builder( + on(_, transparent), + finish_fn = finish, + on(String, into), +)] +fn non_consecutive_on1() {} + +#[builder( + start_fn = start, + on(_, transparent), + finish_fn = finish, + on(String, into), +)] +fn non_consecutive_on2() {} + +#[builder( + start_fn = start, + on(_, transparent), + finish_fn = finish, + on(String, into), + builder_type = Builder, +)] +fn non_consecutive_on3() {} + +#[builder(on(_, into), on(_, transparent))] +fn non_first_transparent() {} + +#[builder(on(u8, transparent))] +fn non_wildcard_transparent() {} + fn main() {} diff --git a/bon/tests/integration/ui/compile_fail/attr_on.stderr b/bon/tests/integration/ui/compile_fail/attr_on.stderr index 500ecc73..cd3baa51 100644 --- a/bon/tests/integration/ui/compile_fail/attr_on.stderr +++ b/bon/tests/integration/ui/compile_fail/attr_on.stderr @@ -42,10 +42,40 @@ error: expected `,` | = note: this error originates in the attribute macro `builder` (in Nightly builds, run with -Z macro-backtrace for more info) -error: this #[builder(on(type_pattern, ...))] contains no options to override the default behavior for the selected setters like `into`, `overwritable`, so it does nothing +error: this #[builder(on(type_pattern, ...))] contains no options to override the default behavior for the selected setters like `into`, `overwritable`, `transparent`, so it does nothing --> tests/integration/ui/compile_fail/attr_on.rs:24:1 | 24 | #[builder(on(_,))] | ^^^^^^^^^^^^^^^^^^ | = note: this error originates in the attribute macro `builder` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: this `on(...)` clause is out of order; all `on(...)` clauses must be consecutive; there shouldn't be any other parameters between them + --> tests/integration/ui/compile_fail/attr_on.rs:30:5 + | +30 | on(String, into), + | ^^ + +error: this `on(...)` clause is out of order; all `on(...)` clauses must be consecutive; there shouldn't be any other parameters between them + --> tests/integration/ui/compile_fail/attr_on.rs:38:5 + | +38 | on(String, into), + | ^^ + +error: this `on(...)` clause is out of order; all `on(...)` clauses must be consecutive; there shouldn't be any other parameters between them + --> tests/integration/ui/compile_fail/attr_on.rs:46:5 + | +46 | on(String, into), + | ^^ + +error: `transparent` can only be specified in the first `on(...)` clause; this restriction may be lifted in the future + --> tests/integration/ui/compile_fail/attr_on.rs:51:30 + | +51 | #[builder(on(_, into), on(_, transparent))] + | ^^^^^^^^^^^ + +error: `transparent` can only be used with the wildcard type pattern i.e. `on(_, transparent)`; this restriction may be lifted in the future + --> tests/integration/ui/compile_fail/attr_on.rs:54:14 + | +54 | #[builder(on(u8, transparent))] + | ^^