JSpecify: read upper bound annotations from bytecode and add tests

nullaway/build.gradle

@@ -178,3 +178,15 @@ tasks.register('buildWithNullAway', JavaCompile) {
 project.tasks.named('check').configure {
     dependsOn 'buildWithNullAway'
 }
+
+tasks.withType(Test).configureEach { test ->
+    if (test.javaVersion < JavaVersion.VERSION_22) {
+        // Certain tests involving reading annotations from bytecode will not pass on pre-JDK-22 javac
+        // until the fix for https://bugs.openjdk.org/browse/JDK-8225377 is backported or until we add
+        // workarounds.  See https://github.com/uber/NullAway/issues/1005.
+        test.filter {
+            excludeTestsMatching "com.uber.nullaway.jspecify.BytecodeGenericsTests.genericsChecksForParamPassingAndReturns"
+            excludeTestsMatching "com.uber.nullaway.jspecify.BytecodeGenericsTests.genericsChecksForFieldAssignments"
+        }
+    }
+}

nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java

@@ -24,6 +24,7 @@
 import com.sun.source.util.TreePath;
 import com.sun.tools.javac.code.Attribute;
 import com.sun.tools.javac.code.Symbol;
+import com.sun.tools.javac.code.TargetType;
 import com.sun.tools.javac.code.Type;
 import com.uber.nullaway.CodeAnnotationInfo;
 import com.uber.nullaway.Config;
@@ -97,25 +98,48 @@ public static void checkInstantiationForParameterizedTypedTree(
     if (baseType == null) {
       return;
     }
+    boolean[] typeParamsWithNullableUpperBound =
+        getTypeParamsWithNullableUpperBound(baseType, config, state, handler);
     com.sun.tools.javac.util.List<Type> baseTypeArgs = baseType.tsym.type.getTypeArguments();
     for (int i = 0; i < baseTypeArgs.size(); i++) {
-      if (nullableTypeArguments.containsKey(i)) {
-
-        Type typeVariable = baseTypeArgs.get(i);
-        Type upperBound = typeVariable.getUpperBound();
-        com.sun.tools.javac.util.List<Attribute.TypeCompound> annotationMirrors =
-            upperBound.getAnnotationMirrors();
-        boolean hasNullableAnnotation =
-            Nullness.hasNullableAnnotation(annotationMirrors.stream(), config)
-                || handler.onOverrideTypeParameterUpperBound(baseType.tsym.toString(), i);
-        // if base type argument does not have @Nullable annotation then the instantiation is
+      if (nullableTypeArguments.containsKey(i) && !typeParamsWithNullableUpperBound[i]) {
+        // if base type variable does not have @Nullable upper bound then the instantiation is
         // invalid
-        if (!hasNullableAnnotation) {
-          reportInvalidInstantiationError(
-              nullableTypeArguments.get(i), baseType, typeVariable, state, analysis);
+        reportInvalidInstantiationError(
+            nullableTypeArguments.get(i), baseType, baseTypeArgs.get(i), state, analysis);
+      }
+    }
+  }
+
+  private static boolean[] getTypeParamsWithNullableUpperBound(
+      Type type, Config config, VisitorState state, Handler handler) {
+    Symbol.TypeSymbol tsym = type.tsym;
+    com.sun.tools.javac.util.List<Type> baseTypeArgs = tsym.type.getTypeArguments();
+    boolean[] result = new boolean[baseTypeArgs.size()];
+    for (int i = 0; i < baseTypeArgs.size(); i++) {
+      Type typeVariable = baseTypeArgs.get(i);
+      Type upperBound = typeVariable.getUpperBound();
+      com.sun.tools.javac.util.List<Attribute.TypeCompound> annotationMirrors =
+          upperBound.getAnnotationMirrors();
+      if (Nullness.hasNullableAnnotation(annotationMirrors.stream(), config)
+          || handler.onOverrideTypeParameterUpperBound(type.tsym.toString(), i)) {
+        result[i] = true;
+      }
+    }
+    // for handling bytecodes
+    com.sun.tools.javac.util.List<Attribute.TypeCompound> rawTypeAttributes =
+        tsym.getRawTypeAttributes();
+    if (rawTypeAttributes != null) {
+      for (Attribute.TypeCompound typeCompound : rawTypeAttributes) {
+        if (typeCompound.position.type.equals(TargetType.CLASS_TYPE_PARAMETER_BOUND)
+            && ASTHelpers.isSameType(
+                typeCompound.type, JSPECIFY_NULLABLE_TYPE_SUPPLIER.get(state), state)) {
+          int index = typeCompound.position.parameter_index;
+          result[index] = true;
         }
       }
     }
+    return result;
   }
 
   private static void reportInvalidInstantiationError(

nullaway/src/test/java/com/uber/nullaway/jspecify/BytecodeGenericsTests.java

@@ -0,0 +1,230 @@
+package com.uber.nullaway.jspecify;
+
+import com.google.errorprone.CompilationTestHelper;
+import com.uber.nullaway.NullAwayTestsBase;
+import java.util.Arrays;
+import org.junit.Test;
+
+public class BytecodeGenericsTests extends NullAwayTestsBase {
+
+  @Test
+  public void basicTypeParamInstantiation() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import org.jspecify.annotations.Nullable;",
+            "import com.uber.lib.generics.NonNullTypeParam;",
+            "import com.uber.lib.generics.NullableTypeParam;",
+            "class Test {",
+            "  // BUG: Diagnostic contains: Generic type parameter",
+            "  static void testBadNonNull(NonNullTypeParam<@Nullable String> t1) {",
+            "    // BUG: Diagnostic contains: Generic type parameter",
+            "    NonNullTypeParam<@Nullable String> t2 = null;",
+            "    NullableTypeParam<@Nullable String> t3 = null;",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void multipleTypeParametersInstantiation() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import org.jspecify.annotations.Nullable;",
+            "import com.uber.lib.generics.MixedTypeParam;",
+            "class Test {",
+            "  static class PartiallyInvalidSubclass",
+            "      // BUG: Diagnostic contains: Generic type parameter",
+            "      extends MixedTypeParam<@Nullable String, String, String, @Nullable String> {}",
+            "  static class ValidSubclass1",
+            "      extends MixedTypeParam<String, @Nullable String, @Nullable String, String> {}",
+            "  static class PartiallyInvalidSubclass2",
+            "      extends MixedTypeParam<",
+            "          String,",
+            "          String,",
+            "          String,",
+            "          // BUG: Diagnostic contains: Generic type parameter",
+            "          @Nullable String> {}",
+            "  static class ValidSubclass2 extends MixedTypeParam<String, String, String, String> {}",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void genericsChecksForAssignments() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import org.jspecify.annotations.Nullable;",
+            "import com.uber.lib.generics.NullableTypeParam;",
+            "class Test {",
+            "  static void testPositive(NullableTypeParam<@Nullable String> t1) {",
+            "    // BUG: Diagnostic contains: Cannot assign from type NullableTypeParam<@Nullable String>",
+            "    NullableTypeParam<String> t2 = t1;",
+            "  }",
+            "  static void testNegative(NullableTypeParam<@Nullable String> t1) {",
+            "    NullableTypeParam<@Nullable String> t2 = t1;",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void genericsChecksForFieldAssignments() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import org.jspecify.annotations.Nullable;",
+            "import com.uber.lib.generics.NullableTypeParam;",
+            "class Test {",
+            "  static void testPositive(NullableTypeParam<String> t1) {",
+            "    // BUG: Diagnostic contains: Cannot assign from type NullableTypeParam<String>",
+            "    NullableTypeParam.staticField = t1;",
+            "    // BUG: Diagnostic contains: Cannot assign from type NullableTypeParam<@Nullable String>",
+            "    NullableTypeParam<String> t2 = NullableTypeParam.staticField;",
+            "  }",
+            "  static void testNegative(NullableTypeParam<@Nullable String> t1) {",
+            "    NullableTypeParam.staticField = t1;",
+            "    NullableTypeParam<@Nullable String> t2 = NullableTypeParam.staticField;",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void genericsChecksForParamPassingAndReturns() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import org.jspecify.annotations.Nullable;",
+            "import com.uber.lib.generics.NullableTypeParam;",
+            "import com.uber.lib.generics.GenericTypeArgMethods;",
+            "class Test {",
+            "  static void testPositive(NullableTypeParam<String> t1) {",
+            "    // BUG: Diagnostic contains: Cannot pass parameter of type NullableTypeParam<String>",
+            "    GenericTypeArgMethods.nullableTypeParamArg(t1);",
+            "    // BUG: Diagnostic contains: Cannot assign from type NullableTypeParam<@Nullable String>",
+            "    NullableTypeParam<String> t2 = GenericTypeArgMethods.nullableTypeParamReturn();",
+            "  }",
+            "  static void testNegative(NullableTypeParam<@Nullable String> t1) {",
+            "    GenericTypeArgMethods.nullableTypeParamArg(t1);",
+            "    NullableTypeParam<@Nullable String> t2 = GenericTypeArgMethods.nullableTypeParamReturn();",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void overrideParameterType() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import org.jspecify.annotations.Nullable;",
+            "import com.uber.lib.generics.Fn;",
+            "class Test {",
+            " static class TestFunc1 implements Fn<@Nullable String, String> {",
+            "  @Override",
+            "  // BUG: Diagnostic contains: parameter s is",
+            "  public String apply(String s) {",
+            "   return s;",
+            "  }",
+            " }",
+            " static class TestFunc2 implements Fn<@Nullable String, String> {",
+            "  @Override",
+            "  public String apply(@Nullable String s) {",
+            "   return \"hi\";",
+            "  }",
+            " }",
+            " static class TestFunc3 implements Fn<String, String> {",
+            "  @Override",
+            "  public String apply(String s) {",
+            "   return \"hi\";",
+            "  }",
+            " }",
+            " static class TestFunc4 implements Fn<String, String> {",
+            "  // this override is legal, we should get no error",
+            "  @Override",
+            "  public String apply(@Nullable String s) {",
+            "   return \"hi\";",
+            "  }",
+            " }",
+            " static void useTestFunc(String s) {",
+            "    Fn<@Nullable String, String> f1 = new TestFunc2();",
+            "    // should get no error here",
+            "    f1.apply(null);",
+            "    Fn<String, String> f2 = new TestFunc3();",
+            "    // BUG: Diagnostic contains: passing @Nullable parameter",
+            "    f2.apply(null);",
+            " }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void overrideReturnTypes() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import org.jspecify.annotations.Nullable;",
+            "import com.uber.lib.generics.Fn;",
+            "class Test {",
+            " static class TestFunc1 implements Fn<String, @Nullable String> {",
+            "  @Override",
+            "  public @Nullable String apply(String s) {",
+            "   return s;",
+            "  }",
+            " }",
+            " static class TestFunc2 implements Fn<String, @Nullable String> {",
+            "  @Override",
+            "  public String apply(String s) {",
+            "   return s;",
+            "  }",
+            " }",
+            " static class TestFunc3 implements Fn<String, String> {",
+            "  @Override",
+            "  // BUG: Diagnostic contains: method returns @Nullable, but superclass",
+            "  public @Nullable String apply(String s) {",
+            "   return s;",
+            "  }",
+            " }",
+            " static class TestFunc4 implements Fn<@Nullable String, String> {",
+            "  @Override",
+            "  // BUG: Diagnostic contains: method returns @Nullable, but superclass",
+            "  public @Nullable String apply(String s) {",
+            "   return s;",
+            "  }",
+            " }",
+            " static void useTestFunc(String s) {",
+            "    Fn<String, @Nullable String> f1 = new TestFunc1();",
+            "    String t1 = f1.apply(s);",
+            "    // BUG: Diagnostic contains: dereferenced expression",
+            "    t1.hashCode();",
+            "    TestFunc2 f2 = new TestFunc2();",
+            "    String t2 = f2.apply(s);",
+            "    // There should not be an error here",
+            "    t2.hashCode();",
+            "    Fn<String, @Nullable String> f3 = new TestFunc2();",
+            "    String t3 = f3.apply(s);",
+            "    // BUG: Diagnostic contains: dereferenced expression",
+            "    t3.hashCode();",
+            "    // BUG: Diagnostic contains: dereferenced expression",
+            "    f3.apply(s).hashCode();",
+            " }",
+            "}")
+        .doTest();
+  }
+
+  private CompilationTestHelper makeHelper() {
+    return makeTestHelperWithArgs(
+        Arrays.asList(
+            "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:JSpecifyMode=true"));
+  }
+}

test-java-lib/src/main/java/com/uber/lib/generics/Fn.java

@@ -0,0 +1,7 @@
+package com.uber.lib.generics;
+
+import org.jspecify.annotations.Nullable;
+
+public interface Fn<P extends @Nullable Object, R extends @Nullable Object> {
+  R apply(P p);
+}

test-java-lib/src/main/java/com/uber/lib/generics/GenericTypeArgMethods.java

@@ -0,0 +1,12 @@
+package com.uber.lib.generics;
+
+import org.jspecify.annotations.Nullable;
+
+public class GenericTypeArgMethods {
+
+  public static void nullableTypeParamArg(NullableTypeParam<@Nullable String> s) {}
+
+  public static NullableTypeParam<@Nullable String> nullableTypeParamReturn() {
+    return new NullableTypeParam<@Nullable String>();
+  }
+}

test-java-lib/src/main/java/com/uber/lib/generics/MixedTypeParam.java

@@ -0,0 +1,5 @@
+package com.uber.lib.generics;
+
+import org.jspecify.annotations.Nullable;
+
+public class MixedTypeParam<E1, E2 extends @Nullable Object, E3 extends @Nullable Object, E4> {}

test-java-lib/src/main/java/com/uber/lib/generics/NonNullTypeParam.java

@@ -0,0 +1,3 @@
+package com.uber.lib.generics;
+
+public class NonNullTypeParam<E> {}

test-java-lib/src/main/java/com/uber/lib/generics/NullableTypeParam.java

@@ -0,0 +1,9 @@
+package com.uber.lib.generics;
+
+import org.jspecify.annotations.Nullable;
+
+public class NullableTypeParam<E extends @Nullable Object> {
+
+  public static NullableTypeParam<@Nullable String> staticField =
+      new NullableTypeParam<@Nullable String>();
+}