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 0bbbb96
Show file tree
Hide file tree
Showing 45 changed files with 248 additions and 105 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
1 change: 1 addition & 0 deletions caffeine/build.gradle.kts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ tasks.named<JavaCompile>("compileJava").configure {
tasks.named<JavaCompile>("compileTestJava").configure {
dependsOn(tasks.jar, compileCodeGenJava)
options.errorprone {
disable("IdentifierName")
disable("Varifier")
disable("Var")
}
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
28 changes: 17 additions & 11 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
28 changes: 27 additions & 1 deletion 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,19 +138,20 @@ 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);

assertThat(valueFuture).isDone();
assertThat(cache).doesNotContainKey(key);
}

@Test(dataProvider = "caches")
@CacheSpec(executor = CacheExecutor.THREADED, executorFailure = ExecutorFailure.IGNORED)
public void getFunc_absent_null_async(AsyncCache<Int, Int> cache, CacheContext context) {
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 +207,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 +248,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 +382,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 +391,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 +401,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 +426,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 +457,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 +624,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 +671,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 +680,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 +690,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 +957,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 +1030,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 +1048,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 +1066,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
14 changes: 12 additions & 2 deletions caffeine/src/test/java/com/github/benmanes/caffeine/cache/BoundedLocalCacheTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -998,8 +998,13 @@ public void evict_retired_victim(BoundedLocalCache<Int, Int> cache, CacheContext

@Test(dataProvider = "caches")
@CacheSpec(compute = Compute.SYNC, population = Population.EMPTY,
maximumSize = Maximum.FULL, weigher = CacheWeigher.VALUE)
maximumSize = Maximum.FULL, weigher = CacheWeigher.MOCKITO)
public void evict_zeroWeight_candidate(BoundedLocalCache<Int, Int> cache, CacheContext context) {
when(context.weigher().weigh(any(), any())).thenAnswer(invocation -> {
Int value = invocation.getArgument(1);
return Math.abs(value.intValue());
});

for (int i = 0; i < context.maximumSize(); i++) {
assertThat(cache.put(Int.valueOf(i), Int.valueOf(1))).isNull();
}
Expand All @@ -1019,8 +1024,13 @@ public void evict_zeroWeight_candidate(BoundedLocalCache<Int, Int> cache, CacheC

@Test(dataProvider = "caches")
@CacheSpec(compute = Compute.SYNC, population = Population.EMPTY,
maximumSize = Maximum.FULL, weigher = CacheWeigher.VALUE)
maximumSize = Maximum.FULL, weigher = CacheWeigher.MOCKITO)
public void evict_zeroWeight_victim(BoundedLocalCache<Int, Int> cache, CacheContext context) {
when(context.weigher().weigh(any(), any())).thenAnswer(invocation -> {
Int value = invocation.getArgument(1);
return Math.abs(value.intValue());
});

for (int i = 0; i < context.maximumSize(); i++) {
assertThat(cache.put(Int.valueOf(i), Int.valueOf(1))).isNull();
}
Expand Down
4 changes: 4 additions & 0 deletions caffeine/src/test/java/com/github/benmanes/caffeine/cache/CaffeineSpecGuavaTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,21 +354,25 @@ public void testEqualsAndHashCode() {
.testEquals();
}

@SuppressWarnings("NullAway")
public void testMaximumWeight_withWeigher() {
Caffeine<Object, Object> builder = Caffeine.from(parse("maximumWeight=9000"));
assertThat(builder.weigher((k, v) -> 42).build(k -> null)).isNotNull();
}

@SuppressWarnings("NullAway")
public void testMaximumWeight_withoutWeigher() {
Caffeine<Object, Object> builder = Caffeine.from(parse("maximumWeight=9000"));
assertThrows(IllegalStateException.class, () -> builder.build(k -> null));
}

@SuppressWarnings("NullAway")
public void testMaximumSize_withWeigher() {
Caffeine<Object, Object> builder = Caffeine.from(parse("maximumSize=9000"));
assertThat(builder.weigher((k, v) -> 42).build(k -> null)).isNotNull();
}

@SuppressWarnings("NullAway")
public void testMaximumSize_withoutWeigher() {
Caffeine<Object, Object> builder = Caffeine.from(parse("maximumSize=9000"));
assertThat(builder.build(k -> null)).isNotNull();
Expand Down
Loading

0 comments on commit 0bbbb96

Please sign in to comment.