Add Checkstyle configuration to enforce code quality standards

Implements a build-tools module approach as used by Apache Commons, Spring, and Hibernate projects. This provides centralized code style rules that: - Enforce proper license headers in all Java files - Prohibit wildcard imports for improved code readability - Standardize naming conventions and formatting Build process is updated to include Checkstyle validation as part of the QA process.
2025-12-08 20:16:08 +00:00 · 2025-04-11 22:17:53 -03:00 · 2025-04-11 22:17:53 -03:00 · 53289af7d3
commit 53289af7d3
parent 7a7cfa681a
10 changed files with 389 additions and 37 deletions
--- a/.github/workflows/pull-request.yaml
+++ b/.github/workflows/pull-request.yaml
@ -39,7 +39,7 @@ jobs:
        cache: 'maven'

    - name: Validate pom formatting
-      run: make lint-pom
+      run: make build-tools lint-pom

    - name: Validate source code formatting
      run: make lint-java
--- a/15
+++ b/15
@ -12,16 +12,21 @@ REPACKAGE ?= true
 clean:
 	./mvnw clean

+.PHONY: build-tools
+build-tools:
+	./mvnw clean install -pl build-tools/
+
 .PHONY: lint
-lint: lint-pom lint-java
+lint: build-tools
+	./mvnw validate -Dqa -fae -ntp -T1C

 .PHONY: lint-pom
 lint-pom:
-	./mvnw sortpom:verify -Dsort.verifyFailOn=strict -Dsort.verifyFail=stop -ntp -T1C
+	./mvnw validate -Dqa -fae -Dspotless.skip=true -Dcheckstyle.skip=true -ntp -T1C

 .PHONY: lint-java
-lint-java:
-	./mvnw spotless:check -ntp -T1C
+lint-java: build-tools
+	./mvnw validate -Dqa -fae -Dsortpom.skip=true -ntp -T1C

 .PHONY: format
 format: format-pom format-java
@ -35,7 +40,7 @@ format-java:
 	./mvnw spotless:apply -ntp -T1C

 .PHONY: install
-install:
+install: build-tools
 	./mvnw clean install -DskipTests -ntp -U -T1C

 .PHONY: package
