Automating Parts of the Peer Review – Part 2! Adding Maven PMD Plugin

TIP: References Quick List

Introduction

Similar to the spotbugs-maven-plugin, from the prior post, we can get early feedback on possible bugs and other issues in our code, even before we even get to the peer review process, using the maven-pmd-plugin. This can help us save time during the peer review, as well as avoid production defects, if issues would otherwise slip through the review process. There will be overlap with the types of issues this plugin and the spotbugs-maven-plugin would find, but they may diverge and help us spot unique issues that the other may not find.

Considerations

What we want to watch for when layering static analysis plugins like spotbugs-maven-plugin and maven-pmd-plugin are things like:

How much time each contributes to the build execution time, and
Whether or not the checks in both plugins are in agreement – or if they will be arguing with each other on the expected format/approach

However, different plugins may check for different issues. For example, the maven-pmd-plugin can look for duplicated code, which is not one of the features offered by the spotbugs-maven-plugin currently.

As with any dependency or plugin, be careful which ones you use. Is the code source reputable? Is it well maintained? Are there issues with the code? The maven-pmd-plugin shows that the plugin was updated earlier this year (2023) and is owned by Apache, the makers of Maven.

Plugin Management – Controlling the Version

In order to control the version of the plugin used in this build (and any Maven sub-modules), we can add an entry to the pom.xml file’s project/build/pluginManagement/plugins section.

<plugin>
    <!-- static code analysis quality gates and reports -->
    <!-- https://maven.apache.org/plugins/maven-pmd-plugin/usage.html -->
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-pmd-plugin</artifactId>
    <version>3.21.0</version>
</plugin>

Reporting Plugin – Updating the Site

Next, let’s add the plugin to the project/reporting/plugins section of the pom.xml in order to include a report in the mvn site‘s logic, then the following will be generated:

target/site/pmd.html
target/site/cpd.html

<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-pmd-plugin</artifactId>
    <configuration>
        <inputEncoding>${project.build.sourceEncoding}</inputEncoding>
        <targetJdk>${maven.compiler.target}</targetJdk>
    </configuration>
</plugin>

The variables used in the snippet above are existing properties already defined in our java101/pom.xml.

Rule Set Files

Let’s create 2 files – build_config/pmd-rules-main.xml and build_config/pmd-rules-test.xml, so that we can vary the configuration used for src/main and src/test files, respectively. The only difference initially will be the comment on the 6th line unless your team wants different behavior from the start. There are additional checks available, too.

