36

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?

vitaly
  • 2,755
  • 4
  • 23
  • 35
  • 4
    You're saying you'll accept null in the signature, and then you're *rejecting* a null value in the body. It sounds like you've misunderstood the purpose of `@Nullable`. – Jon Skeet Sep 14 '12 at 11:16
  • 4
    The only reason why I added `@Nullable` for input parameter is that it is what `Function` interface defines. Anyway, I removed `@Nullable` annotation from the parameter as @Xaerxess suggested but FindBugs keeps complaining. – vitaly Sep 14 '12 at 13:27

3 Answers3

28

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).

Grzegorz Rożniecki
  • 27,415
  • 11
  • 90
  • 112
  • Which makes me wonder if it is ok to remove annotations specified by the interface in implementation? – vitaly Sep 14 '12 at 12:54
  • 1
    You don't remove them from interface, you add the to your implementation. If `Function` allows you to use null as argument that doesn't mean you'll have to allow null in your impl, just as `Function` contract states. – Grzegorz Rożniecki Sep 14 '12 at 13:14
  • 4
    The annotation are already specified by the framework interface and I can not (and not intending to) remove if from there. The only code I can edit is my own code (=implementation). I removed the @Nullable annotations from my implementation but FindBugs keeps complaining. – vitaly Sep 14 '12 at 13:20
  • 2
    Marked with `@javax.annotation.Nonnull` - but it did not help, still getting `This parameter is always used in a way that requires it to be nonnull, but the parameter is explicitly annotated as being Nullable.` – vitaly Sep 14 '12 at 14:51
  • 3
    @vitaly seems that's a [known issue](http://code.google.com/p/guava-libraries/issues/detail?id=920) and probably depends on FindBugs version you use. – Grzegorz Rożniecki Sep 14 '12 at 15:06
  • Thanks, @Xaerxess, I guess I will just accept the answer then. Additionally, here's the example code: https://github.com/lischenko/flaming-ninja.git – vitaly Sep 14 '12 at 16:05
  • 1
    You use Findbugs 2.X, according to that Guava issue I'll say it's Findbugs 2.X bug ;) – Grzegorz Rożniecki Sep 14 '12 at 16:12
  • 1
    Re Guava vs FindBugs issue: Neither Eclipse Juno nor IntelliJ IDEA like this, either. So it's probably a misuse of `@Nullable`. I also understand this as "Will not throw an exception for `null` values". Otherwise, what's the point of the annotation anyway? – Aaron Digulla Oct 09 '12 at 07:19
  • 1
    This also means your second suggestion (explicitly throwing NPE) doesn't fix the issue; only the first version (`input == null ? null : input.field`) fixes the warning. – Aaron Digulla Oct 09 '12 at 07:20
  • @Xaerxess [updated issue link](https://github.com/google/guava/issues/920), and [new umbrella issue](https://github.com/google/guava/issues/1812) – Jesse Glick Dec 18 '14 at 17:54
4

Marking the parameter @Nonnull resolves the issue from findbugs.

0

It seems as though by default Google Guava Functions are @Nullable by default - I was getting Findbugs errors stating that "result must be nonnull but is marked as nullable" when there was no annotation. Adding @Nonnull to the function declaration in the following way helped:

new Function<Object, Object>() {
            @Nonnull
            public Object apply(@Nonnull Object object) {

and now Findbugs is happy. Thanks all

Ben Thomas
  • 33
  • 6