False positives

False positives

We have silenced the errors to do with (unannotated) library usage.

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 [initialization.fields.uninitialized] the constructor does not initialize fields errors. 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 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.

talk about dependency injection @SuppressWarnings("initialization.fields.uninitialized") 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);
    // ...
}

another one

I have committed all this in commit HASH LINK.

All that remains now are a couple of real bugs [how many?]. Flip.

NO! bad example, interaction with skipUses

The following bit of code is from the PetController LINK.

@RequestMapping(value = "/pets/new", method = RequestMethod.POST)
public String processCreationForm(Owner owner, @Valid Pet pet, BindingResult result, ModelMap model) {
    if (StringUtils.hasLength(pet.getName()) && pet.isNew() && owner.getPet(pet.getName(), true) != null){
        result.rejectValue("name", "duplicate", "already exists");
    }
    // ...
}

Here, the error is towards the end of the long line, where Owner.getPet(String, boolean) does not accept a nullable first argument. But remember that we declared Pet.getName to be @Nullable. This line of code is correct, however: StringUtils.hasLength(pet.getName()) guards against a null name.

In this case we can split the conditional expressions and assert that the name is not null: