diff --git a/checker-qual/src/main/java/org/checkerframework/common/delegation/qual/Delegate.java b/checker-qual/src/main/java/org/checkerframework/common/delegation/qual/Delegate.java new file mode 100644 index 00000000000..f59642d8a83 --- /dev/null +++ b/checker-qual/src/main/java/org/checkerframework/common/delegation/qual/Delegate.java @@ -0,0 +1,33 @@ +package org.checkerframework.common.delegation.qual; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * This is an annotation that indicates a field is a delegate, fields are not delegates by default. + * + *

Here is a way that this annotation may be used: + * + *


+ * class MyEnumeration<T> implements Enumeration<T> {
+ *    {@literal @}Delegate
+ *    private Enumeration<T> e;
+ *
+ *    public boolean hasMoreElements() {
+ *      return e.hasMoreElements();
+ *    }
+ * }
+ * 
+ * + * In the example above, {@code MyEnumeration.hasMoreElements()} delegates a call to {@code + * e.hasMoreElements()}. + * + * @checker_framework.manual #non-empty-checker Non-Empty Checker + */ +@Documented +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.FIELD}) +public @interface Delegate {} diff --git a/checker-qual/src/main/java/org/checkerframework/common/delegation/qual/DelegatorMustOverride.java b/checker-qual/src/main/java/org/checkerframework/common/delegation/qual/DelegatorMustOverride.java new file mode 100644 index 00000000000..5e89bd2026f --- /dev/null +++ b/checker-qual/src/main/java/org/checkerframework/common/delegation/qual/DelegatorMustOverride.java @@ -0,0 +1,49 @@ +package org.checkerframework.common.delegation.qual; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * This is an annotation that indicates a method that must be overridden in order for a + * conditional postcondition to hold for a delegating class. + * + *

Here is a way that this annotation may be used: + * + *

Given a class that declares a method with a postcondition annotation: + * + *


+ * class ArrayListVariant<T> {
+ *    {@literal @}EnsuresPresentIf(result = true)
+ *    {@literal @}DelegatorMustOverride
+ *    public boolean hasMoreElements() {
+ *      return e.hasMoreElements();
+ *    }
+ * }
+ * 
+ * + * A delegating client must override the method: + * + *

+ * class MyArrayListVariant<T> extends ArrayListVariant<T> {
+ *
+ *    {@literal @}Delegate
+ *     private ArrayListVariant<T> myList;
+ *
+ *
+ *    {@literal @}Override
+ *    {@literal @}EnsuresPresentIf(result = true)
+ *    public boolean hasMoreElements() {
+ *      return myList.hasMoreElements();
+ *    }
+ * }
+ * 
+ * + * Otherwise, a warning will be raised. + * + * @checker_framework.manual #non-empty-checker Non-Empty Checker + */ +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.METHOD}) +public @interface DelegatorMustOverride {} diff --git a/framework/src/main/java/org/checkerframework/common/delegation/DelegationAnnotatedTypeFactory.java b/framework/src/main/java/org/checkerframework/common/delegation/DelegationAnnotatedTypeFactory.java new file mode 100644 index 00000000000..9d03213f98d --- /dev/null +++ b/framework/src/main/java/org/checkerframework/common/delegation/DelegationAnnotatedTypeFactory.java @@ -0,0 +1,22 @@ +package org.checkerframework.common.delegation; + +import java.lang.annotation.Annotation; +import java.util.Set; +import org.checkerframework.common.basetype.BaseAnnotatedTypeFactory; +import org.checkerframework.common.basetype.BaseTypeChecker; +import org.checkerframework.common.delegation.qual.Delegate; + +/** Annotated type factory for the Delegation Checker. */ +public class DelegationAnnotatedTypeFactory extends BaseAnnotatedTypeFactory { + + /** Create the type factory. */ + public DelegationAnnotatedTypeFactory(BaseTypeChecker checker) { + super(checker); + this.postInit(); + } + + @Override + protected Set> createSupportedTypeQualifiers() { + return getBundledTypeQualifiers(Delegate.class); + } +} diff --git a/framework/src/main/java/org/checkerframework/common/delegation/DelegationChecker.java b/framework/src/main/java/org/checkerframework/common/delegation/DelegationChecker.java new file mode 100644 index 00000000000..59001372855 --- /dev/null +++ b/framework/src/main/java/org/checkerframework/common/delegation/DelegationChecker.java @@ -0,0 +1,17 @@ +package org.checkerframework.common.delegation; + +import org.checkerframework.common.basetype.BaseTypeChecker; +import org.checkerframework.common.delegation.qual.Delegate; + +/** + * This class enforces checks for the {@link Delegate} annotation. + * + *

