FindBugs: make your code better with Nullability Annotations

Handling null values is not that easy. For example, there are scenarios in which null values will never happen: this makes code checking whether a value is null not only unnecessary, but misleading -because it creates the wrong expectations.

However, in other cases, a null value might happen, and we should test for this before using it -unless we are happy when with null related exceptions are raised.

Sadly, the fact is the Java compiler will not provide help with this. But all is not lost: FindBugs provides several annotations that might help, especially because it can perform some code analysis that will detect potentially dangerous or unnecesary code.

Null checks

There are three scenarios when it comes to non-primitive values: you assume that values can’t be null, you assume that they can…or you assume nothing. FindBug provides us with three corresponding annotations: @NonNull, @CheckForNull and @UnknownNullness.

A quick example

Let’s take an in-depth look at this class:

public static class NullHandling {
  public String value;
  public int getValueLength() {
    return this.value.length();
  } 
  public String myToUppercase( String parameter )
  {
    if( parameter == null )
      return "";
    return parameter.toUpperCase(Locale.getDefault());
  }

  public String getValue() {
    return this.value;
  } 
  public void justUseGetValue() {
    int length;
    if( getValue() == null )
      length = 0;
    else
      length = getValue().length();
    System.out.println( "Value length: " + length);
  } 
  public String justUseMyToUppercase() {
    return myToUppercase(null);
  }
}

Let me pinpoint some potential problems in this code:

  • If the value field can be null, then this.value.length() in getValueLength will bomb.
  • If getValue can’t return a null value, then code using it (as justUseGetValue does) need not check for null values and can be simpler.
  • If parameter can’t be null in myToUppercase, checking for it is unnecesary -and misleading.
  • If myToUppercase can’t receive a null parameter, then calling it the way justUseMyToUppercase does is a bug.

Well, we are not short of potential problems or unnecesary code, but the compiler remains completely mute. And this is only logical, because there is no way to tell the compiler about the nullability restrictions for fields, parameters or method return values.

Now, let’s use FindBugs annotations to provide clarity and helpful diagnostics, if only for illustative purposes.

As a first change, let’s pretend that the value field might be null. To that end, we need to annotate it with @CheckForNull as follows:

@CheckForNull
public String value;

This causes a warning to be issued by FindBugs for the following code:

public int getValueLength() {
  return this.value.length();
}

Effectively, this.value migth be null, and therefore calling this.value.length() in getValueLength without checking whether it is null is not something I would recommend.

Here is a better implementation of getValueLength, which makes FindBugs happier:

public int getValueLength() {
  if( this.value == null )
    return 0;
  return this.value.length();
}

Now, let’s pretend that myToUppercase can’t be called with a null value, as declared in the following code:

public String myToUppercase(
@NonNull tString parameter )

This causes FindBugs to report two issues. The first one in myToUppercase, because we are checking whether parameter is null, when that should never happen.

To solve this, we just remove the null check, leaving the method body as follows:

return parameter.toUpperCase(Locale.getDefault());

The second issue is reported in justUseMyToUppercase, which was calling myToUppercase passing a null value, something illegal: the fix is to pass it a valid value.

We have seen examples of nullability annotations for fields and method parameters: let’s see such an annotation for a method, so that we have an example of each possible usage of these annotations. To that end, we will pretend that getValue just can’t return a null value, as follows:

@NonNull
public String getValue() {
  return this.value;
}

That makes FindBugs complain, because this.value might be null, and in that scenario getValue will be breaking the guarantee. We can fix getValue by checking for this.value being null as follows:

@NonNull
public String getValue() {
  if( this.value == null )
    return "";
  return this.value;
}

FindBugs will complain about justUseGetValue (which I made rather convoluted on purpose!), because it is checking for getValue returning null when it just can’t do that. The fix is as easy as rewriting justUseGetValue as follows:

public void justUseGetValue() {
  int length = getValue().length();
  System.out.println( "Value length: " + length);
}

I think that these examples illustrate quite well the usefulness of explicitly declaring whether a field, method parameter or returned value can be nullable or not, and how things can go wrong in subtle ways when you choose to ignore the issue (the default case for Java code).

By the way, I want to mention the fact that I have made all code self contained in a class to make code shorter and easier to understand: however, FindBugs will make checks when external code uses fields, parameters or methods with nullability annotations. This is very interesting, because users consuming our code will probably know very little about our assumptions: annotations and the ability to check them will be even more valuable in that scenario.

What to annotate: method parameters, fields and method results

