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

Consider allowing some annotations on Builder class #731

Open
lombokissues opened this issue Jul 14, 2015 · 15 comments
Open

Consider allowing some annotations on Builder class #731

lombokissues opened this issue Jul 14, 2015 · 15 comments

Comments

@lombokissues
Copy link

Migrated from Google Code (issue 696)

@lombokissues
Copy link
Author

👤 Maaartinus   🕗 Jun 14, 2014 at 15:18 UTC

On the Builder class many (all?) Lombok annotations are forbidden. Sure, most of them don't make any sense, but some do.

Currently, @ Builder supports just the most straightforward way of building the argument list for a constructor (or a static method). Sometimes a bit more is need, e.g., inspection of the Builder fields (there were many similar requests in issue #89).

I could imagine also allowing @ Setter so that the builder can be also a valid bean (this leads to having both x(1) and setX(1), but who cares, when this is what the user explicitly requested?).

Allowing @ AllArgsConstructor could help those who'd like to convert an object into its builder (easier workaround for issue #635).

All the above needs nothing but removing the classes from HandlerUtil.INVALID_ON_BUILDERS and writing some tests (which I could do).

Possibly also @ ToString(includeFieldNames=false) may make sense, but this would require more work.

All this said, I'm specially interested in @ Getter.

@lombokissues
Copy link
Author

👤 jorn86   🕗 Aug 05, 2014 at 12:17 UTC

+1, this would help me with extending @ Builder-generated functionality

@lombokissues
Copy link
Author

👤 stefan.baur@namics.com   🕗 Apr 07, 2015 at 16:16 UTC

It would be nice if you could tell the builder the use the getters/setters instead of the fields directly.
Also, it would be great to tell the Builder the target Class to generate the Builder for.

Using this, you can generate a Builder for a Class that comes from a Framework.

Example:
@ Builder(forClass = org.thymeleaf.Configuration.class, builderClassName = "ThConfigBuilder",builderMethodName = "thBuilder")
public class BuilderTest {
}

@lombokissues
Copy link
Author

👤 stefan.baur@namics.com   🕗 Apr 07, 2015 at 16:18 UTC

@ Builder(forClass = org.thymeleaf.Configuration.class, useSetters=true, builderClassName = "ThConfigBuilder",builderMethodName = "thBuilder")
public class BuilderTest {
}

@lombokissues
Copy link
Author

End of migration

@simplysoft
Copy link

Adding support for @ToString(includeFieldNames=false) to Builder would be great

@mthmulders
Copy link
Contributor

Something I often see is a POJO (say MyPojo) with @Builder on it, and then:

@com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder(withPrefix = "")
MyPojoBuilder {
}

This way, Jackson can also use the generated MyPojoBuilder and MyPojo is truly immutable. Although it works great, it's a bit repetitive. So a onBuilderClass attribute for @Builder would be nice, allowing you to specify any annotations that must be put on the generated builder class. I suppose it would work pretty much just like onConstructor for @AllArgsConstructor.

@mthmulders
Copy link
Contributor

@rzwitserloot, would you consider a PR that adds onBuilderClass to @Builder? I have a working concept, just need to polish a few things and add some documentation.

@rzwitserloot
Copy link
Collaborator

@mthmulders Yeah, that sounds like a good addition. I'm leaving this open for the onBuilderClass request.

Let's not allow @Setter and co. on Builder. Don't use em as beans. I can't foresee the impact having to support that idea in future updates is going to have.

@Maaartinus
Copy link
Contributor

@mthmulders Is onBuilderClass a good idea when we can add an empty public static class Builder {} and annotate it? It's more readable and doesn't depend on the @__ hack.

@mthmulders
Copy link
Contributor

@Maaartinus It saves you boiler plate code, and as far as I'm concerned, that's one of the main features of project Lombok. Explicitly writing the XBuilder class for the sole reason to put an annotation on it feels redundant to me.

@Maaartinus
Copy link
Contributor

@mthmulders Sure, but there may be other reasons why you need to write the class (making it package-protected or adding some other stuff). With this annotation parameter, you can save about ten characters, but when you later need to create the builder class, you actually wasted about ten characters. I'm not really opposed to the idea, it's just that it gives me a choice I don't like.

@mthmulders
Copy link
Contributor

Maybe I don't understand your concern correctly, @Maaartinus, but if it's a choice you don't like then you're free to pick the other option? You could still write your XBuilder by hand and modify it the way you like?

@Maaartinus
Copy link
Contributor

@mthmulders I'd prefer not to be able to pick (unless the options bring a lot of value). I hate TMTOWTDI, but it may be just me. Just don't let you stop by it.

@mthmulders
Copy link
Contributor

Update: I'm not planning to work on the onBuilderClass annotation. It turns out that #1180 actually saves most of the Jackson-related boilerplate: manually defining and annotating it with @JsonPOJOBuilder.

lianhaijun pushed a commit to lianhaijun/lombok that referenced this issue May 8, 2020
Bumps org.jetbrains.intellij from 0.4.15 to 0.4.16.

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
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

5 participants