Skip to content

Commit

Permalink
Add @CheckReturnValue at the package level (fixes ben-manes#863)
Browse files Browse the repository at this point in the history
This annotation is used by ErrorProne and Spotbugs to warn users when
they ignore the results of a method call. This is now the default
behavior rather than a few opt in cases. In rare cases the result is
an ignorable signal, like List's add(e) or our LoadingCache.refresh(k),
so @CanIgnoreReturnValue is used to disable the check for those methods.
For all other methods then users can suppress the warning when desired.
  • Loading branch information
ben-manes committed Feb 6, 2023
1 parent 310a132 commit ff12385
Show file tree
Hide file tree
Showing 99 changed files with 305 additions and 91 deletions.
2 changes: 2 additions & 0 deletions caffeine/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,11 @@ tasks.named('compileJavaPoetJava').configure {
tasks.named('compileJava').configure {
dependsOn generateLocalCaches, generateNodes
finalizedBy compileCodeGenJava
options.errorprone.disable('CheckReturnValue')
}
tasks.named('compileTestJava').configure {
dependsOn jar, compileCodeGenJava
options.errorprone.disable('CheckReturnValue')
}
tasks.named('sourcesJar').configure {
dependsOn generateLocalCaches, generateNodes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import com.github.benmanes.caffeine.cache.BasicCache;
import com.github.benmanes.caffeine.cache.CacheType;

import site.ycsb.generator.NumberGenerator;
import site.ycsb.generator.ScrambledZipfianGenerator;

Expand Down Expand Up @@ -68,6 +69,7 @@ protected void profile() {
}

/** Spins forever reading from the cache. */
@SuppressWarnings("CheckReturnValue")
private void reads() {
int index = random.nextInt();
for (;;) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@

import org.checkerframework.checker.nullness.qual.Nullable;

import com.google.errorprone.annotations.CheckReturnValue;

/**
* 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
Expand Down Expand Up @@ -185,7 +183,6 @@ CompletableFuture<Map<K, V>> getAll(Iterable<? extends K> keys,
*
* @return a thread-safe view of this cache supporting all of the optional {@link Map} operations
*/
@CheckReturnValue
ConcurrentMap<K, CompletableFuture<V>> asMap();

/**
Expand All @@ -196,6 +193,5 @@ CompletableFuture<Map<K, V>> getAll(Iterable<? extends K> keys,
*
* @return a thread-safe synchronous view of this cache
*/
@CheckReturnValue
Cache<K, V> synchronous();
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
import java.util.function.BiFunction;
import java.util.function.Function;

import com.google.errorprone.annotations.CheckReturnValue;

/**
* Computes or retrieves values asynchronously, based on a key, for use in populating a
* {@link AsyncLoadingCache}.
Expand Down Expand Up @@ -134,7 +132,6 @@ default CompletableFuture<? extends V> asyncReload(
* @return an asynchronous cache loader that delegates to the supplied {@code mappingFunction}
* @throws NullPointerException if the mappingFunction is null
*/
@CheckReturnValue
static <K, V> AsyncCacheLoader<K, V> bulk(Function<? super Set<? extends K>,
? extends Map<? extends K, ? extends V>> mappingFunction) {
return CacheLoader.bulk(mappingFunction);
Expand All @@ -158,7 +155,6 @@ static <K, V> AsyncCacheLoader<K, V> bulk(Function<? super Set<? extends K>,
* @return an asynchronous cache loader that delegates to the supplied {@code mappingFunction}
* @throws NullPointerException if the mappingFunction is null
*/
@CheckReturnValue
static <K, V> AsyncCacheLoader<K, V> bulk(BiFunction<? super Set<? extends K>, ? super Executor,
? extends CompletableFuture<? extends Map<? extends K, ? extends V>>> mappingFunction) {
requireNonNull(mappingFunction);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
import java.util.Map;
import java.util.concurrent.CompletableFuture;

import com.google.errorprone.annotations.CheckReturnValue;

/**
* 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.
Expand Down Expand Up @@ -88,6 +86,5 @@ public interface AsyncLoadingCache<K, V> extends AsyncCache<K, V> {
* @return a thread-safe synchronous view of this cache
*/
@Override
@CheckReturnValue
LoadingCache<K, V> synchronous();
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.checkerframework.checker.nullness.qual.PolyNull;

import com.github.benmanes.caffeine.cache.stats.CacheStats;
import com.google.errorprone.annotations.CheckReturnValue;

/**
* A semi-persistent mapping from keys to values. Cache entries are manually added using
Expand Down Expand Up @@ -182,7 +181,6 @@ Map<K, V> getAll(Iterable<? extends K> keys,
* @return the estimated number of mappings
*/
@NonNegative
@CheckReturnValue
long estimatedSize();

/**
Expand All @@ -194,7 +192,6 @@ Map<K, V> getAll(Iterable<? extends K> keys,
*
* @return the current snapshot of the statistics of this cache
*/
@CheckReturnValue
CacheStats stats();

/**
Expand All @@ -212,7 +209,6 @@ Map<K, V> getAll(Iterable<? extends K> keys,
*
* @return a thread-safe view of this cache supporting all of the optional {@link Map} operations
*/
@CheckReturnValue
ConcurrentMap<K, V> asMap();

/**
Expand All @@ -231,6 +227,5 @@ Map<K, V> getAll(Iterable<? extends K> keys,
*
* @return access to inspect and perform advanced operations based on the cache's characteristics
*/
@CheckReturnValue
Policy<K, V> policy();
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@

import org.checkerframework.checker.nullness.qual.Nullable;

import com.google.errorprone.annotations.CheckReturnValue;

/**
* Computes or retrieves values, based on a key, for use in populating a {@link LoadingCache} or
* {@link AsyncLoadingCache}.
Expand Down Expand Up @@ -221,7 +219,6 @@ default CompletableFuture<? extends V> asyncReload(
* @return a cache loader that delegates to the supplied {@code mappingFunction}
* @throws NullPointerException if the mappingFunction is null
*/
@CheckReturnValue
@SuppressWarnings("FunctionalInterfaceClash")
static <K, V> CacheLoader<K, V> bulk(Function<? super Set<? extends K>,
? extends Map<? extends K, ? extends V>> mappingFunction) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@
import com.github.benmanes.caffeine.cache.stats.CacheStats;
import com.github.benmanes.caffeine.cache.stats.ConcurrentStatsCounter;
import com.github.benmanes.caffeine.cache.stats.StatsCounter;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.errorprone.annotations.CheckReturnValue;
import com.google.errorprone.annotations.FormatMethod;

/**
Expand Down Expand Up @@ -250,13 +248,11 @@ static int calculateHashMapCapacity(Iterable<?> iterable) {
*
* @return a new instance with default settings
*/
@CheckReturnValue
public static Caffeine<Object, Object> newBuilder() {
return new Caffeine<>();
}

/** Returns a cache that is optimized for weak reference interning (see {@link Interner}). */
@CheckReturnValue
static <K> BoundedLocalCache<K, Boolean> newWeakInterner() {
var builder = new Caffeine<K, Boolean>().executor(Runnable::run).weakKeys();
builder.interner = true;
Expand All @@ -269,7 +265,6 @@ static <K> BoundedLocalCache<K, Boolean> newWeakInterner() {
* @param spec the specification to build from
* @return a new instance with the specification's settings
*/
@CheckReturnValue
public static Caffeine<Object, Object> from(CaffeineSpec spec) {
Caffeine<Object, Object> builder = spec.toBuilder();
builder.strictParsing = false;
Expand All @@ -282,7 +277,6 @@ public static Caffeine<Object, Object> from(CaffeineSpec spec) {
* @param spec a String in the format specified by {@link CaffeineSpec}
* @return a new instance with the specification's settings
*/
@CheckReturnValue
public static Caffeine<Object, Object> from(String spec) {
return from(CaffeineSpec.parse(spec));
}
Expand All @@ -297,7 +291,6 @@ public static Caffeine<Object, Object> from(String spec) {
* @throws IllegalArgumentException if {@code initialCapacity} is negative
* @throws IllegalStateException if an initial capacity was already set
*/
@CanIgnoreReturnValue
public Caffeine<K, V> initialCapacity(@NonNegative int initialCapacity) {
requireState(this.initialCapacity == UNSET_INT,
"initial capacity was already set to %s", this.initialCapacity);
Expand Down Expand Up @@ -331,7 +324,6 @@ int getInitialCapacity() {
* @return this {@code Caffeine} instance (for chaining)
* @throws NullPointerException if the specified executor is null
*/
@CanIgnoreReturnValue
public Caffeine<K, V> executor(Executor executor) {
requireState(this.executor == null, "executor was already set to %s", this.executor);
this.executor = requireNonNull(executor);
Expand Down Expand Up @@ -363,7 +355,6 @@ Executor getExecutor() {
* @return this {@code Caffeine} instance (for chaining)
* @throws NullPointerException if the specified scheduler is null
*/
@CanIgnoreReturnValue
public Caffeine<K, V> scheduler(Scheduler scheduler) {
requireState(this.scheduler == null, "scheduler was already set to %s", this.scheduler);
this.scheduler = requireNonNull(scheduler);
Expand Down Expand Up @@ -398,7 +389,6 @@ Scheduler getScheduler() {
* @throws IllegalArgumentException if {@code size} is negative
* @throws IllegalStateException if a maximum size or weight was already set
*/
@CanIgnoreReturnValue
public Caffeine<K, V> maximumSize(@NonNegative long maximumSize) {
requireState(this.maximumSize == UNSET_INT,
"maximum size was already set to %s", this.maximumSize);
Expand Down Expand Up @@ -435,7 +425,6 @@ public Caffeine<K, V> maximumSize(@NonNegative long maximumSize) {
* @throws IllegalArgumentException if {@code maximumWeight} is negative
* @throws IllegalStateException if a maximum weight or size was already set
*/
@CanIgnoreReturnValue
public Caffeine<K, V> maximumWeight(@NonNegative long maximumWeight) {
requireState(this.maximumWeight == UNSET_INT,
"maximum weight was already set to %s", this.maximumWeight);
Expand Down Expand Up @@ -528,7 +517,6 @@ <K1 extends K, V1 extends V> Weigher<K1, V1> getWeigher(boolean isAsync) {
* @return this {@code Caffeine} instance (for chaining)
* @throws IllegalStateException if the key strength was already set
*/
@CanIgnoreReturnValue
public Caffeine<K, V> weakKeys() {
requireState(keyStrength == null, "Key strength was already set to %s", keyStrength);
keyStrength = Strength.WEAK;
Expand Down Expand Up @@ -558,7 +546,6 @@ boolean isStrongKeys() {
* @return this {@code Caffeine} instance (for chaining)
* @throws IllegalStateException if the value strength was already set
*/
@CanIgnoreReturnValue
public Caffeine<K, V> weakValues() {
requireState(valueStrength == null, "Value strength was already set to %s", valueStrength);
valueStrength = Strength.WEAK;
Expand Down Expand Up @@ -595,7 +582,6 @@ boolean isWeakValues() {
* @return this {@code Caffeine} instance (for chaining)
* @throws IllegalStateException if the value strength was already set
*/
@CanIgnoreReturnValue
public Caffeine<K, V> softValues() {
requireState(valueStrength == null, "Value strength was already set to %s", valueStrength);
valueStrength = Strength.SOFT;
Expand All @@ -618,7 +604,6 @@ public Caffeine<K, V> softValues() {
* @throws IllegalStateException if the time to live or variable expiration was already set
* @throws ArithmeticException for durations greater than +/- approximately 292 years
*/
@CanIgnoreReturnValue
public Caffeine<K, V> expireAfterWrite(Duration duration) {
return expireAfterWrite(saturatedToNanos(duration), TimeUnit.NANOSECONDS);
}
Expand All @@ -642,7 +627,6 @@ public Caffeine<K, V> expireAfterWrite(Duration duration) {
* @throws IllegalArgumentException if {@code duration} is negative
* @throws IllegalStateException if the time to live or variable expiration was already set
*/
@CanIgnoreReturnValue
public Caffeine<K, V> expireAfterWrite(@NonNegative long duration, TimeUnit unit) {
requireState(expireAfterWriteNanos == UNSET_INT,
"expireAfterWrite was already set to %s ns", expireAfterWriteNanos);
Expand Down Expand Up @@ -679,7 +663,6 @@ boolean expiresAfterWrite() {
* @throws IllegalStateException if the time to idle or variable expiration was already set
* @throws ArithmeticException for durations greater than +/- approximately 292 years
*/
@CanIgnoreReturnValue
public Caffeine<K, V> expireAfterAccess(Duration duration) {
return expireAfterAccess(saturatedToNanos(duration), TimeUnit.NANOSECONDS);
}
Expand All @@ -706,7 +689,6 @@ public Caffeine<K, V> expireAfterAccess(Duration duration) {
* @throws IllegalArgumentException if {@code duration} is negative
* @throws IllegalStateException if the time to idle or variable expiration was already set
*/
@CanIgnoreReturnValue
public Caffeine<K, V> expireAfterAccess(@NonNegative long duration, TimeUnit unit) {
requireState(expireAfterAccessNanos == UNSET_INT,
"expireAfterAccess was already set to %s ns", expireAfterAccessNanos);
Expand Down Expand Up @@ -797,7 +779,6 @@ boolean expiresVariable() {
* @throws IllegalStateException if the refresh interval was already set
* @throws ArithmeticException for durations greater than +/- approximately 292 years
*/
@CanIgnoreReturnValue
public Caffeine<K, V> refreshAfterWrite(Duration duration) {
return refreshAfterWrite(saturatedToNanos(duration), TimeUnit.NANOSECONDS);
}
Expand Down Expand Up @@ -825,7 +806,6 @@ public Caffeine<K, V> refreshAfterWrite(Duration duration) {
* @throws IllegalArgumentException if {@code duration} is zero or negative
* @throws IllegalStateException if the refresh interval was already set
*/
@CanIgnoreReturnValue
public Caffeine<K, V> refreshAfterWrite(@NonNegative long duration, TimeUnit unit) {
requireNonNull(unit);
requireState(refreshAfterWriteNanos == UNSET_INT,
Expand Down Expand Up @@ -855,7 +835,6 @@ boolean refreshAfterWrite() {
* @throws IllegalStateException if a ticker was already set
* @throws NullPointerException if the specified ticker is null
*/
@CanIgnoreReturnValue
public Caffeine<K, V> ticker(Ticker ticker) {
requireState(this.ticker == null, "Ticker was already set to %s", this.ticker);
this.ticker = requireNonNull(ticker);
Expand Down Expand Up @@ -982,7 +961,6 @@ public <K1 extends K, V1 extends V> Caffeine<K1, V1> removalListener(
*
* @return this {@code Caffeine} instance (for chaining)
*/
@CanIgnoreReturnValue
public Caffeine<K, V> recordStats() {
requireState(this.statsCounterSupplier == null, "Statistics recording was already set");
statsCounterSupplier = ENABLED_STATS_COUNTER_SUPPLIER;
Expand All @@ -999,7 +977,6 @@ public Caffeine<K, V> recordStats() {
* @param statsCounterSupplier a supplier instance that returns a new {@link StatsCounter}
* @return this {@code Caffeine} instance (for chaining)
*/
@CanIgnoreReturnValue
public Caffeine<K, V> recordStats(Supplier<? extends StatsCounter> statsCounterSupplier) {
requireState(this.statsCounterSupplier == null, "Statistics recording was already set");
requireNonNull(statsCounterSupplier);
Expand Down Expand Up @@ -1042,7 +1019,6 @@ boolean isBounded() {
* @param <V1> the value type of the cache
* @return a cache having the requested features
*/
@CheckReturnValue
public <K1 extends K, V1 extends V> Cache<K1, V1> build() {
requireWeightWithWeigher();
requireNonLoadingCache();
Expand All @@ -1068,7 +1044,6 @@ public <K1 extends K, V1 extends V> Cache<K1, V1> build() {
* @param <V1> the value type of the loader
* @return a cache having the requested features
*/
@CheckReturnValue
public <K1 extends K, V1 extends V> LoadingCache<K1, V1> build(
CacheLoader<? super K1, V1> loader) {
requireWeightWithWeigher();
Expand Down Expand Up @@ -1100,7 +1075,6 @@ public <K1 extends K, V1 extends V> LoadingCache<K1, V1> build(
* @param <V1> the value type of the cache
* @return a cache having the requested features
*/
@CheckReturnValue
public <K1 extends K, V1 extends V> AsyncCache<K1, V1> buildAsync() {
requireState(valueStrength == null, "Weak or soft values can not be combined with AsyncCache");
requireState(isStrongKeys() || (evictionListener == null),
Expand Down Expand Up @@ -1133,7 +1107,6 @@ public <K1 extends K, V1 extends V> AsyncCache<K1, V1> buildAsync() {
* @param <V1> the value type of the loader
* @return a cache having the requested features
*/
@CheckReturnValue
public <K1 extends K, V1 extends V> AsyncLoadingCache<K1, V1> buildAsync(
CacheLoader<? super K1, V1> loader) {
return buildAsync((AsyncCacheLoader<? super K1, V1>) loader);
Expand All @@ -1157,7 +1130,6 @@ public <K1 extends K, V1 extends V> AsyncLoadingCache<K1, V1> buildAsync(
* @param <V1> the value type of the loader
* @return a cache having the requested features
*/
@CheckReturnValue
public <K1 extends K, V1 extends V> AsyncLoadingCache<K1, V1> buildAsync(
AsyncCacheLoader<? super K1, V1> loader) {
requireState(valueStrength == null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionException;

import com.google.errorprone.annotations.CanIgnoreReturnValue;

/**
* A semi-persistent mapping from keys to values. Values are automatically loaded by the cache,
* and are stored in the cache until either evicted or manually invalidated.
Expand Down Expand Up @@ -104,6 +106,7 @@ public interface LoadingCache<K, V> extends Cache<K, V> {
* @return the future that is loading the value
* @throws NullPointerException if the specified key is null
*/
@CanIgnoreReturnValue
CompletableFuture<V> refresh(K key);

/**
Expand All @@ -123,5 +126,6 @@ public interface LoadingCache<K, V> extends Cache<K, V> {
* that are loading the values
* @throws NullPointerException if the specified collection is null or contains a null element
*/
@CanIgnoreReturnValue
CompletableFuture<Map<K, V>> refreshAll(Iterable<? extends K> keys);
}

0 comments on commit ff12385

Please sign in to comment.