File refactors (#153)

* Leave checkstyle files in, add more rules and supressions to PMD

The PMD tests are deleted from the CI because a normal gradle build will run through them.

* Swap of overall API of the backend from File to Path. This was for a reason which I have now forgotten. Something to do with IO errors or something?

azure-pipelines.yml

@@ -65,7 +65,6 @@ stages:
           jdkArchitectureOption: x86
       - task: Gradle@2
         inputs:
-          options: '-x pmdMain -x pmdTest -PbuildServer'
           workingDirectory: ''
           gradleWrapperFile: 'gradlew'
           gradleOptions: '-Xmx1024m'

build.gradle.kts

@@ -134,20 +134,12 @@ dependencies {
     compile(group = "edu.wpi.first.wpiutil", name = "wpiutil-java", version = "2020.+")
     compile(group = "edu.wpi.first.wpilibj", name = "wpilibj-java", version = "2020.+")
 
-    fun junitJupiter(name: String, version: String = "5.2.0") =
+    fun junitJupiter(name: String, version: String = "5.5.0") =
         create(group = "org.junit.jupiter", name = name, version = version)
-    fun testFx(name: String, version: String = "4.0.13-alpha") =
-        create(group = "org.testfx", name = name, version = version)
 
     testCompile(junitJupiter(name = "junit-jupiter-api"))
-    testCompile(junitJupiter(name = "junit-jupiter-engine"))
     testCompile(junitJupiter(name = "junit-jupiter-params"))
-    testCompile(group = "com.google.guava", name = "guava-testlib", version = "23.0")
-    testCompile(testFx(name = "testfx-core"))
-    testCompile(testFx(name = "testfx-junit5"))
-
-    testRuntime(testFx(name = "openjfx-monocle", version = "jdk-9+181"))
-    testRuntime(group = "org.junit.platform", name = "junit-platform-launcher", version = "1.0.0")
+    testRuntime(junitJupiter(name = "junit-jupiter-engine"))
 }
 
 tasks.withType<JavaCompile>().configureEach {
@@ -162,7 +154,7 @@ jacoco {
 }
 
 extensions.getByType<PmdExtension>().apply {
-    toolVersion = "6.8.0"
+    toolVersion = "6.19.0"
     isConsoleOutput = true
     reportsDir = file("${project.buildDir}/reports/pmd")
     ruleSetFiles = files(file("$rootDir/pmd-ruleset.xml"))
@@ -177,13 +169,7 @@ tasks.withType<JacocoReport>().configureEach {
 }
 
 tasks.withType<Test>().configureEach {
-    // TODO: re-enable when TestFX (or the underlying JavaFX problem) is fixed
-    println("UI tests will not be run due to TestFX being broken when headless on Java 10.")
-    println("See: https://github.com/javafxports/openjdk-jfx/issues/66")
-    // Link: https://github.com/javafxports/openjdk-jfx/issues/66
-    useJUnitPlatform {
-        excludeTags("UI")
-    }
+    useJUnitPlatform()
 }
 
 val nativeShadowTasks = NativePlatforms.values().map { platform ->

pmd-ruleset.xml

@@ -3,57 +3,102 @@
          xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
          xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
          xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
+
     <description>
         The PMD RuleSet for PathWeaver.
     </description>
+
     <rule ref="category/java/bestpractices.xml">
         <exclude name="AccessorClassGeneration"/>
         <exclude name="AccessorMethodGeneration"/>
+        <exclude name="AvoidPrintStackTrace"/>
+        <exclude name="AvoidReassigningParameters"/>
+        <exclude name="JUnitAssertionsShouldIncludeMessage"/>
+        <exclude name="JUnitTestContainsTooManyAsserts"/>
+        <exclude name="JUnit4TestShouldUseAfterAnnotation"/>
+        <exclude name="JUnit4TestShouldUseBeforeAnnotation"/>
+        <exclude name="JUnit4TestShouldUseTestAnnotation"/>
+        <exclude name="SystemPrintln"/>
+        <exclude name="AvoidUsingHardCodedIP"/>
         <exclude name="UseVarargs"/>
         <exclude name="GuardLogStatement"/>
+        <exclude name="SwitchStmtsShouldHaveDefault"/>
     </rule>
-    <rule ref="category/java/bestpractices.xml/UnusedPrivateMethod">
+
+    <rule ref="category/java/codestyle.xml/FieldNamingConventions">
         <properties>
-            <property name="ignoredAnnotations" value="javafx.fxml.FXML"/>
+            <property name="publicConstantPattern" value="[A-Z][A-Z_0-9]*"/>
+            <property name="constantPattern" value="[A-Z][A-Z_0-9]*"/>
+            <property name="enumConstantPattern" value="[A-Z][A-Z_0-9]*"/>
+            <property name="finalFieldPattern" value=".*"/>
+            <property name="staticFieldPattern" value=".*"/>
+            <property name="defaultFieldPattern" value=".*"/>
+            <property name="exclusions" value="serialVersionUID|serialPersistentFields"/>
         </properties>
     </rule>
+
     <rule ref="category/java/codestyle.xml">
         <exclude name="AtLeastOneConstructor"/>
-        <exclude name="LocalVariableCouldBeFinal"/>
+        <exclude name="CallSuperInConstructor"/>
+        <exclude name="ClassNamingConventions"/>
+        <exclude name="FormalParameterNamingConventions"/>
+        <exclude name="GenericsNaming"/>
+        <exclude name="LocalVariableNamingConventions"/>
         <exclude name="MethodArgumentCouldBeFinal"/>
+        <exclude name="LocalVariableCouldBeFinal"/>
         <exclude name="OnlyOneReturn"/>
+        <exclude name="ShortClassName"/>
+        <exclude name="ShortMethodName"/>
         <exclude name="ShortVariable"/>
-        <exclude name="TooManyStaticImports"/>
-    </rule>
-    <rule ref="category/java/codestyle.xml/FieldNamingConventions">
-        <properties>
-            <property name="constantPattern" value="log|logger|[A-Z][A-Z_0-9]*"/>
-        </properties>
+        <exclude name="LongVariable"/>
+        <exclude name="UselessParentheses"/>
+        <exclude name="UseUnderscoresInNumericLiterals"/>
     </rule>
+
     <rule ref="category/java/design.xml/TooManyMethods">
         <properties>
-            <property name="maxmethods" value="11"></property>
-        </properties>
-    </rule>
-    <rule ref="category/java/design.xml/TooManyFields">
-        <properties>
-            <property name="maxfields" value="16"></property>
-        </properties>
-    </rule>
-    <rule ref="category/java/codestyle.xml/ShortClassName">
-        <properties>
-            <property name="minimum" value="4"></property>
+            <property name="maxmethods" value="20"/>
         </properties>
     </rule>
+
     <rule ref="category/java/design.xml">
+        <exclude name="AvoidDeeplyNestedIfStmts"/>
+        <exclude name="CouplingBetweenObjects"/>
+        <exclude name="CyclomaticComplexity"/>
         <exclude name="DataClass"/>
+        <exclude name="AvoidDeeplyNestedIfStmts"/>
         <exclude name="LawOfDemeter"/>
         <exclude name="LoosePackageCoupling"/>
+        <exclude name="NcssConstructorCount"/>
+        <exclude name="NcssCount"/>
+        <exclude name="NcssMethodCount"/>
+        <exclude name="NPathComplexity"/>
     </rule>
-    <rule ref="category/java/design.xml/NcssCount">
-        <properties>
-            <property name="methodReportLevel" value="20"/>
-            <property name="classReportLevel" value="250"/>
-        </properties>
+
+    <rule ref="category/java/performance.xml">
+        <exclude name="AvoidInstantiatingObjectsInLoops"/>
+        <exclude name="RedundantFieldInitializer"/>
+    </rule>
+
+    <rule ref="category/java/errorprone.xml">
+        <exclude name="AvoidDuplicateLiterals"/>
+        <exclude name="AvoidLiteralsInIfCondition"/>
+        <exclude name="BeanMembersShouldSerialize"/>
+        <exclude name="ConstructorCallsOverridableMethod"/>
+        <exclude name="DataflowAnomalyAnalysis"/>
+        <exclude name="DoNotCallSystemExit"/>
+        <exclude name="FinalizeDoesNotCallSuperFinalize"/>
+        <exclude name="JUnitSpelling"/>
+        <exclude name="MissingSerialVersionUID"/>
+        <exclude name="NullAssignment"/>
+        <exclude name="CompareObjectsWithEquals"/>
+        <exclude name="OverrideBothEqualsAndHashcode"/>
+        <exclude name="EmptyIfStmt"/>
+    </rule>
+
+    <rule ref="category/java/multithreading.xml">
+        <exclude name="AvoidSynchronizedAtMethodLevel"/>
+        <exclude name="AvoidUsingVolatile"/>
+        <exclude name="DoNotUseThreads"/>
     </rule>
 </ruleset>

src/main/java/edu/wpi/first/pathweaver/CreateProjectController.java

@@ -30,9 +30,8 @@
 import javax.measure.Unit;
 import javax.measure.quantity.Length;
 
-@SuppressWarnings("PMD.TooManyFields")
+@SuppressWarnings("PMD")
 public class CreateProjectController {
-
   @FXML
   private Label title;
   @FXML
@@ -91,7 +90,7 @@ public class CreateProjectController {
   private boolean editing = false;
 
   @FXML
-  @SuppressWarnings("PMD.NcssCount")
+
   private void initialize() {
     ObservableList<TextField> numericFields = FXCollections.observableArrayList(timeStep, maxVelocity,
         maxAcceleration, maxJerk, wheelBase);
@@ -102,7 +101,7 @@ private void initialize() {
     var outputControls = List.of(outputLabel, outputDirectory, browseOutput);
     var timeControls = List.of(timeLabel, timeStep, timeUnits);
     var velocityControls = List.of(velocityLabel, maxVelocity, velocityUnits);
-    var accelerationControls = List.of(accelerationLabel, maxAcceleration, accelerationUnits); // NOPMD
+    var accelerationControls = List.of(accelerationLabel, maxAcceleration, accelerationUnits);
     var jerkControls = List.of(jerkLabel, maxJerk, jerkUnits);
     var wheelBaseControls = List.of(wheelBaseLabel, wheelBase, wheelBaseUnits);
 
@@ -187,7 +186,7 @@ public Unit<Length> fromString(String string) {
   }
 
   @FXML
-  @SuppressWarnings("PMD.NcssCount")
+
   private void createProject() {
     String folderString = directory.getText().trim();
     // create a "PathWeaver" subdirectory if not editing an existing project

src/main/java/edu/wpi/first/pathweaver/DataFormats.java

@@ -5,7 +5,7 @@
 /**
  * Custom data formats for PathWeaver.
  */
-public final class DataFormats { //NOPMD
+public final class DataFormats {
 
   public static final String APP_PREFIX = "pathweaver";
 

src/main/java/edu/wpi/first/pathweaver/EditWaypointController.java

@@ -13,6 +13,7 @@
 import javafx.scene.input.KeyEvent;
 import javafx.util.converter.NumberStringConverter;
 
+@SuppressWarnings("PMD.UnusedPrivateMethod")
 public class EditWaypointController {
   @FXML
   private TextField xPosition;

src/main/java/edu/wpi/first/pathweaver/Field.java

@@ -120,7 +120,7 @@ private void updateCoord() {
   }
 
   private void updateScale() {
-    this.scale = ((pixelWidth / realWidth()) + (pixelLength / realLength())) / 2; //NOPMD Useless parentheses
+    this.scale = ((pixelWidth / realWidth()) + (pixelLength / realLength())) / 2;
   }
 
   public Unit<Length> getUnit() {

src/main/java/edu/wpi/first/pathweaver/FieldDisplayController.java

@@ -21,7 +21,7 @@
 import javafx.scene.layout.Pane;
 import javafx.scene.transform.Scale;
 
-@SuppressWarnings("PMD.TooManyMethods")
+@SuppressWarnings("PMD.UnusedPrivateMethod")
 public class FieldDisplayController {
     private static final PseudoClass SELECTED_CLASS = PseudoClass.getPseudoClass("selected");
 

src/main/java/edu/wpi/first/pathweaver/FxUtils.java

@@ -23,7 +23,7 @@
 import javafx.stage.Stage;
 
 public final class FxUtils {
-  private static final List<PseudoClass> SUBCHILD_SELECTORS; // NOPMD
+  private static final List<PseudoClass> SUBCHILD_SELECTORS;
 
   static {
     PseudoClass[] selectors = new PseudoClass[8];

src/main/java/edu/wpi/first/pathweaver/Game.java

@@ -31,10 +31,8 @@ private Game(String name, Field field) {
    * @param field the game field
    *
    * @return the created game
-   *
-   * @throws DuplicateGameException if a game has already been created with the given name
    */
-  public static Game create(String name, Field field) throws DuplicateGameException {
+  public static Game create(String name, Field field) {
     if (GAMES.stream().map(Game::getName).anyMatch(name::equals)) {
       throw new DuplicateGameException("A game already exists with the name \"" + name + "\"");
     }

src/main/java/edu/wpi/first/pathweaver/Loggers.java

@@ -17,7 +17,7 @@
 /**
  * Helper class for setting up the application loggers.
  */
-@SuppressWarnings("PMD.ClassNamingConventions")
+
 final class Loggers {
 
   private Loggers() {
@@ -27,7 +27,7 @@ private Loggers() {
   /**
    * Sets up loggers to print to stdout (rather than stderr) and log to ~/PathWeaver/pathweaver.time.log
    */
-  @SuppressWarnings("PMD.NcssCount")
+
   public static void setupLoggers() throws IOException {
     //Set up the global level logger. This handles IO for all loggers.
     final Logger globalLogger = LogManager.getLogManager().getLogger("");

src/main/java/edu/wpi/first/pathweaver/Main.java

@@ -7,7 +7,8 @@
  * reflectively access the JavaFX application launcher classes - this will fail because there is no module path;
  * everything is in the same, unnamed module.
  */
-@SuppressWarnings("PMD.UseUtilityClass") // Nope.
+ // Nope.
+@SuppressWarnings("PMD.UseUtilityClass")
 public final class Main {
   public static void main(String[] args) {
     // JavaFX 11+ uses GTK3 by default, and has problems on some display servers