run pmd static analyzer agaisnt the test suite

.github/scripts/analyze.sh

@@ -2,7 +2,7 @@
 set -eux
 
 ./gradlew \
- forbiddenApis -DforbiddenApis \
- pmdJavaPoet pmdMain pmdCodeGen pmdJmh -Dpmd \
+ forbiddenApis forbiddenApisTest -DforbiddenApis \
+ pmdJavaPoet pmdMain pmdCodeGen pmdJmh pmdTest -Dpmd \
  spotbugsJavaPoet spotbugsMain spotbugsCodeGen spotbugsJmh -Dspotbugs \
  "$@"

.github/workflows/analysis.yml

@@ -36,7 +36,7 @@ jobs:
  uses: ./.github/actions/run-gradle
  with:
  java: ${{ env.JAVA_VERSION }}
- arguments: forbiddenApis -DforbiddenApis
+ arguments: forbiddenApis forbiddenApisTest -DforbiddenApis
 
  pmd:
  runs-on: ubuntu-latest
@@ -54,7 +54,7 @@ jobs:
  uses: ./.github/actions/run-gradle
  with:
  java: ${{ env.JAVA_VERSION }}
- arguments: pmdJavaPoet pmdMain pmdCodeGen pmdJmh -Dpmd
+ arguments: pmdJavaPoet pmdMain pmdCodeGen pmdJmh pmdTest -Dpmd
 
  spotbugs:
  runs-on: ubuntu-latest

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

@@ -63,7 +63,8 @@ protected long getCurrentBufferCapacity(long mask) {
  }
 }
 
-@SuppressWarnings({"MultiVariableDeclaration", "OvershadowingSubclassFields"})
+@SuppressWarnings({"MultiVariableDeclaration",
+ "OvershadowingSubclassFields", "PMD.OneDeclarationPerLine"})
 abstract class MpscChunkedArrayQueue<E> extends MpscChunkedArrayQueueColdProducerFields<E> {
  byte p000, p001, p002, p003, p004, p005, p006, p007;
  byte p008, p009, p010, p011, p012, p013, p014, p015;
@@ -112,7 +113,7 @@ abstract class MpscChunkedArrayQueueColdProducerFields<E> extends BaseMpscLinked
  }
 }
 
-@SuppressWarnings("MultiVariableDeclaration")
+@SuppressWarnings({"MultiVariableDeclaration", "PMD.OneDeclarationPerLine"})
 abstract class BaseMpscLinkedArrayQueuePad1<E> extends AbstractQueue<E> {
  byte p000, p001, p002, p003, p004, p005, p006, p007;
  byte p008, p009, p010, p011, p012, p013, p014, p015;
@@ -135,7 +136,8 @@ abstract class BaseMpscLinkedArrayQueueProducerFields<E> extends BaseMpscLinkedA
  protected long producerIndex;
 }
 
-@SuppressWarnings({"MultiVariableDeclaration", "OvershadowingSubclassFields"})
+@SuppressWarnings({"MultiVariableDeclaration",
+ "OvershadowingSubclassFields", "PMD.OneDeclarationPerLine"})
 abstract class BaseMpscLinkedArrayQueuePad2<E> extends BaseMpscLinkedArrayQueueProducerFields<E> {
  byte p000, p001, p002, p003, p004, p005, p006, p007;
  byte p008, p009, p010, p011, p012, p013, p014, p015;
@@ -161,7 +163,8 @@ abstract class BaseMpscLinkedArrayQueueConsumerFields<E> extends BaseMpscLinkedA
  protected long consumerIndex;
 }
 
-@SuppressWarnings({"MultiVariableDeclaration", "OvershadowingSubclassFields"})
+@SuppressWarnings({"MultiVariableDeclaration",
+ "OvershadowingSubclassFields", "PMD.OneDeclarationPerLine"})
 abstract class BaseMpscLinkedArrayQueuePad3<E> extends BaseMpscLinkedArrayQueueConsumerFields<E> {
  byte p000, p001, p002, p003, p004, p005, p006, p007;
  byte p008, p009, p010, p011, p012, p013, p014, p015;

caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsMapTest.java

@@ -58,7 +58,6 @@
 
 import org.eclipse.collections.impl.factory.Sets;
 import org.mockito.Mockito;
-import org.testng.Assert;
 import org.testng.annotations.Listeners;
 import org.testng.annotations.Test;
 
@@ -1009,10 +1008,9 @@ public void computeIfAbsent_recursive(Map<Int, Int> map, CacheContext context) {
  return map.computeIfAbsent(key, this);
  }
  };
