[REEF-2061] Upgrade checkstyle and related plugins to newer versions …

…and fix new style issues in the code (#1502) Summary of changes: * Update CheckStyle to version 8.29 * Update Maven CheckStyle plugin to version 3.1.0 * Update CheckStyle configuration files to match the newer version (remove outdated parameters, etc.) * Update the Java code and fix the newly found CheckStyle warnings JIRA: [REEF-2061](https://issues.apache.org/jira/projects/REEF/issues/REEF-2061)
apache · May 22, 2020 · 6cfa540 · 6cfa540
1 parent 067e162
commit 6cfa540
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 57 deletions.
diff --git a/lang/java/reef-common/src/main/resources/checkstyle-strict.xml b/lang/java/reef-common/src/main/resources/checkstyle-strict.xml
@@ -1,7 +1,7 @@
 <?xml version="1.0"?>
 <!DOCTYPE module PUBLIC
-        "-//Puppy Crawl//DTD Check Configuration 1.2//EN"
-        "http://www.puppycrawl.com/dtds/configuration_1_2.dtd">
+        "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
+        "https://checkstyle.org/dtds/configuration_1_3.dtd">
 
 <!--
    Licensed to the Apache Software Foundation (ASF) under one or more
@@ -81,6 +81,11 @@
         <property name="fileExtensions" value="java"/>
     </module>
 
+    <module name="LineLength">
+        <property name="ignorePattern" value="^import"/>
+        <property name="max" value="120"/>
+    </module>
+
     <module name="TreeWalker">
         <!-- not a check -->
         <module name="SuppressWarningsHolder"/>
@@ -115,10 +120,6 @@
 
         <!-- Checks for Size Violations.                    -->
         <!-- See http://checkstyle.sf.net/config_sizes.html -->
-        <module name="LineLength">
-            <property name="ignorePattern" value="^import"/>
-            <property name="max" value="120"/>
-        </module>
         <module name="MethodLength"/>
         <module name="ParameterNumber">
             <property name="max" value="7"/>
@@ -174,10 +175,6 @@
         <module name="SimplifyBooleanExpression"/>
         <module name="SimplifyBooleanReturn"/>
 
-        <!-- Detects places in Java code where the diamond operator can be used -->
-        <!-- See REEF-838 -->
-        <module name="DiamondOperatorForVariableDefinition"/>
-
         <module name="FinalParameters"/>
         <module name="UnnecessaryParentheses"/>
 
@@ -188,6 +185,7 @@
         <module name="InterfaceIsType"/>
         <module name="VisibilityModifier">
             <property name="protectedAllowed" value="true"/>
+            <property name="packageAllowed" value="true"/>
         </module>
 
 
@@ -214,9 +212,6 @@
         <!-- Exception-handling checks. -->
         <!-- See https://issues.apache.org/jira/browse/REEF-864 -->
         <module name="MutableException"/>
-        <module name="ForbidReturnInFinallyBlockCheck"/>
-        <module name="ForbidThrowAnonymousExceptionsCheck"/>
-        <module name="UselessSingleCatchCheck"/>
 
         <module name="EmptyCatchBlock">
             <property name="commentFormat" value="This is expected"/>
@@ -237,13 +232,6 @@
             <property name="classes" value="java.lang.Throwable, java.lang.Error"/>
         </module>
 
-        <module name="AvoidHidingCauseExceptionCheck" />
-
-        <!-- ConstructorWithoutParams is suppressed for files with the word "Test" in the filename -->
-        <module name="ConstructorWithoutParamsCheck" >
-            <property name="classNameFormat" value=".*Exception$" />
-            <property name="ignoredClassNameFormat" value="UnsupportedOperationException|NotImplementedException|NoSuchElementException" />
-        </module>
 
         <!-- Checks for System.exit() call -->
         <!-- See https://issues.apache.org/jira/browse/REEF-1139 -->

diff --git a/lang/java/reef-common/src/main/resources/checkstyle-suppress.xml b/lang/java/reef-common/src/main/resources/checkstyle-suppress.xml
@@ -1,7 +1,7 @@
 <?xml version="1.0"?>
 <!DOCTYPE suppressions PUBLIC
-        "-//Puppy Crawl//DTD Suppressions 1.0//EN"
-        "http://www.puppycrawl.com/dtds/suppressions_1_0.dtd">
+        "-//Checkstyle//DTD SuppressionFilter Configuration 1.2//EN"
+        "https://checkstyle.org/dtds/suppressions_1_2.dtd">
 
 <!--
    Licensed to the Apache Software Foundation (ASF) under one or more

diff --git a/lang/java/reef-common/src/main/resources/checkstyle.xml b/lang/java/reef-common/src/main/resources/checkstyle.xml
@@ -1,7 +1,7 @@
 <?xml version="1.0"?>
 <!DOCTYPE module PUBLIC
-        "-//Puppy Crawl//DTD Check Configuration 1.2//EN"
-        "http://www.puppycrawl.com/dtds/configuration_1_2.dtd">
+        "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
+        "https://checkstyle.org/dtds/configuration_1_3.dtd">
 
 <!--
    Licensed to the Apache Software Foundation (ASF) under one or more
@@ -81,6 +81,11 @@
         <property name="fileExtensions" value="java"/>
     </module>
 
+    <module name="LineLength">
+        <property name="ignorePattern" value="^import"/>
+        <property name="max" value="120"/>
+    </module>
+
     <module name="TreeWalker">
         <!-- not a check -->
         <module name="SuppressWarningsHolder"/>
@@ -117,10 +122,6 @@
 
         <!-- Checks for Size Violations.                    -->
         <!-- See http://checkstyle.sf.net/config_sizes.html -->
-        <module name="LineLength">
-            <property name="ignorePattern" value="^import"/>
-            <property name="max" value="120"/>
-        </module>
         <module name="MethodLength"/>
         <module name="ParameterNumber">
             <property name="max" value="7"/>
@@ -176,10 +177,6 @@
         <module name="SimplifyBooleanExpression"/>
         <module name="SimplifyBooleanReturn"/>
 
-        <!-- Detects places in Java code where the diamond operator can be used -->
-        <!-- See REEF-838 -->
-        <module name="DiamondOperatorForVariableDefinition" />
-
         <module name="FinalParameters"/>
         <module name="UnnecessaryParentheses"/>
 
@@ -190,9 +187,9 @@
         <module name="InterfaceIsType"/>
         <module name="VisibilityModifier">
             <property name="protectedAllowed" value="true"/>
+            <property name="packageAllowed" value="true"/>
         </module>
 
-
         <!-- Miscellaneous other checks.                   -->
         <!-- See http://checkstyle.sf.net/config_misc.html -->
         <module name="ArrayTypeStyle"/>
@@ -217,9 +214,6 @@
         <!-- Exception-handling checks. -->
         <!-- See https://issues.apache.org/jira/browse/REEF-864 -->
         <module name="MutableException"/>
-        <module name="ForbidReturnInFinallyBlockCheck"/>
-        <module name="ForbidThrowAnonymousExceptionsCheck"/>
-        <module name="UselessSingleCatchCheck"/>
         <module name="EmptyCatchBlock">
             <property name="commentFormat" value="This is expected"/>
             <property name="exceptionVariableName" value="expected|ignored"/>
@@ -239,14 +233,6 @@
             <property name="classes" value="java.lang.Throwable, java.lang.Error"/>
         </module>
 
-        <module name="AvoidHidingCauseExceptionCheck" />
-
-        <!-- ConstructorWithoutParams is suppressed for files with the word "Test" in the filename -->
-        <module name="ConstructorWithoutParamsCheck" >
-          <property name="classNameFormat" value=".*Exception$" />
-          <property name="ignoredClassNameFormat" value="UnsupportedOperationException|NotImplementedException|NoSuchElementException" />
-        </module>
-
         <!-- Checks for System.exit() call -->
         <!-- See https://issues.apache.org/jira/browse/REEF-1139 -->
         <module name="Regexp">
@@ -259,4 +245,3 @@
     </module>
 
 </module>
-
diff --git a/pom.xml b/pom.xml
@@ -59,9 +59,9 @@ under the License.
         <httpclient.version>4.5.6</httpclient.version>
         <jackson.version>2.10.2</jackson.version>
         <protobuf.version>2.5.0</protobuf.version>
-        <maven-checkstyle-plugin.version>2.17</maven-checkstyle-plugin.version>
-        <sevntu.checkstyle.plugin.version>1.20.0</sevntu.checkstyle.plugin.version>
-        <checkstyle.version>6.17</checkstyle.version>
+        <maven-checkstyle-plugin.version>3.1.0</maven-checkstyle-plugin.version>
+        <sevntu.checkstyle.plugin.version>1.35.0</sevntu.checkstyle.plugin.version>
+        <checkstyle.version>8.29</checkstyle.version>
         <findbugs.version>3.0.4</findbugs.version>
         <reflections.version>0.9.9-RC1</reflections.version>
         <jsr305.version>3.0.1</jsr305.version>
@@ -112,14 +112,6 @@ under the License.
         <maven>3.0</maven>
     </prerequisites>
 
-    <pluginRepositories>
-        <pluginRepository>
-            <id>sevntu-maven</id>
-            <name>sevntu-maven</name>
-            <url>http://sevntu-checkstyle.github.io/sevntu.checkstyle/maven2</url>
-        </pluginRepository>
-    </pluginRepositories>
-
     <build>
         <pluginManagement>
             <plugins>
@@ -353,7 +345,7 @@ under the License.
                             <version>${checkstyle.version}</version>
                         </dependency>
                         <dependency>
-                            <groupId>com.github.sevntu.checkstyle</groupId>
+                            <groupId>com.github.sevntu-checkstyle</groupId>
                             <artifactId>sevntu-checkstyle-maven-plugin</artifactId>
                             <version>${sevntu.checkstyle.plugin.version}</version>
                         </dependency>