javax.annotation.Nonnull vs assert

Question

I'm using Findbugs and javax.annotation.Nonnull on method parameters.

On private methods I usually add an assert line to check for nullness like

private void myMethod(@Nonnull String str) {
    assert str != null
    ....

Latest Netbeans version (7.3rc2) is reporting that the assert check is not necessary (because of the Nonnull annotation). I'm not fully sure this is a Netbeans bug or not.

Can the assert line be removed because I specified the @Nonnull annotation ?

As far as I understand, the annotation is used only during static analysis while assert is, when enabled, active during execution so the twos are not alternative.

Solution 1

The assert is evaluated at runtime, the annotation helps FindBugs catch problems during the analysis before runtime. As both checks are not really conflicting you could keep them both. I would find it annoying if my IDE told me to remove the assert.

Solution 2

Netbeans is right. If you think it can be null: remove the annotation. If you know it can't: remove the assert.

If there's ANY chance that your method could be called with a null value, then @Nonnull annotation shouldn't be there.

Like you said, that annotation doesn't actually do anything at runtime: it is only used by IDEs and static code analysis tools. It doesn't ensure that things aren't null.

Solution 3

Since this is private method, we can ensure that annotated parameter cannot be null. I think you can remove this assertion.

If NetBeans warns to public method, I think it has problem. I recommend you to put assertion.

If you still feel that assertion in private method is necessary, I think you can use bytecode injection. For instance, here is a maven plugin to inject null check. Sorry this is my personal project, but it works to me. I guess it can suit your need. https://github.com/KengoTODA/jsr305-maven-plugin

Comments

[Gualtiero Testa]

In the example, I assume that the parameter should never be null.

[Gualtiero Testa]

By adding the annotation I'm telling to Findbugs that the "The annotated element must not be null." Findbugs will check all calls to the method to ensure that parameter is never null. So annotation is needed (in this case). The question is "is the assert usefull or not ?". Findbugs can be wrong in the analysis so the assert can cover dynamic situations not detected by Findbugs

[Christian Strempfer]

Actually a developer wants both. The assertion will make it fail fast, when something happens that wasn't covered by code analysis. The annotation will highlight mistakes during development. So while you're answer is theoretically correct, I wouldn't advice to remove anything.

[Christophe Roussy]

FindBugs is indeed not bullet proof, it tries hard to find problems but it will not always work. The assertion is your runtime safety net.

[Benny Bottema]

Not only FindBugs, but modern IDE's such as IntelliJ also use it in its analyses.

[Christophe Roussy]

Yes IDEs have been catching up, but I also like running this during the build (outside the IDE) and fail the build in case it finds serious issues.