Our examples have shown all places where these annotations can be placed: in fields, method parameters and method results. The only thing worth mentioning is the fact that, for fields, @NonNull just means the fields will never be null after construction, whereas @CheckForNull means they can be null after construction.

And, of course, for methods the annotation refers to the returned value.

The @Nullable annotation

I have to make a little confession: FindBugs provides an additional @Nullable annotation too. I haven’t discussed this one because, as far as I know, this annotation has no well defined semantics: see the discussion at http://code.google.com/p/google-collections/issues/detail?id=247, or http://osdir.com/ml/java.findbugs.general/2006-09/msg00063.html, for example.

For the time being, just ignore it unless you want to make your life more difficult:@CheckForNull and @NonNul seem to be more than enough.

And, of course, if somebody out there knows better, I’ll be glad to hear from him!

Other issues

I’m a fervent fan of Java assertions, and I write assertions checking that a parameter is not null so often that it has become part of the landscape in my projects. Granted, we’ve got the FindBugs @NonNull annotation, but I still want to write an assertion that checks the parameter is not null.

Why? Because I might be writing my code in an environment where FindBugs is not used, and they might refuse my request to add the required jars for compile time only –stuff happens!

I have verified that FindBugs will ignore the attempt to check for a value being null in an assertion, making it possible for me to have both things: the annotation plus FindBugs checks, and the assertion. Therefore, this code will compile and pass FindBugs checks.

public String myToUppercase(
@NonNull String parameter )
{
assert parameter != null;
return parameter.toUpperCase(Locale.getDefault());
}

And, just for completeness: there is a fourth annotation, @PossiblyNull, which is deprecated –and equivalent to @CheckForNull.

Following a strict policy

I hope the prior examples have shown that there are many subtle issues with regard to null values handling and that FindBugs makes handling them a breeze.

Therefore, I would advise a very strict policy: add @CheckForNull and @NonNull annotations to all fields, parameters and methods handling non primitive types. To be fair, I find this so helpful that I feel you should never have a I can’t assume anything about nullability policy in our own code, unless there is some really good reason.

Besides, it might be very interesting to set the default policy to @CheckNotNull or @NonNull, something you can do at the package or class level using one or more of the following annotations: @DefaultAnnotation, @DefaultAnnotationForFields, @DefaultAnnotationForMethods, @DefaultAnnotationForParameters or @ReturnValuesAreNonnullByDefault.

For example, this code sets @NonNull as the default policy for MyClass:

@edu.umd.cs.findbugs.annotations.DefaultAnnotation(value=NonNull.class)
public class MyClass {
// ...

It is possibly to set the default policy for all classes in a package too.

In fact, several papers suggest that the non-null policy should probably be the default one for Java, because “the authors conducted an empirical study of five open source projects totalling 700 K lines-of-code, which confirms that, on average, 75% of reference declarations are meant to be non-null, by design” (see Reducing the use of nullable types through non-null by default and monotonic non-null), or http://www.disi.unige.it/person/AnconaD/FTfJP06/paper03.pdf

Tool and library versions

The FindBugs version we have used for this article is 1.3.9, with the corresponding plugin running in Eclipse 3.3.0.

By the way, note that, to use these annotations, you need to add a pair of jars FindBugs provides: annotations.jar and jsr305.jar. However, these jars need not be redistributed, they are used at compile time only.

Related articles

You might be interested in taking a look at other FindBugs posts and articles:

  • This article is about using FindBugs annotations to make sure that resources (connections, streams, sockets, etc.) are released.
  • This article is about using the FindBugs @CheckReturnValue annotation to make sure programmers consuming our methods use the returned value for some of them.

2 Respuestas a “FindBugs: make your code better with Nullability Annotations

  1. Pingback: » Setting up Agile development of Eclipse RCP Code Bits

  2. Stephen Cooper

    One annoying thing with setting the default annotation for fields to CheckNull is that final fields which are initialized in the declaration still cause FindBugs to flag that you have to null check them (or add @Nonnull to the already final declaration).

Responder

Introduce tus datos o haz clic en un icono para iniciar sesión:

Logo de WordPress.com

Estás comentando usando tu cuenta de WordPress.com. Cerrar sesión / Cambiar )

Imagen de Twitter

Estás comentando usando tu cuenta de Twitter. Cerrar sesión / Cambiar )

Foto de Facebook

Estás comentando usando tu cuenta de Facebook. Cerrar sesión / Cambiar )

Google+ photo

Estás comentando usando tu cuenta de Google+. Cerrar sesión / Cambiar )

Conectando a %s