Add @CheckReturnValue at the package level (fixes #863)

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.

caffeine/build.gradle

@@ -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

caffeine/src/jmh/java/com/github/benmanes/caffeine/profiler/CacheProfiler.java

@@ -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;
 
@@ -68,6 +69,7 @@ protected void profile() {
   }
 
   /** Spins forever reading from the cache. */
+  @SuppressWarnings("CheckReturnValue")
   private void reads() {
     int index = random.nextInt();
     for (;;) {

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

@@ -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
@@ -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();
 
   /**
@@ -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();
 }

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

@@ -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}.
@@ -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);
@@ -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);

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

@@ -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.
@@ -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();
 }

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

@@ -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
@@ -182,7 +181,6 @@ Map<K, V> getAll(Iterable<? extends K> keys,
    * @return the estimated number of mappings
    */
   @NonNegative
-  @CheckReturnValue
   long estimatedSize();
 
   /**
@@ -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();
 
   /**
@@ -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();
 
   /**
@@ -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();
 }

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

@@ -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}.
@@ -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) {

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

@@ -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;
 
 /**
@@ -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;
@@ -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;
@@ -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));
   }
@@ -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);
@@ -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);
@@ -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);
@@ -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);
@@ -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);
@@ -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;
@@ -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;
@@ -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;
@@ -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);
   }
@@ -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);
@@ -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);
   }
@@ -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);
@@ -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);
   }
@@ -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,
@@ -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);
@@ -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;
@@ -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);
@@ -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();
@@ -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();
@@ -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),
@@ -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);
@@ -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,

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

@@ -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.
@@ -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);
 
   /**
@@ -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);
 }