Fix JarInfer handling of generic types

gradle/dependencies.gradle

@@ -48,7 +48,7 @@ def versions = [
     // The version of Error Prone that NullAway is compiled and tested against
     errorProneApi          : errorProneVersionToCompileAgainst,
     support                : "27.1.1",
-    wala                   : "1.6.7",
+    wala                   : "1.6.9",
     commonscli             : "1.4",
     autoValue              : "1.10.2",
     autoService            : "1.1.1",

jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java

@@ -42,6 +42,9 @@
 import com.ibm.wala.ssa.SSAInstruction;
 import com.ibm.wala.types.ClassLoaderReference;
 import com.ibm.wala.types.TypeReference;
+import com.ibm.wala.types.generics.MethodTypeSignature;
+import com.ibm.wala.types.generics.TypeSignature;
+import com.ibm.wala.types.generics.TypeVariableSignature;
 import com.ibm.wala.util.collections.Iterator2Iterable;
 import com.ibm.wala.util.config.FileOfClasses;
 import com.uber.nullaway.libmodel.MethodAnnotationsRecord;
@@ -505,36 +508,94 @@
    * @param mtd Method reference.
    * @return String Method signature.
    */
-  // TODO: handle generics and inner classes
   private static String getAstubxSignature(IMethod mtd) {
-    String classType =
-        mtd.getDeclaringClass().getName().toString().replaceAll("/", "\\.").substring(1);
-    classType = classType.replaceAll("\\$", "\\."); // handle inner class
-    String returnType = mtd.isInit() ? null : getSimpleTypeName(mtd.getReturnType());
-    String strArgTypes = "";
-    int argi = mtd.isStatic() ? 0 : 1; // Skip 'this' parameter
-    for (; argi < mtd.getNumberOfParameters(); argi++) {
-      strArgTypes += getSimpleTypeName(mtd.getParameterType(argi));
-      if (argi < mtd.getNumberOfParameters() - 1) {
-        strArgTypes += ", ";
+    Preconditions.checkArgument(
+        mtd instanceof ShrikeCTMethod, "Method is not a ShrikeCTMethod from bytecodes");
+    String classType = getSourceLevelQualifiedTypeName(mtd.getDeclaringClass().getReference());
+    MethodTypeSignature genericSignature = null;
+    try {
+      genericSignature = ((ShrikeCTMethod) mtd).getMethodTypeSignature();
+    } catch (InvalidClassFileException e) {
+      // don't fail, just proceed without the generic signature
+      LOG(DEBUG, "DEBUG", "Invalid class file exception: " + e.getMessage());
+    }
+    String returnType;
+    int numParams = mtd.isStatic() ? mtd.getNumberOfParameters() : mtd.getNumberOfParameters() - 1;
+    String[] argTypes = new String[numParams];
+    if (genericSignature != null) {
+      // get types that include generic type arguments
+      returnType = getSourceLevelQualifiedTypeName(genericSignature.getReturnType().toString());
+      TypeSignature[] argTypeSigs = genericSignature.getArguments();
+      for (int i = 0; i < argTypeSigs.length; i++) {
+        argTypes[i] = getSourceLevelQualifiedTypeName(argTypeSigs[i].toString());
+      }
+    } else { // no generics
+      returnType = mtd.isInit() ? null : getSourceLevelQualifiedTypeName(mtd.getReturnType());
+      int argi = mtd.isStatic() ? 0 : 1; // Skip 'this' parameter
+      for (int i = 0; i < numParams; i++) {
+        argTypes[i] = getSourceLevelQualifiedTypeName(mtd.getParameterType(argi++));
       }
     }
     return classType
         + ":"
         + (returnType == null ? "void " : returnType + " ")
         + mtd.getName().toString()
         + "("
-        + strArgTypes
+        + String.join(", ", argTypes)
         + ")";
   }
 
   /**
-   * Get simple unqualified type name.
+   * Get the source-level qualified type name for a TypeReference.
    *
    * @param typ Type Reference.
-   * @return String Unqualified type name.
+   * @return source-level qualified type name.
+   * @see #getSourceLevelQualifiedTypeName(String)
+   */
+  private static String getSourceLevelQualifiedTypeName(TypeReference typ) {
+    String typeName = typ.getName().toString();
+    return getSourceLevelQualifiedTypeName(typeName);
+  }
+
+  /**
+   * Converts a JVM-level qualified type (e.g., {@code Lcom/example/Foo$Baz;}) to a source-level
+   * qualified type (e.g., {@code com.example.Foo.Baz}). Nested types like generic type arguments
+   * are converted recursively.
+   *
+   * @param typeName JVM-level qualified type name.
+   * @return source-level qualified type name.
    */
-  private static String getSimpleTypeName(TypeReference typ) {
-    return StringStuff.jvmToBinaryName(typ.getName().toString());
+  private static String getSourceLevelQualifiedTypeName(String typeName) {
+    if (!typeName.endsWith(";")) {
+      // we need the semicolon since some of WALA's TypeSignature APIs expect it
+      typeName = typeName + ";";
+    }
+    boolean isGeneric = typeName.contains("<");
+    if (!isGeneric) { // base case
+      TypeSignature ts = TypeSignature.make(typeName);
+      if (ts.isTypeVariable()) {
+        // TypeVariableSignature's toString() returns more than just the identifier
+        return ((TypeVariableSignature) ts).getIdentifier();
+      } else {
+        String tsStr = ts.toString();
+        if (tsStr.endsWith(";")) {
+          // remove trailing semicolon
+          tsStr = tsStr.substring(0, tsStr.length() - 1);
+        }
+        return StringStuff.jvmToReadableType(tsStr);
+      }
+    } else { // generic type
+      int idx = typeName.indexOf("<");
+      String baseType = typeName.substring(0, idx);
+      // generic type args are separated by semicolons in signature stored in bytecodes
+      String[] genericTypeArgs = typeName.substring(idx + 1, typeName.length() - 2).split(";");
+      for (int i = 0; i < genericTypeArgs.length; i++) {
+        genericTypeArgs[i] = getSourceLevelQualifiedTypeName(genericTypeArgs[i]);
+      }
+      return getSourceLevelQualifiedTypeName(baseType)
+          + "<"
+          + String.join(",", genericTypeArgs)
+          + ">";
+    }
   }
 }

jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java

@@ -448,8 +448,7 @@ public void testGenericMethod() throws Exception {
         "testGenericMethod",
         "generic",
         "TestGeneric",
-        ImmutableMap.of(
-            "generic.TestGeneric:java.lang.String foo(java.lang.Object)", Sets.newHashSet(0)),
+        ImmutableMap.of("generic.TestGeneric:java.lang.String foo(T)", Sets.newHashSet(0)),
         "public class TestGeneric<T> {",
         "  public String foo(T t) {",
         "    return t.toString();",
@@ -461,6 +460,27 @@ public void testGenericMethod() throws Exception {
         "}");
   }
 
+  @Test
+  public void testMethodWithGenericParameter() throws Exception {
+    testTemplate(
+        "testMethodWithGenericParameter",
+        "generic",
+        "TestGeneric",
+        ImmutableMap.of(
+            "generic.TestGeneric:java.lang.String getString(generic.TestGeneric.Generic<java.lang.String,java.lang.String>)",
+            Sets.newHashSet(0)),
+        "public class TestGeneric {",
+        "  static class Generic<T,U> {",
+        "    public String foo(T t) {",
+        "      return \"hi\";",
+        "    }",
+        "  }",
+        "  public String getString(Generic<String,String> g) {",
+        "    return g.foo(\"test\");",
+        "  }",
+        "}");
+  }
+
   @Test
   public void toyJARAnnotatingClasses() throws Exception {
     testAnnotationInJarTemplate(

jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java

@@ -88,6 +88,8 @@ public void genericsTest() {
             "    Toys.Generic<String> g = new Toys.Generic<>();",
             "    // BUG: Diagnostic contains: passing @Nullable parameter 'null'",
             "    g.getString(null);",
+            "    // BUG: Diagnostic contains: passing @Nullable parameter 'null'",
+            "    Toys.genericParam(null);",
             "  }",
             "}")
         .doTest();

jar-infer/test-java-lib-jarinfer/src/main/java/com/uber/nullaway/jarinfer/toys/unannotated/Toys.java

@@ -43,6 +43,10 @@ public String getString(T t) {
     }
   }
 
+  public static void genericParam(Generic<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");

nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java

@@ -27,7 +27,6 @@
 import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.Tree;
 import com.sun.tools.javac.code.Symbol;
-import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.util.Context;
 import com.uber.nullaway.NullAway;
 import com.uber.nullaway.Nullness;
@@ -38,7 +37,6 @@
 import java.util.Map;
 import java.util.Set;
 import javax.lang.model.element.Modifier;
-import javax.lang.model.type.TypeKind;
 import org.checkerframework.nullaway.dataflow.cfg.node.MethodInvocationNode;
 import org.jspecify.annotations.Nullable;
 
@@ -230,27 +228,17 @@ private String getMethodSignature(Symbol.MethodSymbol method) {
     String methodSign =
         method.enclClass().getQualifiedName().toString()
             + ":"
-            + (method.isStaticOrInstanceInit()
-                ? ""
-                : getSimpleTypeName(method.getReturnType()) + " ")
+            + (method.isStaticOrInstanceInit() ? "" : method.getReturnType() + " ")
             + method.getSimpleName()
             + "(";
     if (!method.getParameters().isEmpty()) {
-      for (Symbol.VarSymbol var : method.getParameters()) {
-        methodSign += getSimpleTypeName(var.type) + ", ";
-      }
-      methodSign = methodSign.substring(0, methodSign.lastIndexOf(','));
+      methodSign +=
+          String.join(
+              ", ",
+              method.getParameters().stream().map(p -> p.type.toString()).toArray(String[]::new));
     }
     methodSign += ")";
     LOG(DEBUG, "DEBUG", "@ method sign: " + methodSign);
     return methodSign;
   }
-
-  private String getSimpleTypeName(Type typ) {
-    if (typ.getKind() == TypeKind.TYPEVAR) {
-      return typ.getUpperBound().tsym.getQualifiedName().toString();
-    } else {
-      return typ.toString();
-    }
-  }
 }