Permit Stream.filter(Optional::isPresent).map(Optional::get)

Co-authored-by: Suzanne Millstein <smillst@cs.washington.edu>

checker/src/main/java/org/checkerframework/checker/optional/OptionalVisitor.java

@@ -6,6 +6,7 @@
 import com.sun.source.tree.ExpressionStatementTree;
 import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.IfTree;
+import com.sun.source.tree.MemberReferenceTree;
 import com.sun.source.tree.MethodInvocationTree;
 import com.sun.source.tree.ParenthesizedTree;
 import com.sun.source.tree.StatementTree;
@@ -92,6 +93,12 @@ public class OptionalVisitor
   /** The element for java.util.Optional.orElseThrow(Supplier), or null if running under JDK 8. */
   private final @Nullable ExecutableElement optionalOrElseThrowSupplier;
 
+  /** The element for java.util.stream.Stream.filter(). */
+  private final ExecutableElement streamFilter;
+
+  /** The element for java.util.stream.Stream.map(). */
+  private final ExecutableElement streamMap;
+
   /** Static methods that create an Optional. */
   private final List<ExecutableElement> optionalCreators;
 
@@ -126,6 +133,9 @@ public OptionalVisitor(BaseTypeChecker checker) {
     optionalOrElseThrow = TreeUtils.getMethodOrNull("java.util.Optional", "orElseThrow", 0, env);
     optionalOrElseThrowSupplier = TreeUtils.getMethod("java.util.Optional", "orElseThrow", 1, env);
 
+    streamFilter = TreeUtils.getMethod("java.util.stream.Stream", "filter", 1, env);
+    streamMap = TreeUtils.getMethod("java.util.stream.Stream", "map", 1, env);
+
     optionalCreators = Arrays.asList(optionalEmpty, optionalOf, optionalOfNullable);
     optionalPropagators =
         optionalOr == null
@@ -644,4 +654,62 @@ public static StatementTree skipBlocks(StatementTree tree) {
     }
     return s;
   }
+
+  @Override
+  public Void visitMemberReference(MemberReferenceTree tree, Void p) {
+    if (isFilterIsPresentMapGet(tree)) {
+      // TODO: This is a (sound) workaround until
+      // https://github.com/typetools/checker-framework/issues/1345
+      // is fixed.
+      return null;
+    }
+    return super.visitMemberReference(tree, p);
+  }
+
+  /**
+   * Returns true if {@code memberRefTree} is the {@code Optional::get} in {@code
+   * Stream.filter(Optional::isPresent).map(Optional::get)}.
+   *
+   * @param memberRefTree a member reference tree
+   * @return true if {@code memberRefTree} the {@code Optional::get} in {@code
+   *     Stream.filter(Optional::isPresent).map(Optional::get)}
+   */
+  private boolean isFilterIsPresentMapGet(MemberReferenceTree memberRefTree) {
+    if (!TreeUtils.elementFromUse(memberRefTree).equals(optionalGet)) {
+      // The method reference is not Optional::get
+      return false;
+    }
+    // "getPath" means "the path to the node `Optional::get`".
+    TreePath getPath = getCurrentPath();
+    TreePath getParentPath = getPath.getParentPath();
+    // "getParent" means "the parent of the node `Optional::get`".
+    Tree getParent = getParentPath.getLeaf();
+    if (getParent.getKind() == Tree.Kind.METHOD_INVOCATION) {
+      MethodInvocationTree hasGetAsArgumentTree = (MethodInvocationTree) getParent;
+      ExecutableElement hasGetAsArgumentElement = TreeUtils.elementFromUse(hasGetAsArgumentTree);
+      if (!hasGetAsArgumentElement.equals(streamMap)) {
+        // Optional::get is not an argument to stream#map
+        return false;
+      }
+      // hasGetAsArgumentTree is an invocation of Stream#map(...).
+      Tree mapReceiverTree = TreeUtils.getReceiverTree(hasGetAsArgumentTree);
+      // Will check whether mapParent is the call `Stream.filter(Optional::isPresent)`.
+      if (mapReceiverTree != null && mapReceiverTree.getKind() == Tree.Kind.METHOD_INVOCATION) {
+        MethodInvocationTree fluentToMapTree = (MethodInvocationTree) mapReceiverTree;
+        ExecutableElement fluentToMapElement = TreeUtils.elementFromUse(fluentToMapTree);
+        if (!fluentToMapElement.equals(streamFilter)) {
+          // The receiver of map(Optional::get) is not Stream#filter
+          return false;
+        }
+        MethodInvocationTree filterInvocationTree = fluentToMapTree;
+        ExpressionTree filterArgTree = filterInvocationTree.getArguments().get(0);
+        if (filterArgTree.getKind() == Tree.Kind.MEMBER_REFERENCE) {
+          ExecutableElement filterArgElement =
+              TreeUtils.elementFromUse((MemberReferenceTree) filterArgTree);
+          return filterArgElement.equals(optionalIsPresent);
+        }
+      }
+    }
+    return false;
+  }
 }

