2.8. Statische Codeanalyse
Wir haben jetzt schon einigen Code geschrieben, aber haben wir in dem Code soweit alles richtig gemacht? Gibt es eventuell Dinge, die wir gemacht haben, die unüblich sind? Und vor allem: Auf was sollte man denn eigentlich alles achten? Wir haben uns da bisher noch nicht wirklich mit auseinandergesetzt.
Bei jedem Projekt ist es von Anfang an wichtig, auf die Qualität zu achten. Das Thema Clean Code ist schon etwas angesprochen worden - speziell in Bezug auf doppelten Code (DRY - Don’t Repeat Yourself) und auch in Bezug auf JavaDoc-Kommentare.
Dies wollen wir nun etwas vertiefen. Jetzt kommt aber keine lange Auflistung an Regeln, die wir einhalten wollen, sondern wir greifen jetzt erst einmal auf das Wissen vieler Entwickler zurück, die Ihr Wissen in ein Tool gepackt haben.
Das Tool PMD ist ein Werkzeug für die statische Codeanalyse. Es hat einen Katalog an Regeln und prüft den Sourcecode auf die Einhaltung dieser Regeln. Wir bauen das Tool jetzt einfach einmal bei uns in das Projekt ein, um eine erste Auswertung zu erhalten. Dabei werde ich eine erste Konfiguration vorgeben, bei der ich ein paar Regeln deaktiviert habe. Die deaktivierten Regeln sind im Angang kurz beschrieben, inklusive einer kurzen Begründung.
Damit PMD beim Maven-Build mit gestartet wird, müssen wir das PMD-Plugin in unserem Projekt hinzufügen und konfigurieren. Da das PMD-Plugin und die PMD-Software unabhängig voneinander entwickelt werden, geben wir im Plugin auch an, welche Version von PMD geladen werden soll. Dies sorgt für einen sehr umfangreichen plugin-Eintrag, aber bitte davon nicht verschrecken lassen: Zum einen ist es nicht so komplex und zum anderen ist es auch ok, wenn man dies im Detail nicht ganz versteht. Bei Maven ist ein Kopieren von Einstellungen durchaus normal und gängige Praxis.
In der pom.xml haben wir ja schon für Ordnung gesorgt und Versionen in die Properties gezogen. Dort fügen wir dann noch die neuen Properties hinzu:
<!-- Dependency versions -->
<pmd.version>6.54.0</pmd.version>
<!-- Plugin dependencies -->
<maven.pmd.plugin>3.16.0</maven.pmd.plugin>
(Ich habe nur die neuen Einträge sowie die Abschnitte, in denen ich diese bei mir in der Regel habe, gepostet)
Im Anschluss können wir das Plugin hinzufügen. Bisher haben wir noch keinerlei Plugins, so dass wir die Grundstruktur erst erstellen müssen: Direkt unterhalb der dependencies können wir ein build-Element einfügen. In dieses kommt ein plugins-Element. Dort können dann alle Plugons in einem eigenen plugin-Element konfiguriert werden. Der Eintrag für das PMD-Plugin sieht wie folgt aus:
</dependencies>
<build>
<plugins>
<!-- PMD static code analysis -->
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-pmd-plugin</artifactId>
<version>${maven.pmd.plugin}</version>
<dependencies>
<dependency>
<groupId>net.sourceforge.pmd</groupId>
<artifactId>pmd-core</artifactId>
<version>${pmd.version}</version>
</dependency>
<dependency>
<groupId>net.sourceforge.pmd</groupId>
<artifactId>pmd-java</artifactId>
<version>${pmd.version}</version>
</dependency>
<dependency>
<groupId>net.sourceforge.pmd</groupId>
<artifactId>pmd-javascript</artifactId>
<version>${pmd.version}</version>
</dependency>
<dependency>
<groupId>net.sourceforge.pmd</groupId>
<artifactId>pmd-jsp</artifactId>
<version>${pmd.version}</version>
</dependency>
</dependencies>
<configuration>
<sourceEncoding>${project.build.sourceEncoding}</sourceEncoding>
<minimumTokens>100</minimumTokens>
<targetJdk>${java.version}</targetJdk>
<rulesets>
<ruleset>pmd-ruleset.xml</ruleset>
</rulesets>
<linkXRef>false</linkXRef>
</configuration>
<executions>
<execution>
<phase>prepare-package</phase>
<goals>
<!-- pmd does not stop build when violations are found -->
<goal>pmd</goal>
<!-- check stops the build when violations are found -->
<!-- <goal>check</goal> -->
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>
</project>
(Ich habe zum besseren Verständnis das schließende Element von dependencies und das schließende Element des alles umschließenden project-Element mit eingefügt.)
Zuletzt brauchen wir noch bei der pom.xml die in der Konfiguration angegebene pmd-ruleset.xml Datei:
<?xml version="1.0" encoding="UTF-8"?>
<ruleset name="Custom Java Ruleset"
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 http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
<description>
Custom ruleset for Java
</description>
<rule ref="category/java/codestyle.xml">
<!-- That is not my code style! -->
<exclude name="AtLeastOneConstructor" />
<!-- long variables are acceptable -->
<exclude name="LongVariable"/>
<!-- Multiple return statements are ok -->
<exclude name="OnlyOneReturn"/>
</rule>
<rule ref="category/java/design.xml">
<!-- Problems with builder and streams/lambdas -->
<exclude name="LawOfDemeter" />
</rule>
<rule ref="category/java/documentation.xml">
<!-- Long comments are not a problem! -->
<exclude name="CommentSize"/>
</rule>
<rule ref="category/java/errorprone.xml" />
<rule ref="category/java/multithreading.xml" />
<rule ref="category/java/performance.xml" />
<rule ref="category/java/security.xml" />
<rule ref="category/java/design.xml/LoosePackageCoupling">
<properties>
<property name="packages">
<value>org.jadv</value>
</property>
</properties>
</rule>
</ruleset>
Damit das PMD einmal durchlaufen kann, lassen wir nun das Maven-Ziel package laufen.
Nach dem Lauf finden wir im target-Verzeichnis eine Datei pmd.xml, welche Hinweise zu Problemen in unserem Code aufwirft. Diese XML-Datei können wir nun einmal anschauen und uns die einzelnen Auffälligkeiten ansehen (Hier greife ich jede Meldung nur einmal auf):
- Enum comments are required -> Wir haben bei unserer Enumeration ApplicationCommands nicht vor jedem Eintrag einen JavaDoc. Den müssen wir noch hinzufügen.
- Perhaps ‘imageService’ could be replaced by a local variable. -> da wir imageService nicht anderweitig benötigen, können wir die Instanzvariable durch eine lokale Variable ersetzen.
- Parameter ‘imageService’ is not assigned and could be declared final -> Diese Meldung sollten wir für alle unsere Parameter bekommen. Parameter sind Eingangswerte. Die zu überschreiben ist ein schlechter Stil. Daher sollte man generell alle Parameter bei Methoden und Konstruktoren final machen!
- Local variable ’level’ could be declared final -> Bei lokalen Variablen ist es auch so, dass man diese - so man nicht plant, diese später noch einmal zu verändern - final machen kann und auch sollte.
- This statement should have braces -> Wenn z.B. nach einem if nur eine Anweisung kommt, dann erlaubt Java, dass man keine geschweiften Klammern nutzt. Dies hat in der Vergangenheit in Projekten aber auch schon durchaus zu Problemen geführt, wenn dann Code verändert wurde. Daher besagt eine Regel, dass immer mit geschweiften Klammern gearbeitet werden soll. Überall, wo wir also auf diese verzichtet haben, müssen wir diese nun noch ergänzen.
- Avoid variables with short names like dx -> Eine sehr wichtige Regel! Wir müssen vernünftige Bezeichner wählen! Es gibt aber teilweise kurze Bezeichner, die üblich sind und im fachlichem Umfeld immer verwendet werden. Beispiele sind hier id, x, y aber auch dx und dy (welches aus der Mathematik kommt und eine Ausdehnung in x bzw. y Richtung bezeichnet). Solche Variablen sind auch im Programmcode ok und wir können diesen Fehler erst einmal ignorieren. Am Anfang von Lektion 3 zeige ich, wie wir die konkrete Meldung entfernen können. Aber immer die Meldungen ansehen - evtl. finden sich ja kurze Parameter, die so nicht korrekt sind wie z.B. “ds”?
- Protected method constructor comments are required -> Fehlende JavaDoc Elemente
- Avoid using Literals in Conditional Statements -> Wir sollten “magic numbers” vermeiden. Statt dessen sollten wir immer Konstanten erstellen und diese nutzen. In ApplicationContext können wir z.B. die Konstante SCALING_MINIMUM mit Wert 25 erstellen und dann in der Methode scale nutzen.
- Number 10000 should separate every third digit with an underscore -> Bei großen Zahlen ist es schwer, die Ziffern zu zählen. Ist ein Wert 1 Millionen oder 10 Millionen? Daher ist es üblich, bei großen Zahlen alle 3 Ziffern durch ein _ kenntlich zu machen. Eine 10_000 wäre besser lesbar als 10000.
- Avoid using redundant field initializer for ‘x’ -> Eine Instanzvariable wird immer automatisch initialisiert. Eine Initialisierung einer int Instanzvariable mit 0 ist somit überflüssig und kann entfernt werden. …
Ich breche die Auflistung an dieser Stelle ab. Aus meiner Sicht ist eine statische Codeanalyse, so wie sie PMD macht, sehr wichtig in der Software-Entwicklung. Das hilft, viele Probleme zu beseitigen und sorgt für einen besser lesbaren Code. Aber es ist wichtig, dass die Regeln verstanden werden. Daher ist es wichtig, dass wir auch Erklärungen selbst finden können.
Der erste Anhaltspunkt ist direkt in dem XML-File zu finden. So finden wir dort eine externalInfoUrl - ein Link zu einer Webseite mit mehr Informationen zu der Regel. Wenn eine Regel zu unverständlich ist: Über eine Suchmaschine lassen sich dann meist auch sehr viele Treffer finden - gerade die komplexeren Regeln werden in vielen Fragen und Antworten auf StackOverflow behandelt. Bei der Suche auch nicht nur die Fehlermeldung angeben, sondern auch den Namen der Regel, der in rule="…" zu finden ist.