@Nullable input in Google Guava Function interface triggers FindBugs warning

vitaly picture vitaly · Sep 14, 2012 · Viewed 14.3k times · Source

The com.google.common.base.Function interface (from Google Guava) defines apply as:

@Nullable T apply(@Nullable F input);

The method has the following javadoc note:

@throws NullPointerException if {@code input} is null and this function does not accept null arguments.

FindBugs complains about my implementation of Function:

private static final class Example implements Function<MyBean, String> {
    @Override
    @Nullable
    public String apply(@Nullable MyBean input) {
        if (null == input) {
            throw new NullPointerException();
        }
        return input.field;
    }
}

with a high-priority warning:

NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE, Priority: High

input must be nonnull but is marked as nullable

This parameter is always used in a way that requires it to be nonnull, but the parameter is explicitly annotated as being Nullable. Either the use of the parameter or the annotation is wrong.

My function does not support null inputs and an exception is thrown if that is the case. If I understand correctly, FindBugs treats this as a requirement for non-null.

To me it looks like a contradiction: input is @Nullable but method @throws NullPointerException when it is null. Am I missing something?

The only way to get rid of the warning that I can see is manual suppression. (Guava code is out of my control, obviously).

Who is wrong about the usage of @Nullable annotation, FindBugs, Guava or myself?

Answer

Xaerxess picture Xaerxess · Sep 14, 2012

Your implementation is wrong ;)

Basically docs says (I'll paraphrase and emphasise):

@throws NullPointerException if input is null and the concrete function implementation does not accept null arguments

By implementing your function you must decide if it accepts nulls or not. In first case:

private static final class Example implements Function<MyBean, String> {
    @Override
    @Nullable
    public String apply(@Nullable MyBean input) {
        return input == null ? null : input.field;
    }
}

In second case:

private static final class Example implements Function<MyBean, String> {
    @Override
    @Nullable
    public String apply(MyBean input) {
        if (null == input) {
            throw new NullPointerException();
        }
        return input.field;
    }
}

In both examples returning null is allowed.

EDIT:

Note that Guava uses @javax.annotation.ParametersAreNonnullByDefault on all packages, hence if @Nullable is present it means "suspend global @Nonnull and allow nulls here" and if not it means "nulls forbidden here".

That said, you may want use @Nonnull annotation on your argument or @ParametersAreNonnullByDefault in package to tell FindBugs Function's argument can't be null.

EDIT 2:

Turns out this case is known issue, see comment #3 (from Guava's lead dev Kevin Bourrillion, about his conversation with Bill Pugh, Findbugs' lead):

My reference was a series of in-person conversations with Bill Pugh. He asserted unambiguously that @Nullable means only that some subtypes might accept null. And this seems to be borne out by findbugs for us -- our code passes the nullability checks pretty cleanly (though we should check again since this particular Function change was made).