Kotlinc explicit non-nullability of varargs arguments

[lazaroclapp]

Context Part 1: Var args nullability on NullAway and JSpecify

Handling of nullability of var args in NullAway is admittedly a bit hacky (see #674). There is a clear answer on what the correct way to interpret @Nullable annotations is for type-use annotations, which we currently don't follow, namely:

foo(@Nullable Object... varargs)

Means: the elements of the varargs array can be null, but the array reference itself is non-null (99.8% of the time, this is what we want, though it is possible in Java to pass a null varargs array by e.g. passing (Object[]) null)

Whereas:

foo(Object @Nullable... varargs)

Can be used to indicate that the array varargs itself is @Nullable, if absolutely needed.

For declaration annotations (i.e. not type-use), the only valid position is:

foo(@Nullable Object... varargs)

And it seems reasonable to treat it equivalently to the same syntax for type-use, both for consistency and because it's more likely than not what the developer intends.

Note: We currently interpret the annotation above to mean both the array and its elements can be nullable, i.e. we read it as what, using type-use annotations, would be written as foo(@Nullable Object @Nullable... varargs), but I think this is a bug/artifact of previous limitations and should be (carefully) fixed.

Context Part 2: Kotlinc generated jars and restrictive @NonNull annotations

The Kotlin compiler adds @Nullable and @NonNull annotations to the bytecode it produces, but only declaration annotations at the full parameter level (i.e. no annotations on generics, elements of arrays, etc). So, code like:

fun bar1(s: String?)

Becomes the equivalent of:

bar1(@Nullable String s)

And:

fun bar1(s: String)

Becomes the equivalent of:

bar1(@NotNull String s)

(The annotations in question are org.jetbrains.annotations.Nullable and org.jetbrains.annotations.NotNull)

This is generally a big help to NullAway, when configured with -XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=..., as it can use kotlinc added nullness information to reason about methods returning nullable or taking non-null arguments.

The issue:

Unfortunately, the following Kotlin code:

fun baz(vararg args: Any?)

Actually becomes:

  public static void baz(java.lang.Object...);
      [...]
    RuntimeInvisibleParameterAnnotations:
      parameter 0:
        0: #30()
          org.jetbrains.annotations.NotNull

Which is the equivalent of:

baz(@NotNull Object... args)

because kotlinc interprets the annotation as applying to the first parameter args as a whole and the var args array itself is non-null.

The solution(s):

1. We could maybe get kotlinc to change the default meaning of declaration annotations in var args to match NullAway's (desired) semantics. However, even if they agree with our interpretation, this would be a big breaking change to their jar ABI.
2. We could generally have NullAway interpret foo(@Nullable Object... varargs) with a declaration annotation as foo(Object @Nullable... varargs) with a type-use annotation. This is very ugly and confusion prone from the point of view of developers using either type of annotation, or worse, a mixture of both in different projects/targets.
3. More realistically: We could hack into our restrictive annotations handler some knowledge of this issue, making it ignore either any non-null annotation or specifically org.jetbrains.annotations.NotNull when looking at a varargs argument.

Neither option is perfect, but this is a rather long brain-dump to convince myself that the hack from Option 3 is the best we can do here (and to document why).

[lazaroclapp]

We have applied fix number 3 here. Wonder if we should just close this issue and move on or leave it open and report the potentially confusing annotation to Kotlin maintainers.

[msridhar]

IMO we can close this as long as #674 tracks the remaining known issues with our handling of varargs. We should track reporting the Kotlin issue separatley.