aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAbhijit Kulkarni <akulk022@ucr.edu>2023-10-15 14:50:52 -0700
committerGitHub <noreply@github.com>2023-10-15 14:50:52 -0700
commite7623f74b667cbeb88fca6a0be7c2af6d111df59 (patch)
tree02436ebb83176f638f80a53350005cc73887631c
parent2d2b829f47c13ac9cfb1a5b5e339b07b82a240be (diff)
downloadnullaway-e7623f74b667cbeb88fca6a0be7c2af6d111df59.tar.gz
JSpecify: handle incorrect method parameter nullability for method reference (#845)
We now report an error for the following test case: ```java class Test { interface A<T1 extends @Nullable Object> { String function(T1 o); } static String foo(Object o) { return o.toString(); } static void testPositive() { // we now report an error here, as foo's parameter need to be @Nullable A<@Nullable Object> p = Test::foo; } static void testNegative() { A<Object> p = Test::foo; } } ``` --------- Co-authored-by: Manu Sridharan <msridhar@gmail.com>
-rw-r--r--nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java12
-rw-r--r--nullaway/src/main/java/com/uber/nullaway/NullAway.java33
-rw-r--r--nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java3
3 files changed, 29 insertions, 19 deletions
diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java
index ce31aeb..045890d 100644
--- a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java
+++ b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java
@@ -879,11 +879,6 @@ public final class GenericsChecks {
VisitorState state,
Config config) {
Type enclosingType = getTypeForSymbol(enclosingSymbol, state);
- if (enclosingType == null) {
- // we have no additional information from generics, so return NONNULL (presence of a @Nullable
- // annotation should have been handled by the caller)
- return Nullness.NONNULL;
- }
return getGenericMethodParameterNullness(parameterIndex, method, enclosingType, state, config);
}
@@ -904,9 +899,14 @@ public final class GenericsChecks {
public static Nullness getGenericMethodParameterNullness(
int parameterIndex,
Symbol.MethodSymbol method,
- Type enclosingType,
+ @Nullable Type enclosingType,
VisitorState state,
Config config) {
+ if (enclosingType == null) {
+ // we have no additional information from generics, so return NONNULL (presence of a top-level
+ // @Nullable annotation is handled elsewhere)
+ return Nullness.NONNULL;
+ }
Type methodType = state.getTypes().memberType(enclosingType, method);
Type paramType = methodType.getParameterTypes().get(parameterIndex);
return getTypeNullness(paramType, config);
diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java
index 79c7c8c..2da65c8 100644
--- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java
+++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java
@@ -731,17 +731,28 @@ public class NullAway extends BugChecker
// -XepOpt:NullAway:AcknowledgeRestrictiveAnnotations flag and its handler).
if (isOverriddenMethodAnnotated) {
for (int i = 0; i < superParamSymbols.size(); i++) {
- overriddenMethodArgNullnessMap[i] =
- Nullness.paramHasNullableAnnotation(overriddenMethod, i, config)
- ? Nullness.NULLABLE
- : (config.isJSpecifyMode()
- ? GenericsChecks.getGenericMethodParameterNullness(
- i,
- overriddenMethod,
- overridingParamSymbols.get(i).owner.owner,
- state,
- config)
- : Nullness.NONNULL);
+ Nullness paramNullness;
+ if (Nullness.paramHasNullableAnnotation(overriddenMethod, i, config)) {
+ paramNullness = Nullness.NULLABLE;
+ } else if (config.isJSpecifyMode()) {
+ // Check if the parameter type is a type variable and the corresponding generic type
+ // argument is @Nullable
+ if (memberReferenceTree != null) {
+ // For a method reference, we get generic type arguments from the javac's inferred type
+ // for the tree, which seems to properly preserve type-use annotations
+ paramNullness =
+ GenericsChecks.getGenericMethodParameterNullness(
+ i, overriddenMethod, ASTHelpers.getType(memberReferenceTree), state, config);
+ } else {
+ // Use the enclosing class of the overriding method to find generic type arguments
+ paramNullness =
+ GenericsChecks.getGenericMethodParameterNullness(
+ i, overriddenMethod, overridingParamSymbols.get(i).owner.owner, state, config);
+ }
+ } else {
+ paramNullness = Nullness.NONNULL;
+ }
+ overriddenMethodArgNullnessMap[i] = paramNullness;
}
}
diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java
index 7f236ca..f0e8827 100644
--- a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java
+++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java
@@ -429,8 +429,7 @@ public class NullAwayJSpecifyGenericsTests extends NullAwayTestsBase {
" return o.toString();",
" }",
" static void testPositive() {",
- " // TODO: we should report an error here, since Test::foo cannot take",
- " // a @Nullable parameter. we don't catch this yet",
+ " // BUG: Diagnostic contains: parameter o of referenced method is @NonNull",
" A<@Nullable Object> p = Test::foo;",
" }",
" static void testNegative() {",