[GitHub] [beam] kennknowles commented on a diff in pull request #27416: Remove SuppressWarnings from NoopLock.java

["kennknowles (via GitHub)" <gi...@apache.org>]

kennknowles commented on code in PR #27416:
URL: https://github.com/apache/beam/pull/27416#discussion_r1259860269


##########
sdks/java/core/src/main/java/org/apache/beam/sdk/util/NoopLock.java:
##########
@@ -56,16 +54,15 @@ public boolean tryLock() {
   }
 
   @Override
-  public boolean tryLock(long time, @Nonnull TimeUnit unit) {
+  public boolean tryLock(long time, @NonNull TimeUnit unit) {
     return true;
   }
 
   @Override
   public void unlock() {}
 
-  @Nonnull
   @Override
-  public Condition newCondition() {
+  public @NonNull Condition newCondition() {

Review Comment:
   Does this annotation override the superclass? That is safe to do here because it is in a covariant position. I am asking only because non-null is default so it would be unnecessary unless the superclass already specified it was nullable.



##########
sdks/java/core/src/main/java/org/apache/beam/sdk/util/NoopLock.java:
##########
@@ -56,16 +54,15 @@ public boolean tryLock() {
   }
 
   @Override
-  public boolean tryLock(long time, @Nonnull TimeUnit unit) {
+  public boolean tryLock(long time, @NonNull TimeUnit unit) {

Review Comment:
   Does this annotation override the superclass? In this case it is unsafe because it is in a contravariant position. If the superclass allows null parameters, then this class must handle them somehow.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org