Enable checkstyle and fix errors it calls out

Updated checkstyle to use the latest version. Enabled it in the build process, and fixed a lot of problems that it found. Note that some checkstyle rules are currently disabled because there are too many violations to fix right now. I hope to address these in the future, but the set of changes here are good.
dirmgr · Mar 31, 2019 · b2a384b · b2a384b
1 parent 9e9f417
commit b2a384b
Show file tree

Hide file tree

Showing 88 changed files with 2,478 additions and 1,945 deletions.
diff --git a/.gitignore b/.gitignore
@@ -1,4 +1,5 @@
 build
 src/com/slamd/common/DynamicConstants.java
+.checkstyle-cache-main
 .idea
 *.iml
diff --git a/build.xml b/build.xml
@@ -110,7 +110,7 @@ package com.slamd.common;
  * This file has been dynamically generated as part of the SLAMD build
  * process.  Do not edit it directly.
  */
-public class DynamicConstants
+public final class DynamicConstants
 {
   /**
    * Indicates whether this is an official build.
@@ -158,6 +158,16 @@ public class DynamicConstants
    * The version qualifier string for this SLAMD build.
    */
   public static final String VERSION_QUALIFIER = "${VERSION_QUALIFIER}";
+
+
+
+  /**
+   * Prevent this utility class from being instantiated.
+   */
+  private DynamicConstants()
+  {
+    // No implementation required.
+  }
 }
 </echo>
   </target>
@@ -335,7 +345,8 @@ public class DynamicConstants
 
 
   <!-- Package the SLAMD Server into a ZIP file -->
-  <target name="package" depends="clean,init,build,create-package-dir"
+  <target name="package"
+          depends="clean,init,build,checkstyle,create-package-dir"
           description="Package the SLAMD Server for distribution">
     <echo message="Packaging SLAMD...." />
 
@@ -422,12 +433,13 @@ public class DynamicConstants
   <!-- Perform Basic Code Style Checks -->
   <target name="checkstyle" description="Perform basic code style checks">
     <echo message="Checking source code style characteristics...." />
-    <taskdef resource="checkstyletask.properties"
-             classpath="${checkstyle.dir}/checkstyle-all-4.1.jar" />
-
+    <taskdef resource="com/puppycrawl/tools/checkstyle/ant/checkstyle-ant-task.properties"
+             classpath="${checkstyle.dir}/checkstyle-8.18-all.jar"/>
     <checkstyle config="${checkstyle.dir}/slamd-checkstyle.xml"
-                failOnViolation="true">
-      <fileset dir="${source.dir}" includes="**/*.java" />
+                classpath="${classes.dir}" failOnViolation="true">
+      <fileset dir="${source.dir}" includes="**/*.java"
+               excludes="**/Version.java" />
+      <formatter type="plain" />
     </checkstyle>
   </target>
 

diff --git a/ext/checkstyle/checkstyle-8.18-all.jar b/ext/checkstyle/checkstyle-8.18-all.jar
diff --git a/ext/checkstyle/checkstyle-all-4.1.jar b/ext/checkstyle/checkstyle-all-4.1.jar
diff --git a/ext/checkstyle/slamd-checkstyle.xml b/ext/checkstyle/slamd-checkstyle.xml
@@ -3,61 +3,222 @@
     "-//Puppy Crawl//DTD Check Configuration 1.2//EN"
     "http://www.puppycrawl.com/dtds/configuration_1_2.dtd">
 
+<!--
+ !                             Sun Public License
+ !
+ ! The contents of this file are subject to the Sun Public License Version
+ ! 1.0 (the "License").  You may not use this file except in compliance with
+ ! the License.  A copy of the License is available at http://www.sun.com/
+ !
+ ! The Original Code is the SLAMD Distributed Load Generation Engine.
+ ! The Initial Developer of the Original Code is Neil A. Wilson.
+ ! Portions created by Neil A. Wilson are Copyright (C) 2004-2019.
+ ! Some preexisting portions Copyright (C) 2002-2006 Sun Microsystems, Inc.
+ ! All Rights Reserved.
+ !
+ ! Contributor(s):  Neil A. Wilson
+ ! -->
 
 <module name="Checker">
-  <module name="TreeWalker">
-    <!-- Ensure that each source file starts with the appropriate header -->
-    <module name="Header">
-      <property name="headerFile"
-                value="ext/checkstyle/sun-public-license.header" />
-    </module>
+  <property name="cacheFile" value=".checkstyle-cache-main" />
 
 
-    <!-- Ensure that all classes and interfaces are documented -->
-    <module name="JavadocType">
-      <property name="scope" value="private" />
-    </module>
+  <!-- Make sure that all source files have an appropriate header -->
+  <module name="Header">
+    <property name="headerFile"
+              value="ext/checkstyle/sun-public-license.header" />
+  </module>
+
 
