feat: null-mark StatusOr using JSpecify

[JarvisCraft]

Description

This null-marks the experimental ValueOr type which whose methods' nullability actually depends on generic parameter nullability. The issue was discovered while enabling NullAway in one of our projects and getting a formally valid but effectively false-postivie nullable error on the result of StatusOr<EquivalentAddressGroup>#getValue().

Problems

This uses JSpecify to mark this specific class which is a preferred (if not only) method of annotation such complicated contracts, but it has the bytecode version 52 (Java 8) while grpc-api module seems to support Java 7.

[JarvisCraft]

I actually am not that familiar with bazel, so I just added the line which seemed to be valid, but have not tested it

[panchenko]

I suggest removing @Nullable from it, see #12338

[shivaspeaks]

Right, so we need to hold this until we upgrade grpc-api to java 8.

[JarvisCraft]

Right, so we need to hold this until we upgrade grpc-api to java 8.

Is there any planned date/release number for this change?
By the way, is there actually the need to follow minimal Java version for the annotations package? I am not sure here and may need to check corner-cases like AnnotatedElement#getAnnotations(), but optimistically it should not be a blocker.

[JarvisCraft]

Out of interest, I created the following test:

MyType.java

import java.lang.reflect.AnnotatedElement;
import java.util.Arrays;

public class MyType {
    private final @Nullable String field;

    public MyType(String field) {
        this.field = field;
    }

    public static void main(String... args) throws NoSuchFieldException, SecurityException {
        var field = MyType.class.getDeclaredField("field");
        var specific = args.length > 0 && args[0].equals("true");

        inspect(field, specific);
        inspect(field.getAnnotatedType(), specific);
    }

    private static void inspect(AnnotatedElement element, boolean specific) {
        System.out.println("getAnnotations() = " + Arrays.toString(element.getAnnotations()));
        System.out.println("getDeclaredAnnotations() = " + Arrays.toString(element.getAnnotations()));
        if (specific) {
            System.out.println("getAnnotation(@Nullable) = " +
                    element.getAnnotation(Nullable.class));
            System.out.println("getDeclaredAnnotation(@Nullable) = " +
                    element.getDeclaredAnnotation(Nullable.class));
        }
    }
}

Nullable.java

// Nullable.java
import static java.lang.annotation.ElementType.TYPE_USE;
import static java.lang.annotation.RetentionPolicy.RUNTIME;

import java.lang.annotation.Documented;
import java.lang.annotation.Retention;
import java.lang.annotation.Target;

@Documented
@Target(TYPE_USE)
@Retention(RUNTIME)
public @interface Nullable {
}

╰─❯ java MyType
getAnnotations() = []
getDeclaredAnnotations() = []
getAnnotations() = [@Nullable()]
getDeclaredAnnotations() = [@Nullable()]

╰─❯ java MyType true
getAnnotations() = []
getDeclaredAnnotations() = []
getAnnotation(@Nullable) = null
getDeclaredAnnotation(@Nullable) = null
getAnnotations() = [@Nullable()]
getDeclaredAnnotations() = [@Nullable()]
getAnnotation(@Nullable) = @Nullable()
getDeclaredAnnotation(@Nullable) = @Nullable()

╰─❯ rm Nullable.class

╰─❯ java MyType
getAnnotations() = []
getDeclaredAnnotations() = []
getAnnotations() = []
getDeclaredAnnotations() = []

╰─❯ java MyType true
getAnnotations() = []
getDeclaredAnnotations() = []
Exception in thread "main" java.lang.NoClassDefFoundError: Nullable
        at MyType.inspect(MyType.java:23)
        at MyType.main(MyType.java:15)
Caused by: java.lang.ClassNotFoundException: Nullable
        at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:641)
        at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
        at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:526)
        ... 2 more

So due to lazy nature of class-loading AND need to explicitly query for annotations AND the annotations being TYPE_USE the problem would only occur if trying to explicitly query for the specific annotation which would only be possible from the Java 8 caller (either with source @Nullable.class or runtime reflection on the annotation which would only list it if it was visible) which would always succeed.

Thus I feel like there is no problem in delivering grpc-api to Java 7 consumers built with Java 8+ annotations.

[ejona86]

grpc-api is already Java 8 bytecode, except for io.grpc.Context and io.grpc.Deadline. Thus, that's not really a limitation.

Because of the API impact nullness annotations have, before actually enabling anything that is functional (although as we've seen in #12338 the current non-functional nullable annotations are starting to have effect as tools retroactively change the semantics to align with JSpecify), I see no option other than to have a null checker at build time that can verify the code. Otherwise, we have no hope of maintaining the API. Note it also requires us to understand the annotations, and where they/the tools are broken/unstable.

Thus this is a premature to use JSpecify. Any problems right now should be solved by removing annotations.

We were mostly waiting for Guava to figure out null problems in the ecosystem and then could copy them. But if things are starting to settle and someone is interested, then we could start going down the path.