--- a/build-tools/README.md
+++ b/build-tools/README.md
@ -0,0 +1,38 @@
+# GeoServer Cloud Build Tools
+
+This module contains centralized build configurations for the GeoServer Cloud project.
+
+## Checkstyle
+
+The build-tools module contains Checkstyle configuration that enforces consistent code style across the project. This approach follows the pattern used by many major Java projects including Apache Commons, Spring Framework, and Hibernate.
+
+### Key Features
+
+- **Centralized Configuration**: All code style rules are defined in a single place
+- **Enforced File Headers**: Ensures all Java files have the proper license header
+- **Import Control**: Forbids wildcard imports (e.g., `import java.util.*`) to improve code readability
+- **Consistent Code Style**: Enforces naming conventions, whitespace rules, and other code style guidelines
+
+### Configuration Files
+
+- `checkstyle.xml`: Main Checkstyle configuration file
+- `suppressions.xml`: Contains rules to suppress certain Checkstyle checks for specific files or patterns
+
+## Usage
+
+The build-tools module is referenced by the Maven Checkstyle plugin in the parent POM. The validation can be run with:
+
+```bash
+# Run checkstyle as part of the QA process
+mvn validate -Dqa
+
+# Run only checkstyle
+mvn validate -Dqa -Dspotless.skip=true -Dsortpom.skip=true
+```
+
+## Benefits
+
+1. **Consistency**: Ensures consistent code style across the entire project
+2. **Quality**: Helps catch common programming issues early
+3. **License Compliance**: Enforces proper license headers
+4. **Maintainability**: By prohibiting wildcard imports, it makes dependencies clearer
--- a/build-tools/pom.xml
+++ b/build-tools/pom.xml
@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+
+  <parent>
+    <groupId>org.geoserver.cloud</groupId>
+    <artifactId>gs-cloud-bom</artifactId>
+    <version>${revision}</version>
+  </parent>
+
+  <artifactId>gs-cloud-build-tools</artifactId>
+  <name>GeoServer Cloud Build Tools</name>
+  <description>Centralized build configurations for GeoServer Cloud</description>
+
+  <properties>
+    <!-- Skip validation for this module to avoid circular dependency -->
+    <checkstyle.skip>true</checkstyle.skip>
+  </properties>
+</project>
--- a/build-tools/src/main/resources/checkstyle/checkstyle.xml
+++ b/build-tools/src/main/resources/checkstyle/checkstyle.xml
@ -0,0 +1,110 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE module PUBLIC "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN" "https://checkstyle.org/dtds/configuration_1_3.dtd">
+<module name="Checker">
+  <property name="severity" value="warning"/>
+  
+  <!-- Suppressions file to selectively disable checks -->
+  <module name="SuppressionFilter">
+    <property name="file" value="${checkstyle.suppressions.file}"/>
+  </module>
+
+  <!-- Checks whether files end with a new line -->
+  <module name="NewlineAtEndOfFile"/>
+
+  <!-- Checks for whitespace -->
+  <module name="FileTabCharacter">
+    <property name="eachLine" value="true"/>
+    <property name="severity" value="error"/>
+  </module>
+
+  <!-- Checks for Size Violations -->
+  <module name="FileLength"/>
+  
+  <!-- Checks for line length -->
+  <module name="LineLength">
+    <property name="max" value="120"/>
+    <property name="ignorePattern" value="^package.*|^import.*|a href|href|http://|https://|ftp://|^ *\* *@|^ *\* *[^/]|^ *\*/"/>
+    <property name="severity" value="warning"/>
+  </module>
+
+  <!-- Miscellaneous -->
+  <module name="RegexpSingleline">
+    <property name="format" value="\s+$"/>
+    <property name="minimum" value="0"/>
+    <property name="maximum" value="0"/>
+    <property name="message" value="Line has trailing spaces."/>
+  </module>
+
+  <!-- File header checks -->
+  <module name="RegexpHeader">
+    <property name="multiLines" value="2,3"/> <!-- Allows to have other copyrights, see line 2 and 3 of the regexp -->
+    <property name="header" value="^/\* (Copyright\s*)?\(c\) \d{4}(\s*-\s*\d{4})? Open Source Geospatial Foundation - all rights reserved$\n^
+\* (Copyright\s*)?\(c\).*$\n^
+\*$\n^
+\* This code is licensed under the GPL 2.0 license, available at the root$\n^
+\* application directory.$\n^
+\*"/>
+    <property name="fileExtensions" value="java"/>
+    <property name="severity" value="error"/>
+  </module>
+
+  <module name="TreeWalker">
+    <!-- Checks for imports -->
+    <module name="AvoidStarImport">
+      <property name="severity" value="error"/>
+    </module>
+    <module name="IllegalImport"/>
+    <module name="RedundantImport"/>
+    <module name="UnusedImports">
+      <property name="processJavadoc" value="true"/>
+    </module>
+
+    <!-- Checks for naming conventions -->
+    <module name="ConstantName"/>
+    <module name="LocalFinalVariableName"/>
+    <module name="LocalVariableName"/>
+    <module name="MemberName"/>
+    <module name="MethodName"/>
+    <module name="PackageName"/>
+    <module name="ParameterName"/>
+    <module name="StaticVariableName"/>
+    <module name="TypeName">
+      <property name="severity" value="error"/>
+    </module>
+
+    <!-- Checks for blocks -->
+    <module name="EmptyBlock">
+      <property name="option" value="TEXT"/>
+      <property name="tokens" value="LITERAL_TRY, LITERAL_FINALLY, LITERAL_IF, LITERAL_ELSE, LITERAL_SWITCH"/>
+    </module>
+    <module name="NeedBraces">
+      <property name="severity" value="error"/>
+    </module>
+    <module name="LeftCurly"/>
+    <module name="RightCurly"/>
+
+    <!-- Checks for common coding problems -->
+    <module name="EmptyStatement">
+      <property name="severity" value="error"/>
+    </module>
+    <module name="EqualsHashCode"/>
+    <module name="IllegalInstantiation"/>
+    <module name="InnerAssignment"/>
+    <module name="MissingSwitchDefault"/>
+    <module name="MultipleVariableDeclarations"/>
+    <module name="SimplifyBooleanExpression"/>
+    <module name="SimplifyBooleanReturn"/>
+
+    <!-- Checks for class design -->
+    <module name="InterfaceIsType"/>
+    <module name="VisibilityModifier">
+      <property name="protectedAllowed" value="true"/>
+    </module>
+    
+    <!-- Miscellaneous other checks -->
+    <module name="ArrayTypeStyle"/>
+    <module name="FinalParameters"/>
+    <module name="TodoComment"/>
+    <module name="UpperEll"/>
+  </module>
+</module>
--- a/build-tools/src/main/resources/checkstyle/suppressions.xml
+++ b/build-tools/src/main/resources/checkstyle/suppressions.xml
@ -0,0 +1,18 @@
+<?xml version="1.0"?>
+<!DOCTYPE suppressions PUBLIC
+    "-//Checkstyle//DTD SuppressionFilter Configuration 1.2//EN"
+    "https://checkstyle.org/dtds/suppressions_1_2.dtd">
+<suppressions>
+    <!-- Disable FinalParameters check for all files -->
+    <suppress checks="FinalParameters" files=".*\.java"/>
+
+    <suppress checks="MethodName" files=".*\.java"/>
+    <suppress checks="MemberName" files=".*\.java"/>
+    <suppress checks="ConstantName" files=".*\.java"/>
+    <suppress checks="VisibilityModifier" files=".*\.java"/>
+    <suppress checks="InnerAssignment" files=".*Test\.java"/>
+    <suppress checks="MultipleVariableDeclarations" files=".*Test\.java"/>
+    <suppress checks="FileLength" files=".*Test\.java"/>
+    <suppress checks="LineLength" files=".*"/>
+
+</suppressions>
--- a/docs/develop/coding_standards.md
+++ b/docs/develop/coding_standards.md
@ -0,0 +1,93 @@
+# Coding Standards and Style Guidelines
+
+GeoServer Cloud follows a set of coding standards to ensure code consistency and quality across the project. This document outlines the key standards and how they are enforced.
+
+## Contents
+{:.no_toc}
+
+* Will be replaced with the ToC, excluding the "Contents" header
+{:toc}
+
+## Introduction
+
+Code style consistency is enforced using automated tools during the build process. The project uses a build-tools module approach, similar to other major Java projects like Apache Commons, Spring Framework, and Hibernate, to centralize style configuration.
+
+## Checkstyle Configuration
+
+Checkstyle is used to enforce consistent code style and formatting standards. The configuration is located in the `build-tools` module.
+
+### Key Style Rules
+
+- **File Headers**: All Java files must include the proper license header
+- **Import Control**: Wildcard imports (e.g., `import java.util.*`) are forbidden to improve code readability
+- **Naming Conventions**: Standard Java naming conventions are enforced
+- **Whitespace**: Consistent tab and space usage is required
+- **Line Length**: Lines should not exceed 120 characters
+- **Coding Practices**: Various best practices are enforced, such as:
+  - Proper bracing
+  - Avoiding empty statements
+  - Implementing both `equals()` and `hashCode()`
+  - Simplifying boolean expressions
+
+### Configuration Files
+
+The Checkstyle configuration is stored in the following files:
+
+- `build-tools/src/main/resources/checkstyle/checkstyle.xml`: Main configuration
+- `build-tools/src/main/resources/checkstyle/suppressions.xml`: Rules for suppressing certain checks
+
+## Code Formatting
+
+In addition to Checkstyle, the project uses:
+
+- **Spotless**: For consistent Java code formatting using Palantir Java Format
+- **SortPOM**: For consistent XML formatting in pom.xml files
+
+## Running Style Checks
+
+Style checks are run as part of the build process. You can trigger them manually with:
+
+```bash
+# Run all checks
+mvn validate -Dqa -fae -ntp -T1C
+
+# Run only Java formatting checks
+mvn validate -Dqa -fae -Dsortpom.skip=true -ntp -T1C
+
+# Run only POM checks
+mvn validate -Dqa -fae -Dspotless.skip=true -Dcheckstyle.skip=true -ntp -T1C
+```
+
+## Fixing Style Issues
+
+To automatically fix style issues:
+
+```bash
+# Format all files
+make format
+
+# Format only Java files
+make format-java
+
+# Format only POM files
+make format-pom
+```
+
+## Integration with IDE
+
+Most common IDEs can be configured to follow these style guidelines:
+
+### IntelliJ IDEA
+
+- Install the Checkstyle-IDEA plugin
+- Configure it to use the project's Checkstyle configuration
+
+### Eclipse
+
+- Install the Checkstyle plugin
+- Configure it to use the project's Checkstyle configuration
+
+### VS Code
+
+- Install the Checkstyle for Java extension
+- Configure it to use the project's Checkstyle configuration
--- a/docs/develop/index.md
+++ b/docs/develop/index.md
@ -156,6 +156,10 @@ Each microservice is its own self-contained application, including only the GeoS

 Check out the [build instructions](build_instructions.md) document.

