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.

Sign up for GitHub

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

Remove SuppressWarnings from NoopLock.java #27416

Merged
merged 2 commits into from
Jul 20, 2023
Merged

Remove SuppressWarnings from NoopLock.java #27416

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4299113
Remove SuppressWarnings from NoopLock.java
damondouglas Jul 9, 2023
481324c
Remove Nonnull annotations from original
damondouglas Jul 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions sdks/java/core/src/main/java/org/apache/beam/sdk/util/NoopLock.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,19 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.Condition;
import java.util.concurrent.locks.Lock;
import javax.annotation.Nonnull;
import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
import org.checkerframework.checker.nullness.qual.NonNull;

/**
* A lock which can always be acquired. It should not be used when a proper lock is required, but it
* is useful as a performance optimization when locking is not necessary but the code paths have to
* be shared between the locking and the non-locking variant.
*/
@SuppressWarnings({
"nullness" // TODO(https://github.com/apache/beam/issues/20497)
})
public class NoopLock implements Lock, Serializable {

private static NoopLock instance;
private static @MonotonicNonNull NoopLock instance;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kennknowles I chose @MonotonicNonNull here as per the JavaDoc it "Indicates that once the field (or variable) becomes non-null, it never becomes null again." I preferred this over the original @Nullable as the static get method along with the private constructor satisfies this condition.


public static NoopLock get() {
public static @NonNull NoopLock get() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The @NonNull annotation seemed appropriate here given that the returned instance, if null, will always be instantiated.

if (instance == null) {
instance = new NoopLock();
}
Expand All @@ -56,14 +54,13 @@ public boolean tryLock() {
}

@Override
public boolean tryLock(long time, @Nonnull TimeUnit unit) {
public boolean tryLock(long time, TimeUnit unit) {
return true;
}

@Override
public void unlock() {}

@Nonnull
@Override
public Condition newCondition() {
throw new UnsupportedOperationException("Not implemented");
Expand Down