<?xml version="1.0" encoding="UTF-8"?>
<ruleset name="Default Maven PMD Plugin Ruleset"
         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">
    <!-- The rules that should be applied to the src/main|test files.  -->
    <!-- See https://maven.apache.org/plugins/maven-pmd-plugin/examples/usingRuleSets.html -->
    <!-- https://docs.pmd-code.org/pmd-doc-6.55.0/pmd_rules_java_bestpractices.html -->
    <rule ref="category/java/bestpractices.xml/AvoidUsingHardCodedIP" />
    <rule ref="category/java/bestpractices.xml/CheckResultSet" />
    <rule ref="category/java/bestpractices.xml/PrimitiveWrapperInstantiation" />
    <rule ref="category/java/bestpractices.xml/UnusedFormalParameter" />
    <rule ref="category/java/bestpractices.xml/UnusedLocalVariable" />
    <rule ref="category/java/bestpractices.xml/UnusedPrivateField" />
    <rule ref="category/java/bestpractices.xml/UnusedPrivateMethod" />
    <!-- https://docs.pmd-code.org/pmd-doc-6.55.0/pmd_rules_java_codestyle.html -->
    <rule ref="category/java/codestyle.xml/EmptyControlStatement" />
    <rule ref="category/java/codestyle.xml/ExtendsObject" />
    <rule ref="category/java/codestyle.xml/ForLoopShouldBeWhileLoop" />
    <rule ref="category/java/codestyle.xml/TooManyStaticImports" />
    <rule ref="category/java/codestyle.xml/UnnecessaryFullyQualifiedName" />
    <rule ref="category/java/codestyle.xml/UnnecessaryImport" />
    <rule ref="category/java/codestyle.xml/UnnecessaryModifier" />
    <rule ref="category/java/codestyle.xml/UnnecessaryReturn" />
    <rule ref="category/java/codestyle.xml/UnnecessarySemicolon" />
    <rule ref="category/java/codestyle.xml/UselessParentheses" />
    <rule ref="category/java/codestyle.xml/UselessQualifiedThis" />
    <!-- https://docs.pmd-code.org/pmd-doc-6.55.0/pmd_rules_java_design.html -->
    <rule ref="category/java/design.xml/CollapsibleIfStatements" />
    <rule ref="category/java/design.xml/SimplifiedTernary" />
    <rule ref="category/java/design.xml/UselessOverridingMethod" />
    <!-- https://docs.pmd-code.org/pmd-doc-6.55.0/pmd_rules_java_documentation.html -->
    <!-- https://docs.pmd-code.org/pmd-doc-6.55.0/pmd_rules_java_errorprone.html -->
    <rule ref="category/java/errorprone.xml/AvoidBranchingStatementAsLastInLoop" />
    <rule ref="category/java/errorprone.xml/AvoidDecimalLiteralsInBigDecimalConstructor" />
    <rule ref="category/java/errorprone.xml/AvoidMultipleUnaryOperators" />
    <rule ref="category/java/errorprone.xml/AvoidUsingOctalValues" />
    <rule ref="category/java/errorprone.xml/BrokenNullCheck" />
    <rule ref="category/java/errorprone.xml/CheckSkipResult" />
    <rule ref="category/java/errorprone.xml/ClassCastExceptionWithToArray" />
    <rule ref="category/java/errorprone.xml/DontUseFloatTypeForLoopIndices" />
    <rule ref="category/java/errorprone.xml/EmptyCatchBlock" />
    <rule ref="category/java/errorprone.xml/JumbledIncrementer" />
    <rule ref="category/java/errorprone.xml/MisplacedNullCheck" />
    <rule ref="category/java/errorprone.xml/OverrideBothEqualsAndHashcode" />
    <rule ref="category/java/errorprone.xml/ReturnFromFinallyBlock" />
    <rule ref="category/java/errorprone.xml/UnconditionalIfStatement" />
    <rule ref="category/java/errorprone.xml/UnnecessaryConversionTemporary" />
    <rule ref="category/java/errorprone.xml/UnusedNullCheckInEquals" />
    <rule ref="category/java/errorprone.xml/UselessOperationOnImmutable" />
    <!-- https://docs.pmd-code.org/pmd-doc-6.55.0/pmd_rules_java_multithreading.html -->
    <rule ref="category/java/multithreading.xml/AvoidThreadGroup" />
    <rule ref="category/java/multithreading.xml/DontCallThreadRun" />
    <rule ref="category/java/multithreading.xml/DoubleCheckedLocking" />
    <!-- https://docs.pmd-code.org/pmd-doc-6.55.0/pmd_rules_java_performance.html -->
    <rule ref="category/java/performance.xml/BigIntegerInstantiation" />
    <!-- https://docs.pmd-code.org/pmd-doc-6.55.0/pmd_rules_java_security.html -->
</ruleset>

Exclusions Files

In case we need to add exclusions to the build, such as if we are adding this to an existing project, or if we find a situation where the maven-pmd-plugin’s checks conflict with spotbugs-maven-plugin, or the like, let’s add the files that will hold the exclusions.

Let’s start by adding a build_config/pmd-exclude-pmd-check.properties that documents the format we should use, if an exclusion is needed for the PMD quality gate.

# Exclusions for the Apache Maven PMD Plugin checks take the following format:
#
# Exclude 2 checks for a given class:
# com.foobar.mypackage.MyClass=UnusedPrivateField,UnusedPrivateMethod

Then, we can add a build_config/pmd-exclude-cpd-check.properties that documents the format we should use, if an exclusion is needed for the copy-paste detector (CPD) quality gate.

# Exclusions for the Copy-Paste Detector (CPD) check take the following format:
#
# Exclude duplicated code across classes
# com.foobar.mypackage.MyClassA,com.foobar.mypackage.MyClassB
#
# Exclude duplicated code within a single class
# com.foobar.mypackage.MyClassC

Quality Gate

With the rules and exclusions files created, we are ready to update the pom.xml file to add the quality gate to the pom.xml. By adding this higher in the pom.xml file than the existing spotbugs-maven-plugin, then we can get it to run first and then, afterward, the spotbugs-maven-plugin will run. That way, we can test that our new plugin, indeed fails the build if issues are found. The plugin version is defined already in the pluginManagement section.

