Lombok: [FEATURE] address @Builder.Default and explicit constructors weird behaviour.

[rzwitserloot]

Problem

Given:

    @Builder @NoArgsConstructor @AllArgsConstructor class Example {
        int a;
        @Builder.Default int x = 10;

        public Example(int i) {
            this.a = i + 15;
        }
    }

The code compiles, and if I were to invoke new Example(5), then the value for x ends up being.... 0. Because the default expression (here, 10) is moved, and 'only' used by lombok-generated code (so, the generated no-args constructor uses it, as does the builder).

the common case: the field is final

Right now, if the field is final, then the above code example does not compile, because the manually written constructor does not definitely assign x. If a manually written constructor DOES 'definitely assign' it, I don't think we have a problem here. So that's easy; we need not change anything.

the uncommon case: the field is not final

For non-final fields with defaults, we check if manually written constructors exist. If they do, we analyse the default value and check if it is either a literal, or, a 'field reference', such as Integer.MAX_VALUE. If it is, we make the initializing expression of the field an invocation to the default-generating method (because we know for sure this is side effect free, so any double-invoke doesn't matter). If it is not (for example, it is counter++ or LocalDate.now()), we generate a warning. This warning cannot be removed, which effectively means that you can't use lombok at all. Let's hope the combination of manually written constructors + non-constant defaults are rare enough that it doesn't matter. Even if the 'non-constant default' is constant after all. For example, LocalDate.of(2020, 1, 1) is constant, but lombok doesn't know that.

OPEN QUESTION: Should we have a parameter on Builder.Default to say: yeahyeah just generate the invoke, I'm aware this means any attempt to construct this thing will neccessarily resolve the initializer expression once EVEN IF an explicit value is set?

[aaditya-taparia]

Any updates on this?

[ikc89]

Hi all,

We are having this problem also. When we are trying to get the class instance with builder().build(), Boolean class type properties can not be set with null and throws NullPointerException, but no problem with String type. Builder.Default is not working too. Is it usual behaviour or am i missing something ?

[indrekpr]

the common case: the field is final
Right now, if the field is final, then the above code example does not compile, because the manually written constructor does not definitely assign x. If a manually written constructor DOES 'definitely assign' it, I don't think we have a problem here. So that's easy; we need not change anything.

Regarding the common case when a field is final, in some cases the code still compiles and so the @Builder.Default behaves unexpectedly by leaving the field uninitialized. Please consider the following example where both the x and y remain null when using builder().build():

class Example {

  // required fields
  @Builder.Default private final AtomicReference<Integer> x = new AtomicReference<>(10);
  @Builder.Default private final Collection<Integer> ys = new CopyOnWriteArrayList<>();

  // optional fields
  @Setter private Integer a;
  @Setter private Integer b;
  @Setter private Integer c;

  public Example(Integer x, Collection<Integer> ys) { // mainly for production use
    this.x.set(x);
    this.ys.addAll(ys);
  }

  @Builder // mainly for tests
  private Example(Integer x, Collection<Integer> ys, Integer a, Integer b, Integer c) {
    this.x.set(x);
    this.ys.addAll(ys);
    this.a = a;
    this.b = b;
    this.c = c;
  }

  public Integer getX() {
    return x.get();
  }

  public Collection<Integer> getYs() {
    return ys;
  }

}

import org.junit.jupiter.api.Test;

import java.util.Arrays;
import java.util.List;

import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;

class ExampleTest {

  @Test
  void defaultValue() {
    int defaultX = 10;

    Example example = Example.builder().build();

    assertAll(
        () -> assertEquals(defaultX, example.getX()),
        () -> assertNotNull(example.getYs())
    );
  }

  @Test
  void explicitValue() {
    int givenX = 2;
    List<Integer> givenYs = Arrays.asList(5, 6);

    Example example = Example.builder().x(givenX).ys(givenYs).build();

    assertAll(
        () -> assertEquals(givenX, example.getX()),
        () -> assertEquals(givenYs, example.getYs())
    );
  }

}

[mjustin]

How does this solution affect the case where the manual constructor is delegating to the Lombok constructor (e.g. using this(); as the first line)? Right now, this is initializing the default field values as expected:

public Example(int i) {
    this();
    this.a = i + 15;
}

[iprem]

Just checking to see if there are any updates on this issue?