Improved generic types and checker framework conformance (#337)

Additional wildcards are used throughput the APIs to more flexibly
accept parameters. For example this allows a wider range of method
references to be used as load functions.

The generic types now match the Checker Framework's rules [1]. This
should improve usage of the cache apis in projects that run the checker.
This project does not and its implementation classes are not compliant
for the nullness checker.

[1] https://checkerframework.org/manual/#generics-instantiation

build.gradle

@@ -1,6 +1,5 @@
 import net.ltgt.gradle.errorprone.CheckSeverity
 
-apply plugin: 'com.github.mjdetullio.gradle.coverity'
 apply plugin: 'com.github.ben-manes.versions'
 apply plugin: 'com.github.kt3k.coveralls'
 apply plugin: 'jacoco'

caffeine/src/jmh/java/com/github/benmanes/caffeine/cache/BasicCache.java

@@ -15,14 +15,15 @@
  */
 package com.github.benmanes.caffeine.cache;
 
+import org.checkerframework.checker.nullness.qual.NonNull;
 import org.checkerframework.checker.nullness.qual.Nullable;
 
 /**
  * A facade for benchmark implementations.
  *
  * @author [email protected] (Ben Manes)
  */
-public interface BasicCache<K, V> {
+public interface BasicCache<K extends @NonNull Object, V extends @NonNull Object> {
 
   /** Returns the value stored in the cache, or null if not present. */
   @Nullable V get(K key);

caffeine/src/main/java/com/github/benmanes/caffeine/cache/AsyncCache.java

@@ -23,6 +23,7 @@
 import java.util.function.BiFunction;
 import java.util.function.Function;
 
+import org.checkerframework.checker.nullness.qual.NonNull;
 import org.checkerframework.checker.nullness.qual.Nullable;
 
 /**
@@ -37,7 +38,7 @@
  * @param <K> the type of keys maintained by this cache
  * @param <V> the type of mapped values
  */
-public interface AsyncCache<K, V> {
+public interface AsyncCache<K extends @NonNull Object, V extends @NonNull Object> {
 
   /**
    * Returns the future associated with {@code key} in this cache, or {@code null} if there is no
@@ -92,8 +93,8 @@ public interface AsyncCache<K, V> {
    * @throws RuntimeException or Error if the mappingFunction does when constructing the future,
    *         in which case the mapping is left unestablished
    */
-  CompletableFuture<V> get(K key,
-      BiFunction<? super K, Executor, CompletableFuture<V>> mappingFunction);
+  CompletableFuture<V> get(K key, BiFunction<? super K, ? super Executor,
+      ? extends CompletableFuture<? extends V>> mappingFunction);
 
   /**
    * Returns the future of a map of the values associated with {@code keys}, creating or retrieving
@@ -119,7 +120,7 @@ CompletableFuture<V> get(K key,
    *         left unestablished
    */
   CompletableFuture<Map<K, V>> getAll(Iterable<? extends K> keys,
-      Function<Set<? extends K>, Map<K, V>> mappingFunction);
+      Function<? super Set<? extends K>, ? extends Map<? extends K, ? extends V>> mappingFunction);
 
   /**
    * Returns the future of a map of the values associated with {@code keys}, creating or retrieving
@@ -145,7 +146,8 @@ CompletableFuture<Map<K, V>> getAll(Iterable<? extends K> keys,
    *         left unestablished
    */
   CompletableFuture<Map<K, V>> getAll(Iterable<? extends K> keys,
-      BiFunction<Set<? extends K>, Executor, CompletableFuture<Map<K, V>>> mappingFunction);
+      BiFunction<? super Set<? extends K>, ? super Executor,
+          ? extends CompletableFuture<? extends Map<? extends K, ? extends V>>> mappingFunction);
 
   /**
    * Associates {@code value} with {@code key} in this cache. If the cache previously contained a
@@ -159,7 +161,7 @@ CompletableFuture<Map<K, V>> getAll(Iterable<? extends K> keys,
    * @param valueFuture value to be associated with the specified key
    * @throws NullPointerException if the specified key or value is null
    */
-  void put(K key, CompletableFuture<V> valueFuture);
+  void put(K key, CompletableFuture<? extends V> valueFuture);
 
   /**
    * Returns a view of the entries stored in this cache as a thread-safe map. Modifications made to

caffeine/src/main/java/com/github/benmanes/caffeine/cache/AsyncCacheLoader.java

@@ -24,6 +24,8 @@
 import java.util.function.BiFunction;
 import java.util.function.Function;
 
+import org.checkerframework.checker.nullness.qual.NonNull;
+
 /**
  * Computes or retrieves values asynchronously, based on a key, for use in populating a
  * {@link AsyncLoadingCache}.
@@ -42,7 +44,7 @@
  */
 @FunctionalInterface
 @SuppressWarnings("PMD.SignatureDeclareThrowsException")
-public interface AsyncCacheLoader<K, V> {
+public interface AsyncCacheLoader<K extends @NonNull Object, V extends @NonNull Object> {
 
   /**
    * Asynchronously computes or retrieves the value corresponding to {@code key}.
@@ -55,7 +57,7 @@ public interface AsyncCacheLoader<K, V> {
    *         treated like any other {@code Exception} in all respects except that, when it is
    *         caught, the thread's interrupt status is set
    */
-  CompletableFuture<V> asyncLoad(K key, Executor executor) throws Exception;
+  CompletableFuture<? extends V> asyncLoad(K key, Executor executor) throws Exception;
 
   /**
    * Asynchronously computes or retrieves the values corresponding to {@code keys}. This method is
@@ -79,7 +81,7 @@ public interface AsyncCacheLoader<K, V> {
    *         treated like any other {@code Exception} in all respects except that, when it is
    *         caught, the thread's interrupt status is set
    */
-  default CompletableFuture<Map<K, V>> asyncLoadAll(
+  default CompletableFuture<? extends Map<? extends K, ? extends V>> asyncLoadAll(
       Set<? extends K> keys, Executor executor) throws Exception {
     throw new UnsupportedOperationException();
   }
@@ -102,7 +104,8 @@ default CompletableFuture<Map<K, V>> asyncLoadAll(
    *         treated like any other {@code Exception} in all respects except that, when it is
    *         caught, the thread's interrupt status is set
    */
-  default CompletableFuture<V> asyncReload(K key, V oldValue, Executor executor) throws Exception {
+  default CompletableFuture<? extends V> asyncReload(
+      K key, V oldValue, Executor executor) throws Exception {
     return asyncLoad(key, executor);
   }
 
@@ -122,14 +125,15 @@ default CompletableFuture<V> asyncReload(K key, V oldValue, Executor executor) t
    * @return an asynchronous cache loader that delegates to the supplied {@code mappingFunction}
    * @throws NullPointerException if the mappingFunction is null
    */
-  static <K, V> AsyncCacheLoader<K, V> bulk(Function<Set<? extends K>, Map<K, V>> mappingFunction) {
+  static <K extends Object, V extends Object> AsyncCacheLoader<K, V> bulk(
+      Function<? super Set<? extends K>, ? extends Map<? extends K, ? extends V>> mappingFunction) {
     return CacheLoader.bulk(mappingFunction);
   }
 
   /**
    * Returns an asynchronous cache loader that delegates to the supplied mapping function for
-   * retrieving the values. Note that {@link #asyncLoad} will discard any additional mappings
-   * loaded when retrieving the {@code key} prior to returning to the value to the cache.
+   * retrieving the values. Note that {@link #asyncLoad} will silently discard any additional
+   * mappings loaded when retrieving the {@code key} prior to returning to the value to the cache.
    * <p>
    * Usage example:
    * <pre>{@code
@@ -142,8 +146,9 @@ static <K, V> AsyncCacheLoader<K, V> bulk(Function<Set<? extends K>, Map<K, V>>
    * @return an asynchronous cache loader that delegates to the supplied {@code mappingFunction}
    * @throws NullPointerException if the mappingFunction is null
    */
-  static <K, V> AsyncCacheLoader<K, V> bulk(
-      BiFunction<Set<? extends K>, Executor, CompletableFuture<Map<K, V>>> mappingFunction) {
+  static <K extends Object, V extends Object> AsyncCacheLoader<K, V> bulk(
+      BiFunction<? super Set<? extends K>, ? super Executor,
+      ? extends CompletableFuture<? extends Map<? extends K, ? extends V>>> mappingFunction) {
     requireNonNull(mappingFunction);
     return new AsyncCacheLoader<>() {
       @Override public CompletableFuture<V> asyncLoad(K key, Executor executor) {
@@ -154,7 +159,9 @@ static <K, V> AsyncCacheLoader<K, V> bulk(
           Set<? extends K> keys, Executor executor) {
         requireNonNull(keys);
         requireNonNull(executor);
-        return mappingFunction.apply(keys, executor);
+        @SuppressWarnings("unchecked")
+        var future = (CompletableFuture<Map<K, V>>) mappingFunction.apply(keys, executor);
+        return future;
       }
     };
   }

caffeine/src/main/java/com/github/benmanes/caffeine/cache/AsyncLoadingCache.java

@@ -18,6 +18,8 @@
 import java.util.Map;
 import java.util.concurrent.CompletableFuture;
 
+import org.checkerframework.checker.nullness.qual.NonNull;
+
 /**
  * A semi-persistent mapping from keys to values. Values are automatically loaded by the cache
  * asynchronously, and are stored in the cache until either evicted or manually invalidated.
@@ -29,7 +31,8 @@
  * @param <K> the type of keys maintained by this cache
  * @param <V> the type of mapped values
  */
-public interface AsyncLoadingCache<K, V> extends AsyncCache<K, V> {
+public interface AsyncLoadingCache<K extends @NonNull Object, V extends @NonNull Object>
+    extends AsyncCache<K, V> {
 
   /**
    * Returns the future associated with {@code key} in this cache, obtaining that value from

caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java

@@ -1224,7 +1224,7 @@ void refreshIfNeeded(Node<K, V> node, long now) {
         && !refreshes().containsKey(keyReference)) {
       long[] startTime = new long[1];
       @SuppressWarnings({"unchecked", "rawtypes"})
-      CompletableFuture<V>[] refreshFuture = new CompletableFuture[1];
+      CompletableFuture<? extends V>[] refreshFuture = new CompletableFuture[1];
       refreshes().computeIfAbsent(keyReference, k -> {
         try {
           startTime[0] = statsTicker().read();
@@ -3976,7 +3976,7 @@ static final class AsyncLoader<K, V> implements CacheLoader<K, V> {
         V newValue = (V) loader.asyncReload(key, oldValue, executor);
         return newValue;
       }
-      @Override public CompletableFuture<V> asyncReload(
+      @Override public CompletableFuture<? extends V> asyncReload(
           K key, V oldValue, Executor executor) throws Exception {
         return loader.asyncReload(key, oldValue, executor);
       }

caffeine/src/main/java/com/github/benmanes/caffeine/cache/Cache.java

@@ -21,6 +21,7 @@
 import java.util.function.Function;
 
 import org.checkerframework.checker.index.qual.NonNegative;
+import org.checkerframework.checker.nullness.qual.NonNull;
 import org.checkerframework.checker.nullness.qual.Nullable;
 import org.checkerframework.checker.nullness.qual.PolyNull;
 
@@ -38,7 +39,7 @@
  * @param <K> the type of keys maintained by this cache
  * @param <V> the type of mapped values
  */
-public interface Cache<K, V> {
+public interface Cache<K extends @NonNull Object, V extends @NonNull Object> {
 
   /**
    * Returns the value associated with the {@code key} in this cache, or {@code null} if there is no
@@ -118,7 +119,7 @@ public interface Cache<K, V> {
    *         left unestablished
    */
   Map<K, V> getAll(Iterable<? extends K> keys,
-      Function<Set<? extends K>, Map<K, V>> mappingFunction);
+      Function<? super Set<? extends K>, ? extends Map<? extends K, ? extends V>> mappingFunction);
 
   /**
    * Associates the {@code value} with the {@code key} in this cache. If the cache previously
@@ -144,7 +145,7 @@ Map<K, V> getAll(Iterable<? extends K> keys,
    * @throws NullPointerException if the specified map is null or the specified map contains null
    *         keys or values
    */
-  void putAll(Map<? extends K,? extends V> map);
+  void putAll(Map<? extends K, ? extends V> map);
 
   /**
    * Discards any cached value for the {@code key}. The behavior of this operation is undefined for

caffeine/src/main/java/com/github/benmanes/caffeine/cache/CacheLoader.java

@@ -24,6 +24,7 @@
 import java.util.concurrent.Executor;
 import java.util.function.Function;
 
+import org.checkerframework.checker.nullness.qual.NonNull;
 import org.checkerframework.checker.nullness.qual.Nullable;
 
 /**
@@ -43,7 +44,8 @@
  */
 @FunctionalInterface
 @SuppressWarnings({"PMD.SignatureDeclareThrowsException", "FunctionalInterfaceMethodChanged"})
-public interface CacheLoader<K, V> extends AsyncCacheLoader<K, V> {
+public interface CacheLoader<K extends @NonNull Object, V extends @NonNull Object>
+    extends AsyncCacheLoader<K, V> {
 
   /**
    * Computes or retrieves the value corresponding to {@code key}.
@@ -83,7 +85,7 @@ public interface CacheLoader<K, V> extends AsyncCacheLoader<K, V> {
    *         treated like any other {@code Exception} in all respects except that, when it is
    *         caught, the thread's interrupt status is set
    */
-  default Map<K, V> loadAll(Set<? extends K> keys) throws Exception {
+  default Map<? extends K, ? extends V> loadAll(Set<? extends K> keys) throws Exception {
     throw new UnsupportedOperationException();
   }
 
@@ -95,7 +97,7 @@ default Map<K, V> loadAll(Set<? extends K> keys) throws Exception {
    * @return the future value associated with {@code key}
    */
   @Override
-  default CompletableFuture<V> asyncLoad(K key, Executor executor) {
+  default CompletableFuture<? extends V> asyncLoad(K key, Executor executor) {
     requireNonNull(key);
     requireNonNull(executor);
     return CompletableFuture.supplyAsync(() -> {
@@ -128,7 +130,8 @@ default CompletableFuture<V> asyncLoad(K key, Executor executor) {
    *         that key; <b>may not contain null values</b>
    */
   @Override
-  default CompletableFuture<Map<K, V>> asyncLoadAll(Set<? extends K> keys, Executor executor) {
+  default CompletableFuture<? extends Map<? extends K, ? extends V>> asyncLoadAll(
+      Set<? extends K> keys, Executor executor) {
     requireNonNull(keys);
     requireNonNull(executor);
     return CompletableFuture.supplyAsync(() -> {
@@ -159,8 +162,7 @@ default CompletableFuture<Map<K, V>> asyncLoadAll(Set<? extends K> keys, Executo
    *         treated like any other {@code Exception} in all respects except that, when it is
    *         caught, the thread's interrupt status is set
    */
-  @Nullable
-  default V reload(K key, V oldValue) throws Exception {
+  default @Nullable V reload(K key, V oldValue) throws Exception {
     return load(key);
   }
 
@@ -179,7 +181,8 @@ default V reload(K key, V oldValue) throws Exception {
    *         {@code null} if the mapping is to be removed
    */
   @Override
-  default CompletableFuture<V> asyncReload(K key, V oldValue, Executor executor) throws Exception {
+  default CompletableFuture<? extends V> asyncReload(
+      K key, V oldValue, Executor executor) throws Exception {
     requireNonNull(key);
     requireNonNull(executor);
     return CompletableFuture.supplyAsync(() -> {
@@ -195,8 +198,8 @@ default CompletableFuture<V> asyncReload(K key, V oldValue, Executor executor) t
 
   /**
    * Returns a cache loader that delegates to the supplied mapping function for retrieving the
-   * values. Note that {@link #load(} will discard any additional mappings loaded when retrieving
-   * the {@code key} prior to returning to the value to the cache.
+   * values. Note that {@link #load} will silently discard any additional mappings loaded when
+   * retrieving the {@code key} prior to returning to the value to the cache.
    * <p>
    * Usage example:
    * <pre>{@code
@@ -208,13 +211,14 @@ default CompletableFuture<V> asyncReload(K key, V oldValue, Executor executor) t
    * @return a cache loader that delegates to the supplied {@code mappingFunction}
    * @throws NullPointerException if the mappingFunction is null
    */
-  static <K, V> CacheLoader<K, V> bulk(Function<Set<? extends K>, Map<K, V>> mappingFunction) {
+  static <K extends Object, V extends Object> CacheLoader<K, V> bulk(
+      Function<? super Set<? extends K>, ? extends Map<? extends K, ? extends V>> mappingFunction) {
     requireNonNull(mappingFunction);
     return new CacheLoader<K, V>() {
       @Override public @Nullable V load(K key) {
         return loadAll(Set.of(key)).get(key);
       }
-      @Override public Map<K, V> loadAll(Set<? extends K> keys) {
+      @Override public Map<? extends K, ? extends V> loadAll(Set<? extends K> keys) {
         return mappingFunction.apply(keys);
       }
     };

caffeine/src/main/java/com/github/benmanes/caffeine/cache/CacheWriter.java

@@ -15,6 +15,7 @@
  */
 package com.github.benmanes.caffeine.cache;
 
+import org.checkerframework.checker.nullness.qual.NonNull;
 import org.checkerframework.checker.nullness.qual.Nullable;
 
 /**
@@ -28,7 +29,7 @@
  * @param <V> the most general type of values this write can write; for example {@code Object} if
  *        any value is acceptable
  */
-public interface CacheWriter<K, V> {
+public interface CacheWriter<K extends @NonNull Object, V extends @NonNull Object> {
 
   /***
    * Writes the value corresponding to the {@code key} to the external resource. The cache will

caffeine/src/main/java/com/github/benmanes/caffeine/cache/Caffeine.java

@@ -36,6 +36,7 @@
 import java.util.function.Supplier;
 
 import org.checkerframework.checker.index.qual.NonNegative;
+import org.checkerframework.checker.nullness.qual.NonNull;
 import org.checkerframework.checker.nullness.qual.Nullable;
 
 import com.github.benmanes.caffeine.cache.Async.AsyncExpiry;
@@ -134,7 +135,7 @@
  *     normally {@code Object} unless it is constrained by using a method like {@code
  *     #removalListener}
  */
-public final class Caffeine<K, V> {
+public final class Caffeine<K extends @NonNull Object, V extends @NonNull Object> {
   static final Logger logger = System.getLogger(Caffeine.class.getName());
   static final Supplier<StatsCounter> ENABLED_STATS_COUNTER_SUPPLIER = ConcurrentStatsCounter::new;
 
@@ -1075,8 +1076,8 @@ public <K1 extends K, V1 extends V> AsyncLoadingCache<K1, V1> buildAsync(
     @SuppressWarnings("unchecked")
     Caffeine<K1, V1> self = (Caffeine<K1, V1>) this;
     return isBounded() || refreshAfterWrite()
-        ? new BoundedLocalCache.BoundedLocalAsyncLoadingCache<>(self, loader)
-        : new UnboundedLocalCache.UnboundedLocalAsyncLoadingCache<>(self, loader);
+        ? new BoundedLocalCache.BoundedLocalAsyncLoadingCache<K1, V1>(self, loader)
+        : new UnboundedLocalCache.UnboundedLocalAsyncLoadingCache<K1, V1>(self, loader);
   }
 
   void requireNonLoadingCache() {