From bb7030ef447ac2d4283987f4050c4c02c35edc95 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 25 Jul 2023 10:23:02 -0700 Subject: [PATCH 1/4] WIP --- .../com/uber/nullaway/GenericsChecks.java | 7 +++- .../NullAwayJSpecifyGenericsTests.java | 36 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index e66430e114..eba788e4fa 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -214,7 +214,12 @@ private Type getTreeType(Tree tree) { } return typeWithPreservedAnnotations(paramTypedTree); } else { - return ASTHelpers.getType(tree); + Type result = ASTHelpers.getType(tree); + if (result.isRaw()) { + // bail out of any checking involving raw types for now + return null; + } + return result; } } diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index 2cac6729cc..6080779583 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -718,6 +718,42 @@ public void varargsParameter() { .doTest(); } + @Test + public void issue791() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.checkerframework.checker.nullness.qual.Nullable;\n" + + "\n" + + "class Test {\n" + + "\n" + + " interface RemovalListener {}\n" + + "\n" + + " static final class AsyncEvictionListener\n" + + "\timplements RemovalListener {\n" + + "\n" + + "\tAsyncEvictionListener(RemovalListener delegate) {}\n" + + " }\n" + + " \n" + + " \n" + + " static class Caffeine {\n" + + "\n" + + " @Nullable RemovalListener evictionListener;\t\n" + + "\n" + + "\t@SuppressWarnings({\"rawtypes\", \"unchecked\"})\n" + + "\t @Nullable RemovalListener getEvictionListener(\n" + + "\t\t\t\t\t\t\t\t\t\t\t boolean async) {\n" + + "\t var castedListener = (RemovalListener) evictionListener;\n" + + "\t return async && (castedListener != null)\n" + + "\t\t? new AsyncEvictionListener(castedListener)\n" + + "\t\t: castedListener;\n" + + "\t}\t\n" + + " }\n" + + "}\n") + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( Arrays.asList( From e59a474f708614dd400fdc7ef61796db34b23c94 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 25 Jul 2023 16:21:45 -0700 Subject: [PATCH 2/4] better test --- .../NullAwayJSpecifyGenericsTests.java | 50 ++++++++----------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index 6080779583..5851113805 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -719,38 +719,32 @@ public void varargsParameter() { } @Test - public void issue791() { + public void rawTypes() { makeHelper() .addSourceLines( "Test.java", "package com.uber;", - "import org.checkerframework.checker.nullness.qual.Nullable;\n" - + "\n" - + "class Test {\n" - + "\n" - + " interface RemovalListener {}\n" - + "\n" - + " static final class AsyncEvictionListener\n" - + "\timplements RemovalListener {\n" - + "\n" - + "\tAsyncEvictionListener(RemovalListener delegate) {}\n" - + " }\n" - + " \n" - + " \n" - + " static class Caffeine {\n" - + "\n" - + " @Nullable RemovalListener evictionListener;\t\n" - + "\n" - + "\t@SuppressWarnings({\"rawtypes\", \"unchecked\"})\n" - + "\t @Nullable RemovalListener getEvictionListener(\n" - + "\t\t\t\t\t\t\t\t\t\t\t boolean async) {\n" - + "\t var castedListener = (RemovalListener) evictionListener;\n" - + "\t return async && (castedListener != null)\n" - + "\t\t? new AsyncEvictionListener(castedListener)\n" - + "\t\t: castedListener;\n" - + "\t}\t\n" - + " }\n" - + "}\n") + "import org.jspecify.annotations.Nullable;", + "class Test {", + " static class NonNullTypeParam {}", + " static class NullableTypeParam {}", + " static void rawLocals() {", + " NonNullTypeParam t1 = new NonNullTypeParam();", + " NullableTypeParam<@Nullable String> t2 = new NullableTypeParam();", + " NonNullTypeParam t3 = new NonNullTypeParam();", + " NullableTypeParam t4 = new NullableTypeParam<@Nullable String>();", + " NonNullTypeParam t5 = new NonNullTypeParam();", + " NullableTypeParam t6 = new NullableTypeParam();", + " }", + " static void rawConditionalExpression(boolean b, NullableTypeParam<@Nullable String> p) {", + " NullableTypeParam<@Nullable String> t = b ? new NullableTypeParam() : p;", + " }", + " static void doNothing(NullableTypeParam<@Nullable String> p) { }", + " static void rawParameterPassing() { doNothing(new NullableTypeParam()); }", + " static NullableTypeParam<@Nullable String> rawReturn() {", + " return new NullableTypeParam();", + "}", + "}") .doTest(); } From c34c402f1a96fe524ba2006af48397ca8e43f520 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 25 Jul 2023 16:31:55 -0700 Subject: [PATCH 3/4] add null check! --- nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index eba788e4fa..4ca3d3ebd2 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -215,7 +215,7 @@ private Type getTreeType(Tree tree) { return typeWithPreservedAnnotations(paramTypedTree); } else { Type result = ASTHelpers.getType(tree); - if (result.isRaw()) { + if (result != null && result.isRaw()) { // bail out of any checking involving raw types for now return null; } From 5704615b8002ff9b8fa3eeb44b1211efd5c9b5bf Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Wed, 26 Jul 2023 09:58:13 -0700 Subject: [PATCH 4/4] add comment --- .../com/uber/nullaway/NullAwayJSpecifyGenericsTests.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index 5851113805..5040606433 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -718,6 +718,11 @@ public void varargsParameter() { .doTest(); } + /** + * Currently this test is solely to ensure NullAway does not crash in the presence of raw types. + * Further study of the JSpecify documents is needed to determine whether any errors should be + * reported for these cases. + */ @Test public void rawTypes() { makeHelper()