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.

See this [related question](http://programmers.stackexchange.com/questions/168395/check-parameters-annotated-with-nonnull-for-null) on Programmers.SE. — David Harkness, Feb 12 '13 at 23:32

Christophe Roussy · Answer 1 · 2013-02-14T13:03:20.490

13

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.

edited Feb 14 '13 at 13:03

answered Feb 14 '13 at 12:25

Christophe Roussy

16,299
4
85
85

Not only FindBugs, but modern IDE's such as IntelliJ also use it in its analyses. – Benny Bottema Aug 23 '17 at 06:39
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. – Christophe Roussy Aug 23 '17 at 07:38

score 8 · Answer 2 · answered Feb 11 '13 at 10:31

8

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.

answered Feb 11 '13 at 10:31

David Lavender

8,021
3
35
55

In the example, I assume that the parameter should never be null. – Gualtiero Testa Feb 11 '13 at 10:42
2

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 – Gualtiero Testa Feb 11 '13 at 10:50
5

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. – Christian Strempfer Feb 19 '13 at 11:03
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. – Christophe Roussy Oct 09 '14 at 07:25

Kengo TODA · Answer 3 · 2015-10-09T22:48:25.860

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

score 0 · Answer 4 · answered Jun 04 '15 at 14:17

I found a different solution, as I was thinking about my IDE warnings.

Initially, I felt that the IDE was wrong. I'm a paranoid programmer, and want to have the label for documentation & static analysis AND a runtime check in case I ever use it from reflection, or another JVM language or something that isn't statically analyzable, so I thought it was wrong to give me a warning and tell me the assert(x != null) statement wasn't needed.

But then I thought about how asserts can be removed depending on the status of the -ea flag passed to Java at Runtime, and that in some ways assert and @Nonnull are really both development-only checks.

Turns out, there's an actual runtime check that can be inserted (Java 7+) Objects.requireNonNull which will throw a NullPointerException and cannot be removed with an -ea assertion. I think I'm going to prefer this to my assert(x != null); use(x); pattern.

public ConstructorForClass(@Nonnull Type x) {
  this.x = Objects.requireNonNull(x);
  //...
}

javax.annotation.Nonnull vs assert

4 Answers4

Linked