-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Fix confusing behaviour of NonNull annotations. #3525
Conversation
if (fqn == null) return Long.MAX_VALUE; | ||
if (fqn == null) { | ||
if (isNonNullAnnotation(annotationNode, annotation)) { | ||
fqn = lombok.NonNull.class.getName(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes the inconsistent behaviour mentioned in #3305 (comment)
We run in this code path, if an unknown annotantion is found on an method or constructor paramerter. In this case we check, if this could be an alternative nonNull annotation. If so, we treat it like the lombok.NonNull annotation.
* @return depending on the config, it either returns a list of all known non-null annotations or just containing the lombok.NonNull annotation | ||
*/ | ||
public static List<String> nonNullAnnotations(Boolean useForeignAnnotations) { | ||
return Boolean.FALSE.equals(useForeignAnnotations) ? LOMBOK_NONNULL_ANNOTATIONS : NONNULL_ANNOTATIONS; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the switch logic, which non-null annotations should lombok use. By default, it uses all known nonNull-annotations. if lombok.nonNull.useForeignAnnotations
is set to false, only the lombok.NonNull annotations (there is currently only one, but we are prepared, if an experimental one is added)
So people, who do not want that lombok use foreign annotations can turn it off now.
It would be also possible to specify a list of nonNull annotations in the configuration. What do you think?
This change would be backwards incompatible and is therefore vetoed. |
See #3305
This PR fixes the inconsistent behaviour, that non-lombok
@NonNull
annotations are honored at fields, but not on method/constructor parameters.It also adds a config option to completely turn off or on the use of foreign nonNull annotations