Skip to content

Commit

Permalink
run spotbugs static analyzer agaisnt the test suite
Browse files Browse the repository at this point in the history
  • Loading branch information
ben-manes committed Sep 20, 2024
1 parent eafcb4a commit 20463e1
Show file tree
Hide file tree
Showing 9 changed files with 79 additions and 35 deletions.
2 changes: 1 addition & 1 deletion caffeine/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}

Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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());
}

Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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());
}

Expand Down Expand Up @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}

Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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());
}

Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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());
}

Expand Down Expand Up @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1306,7 +1306,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(Map.entry(key, value));
});
return expected;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
}

Expand Down
41 changes: 41 additions & 0 deletions gradle/config/spotbugs/exclude.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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),
Expand Down

0 comments on commit 20463e1

Please sign in to comment.