Skip to content
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

Please add NonNull/Nullable annotations #248

Closed
remal opened this issue Jul 22, 2019 · 14 comments
Closed

Please add NonNull/Nullable annotations #248

remal opened this issue Jul 22, 2019 · 14 comments
Labels

Comments

@remal
Copy link

remal commented Jul 22, 2019

I'd like to use the library with Kotlin language, which can handle NonNull/Nullable annotations during compile-time.

Please add these annotations in the library. If you want, I can do it myself by sending a PR.

@keilw
Copy link
Member

keilw commented Jul 22, 2019

Which ones? there are almost too many such annotations, and several of those projects are dead or have very little activity.

@remal
Copy link
Author

remal commented Jul 23, 2019

I'd choose between JSR305 (com.google.code.findbugs:jsr305 for example), or Jetbrains' ones: org.jetbrains:annotations.

The full list of annotations supported by Kotlin lang can be found here: https://github.com/JetBrains/kotlin/blob/master/core/descriptors.jvm/src/org/jetbrains/kotlin/load/java/JvmAnnotationNames.kt

@keilw
Copy link
Member

keilw commented Jul 23, 2019

According to https://jcp.org/en/jsr/detail?id=305 JSR 305 is dormant for over 7 years now.
While javax.annotation.Nullable sounds best, it is part of that dormant JSR and worse, the package name is shared with Jakarta Common Annotations, an absolute No-go unless that annotation became part of Jakarta Annotations some day (package-splitting is prohibited by Jigsaw) These https://github.com/JetBrains/java-annotations by JetBrains look decent, most of the others are either defunct or come with to much overhead (eg. RXJava does not have a separate annotation module) or target Android mostly.

@keilw
Copy link
Member

keilw commented Jul 23, 2019

This is also mentioned in jakartaee/common-annotations-api#50, so in a future version we might simply use Jakarta Annotations (which is already used by SI Units 2.x) for both purposes.

@teobais
Copy link
Member

teobais commented Jul 26, 2019

For the reasons mentioned by @keilw in the above two comments, I would also not investigate such a possibility at the moment.

@keilw
Copy link
Member

keilw commented Jul 26, 2019

@remal What would the annotations bring in particular, and have you checked https://github.com/Tenkiv/Physikal for use with Kotlin?

@remal
Copy link
Author

remal commented Jul 26, 2019

@keilw

  1. There are some static analysis tools that rely on NonNull/Nullable annotations. They help to avoid NullPointerException bugs
  2. No, I haven't checked this project, I'll take a close look, thanks! It looks like only a list extensions...

@keilw keilw added this to the 2.0.1 milestone Jul 26, 2019
@keilw
Copy link
Member

keilw commented Jul 26, 2019

Well 2.0 is already done, but we could include it in a follow up version.

@remal
Copy link
Author

remal commented Aug 12, 2019

Just to be on the same page. @keilw, you're going to add @NonNull/Nullable annotations to one of the next versions, right?

@keilw keilw added the deferred label Aug 13, 2019
@keilw
Copy link
Member

keilw commented Aug 13, 2019

@remal If there is a more stable spec like Jakarta Annotations including such annotations, we might do that I see no reason against it, especially @Priority is already used, so we would get it for free. Until then marking it as deferred.

@keilw keilw removed this from the 2.0.1 milestone Aug 22, 2019
@keilw
Copy link
Member

keilw commented Sep 27, 2020

We won't do that until Jakarta Annotations included the currently defunct JSR 305 annotations: jakartaee/common-annotations-api#50

@keilw
Copy link
Member

keilw commented Feb 2, 2021

@remal @thodorisbais after looking at this again, quite frankly I would leave it to application code to apply a @NonNull or any annotation of their choice. Although it may lead to undesired side-effects there are good reasons why e.g. the symbol inside a Unit implementation or a Quantity value could be null, e.g. SPI base types like Range where we evaluated Optional, but unfortunately that is not very well-designed for nested generics either and it caused problems, therefore nullness is allowed to represent a missing upper or lower-bound. Almost everywhere else internally there are null-checks, unless a null-value is intentionally allowed, so there is not a real value to use those annotations inside the RI, while it is up to developers to combine it with Bean Validation or other annotations in their own code.

@keilw keilw closed this as completed Feb 2, 2021
@remal
Copy link
Author

remal commented Feb 2, 2021

@keilw Basically, this issue is not about runtime checks inside the library itself - it's mainly about static analysis. Indeed, Java developers won't benefit much from it (only from static analysis perspective), but Kotlin developers will, as Kotlin handles nullity annotations and behaves accordingly.

@keilw
Copy link
Member

keilw commented Feb 2, 2021

@remal Unfortunately there is no accepted standard for that, see https://stackoverflow.com/questions/4963300/which-notnull-java-annotation-should-i-use. So please keep an eye on Jakarta Annotations, if they ever pick up the "Walking Dead" of JSR 305 we might consider that (as we already support Jakarta Annotations even all the way down to the API) feel free to raise a new ticket, but in the meantime it makes no sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants