JSpecify: changes for issue #861 (#863)

Removing unnecessary checks performed for ClassType in
GenericChecks.java in reference to issue #861. Added some null checks
since some of the checks for ClassType were also indirectly checking for
nullability and hence removing them entirely cause the test rawTypes to
crash.

Fixes #861

---------

Co-authored-by: Manu Sridharan <msridhar@gmail.com>

3 files changed, 76 insertions, 18 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 025c3ee..de86547 100644
--- a/nullaway/src/main/java/com/uber/nullaway/generics/CompareNullabilityVisitor.java
+++ b/nullaway/src/main/java/com/uber/nullaway/generics/CompareNullabilityVisitor.java
@@ -6,6 +6,7 @@ import com.sun.tools.javac.code.Attribute;
 import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.code.Types;
 import java.util.List;
+import javax.lang.model.type.NullType;
 
 /**
  * Visitor that checks equality of nullability annotations for all nested generic type arguments
@@ -21,22 +22,25 @@ public class CompareNullabilityVisitor extends Types.DefaultTypeVisitor<Boolean,
 
   @Override
   public Boolean visitClassType(Type.ClassType lhsType, Type rhsType) {
+    if (rhsType instanceof NullType) {
+      return true;
+    }
     Types types = state.getTypes();
     // The base type of rhsType may be a subtype of lhsType's base type.  In such cases, we must
     // compare lhsType against the supertype of rhsType with a matching base type.
-    rhsType = types.asSuper(rhsType, lhsType.tsym);
+    Type rhsTypeAsSuper = types.asSuper(rhsType, lhsType.tsym);
     // This is impossible, considering the fact that standard Java subtyping succeeds before
     // running NullAway
-    if (rhsType == null) {
+    if (rhsTypeAsSuper == null) {
       throw new RuntimeException("Did not find supertype of " + rhsType + " matching " + lhsType);
     }
     List<Type> lhsTypeArguments = lhsType.getTypeArguments();
-    List<Type> rhsTypeArguments = rhsType.getTypeArguments();
+    List<Type> rhsTypeArguments = rhsTypeAsSuper.getTypeArguments();
     // This is impossible, considering the fact that standard Java subtyping succeeds before
     // running NullAway
     if (lhsTypeArguments.size() != rhsTypeArguments.size()) {
       throw new RuntimeException(
-          "Number of types arguments in " + rhsType + " does not match " + lhsType);
+          "Number of types arguments in " + rhsTypeAsSuper + " does not match " + lhsType);
     }
     for (int i = 0; i < lhsTypeArguments.size(); i++) {
       Type lhsTypeArgument = lhsTypeArguments.get(i);
@@ -77,6 +81,9 @@ public class CompareNullabilityVisitor extends Types.DefaultTypeVisitor<Boolean,
 
   @Override
   public Boolean visitArrayType(Type.ArrayType lhsType, Type rhsType) {
+    if (rhsType instanceof NullType) {
+      return true;
+    }
     Type.ArrayType arrRhsType = (Type.ArrayType) rhsType;
     return lhsType.getComponentType().accept(this, arrRhsType.getComponentType());
   }
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 23affc4..77f185e 100644
--- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java
+++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java
@@ -301,7 +301,7 @@ public final class GenericsChecks {
     Type lhsType = getTreeType(lhsTree, state);
     Type rhsType = getTreeType(rhsTree, state);
 
-    if (lhsType instanceof Type.ClassType && rhsType instanceof Type.ClassType) {
+    if (lhsType != null && rhsType != null) {
       boolean isAssignmentValid = compareNullabilityAnnotations(lhsType, rhsType, state);
       if (!isAssignmentValid) {
         reportInvalidAssignmentInstantiationError(tree, lhsType, rhsType, state, analysis);
@@ -333,8 +333,7 @@ public final class GenericsChecks {
       return;
     }
     Type returnExpressionType = getTreeType(retExpr, state);
-    if (formalReturnType instanceof Type.ClassType
-        && returnExpressionType instanceof Type.ClassType) {
+    if (formalReturnType != null && returnExpressionType != null) {
       boolean isReturnTypeValid =
           compareNullabilityAnnotations(formalReturnType, returnExpressionType, state);
       if (!isReturnTypeValid) {
@@ -407,14 +406,14 @@ public final class GenericsChecks {
     // The condExpr type should be the least-upper bound of the true and false part types.  To check
     // the nullability annotations, we check that the true and false parts are assignable to the
     // type of the whole expression
-    if (condExprType instanceof Type.ClassType) {
-      if (truePartType instanceof Type.ClassType) {
+    if (condExprType != null) {
+      if (truePartType != null) {
         if (!compareNullabilityAnnotations(condExprType, truePartType, state)) {
           reportMismatchedTypeForTernaryOperator(
               truePartTree, condExprType, truePartType, state, analysis);
         }
       }
-      if (falsePartType instanceof Type.ClassType) {
+      if (falsePartType != null) {
         if (!compareNullabilityAnnotations(condExprType, falsePartType, state)) {
           reportMismatchedTypeForTernaryOperator(
               falsePartTree, condExprType, falsePartType, state, analysis);
@@ -452,8 +451,7 @@ public final class GenericsChecks {
       Type formalParameter = formalParams.get(i).type;
       if (!formalParameter.getTypeArguments().isEmpty()) {
         Type actualParameter = getTreeType(actualParams.get(i), state);
-        if (formalParameter instanceof Type.ClassType
-            && actualParameter instanceof Type.ClassType) {
+        if (actualParameter != null) {
           if (!compareNullabilityAnnotations(formalParameter, actualParameter, state)) {
             reportInvalidParametersNullabilityError(
                 formalParameter, actualParameter, actualParams.get(i), state, analysis);
@@ -468,8 +466,7 @@ public final class GenericsChecks {
       if (!varargsElementType.getTypeArguments().isEmpty()) {
         for (int i = formalParams.size() - 1; i < actualParams.size(); i++) {
           Type actualParameter = getTreeType(actualParams.get(i), state);
-          if (varargsElementType instanceof Type.ClassType
-              && actualParameter instanceof Type.ClassType) {
+          if (actualParameter != null) {
             if (!compareNullabilityAnnotations(varargsElementType, actualParameter, state)) {
               reportInvalidParametersNullabilityError(
                   varargsElementType, actualParameter, actualParams.get(i), state, analysis);
@@ -777,8 +774,7 @@ public final class GenericsChecks {
     for (int i = 0; i < methodParameters.size(); i++) {
       Type overridingMethodParameterType = ASTHelpers.getType(methodParameters.get(i));
       Type overriddenMethodParameterType = overriddenMethodParameterTypes.get(i);
-      if (overriddenMethodParameterType instanceof Type.ClassType
-          && overridingMethodParameterType instanceof Type.ClassType) {
+      if (overriddenMethodParameterType != null && overridingMethodParameterType != null) {
         if (!compareNullabilityAnnotations(
             overriddenMethodParameterType, overridingMethodParameterType, state)) {
           reportInvalidOverridingMethodParamTypeError(
@@ -806,10 +802,10 @@ public final class GenericsChecks {
       MethodTree tree, Type overriddenMethodType, NullAway analysis, VisitorState state) {
     Type overriddenMethodReturnType = overriddenMethodType.getReturnType();
     Type overridingMethodReturnType = ASTHelpers.getType(tree.getReturnType());
-    if (!(overriddenMethodReturnType instanceof Type.ClassType)) {
+    if (overriddenMethodReturnType == null) {
       return;
     }
-    Preconditions.checkArgument(overridingMethodReturnType instanceof Type.ClassType);
+    Preconditions.checkArgument(overridingMethodReturnType != null);
     if (!compareNullabilityAnnotations(
         overriddenMethodReturnType, overridingMethodReturnType, state)) {
       reportInvalidOverridingMethodReturnTypeError(
diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java
index 68f1d11..8e3452e 100644
--- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java
+++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java
@@ -1021,6 +1021,23 @@ public class NullAwayJSpecifyGenericsTests extends NullAwayTestsBase {
   }
 
   @Test
+  public void nestedGenericTypeAssignment2() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import org.jspecify.annotations.Nullable;",
+            "class Test {",
+            "  static class A<T extends @Nullable Object> { }",
+            "  static void testPositive() {",
+            "    // BUG: Diagnostic contains: Cannot assign from type",
+            "    A<A<String>[]> var2 = new A<A<@Nullable String>[]>();",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
   public void genericPrimitiveArrayTypeAssignment() {
     makeHelper()
         .addSourceLines(
@@ -1560,6 +1577,44 @@ public class NullAwayJSpecifyGenericsTests extends NullAwayTestsBase {
         .doTest();
   }
 
+  @Test
+  public void testForNullRhsTypeWhenReturnedForGenericType() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import org.jspecify.annotations.Nullable;",
+            "class Test {",
+            "  static class A<T extends @Nullable Object> { }",
+            "  static A<String> testPositive() {",
+            "   // BUG: Diagnostic contains: returning @Nullable expression from method with @NonNull return type",
+            "   return null;",
+            "  }",
+            "  static @Nullable A<String> testNegative() {",
+            "   return null;",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void testForNullTypeRhsTypeForArrayType() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import org.jspecify.annotations.Nullable;",
+            "import java.util.List;",
+            "import java.util.ArrayList;",
+            "class Test {",
+            "  static void testNegative() {",
+            "   List<String> a = new ArrayList<String>();",
+            "   Object[] o = a != null ? a.toArray() : null;",
+            "  }",
+            "}")
+        .doTest();
+  }
+
   private CompilationTestHelper makeHelper() {
     return makeTestHelperWithArgs(
         Arrays.asList(