15

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.

2

4 Answers 4

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.

2
  • Not only FindBugs, but modern IDE's such as IntelliJ also use it in its analyses. Aug 23, 2017 at 6: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. Aug 23, 2017 at 7:38
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.

4
  • In the example, I assume that the parameter should never be null. Feb 11, 2013 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 Feb 11, 2013 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. Feb 19, 2013 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. Oct 9, 2014 at 7:25
1

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

0

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);
  //...
}

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.