<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-pmd-plugin</artifactId>
    <configuration>
        <inputEncoding>${project.build.sourceEncoding}</inputEncoding>
        <targetJdk>${maven.compiler.target}</targetJdk>
    </configuration>
    <executions>
        <execution>
            <!-- Checking for coding issues in main files. -->
            <id>pmd-main</id>
            <goals>
                <goal>check</goal>
            </goals>
            <phase>verify</phase>
            <configuration>
                <verbose>true</verbose>
                <includeTests>false</includeTests>
                <rulesets>
                    <ruleset>build_config/pmd-rules-main.xml</ruleset>
                </rulesets>
                <excludeFromFailureFile>build_config/pmd-exclude-pmd-check.properties</excludeFromFailureFile>
            </configuration>
        </execution>
        <execution>
            <!-- Checking for coding issues in test files. -->
            <id>cpd-main</id>
            <goals>
                <goal>cpd-check</goal>
            </goals>
            <phase>verify</phase>
            <configuration>
                <verbose>true</verbose>
                <includeTests>false</includeTests>
                <rulesets>
                    <ruleset>build_config/pmd-rules-main.xml</ruleset>
                </rulesets>
                <excludeFromFailureFile>build_config/pmd-exclude-cpd-check.properties</excludeFromFailureFile>
            </configuration>
        </execution>
        <execution>
            <!-- Checking for code duplication (copy-paste detector) in main files. -->
            <id>pmd-test</id>
            <goals>
                <goal>check</goal>
            </goals>
            <phase>verify</phase>
            <configuration>
                <verbose>true</verbose>
                <includeTests>true</includeTests>
                <excludeRoots>
                    <excludeRoot>${basedir}/src/main/java</excludeRoot>
                </excludeRoots>
                <rulesets>
                    <ruleset>build_config/pmd-rules-test.xml</ruleset>
                </rulesets>
                <excludeFromFailureFile>build_config/pmd-exclude-pmd-check.properties</excludeFromFailureFile>
            </configuration>
        </execution>
        <execution>
            <!-- Checking for code duplication (copy-paste detector) in test files. -->
            <id>cpd-test</id>
            <goals>
                <goal>cpd-check</goal>
            </goals>
            <phase>verify</phase>
            <configuration>
                <verbose>true</verbose>
                <includeTests>true</includeTests>
                <excludeRoots>
                    <excludeRoot>${basedir}/src/main/java</excludeRoot>
                </excludeRoots>
                <rulesets>
                    <ruleset>build_config/pmd-rules-test.xml</ruleset>
                </rulesets>
                <excludeFromFailureFile>build_config/pmd-exclude-cpd-check.properties</excludeFromFailureFile>
            </configuration>
        </execution>
    </executions>
</plugin>

Checking the Quality Gate

Now that the plugin is configured, let’s make sure it works as a quality gate – failing the build if violation(s) are found.

Adding a Violation

Let’s add the same code to the codingchica.java101.App that we used in the prior post for the spotbugs-maven-plugin, so we can compare the errors returned.

// TODO Delete this method - do not commit
@Override
public boolean equals(Object o) {
  App app = (App) o;
  return name == app.name;
}

Running the Build for Failure

If we now run the mvn clean site install command to generate the report and then fail the build if an issue is found, we should see output like:

[INFO] --- pmd:3.21.0:check (pmd-main) @ java101 ---
[INFO] PMD version: 6.55.0
[INFO] PMD Failure: codingchica.java101.App:83 Rule:OverrideBothEqualsAndHashcode Priority:3 Ensure you override both equals() and hashCode().
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
...
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-pmd-plugin:3.21.0:check (pmd-main) on project java101: You have 1 PMD violation. For more details see: C:\Users\Heather\java101\target\pmd.xml -> [Help 1]

The issue details are included because we included <verbose>true</verbose> in our pom.xml configuration for this plugin. If you do not see error details, then double check that this is part of the plugin’s configuration.

Viewing Errors In Site

Now, if we want to see more details, such as getting a link to the rule description for each failure, in IntelliJ, we can right click on java101/target/site/pmd.html -> Open In -> Browser -> Your favorite browser here.

The page displayed will contain a listing of issues by priority and by file. Each broken rule will have a link to the rule’s documentation, as well as a link to the source code where the issue was found.

For example, in this build, there is a link to the rule: OverrideBothEqualsAndHashCode.

Comparing to SpotBugs Results

As discussed earlier, although plugins may overlap, such as both finding the need to override both equals and hashCode if either is overridden in a class, they may also find separate issues. With the same problem code change, spotbugs-maven-plugin found 4 issues.

