Upgrade to PMD 6.0 and enable optional ErrorProne checks

ben-manes · Dec 27, 2017 · e785458 · e785458
1 parent 78e97fc
commit e785458
Show file tree

Hide file tree

Showing 30 changed files with 149 additions and 131 deletions.
diff --git a/build.gradle b/build.gradle
@@ -48,6 +48,7 @@ allprojects {
 
 subprojects {
   apply plugin: 'com.github.ethankhall.semantic-versioning'
+  apply plugin: 'net.ltgt.errorprone'
   apply plugin: 'propdeps-maven'
   apply plugin: 'eclipse'
   apply plugin: 'java'
@@ -61,11 +62,6 @@ subprojects {
 
   if (JavaVersion.current().isJava9Compatible()) {
     tasks.uploadArchives.enabled = false
-  } else {
-    apply plugin: 'net.ltgt.errorprone'
-    dependencies {
-      errorprone libraries.errorProneCore
-    }
   }
 
   sourceCompatibility = JavaVersion.VERSION_1_8
@@ -87,6 +83,8 @@ subprojects {
     provided libraries.jsr305
     provided libraries.errorProneAnnotations
 
+    errorprone libraries.errorProneCore
+
     testCompile libraries.guava
     testCompile testLibraries.mockito
     testCompile testLibraries.hamcrest

diff --git a/...eine/src/javaPoet/java/com/github/benmanes/caffeine/cache/LocalCacheFactoryGenerator.java b/...eine/src/javaPoet/java/com/github/benmanes/caffeine/cache/LocalCacheFactoryGenerator.java
@@ -32,6 +32,7 @@
 import java.nio.file.Paths;
 import java.time.Year;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.NavigableMap;
@@ -204,10 +205,7 @@ private Set<Feature> getFeatures(List<Object> combination) {
 
   private Set<List<Object>> combinations() {
     Set<Boolean> options = ImmutableSet.of(true, false);
-    List<Set<Boolean>> sets = new ArrayList<>();
-    for (int i = 0; i < featureByIndex.length; i++) {
-      sets.add(options);
-    }
+    List<Set<Boolean>> sets = Collections.nCopies(featureByIndex.length, options);
     return Sets.cartesianProduct(sets);
   }
 

diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java
@@ -1864,7 +1864,7 @@ public V replace(K key, V value) {
     K[] nodeKey = (K[]) new Object[1];
     @SuppressWarnings("unchecked")
     V[] oldValue = (V[]) new Object[1];
-    long now[] = new long[1];
+    long[] now = new long[1];
     int weight = weigher.weigh(key, value);
     Node<K, V> node = data.computeIfPresent(nodeFactory.newLookupKey(key), (k, n) -> {
       synchronized (n) {

diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Caffeine.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Caffeine.java
@@ -15,6 +15,7 @@
  */
 package com.github.benmanes.caffeine.cache;
 
+import static java.util.Locale.US;
 import static java.util.Objects.requireNonNull;
 
 import java.lang.ref.SoftReference;
@@ -979,10 +980,10 @@ public String toString() {
       s.append("refreshNanos=").append(refreshNanos).append("ns, ");
     }
     if (keyStrength != null) {
-      s.append("keyStrength=").append(keyStrength.toString().toLowerCase()).append(", ");
+      s.append("keyStrength=").append(keyStrength.toString().toLowerCase(US)).append(", ");
     }
     if (valueStrength != null) {
-      s.append("valueStrength=").append(valueStrength.toString().toLowerCase()).append(", ");
+      s.append("valueStrength=").append(valueStrength.toString().toLowerCase(US)).append(", ");
     }
     if (removalListener != null) {
       s.append("removalListener, ");

diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/MpscGrowableArrayQueue.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/MpscGrowableArrayQueue.java
@@ -210,6 +210,7 @@ public String toString() {
   }
 
   @Override
+  @SuppressWarnings("MissingDefault")
   public boolean offer(final E e) {
     if (null == e) {
       throw new NullPointerException();
@@ -371,7 +372,7 @@ private E[] getNextBuffer(final E[] buffer, final long mask) {
     return nextBuffer;
   }
 
-  private long nextArrayOffset(final long mask) {
+  private static long nextArrayOffset(final long mask) {
     return modifiedCalcElementOffset(mask + 2, Long.MAX_VALUE);
   }
 

diff --git a/config/pmd/rulesSets.xml b/config/pmd/rulesSets.xml
@@ -5,121 +5,97 @@
     xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
   <description>PMD rules for Caffeine</description>
 
-  <rule ref="rulesets/java/basic.xml">
-    <exclude name="AvoidBranchingStatementAsLastInLoop"/>
-  </rule>
-
-  <rule ref="rulesets/java/braces.xml"/>
-
-  <rule ref="rulesets/java/clone.xml"/>
-
-  <rule ref="rulesets/java/codesize.xml">
-    <exclude name="NcssTypeCount"/>
-    <exclude name="TooManyMethods"/>
-    <exclude name="ExcessivePublicCount"/>
-    <exclude name="ExcessiveClassLength"/>
-    <exclude name="ExcessiveMethodLength"/>
-
-    <exclude name="NPathComplexity"/>
-    <exclude name="CyclomaticComplexity"/>
-    <exclude name="StdCyclomaticComplexity"/>
-    <exclude name="ModifiedCyclomaticComplexity"/>
-  </rule>
-
-  <rule ref="rulesets/java/comments.xml">
-    <exclude name="CommentSize"/>
-    <exclude name="CommentRequired"/>
-    <exclude name="CommentDefaultAccessModifier"/>
-  </rule>
-
-  <rule ref="rulesets/java/coupling.xml">
-    <exclude name="LawOfDemeter"/>
-    <exclude name="ExcessiveImports"/>
-    <exclude name="LoosePackageCoupling"/>
-    <exclude name="CouplingBetweenObjects"/>
-  </rule>
-
-  <rule ref="rulesets/java/design.xml">
-    <exclude name="GodClass"/>
+  <rule ref="category/java/bestpractices.xml">
     <exclude name="UseVarargs"/>
-    <exclude name="ConfusingTernary"/>
-    <exclude name="OptimizableToArrayCall"/>
-    <exclude name="CompareObjectsWithEquals"/>
-    <exclude name="UncommentedEmptyMethodBody"/>
+    <exclude name="SystemPrintln"/>
+    <exclude name="OneDeclarationPerLine"/>
     <exclude name="AvoidReassigningParameters"/>
-    <exclude name="UncommentedEmptyConstructor"/>
-    <exclude name="UnnecessaryLocalBeforeReturn"/>
-    <exclude name="UseLocaleWithCaseConversions"/>
-    <exclude name="SwitchStmtsShouldHaveDefault"/>
-    <exclude name="AvoidSynchronizedAtMethodLevel"/>
-    <exclude name="ConstructorCallsOverridableMethod"/>
     <exclude name="PositionLiteralsFirstInComparisons"/>
-    <exclude name="FieldDeclarationsShouldBeAtStartOfClass"/>
-    <exclude name="EmptyMethodInAbstractClassShouldBeAbstract"/>
     <exclude name="PositionLiteralsFirstInCaseInsensitiveComparisons"/>
   </rule>
-  <rule ref="rulesets/java/design.xml/ConfusingTernary">
-    <properties>
-      <property name="ignoreElseIf" value="true"/>
-    </properties>
-  </rule>
-
-  <rule ref="rulesets/java/empty.xml">
-    <exclude name="EmptyWhileStmt"/>
-  </rule>
-
-  <rule ref="rulesets/java/finalizers.xml"/>
 
-  <rule ref="rulesets/java/imports.xml">
-    <exclude name="DontImportJavaLang"/>
-    <exclude name="TooManyStaticImports"/>
-  </rule>
-
-  <rule ref="rulesets/java/naming.xml">
+  <rule ref="category/java/codestyle.xml">
     <exclude name="LongVariable"/>
+    <exclude name="OnlyOneReturn"/>
     <exclude name="ShortVariable"/>
-    <exclude name="ShortClassName"/>
     <exclude name="AbstractNaming"/>
-    <exclude name="VariableNamingConventions"/>
-    <exclude name="AvoidFieldNameMatchingMethodName"/>
-  </rule>
-
-  <rule ref="rulesets/java/optimizations.xml">
+    <exclude name="DefaultPackage"/>
+    <exclude name="ShortClassName"/>
+    <exclude name="ConfusingTernary"/>
+    <exclude name="DontImportJavaLang"/>
+    <exclude name="UselessParentheses"/>
     <exclude name="PrematureDeclaration"/>
+    <exclude name="TooManyStaticImports"/>
+    <exclude name="AtLeastOneConstructor"/>
+    <exclude name="CallSuperInConstructor"/>
     <exclude name="LocalVariableCouldBeFinal"/>
+    <exclude name="VariableNamingConventions"/>
     <exclude name="MethodArgumentCouldBeFinal"/>
-    <exclude name="AvoidInstantiatingObjectsInLoops"/>
+    <exclude name="CommentDefaultAccessModifier"/>
+    <exclude name="FieldDeclarationsShouldBeAtStartOfClass"/>
+    <exclude name="EmptyMethodInAbstractClassShouldBeAbstract"/>
+  </rule>
+  <rule ref="category/java/codestyle.xml/ConfusingTernary">
+    <properties>
+      <property name="ignoreElseIf" value="true"/>
+    </properties>
   </rule>
 
-  <rule ref="rulesets/java/strictexception.xml">
-    <exclude name="AvoidCatchingThrowable"/>
+  <rule ref="category/java/design.xml">
+    <exclude name="GodClass"/>
+    <exclude name="DataClass"/>
+    <exclude name="NcssCount"/>
+    <exclude name="LawOfDemeter"/>
+    <exclude name="TooManyMethods"/>
+    <exclude name="NPathComplexity"/>
+    <exclude name="ExcessiveImports"/>
+    <exclude name="CyclomaticComplexity"/>
+    <exclude name="ExcessiveClassLength"/>
+    <exclude name="ExcessivePublicCount"/>
+    <exclude name="LoosePackageCoupling"/>
+    <exclude name="ExcessiveMethodLength"/>
+    <exclude name="CouplingBetweenObjects"/>
     <exclude name="AvoidRethrowingException"/>
     <exclude name="AvoidCatchingGenericException"/>
     <exclude name="AvoidThrowingRawExceptionTypes"/>
   </rule>
 
-  <rule ref="rulesets/java/strings.xml">
+  <rule ref="category/java/documentation.xml">
+    <exclude name="CommentSize"/>
+    <exclude name="CommentRequired"/>
+    <exclude name="UncommentedEmptyMethodBody"/>
+    <exclude name="UncommentedEmptyConstructor"/>
+  </rule>
+
+  <rule ref="category/java/errorprone.xml">
+    <exclude name="DontImportSun"/>
+    <exclude name="EmptyWhileStmt"/>
+    <exclude name="NullAssignment"/>
+    <exclude name="AssignmentInOperand"/>
+    <exclude name="UseProperClassLoader"/>
+    <exclude name="AvoidCatchingThrowable"/>
     <exclude name="AvoidDuplicateLiterals"/>
+    <exclude name="DataflowAnomalyAnalysis"/>
+    <exclude name="CompareObjectsWithEquals"/>
+    <exclude name="AvoidLiteralsInIfCondition"/>
+    <exclude name="BeanMembersShouldSerialize"/>
+    <exclude name="AvoidFieldNameMatchingMethodName"/>
+    <exclude name="ConstructorCallsOverridableMethod"/>
+    <exclude name="AvoidBranchingStatementAsLastInLoop"/>
   </rule>
-  <rule ref="rulesets/java/strings.xml/AvoidDuplicateLiterals">
+  <rule ref="category/java/errorprone.xml/AvoidDuplicateLiterals">
     <properties>
       <property name="skipAnnotations" value="true"/>
     </properties>
   </rule>
 
-  <rule ref="rulesets/java/sunsecure.xml"/>
-
-  <rule ref="rulesets/java/typeresolution.xml"/>
-
-  <rule ref="rulesets/java/unnecessary.xml">
-    <exclude name="UselessParentheses"/>  
+  <rule ref="category/java/multithreading.xml">
+    <exclude name="DoNotUseThreads"/>
+    <exclude name="AvoidUsingVolatile"/>
+    <exclude name="UseConcurrentHashMap"/>
   </rule>
 
-  <rule ref="rulesets/java/unusedcode.xml">
-    <exclude name="UnusedPrivateMethod"/>
+  <rule ref="category/java/performance.xml">
+    <exclude name="AvoidInstantiatingObjectsInLoops"/>
   </rule>
-
-  <!--
-  <rule ref="rulesets/java/controversial.xml"/>
-  -->
 </ruleset>
diff --git a/gradle/codeQuality.gradle b/gradle/codeQuality.gradle
@@ -74,6 +74,26 @@ sonarqube {
 }
 tasks.sonarqube.dependsOn(jacocoMerge)
 
+tasks.withType(JavaCompile) {
+  options.compilerArgs += [
+    '-Xep:ClassName',
+    '-Xep:ComparisonContractViolated',
+    '-Xep:DepAnn',
+    '-Xep:DivZero',
+    '-Xep:LockMethodChecker',
+    '-Xep:LongLiteralLowerCaseSuffix',
+    '-Xep:NoAllocation',
+    '-Xep:NumericEquality',
+    '-Xep:UnlockMethod',
+    '-Xep:FunctionalInterfaceClash',
+    '-Xep:MissingDefault',
+    '-Xep:RedundantThrows',
+    '-Xep:UnnecessaryDefaultInEnumSwitch',
+    '-Xep:MixedArrayDimensions',
+    '-Xep:RemoveUnusedImports',
+  ]
+}
+
 tasks.withType(Test) {
   if (System.properties.containsKey('debug')) {
     jvmArgs '-Xdebug', '-Xrunjdwp:transport=dt_socket,server=y,suspend=y,address=5005'

diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle
@@ -83,7 +83,7 @@ ext {
     jmh: '0.4.5',
     jmhReport: '0.7.0',
     nexus: '2.3.1',
-    pmd: '5.8.1',
+    pmd: '6.0.0',
     propdeps: '0.0.10.RELEASE',
     semanticVersioning: '1.1.0',
     shadow: '2.0.2',

diff --git a/guava/src/main/java/com/github/benmanes/caffeine/guava/CaffeinatedGuavaCache.java b/guava/src/main/java/com/github/benmanes/caffeine/guava/CaffeinatedGuavaCache.java
@@ -48,7 +48,7 @@
 class CaffeinatedGuavaCache<K, V> implements Cache<K, V>, Serializable {
   static final long serialVersionUID = 1L;
 
-  private final com.github.benmanes.caffeine.cache.Cache<K, V> cache;
+  final com.github.benmanes.caffeine.cache.Cache<K, V> cache;
 
   CaffeinatedGuavaCache(com.github.benmanes.caffeine.cache.Cache<K, V> cache) {
     this.cache = requireNonNull(cache);

diff --git a/jcache/src/main/java/com/github/benmanes/caffeine/jcache/CacheProxy.java b/jcache/src/main/java/com/github/benmanes/caffeine/jcache/CacheProxy.java
@@ -793,7 +793,7 @@ public <T> T invoke(K key, EntryProcessor<K, V, T> entryProcessor, Object... arg
   }
 
   /** Returns the updated expirable value after performing the post processing actions. */
-  @SuppressWarnings({"fallthrough", "PMD.MissingBreakInSwitch"})
+  @SuppressWarnings({"fallthrough", "PMD.MissingBreakInSwitch", "PMD.SwitchStmtsShouldHaveDefault"})
   private Expirable<V> postProcess(Expirable<V> expirable,
       EntryProcessorEntry<K, V> entry, long currentTimeMS) {
     switch (entry.getAction()) {

diff --git a/jcache/src/main/java/com/github/benmanes/caffeine/jcache/copy/AbstractCopier.java b/jcache/src/main/java/com/github/benmanes/caffeine/jcache/copy/AbstractCopier.java
@@ -133,7 +133,7 @@ private boolean isArrayOfImmutableTypes(Class<?> clazz) {
   }
 
   /** @return a shallow copy of the array. */
-  private <T> T arrayCopy(T object) {
+  private static <T> T arrayCopy(T object) {
     int length = Array.getLength(object);
     @SuppressWarnings("unchecked")
     T copy = (T) Array.newInstance(object.getClass().getComponentType(), length);

diff --git a/jcache/src/main/java/com/github/benmanes/caffeine/jcache/event/EventTypeAwareListener.java b/jcache/src/main/java/com/github/benmanes/caffeine/jcache/event/EventTypeAwareListener.java
@@ -48,6 +48,7 @@ public EventTypeAwareListener(CacheEntryListener<? super K, ? super V> listener)
   }
 
   /** Returns if the backing listener consumes this type of event. */
+  @SuppressWarnings("PMD.SwitchStmtsShouldHaveDefault")
   public boolean isCompatible(@Nonnull EventType eventType) {
     switch (eventType) {
       case CREATED:
@@ -63,6 +64,7 @@ public boolean isCompatible(@Nonnull EventType eventType) {
   }
 
   /** Processes the event and logs if an exception is thrown. */
+  @SuppressWarnings("PMD.SwitchStmtsShouldHaveDefault")
   public void dispatch(@Nonnull JCacheEntryEvent<K, V> event) {
     try {
       if (event.getSource().isClosed()) {

diff --git a/jcache/src/main/java/com/github/benmanes/caffeine/jcache/event/EventTypeFilter.java b/jcache/src/main/java/com/github/benmanes/caffeine/jcache/event/EventTypeFilter.java
@@ -49,6 +49,7 @@ public boolean evaluate(CacheEntryEvent<? extends K, ? extends V> event) {
     return isCompatible(event) && filter.evaluate(event);
   }
 
+  @SuppressWarnings("PMD.SwitchStmtsShouldHaveDefault")
   private boolean isCompatible(CacheEntryEvent<? extends K, ? extends V> event) {
     switch (event.getEventType()) {
       case CREATED:

diff --git a/jcache/src/main/java/com/github/benmanes/caffeine/jcache/processor/EntryProcessorEntry.java b/jcache/src/main/java/com/github/benmanes/caffeine/jcache/processor/EntryProcessorEntry.java
@@ -84,7 +84,7 @@ public void remove() {
   public void setValue(V value) {
     requireNonNull(value);
     if (action != Action.CREATED) {
-      action = (exists() && hasEntry) ? Action.UPDATED : Action.CREATED;
+      action = (hasEntry && exists()) ? Action.UPDATED : Action.CREATED;
     }
     this.value = value;
   }

diff --git a/simulator/build.gradle b/simulator/build.gradle
@@ -48,6 +48,10 @@ jar.manifest {
     'com.github.benmanes.caffeine.simulator'
 }
 
+tasks.withType(Pmd) {
+  enabled &= JavaVersion.current().isJava9Compatible()
+}
+
 tasks.withType(Javadoc) {
   options.addStringOption('Xdoclint:none', '-quiet')
   // https://github.com/akka/akka/issues/21165