Skip to content

Commit

Permalink
enable nullaway on unit tests (wip)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ben-manes
ben-manes committed Dec 15, 2024
1 parent e512b50 commit 870e58f
Show file tree
Hide file tree
Showing 59 changed files with 462 additions and 186 deletions.
1 change: 1 addition & 0 deletions .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ jitwatch.out
.settings
.project
.gradle
.kotlin
.vscode
.idea
*.iml
Expand Down
2 changes: 2 additions & 0 deletions caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3163,6 +3163,7 @@ <T> T expireAfterAccessOrder(boolean oldest, Function<@Nullable V, @Nullable V>
* @param mappingFunction the mapping function to compute a value
* @return the computed value
*/
@SuppressWarnings("NullAway")
<T> T snapshot(Iterable<Node<K, V>> iterable, Function<@Nullable V, @Nullable V> transformer,
Function<Stream<CacheEntry<K, V>>, T> mappingFunction) {
requireNonNull(mappingFunction);
Expand Down Expand Up @@ -4009,6 +4010,7 @@ static final class BoundedPolicy<K, V> implements Policy<K, V> {
@Override public @Nullable V getIfPresentQuietly(K key) {
return transformer.apply(cache.getIfPresentQuietly(key));
}
@SuppressWarnings("NullAway")
@Override public @Nullable CacheEntry<K, V> getEntryIfPresentQuietly(K key) {
Node<K, V> node = cache.data.get(cache.nodeFactory.newLookupKey(key));
return (node == null) ? null : cache.nodeToCacheEntry(node, transformer);
Expand Down
52 changes: 34 additions & 18 deletions caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncAsMapTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.truth.Truth.assertThat;
import static java.util.Objects.requireNonNull;
import static java.util.function.Function.identity;
import static org.junit.Assert.assertThrows;
import static org.mockito.ArgumentMatchers.any;
Expand Down Expand Up @@ -292,8 +293,8 @@ public void put_insert(AsyncCache<Int, Int> cache, CacheContext context) {
public void put_replace_sameValue(AsyncCache<Int, Int> cache, CacheContext context) {
var replaced = new HashMap<Int, Int>();
for (Int key : context.firstMiddleLastKeys()) {
var oldValue = cache.asMap().get(key);
var value = cache.asMap().get(key).thenApply(val -> intern(new Int(val)));
var oldValue = requireNonNull(cache.asMap().get(key));
var value = oldValue.thenApply(val -> intern(new Int(val)));
assertThat(cache.asMap().put(key, value)).isSameInstanceAs(oldValue);
assertThat(cache).containsEntry(key, value);
replaced.put(key, context.original().get(key));
Expand All @@ -307,7 +308,7 @@ public void put_replace_sameValue(AsyncCache<Int, Int> cache, CacheContext conte
@CacheSpec(population = { Population.SINGLETON, Population.PARTIAL, Population.FULL })
public void put_replace_sameInstance(AsyncCache<Int, Int> cache, CacheContext context) {
for (Int key : context.firstMiddleLastKeys()) {
var value = cache.asMap().get(key);
var value = requireNonNull(cache.asMap().get(key));
assertThat(cache.asMap().put(key, value)).isSameInstanceAs(value);
assertThat(cache).containsEntry(key, value);
}
Expand Down Expand Up @@ -435,7 +436,7 @@ public void putIfAbsent_nullKeyAndValue(AsyncCache<Int, Int> cache, CacheContext
removalListener = { Listener.DISABLED, Listener.REJECTING })
public void putIfAbsent_present(AsyncCache<Int, Int> cache, CacheContext context) {
for (Int key : context.firstMiddleLastKeys()) {
var value = cache.asMap().get(key);
var value = requireNonNull(cache.asMap().get(key));
assertThat(cache.asMap().putIfAbsent(key, key.asFuture())).isEqualTo(value);
assertThat(cache).containsEntry(key, value);
}
Expand Down Expand Up @@ -589,8 +590,8 @@ public void replace_failure(AsyncCache<Int, Int> cache, CacheContext context) {
public void replace_sameValue(AsyncCache<Int, Int> cache, CacheContext context) {
var replaced = new HashMap<Int, Int>();
for (Int key : context.firstMiddleLastKeys()) {
var oldValue = cache.asMap().get(key);
var newValue = cache.asMap().get(key).thenApply(val -> intern(new Int(val)));
var oldValue = requireNonNull(cache.asMap().get(key));
var newValue = oldValue.thenApply(val -> intern(new Int(val)));
assertThat(cache.asMap().replace(key, newValue)).isSameInstanceAs(oldValue);
assertThat(cache).containsEntry(key, newValue);
replaced.put(key, context.original().get(key));
Expand Down Expand Up @@ -718,8 +719,8 @@ public void replaceConditionally_wrongOldValue(AsyncCache<Int, Int> cache, Cache
public void replaceConditionally_sameValue(AsyncCache<Int, Int> cache, CacheContext context) {
var replaced = new HashMap<Int, Int>();
for (Int key : context.firstMiddleLastKeys()) {
var oldValue = cache.asMap().get(key);
var newValue = cache.asMap().get(key).thenApply(val -> intern(new Int(val)));
var oldValue = requireNonNull(cache.asMap().get(key));
var newValue = oldValue.thenApply(val -> intern(new Int(val)));
assertThat(cache.asMap().replace(key, oldValue, newValue)).isTrue();
assertThat(cache).containsEntry(key, newValue);
replaced.put(key, context.original().get(key));
Expand Down Expand Up @@ -1310,7 +1311,7 @@ public void merge_absent(AsyncCache<Int, Int> cache, CacheContext context) {
public void merge_sameValue(AsyncCache<Int, Int> cache, CacheContext context) {
var replaced = new HashMap<Int, CompletableFuture<Int>>();
for (Int key : context.firstMiddleLastKeys()) {
var value = cache.asMap().get(key).thenApply(Int::new);
var value = requireNonNull(cache.asMap().get(key)).thenApply(Int::new);
var result = cache.asMap().merge(key, key.negate().asFuture(), (oldValue, v) -> value);
assertThat(result).isSameInstanceAs(value);
replaced.put(key, value);
Expand Down Expand Up @@ -1566,7 +1567,9 @@ public void keySet_removeAll_partial(AsyncCache<Int, Int> cache, CacheContext co
assertThat(cache.asMap().keySet().removeAll(context.firstMiddleLastKeys())).isTrue();
assertThat(cache.synchronous().asMap()).isEqualTo(expected);
assertThat(context).removalNotifications().withCause(EXPLICIT)
.contains(Maps.asMap(context.firstMiddleLastKeys(), context.original()::get)).exclusively();
.contains(Maps.asMap(context.firstMiddleLastKeys(),
key -> requireNonNull(context.original().get(key))))
.exclusively();
}

@CheckNoStats
Expand Down Expand Up @@ -1739,7 +1742,9 @@ public void keySet_retainAll_partial(AsyncCache<Int, Int> cache, CacheContext co
assertThat(cache.asMap().keySet().retainAll(expected.keySet())).isTrue();
assertThat(cache.asMap()).isEqualTo(expected);
assertThat(context).removalNotifications().withCause(EXPLICIT)
.contains(Maps.asMap(context.firstMiddleLastKeys(), context.original()::get)).exclusively();
.contains(Maps.asMap(context.firstMiddleLastKeys(),
key -> requireNonNull(context.original().get(key))))
.exclusively();
}

@CheckNoStats
Expand Down Expand Up @@ -1872,6 +1877,7 @@ public void keySpliterator_estimateSize(AsyncCache<Int, Int> cache, CacheContext
/* ---------------- Values -------------- */

@CheckNoStats
@SuppressWarnings("NullAway")
@Test(dataProvider = "caches")
@CacheSpec(removalListener = { Listener.DISABLED, Listener.REJECTING })
public void values_toArray_null(AsyncCache<Int, Int> cache, CacheContext context) {
Expand Down Expand Up @@ -1978,12 +1984,15 @@ public void values_removeAll_none_populated(AsyncCache<Int, Int> cache, CacheCon
public void values_removeAll_partial(AsyncCache<Int, Int> cache, CacheContext context) {
var expected = new HashMap<>(context.original());
expected.keySet().removeAll(context.firstMiddleLastKeys());
var removed = Maps.asMap(context.firstMiddleLastKeys(), cache.asMap()::get);
var removed = Maps.asMap(context.firstMiddleLastKeys(),
key -> requireNonNull(cache.asMap().get(key)));

assertThat(cache.asMap().values().removeAll(removed.values())).isTrue();
assertThat(cache.synchronous().asMap()).isEqualTo(expected);
assertThat(context).removalNotifications().withCause(EXPLICIT)
.contains(Maps.asMap(context.firstMiddleLastKeys(), context.original()::get)).exclusively();
.contains(Maps.asMap(context.firstMiddleLastKeys(),
key -> requireNonNull(context.original().get(key))))
.exclusively();
}

@CheckNoStats
Expand Down Expand Up @@ -2026,7 +2035,7 @@ public void values_remove_none(AsyncCache<Int, Int> cache, CacheContext context)
@Test(dataProvider = "caches")
@CacheSpec(population = Population.FULL)
public void values_remove(AsyncCache<Int, Int> cache, CacheContext context) {
var future = cache.asMap().get(context.firstKey());
var future = requireNonNull(cache.asMap().get(context.firstKey()));
assertThat(cache.asMap().values().remove(future)).isTrue();
var expected = new HashMap<>(context.original());
expected.remove(context.firstKey());
Expand Down Expand Up @@ -2167,7 +2176,9 @@ public void values_retainAll_partial(AsyncCache<Int, Int> cache, CacheContext co
assertThat(cache.asMap().values().retainAll(expected.values())).isTrue();
assertThat(cache.asMap()).isEqualTo(expected);
assertThat(context).removalNotifications().withCause(EXPLICIT)
.contains(Maps.asMap(context.firstMiddleLastKeys(), context.original()::get)).exclusively();
.contains(Maps.asMap(context.firstMiddleLastKeys(),
key -> requireNonNull(context.original().get(key))))
.exclusively();
}

@CheckNoStats
Expand Down Expand Up @@ -2438,14 +2449,17 @@ public void entrySet_removeAll_none_populated(AsyncCache<Int, Int> cache, CacheC
@Test(dataProvider = "caches")
@CacheSpec(population = Population.FULL)
public void entrySet_removeAll_partial(AsyncCache<Int, Int> cache, CacheContext context) {
var removed = Maps.asMap(context.firstMiddleLastKeys(), cache.asMap()::get);
var removed = Maps.asMap(context.firstMiddleLastKeys(),
key -> requireNonNull(cache.asMap().get(key)));
var expected = new HashMap<>(context.original());
expected.keySet().removeAll(removed.keySet());

assertThat(cache.asMap().entrySet().removeAll(removed.entrySet())).isTrue();
assertThat(cache.synchronous().asMap()).isEqualTo(expected);
assertThat(context).removalNotifications().withCause(EXPLICIT)
.contains(Maps.asMap(context.firstMiddleLastKeys(), context.original()::get)).exclusively();
.contains(Maps.asMap(context.firstMiddleLastKeys(),
key -> requireNonNull(context.original().get(key))))
.exclusively();
}

@CheckNoStats
Expand Down Expand Up @@ -2656,7 +2670,9 @@ public void entrySet_retainAll_partial(AsyncCache<Int, Int> cache, CacheContext
assertThat(cache.asMap().entrySet().retainAll(expected.entrySet())).isTrue();
assertThat(cache.asMap()).isEqualTo(expected);
assertThat(context).removalNotifications().withCause(EXPLICIT)
.contains(Maps.asMap(context.firstMiddleLastKeys(), context.original()::get)).exclusively();
.contains(Maps.asMap(context.firstMiddleLastKeys(),
key -> requireNonNull(context.original().get(key))))
.exclusively();
}

@CheckNoStats
Expand Down
27 changes: 27 additions & 0 deletions caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncCacheTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.truth.Truth.assertThat;
import static java.util.Objects.requireNonNull;
import static java.util.function.Function.identity;
import static org.junit.Assert.assertThrows;
import static org.mockito.ArgumentMatchers.any;
Expand Down Expand Up @@ -85,6 +86,7 @@ public final class AsyncCacheTest {

/* --------------- getIfPresent --------------- */

@SuppressWarnings("NullAway")
@Test(dataProvider = "caches")
@CacheSpec(removalListener = { Listener.DISABLED, Listener.REJECTING })
public void getIfPresent_nullKey(AsyncCache<Int, Int> cache, CacheContext context) {
Expand All @@ -111,19 +113,22 @@ public void getIfPresent_present(AsyncCache<Int, Int> cache, CacheContext contex
/* --------------- getFunc --------------- */

@CacheSpec
@SuppressWarnings("NullAway")
@Test(dataProvider = "caches")
public void getFunc_nullKey(AsyncCache<Int, Int> cache, CacheContext context) {
assertThrows(NullPointerException.class, () -> cache.get(null, key -> null));
}

@CacheSpec
@SuppressWarnings("NullAway")
@Test(dataProvider = "caches")
public void getFunc_nullLoader(AsyncCache<Int, Int> cache, CacheContext context) {
Function<Int, Int> mappingFunction = null;
assertThrows(NullPointerException.class, () -> cache.get(context.absentKey(), mappingFunction));
}

@CacheSpec
@SuppressWarnings("NullAway")
@Test(dataProvider = "caches")
public void getFunc_nullKeyAndLoader(AsyncCache<Int, Int> cache, CacheContext context) {
assertThrows(NullPointerException.class, () -> cache.get(null, (Function<Int, Int>) null));
Expand All @@ -133,6 +138,7 @@ public void getFunc_nullKeyAndLoader(AsyncCache<Int, Int> cache, CacheContext co
@Test(dataProvider = "caches")
public void getFunc_absent_null(AsyncCache<Int, Int> cache, CacheContext context) {
Int key = context.absentKey();
@SuppressWarnings("NullAway")
var valueFuture = cache.get(key, k -> null);
assertThat(context).stats().hits(0).misses(1).success(0).failures(1);

Expand All @@ -146,6 +152,7 @@ public void getFunc_absent_null_async(AsyncCache<Int, Int> cache, CacheContext c
Int key = context.absentKey();
var ready = new AtomicBoolean();
var done = new AtomicBoolean();
@SuppressWarnings("NullAway")
var valueFuture = cache.get(key, k -> {
await().untilTrue(ready);
return null;
Expand Down Expand Up @@ -201,6 +208,7 @@ public void getFunc_absent_failure_async(AsyncCache<Int, Int> cache, CacheContex
@CacheSpec(executor = CacheExecutor.THREADED, executorFailure = ExecutorFailure.IGNORED)
public void getFunc_absent_cancelled(AsyncCache<Int, Int> cache, CacheContext context) {
var done = new AtomicBoolean();
@SuppressWarnings("NullAway")
var valueFuture = cache.get(context.absentKey(), k -> {
await().until(done::get);
return null;
Expand Down Expand Up @@ -241,20 +249,23 @@ public void getFunc_present(AsyncCache<Int, Int> cache, CacheContext context) {
/* --------------- getBiFunc --------------- */

@CacheSpec
@SuppressWarnings("NullAway")
@Test(dataProvider = "caches")
public void getBiFunc_nullKey(AsyncCache<Int, Int> cache, CacheContext context) {
assertThrows(NullPointerException.class, () ->
cache.get(null, (key, executor) -> CompletableFuture.completedFuture(null)));
}

@CacheSpec
@SuppressWarnings("NullAway")
@Test(dataProvider = "caches")
public void getBiFunc_nullLoader(AsyncCache<Int, Int> cache, CacheContext context) {
BiFunction<Int, Executor, CompletableFuture<Int>> mappingFunction = null;
assertThrows(NullPointerException.class, () -> cache.get(context.absentKey(), mappingFunction));
}

@CacheSpec
@SuppressWarnings("NullAway")
@Test(dataProvider = "caches")
public void getBiFunc_nullKeyAndLoader(AsyncCache<Int, Int> cache, CacheContext context) {
BiFunction<Int, Executor, CompletableFuture<Int>> mappingFunction = null;
Expand Down Expand Up @@ -372,6 +383,7 @@ public void getBiFunc_present(AsyncCache<Int, Int> cache, CacheContext context)
/* --------------- getAllFunc --------------- */

@CheckNoStats
@SuppressWarnings("NullAway")
@Test(dataProvider = "caches")
@CacheSpec(removalListener = { Listener.DISABLED, Listener.REJECTING })
public void getAllFunction_nullKeys(AsyncCache<Int, Int> cache, CacheContext context) {
Expand All @@ -380,6 +392,7 @@ public void getAllFunction_nullKeys(AsyncCache<Int, Int> cache, CacheContext con
}

@CheckNoStats
@SuppressWarnings("NullAway")
@Test(dataProvider = "caches")
@CacheSpec(removalListener = { Listener.DISABLED, Listener.REJECTING })
public void getAllFunction_nullKeys_nullFunction(
Expand All @@ -389,6 +402,7 @@ public void getAllFunction_nullKeys_nullFunction(
}

@CheckNoStats
@SuppressWarnings("NullAway")
@Test(dataProvider = "caches")
@CacheSpec(removalListener = { Listener.DISABLED, Listener.REJECTING })
public void getAllFunction_nullFunction(AsyncCache<Int, Int> cache, CacheContext context) {
Expand All @@ -413,6 +427,7 @@ public void getAllFunction_iterable_empty(AsyncCache<Int, Int> cache, CacheConte
assertThat(result).isExhaustivelyEmpty();
}

@SuppressWarnings("NullAway")
@Test(dataProvider = "caches")
@CacheSpec(removalListener = { Listener.DISABLED, Listener.REJECTING })
public void getAllFunction_nullLookup(AsyncCache<Int, Int> cache, CacheContext context) {
Expand Down Expand Up @@ -443,6 +458,7 @@ public void getAllFunction_immutable_result(AsyncCache<Int, Int> cache, CacheCon
}

@CacheSpec
@SuppressWarnings("NullAway")
@Test(dataProvider = "caches")
public void getAllFunction_absent_null(AsyncCache<Int, Int> cache, CacheContext context) {
assertThat(cache.getAll(context.absentKeys(), keys -> null))
Expand Down Expand Up @@ -609,6 +625,7 @@ public void getAllFunction_canceled_individual(AsyncCache<Int, Int> cache, Cache
});
for (var key : context.absentKeys()) {
var future = cache.getIfPresent(key);
requireNonNull(future);
future.cancel(true);
assertThat(future).hasCompletedExceptionally();
}
Expand Down Expand Up @@ -655,6 +672,7 @@ public void getAllFunction_badLoader(AsyncCache<Int, Int> cache, CacheContext co
/* --------------- getAllBiFunc --------------- */

@CheckNoStats
@SuppressWarnings("NullAway")
@Test(dataProvider = "caches")
@CacheSpec(removalListener = { Listener.DISABLED, Listener.REJECTING })
public void getAllBifunction_nullKeys(AsyncCache<Int, Int> cache, CacheContext context) {
Expand All @@ -663,6 +681,7 @@ public void getAllBifunction_nullKeys(AsyncCache<Int, Int> cache, CacheContext c
}

@CheckNoStats
@SuppressWarnings("NullAway")
@Test(dataProvider = "caches")
@CacheSpec(removalListener = { Listener.DISABLED, Listener.REJECTING })
public void getAllBifunction_nullKeys_nullBifunction(
Expand All @@ -672,6 +691,7 @@ public void getAllBifunction_nullKeys_nullBifunction(
}

@CheckNoStats
@SuppressWarnings("NullAway")
@Test(dataProvider = "caches")
@CacheSpec(removalListener = { Listener.DISABLED, Listener.REJECTING })
public void getAllBifunction_nullBifunction(AsyncCache<Int, Int> cache, CacheContext context) {
Expand Down Expand Up @@ -938,6 +958,7 @@ public void getAllBifunction_canceled_individual(
});
for (var key : context.absentKeys()) {
var future = cache.getIfPresent(key);
requireNonNull(future);
future.cancel(true);
assertThat(future).hasCompletedExceptionally();
}
Expand Down Expand Up @@ -1010,6 +1031,7 @@ public void getAllBifunction_early_success(AsyncCache<Int, Int> cache, CacheCont
var result = cache.getAll(context.absentKeys(), (keysToLoad, executor) -> bulk);
var future = cache.asMap().get(key);

requireNonNull(future);
future.complete(value);
bulk.complete(context.absent()); // obtrudes the future's value

Expand All @@ -1027,6 +1049,8 @@ public void getAllBifunction_early_failure(AsyncCache<Int, Int> cache, CacheCont
var bulk = new CompletableFuture<Map<Int, Int>>();
var result = cache.getAll(context.absentKeys(), (keysToLoad, executor) -> bulk);
var future = cache.asMap().get(key);

requireNonNull(future);
future.completeExceptionally(error);

bulk.complete(context.absent());
Expand All @@ -1043,19 +1067,22 @@ public void getAllBifunction_early_failure(AsyncCache<Int, Int> cache, CacheCont

/* --------------- put --------------- */

@SuppressWarnings("NullAway")
@Test(dataProvider = "caches")
@CacheSpec(removalListener = { Listener.DISABLED, Listener.REJECTING })
public void put_nullKey(AsyncCache<Int, Int> cache, CacheContext context) {
var value = context.absentValue().asFuture();
assertThrows(NullPointerException.class, () -> cache.put(null, value));
}

@SuppressWarnings("NullAway")
@Test(dataProvider = "caches")
@CacheSpec(removalListener = { Listener.DISABLED, Listener.REJECTING })
public void put_nullValue(AsyncCache<Int, Int> cache, CacheContext context) {
assertThrows(NullPointerException.class, () -> cache.put(context.absentKey(), null));
}

@SuppressWarnings("NullAway")
@Test(dataProvider = "caches")
@CacheSpec(removalListener = { Listener.DISABLED, Listener.REJECTING })
public void put_nullKeyAndValue(AsyncCache<Int, Int> cache, CacheContext context) {
Expand Down
Loading

0 comments on commit 870e58f

Please sign in to comment.