uber/NullAway

NullAway doesn't recognize @Nullable annotation in a different compilation unit

gavlyukovskiy opened this issue · 12 comments

I've been migrating on of our projects to jspecify and noticed that NullAway reports nullability problems for passing null values to the @Nullable generic parameters in another compilation unit (main source set).

src/main/java:

public class Main {
    public static void main(String[] args) {
        // ok
        withFunction(x -> null);
    }

    public static void withFunction(Function<String, @Nullable String> f) {}
}

src/test/java:

class MainTest {
    public static void main(String[] args) {
        // error: [NullAway] returning @Nullable expression from method with @NonNull return type
        Main.withFunction(x -> null);
    }
}

Compiling the test class gives the following error:

nullaway-reproducer/src/test/java/com/github/gavlyukovskiy/MainTest.java:6: error: [NullAway] returning @Nullable expression from method with @NonNull return type
        Main.withFunction(x -> null);
                          ^
    (see http://t.uber.com/nullaway )
1 error

I've also tried extracting the expression into Function<String, @Nullable String> variable, but passing that gives slightly different error:

nullaway-reproducer/src/test/java/com/github/gavlyukovskiy/MainTest.java:11: error: [NullAway] Cannot pass parameter of type Function<String, @Nullable String>, as formal parameter has type Function<String, String>, which has mismatched type parameter nullability
        Main.withFunction(f);
                          ^
    (see http://t.uber.com/nullaway )
1 error

I've tried the same test but without generic parameters:

public static void with(@Nullable String v) {}

and both main and test source sets react to it correctly (both with and without @Nullable annotation), so the problems appears to be with generics.

I've tried reproducing the situation inside com.uber.nullaway.jspecify.GenericsTests, but as they're the same compilation unit it wasn't possible.

Reproducer: https://github.com/gavlyukovskiy/nullaway-reproducer (./gradlew build)

Versions:

  • org.jspecify:jspecify:1.0.0
  • com.google.errorprone:error_prone_core:2.29.2
  • com.uber.nullaway:nullaway:0.11.1
  • net.ltgt.errorprone:4.0.1

Hi @gavlyukovskiy, one caveat is that our JSpecify mode checks are still limited. That said I think your specific test case should work. You may be running into #1005. Any chance you could try compiling using JDK 22 and see if that fixes the problem?

Hi @msridhar, thank you for looking into that. I've just tried temurin-22.0.2, but get the same error

This is very interesting, thanks for reporting. I can reproduce the problem, even on JDK 22, and I've pushed a test case here. This to me looks like a bug in javac. NullAway gets the type for the lambda being passed as a parameter here:

methodSymbol, ASTHelpers.getType(lambdaTree), state, config)

On JDK 21, this type is Function<String,String> with the type-use annotation missing (as expected). On JDK 22, we see the type Function<@Nullable String,String>, which is also wrong; the @Nullable should be on the second type argument. At the invocation of withFunction I see the type of the function as (java.util.function.Function<java.lang.@org.jspecify.annotations.Nullable String,java.lang.String>)void which also has the type-use annotation in the wrong place.

@cushon @cpovirk do you agree this is potentially a bug in javac's reading of type-use annotations from bytecode? Is this a known issue?

FWIW, here is what I see for the withFunction method when I run javap -v on the class file:

  public static void withFunction(java.util.function.Function<java.lang.String, java.lang.String>);
    descriptor: (Ljava/util/function/Function;)V
    flags: (0x0009) ACC_PUBLIC, ACC_STATIC
    Code:
      stack=0, locals=1, args_size=1
         0: return
      LineNumberTable:
        line 8: 0
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0       1     0     f   Ljava/util/function/Function;
      LocalVariableTypeTable:
        Start  Length  Slot  Name   Signature
            0       1     0     f   Ljava/util/function/Function<Ljava/lang/String;Ljava/lang/String;>;
    Signature: #21                          // (Ljava/util/function/Function<Ljava/lang/String;Ljava/lang/String;>;)V
    RuntimeVisibleTypeAnnotations:
      0: #23(): METHOD_FORMAL_PARAMETER, param_index=0, location=[TYPE_ARGUMENT(1)]
        org.jspecify.annotations.Nullable

I'm pretty sure TYPE_ARGUMENT(1) means the second type argument. Also, within NullAway if I call getRawTypeAttributes() on the MethodSymbol for withFunction I see the position of the @Nullable annotation as:

[METHOD_FORMAL_PARAMETER, param_index = 0, location = (TYPE_ARGUMENT(1)), pos = -1]

That's an interesting find. It seems like it already puts an annotation on the first argument as well for BiFunction or custom functional interface.

@gavlyukovskiy I've confirmed this is an issue in javac. I'll put updates here as there is progress towards a fix.

I should say a bit more here. This test case exposes a new issue on JDK 22+. On JDK 21 and earlier the issue with not reading type annotations from bytecode was already known and is covered by #1005

There is now a PR openjdk/jdk#20460 up to fix this issue. Once a fix lands, it will have to get into a released JDK before NullAway users can benefit from it. Thank you again for reporting this! I will leave this issue open until a fix reaches a JDK release.

Small update here. The above PR has landed and will ship with JDK 24. There is a backport for JDK 23: openjdk/jdk23u#67 Assuming the backport goes through, I think this would be fixed in the first patch release of JDK 23 (it's too late to make the GA release).

Thank you for the update @msridhar! It might be a wrong place to ask, but is there any chance such a fix will be backported to jdk21u?

@gavlyukovskiy there is an effort to get related fixes for reading of type annotations from bytecodes backported to JDK 21 and possibly earlier. See jspecify/jspecify#365, https://bugs.openjdk.org/browse/JDK-8323093, and https://mail.openjdk.org/pipermail/compiler-dev/2024-July/027344.html. At this point, I imagine that if that backport is approved, the fixes for this issue would be included. /cc @cushon