From f7f2607a8c773c8cf69f30c036a5f0c865f6fb10 Mon Sep 17 00:00:00 2001 From: Chris Povirk Date: Mon, 3 Aug 2020 13:35:51 -0400 Subject: [PATCH 1/2] Finish annotating some classes for nullness. Parts of this are relevant to https://github.com/typetools/checker-framework/issues/3197 Other parts are a followup to https://github.com/typetools/checker-framework/pull/3063 Merge with https://github.com/typetools/checker-framework/pull/3549 --- .../classes/java/io/InputStreamReader.java | 2 +- .../share/classes/java/util/Scanner.java | 10 +++--- .../share/classes/java/util/WeakHashMap.java | 2 +- .../util/concurrent/ArrayBlockingQueue.java | 15 ++++++--- .../util/concurrent/CopyOnWriteArrayList.java | 10 +++--- .../util/concurrent/CopyOnWriteArraySet.java | 6 ++-- .../util/concurrent/LinkedBlockingDeque.java | 33 +++++++++++-------- .../util/concurrent/LinkedBlockingQueue.java | 20 ++++++----- .../concurrent/PriorityBlockingQueue.java | 20 ++++++----- 9 files changed, 71 insertions(+), 47 deletions(-) diff --git a/src/java.base/share/classes/java/io/InputStreamReader.java b/src/java.base/share/classes/java/io/InputStreamReader.java index 32e0d34fb3dd4..3f40ed74e58a5 100644 --- a/src/java.base/share/classes/java/io/InputStreamReader.java +++ b/src/java.base/share/classes/java/io/InputStreamReader.java @@ -67,7 +67,7 @@ * @since 1.1 */ -@AnnotatedFor({"index"}) +@AnnotatedFor({"index", "nullness"}) public class InputStreamReader extends Reader { private final StreamDecoder sd; diff --git a/src/java.base/share/classes/java/util/Scanner.java b/src/java.base/share/classes/java/util/Scanner.java index 838cd6b825d24..5f97349c878b0 100644 --- a/src/java.base/share/classes/java/util/Scanner.java +++ b/src/java.base/share/classes/java/util/Scanner.java @@ -317,7 +317,7 @@ * * @since 1.5 */ -@AnnotatedFor({"index", "interning", "lock", "signedness"}) +@AnnotatedFor({"index", "interning", "lock", "nullness", "signedness"}) public final @UsesObjectEquals class Scanner implements Iterator, Closeable { // Internal buffer used to hold input @@ -1685,7 +1685,7 @@ public String nextLine(@GuardSatisfied Scanner this) { * @return the text that matched the specified pattern * @throws IllegalStateException if this scanner is closed */ - public String findInLine(String pattern) { + public @Nullable String findInLine(String pattern) { return findInLine(patternCache.forName(pattern)); } @@ -1707,7 +1707,7 @@ public String findInLine(String pattern) { * @return the text that matched the specified pattern * @throws IllegalStateException if this scanner is closed */ - public String findInLine(Pattern pattern) { + public @Nullable String findInLine(Pattern pattern) { ensureOpen(); if (pattern == null) throw new NullPointerException(); @@ -1754,7 +1754,7 @@ public String findInLine(Pattern pattern) { * @throws IllegalStateException if this scanner is closed * @throws IllegalArgumentException if horizon is negative */ - public String findWithinHorizon(String pattern, @NonNegative int horizon) { + public @Nullable String findWithinHorizon(String pattern, @NonNegative int horizon) { return findWithinHorizon(patternCache.forName(pattern), horizon); } @@ -1789,7 +1789,7 @@ public String findWithinHorizon(String pattern, @NonNegative int horizon) { * @throws IllegalStateException if this scanner is closed * @throws IllegalArgumentException if horizon is negative */ - public String findWithinHorizon(Pattern pattern, @NonNegative int horizon) { + public @Nullable String findWithinHorizon(Pattern pattern, @NonNegative int horizon) { ensureOpen(); if (pattern == null) throw new NullPointerException(); diff --git a/src/java.base/share/classes/java/util/WeakHashMap.java b/src/java.base/share/classes/java/util/WeakHashMap.java index 80bb00e2870a6..7b47a7d69fd50 100644 --- a/src/java.base/share/classes/java/util/WeakHashMap.java +++ b/src/java.base/share/classes/java/util/WeakHashMap.java @@ -145,7 +145,7 @@ * @see java.lang.ref.WeakReference */ @CFComment({"lock: permits null keys and values"}) -@AnnotatedFor({"lock", "index"}) +@AnnotatedFor({"lock", "index", "nullness"}) public class WeakHashMap extends AbstractMap implements Map { diff --git a/src/java.base/share/classes/java/util/concurrent/ArrayBlockingQueue.java b/src/java.base/share/classes/java/util/concurrent/ArrayBlockingQueue.java index 492b5ead0c52e..bda08eb9a8992 100644 --- a/src/java.base/share/classes/java/util/concurrent/ArrayBlockingQueue.java +++ b/src/java.base/share/classes/java/util/concurrent/ArrayBlockingQueue.java @@ -38,6 +38,8 @@ import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.Nullable; import org.checkerframework.dataflow.qual.Pure; +import org.checkerframework.framework.qual.AnnotatedFor; +import org.checkerframework.framework.qual.CFComment; import java.lang.ref.WeakReference; import java.util.AbstractQueue; @@ -87,7 +89,8 @@ * @author Doug Lea * @param the type of elements held in this queue */ -public class ArrayBlockingQueue extends AbstractQueue +@AnnotatedFor({"nullness"}) +public class ArrayBlockingQueue extends AbstractQueue implements BlockingQueue, java.io.Serializable { /* @@ -509,7 +512,8 @@ public int remainingCapacity() { * @param o element to be removed from this queue, if present * @return {@code true} if this queue changed as a result of the call */ - public boolean remove(@Nullable Object o) { + @CFComment("probably accepts null in practice, but docs at best imply this") + public boolean remove(Object o) { if (o == null) return false; final ReentrantLock lock = this.lock; lock.lock(); @@ -541,7 +545,8 @@ public boolean remove(@Nullable Object o) { * @param o object to be checked for containment in this queue * @return {@code true} if this queue contains the specified element */ - public boolean contains(@Nullable Object o) { + @CFComment("probably accepts null in practice, but docs at best imply this") + public boolean contains(Object o) { if (o == null) return false; final ReentrantLock lock = this.lock; lock.lock(); @@ -1478,7 +1483,7 @@ public boolean removeIf(Predicate filter) { /** * @throws NullPointerException {@inheritDoc} */ - public boolean removeAll(Collection c) { + public boolean removeAll(Collection c) { Objects.requireNonNull(c); return bulkRemove(e -> c.contains(e)); } @@ -1486,7 +1491,7 @@ public boolean removeAll(Collection c) { /** * @throws NullPointerException {@inheritDoc} */ - public boolean retainAll(Collection c) { + public boolean retainAll(Collection c) { Objects.requireNonNull(c); return bulkRemove(e -> !c.contains(e)); } diff --git a/src/java.base/share/classes/java/util/concurrent/CopyOnWriteArrayList.java b/src/java.base/share/classes/java/util/concurrent/CopyOnWriteArrayList.java index dad5eaa8bd952..bd1bd5e9f29a0 100644 --- a/src/java.base/share/classes/java/util/concurrent/CopyOnWriteArrayList.java +++ b/src/java.base/share/classes/java/util/concurrent/CopyOnWriteArrayList.java @@ -39,6 +39,7 @@ import org.checkerframework.checker.nullness.qual.Nullable; import org.checkerframework.dataflow.qual.Pure; import org.checkerframework.dataflow.qual.SideEffectFree; +import org.checkerframework.framework.qual.AnnotatedFor; import java.lang.invoke.VarHandle; import java.lang.reflect.Field; @@ -96,6 +97,7 @@ * @author Doug Lea * @param the type of elements held in this list */ +@AnnotatedFor({"nullness"}) public class CopyOnWriteArrayList implements List, RandomAccess, Cloneable, java.io.Serializable { private static final long serialVersionUID = 8673264195747942595L; @@ -262,7 +264,7 @@ public int indexOf(@Nullable Object o) { * {@code -1} if the element is not found. * @throws IndexOutOfBoundsException if the specified index is negative */ - public int indexOf(E e, int index) { + public int indexOf(@Nullable E e, int index) { Object[] es = getArray(); return indexOfRange(e, es, index, es.length); } @@ -291,7 +293,7 @@ public int lastIndexOf(@Nullable Object o) { * @throws IndexOutOfBoundsException if the specified index is greater * than or equal to the current size of this list */ - public int lastIndexOf(E e, int index) { + public int lastIndexOf(@Nullable E e, int index) { Object[] es = getArray(); return lastIndexOfRange(e, es, 0, index + 1); } @@ -658,7 +660,7 @@ public boolean containsAll(Collection c) { * or if the specified collection is null * @see #remove(Object) */ - public boolean removeAll(Collection c) { + public boolean removeAll(Collection<@Nullable ?> c) { Objects.requireNonNull(c); return bulkRemove(e -> c.contains(e)); } @@ -679,7 +681,7 @@ public boolean removeAll(Collection c) { * or if the specified collection is null * @see #remove(Object) */ - public boolean retainAll(Collection c) { + public boolean retainAll(Collection<@Nullable ?> c) { Objects.requireNonNull(c); return bulkRemove(e -> !c.contains(e)); } diff --git a/src/java.base/share/classes/java/util/concurrent/CopyOnWriteArraySet.java b/src/java.base/share/classes/java/util/concurrent/CopyOnWriteArraySet.java index 1b3c8cdd9e633..42c801242cd8b 100644 --- a/src/java.base/share/classes/java/util/concurrent/CopyOnWriteArraySet.java +++ b/src/java.base/share/classes/java/util/concurrent/CopyOnWriteArraySet.java @@ -40,6 +40,7 @@ import org.checkerframework.checker.nullness.qual.Nullable; import org.checkerframework.dataflow.qual.Pure; import org.checkerframework.dataflow.qual.SideEffectFree; +import org.checkerframework.framework.qual.AnnotatedFor; import java.util.AbstractSet; import java.util.Collection; @@ -101,6 +102,7 @@ * @author Doug Lea * @param the type of elements held in this set */ +@AnnotatedFor({"nullness"}) public class CopyOnWriteArraySet extends AbstractSet implements java.io.Serializable { private static final long serialVersionUID = 5457747651344034263L; @@ -355,7 +357,7 @@ public boolean addAll(Collection c) { * or if the specified collection is null * @see #remove(Object) */ - public boolean removeAll(Collection c) { + public boolean removeAll(Collection<@Nullable ?> c) { return al.removeAll(c); } @@ -378,7 +380,7 @@ public boolean removeAll(Collection c) { * or if the specified collection is null * @see #remove(Object) */ - public boolean retainAll(Collection c) { + public boolean retainAll(Collection<@Nullable ?> c) { return al.retainAll(c); } diff --git a/src/java.base/share/classes/java/util/concurrent/LinkedBlockingDeque.java b/src/java.base/share/classes/java/util/concurrent/LinkedBlockingDeque.java index 6026ebf9d3fb3..530b13636a66e 100644 --- a/src/java.base/share/classes/java/util/concurrent/LinkedBlockingDeque.java +++ b/src/java.base/share/classes/java/util/concurrent/LinkedBlockingDeque.java @@ -38,6 +38,8 @@ import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.Nullable; import org.checkerframework.dataflow.qual.Pure; +import org.checkerframework.framework.qual.AnnotatedFor; +import org.checkerframework.framework.qual.CFComment; import java.util.AbstractQueue; import java.util.Collection; @@ -79,7 +81,8 @@ * @author Doug Lea * @param the type of elements held in this deque */ -public class LinkedBlockingDeque +@AnnotatedFor({"nullness"}) +public class LinkedBlockingDeque extends AbstractQueue implements BlockingDeque, java.io.Serializable { @@ -458,7 +461,7 @@ public E removeLast() { return x; } - public E pollFirst() { + public @Nullable E pollFirst() { final ReentrantLock lock = this.lock; lock.lock(); try { @@ -468,7 +471,7 @@ public E pollFirst() { } } - public E pollLast() { + public @Nullable E pollLast() { final ReentrantLock lock = this.lock; lock.lock(); try { @@ -504,7 +507,7 @@ public E takeLast() throws InterruptedException { } } - public E pollFirst(long timeout, TimeUnit unit) + public @Nullable E pollFirst(long timeout, TimeUnit unit) throws InterruptedException { long nanos = unit.toNanos(timeout); final ReentrantLock lock = this.lock; @@ -522,7 +525,7 @@ public E pollFirst(long timeout, TimeUnit unit) } } - public E pollLast(long timeout, TimeUnit unit) + public @Nullable E pollLast(long timeout, TimeUnit unit) throws InterruptedException { long nanos = unit.toNanos(timeout); final ReentrantLock lock = this.lock; @@ -558,7 +561,7 @@ public E getLast() { return x; } - public E peekFirst() { + public @Nullable E peekFirst() { final ReentrantLock lock = this.lock; lock.lock(); try { @@ -568,7 +571,7 @@ public E peekFirst() { } } - public E peekLast() { + public @Nullable E peekLast() { final ReentrantLock lock = this.lock; lock.lock(); try { @@ -578,6 +581,7 @@ public E peekLast() { } } + @CFComment("probably accepts null in practice, but docs at best imply this") public boolean removeFirstOccurrence(Object o) { if (o == null) return false; final ReentrantLock lock = this.lock; @@ -595,6 +599,7 @@ public boolean removeFirstOccurrence(Object o) { } } + @CFComment("probably accepts null in practice, but docs at best imply this") public boolean removeLastOccurrence(Object o) { if (o == null) return false; final ReentrantLock lock = this.lock; @@ -667,7 +672,7 @@ public E remove() { return removeFirst(); } - public E poll() { + public @Nullable E poll() { return pollFirst(); } @@ -675,7 +680,7 @@ public E take() throws InterruptedException { return takeFirst(); } - public E poll(long timeout, TimeUnit unit) throws InterruptedException { + public @Nullable E poll(long timeout, TimeUnit unit) throws InterruptedException { return pollFirst(timeout, unit); } @@ -693,7 +698,7 @@ public E element() { return getFirst(); } - public E peek() { + public @Nullable E peek() { return peekFirst(); } @@ -787,6 +792,7 @@ public E pop() { * @param o element to be removed from this deque, if present * @return {@code true} if this deque changed as a result of the call */ + @CFComment("probably accepts null in practice, but docs at best imply this") public boolean remove(Object o) { return removeFirstOccurrence(o); } @@ -815,7 +821,8 @@ public int size() { * @param o object to be checked for containment in this deque * @return {@code true} if this deque contains the specified element */ - public boolean contains(@Nullable Object o) { + @CFComment("probably accepts null in practice, but docs at best imply this") + public boolean contains(Object o) { if (o == null) return false; final ReentrantLock lock = this.lock; lock.lock(); @@ -1341,7 +1348,7 @@ public boolean removeIf(Predicate filter) { /** * @throws NullPointerException {@inheritDoc} */ - public boolean removeAll(Collection c) { + public boolean removeAll(Collection c) { Objects.requireNonNull(c); return bulkRemove(e -> c.contains(e)); } @@ -1349,7 +1356,7 @@ public boolean removeAll(Collection c) { /** * @throws NullPointerException {@inheritDoc} */ - public boolean retainAll(Collection c) { + public boolean retainAll(Collection c) { Objects.requireNonNull(c); return bulkRemove(e -> !c.contains(e)); } diff --git a/src/java.base/share/classes/java/util/concurrent/LinkedBlockingQueue.java b/src/java.base/share/classes/java/util/concurrent/LinkedBlockingQueue.java index 4fda5c829f03c..e3f478a75c464 100644 --- a/src/java.base/share/classes/java/util/concurrent/LinkedBlockingQueue.java +++ b/src/java.base/share/classes/java/util/concurrent/LinkedBlockingQueue.java @@ -38,6 +38,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.Nullable; import org.checkerframework.dataflow.qual.Pure; +import org.checkerframework.framework.qual.AnnotatedFor; import java.util.AbstractQueue; import java.util.Collection; @@ -82,7 +83,8 @@ * @author Doug Lea * @param the type of elements held in this queue */ -public class LinkedBlockingQueue extends AbstractQueue +@AnnotatedFor({"nullness"}) +public class LinkedBlockingQueue extends AbstractQueue implements BlockingQueue, java.io.Serializable { private static final long serialVersionUID = -6903933977591709194L; @@ -449,7 +451,7 @@ public E take() throws InterruptedException { return x; } - public E poll(long timeout, TimeUnit unit) throws InterruptedException { + public @Nullable E poll(long timeout, TimeUnit unit) throws InterruptedException { final E x; final int c; long nanos = unit.toNanos(timeout); @@ -474,7 +476,7 @@ public E poll(long timeout, TimeUnit unit) throws InterruptedException { return x; } - public E poll() { + public @Nullable E poll() { final AtomicInteger count = this.count; if (count.get() == 0) return null; @@ -497,7 +499,7 @@ public E poll() { return x; } - public E peek() { + public @Nullable E peek() { final AtomicInteger count = this.count; if (count.get() == 0) return null; @@ -537,7 +539,8 @@ void unlink(Node p, Node pred) { * @param o element to be removed from this queue, if present * @return {@code true} if this queue changed as a result of the call */ - public boolean remove(@Nullable Object o) { + @CFComment("probably accepts null in practice, but docs for remove at best imply this") + public boolean remove(Object o) { if (o == null) return false; fullyLock(); try { @@ -563,7 +566,8 @@ public boolean remove(@Nullable Object o) { * @param o object to be checked for containment in this queue * @return {@code true} if this queue contains the specified element */ - public boolean contains(@Nullable Object o) { + @CFComment("probably accepts null in practice, but docs for remove at best imply this") + public boolean contains(Object o) { if (o == null) return false; fullyLock(); try { @@ -1028,7 +1032,7 @@ public boolean removeIf(Predicate filter) { /** * @throws NullPointerException {@inheritDoc} */ - public boolean removeAll(Collection c) { + public boolean removeAll(Collection c) { Objects.requireNonNull(c); return bulkRemove(e -> c.contains(e)); } @@ -1036,7 +1040,7 @@ public boolean removeAll(Collection c) { /** * @throws NullPointerException {@inheritDoc} */ - public boolean retainAll(Collection c) { + public boolean retainAll(Collection c) { Objects.requireNonNull(c); return bulkRemove(e -> !c.contains(e)); } diff --git a/src/java.base/share/classes/java/util/concurrent/PriorityBlockingQueue.java b/src/java.base/share/classes/java/util/concurrent/PriorityBlockingQueue.java index 0a4b08faf3ae5..58d865f908bf9 100644 --- a/src/java.base/share/classes/java/util/concurrent/PriorityBlockingQueue.java +++ b/src/java.base/share/classes/java/util/concurrent/PriorityBlockingQueue.java @@ -38,6 +38,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.Nullable; import org.checkerframework.dataflow.qual.Pure; +import org.checkerframework.framework.qual.AnnotatedFor; import java.lang.invoke.MethodHandles; import java.lang.invoke.VarHandle; @@ -114,8 +115,9 @@ * @author Doug Lea * @param the type of elements held in this queue */ +@AnnotatedFor({"nullness"}) @SuppressWarnings("unchecked") -public class PriorityBlockingQueue extends AbstractQueue +public class PriorityBlockingQueue extends AbstractQueue implements BlockingQueue, java.io.Serializable { private static final long serialVersionUID = 5595510919245408276L; @@ -532,7 +534,7 @@ public boolean offer(E e, long timeout, TimeUnit unit) { return offer(e); // never need to block } - public E poll() { + public @Nullable E poll() { final ReentrantLock lock = this.lock; lock.lock(); try { @@ -555,7 +557,7 @@ public E take() throws InterruptedException { return result; } - public E poll(long timeout, TimeUnit unit) throws InterruptedException { + public @Nullable E poll(long timeout, TimeUnit unit) throws InterruptedException { long nanos = unit.toNanos(timeout); final ReentrantLock lock = this.lock; lock.lockInterruptibly(); @@ -569,7 +571,7 @@ public E poll(long timeout, TimeUnit unit) throws InterruptedException { return result; } - public E peek() { + public @Nullable E peek() { final ReentrantLock lock = this.lock; lock.lock(); try { @@ -659,7 +661,8 @@ private void removeAt(int i) { * @param o element to be removed from this queue, if present * @return {@code true} if this queue changed as a result of the call */ - public boolean remove(@Nullable Object o) { + @CFComment("probably accepts null in practice, but docs at best imply this") + public boolean remove(Object o) { final ReentrantLock lock = this.lock; lock.lock(); try { @@ -702,7 +705,8 @@ void removeEq(Object o) { * @param o object to be checked for containment in this queue * @return {@code true} if this queue contains the specified element */ - public boolean contains(@Nullable Object o) { + @CFComment("probably accepts null in practice, but docs at best imply this") + public boolean contains(Object o) { final ReentrantLock lock = this.lock; lock.lock(); try { @@ -1033,7 +1037,7 @@ public boolean removeIf(Predicate filter) { /** * @throws NullPointerException {@inheritDoc} */ - public boolean removeAll(Collection c) { + public boolean removeAll(Collection c) { Objects.requireNonNull(c); return bulkRemove(e -> c.contains(e)); } @@ -1041,7 +1045,7 @@ public boolean removeAll(Collection c) { /** * @throws NullPointerException {@inheritDoc} */ - public boolean retainAll(Collection c) { + public boolean retainAll(Collection c) { Objects.requireNonNull(c); return bulkRemove(e -> !c.contains(e)); } From 0d42c9a821ae9d7fea125bb9d5c68960c73edeb0 Mon Sep 17 00:00:00 2001 From: Chris Povirk Date: Mon, 3 Aug 2020 14:30:39 -0400 Subject: [PATCH 2/2] Add missing imports. --- .../share/classes/java/util/concurrent/LinkedBlockingQueue.java | 1 + .../classes/java/util/concurrent/PriorityBlockingQueue.java | 1 + 2 files changed, 2 insertions(+) diff --git a/src/java.base/share/classes/java/util/concurrent/LinkedBlockingQueue.java b/src/java.base/share/classes/java/util/concurrent/LinkedBlockingQueue.java index e3f478a75c464..2302d5a055f3a 100644 --- a/src/java.base/share/classes/java/util/concurrent/LinkedBlockingQueue.java +++ b/src/java.base/share/classes/java/util/concurrent/LinkedBlockingQueue.java @@ -39,6 +39,7 @@ import org.checkerframework.checker.nullness.qual.Nullable; import org.checkerframework.dataflow.qual.Pure; import org.checkerframework.framework.qual.AnnotatedFor; +import org.checkerframework.framework.qual.CFComment; import java.util.AbstractQueue; import java.util.Collection; diff --git a/src/java.base/share/classes/java/util/concurrent/PriorityBlockingQueue.java b/src/java.base/share/classes/java/util/concurrent/PriorityBlockingQueue.java index 58d865f908bf9..9cc0a46343381 100644 --- a/src/java.base/share/classes/java/util/concurrent/PriorityBlockingQueue.java +++ b/src/java.base/share/classes/java/util/concurrent/PriorityBlockingQueue.java @@ -39,6 +39,7 @@ import org.checkerframework.checker.nullness.qual.Nullable; import org.checkerframework.dataflow.qual.Pure; import org.checkerframework.framework.qual.AnnotatedFor; +import org.checkerframework.framework.qual.CFComment; import java.lang.invoke.MethodHandles; import java.lang.invoke.VarHandle;