Veetaha's amendments for #[builder(getter)]

.githooks/pre-commit

@@ -21,4 +21,7 @@ taplo fmt
 # Add back the modified/prettified files to staging
 echo "$FILES" | xargs git add
 
+# Check for typos
+typos
+
 exit 0

.github/workflows/ci.yml

@@ -108,10 +108,21 @@ jobs:
 
       - run: cargo test --locked --all-features --all-targets
       - run: cargo test --locked --all-features --doc
-      - run: cd bon && cargo test --locked --no-default-features --features=experimental-overwritable
-      - run: cd bon && cargo test --locked --no-default-features --features=experimental-overwritable,alloc
-      - run: cd bon && cargo test --locked --no-default-features --features=experimental-overwritable,implied-bounds
-      - run: cd bon && cargo test --locked --no-default-features --features=experimental-overwritable,alloc,implied-bounds
+      - run: |
+          cd bon && cargo test --locked --no-default-features \
+            --features=experimental-overwritable,experimental-getter
+
+      - run: |
+          cd bon && cargo test --locked --no-default-features \
+            --features=experimental-overwritable,experimental-getter,alloc
+
+      - run: |
+          cd bon && cargo test --locked --no-default-features \
+            --features=experimental-overwritable,experimental-getter,implied-bounds
+
+      - run: |
+          cd bon && cargo test --locked --no-default-features \
+            --features=experimental-overwritable,experimental-getter,alloc,implied-bounds
 
   test-msrv:
     runs-on: ${{ matrix.os }}-latest
@@ -226,7 +237,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
-      - uses: crate-ci/typos@v1.22.3
+      - uses: crate-ci/typos@v1.28.1
 
   website-build:
     runs-on: ubuntu-latest

bon-macros/src/builder/builder_gen/getters.rs

