bug fix: did not handle direct dereference of method call expression …

…with generic @nullable return type
uber · Jun 15, 2023 · 97ec62a · 97ec62a
1 parent 08596f0
commit 97ec62a
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 3 deletions.
diff --git a/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/GenericsChecks.java
@@ -603,6 +603,52 @@ public static Nullness getGenericMethodReturnTypeNullness(
     return getTypeNullness(overriddenMethodType.getReturnType(), config);
   }
 
+  /**
+   * Computes the nullness of the return of a generic method at an invocation, in the context of the
+   * declared type of its receiver argument. If the return type is a type variable, its nullness
+   * depends on the nullability of the corresponding type parameter in the receiver's type.
+   *
+   * <p>Consider the following example:
+   *
+   * <pre>
+   *     interface Fn<P extends @Nullable Object, R extends @Nullable Object> {
+   *         R apply(P p);
+   *     }
+   *     class C implements Fn<String, @Nullable String> {
+   *         public @Nullable String apply(String p) {
+   *             return null;
+   *         }
+   *     }
+   *     static void m() {
+   *         Fn<String, @Nullable String> f = new C();
+   *         f.apply("hello").hashCode(); // NPE
+   *     }
+   * </pre>
+   *
+   * The declared type of {@code f} passes {@code Nullable String} as the type parameter for type
+   * variable {@code R}. So, the call {@code f.apply("hello")} returns {@code @Nullable} and an
+   * error should be reported.
+   *
+   * @param invokedMethodSymbol symbol for the invoked method
+   * @param tree the tree for the invocation
+   * @return Nullness of invocation's return type, or {@code NONNULL} if the call does not invoke an
+   *     instance method
+   */
+  public static Nullness getGenericReturnNullnessAtInvocation(
+      Symbol.MethodSymbol invokedMethodSymbol,
+      MethodInvocationTree tree,
+      VisitorState state,
+      Config config) {
+    if (!(tree.getMethodSelect() instanceof MemberSelectTree)) {
+      return Nullness.NONNULL;
+    }
+    Type methodReceiverType =
+        castToNonNull(
+            ASTHelpers.getType(((MemberSelectTree) tree.getMethodSelect()).getExpression()));
+    return getGenericMethodReturnTypeNullness(
+        invokedMethodSymbol, methodReceiverType, state, config);
+  }
+
   /**
    * Computes the nullness of a formal parameter of a generic method at an invocation, in the
    * context of the declared type of its receiver argument. If the formal parameter's type is a type

diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java
@@ -2309,7 +2309,9 @@ private boolean mayBeNullExpr(VisitorState state, ExpressionTree expr) {
                   + " for method invocation "
                   + state.getSourceForNode(expr));
         }
-        exprMayBeNull = mayBeNullMethodCall((Symbol.MethodSymbol) exprSymbol);
+        exprMayBeNull =
+            mayBeNullMethodCall(
+                (Symbol.MethodSymbol) exprSymbol, (MethodInvocationTree) expr, state);
         break;
       case CONDITIONAL_EXPRESSION:
       case ASSIGNMENT:
@@ -2328,11 +2330,16 @@ private boolean mayBeNullExpr(VisitorState state, ExpressionTree expr) {
     return exprMayBeNull && nullnessFromDataflow(state, expr);
   }
 
-  private boolean mayBeNullMethodCall(Symbol.MethodSymbol exprSymbol) {
+  private boolean mayBeNullMethodCall(
+      Symbol.MethodSymbol exprSymbol, MethodInvocationTree invocationTree, VisitorState state) {
     if (codeAnnotationInfo.isSymbolUnannotated(exprSymbol, config)) {
       return false;
     }
-    return Nullness.hasNullableAnnotation(exprSymbol, config);
+    return Nullness.hasNullableAnnotation(exprSymbol, config)
+        || (config.isJSpecifyMode()
+            && GenericsChecks.getGenericReturnNullnessAtInvocation(
+                    exprSymbol, invocationTree, state, config)
+                .equals(Nullness.NULLABLE));
   }
 
   public boolean nullnessFromDataflow(VisitorState state, ExpressionTree expr) {

diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java
@@ -764,6 +764,8 @@ public void overrideReturnTypes() {
             "    String t3 = f3.apply(s);",
             "    // BUG: Diagnostic contains: dereferenced expression",
             "    t3.hashCode();",
+            "    // BUG: Diagnostic contains: dereferenced expression",
+            "    f3.apply(s).hashCode();",
             " }",
             "}")
         .doTest();