Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix handling of lifetimes not used in fn param types #208

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions bon-macros/src/builder/builder_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,8 +289,8 @@ impl BuilderGenCtx {
_ => None,
});

let types = receiver_ty
.into_iter()
let types = std::iter::empty()
.chain(receiver_ty)
.chain(member_types)
.chain(generic_types)
.map(|ty| {
Expand All @@ -305,6 +305,11 @@ impl BuilderGenCtx {
quote!(fn() -> ::core::marker::PhantomData<#ty>)
});

let lifetimes = self.generics.args.iter().filter_map(|arg| match arg {
syn::GenericArgument::Lifetime(lifetime) => Some(lifetime),
_ => None,
});

let state_var = &self.state_var;

quote! {
Expand All @@ -318,6 +323,16 @@ impl BuilderGenCtx {
// applicability of the auto traits.
fn() -> #state_var,

// Even though lifetimes will most likely be used somewhere in
// member types, it is not guaranteed in case of functions/methods,
// so we mention them all separately. This covers a special case
// for function builders where the lifetime can be entirely unused
// (the language permis that).
//
// This edge case was discovered thanks to @tonywu6 ❤️:
// https://github.com/elastio/bon/issues/206
#( &#lifetimes (), )*

// There is an interesting quirk with lifetimes in Rust, which is the
// reason why we thoughtlessly store all the function parameter types
// in phantom data here.
Expand Down
133 changes: 133 additions & 0 deletions bon/tests/integration/builder/generics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,3 +209,136 @@ fn lifetimes_with_bounds() {

sut().arg(&42).arg2(&42).call();
}

// This is based on the issue https://github.com/elastio/bon/issues/206
mod lifetimes_used_only_in_type_predicates {
use crate::prelude::*;

trait Trait<'a> {}

impl Trait<'_> for u32 {}

#[test]
fn function() {
#[builder]
fn sut<'a, T: Trait<'a>>(x1: T) -> T {
x1
}

assert_eq!(sut().x1(2).call(), 2);
}

#[test]
fn generic_method() {
struct Sut;

#[bon]
impl Sut {
#[builder]
fn method<'a, T: Trait<'a>>(x1: T) -> T {
x1
}

#[builder]
fn with_self<'a, T: Trait<'a>>(&self, x1: T) -> T {
let _ = self;
x1
}
}

assert_eq!(Sut::method().x1(2).call(), 2);
assert_eq!(Sut.with_self().x1(2).call(), 2);
}

#[test]
fn generic_impl() {
struct Sut<T>(T);

#[bon]
impl<'a, T: Trait<'a>> Sut<T> {
#[builder]
fn method(x1: T) -> T {
x1
}

#[builder]
fn with_self(&self, x1: T) -> T {
let _ = self;
x1
}
}

assert_eq!(Sut::<u32>::method().x1(2).call(), 2);
assert_eq!(Sut(1).with_self().x1(2).call(), 2);
}
}

mod unused_lifetimes {
use crate::prelude::*;

#[test]
fn function() {
#[builder]
#[allow(clippy::extra_unused_lifetimes, unused_lifetimes)]
fn sut<'a, 'b>() {}

sut().call();
}

#[test]
fn generic_method() {
struct Sut;

#[bon]
impl Sut {
#[builder]
#[allow(clippy::extra_unused_lifetimes, unused_lifetimes)]
fn method<'a, 'b>() {}

#[builder]
#[allow(clippy::extra_unused_lifetimes, unused_lifetimes)]
fn with_self<'a, 'b>(&self) {
let _ = self;
}
}

Sut::method().call();
Sut.with_self().call();
}

#[test]
fn generic_impl() {
struct Sut<T>(T);

// Interestingly, this code doesn't produce an `unused_lifetimes` warning
// because the generated starting functions are inserted into this impl
// block and they do use the lifetimes by storing them in the builder.
// There isn't a good way to deal with this problem.
//
// We could generate the starting functions in a separate impl block, but
// then it would break the lexical order of methods as they are declared
// in this impl block in regards to how they are displayed in `rustdoc`.
//
// Also, rustdoc permits documentation on the impl block itself, so if
// we create a separate impl block for the starting functions, that
// would be rendered as separate impl blocks in `rustdoc` as well and we
// would need to do something about the docs on the original impl block,
// (e.g. copy them to the impl block for starting functions?).
//
// Anyway, the problem of unused lifetimes lint false-negative is probably
// not that serious to justify the complexity of the solution to fix it.
#[bon]
impl<'a, 'b, T> Sut<T> {
#[builder]
fn method() {}

#[builder]
fn with_self(&self) {
let _ = self;
}
}

Sut::<u32>::method().call();
Sut(1).with_self().call();
}
}
1 change: 1 addition & 0 deletions scripts/test-msrv.sh
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ step cargo update -p syn --precise 2.0.56
step cargo update -p tokio --precise 1.29.1
step cargo update -p expect-test --precise 1.4.1
step cargo update -p windows-sys --precise 0.52.0
step cargo update -p libc --precise 0.2.163

export RUSTFLAGS="${RUSTFLAGS:-} --allow unknown-lints"

Expand Down
Loading