+  <!-- Ensure that there is always a newline at the end of each file. -->
+  <module name="NewlineAtEndOfFile" />
 
-    <!-- Ensure that all methods are documented -->
+
+  <module name="TreeWalker">
+    <!-- Ensure that all methods have Javadoc documentation. -->
+    <!--
     <module name="JavadocMethod">
-      <property name="scope" value="private" />
-      <property name="allowUndeclaredRTE" value="false" />
-      <property name="allowMissingParamTags" value="false" />
-      <property name="allowMissingJavadoc" value="false" />
+      <property name="scope"                       value="private" />
+      <property name="allowUndeclaredRTE"          value="false"   />
+      <property name="allowMissingParamTags"       value="false"   />
+      <property name="allowMissingThrowsTags"      value="false"   />
+      <property name="allowMissingReturnTag"       value="false"   />
+      <property name="allowMissingJavadoc"         value="false"   />
+      <property name="allowMissingPropertyJavadoc" value="false"   />
+      <property name="suppressLoadErrors"          value="true"    />
     </module>
+     -->
 
 
-    <!-- Ensure that all public and protected fields are documented -->
+    <!-- Ensure that all non-private variables have Javadoc documentation. -->
+    <!--
     <module name="JavadocVariable">
-      <property name="scope" value="protected" />
+      <property name="scope" value="package" />
     </module>
+     -->
 
 
-    <!-- Ensure that all Javadoc comments are well-formed -->
+    <!-- Ensure that all Javadoc comments are well-formed. -->
+    <!--
     <module name="JavadocStyle">
-      <property name="scope" value="private" />
-      <property name="checkFirstSentence" value="true" />
-      <property name="checkEmptyJavadoc" value="true" />
-      <property name="checkHtml" value="true" />
+      <property name="scope"              value="private" />
+      <property name="checkFirstSentence" value="true"    />
+      <property name="checkEmptyJavadoc"  value="false"   />
+      <property name="checkHtml"          value="true"    />
     </module>
+     -->
 
 
-    <!-- Ensure that no line exceeds 80 characters in length -->
+    <!-- Ensure that there are no star imports. -->
+    <module name="AvoidStarImport">
+      <property name="allowStaticMemberImports" value="true" />
+    </module>
+
+    <!-- Ensure that there are no imports from a "sun.*" package. -->
+    <module name="IllegalImport">
+      <property name="illegalPkgs" value="sun" />
+    </module>
+
+
+    <!-- Ensure that there are no redundant imports. -->
+    <module name="RedundantImport" />
+
+
+    <!-- Ensure that there are no unused imports. -->
+    <module name="UnusedImports" />
+
+
+    <!-- Ensure that there are no lines longer than 80 characters. -->
     <module name="LineLength">
       <property name="max" value="80" />
     </module>
 
 
-    <!-- Ensure that no tab characters are used -->
-    <module name="TabCharacter" />
+    <!-- Ensure that modifiers are provided in the correct order. -->
+    <module name="ModifierOrder" />
+
 
+    <!-- Check to ensure there are no redundant modifiers. -->
+    <module name="RedundantModifier" />
 
-    <!-- Ensure that no line ends with whitespace -->
-    <module name="GenericIllegalRegexp">
-      <property name="format" value="\s$" />
-      <property name="message" value="Line ends with whitespace." />
+
+    <!-- Check to ensure that all code blocks include curly braces. -->
+    <module name="NeedBraces" />
+
+
+    <!-- Ensure that any class containing an equals method includes an
+         equals(Object) method. -->
+    <module name="CovariantEquals" />
+
+
+    <!-- Ensure that there are no empty statements in the code.  -->
+    <module name="EmptyStatement" />
+
+
+    <!-- Ensure that any class containing an equals method also includes a
+         hashCode method. -->
+    <module name="EqualsHashCode" />
+
+
+    <!-- Ensure that local variables do not hide class variables. -->
+    <!--
+    <module name="HiddenField">
+      <property name="tokens" value="VARIABLE_DEF" />
     </module>
