FreeBuilder icon indicating copy to clipboard operation
FreeBuilder copied to clipboard

Support @CheckForNull annotation

Open tkruse opened this issue 8 years ago • 3 comments

Hi, when annotating a method for freebuilder 1.12.3 like so:

package com.beantest.freebuilder;

import org.inferred.freebuilder.FreeBuilder;

import javax.annotation.CheckForNull;
import javax.annotation.Nullable;

/**
 */
@FreeBuilder
public abstract class Bus {

    @CheckForNull // freebuilder ignores CheckForNull
    @Nullable  // passed on by freebuilder, but ignored by Findbugs
    abstract String driverName();

    static class Builder extends Bus_Builder {
    }
}

Then the generated classes will not have the @javax.annotation.CheckForNull annotation (However, the @Nullable annotation is passed on). The @CheckForNull annotation however is the one that findbugs will use for class members (it's a long-standing confusion over findbugs and jsr305).

I.e. the following will not raise a warning using FIndbugs:

        Bus bus3 = new Bus.Builder().build();
        // Not OK: Findbug no warnings about NullPointerDereference
        System.out.println(bus3.driverName().length());

I'd recommend doing the same for @CheckForNull as for @Nullable.

tkruse avatar Feb 04 '17 12:02 tkruse

Note AutoValue does pass on @CheckForNull

tkruse avatar Feb 04 '17 13:02 tkruse

Would you expect the @CheckForNull annotations to be placed on the private member fields, too? Or should those be marked @Nullable and only the getter methods marked @CheckForNull? I'm not familiar with how FindBugs treats the annotation.

alicederyn avatar Feb 04 '17 16:02 alicederyn

Tricky question. The problem with Findbugs is, that it's behavior often defies rational thinking. So I tried this out manually, and all I can say is that with the current version of findbugs 3.0.1, I only got consistently good results if @CheckForNull was only applied to the accessor method, and NOT to the private field.

In particular combining @CheckForNull with @Nullable, I got inconsistent behavior when both were being added to the private fields.

This might well a due to bugs in findbugs, so if you want to document the design you choose, I think you have no other option than to write down that the annotation is not applied to the field because this is known to have undesired effect in Findbugs 3.0.1

Nobody knows for sure how Findbugs should or will treat those annotations. Some description is to be found here:

  • https://checkerframework.org/manual/#findbugs-nullable
  • http://stackoverflow.com/questions/31157511/how-to-use-findbugs-checkfornull-nonnull-and-nullable-annotations-correctly

tkruse avatar Feb 05 '17 02:02 tkruse