From 6e3ff707ecf5882b30a383f6a8b3c0dfd1ed286b Mon Sep 17 00:00:00 2001 From: Ben Manes Date: Sat, 23 Nov 2024 15:54:29 -0800 Subject: [PATCH] enable jspecify mode in nullaway for stricter null checks --- .../caffeine/cache/BoundedLocalCache.java | 81 ++++++++++--------- .../caffeine/cache/LocalAsyncCache.java | 8 +- .../cache/LocalAsyncLoadingCache.java | 2 +- .../benmanes/caffeine/cache/LocalCache.java | 2 +- .../caffeine/cache/LocalLoadingCache.java | 6 +- .../caffeine/cache/UnboundedLocalCache.java | 17 ++-- .../quality/errorprone.caffeine.gradle.kts | 1 + .../caffeine/jcache/CacheFactory.java | 6 +- 8 files changed, 64 insertions(+), 59 deletions(-) diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java index 225b412418..cf2daabd59 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java @@ -2095,10 +2095,10 @@ public void clear() { @SuppressWarnings("GuardedByChecker") void removeNode(Node node, long now) { K key = node.getKey(); - @SuppressWarnings("unchecked") - var value = (V[]) new Object[1]; var cause = new RemovalCause[1]; var keyReference = node.getKeyReference(); + @SuppressWarnings({"unchecked", "Varifier"}) + @Nullable V[] value = (V[]) new Object[1]; data.computeIfPresent(keyReference, (k, n) -> { if (n != node) { @@ -2437,8 +2437,8 @@ public void putAll(Map map) { var castKey = (K) key; @SuppressWarnings({"rawtypes", "unchecked"}) Node[] node = new Node[1]; - @SuppressWarnings("unchecked") - var oldValue = (V[]) new Object[1]; + @SuppressWarnings({"unchecked", "Varifier"}) + @Nullable V[] oldValue = (V[]) new Object[1]; RemovalCause[] cause = new RemovalCause[1]; Object lookupKey = nodeFactory.newLookupKey(key); @@ -2479,10 +2479,10 @@ public boolean remove(Object key, Object value) { @SuppressWarnings({"rawtypes", "unchecked"}) Node[] removed = new Node[1]; - @SuppressWarnings("unchecked") - var oldKey = (K[]) new Object[1]; - @SuppressWarnings("unchecked") - var oldValue = (V[]) new Object[1]; + @SuppressWarnings({"unchecked", "Varifier"}) + @Nullable K[] oldKey = (K[]) new Object[1]; + @SuppressWarnings({"unchecked", "Varifier"}) + @Nullable V[] oldValue = (V[]) new Object[1]; RemovalCause[] cause = new RemovalCause[1]; Object lookupKey = nodeFactory.newLookupKey(key); @@ -2526,10 +2526,10 @@ public boolean remove(Object key, Object value) { long[] now = new long[1]; var oldWeight = new int[1]; - @SuppressWarnings("unchecked") - var nodeKey = (K[]) new Object[1]; - @SuppressWarnings("unchecked") - var oldValue = (V[]) new Object[1]; + @SuppressWarnings({"unchecked", "Varifier"}) + @Nullable K[] nodeKey = (K[]) new Object[1]; + @SuppressWarnings({"unchecked", "Varifier"}) + @Nullable V[] oldValue = (V[]) new Object[1]; int weight = weigher.weigh(key, value); Node node = data.computeIfPresent(nodeFactory.newLookupKey(key), (k, n) -> { synchronized (n) { @@ -2555,7 +2555,7 @@ public boolean remove(Object key, Object value) { } }); - if (oldValue[0] == null) { + if ((nodeKey[0] == null) || (oldValue[0] == null)) { return null; } @@ -2582,11 +2582,10 @@ public boolean replace(K key, V oldValue, V newValue, boolean shouldDiscardRefre requireNonNull(newValue); int weight = weigher.weigh(key, newValue); - boolean[] replaced = new boolean[1]; - @SuppressWarnings("unchecked") - var nodeKey = (K[]) new Object[1]; - @SuppressWarnings("unchecked") - var prevValue = (V[]) new Object[1]; + @SuppressWarnings({"unchecked", "Varifier"}) + @Nullable K[] nodeKey = (K[]) new Object[1]; + @SuppressWarnings({"unchecked", "Varifier"}) + @Nullable V[] prevValue = (V[]) new Object[1]; int[] oldWeight = new int[1]; long[] now = new long[1]; Node node = data.computeIfPresent(nodeFactory.newLookupKey(key), (k, n) -> { @@ -2597,6 +2596,7 @@ public boolean replace(K key, V oldValue, V newValue, boolean shouldDiscardRefre oldWeight[0] = n.getWeight(); if ((nodeKey[0] == null) || (prevValue[0] == null) || !n.containsValue(oldValue) || hasExpired(n, now[0] = expirationTicker().read())) { + prevValue[0] = null; return n; } @@ -2607,7 +2607,6 @@ public boolean replace(K key, V oldValue, V newValue, boolean shouldDiscardRefre setVariableTime(n, varTime); setAccessTime(n, now[0]); setWriteTime(n, now[0]); - replaced[0] = true; if (shouldDiscardRefresh) { discardRefresh(k); @@ -2616,7 +2615,7 @@ public boolean replace(K key, V oldValue, V newValue, boolean shouldDiscardRefre return n; }); - if (!replaced[0]) { + if ((nodeKey[0] == null) || (prevValue[0] == null)) { return false; } @@ -2673,13 +2672,14 @@ public void replaceAll(BiFunction function) { /** Returns the current value from a computeIfAbsent invocation. */ @Nullable V doComputeIfAbsent(K key, Object keyRef, - Function mappingFunction, long[/* 1 */] now, boolean recordStats) { - @SuppressWarnings("unchecked") - var oldValue = (V[]) new Object[1]; - @SuppressWarnings("unchecked") - var newValue = (V[]) new Object[1]; - @SuppressWarnings("unchecked") - var nodeKey = (K[]) new Object[1]; + Function mappingFunction, long[/* 1 */] now, + boolean recordStats) { + @SuppressWarnings({"unchecked", "Varifier"}) + @Nullable V[] oldValue = (V[]) new Object[1]; + @SuppressWarnings({"unchecked", "Varifier"}) + @Nullable V[] newValue = (V[]) new Object[1]; + @SuppressWarnings({"unchecked", "Varifier"}) + @Nullable K[] nodeKey = (K[]) new Object[1]; @SuppressWarnings({"rawtypes", "unchecked"}) Node[] removed = new Node[1]; @@ -2838,14 +2838,14 @@ public void replaceAll(BiFunction function) { */ @SuppressWarnings("PMD.EmptyControlStatement") @Nullable V remap(K key, Object keyRef, - BiFunction remappingFunction, + BiFunction remappingFunction, Expiry expiry, long[/* 1 */] now, boolean computeIfAbsent) { - @SuppressWarnings("unchecked") - var nodeKey = (K[]) new Object[1]; - @SuppressWarnings("unchecked") - var oldValue = (V[]) new Object[1]; - @SuppressWarnings("unchecked") - var newValue = (V[]) new Object[1]; + @SuppressWarnings({"unchecked", "Varifier"}) + @Nullable K[] nodeKey = (K[]) new Object[1]; + @SuppressWarnings({"unchecked", "Varifier"}) + @Nullable V[] oldValue = (V[]) new Object[1]; + @SuppressWarnings({"unchecked", "Varifier"}) + @Nullable V[] newValue = (V[]) new Object[1]; @SuppressWarnings({"rawtypes", "unchecked"}) Node[] removed = new Node[1]; @@ -2929,6 +2929,7 @@ public void replaceAll(BiFunction function) { if (cause[0] != null) { if (cause[0] == RemovalCause.REPLACED) { + requireNonNull(newValue[0]); notifyOnReplace(key, oldValue[0], newValue[0]); } else { if (cause[0].wasEvicted()) { @@ -3186,7 +3187,8 @@ T snapshot(Iterable> iterable, Function transformer, } /** Returns an entry for the given node if it can be used externally, else null. */ - @Nullable CacheEntry nodeToCacheEntry(Node node, Function transformer) { + @Nullable CacheEntry nodeToCacheEntry( + Node node, Function<@Nullable V, @Nullable V> transformer) { V value = transformer.apply(node.getValue()); K key = node.getKey(); long now; @@ -3980,8 +3982,8 @@ private Object writeReplace() { @SuppressWarnings({"NullableOptional", "OptionalAssignedToNull"}) static final class BoundedPolicy implements Policy { + final Function<@Nullable V, @Nullable V> transformer; final BoundedLocalCache cache; - final Function transformer; final boolean isWeighted; @Nullable Optional> eviction; @@ -3990,7 +3992,8 @@ static final class BoundedPolicy implements Policy { @Nullable Optional> afterAccess; @Nullable Optional> variable; - BoundedPolicy(BoundedLocalCache cache, Function transformer, boolean isWeighted) { + BoundedPolicy(BoundedLocalCache cache, + Function<@Nullable V, @Nullable V> transformer, boolean isWeighted) { this.transformer = transformer; this.isWeighted = isWeighted; this.cache = cache; @@ -4523,7 +4526,7 @@ public Policy policy() { if (policy == null) { @SuppressWarnings("unchecked") var castCache = (BoundedLocalCache) cache; - Function, V> transformer = Async::getIfReady; + Function, @Nullable V> transformer = Async::getIfReady; @SuppressWarnings("unchecked") var castTransformer = (Function) transformer; policy = new BoundedPolicy<>(castCache, castTransformer, isWeighted); @@ -4575,7 +4578,7 @@ public Policy policy() { if (policy == null) { @SuppressWarnings("unchecked") var castCache = (BoundedLocalCache) cache; - Function, V> transformer = Async::getIfReady; + Function, @Nullable V> transformer = Async::getIfReady; @SuppressWarnings("unchecked") var castTransformer = (Function) transformer; policy = new BoundedPolicy<>(castCache, castTransformer, isWeighted); diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalAsyncCache.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalAsyncCache.java index 7b9237f41c..5170ac155f 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalAsyncCache.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalAsyncCache.java @@ -434,12 +434,12 @@ public boolean replace(K key, CompletableFuture oldValue, CompletableFuture computeIfPresent(K key, BiFunction computeIfPresent(K key, BiFunction, ? extends CompletableFuture> remappingFunction) { requireNonNull(remappingFunction); @SuppressWarnings({"rawtypes", "unchecked"}) - CompletableFuture[] result = new CompletableFuture[1]; + @Nullable CompletableFuture[] result = new CompletableFuture[1]; long startTime = asyncCache.cache().statsTicker().read(); asyncCache.cache().compute(key, (k, oldValue) -> { result[0] = (oldValue == null) ? null : remappingFunction.apply(k, oldValue); @@ -796,8 +796,8 @@ public boolean remove(Object key, Object value) { public @Nullable V replace(K key, V value) { requireNonNull(value); - @SuppressWarnings({"rawtypes", "unchecked"}) - var oldValue = (V[]) new Object[1]; + @SuppressWarnings({"rawtypes", "unchecked", "Varifier"}) + @Nullable V[] oldValue = (V[]) new Object[1]; boolean[] done = { false }; for (;;) { CompletableFuture future = delegate.getIfPresentQuietly(key); diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalAsyncLoadingCache.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalAsyncLoadingCache.java index 24d8077f0e..fd406d3c4b 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalAsyncLoadingCache.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalAsyncLoadingCache.java @@ -254,7 +254,7 @@ public CompletableFuture> refreshAll(Iterable keys) { long[] startTime = new long[1]; boolean[] refreshed = new boolean[1]; @SuppressWarnings({"rawtypes", "unchecked"}) - CompletableFuture[] oldValueFuture = new CompletableFuture[1]; + @Nullable CompletableFuture[] oldValueFuture = new CompletableFuture[1]; var future = asyncCache.cache().refreshes().computeIfAbsent(keyReference, k -> { oldValueFuture[0] = asyncCache.cache().getIfPresentQuietly(key); V oldValue = Async.getIfReady(oldValueFuture[0]); diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalCache.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalCache.java index e9e62661e4..0024e52edd 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalCache.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalCache.java @@ -131,7 +131,7 @@ default void invalidateAll(Iterable keys) { /** Notify the removal listener of a replacement if the value reference was changed. */ @SuppressWarnings("FutureReturnValueIgnored") - default void notifyOnReplace(K key, V oldValue, V newValue) { + default void notifyOnReplace(K key, @Nullable V oldValue, V newValue) { if ((oldValue == null) || (oldValue == newValue)) { return; } else if (isAsync()) { diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalLoadingCache.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalLoadingCache.java index 0f62d1aefe..0765da35b7 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalLoadingCache.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/LocalLoadingCache.java @@ -101,8 +101,8 @@ default CompletableFuture refresh(K key) { requireNonNull(key); long[] startTime = new long[1]; - @SuppressWarnings("unchecked") - var oldValue = (V[]) new Object[1]; + @SuppressWarnings({"unchecked", "Varifier"}) + @Nullable V[] oldValue = (V[]) new Object[1]; @SuppressWarnings({"rawtypes", "unchecked"}) CompletableFuture[] reloading = new CompletableFuture[1]; Object keyReference = cache().referenceKey(key); @@ -179,7 +179,7 @@ default CompletableFuture> refreshAll(Iterable keys) { } /** Returns a mapping function that adapts to {@link CacheLoader#load}. */ - static Function newMappingFunction(CacheLoader cacheLoader) { + static Function newMappingFunction(CacheLoader cacheLoader) { return key -> { try { return cacheLoader.load(key); diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/UnboundedLocalCache.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/UnboundedLocalCache.java index 0170a5ac42..49a4883891 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/UnboundedLocalCache.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/UnboundedLocalCache.java @@ -251,10 +251,10 @@ public void replaceAll(BiFunction function) { requireNonNull(function); // ensures that the removal notification is processed after the removal has completed - @SuppressWarnings({"rawtypes", "unchecked"}) - var notificationKey = (K[]) new Object[1]; - @SuppressWarnings({"rawtypes", "unchecked"}) - var notificationValue = (V[]) new Object[1]; + @SuppressWarnings({"rawtypes", "unchecked", "Varifier"}) + @Nullable K[] notificationKey = (K[]) new Object[1]; + @SuppressWarnings({"rawtypes", "unchecked", "Varifier"}) + @Nullable V[] notificationValue = (V[]) new Object[1]; data.replaceAll((key, value) -> { if (notificationKey[0] != null) { notifyRemoval(notificationKey[0], notificationValue[0], RemovalCause.REPLACED); @@ -1060,10 +1060,11 @@ Object writeReplace() { /** An eviction policy that supports no bounding. */ static final class UnboundedPolicy implements Policy { + final Function<@Nullable V, @Nullable V> transformer; final UnboundedLocalCache cache; - final Function transformer; - UnboundedPolicy(UnboundedLocalCache cache, Function transformer) { + UnboundedPolicy(UnboundedLocalCache cache, + Function<@Nullable V, @Nullable V> transformer) { this.transformer = transformer; this.cache = cache; } @@ -1187,7 +1188,7 @@ public Cache synchronous() { public Policy policy() { @SuppressWarnings("unchecked") var castCache = (UnboundedLocalCache) cache; - Function, V> transformer = Async::getIfReady; + Function, @Nullable V> transformer = Async::getIfReady; @SuppressWarnings("unchecked") var castTransformer = (Function) transformer; return (policy == null) @@ -1240,7 +1241,7 @@ public ConcurrentMap> asMap() { public Policy policy() { @SuppressWarnings("unchecked") var castCache = (UnboundedLocalCache) cache; - Function, V> transformer = Async::getIfReady; + Function, @Nullable V> transformer = Async::getIfReady; @SuppressWarnings("unchecked") var castTransformer = (Function) transformer; return (policy == null) diff --git a/gradle/plugins/src/main/kotlin/quality/errorprone.caffeine.gradle.kts b/gradle/plugins/src/main/kotlin/quality/errorprone.caffeine.gradle.kts index 39abfb54ee..8324b83edc 100644 --- a/gradle/plugins/src/main/kotlin/quality/errorprone.caffeine.gradle.kts +++ b/gradle/plugins/src/main/kotlin/quality/errorprone.caffeine.gradle.kts @@ -71,6 +71,7 @@ tasks.withType().configureEach { checkOptionalEmptiness = true suggestSuppressions = true checkContracts = true + isJSpecifyMode = true } } } diff --git a/jcache/src/main/java/com/github/benmanes/caffeine/jcache/CacheFactory.java b/jcache/src/main/java/com/github/benmanes/caffeine/jcache/CacheFactory.java index 03b4a044dc..63c5d27860 100644 --- a/jcache/src/main/java/com/github/benmanes/caffeine/jcache/CacheFactory.java +++ b/jcache/src/main/java/com/github/benmanes/caffeine/jcache/CacheFactory.java @@ -206,10 +206,10 @@ private boolean isReadThrough() { /** Creates a cache that does not read through on a cache miss. */ private CacheProxy newCacheProxy() { - Optional> cacheLoader = - Optional.ofNullable(config.getCacheLoaderFactory()).map(Factory::create); + var cacheLoaderFactory = config.getCacheLoaderFactory(); + var cacheLoader = (cacheLoaderFactory == null) ? null : cacheLoaderFactory.create(); return new CacheProxy<>(cacheName, executor, cacheManager, config, caffeine.build(), - dispatcher, cacheLoader, expiryPolicy, ticker, statistics); + dispatcher, Optional.ofNullable(cacheLoader), expiryPolicy, ticker, statistics); } /** Creates a cache that reads through on a cache miss. */