Supports android.support.annotation.Nullable/NonNull

[martin-rust]

Hi, this introduces support for FindBugs to recognize
android.support.annotation.Nullable as CHECK_FOR_NULL and
android.support.annotation.NonNull as NONNULL.
While it is annoying that there are so many nullability annotations out in the wild and these are just another set of them, these are the only nullability annotations supported by Android Studio, so even if you develop on another IDE, but share code with Android Studio developers, these are the only way to go.
Hopefully soon there will be a Java standard for null annotations ...

[benjaminhollissc]

Shouldn't https://github.com/findbugsproject/findbugs/pull/117/files#diff-f9104e6f1e61769382ad798b84dc317aR56 mean that these are already supported, because the class ends in "Nullable" or "NonNull"?

[martin-rust]

That probably was the intention, but still FindBugs did produce false warnings. I will provide a demo case.

[martin-rust]

Here's a test snippet. It exposes:

• erroneous FindBugs warnings
• expected, but missing FindBugs warnings (I verified by using Eclipse null annotations instead of Android, and ran FindBugs)

import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
public class AnnotationDemo {
    private @Nullable String nullableField = "dummy";
    private @NonNull String nonNullField = "dummy";
    public void foo() {
    // Expected, but missing:
    // NP_NULL_ON_SOME_PATH:
    System.out.println(nullableField.hashCode());
    
    System.out.println(nonNullField.hashCode());
    
    nullableField = null;
    
    // Expected, but missing:
    // NP_STORE_INTO_NONNULL_FIELD:
    nonNullField = null;
    String localString;
    localString = nullableField;
    
    // Not sure if these are justified:
    // NP_LOAD_OF_KNOWN_NULL_VALUE:
    // RCN_REDUNDANT_NULLCHECK_OF_NULL_VALUE:
    if (null != localString) {
        System.out.println(localString.hashCode());
    }
    localString = nonNullField;
    
    // Erroneous (but here FindBugs is also wrong when using other null
    // annotations):
    // RCN_REDUNDANT_NULLCHECK_OF_NULL_VALUE:
    if (null != localString) {
        System.out.println(localString.hashCode());
    }
    localString = getNullable();
    if (null != localString) {
        System.out.println(localString.hashCode());
    }
    localString = getNonNull();
    
    // Expected, but missing:
    // RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE:
    if (null != localString) {
        System.out.println(localString.hashCode());
    }
    // Erroneous:
    // NP_NULL_PARAM_DEREF_ALL_TARGETS_DANGEROUS
    setNullable(null);
    // Expected, but missing:
    // NP_NONNULL_PARAM_VIOLATION:
    setNonNull(null);
    }
    public @Nullable String getNullable() {
    return null;
    }
    public @NonNull String getNonNull() {
    
        // Expected, but missing:
    // NP_NONNULL_RETURN_VIOLATION:
    return null;
    }
    public void setNullable(@Nullable String s) {
    
        // Expected, but missing:
    // NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE:
    System.out.println(s.hashCode());
    }
    public void setNonNull(@NonNull String s) {
    System.out.println(s.hashCode());
    }
    public void setNullableField(String nullableField) {
    this.nullableField = nullableField;
    }
    public void setNonNullField(String nonNullField) {
    this.nonNullField = nonNullField;
    }
}

[benjaminhollissc]

Wow! It'd be great to get this merged and released, then.

[scheffield]

What is the status of this PR. The solution seems reasonable...

[jsotuyod]

I fear FindBugs in it's current form is dead. See this

However, the spotbugs project is it's successor, and hopefully soon will have a 3.1.0 release.

For 3.2.0 / 4.0.0 these kind of changes are really something we want to look into. As is, the only thing we would add to merge this into SpotBugs is adding proper unit tests using our new test-harness.

If someone wants to take this over, and submit a PR with these changeset and unit tests, that would certainly help speed things up.

[iloveeclipse]

Fixed with spotbugs/spotbugs#182