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

Builders require generic types to impl Default even when not needed. #178

Closed
ColonelThirtyTwo opened this issue Oct 16, 2020 · 5 comments
Closed

Comments

@ColonelThirtyTwo
Copy link
Contributor

Example code:

use derive_builder::Builder;

// Does not impl Default
struct Foo;

#[derive(Builder)]
struct Bar<T> {
	value: T
}

fn main() {
	BarBuilder::default()
		.value(Foo)
		.build()
}

This doesn't build; it complains that Foo does not implement Default. It obviously doesn't need to, but the Default implementation that the #[derive(Default)] adds the bound anyway. This makes the builder impossible to instantiate for types that take generic types that do not implement Default.

PS Is this crate still maintained? Last release was a year ago.

@rardiol
Copy link

rardiol commented Nov 2, 2020

I believe you can use the derivative crate to derive Default and then set Default to boundless: https://mcarton.github.io/rust-derivative/Default.html#custom-bound

@ColonelThirtyTwo
Copy link
Contributor Author

@rardiol Well, I can't, because the generated builder has to have the derive and there's no way to add the derive to it.

I made a PR fixing this issue here, #179, but since has gone 17 days without any sort of response I can only conclude that this crate is not actively maintained.

@TedDriggs
Copy link
Collaborator

You're correct; I've been short on time and haven't had resources to make sure changes to this crate don't break anyone depending on it. The past few efforts have led to revving the minor version for most changes, but that's not great for people either.

The standard library applies the Default bound to all type parameters, even if it technically doesn't need them. I think your change is safe, but I'm concerned about deviating from the native derive behaviors since I'm not sure if it sets us up for problems down the line. For example, are there any cases we'd need other bounds on those type parameters? (I think not because we initialize everything to None)

@ColonelThirtyTwo
Copy link
Contributor Author

Well, this crate is literally unusuable with generic types, unless they all happen to derive Default. It's frustrating that the seemingly most popular builder crate can't get this fairly basic case right.

None should be valid for every Option<T>. If for some reason you need more bounds on the builder, you can do what serde does and expose an attribute for adding them.

@TedDriggs
Copy link
Collaborator

This is now live in 0.10.0-alpha. Please test it out and make sure it's working as-expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants