Clarifications and small fixes for checking JSpecify @Nullable annotation (#859)

Rename a variable and add docs to clarify that in certain places, our
JSpecify support specifically checks for
`@org.jspecify.annotations.Nullable` annotations and not others. Also,
fix a couple of places where we were comparing types by their `String`
representation.

4 files changed, 34 insertions, 7 deletions

diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/CompareNullabilityVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/CompareNullabilityVisitor.java
index bd34553..760d200 100644
--- a/nullaway/src/main/java/com/uber/nullaway/generics/CompareNullabilityVisitor.java
+++ b/nullaway/src/main/java/com/uber/nullaway/generics/CompareNullabilityVisitor.java
@@ -1,6 +1,7 @@
 package com.uber.nullaway.generics;
 
 import com.google.errorprone.VisitorState;
+import com.google.errorprone.util.ASTHelpers;
 import com.sun.tools.javac.code.Attribute;
 import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.code.Types;
@@ -43,17 +44,19 @@ public class CompareNullabilityVisitor extends Types.DefaultTypeVisitor<Boolean,
       boolean isLHSNullableAnnotated = false;
       List<Attribute.TypeCompound> lhsAnnotations = lhsTypeArgument.getAnnotationMirrors();
       // To ensure that we are checking only jspecify nullable annotations
+      Type jspecifyNullableType = GenericsChecks.JSPECIFY_NULLABLE_TYPE_SUPPLIER.get(state);
       for (Attribute.TypeCompound annotation : lhsAnnotations) {
-        if (annotation.getAnnotationType().toString().equals(GenericsChecks.NULLABLE_NAME)) {
+        if (ASTHelpers.isSameType(
+            (Type) annotation.getAnnotationType(), jspecifyNullableType, state)) {
           isLHSNullableAnnotated = true;
           break;
         }
       }
       boolean isRHSNullableAnnotated = false;
       List<Attribute.TypeCompound> rhsAnnotations = rhsTypeArgument.getAnnotationMirrors();
-      // To ensure that we are checking only jspecify nullable annotations
       for (Attribute.TypeCompound annotation : rhsAnnotations) {
-        if (annotation.getAnnotationType().toString().equals(GenericsChecks.NULLABLE_NAME)) {
+        if (ASTHelpers.isSameType(
+            (Type) annotation.getAnnotationType(), jspecifyNullableType, state)) {
           isRHSNullableAnnotated = true;
           break;
         }
diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java
index 37a56f6..247988b 100644
--- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java
+++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java
@@ -38,9 +38,13 @@ import javax.lang.model.type.ExecutableType;
 /** Methods for performing checks related to generic types and nullability. */
 public final class GenericsChecks {
 
-  static final String NULLABLE_NAME = "org.jspecify.annotations.Nullable";
-
-  static final Supplier<Type> NULLABLE_TYPE_SUPPLIER = Suppliers.typeFromString(NULLABLE_NAME);
+  /**
+   * Supplier for the JSpecify {@code @Nullable} annotation. Required since for now, certain checks
+   * related to generics specifically look for {@code @org.jspecify.ananotations.Nullable}
+   * annotations and do not apply to other {@code @Nullable} annotations.
+   */
+  static final Supplier<Type> JSPECIFY_NULLABLE_TYPE_SUPPLIER =
+      Suppliers.typeFromString("org.jspecify.annotations.Nullable");
 
   /** Do not instantiate; all methods should be static */
   private GenericsChecks() {}
diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java
index e5971d5..2cde64f 100644
--- a/nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java
+++ b/nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java
@@ -42,7 +42,7 @@ public class PreservedAnnotationTreeVisitor extends SimpleTreeVisitor<Type, Void
   public Type visitParameterizedType(ParameterizedTypeTree tree, Void p) {
     Type.ClassType type = (Type.ClassType) ASTHelpers.getType(tree);
     Preconditions.checkNotNull(type);
-    Type nullableType = GenericsChecks.NULLABLE_TYPE_SUPPLIER.get(state);
+    Type nullableType = GenericsChecks.JSPECIFY_NULLABLE_TYPE_SUPPLIER.get(state);
     List<? extends Tree> typeArguments = tree.getTypeArguments();
     List<Type> newTypeArgs = new ArrayList<>();
     for (int i = 0; i < typeArguments.size(); i++) {
diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java
index eb7c02f..68f1d11 100644
--- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java
+++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java
@@ -292,6 +292,26 @@ public class NullAwayJSpecifyGenericsTests extends NullAwayTestsBase {
   }
 
   @Test
+  public void genericsChecksForAssignmentsWithNonJSpecifyAnnotations() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import org.checkerframework.checker.nullness.qual.Nullable;",
+            "class Test {",
+            "  static class NullableTypeParam<E extends @Nullable Object> {}",
+            "  static void testNoWarningForMismatch(NullableTypeParam<@Nullable String> t1) {",
+            "    // no error here since we only do our checks for JSpecify @Nullable annotations",
+            "    NullableTypeParam<String> t2 = t1;",
+            "  }",
+            "  static void testNegative(NullableTypeParam<@Nullable String> t1) {",
+            "    NullableTypeParam<@Nullable String> t2 = t1;",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
   public void nestedChecksForAssignmentsMultipleArguments() {
     makeHelper()
         .addSourceLines(