Builder.Default causes fields to be null when instantiating with new

[kevcodez]

When using the @Builder.Default annotation on a list/set/map and instantiating the object using the new keyword, the field is null.

@Data
@Builder
@AllArgsConstructor
@NoArgsConstructor
@Entity
public class MyClass {
    
   @OneToMany(mappedBy = "orderItem", cascade = CascadeType.ALL)
   @Builder.Default
   private List<Entitlement> entitlements = new ArrayList<>();

}

// entitlements is null, should be empty ArrayList
MyClass myClass = new MyClass();

[heapifyman]

This issue also occures with any other non-primitive type, not only list/set/map.

@Getter
@Setter
@AllArgsConstructor
@NoArgsConstructor
@Builder
@ToString(callSuper = true)
public class Test {

    public static final int DEFAULT_INT = 42;

    public enum Status {
        STATUS_ONE,
        STATUS_TWO;
    }

    @Builder.Default
    private Integer someInt = DEFAULT_INT;

    @Builder.Default
    private Status currentStatus = Status.STATUS_ONE;

    public static void main(String[] args) {
        Test test = new Test();
        System.out.println("test = " + test);
    }

}

[TokenR1ng]

This Issue makes the Release 1.16.16 totally unusable for us. Hope this gets fixed in the next Release since we could really use @Builder.Default Annotation.

[wgorski-antrego]

Are there any plans for fixing this? To be honest, it's a HUGE problem as it's super unintuitive and introduces bugs that are really hard to find. To be honest, the most intuitive way of generating a default value for the builder would be to take the value from the field initialization, instead of requiring an additional Builder.Default annotation.

[joaodelgado]

From a quick look at the source code, I found that the @Builder.Default is clearing the initialization part of every annotated field. Line 585:
https://github.com/rzwitserloot/lombok/blob/e92e285d95e0fb08c32e7059f85806a30985084b/src/core/lombok/javac/handlers/HandleBuilder.java#L580-L590

What's the purpose of clearing this initialization? Is it because of potential side effects of the initialization expression?

[omega09]

@joaodelgado See here. Initialization has been moved to the builder so if you don't nullify the field it will be initialized twice.

[davidje13]

@omega09 in that case I would say the @NoArgsConstructor (and similar) must be converted to set the fields too. As it is, this causes bizarre bugs and makes the @builder annotation harmful. It's a clear violation of least-surprise, since it's removing a commonly used language-level feature.

I'm guessing builders have only been considered in isolation, but it is often useful to have @NoArgsConstructor and a builder. One use case is when working with ORM or JSON (where libraries often need no-args constructors). Another is testing (where the main code may use constructors but the tests use builders).

For now, it looks like the only workaround is to manually write a no-args-constructor, which is a pain.

[mimfgg]

writing the constructors manually also means that the constraints annotation like @nonnull have to be processed manually there as well, which is not really elegant.

Any plan to fix the @Builder.default current behaviour?

[omega09]

@davidje13 In that case go write your opinion on the PR to the person who has the authority and not to me, who just explained the reason for this to someone who asked about the implementation detail.

[aburmeis]

To clean the initialization should be done in combination with final fields (like when applying Value) but not for non-final members like when combining Builder with Data, imho.

[dahn-zk]

Any updates?

[TokenR1ng]

We really need this issue to be solved. We cannot update to newer versions because of this and now we are also getting conflicts with other newer libs, e.g. Jackson >2.9. So we cannot update jackson because we cannot update lombok and so on.

It really is a problem for us since we have a lot of classes, e.g. value objects with some fields initialized, that use builder as well as constructor annotations. Even writing the constructors manually would not fix this problem, since @Builder.Default seems to remove the initialization.

Can we please have any feedback on this?
thx

[FerrariBruno]

It seems to work if you write the no args constructor manually. I haven't had any issues. I agree this should be resolved ASAP though.
For me it caused a lot of trouble when paired with DBFlow v3, as it uses constructors to create objects in the generated classes.

[mkeller-ergon]

But only if you duplicate the field defaults again in the default constructor... An empty default constructor won't work.

[FerrariBruno]

Yeah, that's true. If you use the @NoArgsConstructor annotation (or write the no args constructor manually but don't initialize the fields) you will face a ton of trouble.

[palme1337]

Is there any update on this? It seems like the easiest solution is to not remove the original instantiation. That way a default NoArgsConstructor will correctly pick up your field variable instantiation and your builder.build() method will just call the generated "default" targets.

[schuettec]