@@ -0,0 +1,101 @@
+use super::{BuilderGenCtx, NamedMember};
+use crate::util::prelude::*;
+
+pub(crate) struct GettersCtx<'a> {
+    base: &'a BuilderGenCtx,
+    member: &'a NamedMember,
+}
+
+struct GetterItem {
+    name: syn::Ident,
+    vis: syn::Visibility,
+    docs: Vec<syn::Attribute>,
+}
+
+impl<'a> GettersCtx<'a> {
+    pub(crate) fn new(base: &'a BuilderGenCtx, member: &'a NamedMember) -> Self {
+        Self { base, member }
+    }
+
+    pub(crate) fn getter_methods(&self) -> TokenStream {
+        let GetterItem { name, vis, docs } = match GetterItem::new(self) {
+            Some(item) => item,
+            None => return quote! {},
+        };
+
+        let index = &self.member.index;
+        let ty = self.member.underlying_norm_ty();
+
+        let (return_type, body) = if self.member.is_required() {
+            (
+                quote! { &#ty },
+                quote! {
+                    unsafe {
+                        // SAFETY: this code is runs in a method that has a where
+                        // bound that ensures the member was set.
+                        ::core::option::Option::unwrap_unchecked(
+                            self.__unsafe_private_named.#index.as_ref()
+                        )
+                    }
+                },
+            )
+        } else {
+            (
+                // We are not using the fully qualified path to `Option` here
+                // to make function signature in IDE popus shorter and more
+                // readable.
+                quote! { Option<&#ty> },
+                quote! { self.__unsafe_private_named.#index.as_ref() },
+            )
+        };
+
+        let state_var = &self.base.state_var;
+        let member_pascal = &self.member.name.pascal;
+        let state_mod = &self.base.state_mod.ident;
+
+        quote! {
+            #( #docs )*
+            #[allow(
+                // This is intentional. We want the builder syntax to compile away
+                clippy::inline_always,
+                clippy::missing_const_for_fn,
+            )]
+            #[inline(always)]
+            #[must_use = "this method has no side effects; it only returns a value"]
+            #vis fn #name(&self) -> #return_type
+            where
+                #state_var::#member_pascal: #state_mod::IsSet,
+            {
+                #body
+            }
+        }
+    }
+}
+
+impl GetterItem {
+    fn new(ctx: &GettersCtx<'_>) -> Option<Self> {
+        let GettersCtx { member, base } = ctx;
+
+        let config = member.config.getter.as_ref()?;
+
+        Some(Self {
+            name: config.name().cloned().unwrap_or_else(|| {
+                syn::Ident::new(
+                    &format!("get_{}", member.name.snake.raw_name()),
+                    member.name.snake.span(),
+                )
+            }),
+            vis: config.vis().unwrap_or(&base.builder_type.vis).clone(),
+            docs: config.docs().map(<[_]>::to_vec).unwrap_or_else(|| {
+                let header = format!(
+                    "_**Getter.**_ Returns `{}`, which must be set before calling this method.\n\n",
+                    member.name.snake,
+                );
+
+                std::iter::once(syn::parse_quote!(#[doc = #header]))
+                    .chain(member.docs.iter().cloned())
+                    .collect()
+            }),
+        })
+    }
+}

bon-macros/src/builder/builder_gen/member/config/getter.rs

@@ -1,12 +1,11 @@
+use crate::parsing::SpannedKey;
+use crate::util::prelude::*;
 use darling::FromMeta;
 
-use super::{Result, SpannedKey};
-
 #[derive(Debug, Default)]
 pub(crate) struct GetterConfig {
     name: Option<SpannedKey<syn::Ident>>,
     vis: Option<SpannedKey<syn::Visibility>>,
-
     docs: Option<SpannedKey<Vec<syn::Attribute>>>,
 }
 
@@ -25,7 +24,7 @@
             name: Option<SpannedKey<syn::Ident>>,
             vis: Option<SpannedKey<syn::Visibility>>,
 
             #[darling(rename = "doc", default, with = parse_docs, map = Some)]
             docs: Option<SpannedKey<Vec<syn::Attribute>>>,
         }
 

bon-macros/src/builder/builder_gen/member/config/mod.rs

@@ -35,10 +35,12 @@ pub(crate) struct MemberConfig {
     #[darling(with = parse_optional_expr, map = Some)]
     pub(crate) field: Option<SpannedKey<Option<syn::Expr>>>,
 
-    /// Make the member gettable by reference.
+    /// Make the member gettable. [`GetterConfig`] specifies the signature for
+    /// the getter.
     ///
     /// This takes the same attributes as the setter fns; `name`, `vis`, and `doc`
-    /// and produces a getter method that returns `&T` for the member.
+    /// and produces a getter method that returns the value of the member.
+    /// By default, the value is returned by a shared reference (&T).
     pub(crate) getter: Option<SpannedKey<GetterConfig>>,
 
     /// Accept the value for the member in the finishing function parameters.
@@ -220,46 +222,40 @@ impl MemberConfig {
             );
         }
 
+        if let Some(getter) = &self.getter {
+            if !cfg!(feature = "experimental-getter") {
+                bail!(
+                    &getter.key,
+                    "🔬 `getter` attribute is experimental and requires \
+                    \"experimental-getter\" cargo feature to be enabled; \
+                    if you find the current design of this attribute already \
+                    solid please leave a 👍 reaction under the issue \
+                    https://github.com/elastio/bon/issues/225; if you have \
+                    any feedback, then feel free to leave a comment under that issue",
+                );
+            }
+
+            self.validate_mutually_exclusive(
+                ParamName::Getter,
+                getter.key.span(),
+                &[ParamName::Overwritable],
+            )?;
+        }
+
         if self.start_fn.is_present() {
             self.validate_mutually_allowed(
                 ParamName::StartFn,
                 self.start_fn.span(),
-                &[ParamName::Into, ParamName::Getter],
+                // TODO: add support for `#[builder(getter)]` with `start_fn`
+                &[ParamName::Into],
             )?;
         }
 
         if self.finish_fn.is_present() {
             self.validate_mutually_allowed(
                 ParamName::FinishFn,
                 self.finish_fn.span(),
-                &[ParamName::Into, ParamName::Getter],
-            )?;
-        }
-
-        if let Some(getter) = &self.getter {
-            if !cfg!(feature = "experimental-getter") {
-                bail!(
-                    &getter.key.span(),
-                    "`getter` attribute is experimental and requires \
-                    \"experimental-getter\" cargo feature to be enabled; \
-                    we would be glad to make this attribute stable if you find it useful; \
-                    please leave a 👍 reaction under the issue https://github.com/elastio/bon/issues/221 \
-                    to help us measure the impact on this feature. If you have \
-                    a use case for this attribute, then open an issue/discussion on \
-                    https://github.com/elastio/bon/issues.",
-                );
-            }
-
-            self.validate_mutually_allowed(
-                ParamName::Getter,
-                getter.key.span(),
-                &[
-                    ParamName::With,
-                    ParamName::Into,
-                    ParamName::Name,
-                    ParamName::Setters,
-                    ParamName::Required,
-                ],
+                &[ParamName::Into],
             )?;
         }
 

bon-macros/src/builder/builder_gen/mod.rs

@@ -1,7 +1,7 @@
 mod builder_decl;
 mod builder_derives;
 mod finish_fn;
-mod getter;
+mod getters;
 mod member;
 mod models;
 mod setters;
@@ -11,11 +11,10 @@ mod top_level_config;
 
 pub(crate) mod input_fn;
 pub(crate) mod input_struct;
-
-use getter::GetterCtx;
 pub(crate) use top_level_config::TopLevelConfig;
 
 use crate::util::prelude::*;
+use getters::GettersCtx;
 use member::{CustomField, Member, MemberOrigin, NamedMember, RawMember, StartFnMember};
 use models::{AssocMethodCtx, AssocMethodReceiverCtx, BuilderGenCtx, FinishFnBody, Generics};
 use setters::SettersCtx;
@@ -104,14 +103,20 @@ impl BuilderGenCtx {
 
     fn builder_impl(&self) -> Result<TokenStream> {
         let finish_fn = self.finish_fn();
-        let setter_methods = self
-            .named_members()
-            .map(|member| SettersCtx::new(self, member).setter_methods())
-            .collect::<Result<Vec<_>>>()?;
-
-        let getter_methods = self
+        let accessor_methods = self
             .named_members()
-            .map(|member| GetterCtx::new(self, member).getter_method());
+            .map(|member| {
+                let setters = SettersCtx::new(self, member).setter_methods()?;
+                let getters = GettersCtx::new(self, member).getter_methods();
+
+                // Output all accessor methods for the same member adjecently.
+                // This is important in the generated rustdoc output, because
+                // rustdoc lists methods in the order they appear in the source.
+                Ok([setters, getters])
+            })
+            .collect::<Result<Vec<_>>>()?
+            .into_iter()
+            .flatten();
 
         let generics_decl = &self.generics.decl_without_defaults;
         let generic_args = &self.generics.args;
@@ -133,8 +138,7 @@ impl BuilderGenCtx {
             #where_clause
             {
                 #finish_fn
-                #(#setter_methods)*
-                #(#getter_methods)*
+                #(#accessor_methods)*
             }
         })
     }