Skip to content

Commit

Permalink
Add support for opt.map(type::method) pattern. (#6370)
Browse files Browse the repository at this point in the history
  • Loading branch information
smillst authored Dec 15, 2023
1 parent 4b5e2c9 commit 886d0b3
Show file tree
Hide file tree
Showing 7 changed files with 202 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
package org.checkerframework.checker.optional;

import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MemberReferenceTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.Tree.Kind;
import java.util.Collection;
import java.util.function.Function;
import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.element.ElementKind;
import javax.lang.model.element.ExecutableElement;
import org.checkerframework.checker.optional.qual.Present;
import org.checkerframework.common.basetype.BaseAnnotatedTypeFactory;
import org.checkerframework.common.basetype.BaseTypeChecker;
import org.checkerframework.framework.flow.CFAbstractAnalysis;
import org.checkerframework.framework.flow.CFStore;
import org.checkerframework.framework.flow.CFTransfer;
import org.checkerframework.framework.flow.CFValue;
import org.checkerframework.framework.type.AnnotatedTypeMirror;
import org.checkerframework.javacutil.AnnotationBuilder;
import org.checkerframework.javacutil.TreeUtils;

/** OptionalAnnotatedTypeFactory for the Optional Checker. */
public class OptionalAnnotatedTypeFactory extends BaseAnnotatedTypeFactory {

/** The element for java.util.Optional.map(). */
private final ExecutableElement optionalMap;

/** The @{@link Present} annotation. */
protected final AnnotationMirror PRESENT = AnnotationBuilder.fromClass(elements, Present.class);

/**
* Creates an OptionalAnnotatedTypeFactory.
*
* @param checker the Optional Checker associated with this type factory
*/
public OptionalAnnotatedTypeFactory(BaseTypeChecker checker) {
super(checker);
postInit();
optionalMap = TreeUtils.getMethodOrNull("java.util.Optional", "map", 1, getProcessingEnv());
}

@Override
public AnnotatedTypeMirror getAnnotatedType(Tree tree) {
AnnotatedTypeMirror result = super.getAnnotatedType(tree);
optionalMapNonNull(tree, result);
return result;
}

/**
* If {@code tree} is a call to {@link java.util.Optional#map(Function)} whose argument is a
* method reference, then this method adds {@code @Present} to {@code type} if the following is
* true:
*
* <ul>
* <li>The type of the receiver to {@link java.util.Optional#map(Function)} is {@code @Present},
* and
* <li>{@link #returnHasNullable(MemberReferenceTree)} returns false.
* </ul>
*
* @param tree a tree
* @param type the type of the tree, which may be side-effected by this method
*/
private void optionalMapNonNull(Tree tree, AnnotatedTypeMirror type) {
if (!TreeUtils.isMethodInvocation(tree, optionalMap, processingEnv)) {
return;
}
MethodInvocationTree mapTree = (MethodInvocationTree) tree;
ExpressionTree argTree = mapTree.getArguments().get(0);
if (argTree.getKind() == Kind.MEMBER_REFERENCE) {
MemberReferenceTree memberReferenceTree = (MemberReferenceTree) argTree;
AnnotatedTypeMirror optType = getReceiverType(mapTree);
if (optType == null || !optType.hasEffectiveAnnotation(Present.class)) {
return;
}
if (!returnHasNullable(memberReferenceTree)) {
// The method still could have a @PolyNull on the return and might return null.
// If @PolyNull is the primary annotation on the parameter and not on any type arguments or
// array elements, then it is still safe to mark the optional type as present.
// TODO: Add the check for poly null on arguments.
type.replaceAnnotation(PRESENT);
}
}
}

/**
* Returns true if the return type of the function type of {@code memberReferenceTree} is
* annotated with {@code @Nullable}.
*
* @param memberReferenceTree a member reference
* @return true if the return type of the function type of {@code memberReferenceTree} is
* annotated with {@code @Nullable}
*/
private boolean returnHasNullable(MemberReferenceTree memberReferenceTree) {
if (TreeUtils.MemberReferenceKind.getMemberReferenceKind(memberReferenceTree)
.isConstructorReference()) {
return false;
}
ExecutableElement memberReferenceFuncType = TreeUtils.elementFromUse(memberReferenceTree);
if (memberReferenceFuncType.getEnclosingElement().getKind() == ElementKind.ANNOTATION_TYPE) {
// Annotation element accessor are always non-null;
return false;
}

if (!checker.hasOption("optionalMapAssumeNonNull")) {
return true;
}
return containsNullable(memberReferenceFuncType.getAnnotationMirrors())
|| containsNullable(memberReferenceFuncType.getReturnType().getAnnotationMirrors());
}

/**
* Returns true if {@code annos} contains a nullable annotation.
*
* @param annos a collection of annotations
* @return true if {@code annos} contains a nullable annotation
*/
private boolean containsNullable(Collection<? extends AnnotationMirror> annos) {
for (AnnotationMirror anno : annos) {
if (anno.getAnnotationType().asElement().getSimpleName().contentEquals("Nullable")) {
return true;
}
}
return false;
}

@Override
public CFTransfer createFlowTransferFunction(
CFAbstractAnalysis<CFValue, CFStore, CFTransfer> analysis) {
return new OptionalTransfer(analysis);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import org.checkerframework.common.basetype.BaseTypeChecker;
import org.checkerframework.framework.qual.RelevantJavaTypes;
import org.checkerframework.framework.qual.StubFiles;
import org.checkerframework.framework.source.SupportedOptions;

/**
* A type-checker that prevents misuse of the {@link java.util.Optional} class.
Expand All @@ -14,6 +15,7 @@
// @NonNull, make the return type have type @Present.
@RelevantJavaTypes(Optional.class)
@StubFiles({"javaparser.astub"})
@SupportedOptions("optionalMapAssumeNonNull")
public class OptionalChecker extends BaseTypeChecker {
/** Create an OptionalChecker. */
public OptionalChecker() {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@
import org.checkerframework.dataflow.cfg.UnderlyingAST.CFGLambda;
import org.checkerframework.dataflow.cfg.node.LocalVariableNode;
import org.checkerframework.dataflow.expression.JavaExpression;
import org.checkerframework.framework.flow.CFAnalysis;
import org.checkerframework.framework.flow.CFAbstractAnalysis;
import org.checkerframework.framework.flow.CFStore;
import org.checkerframework.framework.flow.CFTransfer;
import org.checkerframework.framework.flow.CFValue;
import org.checkerframework.framework.type.AnnotatedTypeFactory;
import org.checkerframework.javacutil.AnnotationBuilder;
import org.checkerframework.javacutil.TreeUtils;
Expand All @@ -45,7 +46,7 @@ public class OptionalTransfer extends CFTransfer {
*
* @param analysis the Optional Checker instance
*/
public OptionalTransfer(CFAnalysis analysis) {
public OptionalTransfer(CFAbstractAnalysis<CFValue, CFStore, CFTransfer> analysis) {
super(analysis);
atypeFactory = analysis.getTypeFactory();
Elements elements = atypeFactory.getElementUtils();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ public class OptionalTest extends CheckerFrameworkPerDirectoryTest {
* @param testFiles the files containing test code, which will be type-checked
*/
public OptionalTest(List<File> testFiles) {
super(testFiles, org.checkerframework.checker.optional.OptionalChecker.class, "optional");
super(
testFiles,
org.checkerframework.checker.optional.OptionalChecker.class,
"optional",
"-AoptionalMapAssumeNonNull");
}

@Parameters
Expand Down
16 changes: 16 additions & 0 deletions checker/tests/optional/MapNoNewNull.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import java.math.BigInteger;
import java.util.Optional;

class MapNoNewNull {

@SuppressWarnings("optional.parameter")
void m(Optional<Digits> digitsAnnotation) {
if (digitsAnnotation.isPresent()) {
BigInteger maxValue = digitsAnnotation.map(Digits::integer).map(BigInteger::valueOf).get();
}
}
}

@interface Digits {
public int integer();
}
34 changes: 34 additions & 0 deletions checker/tests/optional/OptionalMapMethodReference.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import java.util.Optional;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.checkerframework.checker.nullness.qual.PolyNull;
import org.checkerframework.checker.optional.qual.Present;

public class OptionalMapMethodReference {
Optional<String> getString() {
return Optional.of("");
}

@Present Optional<Integer> method() {
Optional<String> o = getString();
@Present Optional<Integer> oInt;
if (o.isPresent()) {
// :: error: (assignment)
oInt = o.map(this::convertNull);
oInt = o.map(this::convertPoly);
return o.map(this::convert);
}
return Optional.of(0);
}

@Nullable Integer convertNull(String s) {
return null;
}

@PolyNull Integer convertPoly(@PolyNull String s) {
return null;
}

Integer convert(String s) {
return 0;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1931,6 +1931,15 @@ public boolean isUnbound() {
return unbound;
}

/**
* Returns whether this kind is a constructor reference.
*
* @return whether this kind is a constructor reference
*/
public boolean isConstructorReference() {
return mode == ReferenceMode.NEW;
}

/**
* Returns the kind of member reference {@code tree} is.
*
Expand Down

0 comments on commit 886d0b3

Please sign in to comment.