From 25963a95f654e9cd9c30d6b924d9890e04cae038 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Tue, 7 Nov 2023 14:53:53 -0800 Subject: [PATCH 1/4] Adding com.google.common to annotatedpackages and fixing the NPE we got from method invocation raw type --- nullaway/build.gradle | 2 +- .../com/uber/nullaway/GenericsChecks.java | 4 +-- .../NullAwayJSpecifyGenericsTests.java | 25 +++++++++++++++++++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/nullaway/build.gradle b/nullaway/build.gradle index 8000dbc9e3..90ac062418 100644 --- a/nullaway/build.gradle +++ b/nullaway/build.gradle @@ -152,7 +152,7 @@ tasks.register('buildWithNullAway', JavaCompile) { sourceSets.main.compileClasspath) options.errorprone.enabled = true options.errorprone { - option("NullAway:AnnotatedPackages", "com.uber,org.checkerframework.nullaway") + option("NullAway:AnnotatedPackages", "com.uber,org.checkerframework.nullaway,com.google.common") option("NullAway:CastToNonNullMethod", "com.uber.nullaway.NullabilityUtil.castToNonNull") option("NullAway:CheckOptionalEmptiness") option("NullAway:AcknowledgeRestrictiveAnnotations") diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java index af79f42486..ef56490473 100644 --- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java @@ -784,7 +784,7 @@ public static Nullness getGenericReturnNullnessAtInvocation( MethodInvocationTree tree, VisitorState state, Config config) { - if (!(tree.getMethodSelect() instanceof MemberSelectTree)) { + if (!(tree.getMethodSelect() instanceof MemberSelectTree) || invokedMethodSymbol.isStatic()) { return Nullness.NONNULL; } Type methodReceiverType = @@ -834,7 +834,7 @@ public static Nullness getGenericParameterNullnessAtInvocation( MethodInvocationTree tree, VisitorState state, Config config) { - if (!(tree.getMethodSelect() instanceof MemberSelectTree)) { + if (!(tree.getMethodSelect() instanceof MemberSelectTree) || invokedMethodSymbol.isStatic()) { return Nullness.NONNULL; } Type enclosingType = diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index 73737b8ca6..510cb174b5 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -1465,12 +1465,37 @@ public void interactionWithContracts() { .doTest(); } + @Test + public void testForGuavaRawTypeBuildIssue() { + makeHelperWithGuava() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "import com.google.common.collect.ImmutableSet;", + "class Test {", + " static void funcImmutableSet(ImmutableSet is){", + " }", + " static void testNegative() {", + " funcImmutableSet(ImmutableSet.of());", + " }", + "}") + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( Arrays.asList( "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:JSpecifyMode=true")); } + private CompilationTestHelper makeHelperWithGuava() { + return makeTestHelperWithArgs( + Arrays.asList( + "-XepOpt:NullAway:AnnotatedPackages=com.uber,com.google.common", + "-XepOpt:NullAway:JSpecifyMode=true")); + } + private CompilationTestHelper makeHelperWithoutJSpecifyMode() { return makeTestHelperWithArgs(Arrays.asList("-XepOpt:NullAway:AnnotatedPackages=com.uber")); } From ffb680ef5de07db715e7101ef6f0445ca51ff180 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Tue, 7 Nov 2023 15:56:48 -0800 Subject: [PATCH 2/4] Adding scenario for ImmutableSet.Of() with a param to the test case testForGuavaRawTypeBuildIssue --- .../java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index 510cb174b5..38632c50e0 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -1479,6 +1479,9 @@ public void testForGuavaRawTypeBuildIssue() { " static void testNegative() {", " funcImmutableSet(ImmutableSet.of());", " }", + " static void testNegative2() {", + " funcImmutableSet(ImmutableSet.of(new Object()));", + " }", "}") .doTest(); } From f04f43ca8b960a42b7628f5940e7af95533dc428 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Wed, 8 Nov 2023 11:29:52 -0800 Subject: [PATCH 3/4] changing the test case to be generic for static method calls instead of being specific to guava ImmutableSet --- .../NullAwayJSpecifyGenericsTests.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java index 38632c50e0..bc574f1711 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -1466,21 +1466,28 @@ public void interactionWithContracts() { } @Test - public void testForGuavaRawTypeBuildIssue() { - makeHelperWithGuava() + public void testForStaticMethodCallAsAParam() { + makeHelper() .addSourceLines( "Test.java", "package com.uber;", "import org.jspecify.annotations.Nullable;", - "import com.google.common.collect.ImmutableSet;", "class Test {", - " static void funcImmutableSet(ImmutableSet is){", + " static class A {", + " public static A returnA(){", + " return new A();", + " }", + " public static A returnAWithParam(Object o){", + " return new A();", + " }", + " }", + " static void func(A a){", " }", " static void testNegative() {", - " funcImmutableSet(ImmutableSet.of());", + " func(A.returnA());", " }", " static void testNegative2() {", - " funcImmutableSet(ImmutableSet.of(new Object()));", + " func(A.returnAWithParam(new Object()));", " }", "}") .doTest(); @@ -1492,13 +1499,6 @@ private CompilationTestHelper makeHelper() { "-XepOpt:NullAway:AnnotatedPackages=com.uber", "-XepOpt:NullAway:JSpecifyMode=true")); } - private CompilationTestHelper makeHelperWithGuava() { - return makeTestHelperWithArgs( - Arrays.asList( - "-XepOpt:NullAway:AnnotatedPackages=com.uber,com.google.common", - "-XepOpt:NullAway:JSpecifyMode=true")); - } - private CompilationTestHelper makeHelperWithoutJSpecifyMode() { return makeTestHelperWithArgs(Arrays.asList("-XepOpt:NullAway:AnnotatedPackages=com.uber")); } From 54bc881a3e92cfdc68f39b9184d170d44c2fa88f Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Wed, 8 Nov 2023 11:34:18 -0800 Subject: [PATCH 4/4] Removing com.google.common from annotated packages to add in a future commit --- nullaway/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nullaway/build.gradle b/nullaway/build.gradle index 90ac062418..8000dbc9e3 100644 --- a/nullaway/build.gradle +++ b/nullaway/build.gradle @@ -152,7 +152,7 @@ tasks.register('buildWithNullAway', JavaCompile) { sourceSets.main.compileClasspath) options.errorprone.enabled = true options.errorprone { - option("NullAway:AnnotatedPackages", "com.uber,org.checkerframework.nullaway,com.google.common") + option("NullAway:AnnotatedPackages", "com.uber,org.checkerframework.nullaway") option("NullAway:CastToNonNullMethod", "com.uber.nullaway.NullabilityUtil.castToNonNull") option("NullAway:CheckOptionalEmptiness") option("NullAway:AcknowledgeRestrictiveAnnotations")