Null Checking

Under Construction!

Cleaning Existing Projects

When you copy this settings file to an existing project which has not been “cleaned” it will probably “turn red” with warnings. In general it is quite simple to see what needs to be done. The following tricks have proven useful. Some of them are not suitable for non test code, but as always your judgement is your best guide.

Annotations

Rarely, annotations are incorrect. These are easily fixed. An obvious example is in-lined definitions of Functions which often require an appropriate annotation added.
Sometimes you can remove several errors by tightening the annotation on a method but if the method has a scope outside the class (or its derived subclasses) things can explode out of control because you may be changing the underlying contract.
Fields are sometimes marked @Nonnull but provoke an error because they are not initialized in the constructor. There are two common causes
- In fact the field is @NonNullAfterInit which is a pain (because the tool doesn’t understand that annotation. Sometimes you can clean this up by adding a new @Nonnull getter which checks the component state, asserts that the result is non null and returns it.
- Sometimes the constructor does set the field, but the tool doesn’t know (because the setting is done in a method). Two fixes. Both ugly.
  - Exbed the setter code into the constructor
  - (tests only) Spoof the tool by assigning the field to itself in the setter (and ignoring the warnings)

Fields can change

In most cases our paradigm is basically that a field is set (in the constructor, before initialization or in PreExecute) and stays set. The tool only spots the first case (by way of the @Nonnull annotation) in other cases we know that the field is null but the tool doesn’t. If we know that the field cannot be null it suffices to assert this before it is being used. If the field is used more than once in then I usually create a local copy of the field, assert that that is nonnull and use it throughout the code.

This flip side of this can be used to fix tests which are actually checking against non-null parameters. Say a test does this

@Test(expectedExceptions=ConstraintViolationException.class) public void test() { 
   foo.setThing(null);  // setThing's parameter is @Nonnull
}

You can “fix” this test by declaring (but never setting) a private Object nullObj. You then supply that as the parameter, casted a appropriately:

private Object nullObj;
...
@SuppressWarnings("null")
Test(expectedExceptions=ConstraintViolationException.class) public void test() { 
   foo.setThing((String)nullObj); 
}

this kludge does not make me proud, but it works.

Method returns may be non-null

A common variant of the above is this

if (foo.getBean()!=null) {
    foo.getBean.dosomething();
}

Again the solution is to declare a local variable. Often the nesting is quite deep (foo.getBean().getOther().getThird().doSomething() in which case you need to do more exbedding. Often ternaries can help

Another common example is where the objects is not from out shibboleth and carries no annotation. Instant.now() is the canonical example. This has to be asserted. In some limited cases we have shim methods which do have the correct annotation. The two classes are (currently)

net.shibboleth.shared.collection.CollectionSupport
- Null safe variants of colelctions generating methods (List.of, Collections.singletonList() and so on)
- It also has a collector (used at the end of Stream chains which returns a NonNullFunction. Using this allows the real null issued in Stream chains to be spotted
net.shibboleth.shared.primitive.LoggerFactory
net.shibboleth.shared.logic.PredicateSupport

Parameters marked null may be non null

Sometimes is suffices to assert that the parameters is non null. But sometime there maybe a genuine concern that the parameter might be null - and this is also true of some method returns which are used unchecked. In this case adding an assert will find any problems during testing but not in production. In situations like that I favor using Constraint.isNonNul(...) which converts a nullable input into a nonnull output (or throws an exception). The motivation here is to convert a run time NPE into a runtime Constraint exception which should have enough garnish to help us remove the bug or (and preferably) allow the deployer to fix their installation: an error “p:foo may not be null” provides more information that a bare NPE.