-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
@NonNull implementation using checker methods instead of if statements #1197
Comments
Thinking about it now, no one cares about how you implement it, as long as it generates as few branches as possible. |
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. |
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 : I'll give it a try and close this issue. |
@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., JaCoCo would have to recognize and ignore the null-check, when |
Yes I was speaking about |
I just rolled out Jacoco 0.8 with the newly released Sonar Java plugin 5.1 in my project. Filtering of Using |
Is there an update on this? I still see branches from |
I'm very interested in this issue since @rspilker: Would you accept a PR for this? For Java 7+ the default could be |
++ |
We would also be happy to see this improved. Besides that, thanks for bringing the lombok library to us! |
This would also avoid warnings about dead code and redundant null checks in Eclipse. |
We have a plan to address this. |
Currently implementing it. The The are planning to "abuse" the current config key and add two more values:
Currently, we scan the first lines in a method until all preconditions are checked. We're expanding this to invocations to any |
We still need to update some documentation. |
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). |
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 Restarting Eclipse (it is Spring Tool Suite for me) correctly showed this then via Help > About Next, I edited After rerunning a single unit test, the respective method definitions with parameters annotated as Anyway, seems to work - thanks a lot for the fix! |
Great work! I just enabled |
Heh, this open-forever issue is very optimistically marked 'soon'. Fortunately, this has been part of lombok for quite a while now :) |
The Supported configuration keys on @NonNull are not up to date. |
Reopening specifically for the issue of: Fix the documentation. |
It did not mention the Guava and JDK options.
It did not mention the Guava and JDK options.
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'sPreconditions.checkNotNull(Object)
.Or maybe just generate a vanilla private implementation of such a method ?
The text was updated successfully, but these errors were encountered: