diff options
author | Abhijit Kulkarni <akulk022@ucr.edu> | 2023-10-15 14:50:52 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-10-15 14:50:52 -0700 |
commit | e7623f74b667cbeb88fca6a0be7c2af6d111df59 (patch) | |
tree | 02436ebb83176f638f80a53350005cc73887631c | |
parent | 2d2b829f47c13ac9cfb1a5b5e339b07b82a240be (diff) | |
download | nullaway-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>
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() {", |