It is not a checker for a type system. It enforces the following syntactic checks: + * + *

+ */ +public class DelegationChecker extends BaseTypeChecker {} diff --git a/framework/src/main/java/org/checkerframework/common/delegation/DelegationVisitor.java b/framework/src/main/java/org/checkerframework/common/delegation/DelegationVisitor.java new file mode 100644 index 00000000000..21055c548b3 --- /dev/null +++ b/framework/src/main/java/org/checkerframework/common/delegation/DelegationVisitor.java @@ -0,0 +1,253 @@ +package org.checkerframework.common.delegation; + +import com.sun.source.tree.BlockTree; +import com.sun.source.tree.ClassTree; +import com.sun.source.tree.ExpressionStatementTree; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MemberSelectTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.tree.MethodTree; +import com.sun.source.tree.ReturnTree; +import com.sun.source.tree.StatementTree; +import com.sun.source.tree.ThrowTree; +import com.sun.source.tree.Tree; +import com.sun.source.tree.VariableTree; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; +import javax.lang.model.element.AnnotationMirror; +import javax.lang.model.element.Element; +import javax.lang.model.element.ExecutableElement; +import javax.lang.model.element.Name; +import javax.lang.model.element.TypeElement; +import javax.lang.model.element.VariableElement; +import javax.lang.model.type.DeclaredType; +import javax.lang.model.type.TypeKind; +import javax.lang.model.util.ElementFilter; +import org.checkerframework.checker.nullness.qual.Nullable; +import org.checkerframework.common.basetype.BaseAnnotatedTypeFactory; +import org.checkerframework.common.basetype.BaseTypeChecker; +import org.checkerframework.common.basetype.BaseTypeVisitor; +import org.checkerframework.common.delegation.qual.Delegate; +import org.checkerframework.common.delegation.qual.DelegatorMustOverride; +import org.checkerframework.framework.type.AnnotatedTypeMirror; +import org.checkerframework.javacutil.TreeUtils; +import org.checkerframework.javacutil.TypesUtils; + +public class DelegationVisitor extends BaseTypeVisitor { + + /** The maximum number of fields marked with {@link Delegate} permitted in a class. */ + private final int MAX_NUM_DELEGATE_FIELDS = 1; + + /** The field marked with {@link Delegate} for the current class. */ + private @Nullable VariableTree delegate; + + public DelegationVisitor(BaseTypeChecker checker) { + super(checker); + } + + @Override + public void processClassTree(ClassTree tree) { + delegate = null; // Unset the previous delegate whenever a new class is visited + // TODO: what about inner classes? + List delegates = getDelegateFields(tree); + if (delegates.size() > MAX_NUM_DELEGATE_FIELDS) { + VariableTree latestDelegate = delegates.get(delegates.size() - 1); + checker.reportError(latestDelegate, "multiple.delegate.annotations"); + } else if (delegates.size() == 1) { + delegate = delegates.get(0); + checkSuperClassOverrides(tree); + } + // Do nothing if no delegate field is found + super.processClassTree(tree); + } + + @Override + public void processMethodTree(String className, MethodTree tree) { + super.processMethodTree(className, tree); + if (delegate == null || !isMarkedWithOverride(tree)) { + return; + } + MethodInvocationTree candidateDelegateCall = getLastExpression(tree.getBody()); + boolean hasExceptionalExit = + hasExceptionalExit(tree.getBody(), UnsupportedOperationException.class); + if (hasExceptionalExit) { + return; + } + if (candidateDelegateCall == null) { + checker.reportWarning(tree, "invalid.delegate", tree.getName(), delegate.getName()); + return; + } + Name enclosingMethodName = tree.getName(); + if (!isValidDelegateCall(enclosingMethodName, candidateDelegateCall)) { + checker.reportWarning(tree, "invalid.delegate", tree.getName(), delegate.getName()); + } + } + + /** + * Return true if the given method call is a valid delegate call for the enclosing method. + * + *

A delegate method call must fulfill the following properties: its receiver must be the + * current field marked with {@link Delegate} in the class, and the name of the method call must + * match that of the enclosing method. + * + * @param enclosingMethodName the name of the enclosing method + * @param delegatedMethodCall the delegated method call + * @return true if the given method call is a valid delegate call for the enclosing method + */ + private boolean isValidDelegateCall( + Name enclosingMethodName, MethodInvocationTree delegatedMethodCall) { + assert delegate != null; // This method should only be invoked when delegate is non-null + ExpressionTree methodSelectTree = delegatedMethodCall.getMethodSelect(); + MemberSelectTree fieldAccessTree = (MemberSelectTree) methodSelectTree; + VariableElement delegatedField = TreeUtils.asFieldAccess(fieldAccessTree.getExpression()); + Name delegatedMethodName = TreeUtils.methodName(delegatedMethodCall); + // TODO: is there a better way to check? Comparing names seems fragile. + return enclosingMethodName.equals(delegatedMethodName) + && delegatedField != null + && delegatedField.getSimpleName().equals(delegate.getName()); + } + + /** + * Returns the fields of a class marked with a {@link Delegate} annotation. + * + * @param tree a class + * @return the fields of a class marked with a {@link Delegate} annotation + */ + private List getDelegateFields(ClassTree tree) { + List delegateFields = new ArrayList<>(); + for (VariableTree field : TreeUtils.fieldsFromClassTree(tree)) { + List annosOnField = + TreeUtils.annotationsFromTypeAnnotationTrees(field.getModifiers().getAnnotations()); + if (annosOnField.stream() + .anyMatch(anno -> atypeFactory.areSameByClass(anno, Delegate.class))) { + delegateFields.add(field); + } + } + return delegateFields; + } + + /** + * Returns the last expression in a method body. + * + *

This method is used to identify a possible delegate method call. It will check whether a + * method has only one statement (a method invocation or a return statement), and return the + * expression that is associated with it. Otherwise, it will return null. + * + * @param tree the method body + * @return the last expression in the method body + */ + private @Nullable MethodInvocationTree getLastExpression(BlockTree tree) { + List stmts = tree.getStatements(); + if (stmts.size() != 1) { + return null; + } + StatementTree stmt = stmts.get(0); + ExpressionTree lastExprInMethod = null; + if (stmt instanceof ExpressionStatementTree) { + lastExprInMethod = ((ExpressionStatementTree) stmt).getExpression(); + } else if (stmt instanceof ReturnTree) { + lastExprInMethod = ((ReturnTree) stmt).getExpression(); + } + if (!(lastExprInMethod instanceof MethodInvocationTree)) { + return null; + } + return (MethodInvocationTree) lastExprInMethod; + } + + /** + * Return true if the last (and only) statement of the block throws an exception of the given + * class. + * + * @param tree a block tree + * @param clazz a class of exception (usually {@link UnsupportedOperationException}) + * @return true if the last and only statement throws an exception of the given class + */ + private boolean hasExceptionalExit(BlockTree tree, Class clazz) { + List stmts = tree.getStatements(); + if (stmts.size() != 1) { + return false; + } + StatementTree lastStmt = stmts.get(0); + if (!(lastStmt instanceof ThrowTree)) { + return false; + } + ThrowTree throwStmt = (ThrowTree) lastStmt; + AnnotatedTypeMirror throwType = atypeFactory.getAnnotatedType(throwStmt.getExpression()); + Class exceptionClass = TypesUtils.getClassFromType(throwType.getUnderlyingType()); + return exceptionClass.equals(clazz); + } + + /** + * Validate whether a class overrides all declared methods in its superclass. + * + *

This is a basic implementation that naively checks whether all the superclass methods have + * been overridden by the subclass. It is unlikely in practice that a delegating subclass needs to + * override all the methods in a superclass for postconditions to hold. + * + * @param tree a class tree + */ + private void checkSuperClassOverrides(ClassTree tree) { + TypeElement classTreeElt = TreeUtils.elementFromDeclaration(tree); + if (classTreeElt == null || classTreeElt.getSuperclass() == null) { + return; + } + DeclaredType superClassMirror = (DeclaredType) classTreeElt.getSuperclass(); + if (superClassMirror == null || superClassMirror.getKind() == TypeKind.NONE) { + return; + } + Set overriddenMethods = + getOverriddenMethods(tree).stream() + .map(ExecutableElement::getSimpleName) + .collect(Collectors.toSet()); + Set methodsDeclaredInSuperClass = + new HashSet<>(ElementFilter.methodsIn(superClassMirror.asElement().getEnclosedElements())); + Set methodsThatMustBeOverriden = + methodsDeclaredInSuperClass.stream() + .filter(e -> atypeFactory.getDeclAnnotation(e, DelegatorMustOverride.class) != null) + .map(ExecutableElement::getSimpleName) + .collect(Collectors.toSet()); + + // TODO: comparing a set of names isn't ideal, what about overloading? + if (!overriddenMethods.containsAll(methodsThatMustBeOverriden)) { + checker.reportWarning( + tree, + "delegate.override", + tree.getSimpleName(), + TypesUtils.getQualifiedName(superClassMirror)); + } + } + + /** + * Return a set of all methods in the class that are marked with {@link Override}. + * + * @param tree the class tree + * @return a set of all methods in the class that are marked with {@link Override} + */ + private Set getOverriddenMethods(ClassTree tree) { + Set overriddenMethods = new HashSet<>(); + for (Tree member : tree.getMembers()) { + if (!(member instanceof MethodTree)) { + continue; + } + MethodTree method = (MethodTree) member; + if (isMarkedWithOverride(method)) { + overriddenMethods.add(TreeUtils.elementFromDeclaration(method)); + } + } + return overriddenMethods; + } + + /** + * Return true if a method is marked with {@link Override}. + * + * @param tree the method declaration + * @return true if the given method declaration is annotated with {@link Override} + */ + private boolean isMarkedWithOverride(MethodTree tree) { + Element method = TreeUtils.elementFromDeclaration(tree); + return atypeFactory.getDeclAnnotation(method, Override.class) != null; + } +} diff --git a/framework/src/main/java/org/checkerframework/common/delegation/messages.properties b/framework/src/main/java/org/checkerframework/common/delegation/messages.properties new file mode 100644 index 00000000000..8835e594447 --- /dev/null +++ b/framework/src/main/java/org/checkerframework/common/delegation/messages.properties @@ -0,0 +1,3 @@ +multiple.delegate.annotations=A class may only have one @Delegate field +invalid.delegate=%s does not delegate a call to %s +delegate.override=%s does not override all methods in %s diff --git a/framework/src/test/java/org/checkerframework/framework/test/junit/DelegationTest.java b/framework/src/test/java/org/checkerframework/framework/test/junit/DelegationTest.java new file mode 100644 index 00000000000..9ae76be9e63 --- /dev/null +++ b/framework/src/test/java/org/checkerframework/framework/test/junit/DelegationTest.java @@ -0,0 +1,21 @@ +package org.checkerframework.framework.test.junit; + +import java.io.File; +import java.util.List; +import org.checkerframework.framework.test.CheckerFrameworkPerDirectoryTest; +import org.junit.runners.Parameterized.Parameters; + +public class DelegationTest extends CheckerFrameworkPerDirectoryTest { + + /** + * @param testFiles the files containing test code, which will be type-checked + */ + public DelegationTest(List testFiles) { + super(testFiles, org.checkerframework.common.delegation.DelegationChecker.class, "delegation"); + } + + @Parameters + public static String[] getTestDirs() { + return new String[] {"delegation"}; + } +} diff --git a/framework/tests/delegation/DelegatedCallTest.java b/framework/tests/delegation/DelegatedCallTest.java new file mode 100644 index 00000000000..5d9b4f3e0ca --- /dev/null +++ b/framework/tests/delegation/DelegatedCallTest.java @@ -0,0 +1,39 @@ +import java.util.IdentityHashMap; +import org.checkerframework.common.delegation.qual.*; + +public class DelegatedCallTest extends IdentityHashMap { + + private static final long serialVersionUID = -5147442142854693854L; + + /** The wrapped map. */ + @Delegate private final IdentityHashMap map; + + private DelegatedCallTest(IdentityHashMap map) { + this.map = map; + } + + @Override + public int size(DelegatedCallTest this) { + return map.size(); + } + + @Override + public boolean isEmpty(DelegatedCallTest this) { + return map.isEmpty(); + } + + @Override + public V get(DelegatedCallTest this, Object key) { + return map.get(key); + } + + @Override + public boolean containsKey(DelegatedCallTest this, Object key) { + return map.containsKey(key); + } + + @Override + public boolean containsValue(DelegatedCallTest this, Object value) { + return map.containsValue(value); + } +} diff --git a/framework/tests/delegation/DelegatedCallThrowsException.java b/framework/tests/delegation/DelegatedCallThrowsException.java new file mode 100644 index 00000000000..1cd095ddc73 --- /dev/null +++ b/framework/tests/delegation/DelegatedCallThrowsException.java @@ -0,0 +1,45 @@ +import java.util.IdentityHashMap; +import java.util.Map; +import org.checkerframework.common.delegation.qual.*; + +public class DelegatedCallThrowsException extends IdentityHashMap { + + private static final long serialVersionUID = -5147442142854693854L; + + /** The wrapped map. */ + @Delegate private final IdentityHashMap map; + + private DelegatedCallThrowsException(IdentityHashMap map) { + this.map = map; + } + + @Override + public int size(DelegatedCallThrowsException this) { + return map.size(); + } + + @Override + public boolean isEmpty(DelegatedCallThrowsException this) { + return map.isEmpty(); + } + + @Override + public V get(DelegatedCallThrowsException this, Object key) { + return map.get(key); + } + + @Override + public boolean containsKey(DelegatedCallThrowsException this, Object key) { + return map.containsKey(key); + } + + @Override + public boolean containsValue(DelegatedCallThrowsException this, Object value) { + return map.containsValue(value); + } + + @Override + public void putAll(DelegatedCallThrowsException this, Map m) { + throw new UnsupportedOperationException(); // OK + } +} diff --git a/framework/tests/delegation/InvalidDelegateTest.java b/framework/tests/delegation/InvalidDelegateTest.java new file mode 100644 index 00000000000..28916743f20 --- /dev/null +++ b/framework/tests/delegation/InvalidDelegateTest.java @@ -0,0 +1,32 @@ +import java.util.IdentityHashMap; +import org.checkerframework.common.delegation.qual.*; + +// :: warning: (delegate.override) +public class InvalidDelegateTest extends IdentityHashMap { + + @Delegate public IdentityHashMap map; + + @Override + public boolean isEmpty() { + return this.map.isEmpty(); // OK + } + + @Override + public int size() { + return map.size(); // OK + } + + @Override + // :: warning: (invalid.delegate) + public boolean containsKey(Object key) { + // :: error: (contracts.conditional.postcondition) + return true; + } + + @Override + // :: warning: (invalid.delegate) + public boolean containsValue(Object value) { + int x = 3; + return map.containsValue(value); + } +} diff --git a/framework/tests/delegation/MultiDelegationTest.java b/framework/tests/delegation/MultiDelegationTest.java new file mode 100644 index 00000000000..584f1bcef2d --- /dev/null +++ b/framework/tests/delegation/MultiDelegationTest.java @@ -0,0 +1,9 @@ +import org.checkerframework.common.delegation.qual.*; + +class MultiDelegationTest { + + @Delegate public int foo; + + // :: error: (multiple.delegate.annotations) + @Delegate public int bar; +} diff --git a/framework/tests/delegation/VoidDelegateTest.java b/framework/tests/delegation/VoidDelegateTest.java new file mode 100644 index 00000000000..b12a053eaf7 --- /dev/null +++ b/framework/tests/delegation/VoidDelegateTest.java @@ -0,0 +1,17 @@ +import java.util.ArrayList; +import org.checkerframework.common.delegation.qual.*; + +// :: warning: (delegate.override) +public class VoidDelegateTest extends ArrayList { + + @Delegate private ArrayList array; + + public VoidDelegateTest(ArrayList array) { + this.array = array; + } + + @Override + public void clear() { + this.array.clear(); // This should be OK + } +}