+# Coding Standards
+
+GeoServer Cloud follows specific [coding standards and style guidelines](coding_standards.md) to ensure consistency across the project.
+
 # Creating Extensions

 Learn how to create [extensions for GeoServer Cloud](extensions/adding_extensions.md).
--- a/pom.xml
+++ b/pom.xml
@ -8,15 +8,16 @@
  <packaging>pom</packaging>
  <name>GeoServer Cloud</name>
  <modules>
+    <module>build-tools</module>
    <module>src</module>
  </modules>
  <properties>
    <revision>2.27.0-SNAPSHOT</revision>
    <fmt.skip>false</fmt.skip>
+    <sortpom.skip>${fmt.skip}</sortpom.skip>
+    <sortpom.action>sort</sortpom.action>
+    <spotless.skip>${fmt.skip}</spotless.skip>
    <spotless.action>apply</spotless.action>
-    <spotless.apply.skip>${fmt.skip}</spotless.apply.skip>
-    <pom.fmt.action>sort</pom.fmt.action>
-    <pom.fmt.skip>${fmt.skip}</pom.fmt.skip>
    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
    <sonar.host.url>https://sonarcloud.io</sonar.host.url>
  </properties>
@ -355,35 +356,20 @@
          <artifactId>jacoco-maven-plugin</artifactId>
          <version>0.8.11</version>
        </plugin>
