Bug free?

Bug-free?

The job is done, we are bug-free – or are we? Before we discuss this question let us look at our achievements in numbers.

Stats

Here are some key figures about this project.

• 1600 sloc (3200 loc) analysed
• 187 lines of churn in 19 commits
• 46 @Nullable annotations added
• 10 bugs fixed, that is possible NullPointerExceptions averted
• 6 assertions added
• 6 @SuppressWarnings annotations added

Of the 46 @Nullable annotations we introduced, only one was outside the mutable entity classes. We talked about how state in the entity classes is nullable (and mutable) through and through, flying in the face of the modern aim and the checker’s preference for non-nullness (and immutability) everywhere.

Disregarding the entity classes, I think we can be pretty pleased with the outcome: we did prevent ten actual NullPointerExceptions, and paid for it with only a small handful of code changes.

What we have achieved …

In this tutorial, we ran the Nullness Checker on the Pet Clinic codebase and managed to resolve all the errors that were reported. What have we achieved by doing this?

The promise of the Nullness Checker was that when it reports no more errors, no NullPointerExceptions can occur at run-time. I think we can say that all the code changes we made we did make with confidence. In each instance the justification for suppressing an error seemed to be sound. On the face of it, we really did make the Pet Clinic code null-safe. Any clients relying on our annotated and checked public API get a guarantee that as long as the Null Checker reports no errors in their code, their code will be null-safe, too.

However, we are not quite yet in a position to answer the titular question with ‘Yes’.

… and what we haven’t

In section four, we used the -AskipUses flag to silence errors in foreign library code. -AskipUses is a heavy tool. At the risk of stating the obvious, suppressing errors without review invalidates the checker’s promise. Therefore, NullPointerExceptions are still possible. Hopefully they will surface only at the boundaries between our annotated system and the mistaken library use, but basically, all bets are off.

This is a disappointment, but there are remedies. Forking the library and annotating it is an option, but not a realistic one. Annotating just those library APIs that we actually use is a better option, and is possible by using stub classes.

As a concrete example, one of the errors that -AskipUses hid is in this fragment from JdbcPetRowMapper.mapRow:

        Date birthDate = rs.getDate("birth_date");
        pet.setBirthDate(new LocalDate(birthDate));   // line 38

[ERROR] src/main/java/org/springframework/samples/petclinic/repository/jdbc/JdbcPetRowMapper.java:
[38,40] [argument.type.incompatible] incompatible types in argument.
  found   : @Initialized @Nullable Date
  required: @Initialized @NonNull Object

LocalDate is a class from the Joda-Time library. LocalDate is not annotated, so the checker assumes its constructor does not accept a null argument. The birthDate that we got out of the result set on the line before, however, will be null if the database record has a null birthdate for some reason. That is, the birthdate Date is @Nullable – hence the error.

Now, according to its Javadoc, the LocalDate constructor handles null arguments just fine. A legitimate resolution would be to introduce a stub class LocalDate.astub and declare this constructor argument to be nullable:

package org.joda.time;

class LocalDate {
    LocalDate(@Nullable Object instant);
}

The checker error goes away, and we’re safe again.

Unfortunately, the code is still broken in a more subtle way, and therein lies a second lesson.

new LocalDate(null) creates a date representing the current date. So, a NULL date in the database will be mapped to the current date in objectland, and for all we know this cannot have been the intent of this code.

Again at the risk of stating the obvious, it is crucial to examine all issues reported by the Checker Framework along with their meaning in context. While we did fix all issues reported by the Nullness Checker, we worked with a narrow focus on null-safety and some of our fixes may have been too shallow. The Pet Clinic codebase would still profit from more thorough refactoring done under a holistic approach.

With that we conclude. There’s always more work to do, but overall we were successful and I’m happy to call it a day.

Here the tutorial ends. I hope you enjoyed it!

Some additional links are in the References section.