FEAT: Raise Unused code during build stage

.vscode/settings.json

@@ -0,0 +1,3 @@
+{
+    "java.configuration.updateBuildConfiguration": "interactive"
+}

ci/pmd-ruleset.xml

@@ -0,0 +1,34 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ruleset name="51Degrees Unused Code"
+         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 https://pmd.sourceforge.io/ruleset_2_0_0.xsd">
+
+    <description>
+        Reports unused code as part of the build. The Java compiler's -Xlint
+        cannot detect unused local variables, private fields, private methods or
+        private/constructor parameters.
+
+        Note: there is no reliable rule for "unused public class/method" as those may
+        form part of the published API and cannot be judged from this codebase alone,
+        so they are intentionally out of scope.
+    </description>
+
+    <!-- Unused local variables, e.g. `Path tempPath = ...;` that is never read. -->
+    <rule ref="category/java/bestpractices.xml/UnusedLocalVariable"/>
+
+    <!-- Private fields that are never read. -->
+    <rule ref="category/java/bestpractices.xml/UnusedPrivateField"/>
+
+    <!-- Private methods that are never called. -->
+    <rule ref="category/java/bestpractices.xml/UnusedPrivateMethod"/>
+
+    <!-- Parameters of private methods/constructors that are never used.
+         (Restricted to private members by default so overrides and interface
+         implementations are not flagged.) -->
+    <rule ref="category/java/bestpractices.xml/UnusedFormalParameter"/>
+
+    <!-- Imports that are not referenced (also covers duplicate/unnecessary imports). -->
+    <rule ref="category/java/codestyle.xml/UnnecessaryImport"/>
+
+</ruleset>

ip-intelligence.cloud/src/main/java/fiftyone/ipintelligence/cloud/data/MultiIPIDataCloud.java

@@ -41,7 +41,7 @@
  * by the 51Degrees Cloud service when evidence matching multiple profiles is
  * provided.
  */
