Can initialize with null value when using with Lombok @Builder together

[blackdiz]

I have a simple class:

@Builder
public class TestC {
    List<String> test;
}

And I use the builder to initialize an object without assigning the test field and expect the framework would raise an error:

       @Test
	void test() {
		System.out.println(TestC.builder().build());
	}

But the test passed, I am not sure if this is a bug or I just misconfigure.

[msridhar]

As I recall Lists in Builders are initialized to an empty list rather than null. So you don't need to explicitly set them. Can you check?

[blackdiz]

Hi, msridhar, here's the generated class:

public class TestC {
    @NonNull @Initialized List<@NonNull @Initialized String> test;

    @Generated
    TestC(final @NonNull @Initialized List<@NonNull @Initialized String> test) {
        this.test = test;
    }

    @Generated
    public static @NonNull @Initialized TestCBuilder builder() {
        return new TestCBuilder();
    }

    @Generated
    public static class TestCBuilder {
        @Generated
        private @NonNull @Initialized List<@NonNull @Initialized String> test;

        @Generated
        TestCBuilder() {
        }

        @Generated
        public @NonNull @Initialized TestCBuilder test(final @NonNull @Initialized List<@NonNull @Initialized String> test) {
            this.test = test;
            return this;
        }

        @Generated
        public @NonNull @Initialized TestC build() {
            return new TestC(this.test);
        }

        @SideEffectFree
        @Generated
        public @NonNull @Initialized String toString() {
            return "TestC.TestCBuilder(test=" + this.test + ")";
        }
    }
}

It looks like the LIst is not initialized to an empty list automatically, so when calling the build() method, it just sets to null.
I think it can just be prevented from assigning a default empty list like:

@Builder
public class TestC {
    @Builder.Default List<String> test = new ArrayList<>();
}

But I am curious when not specifying a default empty list why is nullity not checked at initialization time?

[kelloggm]

@msridhar, is it possible you were thinking of the combination of the @Singular and @Builder annotations? A list annotated as @Singular would be initialized to empty by the builder rather than to null.

I don't think the Nullness Checker ought to do any special handling of lists that are not annotated as @Singular, though, which appears to be the case in the snippet that @blackdiz originally provided. So, this does appear to be a bug.

@blackdiz, I know that there are some situations in which the Checker Framework and Lombok do not play nicely together, as described in the manual here: https://checkerframework.org/manual/#lombok-typechecking. When debugging issues related to Lombok, I've definitely forgotten to delombok the code first and then been surprised by a missing error message...

[msridhar]

@msridhar, is it possible you were thinking of the combination of the @Singular and @Builder annotations? A list annotated as @Singular would be initialized to empty by the builder rather than to null.

Yes that's what I was thinking of! Sorry for the confusion.

[msridhar]

I spent some time investigating this. Here is my branch:

https://github.com/msridhar/tainting-example/tree/called-methods-lombok

First, for the original example, we should not report an error. The reason is that the List<String> test field is not marked as @NonNull, so it is not required by Lombok. I observed that calling the build() method in this scenario does not lead to a crash.

That said, even after adding the @lombok.NonNull annotation to the field, we still do not report an error in my example, which is incorrect. You can observe this by running ./gradlew compileJava on the linked branch. The test case is in this file:

https://github.com/msridhar/tainting-example/blob/called-methods-lombok/src/main/java/calledmethods/TestC.java

I also copied over a BuilderTest class from the Checker Framework regression tests into that same package, and the expected errors are not reported on that class either. In contrast, I created a class EasyTest that does not use Lombok:

https://github.com/msridhar/tainting-example/blob/called-methods-lombok/src/main/java/calledmethods/EasyTest.java

If you comment out line 13 in that file and run ./gradlew compileJava, the Called Methods Checker does report an error.

At this point, I suspect this issue is due to some Gradle / CF Gradle Plugin issue. In the main CF repo, I confirmed that if I modify BuilderTest and remove one of the // :: error: (finalizer.invocation) comments, CalledMethodsLombokTest fails as expected, so the error seems to be getting reported there.

@kelloggm do you have any time to investigate what is happening here?

[msridhar]

@blackdiz were you using the Checker Framework Gradle plugin when trying to compile your initial example?

[kelloggm]

I'll look into it when I get a chance (unfortunately, not right now).

One possibility that comes to mind: do you have a lombok.config file with addLombokGeneratedAnnotations = true? At some point, we used to be able to generate that when running with the called methods checker, but I think that's been broken (and replaced with a warning message) for a year or two now - the io.freefair.lombok plugin used to support us modifying the config directly, but they (reasonably) removed that support in newer versions of their plugin.

[msridhar]

Good guess @kelloggm! Manually adding lombok.config to my test branch fixes the problem. I've pushed a commit with that change to the branch linked above.

I've created an issue on the CF Gradle Plugin around this:

kelloggm/checkerframework-gradle-plugin#220

Since there seems to be no issue in the Checker Framework proper, I'll go ahead and close this issue in favor of that one.