checkstyle compliance

CHANGELOG.md → docs/CHANGELOG.md

@@ -38,3 +38,10 @@ _1.28.2019_
 - `getOutput()` without parameters defaults to returning the standard out.
 - Some (very basic) unit tests in `CLITesterExecutionResultTest` to test these tweaks and that they work as expected.
 
+### 1.0.3
+_1.29.2019_
+- Checkstyle compliance in all files.
+- Docs based on GitHub's community standards.
+- Maven checkstyle plugin added to object model.
+- Updated naming of jars to **not** include patch number.
+

docs/CONTRIBUTING.md

@@ -40,7 +40,7 @@ _A guide for contributing to this repository ._
 ## Style
 
 ### Java Style
-No checkstyle config has been set up _yet_. Stay tuned. Not particularly picky.
+I use the `jgrade_checks.xml` configuration file in the `res` folder. Use this to be checkstyle compliant.
 
 ### Branching Names
 See the note in the [SOP branching section](#branching)

pom.xml

@@ -84,7 +84,7 @@
                             <goal>single</goal>
                         </goals>
                         <configuration>
-                            <finalName>jgrade-1.0.2-all</finalName>
+                            <finalName>jgrade-1.0-all</finalName>
                             <descriptorRefs>
                                 <descriptorRef>jar-with-dependencies</descriptorRef>
                             </descriptorRefs>
@@ -99,6 +99,20 @@
                 </executions>
             </plugin>
 
+            <!-- Checkstyle (not required for build) -->
+            <!-- Run with `mvn checkstyle:checkstyle` -->
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-checkstyle-plugin</artifactId>
+                <version>3.0.0</version>
+                <configuration>
+                    <configLocation>res/jgrade_checks.xml</configLocation>
+                    <encoding>UTF-8</encoding>
+                    <consoleOutput>true</consoleOutput>
+                    <failsOnError>false</failsOnError>
+                </configuration>
+            </plugin>
+
         </plugins>
     </build>
 </project>

res/jgrade_checks.xml

@@ -0,0 +1,173 @@
+<?xml version="1.0"?>
+<!DOCTYPE module PUBLIC
+          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
+          "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">
+
+
+<!--
+    JGrade Checkstyle Configuration.
+
+    Started from phf's 226_checks (which is a modification of sun_checks),
+    with a few tweaks. Left the Peter-speak in the comments.
+-->
+
+<module name="Checker">
+    <!-- hard to avoid on Windows, more of a pain to keep it -->
+    <!--<module name="NewlineAtEndOfFile"/>-->
+
+    <!-- maximum 2000 lines by default -->
+    <module name="FileLength"/>
+
+    <!-- tabs are not popular in Java -->
+    <module name="FileTabCharacter"/>
+
+    <module name="RegexpSingleline">
+       <property name="format" value="\s+$"/>
+       <property name="message" value="Line has trailing whitespace."/>
+    </module>
+
+    <module name="TreeWalker">
+        <module name="JavadocMethod">
+           <property name="scope" value="protected"/>
+        </module>
+        <module name="JavadocType">
+           <property name="scope" value="protected"/>
+        </module>
+        <module name="JavadocVariable">
+           <property name="scope" value="protected"/>
+        </module>
+        <module name="JavadocStyle">
+           <property name="scope" value="protected"/>
+           <property name="checkEmptyJavadoc" value="true"/>
+        </module>
+
+        <!-- being super-picky here, like Google -->
+        <module name="JavadocTagContinuationIndentation"/> <!-- not in 5.9 -->
+
+        <!-- various naming conventions -->
+        <module name="ConstantName"/>
+        <module name="LocalFinalVariableName"/>
+        <module name="LocalVariableName"/>
+        <module name="MemberName"/>
+        <module name="MethodName"/>
+        <module name="PackageName"/>
+        <module name="ParameterName"/>
+        <module name="StaticVariableName"/>
+        <module name="TypeName"/>
+        <module name="CatchParameterName"/> <!-- not in 5.9 -->
+        <module name="ClassTypeParameterName"/>
+        <module name="InterfaceTypeParameterName"/>
+        <module name="MethodTypeParameterName"/>
+
+        <!-- enforce sane imports -->
+        <module name="AvoidStarImport"/>
+        <module name="IllegalImport"/> <!-- default sun.* packages -->
+        <module name="RedundantImport"/>
+        <module name="UnusedImports"/>
+
+        <!-- size violations -->
+        <module name="AnonInnerLength"/> <!-- default 20 lines -->
+        <module name="LineLength">
+            <property name="max" value="120"/>
+        </module> <!-- default 80 chars -->
+        <module name="MethodLength"/> <!-- default 150 lines -->
+        <module name="ParameterNumber"/> <!-- default 7 parameters -->
+        <module name="OuterTypeNumber"/> <!-- default 1 per file -->
+
+        <!-- whitespace checks -->
+        <module name="EmptyForInitializerPad"/>
+        <module name="EmptyForIteratorPad"/>
+        <module name="EmptyLineSeparator">
+            <property name="allowNoEmptyLineBetweenFields" value="true" />
+        </module>
+        <module name="GenericWhitespace"/>
+        <module name="MethodParamPad"/>
+        <module name="NoLineWrap"/>
+        <module name="NoWhitespaceAfter"/>
+        <module name="NoWhitespaceBefore"/>
+        <module name="OperatorWrap"/>
+        <module name="ParenPad"/>
+        <module name="TypecastParenPad"/>
+        <module name="WhitespaceAfter"/>
+        <module name="WhitespaceAround">
+           <!-- empty methods look better this way -->
+           <property name="allowEmptyMethods" value="true" />
+           <property name="allowEmptyConstructors" value="true" />
+        </module>
+
+        <!-- sane use of modifiers (sane is a relative term) -->
+        <module name="ModifierOrder"/>
+        <module name="RedundantModifier"/>
+
+        <!-- block checks -->
+        <module name="AvoidNestedBlocks"/>
+        <module name="EmptyBlock"/>
+        <module name="EmptyCatchBlock"/> <!-- not in 5.9 -->
+        <module name="LeftCurly"/>
+        <module name="NeedBraces"/>
+        <module name="RightCurly"/>
+
+        <!-- coding style -->
+        <module name="ArrayTrailingComma"/>
+        <!--
+            I want them to use ?: every now and then; sadly
+            there's no option to just disallow complicated ones.
+        -->
+        <!--<module name="AvoidInlineConditionals"/>-->
+        <module name="CovariantEquals"/> <!-- avoid accidental overloading -->
+        <module name="DeclarationOrder"/> <!-- standardize classes -->
+        <module name="DefaultComesLast"/> <!-- standardize switch -->
+        <module name="EmptyStatement"/>
+        <module name="EqualsAvoidNull"/>
+        <module name="EqualsHashCode"/>
+        <module name="ExplicitInitialization"/> <!-- avoid initializing twice -->
+        <module name="FallThrough"/> <!-- avoid forgetting breaks -->
+        <!-- <module name="HiddenField"/>  don't see point in this with RequireThis... -->
+        <module name="IllegalCatch"/> <!-- avoid overly generic catch -->
+        <module name="IllegalThrows"/> <!-- avoid overly generic throw -->
+        <module name="InnerAssignment"/> <!-- avoid assignments as expressions -->
+        <!--<module name="MagicNumber"/>--> <!-- more trouble than it's worth -->
+        <module name="MissingSwitchDefault"/> <!-- standardize switch -->
+        <module name="ModifiedControlVariable"/> <!-- TODO: should for-each be different? -->
+        <module name="MultipleVariableDeclarations"/>
+        <module name="NestedTryDepth"/> <!-- no try inside a try -->
+        <module name="NoClone"/>
+        <module name="NoFinalizer"/>
+        <module name="OneStatementPerLine"/>
+        <module name="OverloadMethodsDeclarationOrder"/>
+        <module name="RequireThis"/> <!-- emphasize non-local stuff -->
+        <module name="SimplifyBooleanExpression"/>
+        <module name="SimplifyBooleanReturn"/>
+        <module name="StringLiteralEquality"/> <!-- reminder to use equals() -->
+
+        <!-- annotation checks -->
+        <module name="AnnotationLocation"/> <!-- standardize classes --> <!-- not in 5.9 -->
+
+        <!-- design checks -->
+        <!--<module name="DesignForExtension"/>--> <!-- too hard to explain -->
+        <module name="FinalClass"/>
+        <module name="HideUtilityClassConstructor"/>
+        <module name="InterfaceIsType"/>
+        <module name="MutableException"/>
+        <module name="OneTopLevelClass"/>
+        <module name="ThrowsCount">
+            <property name="ignorePrivateMethods" value="false"/> <!-- not in 5.9 -->
+        </module>
+        <!--<module name="VisibilityModifier"/>--> <!-- too restrictive for nested classes -->
+
+        <!-- code complexity checks -->
+	<!--<module name="ClassDataAbstractionCoupling"/>--> <!-- too restrictive for polymorphic test drivers that make instances of a lot of classes -->
+        <module name="ClassFanOutComplexity"/> <!-- TODO: on probation -->
+        <module name="CyclomaticComplexity"/> <!-- keep methods managable -->
+        <module name="NPathComplexity"/> <!-- TODO: on probation -->
+
+        <!-- miscellaneous checks -->
+        <module name="ArrayTypeStyle"/> <!-- it's not a C course -->
+        <module name="CommentsIndentation"/> <!-- not in 5.9 -->
+        <!--<module name="FinalParameters"/>--> <!-- ruins pass-by-value -->
+        <module name="Indentation"/> <!-- standardize indentation TODO: case labels? -->
+        <module name="OuterTypeFilename"/>
+        <module name="TodoComment"/>
+        <module name="UpperEll"/>
+    </module>
+</module>

src/main/java/com/github/tkutche1/jgrade/CLITester.java

@@ -103,7 +103,9 @@ public void initCommand() {
      * {@link #runCommand()}.
      * @return The current command (which is a List of Strings).
      */
-    public List<String> getCommand() { return this.command; }
+    public List<String> getCommand() {
+        return this.command;
+    }
 
     /**
      * Get the {@link ProcessBuilder} that was initialized from
@@ -113,15 +115,19 @@ public void initCommand() {
      * the builder.
      * @return The {@link ProcessBuilder} being used.
      */
-    public ProcessBuilder getBuilder() { return this.builder; }
+    public ProcessBuilder getBuilder() {
+        return this.builder;
+    }
 
     /**
      * Set whether or not to print the captured output. If true, then when
      * {@link #runCommand()} is called it will automatically print the output
      * to System.out.
      * @param to The boolean value to set whether or not to print output.
      */
-    public void setPrintOutput(boolean to) { this.printOutput = to; }
+    public void setPrintOutput(boolean to) {
+        this.printOutput = to;
+    }
 
     /**
      * Run a command with input. See {@link #runCommand()}.
@@ -131,8 +137,9 @@ public void initCommand() {
     protected CLIResult runCommand(String withInput) {
         this.builder.command(this.command);
         CLIResult output = executeProcess(this.builder, withInput);
-        if (printOutput)
+        if (printOutput) {
             ((ExecutionResult) output).dump();
+        }
         return output;
     }
 
@@ -194,7 +201,7 @@ public static CLIResult executeProcess(ProcessBuilder builder) {
 
     // Reads from an InputStream and returns the String.
     private static String getStringFromStream(InputStream stream) throws IOException {
-        byte streamBytes[] = new byte[stream.available()];
+        byte[] streamBytes = new byte[stream.available()];
         stream.read(streamBytes, 0, streamBytes.length);
         return new String(streamBytes);
     }

src/main/java/com/github/tkutche1/jgrade/CheckstyleGrader.java

@@ -98,7 +98,7 @@ public GradedTestResult runForGradedTestResult() {
                     new ProcessBuilder(command))
                     .getOutput(CLIResult.STREAM.STDOUT);
             return xmlToGradedTestResult(xmlOutput);
-        } catch (InternalError | IOException | RuntimeException e) {
+        } catch (InternalError | IOException e) {
             e.printStackTrace();
             e.printStackTrace(System.err);
             return internalErrorResult(e.toString());
@@ -139,13 +139,15 @@ private GradedTestResult xmlToGradedTestResult(String checkstyleOutput) throws I
         NodeList filesWithErrors = d.getElementsByTagName(FILE_TAG);
 
         int numErrors = 0;
-        for (int i = 0; i < filesWithErrors.getLength(); i++)
+        for (int i = 0; i < filesWithErrors.getLength(); i++) {
             numErrors += addOutputForFileNode(result, filesWithErrors.item(i));
+        }
 
         result.setScore(Math.max(this.points - (numErrors * this.deduct), 0));
 
-        if (numErrors == 0)
+        if (numErrors == 0) {
             result.addOutput("Passed all checks!");
+        }
 
         return result;
     }
@@ -174,8 +176,9 @@ private static String getAttributeValue(Node attribute) {
     }
 
     private static String getOutputForErrorNode(NamedNodeMap attributes) {
-        if (attributes == null)
+        if (attributes == null) {
             throw new InternalError();
+        }
         Node lineAttribute = attributes.getNamedItem(LINE_ATTR);
         Node columnAttribute = attributes.getNamedItem(COL_ATTR);
         Node messageAttribute = attributes.getNamedItem(MSG_ATTR);
@@ -188,11 +191,13 @@ private static int addOutputForFileNode(GradedTestResult result, Node elementNod
         String fileName = fullPath.substring(fullPath.lastIndexOf('/') + 1, fullPath.length() - 1);
         NodeList errorNodes = ((Element) elementNode).getElementsByTagName(ERROR_TAG);
 
-        if (errorNodes.getLength() > 0)
+        if (errorNodes.getLength() > 0) {
             result.addOutput(fileName + ":\n");
+        }
 
-        for (int i = 0; i < errorNodes.getLength(); i++)
+        for (int i = 0; i < errorNodes.getLength(); i++) {
             result.addOutput(getOutputForErrorNode(errorNodes.item(i).getAttributes()));
+        }
 
         return errorNodes.getLength();
     }