@Singular breaks ability to set null to a list

[GoogleCodeExporter]

I have a class annotated with @Builder.
In it there is a list (List<>) attribute annotated with @Singular.

Sample:

@Builder
public class MyClass {
    @Singular("item") private List<String> items;
    private List<String> items2;
}

If I try to set to null that attribute using the Builder, a 
NullPointerException is thrown. Looks like the builder method tries to add null 
to the list.

MyClass.builder().items( null ).build(); // <--- null pointer exception

If i try to set to null a list attribute which is not annotated with @Singular, 
everything is fine.

MyClass.builder().items2( null ).build(); // <--- works

I attach the test case.
I tried with Java 7 and Lombok 1.16.2.

Original issue reported on code.google.com by robyfo...@gmail.com on 11 Feb 2015 at 4:33

Attachments:

• LombokTest.java

[GoogleCodeExporter]

This is actually documented in 
http://projectlombok.org/features/Builder.html#singular

The method is not a setter, but this:

public DataClassBuilder itemsWithSingular(final java.util.Collection<? extends 
String> itemsWithSingular) {
    if (this.itemsWithSingular == null) this.itemsWithSingular = new java.util.ArrayList<String>();
    this.itemsWithSingular.addAll(itemsWithSingular);
    return this;
}

I'm not sure what's best here.

1. Ignore it. A terrible error-hiding idea.
2. Set this.itemsWithSingular to null. This is a sort-of way for cleaning the 
list, but it's pretty terrible, too.
3. Keep it as is.

I guess 3 is best. Maybe document it in the Javadoc (where there's only `add` 
mentioned). Maybe add a null check with an explanation.

Original comment by Maaarti...@gmail.com on 13 Feb 2015 at 10:27

[GoogleCodeExporter]

We've considered it, and it is as Maaartinus says: ".items()" here doesn't 
_SET_ the items list, it _ADDS_ all the items in the provided item list, to the 
running tally that will form the collection once you build(). The concept of 
"Please add every element in this non-existent list to the object" makes no 
sense; the right way to address this is to throw an exception instead of trying 
to take a wild stab as to what the user intends to happen.

The NPE will stand.

Original comment by reini...@gmail.com on 14 Apr 2015 at 7:50

• Changed state: WontFix

[GoogleCodeExporter]

I'm sorry to bother about this issue but I really don't agree.

Think about a library having various builders with list attributes, some having 
the singular annotation and some having not. How is a developer using this 
library supposed to understand that passing null sometimes sets and sometimes 
adds and crashes.

This inconsistency will be addressed as a bug.

Another scenario is a builder having a list with a default content which you 
want instead set to null. You just can't because you can no more set this 
attribute to null. Remember that an empty list and null are not the same and 
are simantically different.

Please reconsider the issue. 

Original comment by robyfo...@gmail.com on 15 Apr 2015 at 12:25