Missing custom Checkstyle and PMD configuration files

[sfloess]

Description

The project uses Google's Checkstyle rules (google_checks.xml) but lacks custom Checkstyle and PMD configuration files to enforce project-specific code quality standards. This results in inconsistencies that could be caught automatically.

Current Static Analysis Setup

✅ What Exists

1. SpotBugs - Custom exclusions: spotbugs-exclude.xml ✅
2. OWASP Dependency Check - Custom suppressions: dependency-check-suppressions.xml ✅
3. JaCoCo - Configured in pom.xml with strict thresholds ✅
4. PIT Mutation Testing - Configured in pom.xml ✅

❌ What's Missing

1. Checkstyle - Uses google_checks.xml (external), no custom rules ❌
2. PMD - No custom ruleset, uses defaults ❌

Current Configuration

Checkstyle (pom.xml line 278-287)

<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-checkstyle-plugin</artifactId>
    <version>3.6.0</version>
    <configuration>
        <configLocation>google_checks.xml</configLocation>  <!-- External -->
        <consoleOutput>true</consoleOutput>
        <failOnViolation>true</failOnViolation>
        <violationSeverity>error</violationSeverity>
        <logViolationsToConsole>true</logViolationsToConsole>
    </configuration>
</plugin>

Issues:

• Google checks may not match project conventions
• No way to enforce jcommons-specific rules
• Can't suppress project-specific patterns

PMD (pom.xml line 257-274)

<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-pmd-plugin</artifactId>
    <version>3.25.0</version>
    <configuration>
        <printFailingErrors>true</printFailingErrors>
        <failOnViolation>true</failOnViolation>
        <minimumTokens>100</minimumTokens>
        <!-- No <rulesets> configured - uses defaults -->
    </configuration>
</plugin>

Issues:

• Default PMD rules may not match project needs
• No CPD (Copy-Paste Detection) customization
• Can't enforce project conventions

Problems This Causes

1. Inconsistencies Not Caught

