use jspecify annotations #13822
use jspecify annotations #13822
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #13822 +/- ##
============================================
+ Coverage 61.75% 61.93% +0.18%
+ Complexity 207 198 -9
============================================
Files 2436 2558 +122
Lines 133233 141224 +7991
Branches 20636 21977 +1341
============================================
+ Hits 82274 87471 +5197
- Misses 44911 47089 +2178
- Partials 6048 6664 +616
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -213,6 +213,7 @@ | |||
| <javax.servlet-api.version>4.0.1</javax.servlet-api.version> | |||
| <jakarta.annotation-api.version>1.3.5</jakarta.annotation-api.version> | |||
| <javax.annotation-api.version>1.3.2</javax.annotation-api.version> | |||
| <jspecify.version>1.0.0</jspecify.version> | |||
There was a problem hiding this comment.
Is this a Java EE libarary? If not, let's keep it separately for easier management
| <dependency> | ||
| <groupId>org.jspecify</groupId> | ||
| <artifactId>jspecify</artifactId> | ||
| </dependency> |
There was a problem hiding this comment.
(minor) We shouldn't need it as it is already included in pinot-spi
| @@ -18,14 +18,14 @@ | |||
| */ | |||
| package org.apache.pinot.common.data; | |||
|
|
|||
| import javax.annotation.Nonnull; | |||
| import org.jspecify.annotations.NonNull; | |||
There was a problem hiding this comment.
We are trying to follow the convention of only annotating the nullable fields, and treat the fields not annotated as non-null. Since you are fixing all annotations, let's remove the Nonnull ones.
Context
https://jspecify.dev/docs/start-here/
https://jspecify.dev/docs/user-guide/
https://jspecify.dev/blog/release-1.0.0/
https://www.infoq.com/news/2024/08/jspecify-java-nullability/