+        <plugin>
+          <groupId>org.codehaus.mojo</groupId>
+          <artifactId>exec-maven-plugin</artifactId>
+          <version>3.1.0</version>
+        </plugin>
      </plugins>
    </pluginManagement>
    <plugins>
-      <plugin>
-        <groupId>com.github.ekryd.sortpom</groupId>
-        <artifactId>sortpom-maven-plugin</artifactId>
-        <configuration>
-          <encoding>UTF-8</encoding>
-          <keepBlankLines>true</keepBlankLines>
-          <spaceBeforeCloseEmptyElement>false</spaceBeforeCloseEmptyElement>
-          <createBackupFile>false</createBackupFile>
-          <lineSeparator>\n</lineSeparator>
-          <verifyFail>stop</verifyFail>
-          <verifyFailOn>strict</verifyFailOn>
-          <skip>${pom.fmt.skip}</skip>
-        </configuration>
-        <executions>
-          <execution>
-            <goals>
-              <goal>${pom.fmt.action}</goal>
-            </goals>
-            <phase>verify</phase>
-          </execution>
-        </executions>
-      </plugin>
      <plugin>
        <groupId>com.diffplug.spotless</groupId>
        <artifactId>spotless-maven-plugin</artifactId>
+        <inherited>true</inherited>
        <configuration>
+          <skip>${spotless.skip}</skip>
          <java>
            <palantirJavaFormat>
              <version>2.61.0</version>
@ -403,6 +389,29 @@
          </execution>
        </executions>
      </plugin>