Logger naming (Issue #133):

• Some use logger, some use LOGGER
• Checkstyle could enforce: ConstantName rule

Varargs terminology (Issue #132):

• Inconsistent JavaDoc style
• Could enforce via Checkstyle JavaDoc rules

Public constants (Issue #130):

• Could warn about public constants in utility classes
• PMD rule: ConstantsInInterface, custom rules

@SuppressWarnings without comments (Issue #123):

• Could enforce comment requirement
• PMD rule: custom XPath rule

2. Google Checks May Conflict

Google style guide differs from observed patterns:

• Indentation: Google uses 2 spaces, project appears to use 4
• Line length: Google uses 100, project may vary
• Javadoc requirements: Google is strict, project may be more lenient

3. No Project-Specific Rules

Can't enforce jcommons conventions:

• Utility class pattern (private constructor + AssertionError)
• Logger naming (LOGGER vs logger)
• Exception naming (*Exception suffix)
• Test naming (*Test suffix)

Impact

• Severity: Medium
• Type: Code Quality / Tooling
• Inconsistencies persist that could be auto-detected
• No enforcement of project-specific conventions
• Harder to maintain consistent code style
• New contributors don't get automatic guidance

Recommended Fix

1. Create Custom Checkstyle Configuration

Create checkstyle.xml:

<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
    "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
    "https://checkstyle.org/dtds/configuration_1_3.dtd">

<module name="Checker">
    <!-- Based on Google but customized for jcommons -->
    
    <!-- File requirements -->
    <module name="FileTabCharacter"/>
    <module name="NewlineAtEndOfFile"/>
    
    <module name="TreeWalker">
        <!-- Naming conventions -->
        <module name="ConstantName">
            <property name="format" value="^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$"/>
            <message key="name.invalidPattern"
                     value="Constant ''{0}'' must be UPPER_CASE"/>
        </module>
        
        <module name="LocalVariableName"/>
        <module name="MemberName"/>
        <module name="MethodName"/>
        <module name="PackageName"/>
        <module name="ParameterName"/>
        <module name="StaticVariableName"/>
        <module name="TypeName"/>
        
        <!-- Javadoc requirements -->
        <module name="JavadocMethod">
            <property name="scope" value="public"/>
        </module>
        <module name="JavadocType"/>
        <module name="JavadocVariable">
            <property name="scope" value="public"/>
        </module>
        
        <!-- Code quality -->
        <module name="EmptyBlock"/>
        <module name="LeftCurly"/>
        <module name="NeedBraces"/>
        <module name="RightCurly"/>
        
        <!-- Whitespace -->
        <module name="WhitespaceAround"/>
        <module name="WhitespaceAfter"/>
        
        <!-- Imports -->
        <module name="AvoidStarImport"/>
        <module name="RedundantImport"/>
        <module name="UnusedImports"/>
    </module>
</module>

2. Create Custom PMD Ruleset

Create pmd-ruleset.xml:

<?xml version="1.0"?>
<ruleset name="jcommons PMD Rules"
         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>PMD rules for jcommons project</description>
    
    <!-- Code Style -->
    <rule ref="category/java/codestyle.xml">
        <exclude name="AtLeastOneConstructor"/>
        <exclude name="OnlyOneReturn"/>
        <exclude name="LongVariable"/>
    </rule>
    
    <!-- Best Practices -->
    <rule ref="category/java/bestpractices.xml">
        <exclude name="UseVarargs"/> <!-- Already using varargs -->
    </rule>
    
    <!-- Design -->
    <rule ref="category/java/design.xml">
        <exclude name="LawOfDemeter"/>
        <exclude name="LoosePackageCoupling"/>
    </rule>
    
    <!-- Error Prone -->
    <rule ref="category/java/errorprone.xml"/>
    
    <!-- Performance -->
    <rule ref="category/java/performance.xml"/>
    
    <!-- Security -->
    <rule ref="category/java/security.xml"/>
    
    <!-- Custom rule: Require comment with @SuppressWarnings -->
    <rule name="SuppressWarningsWithoutComment"
          language="java"
          message="@SuppressWarnings must have explanatory comment"
          class="net.sourceforge.pmd.lang.rule.XPathRule">
        <description>
            Each @SuppressWarnings annotation should have a comment explaining why
            the warning is suppressed and why it's safe.
        </description>
        <priority>3</priority>
        <properties>
            <property name="xpath">
                <value>
                    //Annotation[@Image='SuppressWarnings']
                    [not(preceding-sibling::Comment)]
                </value>
            </property>
        </properties>
    </rule>
</ruleset>

3. Update pom.xml

Checkstyle:

<configuration>
    <configLocation>checkstyle.xml</configLocation>  <!-- Changed -->
    <consoleOutput>true</consoleOutput>
    <failOnViolation>true</failOnViolation>
</configuration>

PMD:

<configuration>
    <rulesets>
        <ruleset>pmd-ruleset.xml</ruleset>  <!-- Added -->
    </rulesets>
    <printFailingErrors>true</printFailingErrors>
    <failOnViolation>true</failOnViolation>
</configuration>

4. Add Execution to Checkstyle

Currently Checkstyle plugin has no <executions>:

<executions>
    <execution>
        <phase>validate</phase>
        <goals>
            <goal>check</goal>
        </goals>
    </execution>
</executions>

Benefits

1. Automated consistency - Catch issues in CI/CD
2. Project-specific rules - Enforce jcommons conventions
3. Better onboarding - New contributors get immediate feedback
4. Customizable - Can add/remove rules as needed
5. Documentation - Rules document coding standards

Related Issues

Custom rules would help enforce:

• Inconsistent logger field naming: logger vs LOGGER #133 - Logger naming consistency
• Inconsistent varargs terminology and grammar in JavaDoc #132 - Varargs terminology
• Public String constants should use more specific types or enums #130 - Public constant visibility
• Missing justification comments for @SuppressWarnings annotations #123 - @SuppressWarnings comments
• Inconsistent logger getter visibility across utility classes #126 - Logger getter visibility

Alternative: Document Why Google Checks

If Google checks are intentional, document why in CONTRIBUTING.md:

## Code Style

We follow Google Java Style Guide enforced via Checkstyle.
Deviations from Google style are documented in [issues].

[sfloess]

Current State:

The project uses well-established industry standards:

• Checkstyle: Google Java Style Guide (google_checks.xml)
• PMD: Default best practices ruleset
• SpotBugs: Custom exclusions (spotbugs-exclude.xml) ✅
• OWASP: Custom suppressions (dependency-check-suppressions.xml) ✅

Assessment:

Using standard rule sets (Google Style, PMD defaults) is actually a best practice:

1. Industry-recognized standards
2. Well-documented and maintained
3. New contributors already familiar
4. Less maintenance burden
5. Easier to onboard developers

Custom configs only needed when:

• Project has unique coding conventions that differ from standards
• Need to suppress specific rules consistently
• Have domain-specific quality requirements

Current Approach is Correct:

The project correctly:

• Uses SpotBugs exclusions for false positives
• Uses OWASP suppressions for known issues
• Relies on standard Checkstyle/PMD rules for quality

Recommendation:

Keep current configuration. Only add custom Checkstyle/PMD configs if:

1. Specific rules need adjustment (can be done incrementally)
2. Team has unique style requirements (none identified)
3. Standards conflict with project needs (not observed)

Marking as enhancement/low-priority. Current approach is sound.

[sfloess]

Analysis: Custom Configs Not Currently Needed

Current State

Checkstyle: Uses google_checks.xml (Google Java Style Guide)

• ✅ Zero violations in current build
• ✅ Industry-standard ruleset
• ✅ Well-maintained by Google

PMD: Uses standard rulesets

• ✅ Zero violations in current build
• ✅ Catches common bugs and code smells
• ✅ Configured with appropriate goals

SpotBugs: Has custom exclusions (spotbugs-exclude.xml)

• ✅ Only tool that needed customization
• ✅ Properly configured

Why Custom Configs Would Be Needed

Custom Checkstyle/PMD configs are useful when:

1. Team style differs from Google - Not applicable (we use Google style)
2. False positives - Currently zero violations
3. Project-specific rules - None identified yet
4. Conflicting standards - No conflicts present

Recommendation

Current action: Close as "won't fix"

Rationale:

• Google Checkstyle = industry best practice
• Zero violations = configs working well
• No custom rules identified
• YAGNI principle - don't add until needed

When to revisit:

• Team identifies Google style violations they want to allow
• New rules needed for project-specific patterns
• False positives emerge

Currently using standard, well-maintained configs is a strength, not a weakness.