Problems with @Nullable parameter declarations and null analysis

[GoogleCodeExporter]

Eclipse 3.7 (i.e., JDT) introduced a null-analysis. This analysis reveals some 
problems with the Guava API, particularly with Function and Predicate. Here is 
an example to illustrate the problem:

Given a method foo defining an anonymous Function (as it usually done in the 
context of some collection operations):

    public void foo() {
        new Function<Object, String>() {
            @Override
            public String apply(@Nullable Object arg0) {
                return String.valueOf(arg0);
            }
        };
    }

Although this code looks quite nice, the null-analysis reveals a problem:

The method equals(Object) from Object cannot implement the corresponding method 
from Function<Object,String> due to incompatible nullness constraints

An indeed, Function redefines the equals method:

@Override boolean equals(@Nullable Object object);

In order to fix this problem, the equals method has to be implemented by the 
anonymous class, e.g.

    public void foo() {
        new Function<Object, String>() {
            @Override
            public String apply(@Nullable Object input) {
                return String.valueOf(input);
            }

            @Override
            public boolean equals(@Nullable Object object) {
                return super.equals(object);
            }
        };
    }

This reveals a new problem, though:

The type new Function<Object,String>(){} should also implement hashCode() since 
it overrides Object.equals()

Again, we can fix this problem by adding a third method to the anonymous class:

    public void foo() {
        new Function<Object, String>() {
            @Override
            public String apply(@Nullable Object input) {
                return String.valueOf(input);
            }

            @Override
            public boolean equals(@Nullable Object object) {
                return super.equals(object);
            }

            @Override
            public int hashCode() {
                return super.hashCode();
            }
        };
    }

Finally the code is fine and produces no more errors or warnings. 

But who wants to write this much useless code, only to get rid of some 
warnings?!
Writing a custom abstract super class implementing euals and hashCode of 
Function in order to get rid of the warnings also seems like too much overhead.

If the @Nullable annotations is used only for documentation reasons, could you 
remove this annotations at least in Function and Predicate and move the 
nullable information into the JavaDoc? Otherwise it is almost impossible to use 
a null-analysis, as too much warnigns are produced.

Original issue reported on code.google.com by jens.von...@numberfour.eu on 4 Feb 2013 at 9:48

• Merged into: #1812

[GoogleCodeExporter]

Isn't the problem here that Eclipse is mistaken about the nullness constraint 
of Object#equals(Object)? Though the JDK doesn't use the @Nullable annotation, 
the JavaDoc clearly states that Object#equals(Object) returns false on null 
input. Thus the nullness constraints *are* compatible.

Original comment by stephan...@gmail.com on 4 Feb 2013 at 10:45

[GoogleCodeExporter]

> If the @Nullable annotations is used only for documentation reasons (...)
No, they aren't - @ParametersAreNonnullByDefault is used in all Guava packages 
(see Javadoc: 
http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/collect
/package-summary.html), so @Nullable "suppresses" the package-wide annotation.

Original comment by xaerx...@gmail.com on 4 Feb 2013 at 11:43

[GoogleCodeExporter]

I don't think there's much hope we could annotate everything to Eclipse's 
satisfaction with the minimal level of documentation that's available, to say 
nothing of maintaining backwards compatibility.

Original comment by wasserman.louis on 4 Feb 2013 at 4:22

[GoogleCodeExporter]

Original comment by kevinb@google.com on 8 Apr 2013 at 7:01

• Added labels: Package-General

[GoogleCodeExporter]

Pasting the first code snippet into a project in Eclipse 4.2, I don't get any 
warnings from Eclipse or FindBugs.

Original comment by dharkn...@gmail.com on 6 May 2013 at 6:40

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

I am getting the same error Message with Eclipse 4.2, I am forced to implement 
the Dummy equals method to get rid of the error:

Multimaps.index(rows, new Function<PlzOrtZbnZuordnungsTabelleRow, PlzKey>() {
      @Override
      public FooKey apply(@Nullable BarRow row) {
        if (row != null) {
          return new FooKey(row.getFoo());
        }
        return null;
      }

      @Override
      public boolean equals(@Nullable Object o) {
        return false;
      }
    });

Original comment by knitel...@gmail.com on 31 Jul 2013 at 9:02

[GoogleCodeExporter]

To get Eclipse (4.2) to warn about Guava-related null annotations at all, you 
need to first uncheck "Use default annotations for null specifications" in the 
preferences under "Java > Compiler > Errors/Warnings > Null analysis", and 
replace the Eclipse-specific annotations with the corresponding "javax" 
annotations in the "Configure" dialog.

To get Eclipse to stop complaining about missing null annotations in subclasses 
(such as "Function"), enable "Inherit null annotations".

Original comment by eric.j...@gmail.com on 26 Oct 2013 at 1:59

[GoogleCodeExporter]

Okay, setting up Eclipse that way indeed shows more errors, and many that are 
utterly useless. I'll stick with FindBugs by itself for now.

For example, Eclipse issues a warning for this code:

    @CheckForNull
    private final Object field;

    public Object getField() {
        if (field == null) {
            throw new NullPointerException("field was not set");
        }
        return field; // Error: Null type mismatch
    }

Fair enough, perhaps it doesn't understand the check for null. So I check 
"Enable syntactic null analysis for fields" on that same page. The error 
remains, but now there's a solution: "Extract to checked local variable". What 
does it do? This:

    public Object getField() {
        if (field == null) {
            throw new NullPointerException("field was not set");
        }
        final Object field2 = field;
        if (field2 != null) {
            return field2;
        }
        else {
            // TODO handle null value
            return null;
        }
    }

Of course, if I negate the if() condition so "return field" is inside the null 
check and remove the crap it just added, the error goes away. So not very smart 
syntactic analysis here, and FindBugs handles this already beautifully.

Original comment by dharkn...@gmail.com on 20 Jan 2014 at 11:06

[GoogleCodeExporter]

I am somewhat inaccurately collapsing a bunch of nullability-annotation bugs 
into <https://code.google.com/p/guava-libraries/issues/detail?id=1812>. My 
apologies for the oversimplification.

Original comment by cpov...@google.com on 21 Jul 2014 at 8:06

• Changed state: Duplicate

[GoogleCodeExporter]

This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:13

• Added labels: MigratedToGitHub

[GoogleCodeExporter]

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:08

• Added labels: Restrict-AddIssueComment-Commit