[ERROR] Medium: Equals method for codingchica.java101.App assumes the argument is of type App [codingchica.java101.App] At App.java:[line 84] BC_EQUALS_METHOD_SHOULD_WORK_FOR_ALL_OBJECTS
[ERROR] Medium: Comparison of String objects using == or != in codingchica.java101.App.equals(Object) [codingchica.java101.App] At App.java:[line 85] ES_COMPARING_STRINGS_WITH_EQ
[ERROR] High: codingchica.java101.App defines equals and uses Object.hashCode() [codingchica.java101.App] At App.java:[lines 84-85] HE_EQUALS_USE_HASHCODE
[ERROR] Medium: codingchica.java101.App.equals(Object) does not check for null argument [codingchica.java101.App] At App.java:[lines 84-85] NP_EQUALS_SHOULD_HANDLE_NULL_ARGUMENT

Not only did it also report that both equal and hashCode should be overridden together, but it also found issues inside of the equals check.

However, there are other rules in the maven-pmd-plugin, such as AvoidDeeplyNestedIfStmts that may not be covered in spotbugs-maven-plugin at present. Thus, as long as doing so doesn’t slow down the build too much or cause conflicting reports that argue with one another, having both plugins in a given Java project may still benefit the developer(s) with early feedback.

Removing Violations

In this case, we only need to delete the code we added to codingchica.java101.App in order to force a build failure.

Adding PMD Exclusions for Existing Issues

If adding this quality gate to an existing Java project, there may be more issues in the existing code than we are prepared to address in one commit or Pull Request (PR). However, were we to wait to add the quality gate until all issues are addressed, then we may see new issues introduced in the interim. Therefore, it may warrant adding temporary exclusions to avoid breaking the build with the new quality gate.

Example error from Maven build output:

[INFO] PMD Failure: codingchica.java101.App:83 Rule:OverrideBothEqualsAndHashcode Priority:3 Ensure you override both equals() and hashCode().

Pattern	Is Regular Expression?	Replacement	Comment
[INFO] PMD Failure:	No		There is a space at the end, after the colon (:). Remove the prefix before the class name.
:\d+ Rule:	Yes	=	Remove line numbers and Rule prefix, replacing with an equals (=) sign.
Priority:.*	Yes		Remove suffix after the rule name.

The approach above works for a single rule violation within a class. If one class has multiple rules violated, then you may still need to manually combine into a single line to match the expected format in the properties file. The way this will likely manifest is by the build continuing to fail in the same spot with a class you have already added an exclusion for – because one line is replacing the other in the properties file.

Example of transformed exclusion that is ready to add to build_config/pmd-exclude-pmd-check.properties:

codingchica.java101.App=OverrideBothEqualsAndHashcode

From here, you may find benefit in pasting the transformed exclusions into Excel or some other spreadsheet processor and sorting them, so it is easier to spot duplicate class names.

Adding CPD Exclusions for Existing Issues

With the copy-paste detector (CPD), it is going to report the location where the duplicated code starts in two spots in the code. The following approach is intended to transform an error across 2 separate files into a single exclusion. If within the same file, or across more than just 2 files, then the approach will need to be varied somewhat.

Example starting error message:

[INFO] CPD Failure: Found 48 lines of duplicated code at locations:
[INFO]     C:\Users\myUser\java101\src\main\java\codingchica\java101\App.java line 8
[INFO]     C:\Users\myUser\java101\src\main\java\codingchica\java101\AppCopy.java line 8

Example commands to transform into exclusions:

Pattern	Is Regular Expression?	Replacement	Comment
\\\|\/	Yes	.	Replacing forward or back slashes in the file names with periods (.).
[INFO]	No		Removing logging level that may interfere in subsequent regular expression searches.
CPD Failure: Found \d+ lines of duplicated code at locations: \s..src.main.java.	Yes		Removing the first line of the violation report and the folder structure ahead of the first file path.
.java line \d \s..src.main.java.	Yes	,	Replacing the content between the two class names with a comma (,).
.java line \d+	Yes		Removing the suffix after the last class name.

Example of transformed exclusion that is ready to add to build_config/pmd-exclude-cpd-check.properties:

codingchica.java101.App,codingchica.java101.AppCopy

Summary

The maven-pmd-plugin is another plugin we can use to try to catch coding issues in a quality gate as part of the build. It can help us find duplicated code (via the copy-paste detector) as well as issues within the code itself. This can help newer developers learn, and catch accidental mistakes of more seasoned developers. Doing so may help speed up peer reviews / pull requests (PRs) and deliver higher quality code whenever released.

Coding Chica