- try {
- map.computeIfAbsent(context.absentKey(), mappingFunction);
- Assert.fail();
- } catch (StackOverflowError | IllegalStateException e) { /* ignored */ }
+ var error = assertThrows(Throwable.class,
+ () -> map.computeIfAbsent(context.absentKey(), mappingFunction));
+ assertThat(error.getClass()).isAnyOf(StackOverflowError.class, IllegalStateException.class);
  }
 
  @CacheSpec
@@ -1023,10 +1021,9 @@ public void computeIfAbsent_pingpong(Map<Int, Int> map, CacheContext context) {
  return map.computeIfAbsent(key.negate(), this);
  }
  };
- try {
- map.computeIfAbsent(context.absentKey(), mappingFunction);
- Assert.fail();
- } catch (StackOverflowError | IllegalStateException e) { /* ignored */ }
+ var error = assertThrows(Throwable.class,
+ () -> map.computeIfAbsent(context.absentKey(), mappingFunction));
+ assertThat(error.getClass()).isAnyOf(StackOverflowError.class, IllegalStateException.class);
  }
 
  @Test(dataProvider = "caches")
@@ -1308,10 +1305,9 @@ public void compute_recursive(Map<Int, Int> map, CacheContext context) {
  return map.compute(key, this);
  }
  };
- try {
- map.compute(context.absentKey(), mappingFunction);
- Assert.fail();
- } catch (StackOverflowError | IllegalStateException e) { /* ignored */ }
+ var error = assertThrows(Throwable.class,
+ () -> map.compute(context.absentKey(), mappingFunction));
+ assertThat(error.getClass()).isAnyOf(StackOverflowError.class, IllegalStateException.class);
  }
 
  @Test(dataProvider = "caches")
@@ -1324,20 +1320,15 @@ public void compute_pingpong(Map<Int, Int> map, CacheContext context) {
  return map.compute(key.equals(key1) ? key2 : key1, this);
  }
  };
- try {
- map.compute(key1, mappingFunction);
- Assert.fail();
- } catch (StackOverflowError | IllegalStateException e) { /* ignored */ }
+ var error = assertThrows(Throwable.class, () -> map.compute(key1, mappingFunction));
+ assertThat(error.getClass()).isAnyOf(StackOverflowError.class, IllegalStateException.class);
  }
 
  @CacheSpec
  @Test(dataProvider = "caches")
  public void compute_error(Map<Int, Int> map, CacheContext context) {
- try {
- map.compute(context.absentKey(), (key, value) -> { throw new IllegalStateException(); });
- Assert.fail();
- } catch (IllegalStateException e) { /* ignored */ }
-
+ assertThrows(IllegalStateException.class, () ->
+ map.compute(context.absentKey(), (key, value) -> { throw new IllegalStateException(); }));
  assertThat(map).isEqualTo(context.original());
  assertThat(context).stats().hits(0).misses(0).success(0).failures(1);
  assertThat(map.compute(context.absentKey(), (k, v) -> intern(k.negate())))
@@ -3095,6 +3086,7 @@ public void writeThroughEntry_equals_hashCode_toString() {
  assertThat(entry.toString()).isNotEqualTo(other.toString());
  }
 
+ @SuppressWarnings("PMD.DoNotExtendJavaLangError")
  static final class ExpectedError extends Error {
  private static final long serialVersionUID = 1L;
  }

caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncAsMapTest.java

@@ -55,7 +55,6 @@
 
 import org.eclipse.collections.impl.factory.Sets;
 import org.mockito.Mockito;
-import org.testng.Assert;
 import org.testng.annotations.Listeners;
 import org.testng.annotations.Test;
 
@@ -829,10 +828,9 @@ public void computeIfAbsent_recursive(AsyncCache<Int, Int> cache, CacheContext c
  return cache.asMap().computeIfAbsent(key, this);
  }
  };