checker/tests/optional/FilterIspresentMapGetTest.java

@@ -0,0 +1,9 @@
+import java.util.Optional;
+import java.util.stream.Stream;
+
+class FilterIspresentMapGetTest {
+
+  void m(Stream<Optional<String>> ss) {
+    ss.filter(Optional::isPresent).map(Optional::get);
+  }
+}

framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java

@@ -3715,7 +3715,7 @@ protected boolean checkOverride(
 
   /**
    * Type checks that a method may override another method. Uses an OverrideChecker subclass as
-   * created by {@link #createOverrideChecker}. This version of the method exposes
+   * created by {@link #createOverrideChecker}. This version of the method exposes the
    * AnnotatedExecutableType of the overriding method. Override this version of the method if you
    * need to access that type.
    *
@@ -3756,7 +3756,7 @@ protected boolean checkOverride(
   }
 
   /**
-   * Check that a method reference is allowed. Using the OverrideChecker class.
+   * Check that a method reference is allowed. Uses the OverrideChecker class.
    *
    * @param memberReferenceTree the tree for the method reference
    * @return true if the method reference is allowed
@@ -3794,9 +3794,7 @@ protected boolean checkMethodReferenceAsOverride(
 
     // ========= Overriding Executable =========
     // The ::method element, see JLS 15.13.1 Compile-Time Declaration of a Method Reference
-    ExecutableElement compileTimeDeclaration =
-        (ExecutableElement) TreeUtils.elementFromUse(memberReferenceTree);
-
+    ExecutableElement compileTimeDeclaration = TreeUtils.elementFromUse(memberReferenceTree);
     if (enclosingType.getKind() == TypeKind.DECLARED
         && ((AnnotatedDeclaredType) enclosingType).isUnderlyingTypeRaw()) {
       if (memRefKind == MemberReferenceKind.UNBOUND) {
@@ -3934,7 +3932,10 @@ private boolean checkMethodReferenceInference(
    */
   public class OverrideChecker {
 
-    /** The declaration of an overriding method. */
+    /**
+     * The declaration of an overriding method. Or, it could be a method reference that is being
+     * passed to a method.
+     */
     protected final Tree overriderTree;
 
     /** True if {@link #overriderTree} is a MEMBER_REFERENCE. */
@@ -3985,10 +3986,10 @@ public OverrideChecker(
       this.overriderTree = overriderTree;
       this.overrider = overrider;
       this.overriderType = overriderType;
+      this.overriderReturnType = overriderReturnType;
       this.overridden = overridden;
       this.overriddenType = overriddenType;
       this.overriddenReturnType = overriddenReturnType;
-      this.overriderReturnType = overriderReturnType;
 
       this.isMethodReference = overriderTree.getKind() == Tree.Kind.MEMBER_REFERENCE;
     }
@@ -4114,7 +4115,7 @@ private void checkPreAndPostConditions() {
      */
     private boolean checkMemberReferenceReceivers() {
       if (overriderType.getKind() == TypeKind.ARRAY) {
-        // Assume the receiver for all method on arrays are @Top
+        // Assume the receiver for all method on arrays are @Top.
         // This simplifies some logic because an AnnotatedExecutableType for an array method
         // (ie String[]::clone) has a receiver of "Array." The UNBOUND check would then
         // have to compare "Array" to "String[]".

javacutil/src/main/java/org/checkerframework/javacutil/TreeUtils.java

@@ -473,6 +473,23 @@ public static ExecutableElement elementFromUse(MethodInvocationTree tree) {
     return (ExecutableElement) result;
   }
 
+  /**
+   * Returns the ExecutableElement for the method reference.
+   *
+   * @param tree a method reference
+   * @return the ExecutableElement for the method reference
+   */
+  @Pure
+  public static ExecutableElement elementFromUse(MemberReferenceTree tree) {
+    Element result = elementFromUse((ExpressionTree) tree);
+    if (!(result instanceof ExecutableElement)) {
+      throw new BugInCF(
+          "Method reference elements should be ExecutableElement. Found: %s [%s]",
+          result, result.getClass());
+    }
+    return (ExecutableElement) result;
+  }
+
   /**
    * Returns the ExecutableElement for the given method declaration.
    *