Code Quality Tools in Java

Before a school magazine goes to print, two different people read it. The copy editor checks the house rules: titles in bold, dates written the same way, no paragraph longer than a page. The fact checker looks for real mistakes: a wrong year, a misspelled name, a number that doesn't add up. Neither needs to watch the magazine being read; they find problems on the page. Code quality tools do both jobs for Java code. They read the source or the compiled classes without running the program, and report broken style rules and likely bugs, on every build, without getting tired. In this lesson, you will run Checkstyle and SpotBugs on a class with real problems, read their reports, fix the code, and see where PMD, SonarQube and your IDE fit in. The rules themselves are covered in "Java Coding Conventions".

What Is Static Analysis?

Static analysis is the checking of a program's code without running it, by a tool that is used to find style violations, likely bugs, security weaknesses and needless complexity. "Static" means the code is read, not executed. Tests show that the code does what you checked; static analysis catches whole classes of mistakes nobody wrote a test for.

A Class with Problems

This class compiles without a single warning. It even seems to work. Let's see what the tools think of it.

package school.report;
 
import java.util.*;
 
public class ReportCard {
    public static int MAX = 100;
    private String Name;
    private int[] marks;
 
    public ReportCard(String name, int[] marks) {
        this.Name = name;
        this.marks = marks;
    }
 
    public boolean sameStudent(ReportCard other) {
        return Name == other.Name;
    }
 
    public int[] getMarks() {
        return marks;
    }
 
    public double average() {
        int total = 0;
        for (int i = 0; i < marks.length; i++) total += marks[i];
        return total / marks.length;
    }
}

Checkstyle

Checkstyle checks source code against a set of style rules. Ready-made rule sets exist (google_checks.xml and sun_checks.xml come with it), but most teams start small. Here is a rule file with seven common rules:

<?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">
  <module name="LineLength">
    <property name="max" value="100"/>
  </module>
  <module name="TreeWalker">
    <module name="AvoidStarImport"/>
    <module name="UnusedImports"/>
    <module name="MemberName"/>
    <module name="StaticVariableName"/>
    <module name="VisibilityModifier"/>
    <module name="NeedBraces"/>
    <module name="MagicNumber"/>
  </module>
</module>

And the Maven plugin that runs it, along with SpotBugs, which is used in the next section:

<plugin>
  <groupId>org.apache.maven.plugins</groupId>
  <artifactId>maven-checkstyle-plugin</artifactId>
  <version>3.6.0</version>
  <dependencies>
    <dependency>
      <groupId>com.puppycrawl.tools</groupId>
      <artifactId>checkstyle</artifactId>
      <version>14.4.0</version>
    </dependency>
  </dependencies>
  <configuration>
    <configLocation>checkstyle.xml</configLocation>
    <consoleOutput>true</consoleOutput>
    <violationSeverity>warning</violationSeverity>
  </configuration>
</plugin>
<plugin>
  <groupId>com.github.spotbugs</groupId>
  <artifactId>spotbugs-maven-plugin</artifactId>
  <version>4.10.5.0</version>
</plugin>

mvn checkstyle:check

[INFO] --- checkstyle:3.6.0:check (default-cli) @ report-card ---
[INFO] Starting audit...
[ERROR] ~/report-card/src/main/java/school/report/ReportCard.java:3:17: Using the '.*' form of import should be avoided - java.util.*. [AvoidStarImport]
[ERROR] ~/report-card/src/main/java/school/report/ReportCard.java:6:23: Variable 'MAX' must be private and have accessor methods. [VisibilityModifier]
[ERROR] ~/report-card/src/main/java/school/report/ReportCard.java:6:23: Name 'MAX' must match pattern '^[a-z][a-zA-Z0-9]*$'. [StaticVariableName]
[ERROR] ~/report-card/src/main/java/school/report/ReportCard.java:6:29: '100' is a magic number. [MagicNumber]
[ERROR] ~/report-card/src/main/java/school/report/ReportCard.java:7:20: Name 'Name' must match pattern '^[a-z][a-zA-Z0-9]*$'. [MemberName]
[ERROR] ~/report-card/src/main/java/school/report/ReportCard.java:25:9: 'for' construct must use '{}'s. [NeedBraces]
Audit done.
[INFO] There are 6 errors reported by Checkstyle 14.4.0 with checkstyle.xml ruleset.
[INFO] ------------------------------------------------------------------------
[ERROR] BUILD FAILURE

Explanation:

1. Each line gives the file, line and column, the problem, and the rule name in brackets, so you can look the rule up or tune it.
2. MAX breaks two rules: a non-final static field is a variable, so it should be named in camelCase, and it is public, so any code can change it. The real fix is to make it a constant: public static final int MAX_MARK = 100;.
3. Name should be name; field names start with a lowercase letter.
4. violationSeverity is warning, so the build fails on any violation. That is what keeps a rule from being ignored.