- try {
- cache.asMap().computeIfAbsent(context.absentKey(), mappingFunction);
- Assert.fail();
- } catch (StackOverflowError | IllegalStateException e) { /* ignored */ }
+ var error = assertThrows(Throwable.class,
+ () -> cache.asMap().computeIfAbsent(context.absentKey(), mappingFunction));
+ assertThat(error.getClass()).isAnyOf(StackOverflowError.class, IllegalStateException.class);
  }
 
  @CacheSpec
@@ -843,10 +841,9 @@ public void computeIfAbsent_pingpong(AsyncCache<Int, Int> cache, CacheContext co
  return cache.asMap().computeIfAbsent(key.negate(), this);
  }
  };
- try {
- cache.asMap().computeIfAbsent(context.absentKey(), mappingFunction);
- Assert.fail();
- } catch (StackOverflowError | IllegalStateException e) { /* ignored */ }
+ var error = assertThrows(Throwable.class,
+ () -> cache.asMap().computeIfAbsent(context.absentKey(), mappingFunction));
+ assertThat(error.getClass()).isAnyOf(StackOverflowError.class, IllegalStateException.class);
  }
 
  @Test(dataProvider = "caches")
@@ -1085,10 +1082,9 @@ public void compute_recursive(AsyncCache<Int, Int> cache, CacheContext context)
  return cache.asMap().compute(key, this);
  }
  };
- try {
- cache.asMap().compute(context.absentKey(), mappingFunction);
- Assert.fail();
- } catch (StackOverflowError | IllegalStateException e) { /* ignored */ }
+ var error = assertThrows(Throwable.class,
+ () -> cache.asMap().compute(context.absentKey(), mappingFunction));
+ assertThat(error.getClass()).isAnyOf(StackOverflowError.class, IllegalStateException.class);
  }
 
  @Test(dataProvider = "caches")
@@ -1101,10 +1097,8 @@ public void compute_pingpong(AsyncCache<Int, Int> cache, CacheContext context) {
  return cache.asMap().compute(key.equals(key1) ? key2 : key1, this);
  }
  };
- try {
- cache.asMap().compute(key1, mappingFunction);
- Assert.fail();
- } catch (StackOverflowError | IllegalStateException e) { /* ignored */ }
+ var error = assertThrows(Throwable.class, () -> cache.asMap().compute(key1, mappingFunction));
+ assertThat(error.getClass()).isAnyOf(StackOverflowError.class, IllegalStateException.class);
  }
 
  @Test(dataProvider = "caches")

caffeine/src/test/java/com/github/benmanes/caffeine/cache/AsyncTest.java

