more fixes, still needs cleanup

uber · Jan 23, 2024 · 026970c · 026970c
1 parent 27918ff
commit 026970c
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 12 deletions.
diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java
@@ -905,7 +905,7 @@ private Description checkReturnExpression(
       if (GenericsChecks.getGenericMethodReturnTypeNullness(
                   methodSymbol, ASTHelpers.getType(lambdaTree), state, config)
               .equals(Nullness.NULLABLE)
-          || GenericsChecks.passingLambdaWithGenericReturnToUnmarkedCode(
+          || GenericsChecks.passingLambdaOrMethodRefWithGenericReturnToUnmarkedCode(
               methodSymbol, lambdaTree, state, config, codeAnnotationInfo)) {
         // In JSpecify mode, the return type of a lambda may be @Nullable via a type argument
         return Description.NO_MATCH;
@@ -1052,8 +1052,10 @@ private boolean overriddenMethodReturnsNonNull(
         // For a method reference, we get generic type arguments from javac's inferred type for the
         // tree, which properly preserves type-use annotations
         return GenericsChecks.getGenericMethodReturnTypeNullness(
-                overriddenMethod, ASTHelpers.getType(memberReferenceTree), state, config)
-            .equals(Nullness.NONNULL);
+                    overriddenMethod, ASTHelpers.getType(memberReferenceTree), state, config)
+                .equals(Nullness.NONNULL)
+            && !GenericsChecks.passingLambdaOrMethodRefWithGenericReturnToUnmarkedCode(
+                overriddenMethod, memberReferenceTree, state, config, codeAnnotationInfo);
       } else {
         // Use the enclosing class of the overriding method to find generic type arguments
         return GenericsChecks.getGenericMethodReturnTypeNullness(

diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java
@@ -13,7 +13,6 @@
 import com.sun.source.tree.AssignmentTree;
 import com.sun.source.tree.ConditionalExpressionTree;
 import com.sun.source.tree.ExpressionTree;
-import com.sun.source.tree.LambdaExpressionTree;
 import com.sun.source.tree.MemberSelectTree;
 import com.sun.source.tree.MethodInvocationTree;
 import com.sun.source.tree.MethodTree;
@@ -846,9 +845,9 @@ public static String prettyTypeForError(Type type, VisitorState state) {
     return type.accept(new GenericTypePrettyPrintingVisitor(state), null);
   }
 
-  public static boolean passingLambdaWithGenericReturnToUnmarkedCode(
+  public static boolean passingLambdaOrMethodRefWithGenericReturnToUnmarkedCode(
       Symbol.MethodSymbol methodSymbol,
-      LambdaExpressionTree lambdaTree,
+      ExpressionTree expressionTree,
       VisitorState state,
       Config config,
       CodeAnnotationInfo codeAnnotationInfo) {
@@ -859,7 +858,7 @@ public static boolean passingLambdaWithGenericReturnToUnmarkedCode(
     }
     boolean callingUnannotated = false;
     TreePath path = state.getPath();
-    while (path != null && !path.getLeaf().equals(lambdaTree)) {
+    while (path != null && !path.getLeaf().equals(expressionTree)) {
       path = path.getParentPath();
     }
     verify(path != null, "did not find lambda tree in TreePath");

diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayJSpecifyGenericsTests.java
@@ -1665,12 +1665,16 @@ public void testForNullReturnLambdaFromStreamMap() {
             "class Test {",
             "  @Nullable",
             "  static Integer foo(){",
-            "   return null;",
-            "   }",
+            "    return null;",
+            "  }",
+            "  @Nullable",
+            "  static Integer foo2(Integer i){",
+            "    return null;",
+            "  }",
             "  static void testNegative() {",
-            "   List<Integer> numbers = Arrays.asList(1, 2, 3, 4, 5);",
-            "   List<Integer> doubledNumbers = numbers.stream().map(number -> foo()).collect(Collectors.toList());",
-            // TODO need the same test but with a method reference
+            "    List<Integer> numbers = Arrays.asList(1, 2, 3, 4, 5);",
+            "    List<Integer> doubledNumbers = numbers.stream().map(number -> foo()).collect(Collectors.toList());",
+            "    List<Integer> other = numbers.stream().map(Test::foo2).collect(Collectors.toList());",
             "  }",
             "}")
         .doTest();