Add support for parameter types with wildcards for JarInfer (#1107)

uber · Dec 22, 2024 · f2bafb2 · f2bafb2
1 parent 2754c45
commit f2bafb2
Show file tree

Hide file tree

Showing 5 changed files with 98 additions and 7 deletions.
diff --git a/...jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java b/...jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java
@@ -566,6 +566,9 @@ private static String getSourceLevelQualifiedTypeName(TypeReference typ) {
    * @return source-level qualified type name.
    */
   private static String getSourceLevelQualifiedTypeName(String typeName) {
+    if (isWildcard(typeName)) {
+      return sourceLevelWildcardType(typeName);
+    }
     if (!typeName.endsWith(";")) {
       // we need the semicolon since some of WALA's TypeSignature APIs expect it
       typeName = typeName + ";";
@@ -598,4 +601,22 @@ private static String getSourceLevelQualifiedTypeName(String typeName) {
           + ">";
     }
   }
+
+  private static boolean isWildcard(String typeName) {
+    char firstChar = typeName.charAt(0);
+    return firstChar == '*' || firstChar == '+' || firstChar == '-';
+  }
+
+  private static String sourceLevelWildcardType(String typeName) {
+    switch (typeName.charAt(0)) {
+      case '*':
+        return "?";
+      case '+':
+        return "? extends " + getSourceLevelQualifiedTypeName(typeName.substring(1));
+      case '-':
+        return "? super " + getSourceLevelQualifiedTypeName(typeName.substring(1));
+      default:
+        throw new RuntimeException("unexpected wildcard type name" + typeName);
+    }
+  }
 }
diff --git a/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java b/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java
@@ -481,6 +481,33 @@ public void testMethodWithGenericParameter() throws Exception {
         "}");
   }
 
+  @Test
+  public void wildcards() throws Exception {
+    testTemplate(
+        "wildcards",
+        "generic",
+        "TestGeneric",
+        ImmutableMap.of(
+            "generic.TestGeneric:void genericWildcardLower(generic.TestGeneric.Generic<? super java.lang.String>)",
+            Sets.newHashSet(0),
+            "generic.TestGeneric:void genericWildcard(generic.TestGeneric.Generic<?>)",
+            Sets.newHashSet(0),
+            "generic.TestGeneric:java.lang.String genericWildcardUpper(generic.TestGeneric.Generic<? extends java.lang.String>)",
+            Sets.newHashSet(0)),
+        "public class TestGeneric {",
+        "  public abstract static class Generic<T> {",
+        "    public String getString(T t) {",
+        "      return \"t\";",
+        "    }",
+        "    public void doNothing() {}",
+        "    public abstract T getSomething();",
+        "  }",
+        "  public static void genericWildcard(Generic<?> g) { g.doNothing(); };",
+        "  public static String genericWildcardUpper(Generic<? extends String> g) { return g.getSomething(); };",
+        "  public static void genericWildcardLower(Generic<? super String> g) { g.getString(\"hello\"); };",
+        "}");
+  }
+
   @Test
   public void toyJARAnnotatingClasses() throws Exception {
     testAnnotationInJarTemplate(

diff --git a/...ay-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java b/...ay-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java
@@ -84,8 +84,7 @@ public void genericsTest() {
             "import org.jspecify.annotations.Nullable;",
             "import com.uber.nullaway.jarinfer.toys.unannotated.Toys;",
             "class Test {",
-            "  void test1() {",
-            "    Toys.Generic<String> g = new Toys.Generic<>();",
+            "  void test1(Toys.Generic<String> g) {",
             "    // BUG: Diagnostic contains: passing @Nullable parameter 'null'",
             "    g.getString(null);",
             "    // BUG: Diagnostic contains: passing @Nullable parameter 'null'",
@@ -95,6 +94,34 @@ public void genericsTest() {
         .doTest();
   }
 
+  @Test
+  public void wildcards() {
+    compilationHelper
+        .setArgs(
+            Arrays.asList(
+                "-d",
+                temporaryFolder.getRoot().getAbsolutePath(),
+                "-XepOpt:NullAway:AnnotatedPackages=com.uber",
+                "-XepOpt:NullAway:JarInferEnabled=true",
+                "-XepOpt:NullAway:UnannotatedSubPackages=com.uber.nullaway.[a-zA-Z0-9.]+.unannotated"))
+        .addSourceLines(
+            "Test.java",
+            "package com.uber;",
+            "import org.jspecify.annotations.Nullable;",
+            "import com.uber.nullaway.jarinfer.toys.unannotated.Toys;",
+            "class Test {",
+            "  void test1() {",
+            "    // BUG: Diagnostic contains: passing @Nullable parameter 'null'",
+            "    Toys.genericWildcard(null);",
+            "    // BUG: Diagnostic contains: passing @Nullable parameter 'null'",
+            "    Toys.genericWildcardUpper(null);",
+            "    // BUG: Diagnostic contains: passing @Nullable parameter 'null'",
+            "    Toys.genericWildcardLower(null);",
+            "  }",
+            "}")
+        .doTest();
+  }
+
   @Test
   public void jarinferNullableReturnsTest() {
     compilationHelper

diff --git a/...est-java-lib-jarinfer/src/main/java/com/uber/nullaway/jarinfer/toys/unannotated/Toys.java b/...est-java-lib-jarinfer/src/main/java/com/uber/nullaway/jarinfer/toys/unannotated/Toys.java
@@ -37,16 +37,32 @@ public static int testArray(Object[] o) {
     return o.hashCode();
   }
 
-  public static class Generic<T> {
+  public abstract static class Generic<T> {
     public String getString(T t) {
       return t.toString();
     }
+
+    public void doNothing() {}
+
+    public abstract T getSomething();
   }
 
   public static void genericParam(Generic<String> g) {
     g.getString("hello");
   }
 
+  public static void genericWildcard(Generic<?> g) {
+    g.doNothing();
+  }
+
+  public static String genericWildcardUpper(Generic<? extends String> g) {
+    return g.getSomething();
+  }
+
+  public static void genericWildcardLower(Generic<? super String> g) {
+    g.getString("hello");
+  }
+
   public static void main(String arg[]) throws java.io.IOException {
     String s = "test string...";
     Foo f = new Foo("let's");

diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java
@@ -1370,8 +1370,8 @@ public ImmutableSetMultimap<MethodRef, Integer> explicitlyNullableParameters() {
           for (Map.Entry<Integer, Set<String>> entry : innerEntry.getValue().entrySet()) {
             Integer index = entry.getKey();
             if (index >= 0 && entry.getValue().stream().anyMatch(a -> a.contains("Nullable"))) {
-              // remove spaces
-              methodNameAndSignature = methodNameAndSignature.replaceAll("\\s", "");
+              // remove spaces after commas
+              methodNameAndSignature = methodNameAndSignature.replaceAll(",\\s", ",");
               mapBuilder.put(methodRef(className, methodNameAndSignature), index);
             }
           }
@@ -1403,8 +1403,8 @@ public ImmutableSetMultimap<MethodRef, Integer> nonNullParameters() {
                           + methodEntry.getKey()
                           + " arg "
                           + argEntry.getKey());
-                  // remove spaces
-                  methodNameAndSignature = methodNameAndSignature.replaceAll("\\s", "");
+                  // remove spaces after commas
+                  methodNameAndSignature = methodNameAndSignature.replaceAll(",\\s", ",");
                   mapBuilder.put(methodRef(className, methodNameAndSignature), index);
                 }
               }