@@ -45,6 +45,7 @@ public final class AsyncTest {
  private static final long ONE_MINUTE = TimeUnit.MINUTES.toNanos(1);
 
  @Test
+ @SuppressWarnings("PMD.AvoidAccessibilityAlteration")
  public void reflectivelyConstruct() throws ReflectiveOperationException {
  var constructor = Async.class.getDeclaredConstructor();
  constructor.setAccessible(true);

caffeine/src/test/java/com/github/benmanes/caffeine/cache/BoundedBufferTest.java

@@ -100,6 +100,7 @@ public void overflow() {
  }
 
  @Test
+ @SuppressWarnings("PMD.AvoidAccessibilityAlteration")
  public void reflectivelyConstruct() throws ReflectiveOperationException {
  var constructor = BBHeader.class.getDeclaredConstructor();
  constructor.setAccessible(true);

caffeine/src/test/java/com/github/benmanes/caffeine/cache/BoundedLocalCacheTest.java

@@ -236,6 +236,7 @@ public void clear_pendingWrites_weakKeys(
  /* --------------- Maintenance --------------- */
 
  @Test
+ @SuppressWarnings("PMD.UnusedAssignment")
  public void cleanupTask_allowGc() {
  var cache = new BoundedLocalCache<Object, Object>(
  Caffeine.newBuilder(), /* cacheLoader= */ null, /* isAsync= */ false) {};
@@ -2206,7 +2207,7 @@ public void refreshIfNeeded_liveliness(CacheContext context) {
 
  // Capture the refresh parameters, should not be retired/dead sentinel entry
  var refreshEntry = new AtomicReference<Map.Entry<Object, Object>>();
- var cache = asBoundedLocalCache(context.build(new CacheLoader<Object, Object>() {
+ var cache = asBoundedLocalCache(context.build(new CacheLoader<>() {
  @Override public Int load(Object key) {
  throw new AssertionError();
  }
@@ -2526,6 +2527,7 @@ private static void checkBrokenEqualityMessage(
  /* --------------- Miscellaneous --------------- */
 
  @Test
+ @SuppressWarnings("PMD.AvoidAccessibilityAlteration")
  public void reflectivelyConstruct() throws ReflectiveOperationException {
  var constructor = BLCHeader.class.getDeclaredConstructor();
  constructor.setAccessible(true);

caffeine/src/test/java/com/github/benmanes/caffeine/cache/CacheTest.java

@@ -333,7 +333,8 @@ public void getAllPresent_ordered(Cache<Int, Int> cache, CacheContext context) {
  @Test(dataProvider = "caches")
  @CacheSpec(population = Population.EMPTY)
  public void getAllPresent_jdk8186171(Cache<Object, Int> cache, CacheContext context) {
- class Key {
+ @SuppressWarnings("PMD.OverrideBothEqualsAndHashcode")
+ final class Key {
  @Override public int hashCode() {
  return 0; // to put keys in one bucket
  }
@@ -582,7 +583,8 @@ public void getAll_present_ordered_exceeds(Cache<Int, Int> cache, CacheContext c
  @Test(dataProvider = "caches")
  @CacheSpec(population = Population.EMPTY)
  public void getAll_jdk8186171(CacheContext context) {
- class Key {
+ @SuppressWarnings("PMD.OverrideBothEqualsAndHashcode")
+ final class Key {
  @Override public int hashCode() {
  return 0; // to put keys in one bucket
  }
@@ -942,6 +944,7 @@ public void serialize(Cache<Int, Int> cache, CacheContext context) {
 
  @CheckNoStats
  @Test(dataProvider = "caches")
+ @SuppressWarnings("PMD.AvoidAccessibilityAlteration")
  @CacheSpec(implementation = Implementation.Caffeine, population = Population.EMPTY)
  public void readObject(CacheContext context) throws NoSuchMethodException {
  var cache = context.isAsync() ? context.asyncCache() : context.cache();

caffeine/src/test/java/com/github/benmanes/caffeine/cache/CaffeineSpecGuavaTest.java

@@ -33,7 +33,8 @@
  *
  * @author Adam Winer
  */
-@SuppressWarnings("PreferJavaTimeOverload")
+@SuppressWarnings({"PMD.DetachedTestCase",
+ "PMD.JUnit4TestShouldUseTestAnnotation", "PreferJavaTimeOverload"})
 public class CaffeineSpecGuavaTest extends TestCase {
 
  public void testParse_empty() {

caffeine/src/test/java/com/github/benmanes/caffeine/cache/CaffeineTest.java

@@ -205,13 +205,13 @@ public void loading_nullLoader() {
  @Test
  public void async_weakValues() {
  var builder = Caffeine.newBuilder().weakValues();
- assertThrows(IllegalStateException.class, () -> builder.buildAsync());
+ assertThrows(IllegalStateException.class, builder::buildAsync);
  }
 
  @Test
  public void async_softValues() {
  var builder = Caffeine.newBuilder().softValues();
- assertThrows(IllegalStateException.class, () -> builder.buildAsync());
+ assertThrows(IllegalStateException.class, builder::buildAsync);
  }
 
  @Test

caffeine/src/test/java/com/github/benmanes/caffeine/cache/EvictionTest.java

@@ -946,9 +946,13 @@ public void merge_update(Cache<Int, Int> cache,
  maximumSize = Maximum.FULL, weigher = CacheWeigher.MOCKITO)
  public void refresh_weigherFails_absent(LoadingCache<Int, Int> cache, CacheContext context) {
  when(context.weigher().weigh(any(), any())).thenThrow(IllegalStateException.class);
- try {
- cache.refresh(context.absentKey()).join();
- } catch (IllegalStateException e) { /* ignored */ }
+ if (context.isAsync()) {
+ assertThrows(IllegalStateException.class, () -> cache.refresh(context.absentKey()).join());
+ } else if (context.isCaffeine()) {
+ assertThat(cache.refresh(context.absentKey())).succeedsWith(context.absentKey().negate());
+ } else {
+ assertThat(cache.refresh(context.absentKey())).succeedsWithNull();
+ }
  assertThat(cache).doesNotContainKey(context.absentKey());
  }
 
@@ -973,10 +977,9 @@ public void refresh_weigherFails_absent_async(
  @CacheSpec(population = Population.FULL, loader = Loader.IDENTITY,
  maximumSize = Maximum.FULL, weigher = CacheWeigher.MOCKITO)
  public void refresh_weigherFails_present(LoadingCache<Int, Int> cache, CacheContext context) {
+ // the refresh's reload is successful but updating the cache fails, is logged, and is discarded
  when(context.weigher().weigh(any(), any())).thenThrow(IllegalStateException.class);
- try {
- cache.refresh(context.firstKey()).join();
- } catch (IllegalStateException e) { /* ignored */ }
+ assertThat(cache.refresh(context.firstKey())).succeedsWith(context.firstKey());
  assertThat(cache).containsExactlyEntriesIn(context.original());
  }
 

caffeine/src/test/java/com/github/benmanes/caffeine/cache/ExpireAfterVarTest.java

@@ -58,7 +58,6 @@
 import java.util.function.BiFunction;
 
 import org.mockito.Mockito;
-import org.testng.Assert;
 import org.testng.annotations.Listeners;
 import org.testng.annotations.Test;
 
@@ -1233,10 +1232,9 @@ public void compute_recursive(CacheContext context, VarExpiration<Int, Int> expi
  return expireAfterVar.compute(key, this, Duration.ofDays(1));
  }
  };
- try {
- expireAfterVar.compute(context.absentKey(), mappingFunction, Duration.ofDays(1));
- Assert.fail();
- } catch (StackOverflowError | IllegalStateException e) { /* ignored */ }
+ var error = assertThrows(Throwable.class, () ->
+ expireAfterVar.compute(context.absentKey(), mappingFunction, Duration.ofDays(1)));
+ assertThat(error.getClass()).isAnyOf(StackOverflowError.class, IllegalStateException.class);
  }
 
  @Test(dataProvider = "caches")
@@ -1250,10 +1248,9 @@ public void compute_pingpong(CacheContext context, VarExpiration<Int, Int> expir
  return expireAfterVar.compute(key.equals(key1) ? key2 : key1, this, Duration.ofDays(1));
  }
  };
- try {
- expireAfterVar.compute(key1, mappingFunction, Duration.ofDays(1));
- Assert.fail();
- } catch (StackOverflowError | IllegalStateException e) { /* ignored */ }
+ var error = assertThrows(Throwable.class, () ->
+ expireAfterVar.compute(key1, mappingFunction, Duration.ofDays(1)));
+ assertThat(error.getClass()).isAnyOf(StackOverflowError.class, IllegalStateException.class);
  }
 
  @Test(dataProvider = "caches")

caffeine/src/test/java/com/github/benmanes/caffeine/cache/InternerTest.java

@@ -48,6 +48,7 @@
  */
 public final class InternerTest extends TestCase {
 
+ @SuppressWarnings("PMD.JUnit4SuitesShouldUseSuiteAnnotation")
  public static TestSuite suite() {
  return SetTestSuiteBuilder
  .using(new TestStringSetGenerator() {
@@ -85,6 +86,7 @@ public void intern(Interner<Int> interner) {
  }
 
  @Test
+ @SuppressWarnings("PMD.UnusedAssignment")
  public void intern_weak_replace() {
  var canonical = new Int(1);
  var other = new Int(1);
@@ -102,6 +104,7 @@ public void intern_weak_replace() {
  }
 
  @Test
+ @SuppressWarnings("PMD.UnusedAssignment")
  public void intern_weak_remove() {
  var canonical = new Int(1);
  var next = new Int(2);