From 964d02799dc8642c3cf762fdac8ae91efea27983 Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Tue, 12 Dec 2023 12:09:55 -0800 Subject: [PATCH] Permit `Stream.filter(Optional::isPresent).map(Optional::get)` Co-authored-by: Suzanne Millstein --- .../checker/optional/OptionalVisitor.java | 68 +++++++++++++++++++ .../optional/FilterIspresentMapGetTest.java | 9 +++ .../common/basetype/BaseTypeVisitor.java | 17 ++--- .../checkerframework/javacutil/TreeUtils.java | 17 +++++ 4 files changed, 103 insertions(+), 8 deletions(-) create mode 100644 checker/tests/optional/FilterIspresentMapGetTest.java diff --git a/checker/src/main/java/org/checkerframework/checker/optional/OptionalVisitor.java b/checker/src/main/java/org/checkerframework/checker/optional/OptionalVisitor.java index d24fa56b140..d909c8bd24d 100644 --- a/checker/src/main/java/org/checkerframework/checker/optional/OptionalVisitor.java +++ b/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 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; + } } diff --git a/checker/tests/optional/FilterIspresentMapGetTest.java b/checker/tests/optional/FilterIspresentMapGetTest.java new file mode 100644 index 00000000000..cb993c151f2 --- /dev/null +++ b/checker/tests/optional/FilterIspresentMapGetTest.java @@ -0,0 +1,9 @@ +import java.util.Optional; +import java.util.stream.Stream; + +class FilterIspresentMapGetTest { + + void m(Stream> ss) { + ss.filter(Optional::isPresent).map(Optional::get); + } +} diff --git a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java index 34a3183bdb2..27f533d48de 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/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[]". diff --git a/javacutil/src/main/java/org/checkerframework/javacutil/TreeUtils.java b/javacutil/src/main/java/org/checkerframework/javacutil/TreeUtils.java index 91fe891ddd6..2258e288dc5 100644 --- a/javacutil/src/main/java/org/checkerframework/javacutil/TreeUtils.java +++ b/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. *