JSpecify: handle varargs whose element type is a type variable (#1200)

Partially addresses #1199. After substituting for a type variable, we
were not retrieving the correct nullability for a varargs parameter,
which should be the nullability of the component, not the array type
itself.

Also adds some simplification of our handling of varargs calls based on
a tip from @jeffrey-easyesi

guava-recent-unit-tests/build.gradle

@@ -33,7 +33,7 @@ dependencies {
         exclude group: "junit", module: "junit"
     }
     testImplementation deps.test.jsr305Annotations
-    testImplementation "com.google.guava:guava:33.4.5-jre"
+    testImplementation "com.google.guava:guava:33.4.8-jre"
 
     errorProneOldest deps.build.errorProneCheckApiOld
     errorProneOldest(deps.build.errorProneTestHelpersOld) {

guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java

@@ -45,6 +45,13 @@ public void setup() {
                 Arrays.asList(
                     "-d",
                     temporaryFolder.getRoot().getAbsolutePath(),
+                    // Since Guava is now @NullMarked we shouldn't need com.google.common in the
+                    // annotated packages list.
+                    // We still need it here since we are still trying to support
+                    // Error Prone 2.14.0, on which reading @NullMarked from the package-info.java
+                    // file isn't working using ASTHelpers for some reason.
+                    // TODO Once we bump our minimum Error Prone version, remove com.google.common
+                    //  from this list.
                     "-XepOpt:NullAway:AnnotatedPackages=com.uber,com.google.common",
                     "-XepOpt:NullAway:UnannotatedSubPackages=com.uber.nullaway.[a-zA-Z0-9.]+.unannotated"));
     jspecifyCompilationHelper =
@@ -241,4 +248,23 @@ public void testFunctionMethodOverride() {
             "}")
         .doTest();
   }
+
+  @Test
+  public void newHashSetPassingNullable() {
+    // to ensure javac reads proper generic types from the Guava jar
+    Assume.assumeTrue(Runtime.version().feature() >= 23);
+    jspecifyCompilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import com.google.common.collect.Sets;",
+            "import java.util.Set;",
+            "import org.jspecify.annotations.*;",
+            "@NullMarked",
+            "class Test {",
+            "  public static void test(@Nullable String s) {",
+            "    Set<@Nullable String> params = Sets.newHashSet(s);",
+            "  }",
+            "}")
+        .doTest();
+  }
 }

nullaway/src/main/java/com/uber/nullaway/NullAway.java

@@ -1941,13 +1941,25 @@ private Description handleInvocation(
           continue;
         }
         actual = actualParams.get(argPos);
-        // check if the varargs arguments are being passed as an array
         VarSymbol formalParamSymbol = formalParams.get(formalParams.size() - 1);
