@NonNull implementation using checker methods instead of if statements

[michaeltecourt]

Lombok generates an if statement in constructors for each field annotated with @NonNull.
Code coverage tools detect this as code branches to test.

It would be cool if Lombok offered an alternative implementation using a checker method like Java 8's Objects.requireNonNull(Object) or Guava's Preconditions.checkNotNull(Object).

lombok.nonNull.impl=[ if | jdk | guava ]

Or maybe just generate a vanilla private implementation of such a method ?

[michaeltecourt]

Thinking about it now, no one cares about how you implement it, as long as it generates as few branches as possible. Objects.requireNonNull is provided by the JDK, I would go with this implementation.

[Maaartinus]

Because of inlining limits, I'd prefer the implementation having the shortest bytecode. Code coverage tools should be able to ignore such branches, but I'm afraid, they are not.

As not everyone uses Guava, it can't be the only implementation. No idea about Lombok compatibility with Java <=6 and how many people still use it. I'd personally be happy with either.

[michaeltecourt]

It's true that Lombok may provide Java 6~7 compatibility.

Anyway it seems like JaCoCo 0.7.10+ and 0.8+ will be able to ignore Lombok generated methods altogether, making this issue obsolete :
http://www.eclemma.org/jacoco/trunk/doc/changes.html

I'll give it a try and close this issue.

[Maaartinus]

@michaeltecourt Do you mean "Methods annotated with @lombok.Generated ...."? This wont always help as there's no generated method, just the added null-check for e.g., void myOwnMethod(@NonNull String s) {}.

JaCoCo would have to recognize and ignore the null-check, when @lombok.NonNull is present an the argument.

[michaeltecourt]

Yes I was speaking about @lombok.Generated methods, and I didn't have @NonNull on method arguments in mind, you're right :)
I only thought about the constructor/builder null checks when attributes are annotated. At least these ones will be skipped by JaCoCo.

[theigl]

I just rolled out Jacoco 0.8 with the newly released Sonar Java plugin 5.1 in my project. Filtering of @lombok.Generated code works perfectly. The only remaining Lombok code that is marked as uncovered are the branches generated by @NonNull.

Using Objects.requireNonNull(Object) when on Java 7+ would solve this.

[sanjeet]

Is there an update on this? I still see branches from @NonNull uncovered.

[theigl]

I'm very interested in this issue since @NonNull causes the only remaining code coverage issues in my application.

@rspilker: Would you accept a PR for this? For Java 7+ the default could be Objects.requireNonNull(Object) or we could make it configurable via a configuration property.

[pbhasker]

I'm very interested in this issue since @nonnull causes the only remaining code coverage issues in my application.

++

[stefan-loewe]

We would also be happy to see this improved.

Besides that, thanks for bringing the lombok library to us!

[wtobi]

This would also avoid warnings about dead code and redundant null checks in Eclipse.
I'm really looking forward to seeing this implemented.

[rspilker]

We have a plan to address this.

[rspilker]

Currently implementing it. The javac works.

The are planning to "abuse" the current config key and add two more values:

• Jdk: adds a call to java.util.Objects.requireNonNull(arg, "arg is marked non-null but is null")
• Guava: adds a call to com.google.common.base.Preconditions.checkNotNull(arg, "arg is marked non-null but is null")

Currently, we scan the first lines in a method until all preconditions are checked. if (arg == null), assert arg != null were considered to be null checks.

We're expanding this to invocations to any checkNotNull or requireNonNull where the first argument is an identifier. Or any assignment where the right-hand side is such method invocation.

[rspilker]

We still need to update some documentation.

[rspilker]

We did create tests but did not execute exhaustive testing of the generated code. Would someone be willing to give it a try in both Eclipse and javac (maven build would be fine, you can use the snapshot repo).

[stefan-loewe]

This indeed works as expected for me, here's what I did.

Download the latest lombok.jar from here, and put it into the Eclipse folder as lombok.jar, with the respective path to that jar set in eclipse.ini via -javaagent:/path/to/eclipse/lombok.jar

Restarting Eclipse (it is Spring Tool Suite for me) correctly showed this then via Help > About

Next, I edited lombok.config in the root folder of my multi-module Spring Boot maven project to include the following key-value pair:
lombok.nonNull.exceptionType=GUAVA

After rerunning a single unit test, the respective method definitions with parameters annotated as @NonNullstill were not fully covered, but after I added @NonNull annotation to another method in the class, and rerun the tests, all was fine (maybe some caching going on somewhere, no idea).

Anyway, seems to work - thanks a lot for the fix!

[theigl]

Great work! I just enabled lombok.nonNull.exceptionType=JDK and my code coverage increased by nearly 3%. 👍

[rzwitserloot]

Heh, this open-forever issue is very optimistically marked 'soon'. Fortunately, this has been part of lombok for quite a while now :)

[Maaartinus]

The Supported configuration keys on @NonNull are not up to date.

[rzwitserloot]

Reopening specifically for the issue of: Fix the documentation.