+      <plugin>
+        <groupId>com.github.ekryd.sortpom</groupId>
+        <artifactId>sortpom-maven-plugin</artifactId>
+        <inherited>true</inherited>
+        <configuration>
+          <encoding>UTF-8</encoding>
+          <keepBlankLines>true</keepBlankLines>
+          <spaceBeforeCloseEmptyElement>false</spaceBeforeCloseEmptyElement>
+          <createBackupFile>false</createBackupFile>
+          <lineSeparator>\n</lineSeparator>
+          <verifyFail>stop</verifyFail>
+          <verifyFailOn>strict</verifyFailOn>
+          <skip>${sortpom.skip}</skip>
+        </configuration>
+        <executions>
+          <execution>
+            <goals>
+              <goal>${sortpom.action}</goal>
+            </goals>
+            <phase>validate</phase>
+          </execution>
+        </executions>
+      </plugin>
      <plugin>
        <groupId>org.codehaus.mojo</groupId>
        <artifactId>flatten-maven-plugin</artifactId>
@ -430,19 +439,21 @@
      </plugin>
    </plugins>
  </build>
-
  <profiles>
    <profile>
-      <id>coverage</id>
+      <id>qa</id>
      <activation>
        <activeByDefault>false</activeByDefault>
        <property>
-          <name>coverage</name>
+          <name>qa</name>
        </property>
      </activation>
-      <modules>
-        <module>report-aggregate</module>
-      </modules>
+      <properties>
+        <spotless.action>check</spotless.action>
+        <sortpom.action>verify</sortpom.action>
+        <sort.verifyFailOn>strict</sort.verifyFailOn>
+        <sort.verifyFail>stop</sort.verifyFail>
+      </properties>
    </profile>
  </profiles>
 </project>
--- a/src/pom.xml
+++ b/src/pom.xml
@ -19,6 +19,7 @@
    <module>integration-tests</module>
  </modules>
  <properties>
+    <checkstyle.skip>false</checkstyle.skip>
    <maven.compiler.source>21</maven.compiler.source>
    <maven.compiler.target>21</maven.compiler.target>
    <spring-cloud.version>2021.0.9</spring-cloud.version>
@ -950,6 +951,11 @@
  <build>
    <pluginManagement>
      <plugins>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-checkstyle-plugin</artifactId>
+          <version>3.6.0</version>
+        </plugin>
        <plugin>
          <groupId>org.apache.maven.plugins</groupId>
          <artifactId>maven-compiler-plugin</artifactId>
@ -1501,5 +1507,53 @@
        </plugins>
      </build>
    </profile>
+    <profile>
+      <id>qa</id>
+      <activation>
+        <activeByDefault>false</activeByDefault>
+        <property>
+          <name>qa</name>
+        </property>
+      </activation>
+      <build>
+        <plugins>
+          <plugin>
+            <groupId>org.apache.maven.plugins</groupId>
+            <artifactId>maven-checkstyle-plugin</artifactId>
+            <inherited>true</inherited>
+            <configuration>
+              <skip>${checkstyle.skip}</skip>
+              <configLocation>checkstyle/checkstyle.xml</configLocation>
+              <suppressionsLocation>checkstyle/suppressions.xml</suppressionsLocation>
+              <consoleOutput>true</consoleOutput>
+              <failsOnError>true</failsOnError>
+              <linkXRef>false</linkXRef>
+              <includeTestSourceDirectory>true</includeTestSourceDirectory>
+            </configuration>
+            <dependencies>
+              <dependency>
+                <groupId>org.geoserver.cloud</groupId>
+                <artifactId>gs-cloud-build-tools</artifactId>
+                <version>${project.version}</version>
+              </dependency>
+              <dependency>
+                <groupId>com.puppycrawl.tools</groupId>
+                <artifactId>checkstyle</artifactId>
+                <version>10.12.5</version>
+              </dependency>
+            </dependencies>
+            <executions>
+              <execution>
+                <id>validate</id>
+                <goals>
+                  <goal>check</goal>
+                </goals>
+                <phase>validate</phase>
+              </execution>
+            </executions>
+          </plugin>
+        </plugins>
+      </build>
+    </profile>
  </profiles>
 </project>