run spotbugs static analyzer agaisnt the test suite

ben-manes · Sep 20, 2024 · abedb62 · abedb62
1 parent eafcb4a
commit abedb62
Show file tree

Hide file tree

Showing 9 changed files with 80 additions and 35 deletions.
diff --git a/caffeine/build.gradle.kts b/caffeine/build.gradle.kts
@@ -378,7 +378,7 @@ data class Scenario(val implementation: Implementation,
   }
 
   fun testName(): String = buildString {
-    append(keys.name.lowercase() + "Keys")
+    append(keys.name.lowercase()).append("Keys")
     append("And").append(values).append("Values")
     if (stats == Stats.Enabled) {
       append("Stats")

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsMapTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsMapTest.java
@@ -39,8 +39,8 @@
 import static org.slf4j.event.Level.WARN;
 
 import java.util.AbstractMap;
-import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -1810,7 +1810,7 @@ public void keySet_removeAll_null(Map<Int, Int> map, CacheContext context) {
   @CheckNoStats
   @Test(dataProvider = "caches")
   public void keySet_removeAll_nullKey(Map<Int, Int> map, CacheContext context) {
-    map.keySet().removeAll(Arrays.asList((Object) null));
+    map.keySet().removeAll(Collections.singletonList((Object) null));
     assertThat(map).isEqualTo(context.original());
   }
 
@@ -1978,7 +1978,7 @@ public void keySet_retainAll_null(Map<Int, Int> map, CacheContext context) {
   @CheckNoStats
   @Test(dataProvider = "caches")
   public void keySet_retainAll_nullKey(Map<Int, Int> map, CacheContext context) {
-    map.keySet().retainAll(Arrays.asList((Object) null));
+    map.keySet().retainAll(Collections.singletonList((Object) null));
     assertThat(map).isExhaustivelyEmpty();
   }
 
@@ -2221,7 +2221,7 @@ public void values_removeAll_null(Map<Int, Int> map, CacheContext context) {
   @CheckNoStats
   @Test(dataProvider = "caches")
   public void values_removeAll_nullValue(Map<Int, Int> map, CacheContext context) {
-    map.values().removeAll(Arrays.asList((Object) null));
+    map.values().removeAll(Collections.singletonList((Object) null));
     assertThat(map).isEqualTo(context.original());
   }
 
@@ -2382,7 +2382,7 @@ public void values_retainAll_null(Map<Int, Int> map, CacheContext context) {
   @CheckNoStats
   @Test(dataProvider = "caches")
   public void values_retainAll_nullValue(Map<Int, Int> map, CacheContext context) {
-    map.values().retainAll(Arrays.asList((Object) null));
+    map.values().retainAll(Collections.singletonList((Object) null));
     assertThat(map).isExhaustivelyEmpty();
   }
 
@@ -2651,7 +2651,7 @@ public void entrySet_removeAll_null(Map<Int, Int> map, CacheContext context) {
   @CheckNoStats
   @Test(dataProvider = "caches")
   public void entrySet_removeAll_nullEntry(Map<Int, Int> map, CacheContext context) {
-    map.entrySet().removeAll(Arrays.asList((Object) null));
+    map.entrySet().removeAll(Collections.singletonList((Object) null));
     assertThat(map).isEqualTo(context.original());
   }
 
@@ -2853,7 +2853,7 @@ public void entrySet_retainAll_null(Map<Int, Int> map, CacheContext context) {
   @CheckNoStats
   @Test(dataProvider = "caches")
   public void entrySet_retainAll_nullEntry(Map<Int, Int> map, CacheContext context) {
-    map.entrySet().retainAll(Arrays.asList((Object) null));
+    map.entrySet().retainAll(Collections.singletonList((Object) null));
     assertThat(map).isExhaustivelyEmpty();
   }
 

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncAsMapTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncAsMapTest.java
@@ -41,6 +41,7 @@
 import java.util.AbstractMap;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -1534,7 +1535,7 @@ public void keySet_removeAll_null(AsyncCache<Int, Int> cache, CacheContext conte
   @CheckNoStats
   @Test(dataProvider = "caches")
   public void keySet_removeAll_nullKey(AsyncCache<Int, Int> cache, CacheContext context) {
-    cache.asMap().keySet().removeAll(Arrays.asList((Object) null));
+    cache.asMap().keySet().removeAll(Collections.singletonList((Object) null));
     assertThat(cache.synchronous().asMap()).isEqualTo(context.original());
   }
 
@@ -1698,7 +1699,7 @@ public void keySet_retainAll_null(AsyncCache<Int, Int> cache, CacheContext conte
   @CheckNoStats
   @Test(dataProvider = "caches")
   public void keySet_retainAll_nullKey(AsyncCache<Int, Int> cache, CacheContext context) {
-    cache.asMap().keySet().retainAll(Arrays.asList((Object) null));
+    cache.asMap().keySet().retainAll(Collections.singletonList((Object) null));
     assertThat(cache).isEmpty();
   }
 
@@ -1948,7 +1949,7 @@ public void values_removeAll_null(AsyncCache<Int, Int> cache, CacheContext conte
   @CheckNoStats
   @Test(dataProvider = "caches")
   public void values_removeAll_nullValue(AsyncCache<Int, Int> cache, CacheContext context) {
-    cache.asMap().values().removeAll(Arrays.asList((Object) null));
+    cache.asMap().values().removeAll(Collections.singletonList((Object) null));
     assertThat(cache.synchronous().asMap()).isEqualTo(context.original());
   }
 
@@ -2126,7 +2127,7 @@ public void values_retainAll_null(AsyncCache<Int, Int> cache, CacheContext conte
   @CheckNoStats
   @Test(dataProvider = "caches")
   public void values_retainAll_nullValue(AsyncCache<Int, Int> cache, CacheContext context) {
-    cache.asMap().values().retainAll(Arrays.asList((Object) null));
+    cache.asMap().values().retainAll(Collections.singletonList((Object) null));
     assertThat(cache).isEmpty();
   }
 
@@ -2410,7 +2411,7 @@ public void entrySet_removeAll_null(AsyncCache<Int, Int> cache, CacheContext con
   @CheckNoStats
   @Test(dataProvider = "caches")
   public void entrySet_removeAll_nullEntry(AsyncCache<Int, Int> cache, CacheContext context) {
-    cache.asMap().entrySet().removeAll(Arrays.asList((Object) null));
+    cache.asMap().entrySet().removeAll(Collections.singletonList((Object) null));
     assertThat(cache.synchronous().asMap()).isEqualTo(context.original());
   }
 
@@ -2614,7 +2615,7 @@ public void entrySet_retainAll_null(AsyncCache<Int, Int> cache, CacheContext con
   @CheckNoStats
   @Test(dataProvider = "caches")
   public void entrySet_retainAll_nullEntry(AsyncCache<Int, Int> cache, CacheContext context) {
-    cache.asMap().entrySet().retainAll(Arrays.asList((Object) null));
+    cache.asMap().entrySet().retainAll(Collections.singletonList((Object) null));
     assertThat(cache).isEmpty();
   }
 

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/CacheTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/CacheTest.java
@@ -868,7 +868,7 @@ public void invalidateAll_removalListener_writeback(Cache<Int, Int> cache, Cache
   @CacheSpec
   @CheckNoStats
   @Test(dataProvider = "caches")
-  public void cleanup(Cache<Int, Int> cache, CacheContext context) {
+  public void cleanUp(Cache<Int, Int> cache, CacheContext context) {
     cache.cleanUp();
   }
 

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ExpirationTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ExpirationTest.java
@@ -42,7 +42,6 @@
 import static org.slf4j.event.Level.WARN;
 
 import java.time.Duration;
-import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -73,7 +72,9 @@
 import com.github.benmanes.caffeine.cache.testing.CheckNoStats;
 import com.github.benmanes.caffeine.testing.Int;
 import com.google.common.base.Splitter;
+import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.Maps;
+import com.google.common.collect.Multimaps;
 import com.google.common.collect.Range;
 import com.google.common.util.concurrent.Futures;
 
@@ -1217,15 +1218,15 @@ public void compute(Map<Int, Int> map, CacheContext context) {
       return value;
     })).isEqualTo(value);
 
-    var evicted = new ArrayList<Map.Entry<Int, Int>>(context.original().size());
+    var evicted = ArrayListMultimap.<Int, Int>create();
     var difference = Maps.difference(context.original(), map);
-    evicted.addAll(difference.entriesOnlyOnRight().entrySet());
-    evicted.addAll(difference.entriesOnlyOnLeft().entrySet());
-    evicted.add(entry(key, context.original().get(key)));
+    evicted.putAll(Multimaps.forMap(difference.entriesOnlyOnRight()));
+    evicted.putAll(Multimaps.forMap(difference.entriesOnlyOnLeft()));
+    evicted.put(key, context.original().get(key));
 
     assertThat(evicted).hasSize(context.original().size() - map.size() + 1);
     assertThat(context).notifications().withCause(EXPIRED)
-        .contains(evicted.toArray(Map.Entry[]::new))
+        .contains(evicted.entries())
         .exclusively();
 
     if (context.expiryType() == CacheExpiry.MOCKITO) {
@@ -1357,15 +1358,15 @@ public void merge(Map<Int, Int> map, CacheContext context) {
       throw new AssertionError("Should never be called");
     })).isEqualTo(value);
 
-    var evicted = new ArrayList<Map.Entry<Int, Int>>(context.original().size());
+    var evicted = ArrayListMultimap.<Int, Int>create();
     var difference = Maps.difference(context.original(), map);
-    evicted.addAll(difference.entriesOnlyOnRight().entrySet());
-    evicted.addAll(difference.entriesOnlyOnLeft().entrySet());
-    evicted.add(entry(key, context.original().get(key)));
+    evicted.putAll(Multimaps.forMap(difference.entriesOnlyOnRight()));
+    evicted.putAll(Multimaps.forMap(difference.entriesOnlyOnLeft()));
+    evicted.put(key, context.original().get(key));
 
     assertThat(evicted).hasSize(context.original().size() - map.size() + 1);
     assertThat(context).notifications().withCause(EXPIRED)
-        .contains(evicted.toArray(Map.Entry[]::new))
+        .contains(evicted.entries())
         .exclusively();
   }
 

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ReferenceTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/ReferenceTest.java
@@ -39,6 +39,7 @@
 
 import java.util.AbstractMap;
 import java.util.AbstractMap.SimpleEntry;
+import java.util.AbstractMap.SimpleImmutableEntry;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
@@ -1306,7 +1307,7 @@ private static List<Map.Entry<Int, Int>> getExpectedAfterGc(
     original.forEach((key, value) -> {
       key = context.isStrongKeys() ? new Int(key) : null;
       value = context.isStrongValues() ? new Int(value) : null;
-      expected.add(new SimpleEntry<>(key, value));
+      expected.add(new SimpleImmutableEntry<>(key, value));
     });
     return expected;
   }

diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/CacheContextSubject.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/CacheContextSubject.java
@@ -32,7 +32,7 @@
 
 import java.util.AbstractMap.SimpleEntry;
 import java.util.Arrays;
-import java.util.List;
+import java.util.Collection;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.function.BiConsumer;
@@ -366,7 +366,7 @@ public Exclusive contains(Map<Int, Int> map) {
       }
 
       @CanIgnoreReturnValue
-      public Exclusive contains(List<Entry<Int, Int>> entries) {
+      public Exclusive contains(Collection<Entry<Int, Int>> entries) {
         return contains(entries.toArray(Map.Entry[]::new));
       }
 

diff --git a/gradle/config/spotbugs/exclude.xml b/gradle/config/spotbugs/exclude.xml
@@ -477,4 +477,45 @@
     <Method name="print"/>
     <Bug pattern="NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE"/>
   </Match>
+
+  <!-- Tests -->
+  <Match>
+    <Or>
+      <Package name="~com\.github\.benmanes\.caffeine\.eclipse.*"/>
+      <Package name="~com\.github\.benmanes\.caffeine\.jsr166.*"/>
+      <Package name="~com\.github\.benmanes\.caffeine\.guava\.compatibility.*"/>
+      <Class name="~com\.github\.benmanes\.caffeine\.cache\.QueueSanityTest.*"/>
+    </Or>
+  </Match>
+  <Match>
+    <Or>
+      <Class name="~.*Test.*"/>
+      <Class name="~.*\.testing.*"/>
+      <Class name="com.github.benmanes.caffeine.cache.Stresser"/>
+    </Or>
+    <Bug pattern="
+      PREDICTABLE_RANDOM,
+      HE_HASHCODE_USE_OBJECT_EQUALS,
+      UTAO_TESTNG_ASSERTION_ODDITIES_NO_ASSERT,
+      "/>
+  </Match>
+  <Match>
+    <Class name="com.github.benmanes.caffeine.testing.Threads"/>
+    <Bug pattern="CRLF_INJECTION_LOGS"/>
+  </Match>
+  <Match>
+    <Class name="com.github.benmanes.caffeine.testing.MapSubject"/>
+    <Bug pattern="NM_SAME_SIMPLE_NAME_AS_SUPERCLASS"/>
+  </Match>
+  <Match>
+    <Class name="~com\.github\.benmanes\.caffeine\.cache\.QueueSanityTest.*"/>
+    <Bug pattern="RV_RETURN_VALUE_IGNORED_BAD_PRACTICE"/>
+  </Match>
+  <Match>
+    <Class name="com.github.benmanes.caffeine.cache.simulator.admission.bloom.MembershipTest"/>
+    <Bug patterns="
+      UC_USELESS_OBJECT,
+      UPM_UNCALLED_PRIVATE_METHOD,
+      WOC_WRITE_ONLY_COLLECTION_LOCAL"/>
+  </Match>
 </FindBugsFilter>
diff --git a/...est/java/com/github/benmanes/caffeine/cache/simulator/admission/bloom/MembershipTest.java b/...est/java/com/github/benmanes/caffeine/cache/simulator/admission/bloom/MembershipTest.java
@@ -32,6 +32,7 @@
 import com.github.benmanes.caffeine.cache.simulator.membership.FilterType;
 import com.github.benmanes.caffeine.cache.simulator.membership.Membership;
 import com.github.benmanes.caffeine.cache.simulator.membership.bloom.BloomFilter;
+import com.google.common.base.CaseFormat;
 import com.google.errorprone.annotations.Var;
 import com.jakewharton.fliptables.FlipTable;
 import com.typesafe.config.Config;
@@ -53,17 +54,17 @@ public final class MembershipTest {
 
   @SuppressWarnings("Varifier")
   @Test(dataProvider = "filterTypes")
-  public void bloomFilterTest(FilterType filterType) {
+  public void bloomFilter(FilterType filterType) {
     var capacities = new IntArrayList(IntList.of(0, 1));
     for (int capacity = 2 << 10; capacity < (2 << 22); capacity <<= 2) {
       capacities.add(capacity);
     }
 
     var random = new Random();
+    var rows = new ArrayList<String[]>();
     for (int capacity : capacities) {
       long[] input = random.longs(capacity).distinct().toArray();
       Config config = getConfig(filterType, capacity);
-      var rows = new ArrayList<String[]>();
 
       Membership filter = filterType.create(config);
       int falsePositives = falsePositives(filter, input);
@@ -75,9 +76,9 @@ public void bloomFilterTest(FilterType filterType) {
       }
       rows.add(row(filterType, capacity, expectedInsertions, falsePositives, falsePositiveRate));
 
-      if (display) {
-        printTable(rows);
-      }
+    }
+    if (display) {
+      printTable(rows);
     }
   }
 
@@ -136,7 +137,7 @@ private static Config getConfig(FilterType filterType, int capacity) {
   private static String[] row(FilterType filterType, int capacity,
       int expectedInsertions, int falsePositives, double falsePositiveRate) {
     return new String[] {
-        filterType.toString(),
+        CaseFormat.UPPER_UNDERSCORE.to(CaseFormat.LOWER_CAMEL, filterType.toString()),
         String.format(US, "%,d", capacity),
         String.format(US, "%,d", expectedInsertions),
         String.format(US, "%,d (%.2f %%)", falsePositives, 100 * falsePositiveRate),