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.

Dependency injection

@SuppressWarnings("initialization") on the class I have suppressed the four similar instances in the repository classes in commit HASH LINK. 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 all other means of gently guiding the Checker to the proper reasoning before applying the @SuppressWarnings chloroform.

assert is an underused feature of the Java programming language.

talk about assumeAssertionsEnabled

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 LINK.

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

The Checker complains that pet.getId() is nullable, but findByPetId does not accept a null argument. 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 in an assert statement:

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

The error in JdbcOwnerRepositoryImpl is quite similar.

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

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 now), 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. 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 commit HASH LINK.

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