Bugs squashed

Bugs squashed

There are a few remaining bugs to squash before we can wrap this up.

Run mvn -Pchecker compile and take a look at the errors around the middle.

Missing null checks

The half a dozen reports of dereference of possibly-null reference in JdbcPetRepositoryImpl and JdbcVisitRepositoryImpl all look very much the same and can be dealt with swiftly.

Before:

    private MapSqlParameterSource createVisitParameterSource(Visit visit) {
        return new MapSqlParameterSource()
            .addValue("id", visit.getId())
            .addValue("visit_date", visit.getDate().toDate())
            .addValue("description", visit.getDescription())
            .addValue("pet_id", visit.getPet().getId());
    }

After:

    private MapSqlParameterSource createVisitParameterSource(Visit visit) {
        DateTime date = visit.getDate();
        Pet pet = visit.getPet();
    
        return new MapSqlParameterSource()
            .addValue("id", visit.getId())
            .addValue("visit_date", date == null ? null : date.toDate())
            .addValue("description", visit.getDescription())
            .addValue("pet_id", pet == null ? null : pet.getId());
    }

Rather lazy how the author of this code chained calls on nullable methods. We fix this broken code by adding the necessary null checks.

This is not an improvement to be too proud of – a better investment of time might have been to perform proper argument validation everywhere. Defocusing for a moment it is not hard to see that the Pet Clinic codebase doesn’t do particularly robust argument validation.

We commit this as the eleventh commit 2d795dd and move on. Five to go.

Unboxing of null

Next on the list is a potential ‘unboxing of nullable’ in EntityUtils.getById on line 47.

[ERROR] src/main/java/org/springframework/samples/petclinic/util/EntityUtils.java:
[47,29] [unboxing.of.nullable] unboxing a possibly-null reference entity.getId()

        if (entity.getId() == entityId && entityClass.isInstance(entity)) {

The first expression, entity.getId() == entityId, compares Integer to int. What’s wrong with that?

Unboxing a boxed primitive can result in a NullPointerException. Don’t worry if you’re not familiar with this Java gotcha. The == comparison here will be done in int, requiring unboxing of the Integer object. In case entity.getId() is null we will attempt to unbox null, earning us a NullPointerException.

This is another error that is ultimately due to poor argument validation. Let’s not worry and simply make the comparison null-safe by boxing the primitive and then using equals instead of ==. I propose this as the replacement expression:

Integer.valueOf(entityId).equals(entity.getId())

Committing this as the twelfth commit fd848ce, but not without making a mental note to write a test. Some day.

Success

I will pass over the small change required in PetController which I committed in commit 77ad340. (Bit tricky this one, can you explain why the fix works?)

todo explain it!

        if (StringUtils.hasLength(pet.getName()) && pet.isNew()
                && owner.getPet(pet.getName(), true) != null) {
            result.rejectValue("name", "duplicate", "already exists");
        }

The Checker Framework reports another plain-as-day possible null dereference in PetTypeFormatter.parse on line 59.

        for (PetType type : findPetTypes) {
            if (type.getName().equals(text)) {    // line 59
                return type;
            }
        }

The JDK offers Objects.equals for null-safe reference comparison, and it is just a perfect fit here.

-            if (type.getName().equals(text)) {
+            if (Objects.equals(type.getName(), text)) {

Committed as the fourteenth commit 07d0b64. Another NullPointerException averted.

It’s two left and we’re done.

[ERROR] src/main/java/org/springframework/samples/petclinic/repository/jdbc/JdbcPetVisitExtractor.java:
[44,20] [return.type.incompatible] incompatible types in return.
  found   : null
  required: @Initialized @NonNull Integer
[ERROR] src/main/java/org/springframework/samples/petclinic/web/PetTypeFormatter.java:
[53,31] [return.type.incompatible] incompatible types in return.
  found   : @Initialized @Nullable String
  required: @Initialized @NonNull String
[INFO] 2 errors

The statement on line 44 of JdbcPetVisitExtractor reads return null;. states an return type incompatibility rather precisely. The method signature reads, with implied @NonNull added

protected @NonNull Integer mapForeignKey(@NonNull ResultSet rs)

A method that may return null is @Nullable and that’s what we need to make explicit by annotating it @Nullable. This is the fifteenth commit 1c3c1d6.

Next, PetTypeFormatter line 53.

The sixteenth commit 963645b.

todo

And with that, finally, mvn -Pchecker compile succeeds. After 16 commits we have reached ‘BUILD SUCCESS’.

Are we now bug-free? Some closing thoughts on the next page.

An exercise for the reader. mvn -Pchecker install still does not succeed, because we haven’t checked the test source yet. (mvn compile only compiles the production source.) Try resolving all errors in the tests too.

todo fix these commit hashes

My solution is in the final three commits, commit 6f0ede7, commit ca07903, and commit 9586c6c.