-public class MultiIPIDataCloud
+public final class MultiIPIDataCloud
     extends AspectDataBase
     implements MultiProfileData<IPIntelligenceData> {
     private static final String PROFILE_LIST_KEY = "profiles";

ip-intelligence.cloud/src/test/java/fiftyone/ipintelligence/cloud/flowelements/IPIntelligenceCloudEngineTests.java

@@ -38,7 +38,6 @@
 
 import java.net.InetAddress;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;

ip-intelligence.cloud/src/test/java/fiftyone/ipintelligence/cloud/flowelements/MissingPropertyHandlingTests.java

@@ -26,7 +26,6 @@
 import fiftyone.pipeline.cloudrequestengine.data.CloudRequestData;
 import fiftyone.pipeline.cloudrequestengine.flowelements.CloudRequestEngine;
 import fiftyone.pipeline.core.data.FlowData;
-import fiftyone.pipeline.core.data.IWeightedValue;
 import fiftyone.pipeline.engines.data.AspectPropertyMetaData;
 import fiftyone.pipeline.engines.data.AspectPropertyMetaDataDefault;
 import fiftyone.pipeline.engines.data.AspectPropertyValue;

ip-intelligence.engine.on-premise/src/test/java/fiftyone/ipintelligence/engine/onpremise/data/DataValidatorHash.java

@@ -26,13 +26,10 @@
 import fiftyone.ipintelligence.shared.IPIntelligenceData;
 import fiftyone.ipintelligence.shared.testhelpers.data.DataValidator;
 import fiftyone.pipeline.core.data.FlowData;
-import fiftyone.pipeline.core.data.IWeightedValue;
 import fiftyone.pipeline.engines.data.AspectPropertyValue;
 import fiftyone.pipeline.engines.exceptions.NoValueException;
 import fiftyone.pipeline.engines.fiftyone.data.FiftyOneAspectPropertyMetaData;
 
-import java.util.Arrays;
-import java.util.List;
 import java.util.Map;
 
 import static org.junit.Assert.*;

ip-intelligence.shared/src/main/java/fiftyone/ipintelligence/shared/flowelements/OnPremiseIPIntelligenceEngineBuilderBase.java

@@ -24,7 +24,6 @@
 
 import fiftyone.pipeline.engines.data.AspectData;
 import fiftyone.pipeline.engines.data.AspectPropertyMetaData;
-import fiftyone.pipeline.engines.data.AspectPropertyValue;
 import fiftyone.pipeline.engines.fiftyone.flowelements.FiftyOneAspectEngine;
 import fiftyone.pipeline.engines.fiftyone.flowelements.FiftyOneOnPremiseAspectEngineBuilderBase;
 import fiftyone.pipeline.engines.services.DataUpdateService;

ip-intelligence.shared/src/test/java/fiftyone/ipintelligence/shared/testhelpers/FileUtilsTest.java

@@ -22,7 +22,6 @@
 
 package fiftyone.ipintelligence.shared.testhelpers;
 
-import org.junit.Assert;
 import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;

ip-intelligence.shared/src/test/java/fiftyone/ipintelligence/shared/testhelpers/ListRuntimeConfigTest.java

@@ -22,7 +22,6 @@
 
 package fiftyone.ipintelligence.shared.testhelpers;
 
-import org.junit.Ignore;
 import org.junit.Test;
 
 public class ListRuntimeConfigTest {

ip-intelligence.shared/src/test/java/fiftyone/ipintelligence/shared/testhelpers/data/MetaDataTests.java

@@ -30,7 +30,6 @@
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Future;
-import java.util.concurrent.atomic.AtomicBoolean;
 
 import static org.junit.Assert.assertEquals;
 

ip-intelligence.shared/src/test/java/fiftyone/ipintelligence/shared/testhelpers/data/ValueTests.java

@@ -33,7 +33,10 @@
 import fiftyone.pipeline.engines.fiftyone.data.FiftyOneAspectPropertyMetaData;
 
 import java.lang.reflect.Method;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
 
 import static fiftyone.pipeline.util.StringManipulation.stringJoin;
 import static org.junit.Assert.*;

ip-intelligence.shared/src/test/java/fiftyone/ipintelligence/shared/testhelpers/flowelements/ProcessTests.java

@@ -27,10 +27,7 @@
 import fiftyone.ipintelligence.shared.testhelpers.data.DataValidator;
 import fiftyone.pipeline.core.data.FlowData;
 
-import java.util.Arrays;
-import java.util.List;
 
-import static fiftyone.pipeline.util.StringManipulation.stringJoin;
 
 public class ProcessTests {
 

ip-intelligence/src/main/java/fiftyone/ipintelligence/IPIntelligenceOnPremisePipelineBuilder.java

@@ -34,7 +34,6 @@
 import fiftyone.pipeline.engines.flowelements.AspectEngine;
 import fiftyone.pipeline.engines.flowelements.PrePackagedPipelineBuilderBase;
 import fiftyone.pipeline.engines.services.DataUpdateService;
-import fiftyone.pipeline.engines.services.HttpClient;
 import org.slf4j.ILoggerFactory;
 import org.slf4j.LoggerFactory;
 

ip-intelligence/src/test/java/fiftyone/ipintelligence/IPIntelligenceOnPremisePipelineBuilderTest.java

@@ -42,7 +42,6 @@
 
 import java.io.File;
 import java.lang.reflect.Field;
-import java.nio.file.Files;
 import java.util.List;
 
 import static org.junit.Assert.*;
@@ -160,7 +159,7 @@ public void setDataFileSystemWatcher_PropertyPropagatedToDataFileConfiguration()
     @Test
     public void setDataUpdateService_RegisterDataFileTriggered() throws Exception {
         DataUpdateServiceDefault dataUpdateServiceDefault = mock(DataUpdateServiceDefault.class);
-        Pipeline pipeline = builder
+        builder
                 .setAutoUpdate(true)
                 .setDataUpdateService(dataUpdateServiceDefault)
                 .build();

ip-intelligence/src/test/java/fiftyone/ipintelligence/IPIntelligenceTests.java

@@ -43,6 +43,7 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.ArrayList;
@@ -56,7 +57,7 @@
 import static fiftyone.ipintelligence.shared.testhelpers.FileUtils.IP_ADDRESSES_FILE_NAME;
 import static fiftyone.pipeline.engines.Constants.PerformanceProfiles.*;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assume.assumeTrue;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
@@ -230,6 +231,8 @@ public void TestOnPremiseBuilder_CreateTempFile() throws Exception {
                         pipeline.getElement(IPIntelligenceOnPremiseEngine.class);
                 String tempDir = engine.getTempDataDirPath();
                 Path tempPath = Paths.get(tempDir);
+                assertTrue("A temp data file should have been created under " + tempPath,
+                        Files.exists(tempPath));
             }
         }
         logger.info("Completed {} creations", i);

pom.xml

@@ -40,6 +40,7 @@
         <org.json.version>20211205</org.json.version>
 
         <maven-compiler-plugin.version>3.9.0</maven-compiler-plugin.version>
+        <maven-pmd-plugin.version>3.26.0</maven-pmd-plugin.version>
         <exec-maven-plugin.version>1.6.0</exec-maven-plugin.version>
         <maven-install-plugin.version>2.5.2</maven-install-plugin.version>
         <maven-jar-plugin.version>3.2.2</maven-jar-plugin.version>
@@ -119,7 +120,7 @@
                         <showWarnings>true</showWarnings>
                         <compilerArgs>
                             <arg>-Xlint:all,-try,-options</arg>
-<!--                            <arg>-Werror</arg>-->
+                            <arg>-Werror</arg>
                         </compilerArgs>
                     </configuration>
                 </plugin>
@@ -158,6 +159,44 @@
                     <waitUntil>published</waitUntil>
                 </configuration>
             </plugin>
+            <plugin>
+                <!--
+                    Reports unused code (local variables, private fields, private
+                    methods, private parameters and unnecessary imports) as part of
+                    the build. javac's -Xlint cannot detect these.
+                     Bound to the compile phase so violations surface during
+                    a normal build and fail it.
+                -->
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-pmd-plugin</artifactId>
+                <version>${maven-pmd-plugin.version}</version>
+                <configuration>
+                    <rulesets>
+                        <!--suppress UnresolvedMavenProperty -->
+                        <ruleset>${maven.multiModuleProjectDirectory}/ci/pmd-ruleset.xml</ruleset>
+                    </rulesets>
+                    <!-- Analyse test sources -->
+                    <includeTests>true</includeTests>
+                    <failOnViolation>true</failOnViolation>
+                    <printFailingErrors>true</printFailingErrors>
+                    <!-- Avoid requiring the site/jxr report just to run the check. -->
+                    <linkXRef>false</linkXRef>
+                    <!-- SWIG-generated interop bindings are machine-generated and
+                         regenerated from the native build, so they must not be gated. -->
+                    <excludes>
+                        <exclude>**/interop/swig/*.java</exclude>
+                    </excludes>
+                </configuration>
+                <executions>
+                    <execution>
+                        <id>check-unused-code</id>
+                        <phase>compile</phase>
+                        <goals>
+                            <goal>check</goal>
+                        </goals>
+                    </execution>
+                </executions>
+            </plugin>
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-surefire-plugin</artifactId>