Skip to content

Checkstyle not bound to build lifecycle - only runs when manually invoked #198

@sfloess

Description

@sfloess

Problem

Checkstyle plugin is configured in pom.xml but not bound to any lifecycle phase. It only runs when explicitly invoked (mvn checkstyle:check), meaning style violations are not automatically caught.

Evidence

Current Configuration (pom.xml lines 276-287)

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

Compare with Other Tools

SpotBugs (lines 237-255):

<plugin>
    <artifactId>spotbugs-maven-plugin</artifactId>
    <executions>
        <execution>
            <goals>
                <goal>check</goal>  <!-- ✅ Runs automatically -->
            </goals>
        </execution>
    </executions>
</plugin>

PMD (lines 257-274):

<plugin>
    <artifactId>maven-pmd-plugin</artifactId>
    <executions>
        <execution>
            <goals>
                <goal>check</goal>  <!-- ✅ Runs automatically -->
                <goal>cpd-check</goal>
            </goals>
        </execution>
    </executions>
</plugin>

Checkstyle: ❌ No <executions> section

Impact

Not Enforced in CI

# Current CI behavior
mvn clean verify
# Runs: compile, test, JaCoCo, SpotBugs, PMD, enforcer
# Does NOT run: Checkstyle

Manual Invocation Required

# Developer must remember to run
mvn checkstyle:check

Style Violations Slip Through

  • Code with style violations can be committed
  • CI builds pass even with Google Style Guide violations
  • Inconsistent code style in codebase
  • PR validation doesn't catch style issues

Inconsistent Quality Gates

Currently enforced automatically:

  • ✅ SpotBugs: Zero bugs
  • ✅ PMD: Code quality rules
  • ✅ JaCoCo: 93% coverage
  • ✅ Maven Enforcer: Java 17+, dependency convergence

Not enforced automatically:

  • ❌ Checkstyle: Google Java Style Guide

Recommended Solution

Add Section

<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-checkstyle-plugin</artifactId>
    <version>3.6.0</version>
    <configuration>
        <configLocation>google_checks.xml</configLocation>
        <consoleOutput>true</consoleOutput>
        <failOnViolation>true</failOnViolation>
        <violationSeverity>error</violationSeverity>
        <logViolationsToConsole>true</logViolationsToConsole>
    </configuration>
    <!-- ADD THIS: -->
    <executions>
        <execution>
            <id>validate-style</id>
            <phase>validate</phase>
            <goals>
                <goal>check</goal>
            </goals>
        </execution>
    </executions>
</plugin>

Why validate Phase?

  • validate: Earliest phase (before compile)
  • Fails fast if style violations exist
  • Saves time (don't compile bad code)
  • Alternative: verify phase (runs after tests)

Expected Behavior After Fix

mvn clean verify
# Now runs Checkstyle automatically
# Fails if Google Style Guide violations found
# Output: "Checkstyle violations found: 3 errors"

CI/CD Impact

main.yml workflow:

mvn clean compile test
# Will now fail on Checkstyle violations
# Enforces Google Style Guide in all PRs

Developer Workflow

# Before commit
mvn verify
# Now catches style violations automatically
# No need to remember mvn checkstyle:check

Phase Selection Considerations

Option 1: validate (Recommended)

<phase>validate</phase>

Pros: Fails fast, before compilation
Cons: Runs on every build (slightly slower)

Option 2: verify

<phase>verify</phase>

Pros: Only after tests pass
Cons: Wastes time if code doesn't compile

Option 3: Default (compile)

<!-- No phase specified - defaults to compile -->

Pros: Standard location
Cons: After compilation started

Recommendation: Use validate for fail-fast behavior

Verification After Fix

# Introduce a style violation
# (e.g., use tab instead of spaces)

mvn clean verify
# Should fail with Checkstyle error

# Fix violation
mvn clean verify
# Should pass

Comparison with Similar Projects

Apache Commons Lang

<plugin>
    <artifactId>maven-checkstyle-plugin</artifactId>
    <executions>
        <execution>
            <phase>verify</phase>  <!-- ✅ Bound to lifecycle -->
            <goals>
                <goal>check</goal>
            </goals>
        </execution>
    </executions>
</plugin>

Google Guava

  • ✅ Checkstyle bound to lifecycle
  • Enforces Google Style Guide automatically

jcommons (Current)

  • ❌ Checkstyle not bound
  • ❌ Not enforced in builds

Alternative: Add to CI Explicitly

If don't want to bind to lifecycle, at least add to CI:

.github/workflows/main.yml:

- name: Run Checkstyle
  run: mvn checkstyle:check

.github/workflows/pr-validation.yml:

- name: Checkstyle Analysis
  run: mvn checkstyle:check

Note: This already exists in pr-validation.yml but not main.yml

Impact Assessment

Breaking Change?

NO - Checkstyle should already pass

  • Code should conform to Google Style Guide
  • CONTRIBUTING.md mentions Google Style
  • If violations exist, they're bugs

Risk

LOW - If current code has violations, build will fail

  • Can be fixed before merging
  • Helps enforce existing standards

Related

Metadata

Metadata

Assignees

No one assigned

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions