JSpecify: handle incorrect method parameter nullability for method re…

…ference (#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>
uber · Oct 15, 2023 · e7623f7 · e7623f7
1 parent 2d2b829
commit e7623f7
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 19 deletions.
diff --git 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 static Nullness getGenericMethodParameterNullness(
       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 static Nullness getGenericMethodParameterNullness(
   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
@@ -731,17 +731,28 @@ private Description checkParamOverriding(
     // -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
@@ -429,8 +429,7 @@ public void testForMethodReferenceInAnAssignment() {
             "    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() {",