Fix bug with computing direct type use annotations on parameters (#864)

NullAway was still treating annotations on generic type arguments as
being on the top-level type of a parameter itself, which could lead to
false positives and potentially also missed bugs.

5 files changed, 46 insertions, 6 deletions

diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle
index 1af2c45..566b5cf 100755
--- a/gradle/dependencies.gradle
+++ b/gradle/dependencies.gradle
@@ -102,7 +102,7 @@ def test = [
         "org.junit.jupiter:junit-jupiter-api:5.0.2",
         "org.apiguardian:apiguardian-api:1.0.0"
     ],
-    jetbrainsAnnotations    : "org.jetbrains:annotations:13.0",
+    jetbrainsAnnotations    : "org.jetbrains:annotations:24.1.0",
     cfQual                  : "org.checkerframework:checker-qual:${versions.checkerFramework}",
     // 2.5.5 is the last release to contain this artifact
     cfCompatQual            : "org.checkerframework:checker-compat-qual:2.5.5",
diff --git a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java
index 2a04d46..cca114b 100644
--- a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java
+++ b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java
@@ -175,7 +175,8 @@ public class NullabilityUtil {
 
   /**
    * NOTE: this method does not work for getting all annotations of parameters of methods from class
-   * files. For that case, use {@link #getAllAnnotationsForParameter(Symbol.MethodSymbol, int)}
+   * files. For that case, use {@link #getAllAnnotationsForParameter(Symbol.MethodSymbol, int,
+   * Config)}
    *
    * @param symbol the symbol
    * @return all annotations on the symbol and on the type of the symbol
@@ -259,10 +260,11 @@ public class NullabilityUtil {
    *
    * @param symbol the method symbol
    * @param paramInd index of the parameter
+   * @param config NullAway configuration
    * @return all declaration and type-use annotations for the parameter
    */
   public static Stream<? extends AnnotationMirror> getAllAnnotationsForParameter(
-      Symbol.MethodSymbol symbol, int paramInd) {
+      Symbol.MethodSymbol symbol, int paramInd, Config config) {
     Symbol.VarSymbol varSymbol = symbol.getParameters().get(paramInd);
     return Stream.concat(
         varSymbol.getAnnotationMirrors().stream(),
@@ -270,7 +272,8 @@ public class NullabilityUtil {
             .filter(
                 t ->
                     t.position.type.equals(TargetType.METHOD_FORMAL_PARAMETER)
-                        && t.position.parameter_index == paramInd));
+                        && t.position.parameter_index == paramInd
+                        && NullabilityUtil.isDirectTypeUseAnnotation(t, config)));
   }
 
   /**
diff --git a/nullaway/src/main/java/com/uber/nullaway/Nullness.java b/nullaway/src/main/java/com/uber/nullaway/Nullness.java
index 845d547..0ed694b 100644
--- a/nullaway/src/main/java/com/uber/nullaway/Nullness.java
+++ b/nullaway/src/main/java/com/uber/nullaway/Nullness.java
@@ -218,7 +218,7 @@ public enum Nullness implements AbstractValue<Nullness> {
       return true;
     }
     return hasNullableAnnotation(
-        NullabilityUtil.getAllAnnotationsForParameter(symbol, paramInd), config);
+        NullabilityUtil.getAllAnnotationsForParameter(symbol, paramInd, config), config);
   }
 
   private static boolean isRecordEqualsParam(Symbol.MethodSymbol symbol, int paramInd) {
@@ -249,6 +249,6 @@ public enum Nullness implements AbstractValue<Nullness> {
   public static boolean paramHasNonNullAnnotation(
       Symbol.MethodSymbol symbol, int paramInd, Config config) {
     return hasNonNullAnnotation(
-        NullabilityUtil.getAllAnnotationsForParameter(symbol, paramInd), config);
+        NullabilityUtil.getAllAnnotationsForParameter(symbol, paramInd, config), config);
   }
 }
diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyTests.java
index ed9acb1..246d7e8 100644
--- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyTests.java
+++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyTests.java
@@ -988,6 +988,31 @@ public class NullAwayJSpecifyTests extends NullAwayTestsBase {
   }
 
   @Test
+  public void nullUnmarkedRestrictiveAnnotationsAndGenerics() {
+    makeTestHelperWithArgs(
+            Arrays.asList(
+                "-d",
+                temporaryFolder.getRoot().getAbsolutePath(),
+                "-XepOpt:NullAway:AnnotatedPackages=com.uber",
+                "-XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true"))
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import org.jspecify.annotations.NullUnmarked;",
+            "import org.jetbrains.annotations.Nullable;",
+            "import org.jetbrains.annotations.NotNull;",
+            "import java.util.List;",
+            "public class Test {",
+            "  @NullUnmarked",
+            "  public static void takesNullable(@Nullable List<@NotNull String> l) {}",
+            "  public static void test() {",
+            "    takesNullable(null);",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
   public void nullMarkedStaticImports() {
     makeTestHelperWithArgs(
             Arrays.asList(
diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayTypeUseAnnotationsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayTypeUseAnnotationsTests.java
index 7af8b25..5e13b02 100644
--- a/nullaway/src/test/java/com/uber/nullaway/NullAwayTypeUseAnnotationsTests.java
+++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayTypeUseAnnotationsTests.java
@@ -38,16 +38,28 @@ public class NullAwayTypeUseAnnotationsTests extends NullAwayTestsBase {
             "import java.util.List;",
             "import java.util.ArrayList;",
             "import org.checkerframework.checker.nullness.qual.Nullable;",
+            "import org.checkerframework.checker.nullness.qual.NonNull;",
             "class TypeArgumentAnnotation {",
             "  List<@Nullable String> fSafe = new ArrayList<>();",
             "  @Nullable List<String> fUnsafe = new ArrayList<>();",
             "  void useParamSafe(List<@Nullable String> list) {",
             "    list.hashCode();",
             "  }",
+            "  void unsafeCall() {",
+            "    // BUG: Diagnostic contains: passing @Nullable parameter",
+            "    useParamSafe(null);",
+            "  }",
             "  void useParamUnsafe(@Nullable List<String> list) {",
             "    // BUG: Diagnostic contains: dereferenced",
             "    list.hashCode();",
             "  }",
+            "  void useParamUnsafeNonNullElements(@Nullable List<@NonNull String> list) {",
+            "    // BUG: Diagnostic contains: dereferenced",
+            "    list.hashCode();",
+            "  }",
+            "  void safeCall() {",
+            "    useParamUnsafeNonNullElements(null);",
+            "  }",
             "  void useFieldSafe() {",
             "    fSafe.hashCode();",
             "  }",