Sound or unsound treatment of collection Object parameters; fixes #3040

changelog.txt

@@ -4,6 +4,11 @@ Renamings:
   -AuseDefaultsForUncheckedCode to -AuseConservativeDefaultsForUncheckedCode
     The old name works temporarily but will be removed in a future release.
 
+For collection methods with `Object` formal parameter type, such as
+contains, indexOf, and remove, the annotated JDK now forbids null as an
+argument.  To make the Nullness Checker permit null, pass
+`-Astubs=checker.jar/collection-object-parameters-may-be-null.astub`.
+
 Implementation details:
  * Removed `@DefaultInUncheckedCodeFor` and
    `@DefaultQualifierInHierarchyInUncheckedCode`.

checker/build.gradle

@@ -2,7 +2,7 @@ ext {
     // See instructions for updating jdk8.jar at
     // https://github.com/typetools/annotated-libraries/blob/master/README.md .
     // Set jdkShaHash to 'local' to use a locally-built version of jdk8.jar .
-    jdkShaHash = '03c7d2f752fde145ecc1f5232cae0ae9c22019d4'
+    jdkShaHash = '672035d415d27dbdebc6ddf8ab4a6c519f65d49a'
     if (rootProject.hasProperty("useLocalJdk")) {
         jdkShaHash = 'local'
     }

checker/jdk/nullness/src/java/util/AbstractCollection.java

@@ -1,4 +1,6 @@
 package java.util;
+
+import org.checkerframework.checker.nullness.qual.NonNull;
 import org.checkerframework.checker.nullness.qual.Nullable;
 import org.checkerframework.checker.nullness.qual.PolyNull;
 import org.checkerframework.dataflow.qual.Pure;
@@ -17,23 +19,23 @@ protected AbstractCollection() {}
   @Override
   @Pure public boolean isEmpty() { throw new RuntimeException("skeleton method"); }
   @Override
-  @Pure public boolean contains(@Nullable Object a1) { throw new RuntimeException("skeleton method"); }
+  @Pure public boolean contains(Object a1) { throw new RuntimeException("skeleton method"); }
   @Override
   @SideEffectFree public Object [] toArray() { throw new RuntimeException("skeleton method"); }
   @Override
   @SideEffectFree public <T> @Nullable T @PolyNull [] toArray(@Nullable T @PolyNull [] a1) { throw new RuntimeException("skeleton method"); }
   @Override
   public boolean add(E a1) { throw new RuntimeException("skeleton method"); }
   @Override
-  public boolean remove(@Nullable Object a1) { throw new RuntimeException("skeleton method"); }
+  public boolean remove(Object a1) { throw new RuntimeException("skeleton method"); }
   @Override
-  @Pure public boolean containsAll(Collection<?> a1) { throw new RuntimeException("skeleton method"); }
+  @Pure public boolean containsAll(Collection<? extends @NonNull Object> c) { throw new RuntimeException("skeleton method"); }
   @Override
   public boolean addAll(Collection<? extends E> a1) { throw new RuntimeException("skeleton method"); }
   @Override
-  public boolean removeAll(Collection<?> a1) { throw new RuntimeException("skeleton method"); }
+  public boolean removeAll(Collection<? extends @NonNull Object> c) { throw new RuntimeException("skeleton method"); }
   @Override
-  public boolean retainAll(Collection<?> a1) { throw new RuntimeException("skeleton method"); }
+  public boolean retainAll(Collection<? extends @NonNull Object> c) { throw new RuntimeException("skeleton method"); }
   @Override
   public void clear() { throw new RuntimeException("skeleton method"); }
   @Override

checker/jdk/nullness/src/java/util/AbstractList.java

@@ -181,7 +181,7 @@ public E remove(int index) {
      * @throws NullPointerException {@inheritDoc}
      */
     @Pure
-    public int indexOf(@Nullable Object o) {
+    public int indexOf(Object o) {
         ListIterator<E> it = listIterator();
         if (o==null) {
             while (it.hasNext())
@@ -207,7 +207,7 @@ public int indexOf(@Nullable Object o) {
      * @throws NullPointerException {@inheritDoc}
      */
     @Pure
-    public int lastIndexOf(@Nullable Object o) {
+    public int lastIndexOf(Object o) {
         ListIterator<E> it = listIterator(size());
         if (o==null) {
             while (it.hasPrevious())

checker/jdk/nullness/src/java/util/AbstractMap.java

@@ -119,7 +119,7 @@ public boolean isEmpty() {
      * @throws NullPointerException {@inheritDoc}
      */
     @Pure
-    public boolean containsValue(@Nullable Object value) {
+    public boolean containsValue(Object value) {
         Iterator<Entry<K,V>> i = entrySet().iterator();
         if (value==null) {
             while (i.hasNext()) {
@@ -153,7 +153,7 @@ public boolean containsValue(@Nullable Object value) {
      */
     @Pure
     @EnsuresKeyForIf(result=true, expression="#1", map="this")
-    public boolean containsKey(@Nullable Object key) {
+    public boolean containsKey(Object key) {
         Iterator<Map.Entry<K,V>> i = entrySet().iterator();
         if (key==null) {
             while (i.hasNext()) {
@@ -186,7 +186,7 @@ public boolean containsKey(@Nullable Object key) {
      * @throws NullPointerException          {@inheritDoc}
      */
     @Pure
-    public @Nullable V get(@Nullable Object key) {
+    public @Nullable V get(Object key) {
         Iterator<Entry<K,V>> i = entrySet().iterator();
         if (key==null) {
             while (i.hasNext()) {
@@ -246,7 +246,7 @@ public boolean containsKey(@Nullable Object key) {
      * @throws ClassCastException            {@inheritDoc}
      * @throws NullPointerException          {@inheritDoc}
      */
-    public @Nullable V remove(@Nullable Object key) {
+    public @Nullable V remove(Object key) {
         Iterator<Entry<K,V>> i = entrySet().iterator();
         Entry<K,V> correctEntry = null;
         if (key==null) {

checker/jdk/nullness/src/java/util/AbstractSet.java

@@ -1,11 +1,13 @@
 package java.util;
-import org.checkerframework.dataflow.qual.Pure;
+
+import org.checkerframework.checker.nullness.qual.NonNull;
 import org.checkerframework.checker.nullness.qual.Nullable;
+import org.checkerframework.dataflow.qual.Pure;
 
 // Subclasses of this interface/class may opt to prohibit null elements
 public abstract class AbstractSet<E> extends AbstractCollection<E> implements Set<E> {
   protected AbstractSet() {}
   @Pure public boolean equals(@Nullable Object a1) { throw new RuntimeException("skeleton method"); }
   @Pure public int hashCode() { throw new RuntimeException("skeleton method"); }
-  public boolean removeAll(Collection<?> a1) { throw new RuntimeException("skeleton method"); }
+  public boolean removeAll(Collection<? extends @NonNull Object> a1) { throw new RuntimeException("skeleton method"); }
 }

checker/jdk/nullness/src/java/util/ArrayDeque.java

@@ -21,8 +21,8 @@ public class ArrayDeque<E extends @NonNull Object> extends AbstractCollection<E>
   public E getLast() { throw new RuntimeException("skeleton method"); }
   public @Nullable E peekFirst() { throw new RuntimeException("skeleton method"); }
   public @Nullable E peekLast() { throw new RuntimeException("skeleton method"); }
-  public boolean removeFirstOccurrence(Object a1) { throw new RuntimeException("skeleton method"); }
-  public boolean removeLastOccurrence(Object a1) { throw new RuntimeException("skeleton method"); }
+  public boolean removeFirstOccurrence(@Nullable Object a1) { throw new RuntimeException("skeleton method"); }
+  public boolean removeLastOccurrence(@Nullable Object a1) { throw new RuntimeException("skeleton method"); }
   public boolean add(E a1) { throw new RuntimeException("skeleton method"); }
   public boolean offer(E a1) { throw new RuntimeException("skeleton method"); }
   public E remove() { throw new RuntimeException("skeleton method"); }

checker/jdk/nullness/src/java/util/Collection.java

@@ -29,6 +29,7 @@
 import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
 
+import org.checkerframework.checker.nullness.qual.NonNull;
 import org.checkerframework.checker.nullness.qual.Nullable;
 import org.checkerframework.checker.nullness.qual.PolyNull;
 import org.checkerframework.dataflow.qual.Pure;
@@ -185,7 +186,7 @@ public interface Collection<E> extends Iterable<E> {
      *         (<a href="#optional-restrictions">optional</a>)
      */
     @Pure
-    boolean contains(@Nullable Object o);
+    boolean contains(Object o);
 
     /**
      * Returns an iterator over the elements in this collection.  There are no
@@ -323,7 +324,7 @@ public interface Collection<E> extends Iterable<E> {
      * @throws UnsupportedOperationException if the <tt>remove</tt> operation
      *         is not supported by this collection
      */
-    boolean remove(@Nullable Object o);
+    boolean remove(Object o);
 
 
     // Bulk Operations
@@ -347,7 +348,7 @@ public interface Collection<E> extends Iterable<E> {
      * @see    #contains(Object)
      */
     @Pure
-    boolean containsAll(Collection<?> c);
+    boolean containsAll(Collection<? extends @NonNull Object> c);
 
     /**
      * Adds all of the elements in the specified collection to this collection
@@ -398,7 +399,7 @@ public interface Collection<E> extends Iterable<E> {
      * @see #remove(Object)
      * @see #contains(Object)
      */
-    boolean removeAll(Collection<?> c);
+    boolean removeAll(Collection<? extends @NonNull Object> c);
 
     /**
      * Removes all of the elements of this collection that satisfy the given
@@ -457,7 +458,7 @@ default boolean removeIf(Predicate<? super E> filter) {
      * @see #remove(Object)
      * @see #contains(Object)
      */
-    boolean retainAll(Collection<?> c);
+    boolean retainAll(Collection<? extends @NonNull Object> c);
 
     /**
      * Removes all of the elements from this collection (optional operation).

checker/jdk/nullness/src/java/util/Collections.java

@@ -39,6 +39,7 @@
 import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
 
+import org.checkerframework.checker.nullness.qual.NonNull;
 import org.checkerframework.checker.nullness.qual.EnsuresKeyFor;
 import org.checkerframework.checker.nullness.qual.EnsuresKeyForIf;
 import org.checkerframework.checker.nullness.qual.Nullable;
@@ -1133,16 +1134,16 @@ public boolean remove(Object o) {
             throw new UnsupportedOperationException();
         }
 
-        public boolean containsAll(Collection<?> coll) {
+        public boolean containsAll(Collection<? extends @NonNull Object> coll) {
             return c.containsAll(coll);
         }
         public boolean addAll(Collection<? extends E> coll) {
             throw new UnsupportedOperationException();
         }
-        public boolean removeAll(Collection<?> coll) {
+        public boolean removeAll(Collection<? extends @NonNull Object> coll) {
             throw new UnsupportedOperationException();
         }
-        public boolean retainAll(Collection<?> coll) {
+        public boolean retainAll(Collection<? extends @NonNull Object> coll) {
             throw new UnsupportedOperationException();
         }
         public void clear() {
@@ -1799,7 +1800,7 @@ public boolean contains(Object o) {
              * an unscrupulous List whose contains(Object o) method senses
              * when o is a Map.Entry, and calls o.setValue.
              */
-            public boolean containsAll(Collection<?> coll) {
+            public boolean containsAll(Collection<? extends @NonNull Object> coll) {
                 for (Object e : coll) {
                     if (!contains(e)) // Invokes safe contains() above
                         return false;
@@ -2141,16 +2142,16 @@ public boolean remove(Object o) {
             synchronized (mutex) {return c.remove(o);}
         }
 
-        public boolean containsAll(Collection<?> coll) {
+        public boolean containsAll(Collection<? extends @NonNull Object> coll) {
             synchronized (mutex) {return c.containsAll(coll);}
         }
         public boolean addAll(Collection<? extends E> coll) {
             synchronized (mutex) {return c.addAll(coll);}
         }
-        public boolean removeAll(Collection<?> coll) {
+        public boolean removeAll(Collection<? extends @NonNull Object> coll) {
             synchronized (mutex) {return c.removeAll(coll);}
         }
-        public boolean retainAll(Collection<?> coll) {
+        public boolean retainAll(Collection<? extends @NonNull Object> coll) {
             synchronized (mutex) {return c.retainAll(coll);}
         }
         public void clear() {
@@ -3185,13 +3186,13 @@ private String badElementMsg(Object o) {
         public boolean remove(Object o)   { return c.remove(o); }
         public void clear()               {        c.clear(); }
 
-        public boolean containsAll(Collection<?> coll) {
+        public boolean containsAll(Collection<? extends @NonNull Object> coll) {
             return c.containsAll(coll);
         }
-        public boolean removeAll(Collection<?> coll) {
+        public boolean removeAll(Collection<? extends @NonNull Object> coll) {
             return c.removeAll(coll);
         }
-        public boolean retainAll(Collection<?> coll) {
+        public boolean retainAll(Collection<? extends @NonNull Object> coll) {
             return c.retainAll(coll);
         }
 
@@ -3963,7 +3964,7 @@ public boolean contains(Object o) {
              * against an unscrupulous collection whose contains(Object o)
              * method senses when o is a Map.Entry, and calls o.setValue.
              */
-            public boolean containsAll(Collection<?> c) {
+            public boolean containsAll(Collection<? extends @NonNull Object> c) {
                 for (Object o : c)
                     if (!contains(o)) // Invokes safe contains() above
                         return false;
@@ -3977,13 +3978,13 @@ public boolean remove(Object o) {
                                 <>((Map.Entry<?,?>)o));
             }
 
-            public boolean removeAll(Collection<?> c) {
+            public boolean removeAll(Collection<? extends @NonNull Object> c) {
                 return batchRemove(c, false);
             }
-            public boolean retainAll(Collection<?> c) {
+            public boolean retainAll(Collection<? extends @NonNull Object> c) {
                 return batchRemove(c, true);
             }
-            private boolean batchRemove(Collection<?> c, boolean complement) {
+            private boolean batchRemove(Collection<? extends @NonNull Object> c, boolean complement) {
                 Objects.requireNonNull(c);
                 boolean modified = false;
                 Iterator<Map.Entry<K,V>> it = iterator();
@@ -4477,7 +4478,7 @@ private static class EmptySet<E>
         public boolean isEmpty() {return true;}
 
         public boolean contains(Object obj) {return false;}
-        public boolean containsAll(Collection<?> c) { return c.isEmpty(); }
+        public boolean containsAll(Collection<? extends @NonNull Object> c) { return c.isEmpty(); }
 
         @SideEffectFree
         public @PolyNull Object[] toArray(EmptySet<@PolyNull E> this) { return new Object[0]; }
@@ -4604,7 +4605,7 @@ public ListIterator<E> listIterator() {
         public boolean isEmpty() {return true;}
 
         public boolean contains(Object obj) {return false;}
-        public boolean containsAll(Collection<?> c) { return c.isEmpty(); }
+        public boolean containsAll(Collection<? extends @NonNull Object> c) { return c.isEmpty(); }
 
         @SideEffectFree
         public @PolyNull Object[] toArray(EmptyList<@PolyNull E> this) { return new Object[0]; }
@@ -5661,9 +5662,9 @@ private static class SetFromMap<E> extends AbstractSet<E>
         public String toString()          { return s.toString(); }
         public int hashCode()             { return s.hashCode(); }
         public boolean equals(@Nullable Object o)   { return o == this || s.equals(o); }
-        public boolean containsAll(Collection<?> c) {return s.containsAll(c);}
-        public boolean removeAll(Collection<?> c)   {return s.removeAll(c);}
-        public boolean retainAll(Collection<?> c)   {return s.retainAll(c);}
+        public boolean containsAll(Collection<? extends @NonNull Object> c) {return s.containsAll(c);}
+        public boolean removeAll(Collection<? extends @NonNull Object> c)   {return s.removeAll(c);}
+        public boolean retainAll(Collection<? extends @NonNull Object> c)   {return s.retainAll(c);}
         // addAll is the only inherited implementation
 
         // Override default methods in Collection
@@ -5744,9 +5745,9 @@ static class AsLIFOQueue<E> extends AbstractQueue<E>
         @SideEffectFree
         public <T> T[] toArray(T[] a)     { return q.toArray(a); }
         public String toString()          { return q.toString(); }
-        public boolean containsAll(Collection<?> c) {return q.containsAll(c);}
-        public boolean removeAll(Collection<?> c)   {return q.removeAll(c);}
-        public boolean retainAll(Collection<?> c)   {return q.retainAll(c);}
+        public boolean containsAll(Collection<? extends @NonNull Object> c) {return q.containsAll(c);}
+        public boolean removeAll(Collection<? extends @NonNull Object> c)   {return q.removeAll(c);}
+        public boolean retainAll(Collection<? extends @NonNull Object> c)   {return q.retainAll(c);}
         // We use inherited addAll; forwarding addAll would be wrong
 
         // Override default methods in Collection

checker/jdk/nullness/src/java/util/Deque.java

@@ -30,8 +30,8 @@ public interface Deque<E> extends Queue<E> {
   public abstract @Nullable E peek();
   public abstract void push(E a1);
   public abstract E pop();
-  public abstract boolean remove(@Nullable Object a1);
-  @Pure public abstract boolean contains(@Nullable Object a1);
+  public abstract boolean remove(Object a1);
+  @Pure public abstract boolean contains(Object a1);
   @Pure public abstract int size();
   @SideEffectFree
   public abstract Iterator<E> iterator();

checker/jdk/nullness/src/java/util/Dictionary.java

@@ -11,7 +11,7 @@ public abstract class Dictionary<K, V> {
   @Pure public abstract boolean isEmpty();
   public abstract Enumeration<K> keys();
   public abstract Enumeration<V> elements();
-  @Pure public abstract @Nullable V get(@Nullable Object a1);
+  @Pure public abstract @Nullable V get(Object a1);
   @EnsuresKeyFor(value="#1", map="this")
   public abstract @Nullable V put(K a1, V a2);
   public abstract @Nullable V remove(Object a1);