dnrajugade / guava-libraries

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

Consistently apply @Nullable to methods when they can return null #1221

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Could you add @Nonnull or @Nullable annotations for the return types of 
functions. This would make working with guava and annotation based null 
processing much easier. And it would further help to document the interface of 
the functions. @ParametersAreNonnullByDefault only applies to the parameters 
and not the return type of the function, that is why explicit annotations are 
neccessary.

An example is the Preconditions function checkNotNull, it will always return a 
non null result so it should be annotated that way.

@Nonnull
public static <T> T checkNotNull(T reference)

Or in ImmutableList

@Nonnull
public static <E> ImmutableList<E> of(E element) 

Or Optional
@Nonnull
public abstract T get();

@Nonnull
public abstract T or(T defaultValue);

Original issue reported on code.google.com by l.brueni...@googlemail.com on 2 Dec 2012 at 4:04

GoogleCodeExporter commented 9 years ago
I don't think we're likely to ever use @Nonnull, but we've been getting better 
about using @Nullable in return types.

Original comment by wasserman.louis on 2 Dec 2012 at 5:33

GoogleCodeExporter commented 9 years ago
I agree, we've always meant to sweep through and apply @Nullable consistently 
to our methods as well as only our parameters.

The one time I think we might need to use @Nonnull is if a non-null-returning 
method *overrides* a @Nullable one; there'd be no other way to counteract the 
information from the parent type.

Could our users help us find a complete list of all the places this should be 
added?

Original comment by kevinb@google.com on 4 Dec 2012 at 5:00

GoogleCodeExporter commented 9 years ago
>I don't think we're likely to ever use @Nonnull
What is the reasoning behind this? I get it for things like Functions or the 
like, but for checkNotNull() or Optional.get() it is the guaranteed behavior so 
why would you not want to annotate it with @Nonnull?

If it is not annotated, then at least the Eclipse compiler will complain about 
possible nullness of the return values. And I don't think that, even if a tool 
could use inference to determine the nullness/nonnullness of a function, 
implicit nonnullness is a good thing for a library. It belongs to the exposed 
API and should be made explicit, also making sure that it does not get changed 
later by accident.
If you decide to change the API later on, then the users will be made aware of 
this change by compiler/findbugs warnings, instead of being surprised by 
strange behavior.

Well thats my 2 cents, I'm really interested in your reasons.

Original comment by l.brueni...@googlemail.com on 5 Dec 2012 at 9:24

GoogleCodeExporter commented 9 years ago
Issue 1396 has been merged into this issue.

Original comment by cgdec...@gmail.com on 4 May 2013 at 5:13

GoogleCodeExporter commented 9 years ago
"We've been getting better about using @Nullable in return types." Why? 
Applying it to return values has no effect because it is the same as having no 
annotation w.r.t. FindBugs. In other words, it doesn't help tooling.

While documentation is great, I'd much rather see methods annotated with 
@Nonnull and @CheckForNull. In fact, we apply @ParametersAreNonnullbyDefault 
and @ReturnValuesAreNonnullByDefault to all packages in our projects and apply 
@CheckForNull as needed. This has been working very well so far.

Original comment by dharkn...@gmail.com on 4 May 2013 at 7:25

GoogleCodeExporter commented 9 years ago
We're not going to be able to do this completely consistently until we have 
tool support for this...but we still should do this when we notice deficiencies.

Original comment by kak@google.com on 22 Aug 2013 at 11:43

GoogleCodeExporter commented 9 years ago
Let me start off by saying I really like Guava, and I really appreciate all the 
work everyone has put into it.

That being said, I agree with the comments here that having return types 
annotated with @Nonnull and @CheckForNull would be very useful for those using 
FindBugs and other tools doing null analysis.  (I've found Eclipse's 
annotation-based null analysis introduced in recent versions to be especially 
useful.)

In addition, although some consider the JSR 305 annotations verbose, I feel 
they're actually one of the most succinct ways to document null-ness.  After a 
little getting used to, I've found them to improve readability considerably.

Would you consider taking a pull request that annotated the return types of at 
least some of the utilities in Guava?  Personally I would love to take care of 
Preconditions#checkNotNull and the factory methods in the immutable collections.

Original comment by andybrow...@gmail.com on 10 Feb 2014 at 4:34

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:06

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:13

GoogleCodeExporter commented 9 years ago

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