False positives

False positives

We have silenced the errors to do with use of unannotated libraries.

However, some of the errors reported in our own code may also be false positives that we want to dismiss. For this we have mostly two means: suppressing the errors, and assertions.

Suppressing errors

Scanning over the remaining errors you’ll notice a few errors saying [initialization.fields.uninitialized] the constructor does not initialize fields. For example, JpaPetRepositoryImpl appears not to initialise the em field.

@Repository
public class JpaPetRepositoryImpl implements PetRepository {

    @PersistenceContext
    private EntityManager em;

    @Override
    public Pet findById(int id) {
        return this.em.find(Pet.class, id);
    }

    // ...

}

Indeed, em is not initialised, but it must also be non-null or else the call to em.find could never succeed.

Well, we know that as long as the application is configured correctly, the EntityManager instance is going to be injected as part of the application wiring. Unfortunately, the Checker Framework doesn’t know this and there is no clean way of expressing this.

In this case, suppression of warning is the only option. We can use the JDK @SuppressWarnings annotation and supply the first part of the error message key as the value.

    @PersistenceContext
    @SuppressWarnings("initialization")
    private EntityManager em;

I’ve made this change on all four instances of the error in commit b5443b5. With that we are down to 13 errors.

Using assertions

Suppressing warnings is really the only option in situations like the above, where the programmer knows something about the application that the Checker cannot. However, it’s good to explore other means to steer the Checker towards the proper reasoning before applying the @SuppressWarnings chloroform.

The assert statement is a useful but underused feature of Java. Combined with the flag -AassumeAssertionsAreEnabled it becomes a useful tool to express ...

I’m going to pick three interesting instances where assert ... the errors in the Controller classes and the errors in the corresponding JDBC repositories.

Open JdbcPetRepositoryImpl on line 100.

        try {
            Map<String, Object> params = new HashMap<>();
            params.put("id", id);
            pet = this.namedParameterJdbcTemplate.queryForObject(
                "SELECT id, name, birth_date, type_id, owner_id FROM pets WHERE id=:id",
                params,
                new JdbcPetRowMapper());
        } catch (EmptyResultDataAccessException ex) {
            throw new ObjectRetrievalFailureException(Pet.class, id);
        }

        // ...

        List<Visit> visits = this.visitRepository.findByPetId(pet.getId());   // line 100

The Checker complains that pet.getId() is nullable, but findByPetId does not accept a null argument. Inspecting the code we see that pet at this point will always be a Pet instance populated from the database, so it must have an ID – the ID cannot be null. We can express this knowledge about the code with an assert statement:

        Integer petId = pet.getId();
        assert petId != null;
        List<Visit> visits = this.visitRepository.findByPetId(petId);

The error in JdbcOwnerRepositoryImpl on line 112 is quite similar.

    public void loadPetsAndVisits(final Owner owner) {
        Map<String, Object> params = new HashMap<>();
        params.put("id", owner.getId());                  // line 112
        // ...
    }

By default, values in Map must not be null, but owner.getId is annotated to be @Nullable. loadPetsAndVisits is a method used only locally (make it private), and all paths to the method obtain the owner from the database. Again we are sure, that given our knowledge of this code ...

    private void loadPetsAndVisits(final Owner owner) {
        Integer ownerId = owner.getId();
        assert ownerId != null;
        Map<String, Object> params = new HashMap<>();
        params.put("id", ownerId);
        // ...
    }

The final error in this batch is in OwnerController on line 90. Blindingly obvious for us but the Checker needs a little help.

        // ...
        if (owner.getLastName() == null) {
            owner.setLastName("");
        }

        Collection<Owner> results = this.clinicService.findOwnerByLastName(owner.getLastName());
        // ...

As before, owner.getLastname carries the @Nullable annotation, but findOwnerByLastName does not accept a null argument. It is true that we’ve just ruled out this case in the guard block above, but the checker does not understand the special relationship between the getters and setters. Once we’re past the owner.getLastName() == null guard an assertion that the last name is now not null is necessary.

I have committed all this in the tenth commit da8d9aa.

All that remains now are a couple of real bugs, ten to be precise.

Give it a try squashing them yourself or follow me to the discussion on the next page, before we wrap up.