Static Analysis for Java: SpotBugs, Checkstyle & SonarQube
Friday, 4:47 PM: the checkout service passes review, compiles clean, and ships. At 2:13 AM — NullPointerException in production, on a coupon path nobody tested. Two senior engineers reviewed it. javac was happy. Tests were green. The bug walked through all three gates because none of them look for this kind of bug.
This post is the fourth gate: three tools, three jobs, one buggy class run through all of them — real output, build wiring, and the traps that make teams quit.
Why the compiler isn't enough
javac answers one question: is this program well-formed? Syntax, types, unreachable code — caught. But whole categories of real bugs are perfectly well-formed programs:
- Null dereference on an unseen path.
javacdoesn't track which paths leave a variable null — aNullPointerExceptionwaiting on thecoupon == nullpath is invisible to it. - Ignored return values.
audit.toUpperCase();compiles — but strings are immutable, so the result evaporates. ==on Strings. Reference comparison is legal Java — just almost never what you meant.- Self-assignment, mutable statics, empty catches. All legal. All bugs.
- Style drift across a team. Five developers, five brace styles. The compiler doesn't care; your future self does.
Decision rule: javac-catchable means compile problem; a pattern in legal code means static analysis — three jobs, three tools:
- SpotBugs — probable bugs. Reads bytecode, matches hundreds of known bug patterns. Answers "this looks like a bug."
- Checkstyle — the team contract. Reads source, enforces agreed style rules. Answers "this doesn't look like our code."
- SonarQube — quality over time. Aggregates bugs, smells, duplication, coverage across runs. Answers "is this codebase getting better or worse?" — its quality gate decides whether the build ships.
Here's where they sit in the build:
Decision rule: SpotBugs and Checkstyle run locally on every build. SonarQube runs per pull request — it needs history and coverage. A SonarQube server nobody gates on is a dashboard nobody reads.
The victim: one class, six bugs
Meet OrderValidator — 51 lines, zero javac warnings, passes review ("looks fine, ship it"). Six genuine bugs. Count how many you spot first:
import java.util.ArrayList;
import java.util.List;
public class OrderValidator {
// NOTE: pricing-team rule — holiday coupons stack with regional discounts, except EU orders over 5000, which cap out.
public static List<String> SUPPORTED_REGIONS = new ArrayList<>();
static {
SUPPORTED_REGIONS.add("US");
SUPPORTED_REGIONS.add("EU");
}
public double discountedTotal(String coupon, double total, String region) {
String code = null;
if (coupon != null) {
code = coupon.trim();
}
boolean hasCoupon = code.length() > 0;
String audit = "coupon=" + code + ", region=" + region;
audit.toUpperCase();
double discount = 0.0;
if (code == "SAVE20") {
discount = 0.20;
}
discount = discount;
if (!SUPPORTED_REGIONS.contains(region)) {
discount = 0.0;
}
try {
recordAudit(audit);
} catch (Exception e) {
}
if (discount > 0.5)
return total * 0.5;
return total * (1 - discount);
}
private void recordAudit(String entry) throws Exception {
if (entry == null) {
throw new Exception("empty audit entry");
}
System.out.println("AUDIT: " + entry);
}
}javac OrderValidator.java — silence. Now the tools:
What SpotBugs sees: five probable bugs
SpotBugs reads bytecode — what the code does, not how it's formatted. mvn spotbugs:check reports:
M C NP_NULL_ON_SOME_PATH: Possible null pointer dereference in OrderValidator.discountedTotal(String, double, String) At OrderValidator.java:[line 20]
M C RV_RETURN_VALUE_IGNORED: Return value of String.toUpperCase() ignored in OrderValidator.discountedTotal(String, double, String) At OrderValidator.java:[line 23]
H C ES_COMPARING_STRINGS_WITH_EQ: Comparison of String objects using == or != in OrderValidator.discountedTotal(String, double, String) At OrderValidator.java:[line 26]
M C SA_LOCAL_SELF_ASSIGNMENT: Self assignment of discount to itself in OrderValidator.discountedTotal(String, double, String) At OrderValidator.java:[line 29]
M M MS_MUTABLE_COLLECTION: Public static field OrderValidator.SUPPORTED_REGIONS is a mutable collection At OrderValidator.java:[line 7]Every code is a real SpotBugs pattern, and every one is a real bug:
- NP_NULL_ON_SOME_PATH (line 20). When
couponis null,codestays null andcode.length()explodes — the 2 AM page. - RV_RETURN_VALUE_IGNORED (line 23).
toUpperCase()has no side effects — it returns a new string. Discarding the result makes the line a no-op. - ES_COMPARING_STRINGS_WITH_EQ (line 26).
==compares references, so equal-valued strings from different sources fail the check and the discount silently never applies. (Why reference comparison lies: the equality and hashing post.) - SA_LOCAL_SELF_ASSIGNMENT (line 29).
discount = discount;— a no-op that usually means the author meant to assign something else. - MS_MUTABLE_COLLECTION (line 7). A public static
ArrayListanyone canclear()from anywhere — shared mutable global state.
What Checkstyle sees: the team contract
Checkstyle doesn't hunt bugs — it enforces the team's code contract. Google checks say:
[WARN] OrderValidator.java:37:9: EmptyCatchBlock: Empty catch block.
[WARN] OrderValidator.java:7:5: VThe empty catch is both bug and contract violation; FinalClass and LineLength are pure consistency. That's the point: Checkstyle ends the braceless-if debate permanently.
What SonarQube sees: the same bugs, plus the trend
SonarQube flags the same six problems with its own rule keys:
java:S2259 Bug Null pointers should not be dereferenced (line 20)
java:S2201 Code Smell Return values from functions without side effects should not
be ignored (line 23)
java:S4973 Bug Strings and Boxed types should be compared using equals() (line 26)
java:S1656 Code Smell Variables should not be self-assigned (line 29)
java:S108 Code Smell Nested blocks of code should not be left empty (line 37)
java:S2386 Vulnerability Mutable fields should not be "public static" (line 7)The overlap is deliberate — SonarQube's value is aggregation: duplication, coverage, complexity trends, new issues. That feeds the quality gate — e.g. "zero new blockers, ≥80% coverage on new code" — which fails the build when violated.
The fixed version
Same class, six bugs gone — each fix a line or two:
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
public final class OrderValidator {
private static final List<String> SUPPORTED_REGIONS;
static {
List<String> regions = new ArrayList<>();
regions.add("US");
regions.add("EU");
SUPPORTED_REGIONS = Collections.unmodifiableList(regions);
}
public double discountedTotal(String coupon, double total, String region) {
String code = coupon == null ? "" : coupon.trim();
boolean hasCoupon = !code.isEmpty();
String audit = "coupon=" + code + ", region=" + region;
String normalizedAudit = audit.toUpperCase();
System.out.println("AUDIT (upper): " + normalizedAudit);
double discount = 0.0;
if (hasCoupon && "SAVE20".equals(code)) {
discount = 0.20;
}
if (!SUPPORTED_REGIONS.contains(region)) {
discount = 0.0;
}
try {
recordAudit(audit);
} catch (Exception e) {
throw new IllegalStateException("audit failed for " + audit, e);
}
if (discount > 0.5) {
return total * 0.5;
}
return total * (1 - discount);
}
private void recordAudit(String entry) throws Exception {
if (entry == null) {
throw new Exception("empty audit entry");
}
System.out.println("AUDIT: " + entry);
}
}What changed, fix by fix:
- Null path closed:
coupon == null ? "" : coupon.trim()—codecan never be null downstream. - Return value used:
toUpperCase()feedsnormalizedAudit, which is printed. - Value comparison:
"SAVE20".equals(code)— content, not references; literal-first is null-safe. - Self-assignment deleted.
- Empty catch replaced: throws
IllegalStateExceptionchaining the cause — audit failures are loud now. (The Exceptions post's "never silently swallow", enforced by a tool.) - Mutable static locked down:
private static final+Collections.unmodifiableList. - Contract nits:
final class, braces everywhere, short lines.
All three tools against this version: silence. Not "no bugs ever" — the whole category of dumb bugs, handled before review.
Wiring it into the build: Maven
Hand-run tools run once; build-bound tools run forever:
SpotBugs — check fails the build on findings, bound to verify:
<plugin>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-maven-plugin</artifactId>
<version>4.8.6</version>
<configuration>
<effort>Max</effort>
<threshold>Medium</threshold>
<failOnError>true</failOnError>
</configuration>
<executions>
<execution>
<phase>verify</phase>
<goals>
<goal>check</goal>
</goals>
</execution>
</executions>
</plugin>effort=Max trades analysis time for fewer misses. Decision rule: start at threshold=High on existing code, lower it once the backlog is clean. Starting Low is trap #1 below.
Checkstyle — failsOnViolation makes a style break fail the build like a test failure:
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-checkstyle-plugin</artifactId>
<version>3.6.0</version>
<configuration>
<configLocation>google_checks.xml</configLocation>
<failsOnViolation>true</failsOnViolation>
</configuration>
<executions>
<execution>
<phase>verify</phase>
<goals>
<goal>check</goal>
</goals>
</execution>
</executions>
</plugin>google_checks.xml ships with the plugin — adopt it verbatim (trap #2 is the 200-rule custom config).
SonarQube needs no binding — invoke it directly after the build:
mvn verify sonar:sonar \
-Dsonar.host.url=https://sonar.yourcompany.com \
-Dsonar.token=$SONAR_TOKEN-Dsonar.qualitygate.wait=true makes the build block on the gate verdict — red gate, red build. Without it, analysis uploads and the build sails on: trap #4.
Gradle: com.github.spotbugs (spotbugsMain/spotbugsTest), built-in checkstyle, org.sonarqube (sonar) — wire analysis into check. Full mechanics in this track's Maven vs Gradle: Builds Demystified post.
The CI quality gate: ~10 lines that enforce everything
Local builds catch what developers run; CI catches what they forget:
name: quality-gates
on: [push, pull_request]
jobs:
analyze:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-java@v4
with: { distribution: temurin, java-version: "21" }
- run: mvn -B verify sonar:sonar -Dsonar.qualitygate.wait=true
env:
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}One command: compile, test, SpotBugs, Checkstyle, coverage, SonarQube, gate verdict — red blocks the merge. (Actions mechanics: this track's CI/CD with GitHub Actions for Java Projects post.) The quality gate belongs in CI, not in a wiki page asking developers nicely.
When it breaks: 4 static-analysis traps
Trap 1 — Analysis fatigue: 500 warnings everyone ignores
Symptom: threshold=Low on a five-year-old codebase — 500+ findings. The team decides the tool is noise and stops looking; five real bugs hide among 495 nits.
Fix: start high-confidence only, fix that list completely, gate the build on it, then ratchet down. Twelve respected findings beat 500 ignored ones.
Trap 2 — Checkstyle wars: hand-tuning 200 rules
Symptom: a sprint bikeshedding import order, ending in a 200-rule custom config nobody understands — developers start coding around the checker.
Fix: adopt google_checks.xml verbatim; change a rule only when the whole team agrees the default hurts. Decision rule: the tool owns formatting debates so humans never have them — its value is consistency, not perfection.
Trap 3 — SpotBugs suppressions without justification
Symptom: a finding looks wrong, so someone slaps @SuppressFBWarnings on the method — no comment, no ticket. Six months later nobody knows if it was a false positive or a misunderstood real bug.
Fix: every suppression carries a justification comment explaining why the tool is wrong, ideally with a test proving the code safe. Suppress the finding, never the tool.
Trap 4 — SonarQube without an enforced gate is theater
Symptom: beautiful dashboard, green "A" rating — but the build never fails on any of it. The gate sits in "warn" mode while Blocker bugs ship.
Fix: the gate must fail the build (sonar.qualitygate.wait=true, CI goes red), with a coverage condition on new code — a 90%-covered legacy codebase hides 0%-covered new files. A dashboard nobody gates on is an expensive screensaver.
What's next
The fourth gate: SpotBugs for bug patterns javac can't see, Checkstyle for the team contract, SonarQube for the trend — with a gate that fails the build. Habit: fast tools on every build, the full gate on every pull request.
Field check: put the buggy OrderValidator in a scratch Maven project with both plugins, run mvn verify, and watch it go red. Fix findings one by one until green. That red-to-green loop — not the dashboard — is what static analysis is.
Next in the track: JMH — because correct, clean code still needs to be fast, and guessing is not a measurement strategy.
Continue: Java Learning Roadmap 2026
Comments
Post a Comment