diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 5e7d1b7a..893c67a6 100755 --- a/.githooks/pre-commit +++ b/.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 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c40ef17b..575ca7b4 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/bon-macros/src/builder/builder_gen/getter.rs b/bon-macros/src/builder/builder_gen/getter.rs deleted file mode 100644 index 291a9249..00000000 --- a/bon-macros/src/builder/builder_gen/getter.rs +++ /dev/null @@ -1,92 +0,0 @@ -use proc_macro2::TokenStream; -use quote::quote; - -use super::{BuilderGenCtx, IdentExt, NamedMember}; - -pub(crate) struct GetterCtx<'a> { - base: &'a BuilderGenCtx, - member: &'a NamedMember, -} - -struct GetterItem { - name: syn::Ident, - vis: syn::Visibility, - docs: Vec, -} - -impl<'a> GetterCtx<'a> { - pub(crate) fn new(base: &'a BuilderGenCtx, member: &'a NamedMember) -> Self { - Self { base, member } - } - - pub(crate) fn getter_method(&self) -> TokenStream { - let Some(GetterItem { name, vis, docs }) = GetterItem::new(self) else { - 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 { ::std::option::Option::unwrap_unchecked(self.__unsafe_private_named.#index.as_ref()) } }, - ) - } else { - ( - quote! { ::core::option::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)] - #vis fn #name(&self) -> #return_type - where #state_var::#member_pascal: #state_mod::IsSet, - { - #body - } - } - } -} - -impl GetterItem { - fn new(ctx: &GetterCtx<'_>) -> Option { - let GetterCtx { member, base } = ctx; - - let spanned_keyed_config = member.config.getter.as_ref()?; - - let common_name = spanned_keyed_config.name(); - let common_vis = spanned_keyed_config.vis(); - let common_docs = spanned_keyed_config.docs(); - - Some(Self { - name: common_name.cloned().unwrap_or_else(|| { - syn::Ident::new( - &format!("get_{}", member.name.snake.raw_name()), - member.name.snake.span(), - ) - }), - vis: common_vis.unwrap_or(&base.builder_type.vis).clone(), - docs: common_docs - .map(<[syn::Attribute]>::to_vec) - .unwrap_or_else(|| { - const HEADER: &str = "_**Getter.**_\n\n"; - - std::iter::once(syn::parse_quote!(#[doc = #HEADER])) - .chain(member.docs.iter().cloned()) - .collect() - }), - }) - } -} diff --git a/bon-macros/src/builder/builder_gen/getters.rs b/bon-macros/src/builder/builder_gen/getters.rs new file mode 100644 index 00000000..b5156f72 --- /dev/null +++ b/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, +} + +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 { + 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() + }), + }) + } +} diff --git a/bon-macros/src/builder/builder_gen/member/config/getter.rs b/bon-macros/src/builder/builder_gen/member/config/getter.rs index 1e95ea56..ddbb1d75 100644 --- a/bon-macros/src/builder/builder_gen/member/config/getter.rs +++ b/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>, vis: Option>, - docs: Option>>, } diff --git a/bon-macros/src/builder/builder_gen/member/config/mod.rs b/bon-macros/src/builder/builder_gen/member/config/mod.rs index a6e5606e..38f31634 100644 --- a/bon-macros/src/builder/builder_gen/member/config/mod.rs +++ b/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>>, - /// 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>, /// Accept the value for the member in the finishing function parameters. @@ -220,11 +222,32 @@ 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], )?; } @@ -232,34 +255,7 @@ impl MemberConfig { 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], )?; } diff --git a/bon-macros/src/builder/builder_gen/mod.rs b/bon-macros/src/builder/builder_gen/mod.rs index 325fbd38..f30edb4b 100644 --- a/bon-macros/src/builder/builder_gen/mod.rs +++ b/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 { let finish_fn = self.finish_fn(); - let setter_methods = self - .named_members() - .map(|member| SettersCtx::new(self, member).setter_methods()) - .collect::>>()?; - - 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::>>()? + .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)* } }) } diff --git a/bon-sandbox/Cargo.toml b/bon-sandbox/Cargo.toml index 8c5e56ca..f76a0d31 100644 --- a/bon-sandbox/Cargo.toml +++ b/bon-sandbox/Cargo.toml @@ -29,7 +29,11 @@ targets = ["x86_64-unknown-linux-gnu"] workspace = true [dependencies] -bon = { path = "../bon", version = "=3.1.1", features = ["experimental-overwritable", "experimental-getter"] } buildstructor = "0.5" derive_builder = "0.20" typed-builder = "0.20" + +[dependencies.bon] +features = ["experimental-overwritable", "experimental-getter"] +path = "../bon" +version = "=3.1.1" diff --git a/bon-sandbox/src/getter.rs b/bon-sandbox/src/attr_getter.rs similarity index 80% rename from bon-sandbox/src/getter.rs rename to bon-sandbox/src/attr_getter.rs index 287fd91e..5ac89dcf 100644 --- a/bon-sandbox/src/getter.rs +++ b/bon-sandbox/src/attr_getter.rs @@ -8,10 +8,12 @@ pub fn full_name_fn(#[builder(getter)] first_name: &str, last_name: &str) -> Str #[derive(Builder)] pub struct FullName { #[builder(getter)] - pub first_name: String, + first_name: String, + #[builder(getter(name = get_the_last_name, vis = "pub(crate)", doc { /// Docs on the getter }))] - pub last_name: String, - pub no_getter: String, + last_name: String, + + no_getter: String, } diff --git a/bon-sandbox/src/lib.rs b/bon-sandbox/src/lib.rs index 49d95423..55455429 100644 --- a/bon-sandbox/src/lib.rs +++ b/bon-sandbox/src/lib.rs @@ -6,10 +6,10 @@ #![allow(missing_debug_implementations, missing_docs, dead_code)] pub mod attr_default; +pub mod attr_getter; pub mod attr_with; pub mod docs_comparison; pub mod functions; -pub mod getter; pub mod macro_rules_wrapper_test; pub mod missing_docs_test; pub mod overrides; diff --git a/bon/Cargo.toml b/bon/Cargo.toml index fca6039e..efa6fdf8 100644 --- a/bon/Cargo.toml +++ b/bon/Cargo.toml @@ -91,8 +91,7 @@ experimental-overwritable = ["bon-macros/experimental-overwritable"] # # See more info at https://bon-rs.com/reference/builder/member/getter. # -# We are considering stabilizing this attribute if you have a use for it. Please leave -# a 👍 reaction under the issue https://github.com/elastio/bon/issues/221 if you need -# this attribute. It would also be cool if you could leave a comment under that issue -# describing your use case for it. +# The fate of this feature depends on your feedback in the tracking issue +# https://github.com/elastio/bon/issues/225. Please, let us know if you +# have any ideas how to make this attribute better, or if it's already good enough! experimental-getter = ["bon-macros/experimental-getter"] diff --git a/bon/tests/integration/builder/attr_getter.rs b/bon/tests/integration/builder/attr_getter.rs index 90c8d995..17a90b1d 100644 --- a/bon/tests/integration/builder/attr_getter.rs +++ b/bon/tests/integration/builder/attr_getter.rs @@ -10,7 +10,7 @@ fn test_struct() { x1: u32, #[builder(getter(name = x2_with_custom_name))] - x2: String, + x2: &'static str, #[builder(getter(vis = "pub(crate)", doc { /// Docs on the getter @@ -20,7 +20,7 @@ fn test_struct() { #[builder(into, getter(name = x5, vis = "pub(crate)", doc { /// The name is a lie }))] - x4_but_its_actually_5: String, + x4_but_its_actually_5: &'static str, not_a_getter: u32, @@ -28,21 +28,24 @@ fn test_struct() { generic_option_getter: Option, x6: (), + + #[builder(getter, default)] + x7: u32, } #[allow(clippy::redundant_clone)] let sut = Sut::<()>::builder(0u32).clone(); - let actual = sut.x2("2".to_owned()).x3(3); + let actual = sut.x2("2").x3(3); - let actual = actual.x4_but_its_actually_5("4".to_owned()); + let actual = actual.x4_but_its_actually_5("4"); let x5 = actual.x5(); - assert_eq!(x5, "4"); + assert_eq!(*x5, "4"); let actual = actual.not_a_getter(5).x6(()); let x2 = actual.x2_with_custom_name(); - assert_eq!(x2, "2"); + assert_eq!(*x2, "2"); let x3 = actual.get_x3(); assert_eq!(x3, &3); @@ -52,6 +55,9 @@ fn test_struct() { let gen_opt_get = actual.get_generic_option_getter(); assert_eq!(gen_opt_get, None); + let actual = actual.x7(7); + assert_eq!(actual.get_x7(), Some(&7)); + assert_debug_eq( &actual, expect![[r#" @@ -62,6 +68,7 @@ fn test_struct() { x4_but_its_actually_5: "4", not_a_getter: 5, x6: (), + x7: 7, }"#]], ); } diff --git a/bon/tests/integration/ui/compile_fail/attr_getter.rs b/bon/tests/integration/ui/compile_fail/attr_getter.rs new file mode 100644 index 00000000..aae0d8be --- /dev/null +++ b/bon/tests/integration/ui/compile_fail/attr_getter.rs @@ -0,0 +1,45 @@ +use bon::Builder; + +#[derive(Builder)] +struct StartFnCompat { + #[builder(getter, start_fn)] + x: u32, +} + +#[derive(Builder)] +struct FinishFnCompat { + #[builder(getter, finish_fn)] + x: u32, +} + +#[derive(Builder)] +struct SkipCompat { + #[builder(getter, skip)] + x: u32, +} + +#[derive(Builder)] +struct OverwritableCompat { + #[builder(getter, overwritable)] + x: u32, +} + +#[derive(Builder)] +struct NegativeTest { + #[builder(getter)] + x1: u32, + + #[builder(getter, default)] + x2: u32, + + #[builder(getter)] + x3: Option +} + +fn main() { + let builder = NegativeTest::builder(); + + builder.get_x1(); + builder.get_x2(); + builder.get_x3(); +} diff --git a/bon/tests/integration/ui/compile_fail/attr_getter.stderr b/bon/tests/integration/ui/compile_fail/attr_getter.stderr new file mode 100644 index 00000000..094c36e6 --- /dev/null +++ b/bon/tests/integration/ui/compile_fail/attr_getter.stderr @@ -0,0 +1,77 @@ +error: `start_fn` attribute can't be specified together with `getter` + --> tests/integration/ui/compile_fail/attr_getter.rs:5:23 + | +5 | #[builder(getter, start_fn)] + | ^^^^^^^^ + +error: `finish_fn` attribute can't be specified together with `getter` + --> tests/integration/ui/compile_fail/attr_getter.rs:11:23 + | +11 | #[builder(getter, finish_fn)] + | ^^^^^^^^^ + +error: `skip` attribute can't be specified together with `getter` + --> tests/integration/ui/compile_fail/attr_getter.rs:17:23 + | +17 | #[builder(getter, skip)] + | ^^^^ + +error: `getter` attribute can't be specified together with `overwritable` + --> tests/integration/ui/compile_fail/attr_getter.rs:23:15 + | +23 | #[builder(getter, overwritable)] + | ^^^^^^ + +error[E0277]: the member `Unset` was not set, but this method requires it to be set + --> tests/integration/ui/compile_fail/attr_getter.rs:42:13 + | +42 | builder.get_x1(); + | ^^^^^^ the member `Unset` was not set, but this method requires it to be set + | + = help: the trait `IsSet` is not implemented for `Unset` + = help: the trait `IsSet` is implemented for `Set` +note: required by a bound in `NegativeTestBuilder::::get_x1` + --> tests/integration/ui/compile_fail/attr_getter.rs:27:10 + | +27 | #[derive(Builder)] + | ^^^^^^^ required by this bound in `NegativeTestBuilder::::get_x1` +... +30 | x1: u32, + | -- 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 `Unset` was not set, but this method requires it to be set + --> tests/integration/ui/compile_fail/attr_getter.rs:43:13 + | +43 | builder.get_x2(); + | ^^^^^^ the member `Unset` was not set, but this method requires it to be set + | + = help: the trait `IsSet` is not implemented for `Unset` + = help: the trait `IsSet` is implemented for `Set` +note: required by a bound in `NegativeTestBuilder::::get_x2` + --> tests/integration/ui/compile_fail/attr_getter.rs:27:10 + | +27 | #[derive(Builder)] + | ^^^^^^^ required by this bound in `NegativeTestBuilder::::get_x2` +... +33 | x2: u32, + | -- 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 `Unset` was not set, but this method requires it to be set + --> tests/integration/ui/compile_fail/attr_getter.rs:44:13 + | +44 | builder.get_x3(); + | ^^^^^^ the member `Unset` was not set, but this method requires it to be set + | + = help: the trait `IsSet` is not implemented for `Unset` + = help: the trait `IsSet` is implemented for `Set` +note: required by a bound in `NegativeTestBuilder::::get_x3` + --> tests/integration/ui/compile_fail/attr_getter.rs:27:10 + | +27 | #[derive(Builder)] + | ^^^^^^^ required by this bound in `NegativeTestBuilder::::get_x3` +... +36 | x3: Option + | -- 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/attr_on.rs b/bon/tests/integration/ui/compile_fail/attr_on.rs index 7e8dbbdd..c91c1931 100644 --- a/bon/tests/integration/ui/compile_fail/attr_on.rs +++ b/bon/tests/integration/ui/compile_fail/attr_on.rs @@ -4,7 +4,7 @@ use bon::builder; fn unnecessary_into(#[builder(into)] _x: String) {} #[builder(on(String, overwritable))] -fn unnecessary_ovewritable(#[builder(overwritable)] _x: String) {} +fn unnecessary_overwritable(#[builder(overwritable)] _x: String) {} #[builder(on(&dyn std::fmt::Debug, into))] fn invalid_type_pattern() {} diff --git a/bon/tests/integration/ui/compile_fail/attr_on.stderr b/bon/tests/integration/ui/compile_fail/attr_on.stderr index 3ad2768a..6b8425d2 100644 --- a/bon/tests/integration/ui/compile_fail/attr_on.stderr +++ b/bon/tests/integration/ui/compile_fail/attr_on.stderr @@ -5,10 +5,10 @@ error: this `#[builder(into)]` attribute is redundant, because `into` is already | ^^^^ error: this `#[builder(overwritable)]` attribute is redundant, because `overwritable` is already implied for this member via the `#[builder(on(...))]` at the top of the function - --> tests/integration/ui/compile_fail/attr_on.rs:7:38 + --> tests/integration/ui/compile_fail/attr_on.rs:7:39 | -7 | fn unnecessary_ovewritable(#[builder(overwritable)] _x: String) {} - | ^^^^^^^^^^^^ +7 | fn unnecessary_overwritable(#[builder(overwritable)] _x: String) {} + | ^^^^^^^^^^^^ error: this syntax is not supported in type patterns yet. If you have a use case for this, please open an issue at https://github.com/elastio/bon/issues. --> tests/integration/ui/compile_fail/attr_on.rs:9:15 diff --git a/bon/tests/integration/ui/compile_fail/warnings.rs b/bon/tests/integration/ui/compile_fail/warnings.rs index a939893e..8a5304a8 100644 --- a/bon/tests/integration/ui/compile_fail/warnings.rs +++ b/bon/tests/integration/ui/compile_fail/warnings.rs @@ -2,7 +2,7 @@ use bon::{bon, builder, Builder}; fn main() { - // Test #[must_use] + // Test #[must_use] for setters { #[allow(dead_code)] #[derive(Builder)] @@ -54,4 +54,26 @@ fn main() { must_use_compiled_out().call(); } + + // Test #[must_use] for getters + { + #[derive(Builder)] + struct Sut { + #[builder(getter)] + x1: u32, + + #[builder(getter, default)] + x2: u32, + + #[builder(getter)] + x3: Option, + } + + let builder = Sut::builder().x1(1).x2(2).x3(3); + + // Make sure there are `#[must_use]` warnings + builder.get_x1(); + builder.get_x2(); + builder.get_x3(); + } } diff --git a/bon/tests/integration/ui/compile_fail/warnings.stderr b/bon/tests/integration/ui/compile_fail/warnings.stderr index ea2d0c22..f4e0e847 100644 --- a/bon/tests/integration/ui/compile_fail/warnings.stderr +++ b/bon/tests/integration/ui/compile_fail/warnings.stderr @@ -108,3 +108,39 @@ help: use `let _ = ...` to ignore the resulting value | 47 | let _ = must_use_under_cfg().call(); | +++++++ + +error: unused return value of `SutBuilder::::get_x1` that must be used + --> tests/integration/ui/compile_fail/warnings.rs:75:9 + | +75 | builder.get_x1(); + | ^^^^^^^^^^^^^^^^ + | + = note: this method has no side effects; it only returns a value +help: use `let _ = ...` to ignore the resulting value + | +75 | let _ = builder.get_x1(); + | +++++++ + +error: unused return value of `SutBuilder::::get_x2` that must be used + --> tests/integration/ui/compile_fail/warnings.rs:76:9 + | +76 | builder.get_x2(); + | ^^^^^^^^^^^^^^^^ + | + = note: this method has no side effects; it only returns a value +help: use `let _ = ...` to ignore the resulting value + | +76 | let _ = builder.get_x2(); + | +++++++ + +error: unused return value of `SutBuilder::::get_x3` that must be used + --> tests/integration/ui/compile_fail/warnings.rs:77:9 + | +77 | builder.get_x3(); + | ^^^^^^^^^^^^^^^^ + | + = note: this method has no side effects; it only returns a value +help: use `let _ = ...` to ignore the resulting value + | +77 | let _ = builder.get_x3(); + | +++++++ diff --git a/rust-toolchain.toml b/rust-toolchain.toml index 66788910..0df4f14f 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,3 +1,3 @@ [toolchain] -channel = "1.82" +channel = "1.83" components = ["cargo"] diff --git a/scripts/init.sh b/scripts/init.sh index bd9c3f9b..21cc7c1a 100755 --- a/scripts/init.sh +++ b/scripts/init.sh @@ -3,11 +3,10 @@ set -euxo pipefail -# Install prettier +# Installs prettier (see top-level package.json) npm ci -# Install taplo -cargo install taplo-cli --locked +cargo install typos-cli taplo-cli --locked # Install the pre-commit hook -ln -s ../../.githooks/pre-commit .git/hooks/pre-commit +ln -sf ../../.githooks/pre-commit .git/hooks/pre-commit diff --git a/scripts/test-msrv.sh b/scripts/test-msrv.sh index 8f2f1950..bddef9c4 100755 --- a/scripts/test-msrv.sh +++ b/scripts/test-msrv.sh @@ -40,7 +40,9 @@ step cargo update -p libc --precise 0.2.163 export RUSTFLAGS="${RUSTFLAGS:-} --allow unknown-lints" -step cargo clippy --all-targets --locked --features=experimental-overwritable +features=experimental-overwritable,experimental-getter + +step cargo clippy --all-targets --locked --features "$features" test_args=( --locked @@ -58,4 +60,4 @@ test_args=( --skip ui::ui ) -step cargo test --features=experimental-overwritable "${test_args[@]}" +step cargo test --features "$features" "${test_args[@]}" diff --git a/website/.vitepress/config.mts b/website/.vitepress/config.mts index 3a6f3ae1..4fee22f4 100644 --- a/website/.vitepress/config.mts +++ b/website/.vitepress/config.mts @@ -189,6 +189,10 @@ export default defineConfig({ text: "Custom Fields", link: "/guide/typestate-api/custom-fields", }, + { + text: "Getters 🔬", + link: "/guide/typestate-api/getters", + }, ], }, { @@ -304,8 +308,8 @@ export default defineConfig({ link: "/reference/builder/member/finish_fn", }, { - text: "getters 🔬", - link: "/reference/builder/member/getters", + text: "getter 🔬", + link: "/reference/builder/member/getter", }, { text: "into", diff --git a/website/src/guide/contributing.md b/website/src/guide/contributing.md index ba31c906..f818c2e3 100644 --- a/website/src/guide/contributing.md +++ b/website/src/guide/contributing.md @@ -14,7 +14,7 @@ However, even though desirable, creating an issue before making a pull request i This repository is a regular [`cargo` workspace](https://doc.rust-lang.org/book/ch14-03-cargo-workspaces.html). Just fork it and do the usual `cargo` business. -Additionally, an init script is provided: `./scripts/init.sh`. This will install required dependencies and set up the commit hooks for our CI. +Additionally, an init script is provided: `./scripts/init.sh`. This will install required dependencies and set up the git pre-commit hook that will perform code formatting and some lightweight linting that would otherwise be performed on CI anyway. ## Testing diff --git a/website/src/guide/typestate-api/getters.md b/website/src/guide/typestate-api/getters.md new file mode 100644 index 00000000..5fb4da8b --- /dev/null +++ b/website/src/guide/typestate-api/getters.md @@ -0,0 +1,31 @@ +# Getters :microscope: + +You can generate a getter method for the member with the experimental attribute [`#[builder(getter)]`](../../reference/builder/member/getter) 🔬. The generated getter is available only when the value for the member was set (i.e. its type state implements the [`IsSet`](./custom-methods#isset-trait) trait). + +You can define a custom getter method on the builder, that adds some custom logic on top of just getting a value. To do that, generate an "internal" getter with private visibility under a different name and define your own getter that uses it in an impl block. + +```rust +use bon::Builder; + +#[derive(Builder)] +struct Example { + #[builder(getter(name = get_x_internal, vis = ""))] + x: u32 +} + +use example_builder::{IsSet, State}; + +impl ExampleBuilder { + // Getter method that performs additional computations + fn get_x(&self) -> u32 + where + S::X: IsSet + { + *self.get_x_internal() * 2 + } +} + +let builder = Example::builder().x(3); + +assert_eq!(builder.get_x(), 6); +``` diff --git a/website/src/reference/builder.md b/website/src/reference/builder.md index 742e8712..1a9f7c07 100644 --- a/website/src/reference/builder.md +++ b/website/src/reference/builder.md @@ -25,7 +25,7 @@ These attributes are placed on a `struct` field or `fn` argument. | [`default`](./builder/member/default) | Makes the member optional with a default value | | [`field`](./builder/member/field) | Defines a private field on the builder without setters | | [`finish_fn`](./builder/member/finish_fn) | Makes the member a positional argument on the finishing function | -| [`getter`](./builder/member/getter) | Makes the member have getter method for `&T` | +| [`getter`](./builder/member/getter) | Generates a getter method for a member | | [`into`](./builder/member/into) | Changes the signature of the setters to accept `impl Into` | | [`name`](./builder/member/name) | Overrides the name of the member used in the builder's API | | [`overwritable` 🔬](./builder/member/overwritable) | Allows calling setters for the same member repeatedly | diff --git a/website/src/reference/builder/member/getter.md b/website/src/reference/builder/member/getter.md index 57081ef3..0960b004 100644 --- a/website/src/reference/builder/member/getter.md +++ b/website/src/reference/builder/member/getter.md @@ -2,20 +2,24 @@ **Applies to:** -Allows getting a reference to an already set member. +Generates a getter method for a member. The method is callable only after the value for the member is set using any of its setters. ::: danger 🔬 **Experimental** This attribute is available under the cargo feature `experimental-getter`. Breaking changes may occur between **minor** releases but not between patch releases. -The fate of this feature depends on your feedback in the tracking issue [#149](https://github.com/elastio/bon/issues/221). Please, let us know if you have a use case for this attribute! +The fate of this feature depends on your feedback in the tracking issue [#225](https://github.com/elastio/bon/issues/225). Please, let us know if you have any ideas on improving this attribute, or if it's already good enough! ::: -This attribute now makes the following possible: +The getter for a required member returns `&T`. -```rust -#[derive(bon::Builder)] +::: code-group + +```rust [Struct] +use bon::Builder; + +#[derive(Builder)] struct Example { #[builder(getter)] // [!code highlight] x: u32, @@ -23,42 +27,101 @@ struct Example { let builder = Example::builder().x(1); -let x_ref = builder.get_x(); // [!code highlight] +let x: &u32 = builder.get_x(); // [!code highlight] -builder.build(); +assert_eq!(*x, 1); ``` -## Config +```rust [Function] +use bon::builder; + +#[builder] +fn example( + #[builder(getter)] // [!code highlight] + x: u32, +) {} + +let builder = example().x(1); + +let x: &u32 = builder.get_x(); // [!code highlight] -`getter` is configured quite similarly to [`setters`](./setters). +assert_eq!(*x, 1); +``` + +```rust [Method] +use bon::bon; + +struct Example; + +#[bon] +impl Example { + #[builder] + fn example( + #[builder(getter)] // [!code highlight] + x: u32, + ) {} +} -You can specify any combination of a `name`, `vis`, and `doc` for the getter, or you can omit all of them and use it as a flag to inherit default values for your getters. +let builder = Example::example().x(1); -### Example +let x: &u32 = builder.get_x(); // [!code highlight] + +assert_eq!(*x, 1); +``` + +::: + +The getter for an [optional member](../../../guide/basics/optional-members) returns `Option<&T>`. ```rust -#[derive(bon::Builder)] +use bon::Builder; + +#[derive(Builder)] struct Example { - #[builder(getter(name = get_my_member, vis = "pub(crate)", doc { // [!code highlight] - /// `get_my_member` gets a ref to the member // [!code highlight] - }))] // [!code highlight] - member: u32, // [!code highlight] + #[builder(getter)] // [!code highlight] + x1: Option, + + // Getters for `default` members are the same // [!code highlight] + // as for members of type `Option` // [!code highlight] + #[builder(getter, default = 99)] // [!code highlight] + x2: u32, + } -// Name of `some_fn` that accepts the non-None value was overridden -let builder = Example::builder().member(2); +let builder = Example::builder().x1(1).x2(2); -let my_member = builder.get_my_member(); // [!code highlight] +let x1: Option<&u32> = builder.get_x1(); // [!code highlight] +let x2: Option<&u32> = builder.get_x2(); // [!code highlight] + +assert_eq!(x1, Some(&1)); +assert_eq!(x2, Some(&2)); ``` -## `name` +::: tip -The default name for getters are chosen according to the following rules: +The getter for the member with `#[builder(default)]` returns `Option` because the default value is never stored in the builder. It's calculated [lazily in the finishing function](./default#evaluation-context). + +::: + +## Config + +You can additionally override the name, visibility and docs for the getter method. + +```attr +#[builder( + getter( + name = custom_name, + vis = "pub(crate)", + doc { + /// Custom docs + } + ) +)] +``` + +## `name` -| Member type | Default | -| ----------- | -------------- | -| Required | `get_{member}` | -| Optional | `get_{member}` | +The default name for getter is `get_{member}`. ## `vis` diff --git a/website/src/v1/reference/builder.md b/website/src/v1/reference/builder.md index c1c317f7..5991612a 100644 --- a/website/src/v1/reference/builder.md +++ b/website/src/v1/reference/builder.md @@ -549,7 +549,7 @@ struct Example { **Applies to:** -Overrdies the name for the setters generated for the member. This is most useful when `#[builder]` is placed on a struct where you'd like to use a different name for the field internally. For functions this attribute makes less sense since it's easy to just create a variable named differently `let new_name = param_name;`. However, this attribute is still supported for functions. +Overrides the name for the setters generated for the member. This is most useful when `#[builder]` is placed on a struct where you'd like to use a different name for the field internally. For functions this attribute makes less sense since it's easy to just create a variable named differently `let new_name = param_name;`. However, this attribute is still supported for functions. **Example:**