Got Findbugs issue “NP_NULL_ON_SOME_PATH” when null test is in a separate method. Is this a false alarm?

Question

I have some code which is similar to the following snippet:

public void foo(Order o) {
    ...
    checkInput(o, "some error message");
    doSomehing(o.getId());
}

private void checkInput(Object o, String message) {
    if (o == null) {
        throw new SomeRuntimeException(message);
    }
}

And I got Findbugs reporting an 'NP_NULL_ON_SOME_PATH' issue.

Here is the description:

There is a branch of statement that, if executed, guarantees that a null value will be dereferenced, which would generate a NullPointerException when the code is executed. Of course, the problem might be that the branch or statement is infeasible and that the null pointer exception can't ever be executed; deciding that is beyond the ability of FindBugs.

My questions are:

1. Can I treat it as a false positive in this example?
2. Is it a good practice to put the null test in a separate method? The actual null check method is a bit longer than the sample method so I don't want to repeat the code everywhere.

Thanks!

Answer 1

It looks like FindBugs is not able to detect this case, at least with 2.0.2 in Eclipse. One workaround is to return the value from checkError and annotate the method with @Nonnull.

public void foo(Order o) {
    ...
    doSomehing(checkInput(o, "some error message").getId());
}

@Nonnull
private Order checkInput(Order o, String message) {
    if (o == null) {
        throw new SomeRuntimeException(message);
    }
    ...
    return o;
}