FindBugs: May expose internal representation by returning reference to mutable object

Wer seine Code-Qualität mit FindBugs überwacht, stößt früher oder später auf folgende Verletzung: “Method-X may expose internal representation by storing/returning an externally mutable object into Field-Y”. In den meisten Fällen handelt es sich dabei um das Abfragen und Speichern von java.util.Date Werten.

Erfahrene Java-Entwickler wissen, dass Date-Objekte nachträglich veränderbar (mutable) sind:

Date now = new Date();
now.setTime(1414141414141L);
System.out.println(now);

Dies könnte für Manipulationen ausgenutzt werden:

public class MyBean {
  private Date createdAt = new Date();
  public Date getCreatedAt() { return createdAt; }
  // no setCreatedAt()
}

Der Aufruf von myBean.getCreatedAt().setYear(100) verändert den internen Zustand der MyBean-Instanz, obwohl der Entwickler dies durch den fehlenden Setter gar nicht zulassen wollte.

Wie soll man mit diesem Problem umgehen?

Zur Lösung bieten sich mehrere Möglichkeiten an:

  1. Man verzichtet auf Date-Felder und speichert den Zeitstempel als Long-Wert, da Longs unveränderlich (immutable) sind.
  2. Man erzeugt in den Gettern/Settern Kopien der Date-Werte und gibt diese nach außen/speichert diese.
  3. Man gelobt, niemals Mutator-Methoden auf Date-Werten aufzurufen und schaltet die FindBugs-Regeln EI_EXPOSE_REP und EI_EXPOSE_REP2 stumm.

Jede dieser Varianten erzeugt gewisse Herausforderungen. Variante 1 ist oft zu kurz gedacht, da man früher oder später doch ein Date-Objekt braucht und dann konvertieren muss. Bei Variante 2 wird gerne vergessen, dass Date-Instanzen auch null sein können und ein einfaches “(Date) value.clone()” nicht ausreicht. Variante 3 lässt sich nicht überprüfen – oder doch?

jQAssistant meldet Mutator-Methoden

Das Werkzeug jQAssistant macht die innere Struktur des eigenen Projekts zugänglich. Auf dieser Struktur können Constraints definiert und abgeprüft werden. Bevor wir hier zur Tat schreiten, soll zum Einstieg die Spielwiese unter http://jqassistant.org/demo/java8/ dienen. Dort lassen sich die Strukturen des JDK 8 abfragen. Die Daten sind in einer Neo4j-Graphen-Datenbank gespeichert, als Abfragesprache dient Cypher.

Um Abfragen stellen zu können, muss man neben der Abfragesprache auch das Datenmodell kennen. Dazu ein Klick im Datenbank-Browser links oben auf das Icon mit den 3 Bällen und es offenbaren sich die vorhandenen Labels und Relationship-Typen. Für unsere Belange benötigen wir die Labels

  • Class
  • Method

und die Relationship-Typen

  • DECLARES
  • INVOKES

Schritt-für-Schritt zur fertigen Abfrage

Da die fertige Abfrage eine gewisse Komplexität nicht verleugnen kann, möchte ich diese Schritt für Schritt herleiten. Fangen wir mit der Date-Klasse an:

match 
  (dateClass:Class) 
where 
  dateClass.fqn = "java.util.Date" 
return 
  dateClass

Es erscheint ein blauer Kreis mit einer Zahl in der Mitte. Ein Klick auf den Kreis bringt ein Properties-Panel hervor. Dort kann man die Attribute des Knotens einsehen und auch seine Darstellung ändern. Anstelle der Node-ID lassen wir uns den Full-Qualified-Name “fqn” anzeigen.

Als nächsten Schritt möchte ich die Methoden in die Abfrage einbeziehen:

match 
  (dateClass:Class)-[:DECLARES]->(method:Method)
where 
  dateClass.fqn = "java.util.Date" 
return 
  dateClass, method

Das Ergebnis wird durch eine Wolke aus vielen Kreisen dargestellt. Auch hier lassen wir uns statt der Node-ID die Namen der Methoden anzeigen.

Der nächste Schritt soll die Subklassen von java.util.Date mit einbeziehen. Dazu muss die Abfrage nur geringfügig geändert werden:

match 
  (dateClass:Class)-[:DECLARES]->(method:Method)
where 
  dateClass.fqn in ["java.util.Date", "java.sql.Date", 
                    "java.sql.Time", "java.sql.Timestamp"]
return 
  dateClass, method

Der geneigte Leser möge nun einwenden, dass sich die Vererbungsbeziehung auch aus dem Modell ableiten lässt. Das ist korrekt, jedoch steht genau diese Information später – wenn die Abfrage auf das eigene Projekt und nicht auf das JDK angewendet wird – nicht zur Verfügung. Leider!

Die Wolke aus Kreisen ist mittlerweile schon sehr ansehnlich. Es wird Zeit, auf die Mutator-Methoden einzuschränken.

match 
  (dateClass:Class)-[:DECLARES]->(mutatorMethod:Method)
where 
  dateClass.fqn in ["java.util.Date", "java.sql.Date",
                    "java.sql.Time", "java.sql.Timestamp"]
  and mutatorMethod.name =~ "set.*"
return 
  dateClass, mutatorMethod

Das Ergebnis sollte ungefähr so aussehen:

Mutator-Methoden aus java.util.Date und Subklassen

Mit diesem Grundstock ist der Schritt zur fertigen Abfrage nur noch klein: Gesucht werden alle Methoden (someMethod) in irgendwelchen Klassen (someClass), die eine Mutator-Methode aufrufen (call).

match
  (dateClass:Class)-[:DECLARES]->(mutatorMethod:Method),
  (someClass:Class)-[:DECLARES]->(someMethod:Method),
  (someMethod)-[call:INVOKES]->(mutatorMethod)
where
  dateClass.fqn in ["java.util.Date", "java.sql.Date",
                    "java.sql.Time", "java.sql.Timestamp"]
  and mutatorMethod.name =~ "set.*"
return someClass.fqn, someMethod.signature, call.lineNumber

Et voilà! Diese Abfrage kann nun in das jQAssistant-Regelwerk aufgenommen werden. Damit bricht der Build, sobald eine Date-Mutator-Methode verwendet wird. Die FindBugs-Regel kann abgeschaltet werden – es sei denn man benutzt in Datenklassen noch andere, unerwartet mutierbare Objekte (Arrays!). Aber das ist ein anderes Problem.

Kommentare sind abgeschaltet.