Improved varargs support.

[lazaroclapp]

Resolves #290 by supporting vararg nullability.

Previously, we ignored the default nullability info of vararg arguments,
but would error out if the vararg formal was deemed non-null by a handler.

Instead, we now correctly acknowledge the nullability of vararg arguments
whether they are non-null due to default code assumptions or due to handlers
(restrictive annotations, @Contract, etc).

This PR also adds tests and support for javax.annotations.Nonnull which
we were ignoring before and is needed for those tests.

[msridhar]

Can we add a test for the case where the varargs are @Nullable? I think we won't get any safety for this case within the body of the function unfortunately (i.e., it will be able to deref the args without a null check). But we should make sure that there is no warning at the call site.

With that change I approve

[lazaroclapp]

@msridhar There is an issue with the @Nullable case above and beyond not getting any safety for the body of the called function, actually. Namely that NullAway interprets @Nullable Object... others to be a nullable array of non-nullable objects. Thus, this fails:

for (Object other : others) { ... }

With the for loop taken as de-referencing others.

Given that the varargs parameter can be, say, assigned to a field inside the method, I am not sure how to fully support this without support for @Nullable generics.

I see two options:

1. Undo most of this change and just make NullAway ignore annotations on the var args
2. Support T1[@Nullable T2] through NullAway
3. Some limited support for nullable var args where you: a) treat the array ref as non-null, b) but set the elements to @Nullable whenever you load from the array or loop over it, c) forbid assigning the var args array to any other field/local

(1) is easy but unsound, (2) is significant work, (3) is... sort of inconsistent?

[msridhar]

It seems like it would be quite rare to want to pass null as a vararg. So how about just leaving the code as is, meaning that NullAway does not allow passing null as a vararg? If we think that is too strict, I think option 1 is best for now.

…

On Fri, Mar 29, 2019 at 8:56 AM Lázaro Clapp ***@***.***> wrote: @msridhar <https://github.com/msridhar> There is an issue with the @nullable case above and beyond not getting any safety for the body of the called function, actually. Namely that NullAway interprets @nullable Object... others to be a nullable array of non-nullable objects. Thus, this fails: for (Object other : others) { ... } With the for loop taken as de-referencing others. Given that the varargs parameter can be, say, assigned to a field inside the method, I am not sure how to fully support this without support for @nullable generics. I see two options: 1. Undo most of this change and just make NullAway ignore annotations on the var args 2. Support ***@***.*** T2] through NullAway 3. Some limited support for nullable var args where you: a) treat the array ref as non-null, b) but set the elements to @nullable whenever you load from the array or loop over it, c) forbid assigning the var args array to any other field/local (1) is easy but unsound, (2) is significant work, (3) is... sort of inconsistent? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#291 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AALyUdk1iCFjC6JpWzmmNrbN0tW6esa5ks5vbjeXgaJpZM4cJT-S> .

[lazaroclapp]

Fine by me, but let me add an error explaining that when we see @Nullable in a var args in the method declaration, then. Otherwise the message is pretty confusing.