Is there anyone working on this one? We have very strange side effects when using Lombok in conjunction with JPA. The list references are nulled when using @Builder.Default.

[rspilker]

Yes, we're looking into this.

[xak2000]

@rspilker The reason to make the change, leading to this issue is to get rid of possible double-initialization, right? It is desribed here #1429.

But, really, the double-initialization is a way way less a problem than this totally unexpected behavior of modifying default initialization of POJO fields. Of course, the best way to solve this is somehow get rid of double-initialization and still not break field initializers. But if this is not possible, or too hard, or too long, then, just make this double-intialization happen! Then document it into @Builder.Default javadocs in bold and many people will say thanks you!

Yes, this can lead to problems if someone uses some sofisticated field initializers, but it is much less common, than problems from unexpectedly nullified fields. IMHO.

At least, document this nullifying-field-behavior in @Builder.Default javadocs in big red warning font. This behavior is really, really surprising and hard to found when a NPE thrown.

[rainerhahnekamp]

I totally agree to @xak2000.

I would even go further and say that @Builder.Default is dangerous because the nullification is not documented enough and it is completely non-intuitive.

Lombok is really great but this needs to be fixed. Is there already a pull request? Otherwise I can volunteer for one.

[janrieke]

I am aware that this is still a problem for manual constructors (now for both @Builder and @SuperBuilder). In a previous post I suggested a possible solution, which I think is implementable and does not have too many drawbacks.
We need a word from the maintainers here on how to proceed.

[EvanNGoldstein]

Any update on this? Spent entirely too much time today trying to figure out why my @Builder.Default field was null when using my own constructor.

[goostleek]

You can try this workaround as a remedy until the fix is released.

[twilco]

@goostleek - I tried that workaround, but then realized that our classes that use @Builder also use @Singular, which breaks our existing code that assumes the existence of singular builder fields. Is there some workaround that accounts for the existence of @Singular fields?

[goostleek]

I cannot see the issue. Could you please provide an MWE (with a unit test) for the problem?

[twilco]

Oops, the solution is easy...

For anyone who needs to use @Singular in conjunction with the workaround proposed in the previously linked StackOverflow comment, do this:

@Getter
@Setter
@NoArgsConstructor
public class SearchRequest {
    // This is the field I want to default
    private Context context = Context.default;

    // @Singular
    // We had a singular here, but it will no longer work since the @Builder annotation 
    // was moved from the `public class` above to the below private constructor
    private List<SearchFilter> filters = new ArrayList<>();

    @Builder
    private SearchRequest(SearchContext context,
                          // move your @Singular annotation here
                          @Singular List<RequestFilter> filters) { 
        this.searchContext = Optional.ofNullable(context).orElse(this.searchContext);
        this.filters = filters;
    }
}

[goostleek]

Nice! I have updated the SO answer by referencing your solution in the comment 👍 .

I saw you already did it, removing... 🤡

[MT-Jacobs]

Is the issue found in #1347 (comment) being tracked in this closed ticket, or was there a separate ticket made for the new issue?

[janrieke]

There was a discussion in the forum on how manual constructors could be modified to mitigate that issue: https://groups.google.com/forum/#!topic/project-lombok/e9PzKXRlXXA
In it, @rzwitserloot gave some examples where there simply is no nice solution. Quoting him: "doing it right seems utterly impossible to me".

Thus, I vote for just emitting a warning if there is no assignment to that field in the manual constructor. You would have to fix the warning by adding an assignment. In my opinion, this would prevent most bugs. A drawback is that if you intentionally want it uninitialized, you'd have to add an otherwise unnecessary line; however, this line makes the code easier to understand for non-expert users of lombok.
But everything is up to the lombok maintainers here.

[alturkovic]

It would be best to remove the feature alltogether, it is dangerous and causes a really hard to track bug. Best bet is to avoid it IMHO.

[gavenkoa]

