Not-null varargs argument with Nullable elements - considered as Nullable

[krisso-rtb]

@Documented
@Retention(RetentionPolicy.RUNTIME)
@Target({ ElementType.TYPE_USE, ElementType.FIELD, ElementType.METHOD, ElementType.PARAMETER,
        ElementType.LOCAL_VARIABLE })
public @interface Nullable {
}
--------------------------------------------------------------------------
@Documented
@Retention(RetentionPolicy.RUNTIME)
@Target({ ElementType.TYPE_USE, ElementType.FIELD, ElementType.METHOD, ElementType.PARAMETER,
        ElementType.LOCAL_VARIABLE })
public @interface NonNull {
}
--------------------------------------------------------------------------
protected static String composeName(@Nullable String @NonNull... names) {
    StringJoiner stringJoiner = new StringJoiner(":");
    for (String name : names) {
        if (name != null && !name.isEmpty()) {
            stringJoiner.add(name);
        }
    }
    return stringJoiner.toString();
}

result:

error: [NullAway] enhanced-for expression names is @Nullable

which points to line:

for (String name : names) {

Key facts:

• names is vararg array, it's expected to be not-null and contain:
• Nullable String elements
• For some reason these two annotations are mixed.

Nice examples:
https://help.eclipse.org/latest/index.jsp?topic=%2Forg.eclipse.jdt.doc.user%2Ftasks%2Ftask-using_null_type_annotations.htm&cp=1_3_9_1_3&anchor=compatibility

There is also one answer I found helpful:
https://stackoverflow.com/questions/32327134/where-does-a-nullable-annotation-refer-to-in-case-of-a-varargs-parameter

[msridhar]

Yes, NullAway doesn't properly parse annotations on arrays (or varargs); it interprets the @Nullable annotation in your example as pertaining to the reference to the array itself, not to the references within the array. Unfortunately fixing this will introduce compatibility issues, so we need to be careful. We plan to do this as part of our efforts to support JSpecify. Thanks for the report!

[cpovirk]

This is probably not a surprise, but I notice the same thing with annotations on type arguments: They're interpreted as if they're on the "top-level" type. For example:

package p;

import java.util.List;
import org.checkerframework.checker.nullness.qual.Nullable;

class TypeArgumentAnnotation {
  void use(List<@Nullable String> list) {
    list.hashCode();
  }
}

TypeArgumentAnnotation.java:8: error: [NullAway] dereferenced expression list is @Nullable
    list.hashCode();
        ^

[msridhar]

Wow @cpovirk that was not expected. I am going to pull that out as a separate issue.

[msridhar]

@cpovirk the issue you observed was fixed in #702

[cpovirk]

The main fix here does still need to be in NullAway, but I did notice one other thing when I was looking back at this issue:

Because the @Nullable annotation in this example includes PARAMETER (as well as TYPE_USE) in its @Target, it does get applied to the entire parameter, too. We can see in the .class file that the parameter has @Nullable on it. That's in addition to the @NonNull annotation on the parameter type and to the @Nullable annotation on the parameter's element type. That means that the array is annotated as both nullable and non-null. I don't know how NullAway normally handles such conflicts. But you could avoid that entirely by removing PARAMETER from the @Target.

(If you still have tools that require PARAMETER in the @Target, you might be able to use a separate @Nullable annotation for them. Unfortunately, whenever you have annotations that include both TYPE_USE and other target locations, you may encounter cases in which they're applied to multiple locations.)

[msridhar]

Thanks a lot @cpovirk! We will keep an eye out for that issue. I would have been very confused without your comment. I hope to get back to this issue at some point soon.