From f2bafb25ab49dcc73a15e84ff76c89c2e7b7ee3b Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sun, 22 Dec 2024 13:42:27 -0500 Subject: [PATCH] Add support for parameter types with wildcards for JarInfer (#1107) --- .../DefinitelyDerefedParamsDriver.java | 21 +++++++++++++ .../uber/nullaway/jarinfer/JarInferTest.java | 27 ++++++++++++++++ .../jarinfer/JarInferIntegrationTest.java | 31 +++++++++++++++++-- .../jarinfer/toys/unannotated/Toys.java | 18 ++++++++++- .../handlers/LibraryModelsHandler.java | 8 ++--- 5 files changed, 98 insertions(+), 7 deletions(-) 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 1cd4c79ade..a78b2a18af 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 @@ -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 index a98ffa4100..ed5c9c34e4 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 @@ -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)", + Sets.newHashSet(0), + "generic.TestGeneric:void genericWildcard(generic.TestGeneric.Generic)", + Sets.newHashSet(0), + "generic.TestGeneric:java.lang.String genericWildcardUpper(generic.TestGeneric.Generic)", + Sets.newHashSet(0)), + "public class TestGeneric {", + " public abstract static class Generic {", + " 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 g) { return g.getSomething(); };", + " public static void genericWildcardLower(Generic g) { g.getString(\"hello\"); };", + "}"); + } + @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 45d7487f6d..c818bb09d0 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 @@ -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 g = new Toys.Generic<>();", + " void test1(Toys.Generic 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/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 f397a14942..27c4b02dc6 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 @@ -37,16 +37,32 @@ public static int testArray(Object[] o) { return o.hashCode(); } - public static class Generic { + public abstract static class Generic { public String getString(T t) { return t.toString(); } + + public void doNothing() {} + + public abstract T getSomething(); } public static void genericParam(Generic g) { g.getString("hello"); } + public static void genericWildcard(Generic g) { + g.doNothing(); + } + + public static String genericWildcardUpper(Generic g) { + return g.getSomething(); + } + + public static void genericWildcardLower(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/LibraryModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java index 646246275b..a5034f96bb 100644 --- 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 explicitlyNullableParameters() { for (Map.Entry> 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 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); } }