@Builder.Default and custom constructor are incompatible ((

I don't need verification that all field of @Builder are set.

But I'd like to have blueprint method with @Builder(toBuilder = true).

So I rewrote code with constructors replaced by of with builders:

@Builder
public class Connection {
    private String user;
    private String pass;

    @Builder.Default
    private long timeout = 10_000;

    @Builder.Default
    private String port = "8080";

    public static Connection of(String user, String pass) {
        return Connection.builder()
            .user(user)
            .pass(pass)
            .build();
    }

    public static Connection of(String user, String pass, String port) {
        return Connection.builder()
            .user(user)
            .pass(pass)
            .port(port)
            .build();
    }

    public static Connection of(String user, String pass, String port, long timeout) {
        return Connection.builder()
            .user(user)
            .pass(pass)
            .port(port)
            .timeout(timeout)
            .build();
    }
}

It's a shame that I forced to create extra Builder object instead of single one with native new ((

[Maaartinus]

@gavenkoa I surely agree that there's something wrong, but your solution seems to be too complicated as for n variables, you need O(n²) lines. I thought, you could define a private @Wither and use

    public static Connection of(String user, String pass, String port, long timeout) {
        return Connection.of(user, pass).withTimeout(timeout);
    }

but this would create even more garbage. But what about chained setters like

    public static Connection of(String user, String pass, String port, long timeout) {
        return Connection.of(user, pass).setTimeout(timeout);
    }

? Sure, it's still O(n²), but not lines.

Actually, with mutable objects, I always wonder what anybody needs builder for?

[gavenkoa]

@Maaartinus @Wither is still experimental feature. Docs says that they can even changes name ((

I don't need builders, I just like chaining and setters chaining breaks Bean API ((

[Maaartinus]

@gavenkoa You're right concerning @Wither being experimental. But it's experimental since 2012 and nobody has come with a better name since then. It's one of my favorites.

@rzwitserloot IMHO, @Wither should be promoted out of experimental, WDYT?

setters chaining breaks Bean API

I see. With the beanscrap, there's no solution. The Lombok team refuses to implement both kinds of setters as this is an ugly API (and I agree; cure worse than the disease). With the Builder, it's ugly in a different, maybe nicer, way.

[rspilker]

We plan to do some work on @Wither soon. So it's going to stay in experimental for a while.

The idea is to improve the usability for Persistent Data Structures.

We're considering the following alternatives, possible we can support all of the,

@RequiredArgsConstructor
class Point {
  @Wither final int x;
  final int y;
}

class Point {
  final int x;
  final int y;

  Point(int x, int y) {
    this.x = x;
    this.y = y;
  }

  // Current implementation
  Point withX(int x) {
    return new Point(x, this.y);
  }

  // Alternative 1
  Point withX(Function<Integer, Integer> xFunction) {
    return new Point(xFunction.apply(this.x), this.y);
  }

  // Alternative 2
  Point withX(Function<Point,Integer> xFunction) {
    return new Point(xFunction.apply(this) this.y);
  }
}

[jasonCodesAway]

This is a long and complicated comment history. Was this issue closed as will not be fixed? I'm definitely hitting this issue and currently trying this workaround from SO, which indicates this is broken

[a-serebryanskiy]

I currently can use both @NoArgsConstructor and @Builder with @Builder.Default and some other constructor this way:

@NoArgsConstructor
@AllArgsConstructor
@Builder
public SomeClass {
  @Builder.Default
  private int someInt = 0;
  @Builder.Default
  private String someString = "initial value";

  public SomeClass(int someInt) {
    this();
    this.someInt = someInt;
  }
}

This way all three constructors and builder correctly initialise fields.

My lombok version is 1.18.2

[rzwitserloot]

@asserebryanskiy cool trick – unfortunately, does not work for final fields.

[rzwitserloot]

@jasonCodesAway no, it was closed because it is fixed. However, right now manually written constructors do still run into the lack of default value issue. The only fix is for us to go in there and manually add a line which is complicated, but @janrieke has a plan. We'll talk about it next time we triage bugs and features. Re-opening, specifically for MANUALLY written constructors.

It'll be hard to do this properly, see commentary on https://groups.google.com/forum/#!topic/project-lombok/e9PzKXRlXXA – but at the very least we should put in some effort to detect that this is happening (a manual constructor referring to a field with a defaulting annotation on there) and emit a warning.

[randakar]

How about just creating a new bug for that? Neither the title not the problem description really match the issue left over ;)

…

On Fri, Jan 17, 2020, 04:24 Reinier Zwitserloot ***@***.***> wrote: @jasonCodesAway <https://github.com/jasonCodesAway> no, it was closed because it is fixed. However, right now manually written constructors do still run into the lack of default value issue. The only fix is for us to go in there and manually add a line which is complicated, but @janrieke <https://github.com/janrieke> has a plan. We'll talk about it next time we triage bugs and features. Re-opening, specifically for *MANUALLY* written constructors. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1347>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABIERMMFBWMZKOXK4GE65TQ6EQINANCNFSM4DFS7BUA> .

[rzwitserloot]

Moved to new feature #2340