-        Type.ArrayType varargsArrayType = (Type.ArrayType) formalParamSymbol.type;
-        Type actualParameterType = ASTHelpers.getType(actual);
-        if (actualParameterType != null
-            && state.getTypes().isAssignable(actualParameterType, varargsArrayType)
-            && actualParams.size() == argPos + 1) {
+        boolean isVarArgsCall = isVarArgsCall(tree);
+        if (isVarArgsCall) {
+          // This is the case were varargs are being passed individually, as 1 or more actual
+          // arguments starting at the position of the var args formal.
+          // If the formal var args accepts `@Nullable`, then there is nothing for us to check.
+          if (!argIsNonNull) {
+            continue;
+          }
+          // TODO report all varargs errors in a single build; this code only reports the first
+          //  error
+          for (ExpressionTree arg : actualParams.subList(argPos, actualParams.size())) {
+            actual = arg;
+            mayActualBeNull = mayBeNullExpr(state, actual);
+            if (mayActualBeNull) {
+              break;
+            }
+          }
+        } else {
           // This is the case where an array is explicitly passed in the position of the var args
           // parameter
           // Only check for a nullable varargs array if the method is annotated, or a @NonNull
@@ -1964,22 +1976,6 @@ private Description handleInvocation(
               mayActualBeNull = mayBeNullExpr(state, actual);
             }
           }
-        } else {
-          // This is the case were varargs are being passed individually, as 1 or more actual
-          // arguments starting at the position of the var args formal.
-          // If the formal var args accepts `@Nullable`, then there is nothing for us to check.
-          if (!argIsNonNull) {
-            continue;
-          }
-          // TODO report all varargs errors in a single build; this code only reports the first
-          //  error
-          for (ExpressionTree arg : actualParams.subList(argPos, actualParams.size())) {
-            actual = arg;
-            mayActualBeNull = mayBeNullExpr(state, actual);
-            if (mayActualBeNull) {
-              break;
-            }
-          }
         }
       } else { // not the vararg position
         actual = actualParams.get(argPos);
@@ -2000,6 +1996,23 @@ private Description handleInvocation(
     return checkCastToNonNullTakesNullable(tree, state, methodSymbol, actualParams);
   }
 
+  /**
+   * Checks if the method invocation is a varargs call, i.e., if individual arguments are being
+   * passed in the varargs position. If false, it means that an array is being passed in the varargs
+   * position.
+   *
+   * @param tree the method invocation tree (MethodInvocationTree or NewClassTree)
+   * @return true if the method invocation is a varargs call, false otherwise
+   */
+  private boolean isVarArgsCall(Tree tree) {
+    // javac sets the varargsElement field to a non-null value if the invocation is a varargs call
+    Type varargsElement =
+        tree instanceof JCTree.JCMethodInvocation
+            ? ((JCTree.JCMethodInvocation) tree).varargsElement
+            : ((JCTree.JCNewClass) tree).varargsElement;
+    return varargsElement != null;
+  }
+
   private Description checkCastToNonNullTakesNullable(
       Tree tree,
       VisitorState state,

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

@@ -1015,6 +1015,9 @@ public Nullness getGenericParameterNullnessAtInvocation(
       Tree tree,
       VisitorState state,
       Config config) {
+    boolean isVarargsParam =
+        invokedMethodSymbol.isVarArgs()
+            && paramIndex == invokedMethodSymbol.getParameters().size() - 1;
     // If generic method invocation
     if (!invokedMethodSymbol.getTypeParameters().isEmpty()) {
       // Substitute the argument types within the MethodType
@@ -1025,7 +1028,9 @@ public Nullness getGenericParameterNullnessAtInvocation(
       // type variables declared on the enclosing class
       if (substitutedParamTypes != null
           && Objects.equals(
-              getTypeNullness(substitutedParamTypes.get(paramIndex), config), Nullness.NULLABLE)) {
+              getParameterTypeNullness(
+                  substitutedParamTypes.get(paramIndex), config, isVarargsParam),
+              Nullness.NULLABLE)) {
         return Nullness.NULLABLE;
       }
     }
@@ -1121,10 +1126,13 @@ public static Nullness getGenericMethodParameterNullness(
       // @Nullable annotation is handled elsewhere)
       return Nullness.NONNULL;
     }
+    boolean isVarargsParam =
+        method.isVarArgs() && parameterIndex == method.getParameters().size() - 1;
+
     Type methodType =
         TypeSubstitutionUtils.memberType(state.getTypes(), enclosingType, method, config);
     Type paramType = methodType.getParameterTypes().get(parameterIndex);
-    return getTypeNullness(paramType, config);
+    return getParameterTypeNullness(paramType, config, isVarargsParam);
   }
 
   /**
@@ -1187,6 +1195,33 @@ private static void checkTypeParameterNullnessForOverridingMethodReturnType(
     }
   }
 
+  /**
+   * Returns the nullness of a formal parameter type, based on the nullability annotations on the
+   * type.
+   *
+   * @param type The type of the parameter
+   * @param config The analysis config
+   * @param isVarargsParam true if the parameter is a varargs parameter
+   * @return The nullness of the parameter type
+   */
+  private static Nullness getParameterTypeNullness(
+      Type type, Config config, boolean isVarargsParam) {
+    if (isVarargsParam) {
+      // type better be an array type
+      verify(
+          type.getKind().equals(TypeKind.ARRAY),
+          "expected array type for varargs parameter, got %s",
+          type);
+      // use the component type to determine nullness
+      Type.ArrayType arrayType = (Type.ArrayType) type;
+      Type componentType = arrayType.getComponentType();
+      return getTypeNullness(componentType, config);
+    } else {
+      // For non-varargs, we just check the type itself
+      return getTypeNullness(type, config);
+    }
+  }
+
   /**
    * @param type A type for which we need the Nullness.
    * @param config The analysis config

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

@@ -526,6 +526,78 @@ public void issue1178() {
         .doTest();
   }
 
+  @Test
+  public void varargsOfGenericType() {
+    makeHelper()
+        .addSourceLines(
+            "Varargs.java",
+            "import org.jspecify.annotations.NullMarked;",
+            "import org.jspecify.annotations.Nullable;",
+            "@NullMarked",
+            "public class Varargs {",
+            "    static <T extends @Nullable Object> void foo(T... args) {",
+            "    }",
+            "    static void testNegative(@Nullable String s) {",
+            "        Varargs.<@Nullable String>foo(s);",
+            "    }",
+            "    static void testPositive(@Nullable String s) {",
+            "        // BUG: Diagnostic contains: passing @Nullable parameter",
+            "        Varargs.<String>foo(s);",
+            "    }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void nullableVarargsArray() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            "import org.jspecify.annotations.Nullable;",
+            "import org.jspecify.annotations.NullMarked;",
+            "@NullMarked",
+            "class Test {",
+            "    <T extends Object> void varargsTest(T @Nullable... args) {}",
+            "    void f() {",
+            "        String[] x = null;",
+            "        this.<String>varargsTest(x);",
+            "        this.<String>varargsTest((String[])null);",
+            "    }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void varargsConstructor() {
+    makeHelper()
+        .addSourceLines(
+            "Test.java",
+            "import org.jspecify.annotations.Nullable;",
+            "import org.jspecify.annotations.NullMarked;",
+            "@NullMarked",
+            "class Test {",
+            "    static class Foo {",
+            "      <T> Foo(T @Nullable... args) {}",
+            "    }",
+            "    void testNegative() {",
+            "        String[] x = null;",
+            "        Foo f = new <String>Foo(x);",
+            "        f = new <String>Foo((String[])null);",
+            "    }",
+            "    static class Bar {",
+            "      <T> Bar(T... args) {}",
+            "    }",
+            "    void testPositive() {",
+            "        String[] x = null;",
+            "        // BUG: Diagnostic contains: passing @Nullable parameter",
+            "        Bar b = new <String>Bar(x);",
+            "        // BUG: Diagnostic contains: passing @Nullable parameter",
+            "        b = new <String>Bar((String[])null);",
+            "    }",
+            "}")
+        .doTest();
+  }
+
   private CompilationTestHelper makeHelper() {
     return makeTestHelperWithArgs(
         Arrays.asList(

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

@@ -2393,6 +2393,32 @@ public void issue1156() {
         .doTest();
   }
 
+  @Test
+  public void varargsOfGenericType() {
+    makeHelper()
+        .addSourceLines(
+            "Varargs.java",
+            "import org.jspecify.annotations.NullMarked;",
+            "import org.jspecify.annotations.Nullable;",
+            "@NullMarked",
+            "public class Varargs {",
+            "    static class Foo<T extends @Nullable Object> {",
+            "        void foo(T... args) {",
+            "        }",
+            "    }",
+            "    static void testNegative(@Nullable String s) {",
+            "        Foo<@Nullable String> f = new Foo<>();",
+            "        f.foo(s);",
+            "    }",
+            "    static void testPositive(@Nullable String s) {",
+            "        Foo<String> f = new Foo<>();",
+            "        // BUG: Diagnostic contains: passing @Nullable parameter",
+            "        f.foo(s);",
+            "    }",
+            "}")
+        .doTest();
+  }
+
   private CompilationTestHelper makeHelper() {
     return makeTestHelperWithArgs(
         Arrays.asList(