@Nonnull with different IDEs - warn about unnecessary null checks

Question

I'm working in an environment where developers use different IDEs - Eclipse, Netbeans and IntelliJ. I'm using the @Nonnull annotation (javax.annotation.Nonnull) to indicate that a method will never return null:

@Nonnull
public List<Bar> getBars() {
  return bars;  // this.bars is a final, initialized list
}

I'd like other developers to get a warning if they do one of the following:

1. Change the method to return null without removing the @Nonnull annotation
2. Unnecessarily check for null for methods that are explicitly defined with @Nonnull: if (foo.getBars() == null) { ... do something ... }

The first scenario is supported e.g. by IntelliJ. The second is not; the clients are not warned that checking for null is unnecessary.

We're in any case planning to move towards returning clones of collections instead of the collections themselves, so is it better to forget @Nonnull and do this instead:

public List<Bar> getBars() {
  return new ArrayList<Bar>(bars);
}

Edit:

To clarify, I'm not considering changing IDE's for this. I'd like to know whether what I described above is supported by the mentioned IDEs - or alternatively, if there is a good reason as to why it is not supported.

I get the point about not relying too much on contracts. However, if I write getBars() with the style in the last paragraph (return a clone of the list), then e.g. IntelliJ flags a warning for any code that does

if (foo.getBars() == null) { ... }

If you choose to follow this warning and remove the null check, you seem to be equally reliant on the getBars() implementation not changing. However, in this case you seem to be depending on implementation details instead of an explicit contract (as is the case with @Nonnull).

Edit #2:

I'm not concerned about execution speed, null checks are indeed very fast. I'm concerned about code readability. Consider the following:

Option A:

if ((foo.getBars() == null || foo.getBars().size() < MAXIMUM_BARS) && 
    (baz.getFoos() == null || baz.getFoos().size() < MAXIMUM_FOOS)) { 
  // do something
}

Option B:

if (foo.getBars().size() < MAXIMUM_BARS && 
    baz.getFoos().size() < MAXIMUM_FOOS) {
  // do something
}

I think Option B is more readable than Option A. Since code is read more often than it is written, I'd like to ensure all code I (and others in our team) write is as readable as possible.

Answer 1

I would recommend the following:

• Stick with your @Nonnull and @Nullable annotations just like you are doing it already.
• Define and enforce a default. It doesn't really matter what the default is, but it should be the same across your entire code base.
• Use FindBugs to check for your cases 1 and 2. FindBugs has plugins for most IDEs, I saw Eclipse, Netbeans, and IntelliJ mentioned, but there are more. (The number 2 case is covered by the RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE rule. I tested it. Just thought I should mention that after reading DavidHarkness' comment elsewhere on this page.)

This approach is IDE agnostic and works also in an external build environment like Hudson or Jenkins.

Answer 2

I guess, with your annotation you want to do two things:

• make the program faster
• make the programmers have to work less (because they don't have to type the null-checks)

For the first one: A Null-check consumes hardly any resources. If you run this sample:

public static void main(String[] args) {
    long ms = System.currentTimeMillis();
    for (int i=0;i<10000;i++) {
        returnsNonNull(i);
        doNothing();
    }
    System.out.println("Without nullcheck took "+(System.currentTimeMillis()-ms)+" ms to complete");

    ms = System.currentTimeMillis();
    for (int i=10000;i<20000;i++) {
        if (returnsNonNull(i)!=null) {
            doNothing();
        }
    }
    System.out.println("With nullcheck took "+(System.currentTimeMillis()-ms)+" ms to complete");
}

public static String returnsNonNull(int i) {
    return "Foo "+i+" Bar";
}
public static void doNothing() {

}

You will see, there is hardly any difference between these two tests. The second one (with the null-check) is sometimes even faster than the first one, meaning they both are pretty much indistinguishable when it comes to resource use.

Now to the second point: You are actually making the programmers work more, not less. Except if there are absolutely no functions in your whole project and framework that ever return null the programmer now has to look up the function each time to check if he has to do a null-check or not. Seeing that about every framework in Java can under some conditions return null in some functions you are not reducing the workload for your programmers.

You said you want a warning to appear when they do a null-check where it is not necessary because of your @Nonnull-annotation. So what they will do is they type out the whole null-check and then get a warning and have to remove it again. See what I mean?

Also what if you make a mistake in your code and mark something as @Nonnull that can return null?

What I'd do is write it into the Javadoc under @return. Then the programmer can decide what he does, which is usually better than forcing others to write code your style.