DaveAKing / guava-libraries

Automatically exported from code.google.com/p/guava-libraries
Apache License 2.0
0 stars 0 forks source link

Create abstract base classes for Function and Predicate to deal with @Nullable issues #1677

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
When implementing Functions or Predicates I often need to add special-case 
boilerplate for dealing with null inputs or be punished by findbugs for 
ignoring the @Nullable parameter annotations. To deal with this, I find myself 
creating abstract classes for both Functions and Predicates in many different 
projects.

For predicates there is only one version that makes sense

public abstract class NullSafePredicate<T> implements Predicate<T> {
    @Override
    public final boolean apply(@Nullable final T input) {
        return input != null && doApply(input);
    }
    protected abstract boolean doApply(@Nonnull T input);
}

For Functions I use two versions, null in -> null out and null in -> NPE

public abstract class NullSafeFunction<I, O> implements Function<I, O> {
    @Nullable @Override
    public final O apply(@Nullable final I input) {
        return input == null ? null : doApply(input);
    }

    protected abstract O doApply(@Nonnull I input);
}

public abstract class NullRejectingFunction<I, O> implements Function<I, O> {
    @Nullable @Override
    public final O apply(@Nullable final I input) {
        return doApply(checkNotNull(input));
    }

    protected abstract O doApply(@Nonnull I input);
}

What are your thoughts on adding such helper classes to Guava?

Original issue reported on code.google.com by SeanPFl...@googlemail.com on 20 Feb 2014 at 4:43

GoogleCodeExporter commented 9 years ago
Can you give an example or two of specific code that FindBugs is punishing you 
for, and what it's saying?

But to be honest, there is no making findbugs completely happy.  To the extent 
that there IS a viable solution for static nullability checking it is almost 
certainly the jsr308-based "checker framework".  Guava has not yet made the 
jump to fully supporting it for a host of reasons, one of which is the 
long-tail complexity (it requires *17* annotations to fully express all 
possible circumstances!).

Original comment by kevinb@google.com on 20 Feb 2014 at 4:51

GoogleCodeExporter commented 9 years ago
Example code:

    public static <I> Function<I, Integer> hashCodeFunction1() {
        return new Function<I, Integer>() {
            @Override
            @Nullable
            public Integer apply(@Nonnull final I input) {
                return input.hashCode();
            }
        };
    }

    public static <I> Function<I, Integer> hashCodeFunction2() {
        return new Function<I, Integer>() {
            @Override
            @Nullable
            public Integer apply(final I input) {
                return input.hashCode();
            }
        };
    }

    public static <I> Function<I, Integer> hashCodeFunction3() {
        return new Function<I, Integer>() {
            @Override
            @Nullable
            public Integer apply(@Nullable final I input) {
                return input.hashCode();
            }
        };
    }

    public static <I> Function<I, Integer> hashCodeFunction4() {
        return new Function<I, Integer>() {
            @Override
            @Nullable
            public Integer apply(@Nullable final I input) {
                if (input == null) {
                    return null;
                }

                return input.hashCode();
            }
        };
    }

    public static <I> Function<I, Integer> hashCodeFunction5() {
        return new Function<I, Integer>() {
            @Override
            @Nullable
            public Integer apply(@Nullable final I input) {
                return checkNotNull(input).hashCode();
            }
        };
    }

of these 5 methods, all but #4 get a findbugs warning with the 
findbugs-maven-plugin in Version 2.5.2

Here are the warnings:

[INFO] x0 must be nonnull but is marked as nullable ["some.pkg.Functions$1"] At 
Functions.java:[lines 17-21]
[INFO] input must be nonnull but is marked as nullable ["some.pkg.Functions$2"] 
At Functions.java:[lines 27-31]
[INFO] input must be nonnull but is marked as nullable ["some.pkg.Functions$3"] 
At Functions.java:[lines 37-41]
[INFO] input must be nonnull but is marked as nullable ["some.pkg.Functions$5"] 
At Functions.java:[lines 61-65]

though the log level is INFO, the build still fails.

Original comment by SeanPFl...@googlemail.com on 21 Feb 2014 at 12:16

GoogleCodeExporter commented 9 years ago
> To the extent that there IS a viable solution for static nullability checking 
it is almost certainly the jsr308-based "checker framework".

Indeed. However, I think the Checker Framework would/should disallow 
'#apply(@Nonnull T)' completely, based on my reading of this FAQ section: 
http://types.cs.washington.edu/checker-framework/current/checkers-manual.html#fa
q-semantics-section

(That said, I just tried it and it seems the Checker Framework does not 
currently complain about '#apply(@Nonnull T)'... did I misunderstand, or is 
that a bug?)

Original comment by stephan...@gmail.com on 23 Feb 2014 at 11:02

GoogleCodeExporter commented 9 years ago
You say all but example #4 get a warning.  But all but example #4 are also 
annotated incorrectly, aren't they?

Original comment by kevinb@google.com on 17 Apr 2014 at 3:47

GoogleCodeExporter commented 9 years ago
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:05

GoogleCodeExporter commented 9 years ago
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:09

GoogleCodeExporter commented 9 years ago

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