Prefer tighter generic bounds on query methods

The Collection Famework's idiom is to be strict on methods that insert
and loose on those that query. For example add(E) and contains(Object).
Guava's cache followed this practice and relied on static analysis to
discover poor usages when the type would never match. That put the
burden on developer tooling for flexibility that isn't very desirable.
See [1] for the technical details behind the idiom.

We now use stricter bounds in hopes this is a friendler experience.
Thanks to asMap().get(Object), there is an escape hatch anyway.

[1] https://errorprone.info/bugpattern/CollectionIncompatibleType#footnote-would-requiring-e-have-been-better

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

@@ -26,8 +26,6 @@
 import org.checkerframework.checker.nullness.qual.NonNull;
 import org.checkerframework.checker.nullness.qual.Nullable;
 
-import com.google.errorprone.annotations.CompatibleWith;
-
 /**
  * A semi-persistent mapping from keys to values. Cache entries are manually added using
  * {@link #get(Object, Function)} or {@link #put(Object, CompletableFuture)}, and are stored in the
@@ -52,7 +50,7 @@ public interface AsyncCache<K, V> {
    * @throws NullPointerException if the specified key is null
    */
   @Nullable
-  CompletableFuture<V> getIfPresent(@NonNull @CompatibleWith("K") Object key);
+  CompletableFuture<V> getIfPresent(@NonNull K key);
 
   /**
    * 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

@@ -1954,7 +1954,7 @@ public boolean containsValue(Object value) {
   }
 
   @Override
-  public @Nullable V getIfPresentQuietly(Object key, long[/* 1 */] writeTime) {
+  public @Nullable V getIfPresentQuietly(K key, long[/* 1 */] writeTime) {
     V value;
     Node<K, V> node = data.get(nodeFactory.newLookupKey(key));
     if ((node == null) || ((value = node.getValue()) == null)
@@ -1966,7 +1966,7 @@ public boolean containsValue(Object value) {
   }
 
   @Override
-  public Map<K, V> getAllPresent(Iterable<?> keys) {
+  public Map<K, V> getAllPresent(Iterable<? extends K> keys) {
     Map<Object, Object> result = new LinkedHashMap<>();
     for (Object key : keys) {
       result.put(key, null);
@@ -3459,7 +3459,7 @@ static final class BoundedPolicy<K, V> implements Policy<K, V> {
     @Override public boolean isRecordingStats() {
       return cache.isRecordingStats();
     }
-    @Override public @Nullable V getIfPresentQuietly(Object key) {
+    @Override public @Nullable V getIfPresentQuietly(K key) {
       Node<K, V> node = cache.data.get(cache.nodeFactory.newLookupKey(key));
       if ((node == null) || cache.hasExpired(node, cache.expirationTicker().read())) {
         return null;

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

@@ -51,7 +51,7 @@ public interface Cache<K, V> {
    * @throws NullPointerException if the specified key is null
    */
   @Nullable
-  V getIfPresent(@NonNull @CompatibleWith("K") Object key);
+  V getIfPresent(@NonNull K key);
 
   /**
    * Returns the value associated with the {@code key} in this cache, obtaining that value from the
@@ -93,7 +93,7 @@ public interface Cache<K, V> {
    * @throws NullPointerException if the specified collection is null or contains a null element
    */
   @NonNull
-  Map<@NonNull K, @NonNull V> getAllPresent(@NonNull Iterable<@NonNull ?> keys);
+  Map<@NonNull K, @NonNull V> getAllPresent(@NonNull Iterable<? extends K> keys);
 
   /**
    * Returns a map of the values associated with the {@code keys}, creating or retrieving those

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

@@ -63,7 +63,7 @@ interface LocalAsyncCache<K, V> extends AsyncCache<K, V> {
   Policy<K, V> policy();
 
   @Override
-  default @Nullable CompletableFuture<V> getIfPresent(@NonNull Object key) {
+  default @Nullable CompletableFuture<V> getIfPresent(@NonNull K key) {
     return cache().getIfPresent(key, /* recordStats */ true);
   }
 
@@ -466,13 +466,13 @@ abstract class AbstractCacheView<K, V> implements Cache<K, V>, Serializable {
     abstract LocalAsyncCache<K, V> asyncCache();
 
     @Override
-    public @Nullable V getIfPresent(Object key) {
+    public @Nullable V getIfPresent(K key) {
       CompletableFuture<V> future = asyncCache().cache().getIfPresent(key, /* recordStats */ true);
       return Async.getIfReady(future);
     }
 
     @Override
-    public Map<K, V> getAllPresent(Iterable<?> keys) {
+    public Map<K, V> getAllPresent(Iterable<? extends K> keys) {
       Map<Object, Object> result = new LinkedHashMap<>();
       for (Object key : keys) {
         result.put(key, null);

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

@@ -72,22 +72,22 @@ interface LocalCache<K, V> extends ConcurrentMap<K, V> {
   Object referenceKey(K key);
 
   /**
-   * See {@link Cache#getIfPresent(Object)}. This method differs by accepting a parameter of whether
+   * See {@link Cache#getIfPresent(K)}. This method differs by accepting a parameter of whether
    * to record the hit and miss statistics based on the success of this operation.
    */
   @Nullable
-  V getIfPresent(@NonNull Object key, boolean recordStats);
+  V getIfPresent(@NonNull K key, boolean recordStats);
 
   /**
-   * See {@link Cache#getIfPresent(Object)}. This method differs by not recording the access with
+   * See {@link Cache#getIfPresent(K)}. This method differs by not recording the access with
    * the statistics nor the eviction policy, and populates the write time if known.
    */
   @Nullable
-  V getIfPresentQuietly(@NonNull Object key, @NonNull long[/* 1 */] writeTime);
+  V getIfPresentQuietly(@NonNull K key, @NonNull long[/* 1 */] writeTime);
 
   /** See {@link Cache#getAllPresent}. */
   @NonNull
-  Map<K, V> getAllPresent(@NonNull Iterable<?> keys);
+  Map<K, V> getAllPresent(@NonNull Iterable<? extends K> keys);
 
   /**
    * See {@link Cache#put(Object, Object)}. This method differs by allowing the operation to not

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

@@ -53,7 +53,7 @@ default void cleanUp() {
   }
 
   @Override
-  default @Nullable V getIfPresent(Object key) {
+  default @Nullable V getIfPresent(K key) {
     return cache().getIfPresent(key, /* recordStats */ true);
   }
 
@@ -63,7 +63,7 @@ default void cleanUp() {
   }
 
   @Override
-  default Map<K, V> getAllPresent(Iterable<?> keys) {
+  default Map<K, V> getAllPresent(Iterable<? extends K> keys) {
     return cache().getAllPresent(keys);
   }
 

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

@@ -26,8 +26,6 @@
 import org.checkerframework.checker.nullness.qual.NonNull;
 import org.checkerframework.checker.nullness.qual.Nullable;
 
-import com.google.errorprone.annotations.CompatibleWith;
-
 /**
  * An access point for inspecting and performing low-level operations based on the cache's runtime
  * characteristics. These operations are optional and dependent on how the cache was constructed
@@ -56,7 +54,7 @@ public interface Policy<K, V> {
    * @throws NullPointerException if the specified key is null
    */
   @Nullable
-  V getIfPresentQuietly(@NonNull @CompatibleWith("K") Object key);
+  V getIfPresentQuietly(@NonNull K key);
 
   /**
    * Returns access to perform operations based on the maximum size or maximum weight eviction

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

@@ -119,7 +119,7 @@ public Object referenceKey(K key) {
   }
 
   @Override
-  public @Nullable V getIfPresentQuietly(Object key, long[/* 1 */] writeTime) {
+  public @Nullable V getIfPresentQuietly(K key, long[/* 1 */] writeTime) {
     return data.get(key);
   }
 
@@ -129,7 +129,7 @@ public long estimatedSize() {
   }
 
   @Override
-  public Map<K, V> getAllPresent(Iterable<?> keys) {
+  public Map<K, V> getAllPresent(Iterable<? extends K> keys) {
     Map<Object, Object> result = new LinkedHashMap<>();
     for (Object key : keys) {
       result.put(key, null);

caffeine/src/test/java/com/github/benmanes/caffeine/cache/CacheTest.java

@@ -270,10 +270,18 @@ public void getAllPresent_duplicates(Cache<Integer, Integer> cache, CacheContext
         context.original().keySet(), context.original().keySet());
     Map<Integer, Integer> result = cache.getAllPresent(keys);
 
-    int misses = context.absentKeys().size();
-    int hits = context.original().keySet().size();
+    long hitCount, missCount;
+    if (context.implementation() == Implementation.Guava) {
+      // Guava does not skip duplicates
+      hitCount = 2L * context.initialSize();
+      missCount = 2L * context.absentKeys().size();
+    } else {
+      hitCount = context.initialSize();
+      missCount = context.absentKeys().size();
+    }
+    assertThat(context, hasHitCount(hitCount));
+    assertThat(context, hasMissCount(missCount));
     assertThat(result, is(equalTo(context.original())));
-    assertThat(context, both(hasMissCount(misses)).and(hasHitCount(hits)));
     assertThat(context, both(hasLoadSuccessCount(0)).and(hasLoadFailureCount(0)));
   }
 

caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/GuavaCacheFromContext.java

@@ -186,8 +186,11 @@ public V get(K key, Function<? super K, ? extends V> mappingFunction) {
     }
 
     @Override
-    public Map<K, V> getAllPresent(Iterable<?> keys) {
-      return cache.getAllPresent(ImmutableSet.copyOf(keys));
+    public Map<K, V> getAllPresent(Iterable<? extends K> keys) {
+      for (K key : keys) {
+        requireNonNull(key);
+      }
+      return cache.getAllPresent(keys);
     }
 
     @Override

guava/src/main/java/com/github/benmanes/caffeine/guava/CaffeinatedGuavaCache.java

@@ -56,7 +56,9 @@ class CaffeinatedGuavaCache<K, V> implements Cache<K, V>, Serializable {
 
   @Override @Nullable
   public V getIfPresent(Object key) {
-    return cache.getIfPresent(key);
+    @SuppressWarnings("unchecked")
+    K castedKey = (K) key;
+    return cache.getIfPresent(castedKey);
   }
 
   @Override
@@ -91,7 +93,9 @@ public V get(K key, Callable<? extends V> valueLoader) throws ExecutionException
 
   @Override
   public ImmutableMap<K, V> getAllPresent(Iterable<?> keys) {
-    return ImmutableMap.copyOf(cache.getAllPresent(keys));
+    @SuppressWarnings("unchecked")
+    var castedKeys = (Iterable<? extends K>) keys;
+    return ImmutableMap.copyOf(cache.getAllPresent(castedKeys));
   }
 
   @Override