Times Have Changed, But An Instant Stays The Same: Checking for Mutability in JSR 310

I recently attended a London Java Community meetup to try out the new date and time API scheduled for Java 8[1]. JSR 310 (or "ThreeTen" as named on GitHub) provides an upgrade to the standard date and time libraries available in the JDK. At first use, ThreeTen looks to have addressed the mistakes of old JDK Date API, and draws influence from the popular, defacto standard date/time library, JodaTime[2].

Overall I was very impressed with the design and discoverability of the API. And of course, I'm a big fan of it's design choice to use immutability wherever possible.

Since JodaTime is somewhat of a poster-child for immutability in Java, and it's unsurprising the same design principle has transferred to ThreeTen. I was curious to see how effective Mutability Detector could be in helping to enforce this principle.

Using the highly rigourous and scientific method of grep'ing the codebase for the term " immutable ", as included in the JavaDoc for such types, I found 55 immutable classes in ThreeTen. For each of those 55, I added a unit test case to assert the class is immutable. Out of those 55, 6 tests are failing. Among the passing tests are some of ThreeTen's core classes: LocalDate, LocalDateTime, Duration, Period, and more. I'm really impressed by how 'cleanly' these immutable implementations are. I'm also happy with the apparent success rate of Mutability Detector on this real-world codebase (though I know that the results assume zero false negatives).

There's two ways to look at 6 failing tests from 55. Firstly, assuming that when a ThreeTen author documents a class as immutable, it is, Mutability Detector produces 6 false positives. Secondly, assuming that Mutability Detector correctly analyzes, ThreeTen has unexpected mutability in 6 classes. Since these conditions are mutually exclusive, over the next few blog entries, I'm going to take a look at the classes in question. First up is DateTimeFormatter.

The assertion:

  assertImmutable(javax.time.format.DateTimeFormatter.class);

Fails with:

  org.mutabilitydetector.unittesting.MutabilityAssertionError: 
  Expected: javax.time.format.DateTimeFormatter to be IMMUTABLE
      but: javax.time.format.DateTimeFormatter is actually NOT_IMMUTABLE
      Reasons:
 Field can have a mutable type (java.util.Locale) assigned to it. [Field: locale, Class: javax.time.format.DateTimeFormatter]
 Field can have a mutable type (javax.time.format.DateTimeFormatterBuilder$CompositePrinterParser) assigned to it. [Field: printerParser, Class: javax.time.format.DateTimeFormatter]
      Allowed reasons:
 None.

DateTimeFormatter is considered mutable because it has two fields of mutable types. The types in question being java.util.Locale, and a ThreeTen internal type, CompositePrinterParser.  Since the result for Locale (for the JDK 1.6u25 incarnation at least) is a false positive relating to The String Problem, which I can suppress in the unit test, I'll take a more detailed look at CompositePrinterParser.

The problem that Mutability Detector has with this class is that it contains a field which is an array type. There are several ways of (ab)using an array field that breaks immutability. The class can: (A) modify the contents of the array field after construction; (B) let a mutable element of the array escape to where it can be modified; (C) let the array escape so that callers can modify the contents; (D) take the array passed to the constructor without performing a defensive copy. Note that Mutability Detector isn't yet capable of this kind of analysis, so it just panics at the sight of an array field, and leaves it up to us, the developer, to investigate.

Out of those potential problems described:

(A) doesn't happen. The class only iterates over the array, it doesn't modify the contents.
(B) isn't applicable here. The element type of the array is an interface which is contracted to be immutable.
(C) and (D) are the interesting parts. There is no getter for the array field, and it's private. The only way the array escapes, is that it is passed as the constructor argument to another instance of the same class. Since we know that same class doesn't modify the array, the destination for the escaping array is a "safe haven". That particular constructor is also only ever called by the same class, so it doesn't need to copy the passed in array. There is a second constructor, but that takes a List, and performs a defensive copy of it, so the class really is in control of that array field.

VERDICT: False Positive

WHAT TO DO?:
For both Locale, and CompositePrinterParser, we can add suppressions, or "allowed reasons" for mutability. Thus the test case can look like this:

assertInstancesOf(javax.time.format.DateTimeFormatter.class, 
                  areImmutable(), 
                  provided(Locale.class).isAlsoImmutable(),
                  provided("javax.time.format.DateTimeFormatterBuilder$CompositePrinterParser").isAlsoImmutable());
               
This will allow the test to pass, ignoring the false positives, but failing again if someone introduces mutability another way.
               
Score so far, ThreeTen 1, Mutability Detector 0. Next up, ResourceZoneRulesDataProvider. (Click here for the next blog entry).

[1] Thanks to OpenGamma, Stephen Colebourne, Richard Warburton, James Gough et al for organising.
[2] This is unsurprising, since one of the technical spec leads of JSR 310, Stephen Colebourne is the creator of JodaTime.