Notice what Checkstyle did not report: comparing strings with == on line 16 and the integer division on line 26. Both are real bugs, but the code is perfectly well formatted. That is the job of the next tool.

SpotBugs

SpotBugs (the successor of FindBugs) analyzes the compiled bytecode for more than 400 known bug patterns. It needs the classes, so compile first:

mvn compile spotbugs:check

[INFO] --- spotbugs:4.10.5.0:check (default-cli) @ report-card ---
[INFO] BugInstance size is 4
[INFO] Error size is 0
[INFO] Total bugs: 4
[ERROR] Medium: school.report.ReportCard.getMarks() may expose internal representation by returning ReportCard.marks [school.report.ReportCard] At ReportCard.java:[line 20] EI_EXPOSE_REP
[ERROR] Medium: new school.report.ReportCard(String, int[]) may expose internal representation by storing an externally mutable object into ReportCard.marks [school.report.ReportCard] At ReportCard.java:[line 12] EI_EXPOSE_REP2
[ERROR] Medium: Comparison of String objects using == or != in school.report.ReportCard.sameStudent(ReportCard) [school.report.ReportCard] At ReportCard.java:[line 16] ES_COMPARING_STRINGS_WITH_EQ
[ERROR] Medium: Integral division result cast to double or float in school.report.ReportCard.average() [school.report.ReportCard] At ReportCard.java:[line 26] ICAST_IDIV_CAST_TO_DOUBLE

Explanation:

1. ES_COMPARING_STRINGS_WITH_EQ: == compares references, not text, so two students with the same name can look different (see "String Comparison in Java").
2. ICAST_IDIV_CAST_TO_DOUBLE: total / marks.length divides two ints, losing the fraction, and only then widens to double. An average of 81.1 comes out as 81.0.
3. EI_EXPOSE_REP and EI_EXPOSE_REP2: the class stores and returns the caller's own array, so code outside can change the marks inside a ReportCard. Copy the array on the way in and on the way out.
4. Each bug has a priority (Medium here) and a pattern code you can search for in the SpotBugs documentation.

The Fixed Class

package school.report;
 
import java.util.Arrays;
 
public class ReportCard {
    public static final int MAX_MARK = 100;
 
    private final String name;
    private final int[] marks;
 
    public ReportCard(String name, int[] marks) {
        this.name = name;
        this.marks = Arrays.copyOf(marks, marks.length);
    }
 
    public boolean sameStudent(ReportCard other) {
        return name.equals(other.name);
    }
 
    public int[] getMarks() {
        return Arrays.copyOf(marks, marks.length);
    }
 
    public double average() {
        int total = 0;
        for (int mark : marks) {
            total += mark;
        }
        return (double) total / marks.length;
    }
}

mvn compile checkstyle:check spotbugs:check

[INFO] --- checkstyle:3.6.0:check (default-cli) @ report-card ---
[INFO] Starting audit...
Audit done.
[INFO] You have 0 Checkstyle violations.
...
[INFO] --- spotbugs:4.10.5.0:check (default-cli) @ report-card ---
[INFO] BugInstance size is 0
[INFO] Error size is 0
[INFO] No errors/warnings found
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS

Both tools are now quiet: the constant is final and named in capitals, fields are private and final, strings are compared with equals, the array is copied both ways, and the division happens in double.

PMD, SonarQube and the IDE

PMD reads source code like Checkstyle, but its rules are about practices rather than layout: unused variables and parameters, empty catch blocks, methods that are too long or too complex, and copy-pasted code (its CPD tool). It runs from Maven (mvn pmd:check) and Gradle in the same way.

SonarQube is a server that runs thousands of rules (style, bugs, security weaknesses, duplication) on every build and keeps a history, so a team can see whether quality is getting better or worse. Its quality gate can block a pull request that adds new issues or lowers test coverage. SonarQube for IDE (called SonarLint until 2024) is the free plugin for IntelliJ IDEA, Eclipse and VS Code that underlines the same issues while you type, so most are fixed before they are ever committed.

Your IDE has its own inspections too. IntelliJ IDEA's yellow highlights flag == on strings and integer division in a floating-point context, exactly like SpotBugs did.

Common Mistakes

Turning on every rule at once. Hundreds of warnings get ignored, including the important ones.

Suppressing instead of fixing. @SuppressWarnings and // CHECKSTYLE:OFF are for rare, explained exceptions, not for silencing a tool.

Treating zero warnings as zero bugs. Static analysis finds known patterns; tests and reviews are still needed.

Running the tools only on one laptop. Put them in the build file so every developer and the CI server use the same rules.

Practice

1. Easy: Add google_checks.xml instead of the custom file (<configLocation>google_checks.xml</configLocation>, built into Checkstyle) and run it on the fixed ReportCard. Which rules does Google's style add?
2. Harder: Add the maven-pmd-plugin to the project and run mvn pmd:check on the original ReportCard. Compare its report with the Checkstyle and SpotBugs reports: which problems do all three find, and which does only one find?

Key Takeaways