javax.annotation.Nonnull vs assert

2019-04-19 00:52发布

问题:

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.

回答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.



回答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.



回答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



回答4:

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