If you dont trust FindBug, it may be an option to use an assertion. This way you avoid the problems mentioned by User MSalters but still have a check. Of course, this approach may not be applicable if such a fail-fast approach is not feasible, i.e. you have to catch any exception.
And to further answer the question whether it is bad practice. Well, this is arguable but I would not think so. I consider this FindBug annotation as a light specification following the design by contract principle.
In design by contract you establish a contract between a method and its caller. The contract consists of preconditions that the caller agrees to fulfill when calling the method and a postcondition that in turn is fulfilled by the method after execution if its precondition is fulfilled.
One of the benefits of this development method is that you can avoid a so called defensive programming style (in which you check for all invalid parameter values explicitly etc.). Instead you can rely on the contract, and avoid redundant checks to increase performance and readibility.
Contracts can be used for runtime-assertion checking which follows the above mentioned fail-first principle in which you want your program to fail if a contract is broken, giving you a clue to identify the source of error. (A failed precondition means that the caller did something wrong, a failed postcondition indicates an implementation error of the given method)
Furthermore, contracts can be used for static analysis, which tries to draw conclusions about the source code without actually running it.
The @nonnull annotation can be seen as a precondition that is used for static analysis by the tool FindBugs. If you prefer an approach such as runtime-assertion checking, i.e. the fail first strategy, you should consider using assertion statements as built-in Java. Or maybe to adapt to a more sophisticated specification strategy using a behavioral interface specification language such as the Java Modeling Language (JML).
JML is an extension of Java in which specification is embedded in special Java Comments. An advantage of this approach is that you can use sophisticated specification, even using a development approach in which you only rely on specification rather than implementation details and use different tools that make use of the specification, such as the mentioned runtime-assertion checking tools, automated generation of unit-tests or documentation.
If you share my point of view of @nonnull being a (light) contract, it would be common practice to not add additional checks because the original idea behind this principle is to avoid defensive programming.
What is a type?
A type is meta data that can be used, at compile time or run time, to describe the shape of our data.
trait A { val b: X; def c(y: Y): Z}
So we know in our program that when ever we see an a: A, we know that a has the shape of A so we can call a.b safely.
So what shape does null have?
null has no data. null does not have a shape. String s = null is allowed as null has no shape, so it can fit any where. MikeFHay above says that an empty collection can be thought of as a null object. This is wrong, an empty collection has exactly the same shape as any other collection.
If a field can be null, then we can no longer rely on a.b to be safe. The shape of the data in this field is not truly A, only potentially A. The other shape it can be is null. That is the shape of this field is a union of A and null.
Most languages allow unions to be implemented as a tagged union. Scala's Option type is a tagged union of None | Some(a:A). Like wise Ceylon adds a ? postfix for types that say that the type may be null as in String? s = null.
Best Answer