Builder should not include transient fields

[GoogleCodeExporter]

What steps will reproduce the problem?
1. have a class with transient fields (i.e. EntityManager)
2. annotate this class with @Builder
3. create an instance of this class using the builder

What is the expected output? What do you see instead?
Expeced:
* transient field is not part of builder
* transient field initialization (i.e. through injection) does not get 
overwritten if not set in builder

Instead:
* transient field is part of builder
* if transient field is not set the inject value gets overwritten with NULL

What version of the product are you using? On what operating system?
Lombok Version: 1.12.2
OS: Windows 7 64bit

Please provide any additional information below.

Original issue reported on code.google.com by rainer.p...@gmail.com on 22 Nov 2013 at 7:54

[GoogleCodeExporter]

maybe it makes sense to also support specific excludes in the Builder

Original comment by rainer.p...@gmail.com on 22 Nov 2013 at 7:56

[GoogleCodeExporter]

this issue also exists with other injected fields (i.e. auto generated ids in 
Entities)

Original comment by rainer.p...@gmail.com on 22 Nov 2013 at 8:00

[GoogleCodeExporter]

Simply skipping transient fields in @Builder is by far the most straightforward 
solution.

What do you want us to do about injected fields? Should we skip when we detect 
any @Inject annotation? I don't like the idea of jumping through resolution 
hoops for this, so it'd be similar to how we detect other 'Nonnull' annotations 
(in any casing) than our own: match the simple name, and nothing more.

Original comment by askon...@gmail.com on 25 Nov 2013 at 9:15

• Added labels: Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

I think having an attribute where I can define which fields to exclude would be 
the most flexible way.

Using the @Inject annotation might not work for everyone as different 
dependency injection frameworks do use different annotations (i.e. @Autowired 
for Spring)

Original comment by rainer.p...@gmail.com on 25 Nov 2013 at 12:16

[GoogleCodeExporter]

I guess it's all about the @XConstructor rather then @Builder, isn't it?

I'm afraid that always excluding the injected field is not an option as you 
often avoid injection in tests and have to set the fields manually.

What about something like

    @CustomConstructor(builderName="XxxBuilder",
        excludeTransient=true,
        excludeAnnotated={Inject.class})

? I believe, this is as flexible as excluding fields by name as you can always 
create your own exclusion annotation. Unlike @EqHC(exclude={"field1", 
"field2"}) it survives refactorings. It needs neither resolution nor hacking 
around with case-insensitive class name matching.

Excluding transient fields should probably be a separate option as it's a 
common case using no annotations.

Original comment by Maaarti...@gmail.com on 7 Dec 2013 at 9:59

[GoogleCodeExporter]

Sounds good to me

Original comment by rainer.p...@gmail.com on 7 Dec 2013 at 10:01

[GoogleCodeExporter]

Now I spotted that this part of the OP's question makes no sense to me:

* transient field initialization (i.e. through injection) does not get 
overwritten if not set in builder

So far the builder creates a new object, so there's nothing to be overriden at 
all. 

Using Guice, you either get your object from the injector or you create it 
yourself (possibly using a builder). If you choose the latter, no injection 
gets done.

You can invoke it explicitly after object creation. There might be even a 
possibility to hack it in via AOP, but that's a hack outside of Guice.

I don't know much about Spring and other IOC containers, but their magic is 
similarly limited.

Original comment by Maaarti...@gmail.com on 8 Dec 2013 at 5:08

[GoogleCodeExporter]

The Bilder uses an all args constructor, so if you dont set a field in the 
builder it passes a null value to the constructor which then overrides any 
initialized values

Original comment by rainer.p...@gmail.com on 8 Dec 2013 at 9:38

[GoogleCodeExporter]

Sure... my point was that there's no injection BEFORE construction and that the 
field initializers (running before the ctor's code) can't help the OP at all. 
What they'd need is to call `injector.injectMembers(someObject)` AFTER the 
object creation.

https://code.google.com/p/google-guice/wiki/Injections

I've never seen injection done by field initializers and it makes no sense to 
me... you need to ask an injector to do it and you have none (unless you switch 
to some dirty hack including static variables).

Original comment by Maaarti...@gmail.com on 8 Dec 2013 at 10:28

[GoogleCodeExporter]

Yes, you are right with injections, but there is still the same issue with 
field initializers

Original comment by rainer.p...@gmail.com on 10 Dec 2013 at 8:48

[GoogleCodeExporter]

If you don't want certain fields in your builder then you should make a static 
method or constructor with the 'right' fields and put @Builder on that.

So, what this is really all about is what @AllArgsConstructor is supposed to 
make. Right now, AAC does NOT skip transient fields. This is technically on 
purpose but I can be convinced that it's the wrong behaviour. It is unfortunate 
that changing this is a breaking change so we can't just throw this 'fix' in a 
point release. We aren't going to add excludes to builder; it makes zero sense 
to do this. It's not builder that's defining which fields are supposed to end 
up in the builder itself. It's what you put @Builder on. By way of shortcut, 
@Builder on a class adds an @AllArgsConstructor and acts as if it was on that 
constructor, but that's just a shortcut. If you want excludes behaviour, just 
write @Builder @AllArgsConstructor(exclude={"someField", "someOtherField"}).

Maaartinus' trick to use a (custom) annotation specified in a field-iterating 
annotation such as @AllArgsConstructor is cool, although I wonder if its not 
simpler to just add i.e. @AllArgsConstructor.Exclude. I think we thought of 
this before and someone raised an issue with that plan but now I can't remember 
what that was. Hmmm..

Original comment by reini...@gmail.com on 23 Jan 2014 at 6:41

[GoogleCodeExporter]

Here is my case: I have a POJO with over 100 properties to integrate with some 
external json api. Some of the fields should be "readonly" eg. its enough for 
me to exclude them from setters (achieved by @Setter(AccessLevel.NONE)) and 
builder. Ideally would be to give @Builder an option to skip AccessLevel.NONE 
properties. 

Argument to make static method or constructor (with few tenth of properties?) 
seems to me against the reason lombok is for :)

Original comment by ciek...@gmail.com on 20 May 2014 at 9:40

[GoogleCodeExporter]

My reason for using custom annotations rather than @AllArgsConstructor.Exclude 
is that existing annotations can be reused. There might be already some 
@Transient or whatever on the field to be excluded.

Original comment by Maaarti...@gmail.com on 20 May 2014 at 9:58