diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index 81af7d7b6f..417be1dde9 100755 --- a/gradle/dependencies.gradle +++ b/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", diff --git a/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java b/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java index ea06ebd4a1..1cd4c79ade 100644 --- a/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java +++ b/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,18 +508,32 @@ private String getSignature(IMethod mtd) { * @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 @@ -524,17 +541,61 @@ private static String getAstubxSignature(IMethod mtd) { + (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) + + ">"; + } } } 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 index 10345ac7d5..a98ffa4100 100644 --- 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 @@ -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 {", " 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)", + Sets.newHashSet(0)), + "public class TestGeneric {", + " static class Generic {", + " public String foo(T t) {", + " return \"hi\";", + " }", + " }", + " public String getString(Generic g) {", + " return g.foo(\"test\");", + " }", + "}"); + } + @Test public void toyJARAnnotatingClasses() throws Exception { testAnnotationInJarTemplate( diff --git a/jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java b/jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java index e7f2c07e98..45d7487f6d 100644 --- a/jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java +++ b/jar-infer/nullaway-integration-test/src/test/java/com/uber/nullaway/jarinfer/JarInferIntegrationTest.java @@ -88,6 +88,8 @@ public void genericsTest() { " Toys.Generic 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(); diff --git a/jar-infer/test-java-lib-jarinfer/src/main/java/com/uber/nullaway/jarinfer/toys/unannotated/Toys.java b/jar-infer/test-java-lib-jarinfer/src/main/java/com/uber/nullaway/jarinfer/toys/unannotated/Toys.java index 1c781faa52..f397a14942 100644 --- a/jar-infer/test-java-lib-jarinfer/src/main/java/com/uber/nullaway/jarinfer/toys/unannotated/Toys.java +++ b/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 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/InferredJARModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java index edaaf9d83b..aec47b9db9 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java +++ b/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(); - } - } }