+     -->
+
+
+    <!-- Ensure that utility classes do not have a public constructor. -->
+    <module name="HideUtilityClassConstructor" />
+
+
+    <!-- Ensure that checks for boolean values are simple where possible. -->
+    <module name="SimplifyBooleanExpression" />
+
+
+    <!-- Ensure that boolean returns are simple where possible. -->
+    <module name="SimplifyBooleanReturn" />
+
+
+    <!-- Ensure that string literal equality doesn't use "==". -->
+    <module name="StringLiteralEquality" />
+
+
+    <!-- Ensure that an overriding clone method invokes super.clone. -->
+    <!--
+    <module name="SuperClone" />
+     -->
+
+
+    <!-- Ensure that an overriding finalize method invokes super.finalize. -->
+    <module name="SuperFinalize" />
+
+
+    <!-- Ensure that all classes have a package declaration. -->
+    <module name="PackageDeclaration" />
+
+
+    <!-- Ensure that all package annotations are in package-info.java. -->
+    <module name="PackageAnnotation" />
+
+
+    <!-- Ensure that any switch statement that has a default clause always has
+         the default clause after all other clauses. -->
+    <module name="DefaultComesLast" />
+
+
+    <!-- Ensure that all cases of a switch statement that have any code also
+         have a break, return, or throw and don't fall through to the next
+         case. -->
+    <module name="FallThrough" />
+
+
+    <!-- Ensure that all local fields are declared private unless they are
+         static final. -->
+    <!--
+    <module name="VisibilityModifier" />
+     -->
+
+
+    <!-- Ensure that classes with only private constructors are final. -->
+    <module name="FinalClass" />
+
+
+    <!-- Ensure that all parameters are final. -->
+    <!--
+    <module name="FinalParameters">
+      <property name="ignorePrimitiveTypes" value="false" />
+      <property name="tokens"
+                value="METHOD_DEF,CTOR_DEF,LITERAL_CATCH,FOR_EACH_CLAUSE" />
+    </module>
+     -->
+
+
+    <!-- Ensure that all local variables that can be final are final. -->
+    <!--
+    <module name="FinalLocalVariable" />
+     -->
+
+
+    <!-- Ensure that all exceptions are immutable. -->
+    <module name="MutableException" />
+
+    <!-- Ensure that all long constants are followed by a capital L. -->
+    <module name="UpperEll" />
+
+
+    <!-- Ensure that all method/constructor/catch parameters are final. -->
+    <!--
+    <module name="FinalParameters" />
+     -->
+
+
+    <!-- Ensure that the @Override annotation is used where appropriate. -->
+    <module name="MissingOverride" />
+
+
+    <!-- Ensure that the @Deprecated annotation and @deprecated javadoc tag are
+         always used together. -->
+    <module name="MissingDeprecated" />
   </module>
 </module>
-
diff --git a/src/com/slamd/admin/AdminAccess.java b/src/com/slamd/admin/AdminAccess.java
@@ -29,8 +29,18 @@
  * This class provides a set of methods for providing access control for the
  * administrative user interface.
  */
-public class AdminAccess
+public final class AdminAccess
 {
+  /**
+   * Prevent this utility class from being instantiated.
+   */
+  private AdminAccess()
+  {
+    // No implementation required.
+  }
+
+
+
   /**
    * Retrieves the access manager associated with this admin servlet.
    *

diff --git a/src/com/slamd/admin/AdminConfig.java b/src/com/slamd/admin/AdminConfig.java
@@ -55,7 +55,7 @@
  * This class provides a set of methods for providing the interface to the
  * SLAMD server configuration.
  */
-public class AdminConfig
+public final class AdminConfig
        implements ConfigSubscriber
 {
   static AdminConfig ADMIN_CONFIG = new AdminConfig();

diff --git a/src/com/slamd/admin/AdminDebug.java b/src/com/slamd/admin/AdminDebug.java
@@ -17,7 +17,6 @@
 
 
 
-import java.lang.reflect.Method;
 import java.util.Enumeration;
 import java.util.Iterator;
 import java.util.Map;
@@ -39,8 +38,18 @@
  * This class provides a set of methods for providing debug access to the
  * administrative interface.
  */
-public class AdminDebug
+public final class AdminDebug
 {
+  /**
+   * Prevent this utility class from being instantiated.
+   */
+  private AdminDebug()
+  {
+    // No implementation required.
+  }
+
+
+
   /**
    * Handles the work of generating debug information about all thread groups
    * and threads defined in the JVM.

diff --git a/src/com/slamd/admin/AdminJob.java b/src/com/slamd/admin/AdminJob.java
@@ -81,6 +81,16 @@
  */
 public final class AdminJob
 {
+  /**
+   * Prevent this utility class from being instantiated.
+   */
+  private AdminJob()
+  {
+    // No implementation required.
+  }
+
+
+
   /**
    * Handle all processing related to viewing summary for information for a set
    * of jobs in a particular category (pending, running, or completed), or for