From 5aeb32cc0f310a4789c0ba47b790e2f7737db810 Mon Sep 17 00:00:00 2001 From: Abhijit Kulkarni Date: Wed, 8 Nov 2023 14:02:36 -0800 Subject: [PATCH] JSpecify: fix crash with calls to static methods (#856) After adding com.google.common to annotated packages for buildWithNullAway in JSpecify Mode, we got the following exception: ```java error: An unhandled exception was thrown by the Error Prone static analysis plugin. return findEnclosingMethodOrLambdaOrInitializer(path, ImmutableSet.of()); com.google.common.util.concurrent.UncheckedExecutionException: java.lang.NullPointerException: castToNonNull failed! at com.google.common.cache.LocalCache$Segment.get(LocalCache.java:2086) at com.google.common.cache.LocalCache.get(LocalCache.java:4012) at com.google.common.cache.LocalCache.getOrLoad(LocalCache.java:4035) at com.google.common.cache.LocalCache$LocalLoadingCache.get(LocalCache.java:5011) Caused by: java.lang.NullPointerException: castToNonNull failed! at com.uber.nullaway.NullabilityUtil.castToNonNull(NullabilityUtil.java:409) at com.uber.nullaway.GenericsChecks.getGenericReturnNullnessAtInvocation(GenericsChecks.java:800) at com.uber.nullaway.dataflow.AccessPathNullnessPropagation.genericReturnIsNullable(AccessPathNullnessPropagation.java:1031) at com.uber.nullaway.dataflow.AccessPathNullnessPropagation.returnValueNullness(AccessPathNullnessPropagation.java:1008) at com.uber.nullaway.dataflow.AccessPathNullnessPropagation.visit ``` With the call to ```java ImmutableSet.of()``` which is a static call, we were wrongly trying to return null from getTreeType in GenericChecks.java for this case which caused the above Exception. A unit test which reproduces the issue has been added to NullAwayJSpecifyGenerickChecks.java: ```java import org.jspecify.annotations.Nullable; import com.google.common.collect.ImmutableSet; class Test { static void funcImmutableSet(ImmutableSet is){ } static void testNegative() { //We were getting the issue on this line funcImmutableSet(ImmutableSet.of()); } } ``` All the unit tests from the NullAway build have passed for these changes. --------- Co-authored-by: Manu Sridharan --- .../com/uber/nullaway/GenericsChecks.java | 4 +-- .../NullAwayJSpecifyGenericsTests.java | 28 +++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) 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 e7227064f3..58f09d1f8a 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java @@ -1494,6 +1494,34 @@ public void interactionWithContracts() { .doTest(); } + @Test + public void testForStaticMethodCallAsAParam() { + makeHelper() + .addSourceLines( + "Test.java", + "package com.uber;", + "import org.jspecify.annotations.Nullable;", + "class Test {", + " 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() {", + " func(A.returnA());", + " }", + " static void testNegative2() {", + " func(A.returnAWithParam(new Object()));", + " }", + "}") + .doTest(); + } + private CompilationTestHelper makeHelper() { return makeTestHelperWithArgs( Arrays.asList(