From 337446324859cb7e354cd417115b23f920d1ed03 Mon Sep 17 00:00:00 2001 From: tamier Date: Sun, 9 Oct 2016 10:28:44 -0400 Subject: [PATCH 01/13] Add new TypeUseLocations --- .../common/basetype/BaseTypeValidator.java | 544 +++++++++++++++++- .../common/basetype/messages.properties | 18 +- .../framework/qual/TypeUseLocation.java | 15 + .../util/defaults/QualifierDefaults.java | 12 +- 4 files changed, 586 insertions(+), 3 deletions(-) diff --git a/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java b/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java index 0edbbc81a6b1..6c3a7eddb02c 100644 --- a/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java +++ b/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java @@ -5,18 +5,37 @@ */ import com.sun.source.tree.AnnotatedTypeTree; +import com.sun.source.tree.ClassTree; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.IdentifierTree; +import com.sun.source.tree.MemberSelectTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.tree.MethodTree; import com.sun.source.tree.NewClassTree; import com.sun.source.tree.ParameterizedTypeTree; import com.sun.source.tree.Tree; +import com.sun.source.tree.TypeParameterTree; import com.sun.source.tree.VariableTree; +import com.sun.source.util.TreePath; +import com.sun.tools.javac.code.Type.WildcardType; +import java.util.Arrays; +import java.util.HashMap; +import java.util.HashSet; +import java.util.IdentityHashMap; import java.util.List; +import java.util.Map; import java.util.Set; import javax.lang.model.element.AnnotationMirror; +import javax.lang.model.element.Element; +import javax.lang.model.element.ElementKind; import javax.lang.model.element.TypeElement; +import javax.lang.model.element.TypeParameterElement; import javax.lang.model.type.TypeKind; +import org.checkerframework.framework.qual.DefaultQualifier; +import org.checkerframework.framework.qual.DefaultQualifiers; import org.checkerframework.framework.qual.PolyAll; +import org.checkerframework.framework.qual.TargetLocations; +import org.checkerframework.framework.qual.TypeUseLocation; import org.checkerframework.framework.source.Result; import org.checkerframework.framework.type.AnnotatedTypeFactory; import org.checkerframework.framework.type.AnnotatedTypeMirror; @@ -31,7 +50,9 @@ import org.checkerframework.framework.type.visitor.AnnotatedTypeScanner; import org.checkerframework.framework.util.AnnotatedTypes; import org.checkerframework.javacutil.AnnotationUtils; +import org.checkerframework.javacutil.CollectionUtils; import org.checkerframework.javacutil.ErrorReporter; +import org.checkerframework.javacutil.InternalUtils; import org.checkerframework.javacutil.Pair; import org.checkerframework.javacutil.TreeUtils; @@ -42,7 +63,7 @@ public class BaseTypeValidator extends AnnotatedTypeScanner implemen protected final BaseTypeChecker checker; protected final BaseTypeVisitor visitor; protected final AnnotatedTypeFactory atypeFactory; - + protected final TypeUseLocationValidator locationValidator; // TODO: clean up coupling between components public BaseTypeValidator( BaseTypeChecker checker, @@ -51,6 +72,7 @@ public BaseTypeValidator( this.checker = checker; this.visitor = visitor; this.atypeFactory = atypeFactory; + locationValidator = new TypeUseLocationValidator(this); } /** @@ -67,6 +89,10 @@ public BaseTypeValidator( public boolean isValid(AnnotatedTypeMirror type, Tree tree) { this.isValid = true; visit(type, tree); + // TODO doesn't passin class tree!!! + //System.out.println("Tree " + tree + " is being checked:"); + //System.out.println("\nEntry: type: " + type + " kind: " + type.getKind() + "\n"); + locationValidator.validate(type, tree); return this.isValid; } @@ -505,3 +531,519 @@ public boolean checkConflictingPrimaryAnnos(final AnnotatedTypeMirror type, fina return error; } } + +class TypeUseLocationValidator { + + TypeUseLocationValidatorImpl impl; + AnnotatedTypeMirror typeToValidateItsMainModifier; + BaseTypeValidator validator; + + TypeUseLocationValidator(BaseTypeValidator validator) { + impl = new TypeUseLocationValidatorImpl(validator); + } + + void validate(AnnotatedTypeMirror type, Tree tree) { + typeToValidateItsMainModifier = type; + // scan is different from visit on not reseting first(reset includes cleaning the visitedNodes map, even though if + // we use IdentityHashMap, it didn't act like an effective cache.) + impl.scan(typeToValidateItsMainModifier, tree); + }; + + // Only validates annotation against TypeUseLocatioins defined in TypeUseLocations enum. + // If in other locations, they are used, it's the type system specific validator's job to + // raise error. So, it's not TypeUseLocationValidator's job + class TypeUseLocationValidatorImpl extends AnnotatedTypeScanner { + + Set> checkedLocations; + private boolean isCheckingTypeArgument; + private boolean isCheckingArrayComponent; + // are we currently defaulting the lower bound of a type variable or wildcard + private boolean isLowerBound = false; + // are we currently defaulting the upper bound of a type variable or wildcard + private boolean isUpperBound = false; + // the bound type of the current wildcard or type variable being defaulted + private BoundType boundType = BoundType.UNBOUND; + private int count = 0; + private boolean printDebug = true; + private BaseTypeValidator typeValidator; + private static final int CACHE_SIZE = 300; + private final Map elementToBoundType = + CollectionUtils.createLRUCache(CACHE_SIZE); + Map visitedTypes; + + public TypeUseLocationValidatorImpl(BaseTypeValidator typeValidator) { + this.typeValidator = typeValidator; + checkedLocations = new HashSet<>(); + visitedTypes = new HashMap<>(); + } + + private Element getElement(Tree tree) { + Element elt; + switch (tree.getKind()) { + case MEMBER_SELECT: + elt = TreeUtils.elementFromUse((MemberSelectTree) tree); + break; + + case IDENTIFIER: + elt = TreeUtils.elementFromUse((IdentifierTree) tree); + break; + + case METHOD_INVOCATION: + elt = TreeUtils.elementFromUse((MethodInvocationTree) tree); + break; + + // TODO cases for array access, etc. -- every expression tree + // (The above probably means that we should use defaults in the + // scope of the declaration of the array. Is that right? -MDE) + /*case TYPE_PARAMETER: + elt = TreeUtils.elementFromDeclaration((TypeParameterTree) tree);*/ + default: + // If no associated symbol was found, use the tree's (lexical) + // scope. + elt = nearestEnclosingExceptLocal(tree); + // elt = nearestEnclosing(tree); + } + return elt; + } + + private Element nearestEnclosingExceptLocal(Tree tree) { + TreePath path = typeValidator.atypeFactory.getPath(tree); + if (path == null) { + Element method = typeValidator.atypeFactory.getEnclosingMethod(tree); + if (method != null) { + return method; + } else { + return InternalUtils.symbol(tree); + } + } + + Tree prev = null; + + for (Tree t : path) { + switch (t.getKind()) { + case VARIABLE: + VariableTree vtree = (VariableTree) t; + ExpressionTree vtreeInit = vtree.getInitializer(); + if (vtreeInit != null && prev == vtreeInit) { + Element elt = TreeUtils.elementFromDeclaration((VariableTree) t); + DefaultQualifier d = elt.getAnnotation(DefaultQualifier.class); + DefaultQualifiers ds = elt.getAnnotation(DefaultQualifiers.class); + + if (d == null && ds == null) { + break; + } + } + if (prev != null && prev.getKind() == Tree.Kind.MODIFIERS) { + // Annotations are modifiers. We do not want to apply the local variable default to + // annotations. Without this, test fenum/TestSwitch failed, because the default for + // an argument became incompatible with the declared type. + break; + } + return TreeUtils.elementFromDeclaration((VariableTree) t); + case METHOD: + return TreeUtils.elementFromDeclaration((MethodTree) t); + case CLASS: + case ENUM: + case INTERFACE: + case ANNOTATION_TYPE: + //System.out.println("Hit!"); + return TreeUtils.elementFromDeclaration((ClassTree) t); + //case TYPE_PARAMETER: + //return TreeUtils.elementFromUse((TypeParameterTree)t); + default: // Do nothing. continue finding parent paths + } + prev = t; + } + // Seems like dead code because there must be a matching case in the for loop and return immediately + return null; + } + + private void checkValidLocation( + AnnotatedTypeMirror type, Tree tree, TypeUseLocation location) { + //System.out.println("AnnotatedTypeVariable: " + type); + for (AnnotationMirror am : type.getAnnotations()) { + //System.out.println("Other than type variable hits"); + Element elementOfAnnotation = am.getAnnotationType().asElement(); + TargetLocations declLocations = + elementOfAnnotation.getAnnotation(TargetLocations.class); + // Null means no TargetLocations specified => Any use is correct. + if (declLocations != null) { + Set set = new HashSet<>(Arrays.asList(declLocations.value())); + if (set.contains(TypeUseLocation.ALL)) continue; + //System.out.println("contain?: " + set.contains(location) + "\n"); + //System.out.println("location: " + location); + //System.out.println("^^^^^^^^^^^^^^^^location is: " + location); + if (((location == TypeUseLocation.EXPLICIT_LOWER_BOUND) + || (location == TypeUseLocation.IMPLICIT_LOWER_BOUND)) + && set.contains(TypeUseLocation.LOWER_BOUND)) { + // TypeUseLocation.LOWER_BOUND already covers both explicit and implicit lower bounds, so no need to check containment + continue; + } else if (((location == TypeUseLocation.EXPLICIT_UPPER_BOUND) + || (location == TypeUseLocation.IMPLICIT_UPPER_BOUND)) + && set.contains(TypeUseLocation.UPPER_BOUND)) { + // TypeUseLocation.UPPER_BOUND already covers both explicit and implicit lower bounds, so no need to check containment + continue; + } else if (!set.contains(location)) reportLocationError(type, tree, location); + } + } + } + + private void reportLocationError( + AnnotatedTypeMirror type, Tree tree, TypeUseLocation location) { + //System.out.println("Error! => type: " + type + " tree: " + tree + " location: " + location); + //System.out.println("Error: " + location.toString().toLowerCase()); + com.sun.tools.javac.util.Pair target = + new com.sun.tools.javac.util.Pair<>(type, tree); + if (checkedLocations.contains(target)) return; + typeValidator.reportValidityResult( + location.toString().toLowerCase() + ".annotation.forbidden", type, tree); + checkedLocations.add(target); + typeValidator.isValid = false; + } + + @Override + protected Void scan(AnnotatedTypeMirror type, Tree p) { + // The "type" here is constantly changing while visiting different types, like type arguments, + // component, upper/lower bound. The "p" parameter is always passed the same from the entry of + // visitXXX method from the top construct until the last any visitXXX method. + Element elt = getElement(p); + ElementKind elementKind = elt.getKind(); + if (printDebug) { + System.out.println( + "\n===>" + + count++ + + ") " + + "Visiting " + + type + + " kind: " + + type.getKind()); + System.out.println("elt: " + elt + " resulteltkind: " + elt.getKind()); + System.out.println("tree is: " + p + " usedtreekind: " + p.getKind()); + if (visitedTypes.containsKey(type)) { + System.out.println("--- Skipped because visited"); + } + } + if (isCheckingTypeArgument) { + checkValidLocation(type, p, TypeUseLocation.TYPE_ARGUMENT); + } + if (isCheckingArrayComponent) { + checkValidLocation(type, p, TypeUseLocation.ARRAY_COMPONENT); + } + if (isCheckingTypeArgument || isCheckingArrayComponent) { + return super.scan(type, p); + } + if (p instanceof TypeParameterTree || p instanceof ClassTree) { + if (isUpperBound && boundType.isOneOf(BoundType.UPPER, BoundType.UNKNOWN)) { + //Explicit upper bound + checkValidLocation(type, p, TypeUseLocation.EXPLICIT_UPPER_BOUND); + } else if (isUpperBound && boundType.isOneOf(BoundType.UNBOUND, BoundType.LOWER)) { + // Implicit upper bound => Do nothing + // Do nothing + } else if (isUpperBound) { + // Upper bound + checkValidLocation(type, p, TypeUseLocation.UPPER_BOUND); + } + + if (isLowerBound && boundType.isOneOf(BoundType.LOWER)) { + // Explicit lower bound + checkValidLocation(type, p, TypeUseLocation.EXPLICIT_LOWER_BOUND); + } else if (isLowerBound + && boundType.isOneOf( + BoundType.UNBOUND, BoundType.UPPER, BoundType.UNKNOWN)) { + // Implicit lower bound + // Do nothing + } else if (isLowerBound) { + checkValidLocation(type, p, TypeUseLocation.LOWER_BOUND); + } + if (isUpperBound || isLowerBound) { + return super.scan(type, p); + } + } else if (p instanceof AnnotatedTypeTree) { + return super.scan(type, p); + } + if (type == typeToValidateItsMainModifier) { + switch (elementKind) { + case FIELD: + // Actual location IS Field! Need to check TypeUseLocation.FIELD is + //inside declared TypeUseLocation of the annotations on this element + checkValidLocation(type, p, TypeUseLocation.FIELD); + break; + case LOCAL_VARIABLE: + checkValidLocation(type, p, TypeUseLocation.LOCAL_VARIABLE); + break; + case RESOURCE_VARIABLE: + checkValidLocation(type, p, TypeUseLocation.RESOURCE_VARIABLE); + break; + case EXCEPTION_PARAMETER: + checkValidLocation(type, p, TypeUseLocation.EXCEPTION_PARAMETER); + break; + case PARAMETER: + // TODO method receciver and return type + if (elt.getSimpleName().contentEquals("this")) { + checkValidLocation(type, p, TypeUseLocation.RECEIVER); + } else { + checkValidLocation(type, p, TypeUseLocation.PARAMETER); + } + break; + case CONSTRUCTOR: + case METHOD: + checkValidLocation(type, p, TypeUseLocation.RETURN); + break; + case CLASS: + case INTERFACE: + case ANNOTATION_TYPE: + case ENUM: + // Not covered since type validator doesn't pass in class tree and a type to validate + //System.out.println("@@@tree: " + p); + // TODO validate class tree also in BaseTypeVisitor + //TODO: we get CLASS element kind for both type parameter tree and annotated type tree. + // The two tress are correct, and consistent with type. BUT, the Element gotton from tree + // has errors. And we use Element to process each location checking, so there are false + // warnings. Originally, these four cases are not supported, so didn't encountered this problem + // Update: this isClassTree if statement is basically because for upper bounds trees, it will + // extract class_name as element, and its kind is type_declaration. The major reason is that: + // getElement() methods returns wrong element for like List as Locations. So, we need + // to make sure we are using class trees, to ensure that we are really at type declaration position. + if (TreeUtils.isClassTree(p)) { + checkValidLocation(type, p, TypeUseLocation.TYPE_DECLARATION); + } + break; + default: + break; + } + } + return super.scan(type, p); + } + + @Override + public void reset() { + // We override one method not only it might be explicitly called in this subclass, but also may be + // a method which is called in superclass. Overriding this method will change the behaviour even if + // it's not explicitly called in overriding subclass. + super.reset(); + visitedTypes.clear(); + resetStates(); + } + + private void resetStates() { + isLowerBound = false; + isUpperBound = false; + boundType = BoundType.UNBOUND; + isCheckingTypeArgument = false; + isCheckingArrayComponent = false; + } + + @Override + public Void visitDeclared(AnnotatedDeclaredType type, Tree p) { + if (visitedTypes.containsKey(type)) { + return visitedTypes.get(type); + } + visitedTypes.put(type, null); + resetStates(); // Reset to check in clean environment without the correlating effect of fact like isCheckingArrayComponent is true, to avoid reporting error in wrong case + isCheckingTypeArgument = true; + scan(type.getTypeArguments(), p); + isCheckingTypeArgument = false; + return null; + } + + @Override + public Void visitArray(AnnotatedArrayType type, Tree p) { + resetStates(); // Reset to check in immediate context + isCheckingArrayComponent = true; + // Begin to check array component + scan(type.getComponentType(), p); + isCheckingArrayComponent = false; + return null; + } + + @Override + public Void visitTypeVariable(AnnotatedTypeVariable type, Tree p) { + if (visitedTypes.containsKey(type)) { + return visitedTypes.get(type); + } + visitedTypes.put(type, null); + resetStates(); + visitBounds(type, type.getUpperBound(), type.getLowerBound(), p); + return null; + } + + @Override + public Void visitWildcard(AnnotatedWildcardType type, Tree p) { + if (visitedTypes.containsKey(type)) { + return visitedTypes.get(type); + } + visitedTypes.put(type, null); + resetStates(); + visitBounds(type, type.getExtendsBound(), type.getSuperBound(), p); + return null; + } + + /** + * Visit the bounds of a type variable or a wildcard and potentially apply qual to those + * bounds. This method will also update the boundType, isLowerBound, and isUpperbound + * fields. + */ + protected void visitBounds( + AnnotatedTypeMirror boundedType, + AnnotatedTypeMirror upperBound, + AnnotatedTypeMirror lowerBound, + Tree p) { + //System.out.println("upper bound: " + upperBound); + final boolean prevIsUpperBound = isUpperBound; + final boolean prevIsLowerBound = isLowerBound; + final BoundType prevBoundType = boundType; + + boundType = getBoundType(boundedType, typeValidator.atypeFactory); + + try { + isLowerBound = true; + isUpperBound = false; + + scanAndReduce(lowerBound, p, null); + + //visitedTypes.put(boundedType, null); + + isLowerBound = false; + isUpperBound = true; + scanAndReduce(upperBound, p, null); + + //visitedTypes.put(boundedType, null); + + } finally { + isUpperBound = prevIsUpperBound; + isLowerBound = prevIsLowerBound; + boundType = prevBoundType; + } + } + /** + * @param type the type whose boundType is returned. + * type must be an AnnotatedWildcardType or AnnotatedTypeVariable + * @return the boundType for type + */ + private BoundType getBoundType( + final AnnotatedTypeMirror type, final AnnotatedTypeFactory typeFactory) { + if (type instanceof AnnotatedTypeVariable) { + return getTypeVarBoundType((AnnotatedTypeVariable) type, typeFactory); + } + + if (type instanceof AnnotatedWildcardType) { + return getWildcardBoundType((AnnotatedWildcardType) type, typeFactory); + } + + ErrorReporter.errorAbort("Unexpected type kind: type=" + type); + return null; // dead code + } + + /** + * @return the bound type of the input typeVar + */ + private BoundType getTypeVarBoundType( + final AnnotatedTypeVariable typeVar, final AnnotatedTypeFactory typeFactory) { + return getTypeVarBoundType( + (TypeParameterElement) typeVar.getUnderlyingType().asElement(), typeFactory); + } + + /** + * @return the boundType (UPPER, UNBOUND, or UNKNOWN) of the declaration of typeParamElem + */ + // Results are cached in {@link elementToBoundType}. + private BoundType getTypeVarBoundType( + final TypeParameterElement typeParamElem, final AnnotatedTypeFactory typeFactory) { + final BoundType prev = elementToBoundType.get(typeParamElem); + if (prev != null) { + return prev; + } + + TreePath declaredTypeVarEle = typeFactory.getTreeUtils().getPath(typeParamElem); + Tree typeParamDecl = declaredTypeVarEle == null ? null : declaredTypeVarEle.getLeaf(); + + final BoundType boundType; + if (typeParamDecl == null) { + // This is not only for elements from binaries, but also + // when the compilation unit is no longer available. + boundType = BoundType.UNKNOWN; + + } else { + if (typeParamDecl.getKind() == Tree.Kind.TYPE_PARAMETER) { + final TypeParameterTree tptree = (TypeParameterTree) typeParamDecl; + + List bnds = tptree.getBounds(); + if (bnds != null && !bnds.isEmpty()) { + boundType = BoundType.UPPER; + } else { + boundType = BoundType.UNBOUND; + } + } else { + ErrorReporter.errorAbort( + "Unexpected tree type for typeVar Element:\n" + + "typeParamElem=" + + typeParamElem + + "\n" + + typeParamDecl); + boundType = null; // dead code + } + } + + elementToBoundType.put(typeParamElem, boundType); + return boundType; + } + + /** + * @return the BoundType of annotatedWildcard. If it is unbounded, use the type parameter to + * which its an argument + */ + public BoundType getWildcardBoundType( + final AnnotatedWildcardType annotatedWildcard, + final AnnotatedTypeFactory typeFactory) { + + final WildcardType wildcard = (WildcardType) annotatedWildcard.getUnderlyingType(); + + final BoundType boundType; + if (wildcard.isUnbound() && wildcard.bound != null) { + boundType = + getTypeVarBoundType( + (TypeParameterElement) wildcard.bound.asElement(), typeFactory); + + } else { + // note: isSuperBound will be true for unbounded and lowers, but the unbounded case is already handled + boundType = wildcard.isSuperBound() ? BoundType.LOWER : BoundType.UPPER; + } + + return boundType; + } + } + + enum BoundType { + + /** + * Indicates an upper bounded type variable or wildcard + */ + UPPER, + + /** + * Indicates a lower bounded type variable or wildcard + */ + LOWER, + + /** + * Neither bound is specified, BOTH are implicit + */ + UNBOUND, + + /** + * For bytecode, or trees for which we no longer have the compilation unit. + * We treat UNKNOWN bounds as if they are an UPPER bound. + */ + UNKNOWN; + + public boolean isOneOf(final BoundType... choices) { + for (final BoundType choice : choices) { + if (this == choice) { + return true; + } + } + + return false; + } + } +} diff --git a/framework/src/org/checkerframework/common/basetype/messages.properties b/framework/src/org/checkerframework/common/basetype/messages.properties index b8ba2794745f..58a37b332cbb 100644 --- a/framework/src/org/checkerframework/common/basetype/messages.properties +++ b/framework/src/org/checkerframework/common/basetype/messages.properties @@ -83,10 +83,26 @@ contracts.conditional.postcondition.false.methodref.invalid=Conditional postcond lambda.unimplemented=This version of the Checker Framework does not type-check lambda expressions. methodref.inference.unimplemented=This version of the Checker Framework does not type-check method references with implicit type arguments. - field.invariant.not.found=the field invariant annotation refers to fields not found in a superclass\nfields not found: %s field.invariant.not.final=the field invariant annotation refers to fields that are not final\nfields not final: %s field.invariant.not.subtype=the qualifier for field %s is not a subtype of the declared type\nfound: %s\ndeclared type: %s field.invariant.not.wellformed=the field invariant annotation does not have equal numbers of fields and qualifiers. field.invariant.not.found.superclass=the field invariant annotation is missing fields that are listed in the superclass field invariant.\nfields not found: %s field.invariant.not.subtype.superclass=the qualifier for field %s is not a subtype of the qualifier in the superclass field invariant\nfound: %s\nsuperclass type: %s + +field.annotation.forbidden= %s is forbidden on field! +local_variable.annotation.forbidden= %s is forbidden on local variable! +resource_variable.annotation.forbidden= %s is forbidden on resource variable! +exception_parameter.annotation.forbidden= %s is forbidden on exception parameter! +receiver.annotation.forbidden= %s is forbidden on method receiver! +parameter.annotation.forbidden= %s is forbidden on method parameter! +return.annotation.forbidden= %s is forbidden on method return type! +lower_bound.annotation.forbidden= %s is forbidden on lower bound! +explicit_lower_bound.annotation.forbidden= %s is forbidden on explicit lower bound! +implicit_lower_bound.annotation.forbidden= %s is forbidden on implicit lower bound! +upper_bound.annotation.forbidden= %s is forbidden on upper bound! +explicit_upper_bound.annotation.forbidden= %s is forbidden on explicit upper bound! +implicit_upper_bound.annotation.forbidden= %s is forbidden on implicit upper bound! +type_declaration.annotation.forbidden= %s is forbidden on type declaration! +type_argument.annotation.forbidden= %s is forbidden on type argument! +array_component.annotation.forbidden= %s is forbidden on array component! diff --git a/framework/src/org/checkerframework/framework/qual/TypeUseLocation.java b/framework/src/org/checkerframework/framework/qual/TypeUseLocation.java index 7a50767265e4..4873965464e7 100644 --- a/framework/src/org/checkerframework/framework/qual/TypeUseLocation.java +++ b/framework/src/org/checkerframework/framework/qual/TypeUseLocation.java @@ -81,6 +81,21 @@ public enum TypeUseLocation { */ IMPLICIT_UPPER_BOUND, + /** + * Apply default annotations to unannotated type declarations: + * {@code @HERE class Demo{}} + */ + TYPE_DECLARATION, + /** + * Represents type argument location in parameterized type + * {@code List<@TA1 ArrayList<@TA2 String>>} + */ + TYPE_ARGUMENT, + /** + * Represents array component location + * {@code @AC2 String [] @AC1 []} + */ + ARRAY_COMPONENT, /** Apply if nothing more concrete is provided. TODO: clarify relation to ALL. */ OTHERWISE, diff --git a/framework/src/org/checkerframework/framework/util/defaults/QualifierDefaults.java b/framework/src/org/checkerframework/framework/util/defaults/QualifierDefaults.java index 6d311459124c..83383810ee2d 100644 --- a/framework/src/org/checkerframework/framework/util/defaults/QualifierDefaults.java +++ b/framework/src/org/checkerframework/framework/util/defaults/QualifierDefaults.java @@ -108,7 +108,8 @@ public class QualifierDefaults { TypeUseLocation.LOCAL_VARIABLE, TypeUseLocation.RESOURCE_VARIABLE, TypeUseLocation.EXCEPTION_PARAMETER, - TypeUseLocation.IMPLICIT_UPPER_BOUND + TypeUseLocation.IMPLICIT_UPPER_BOUND, + TypeUseLocation.TYPE_DECLARATION }; /** CLIMB locations whose standard default is bottom for a given type system. */ @@ -975,6 +976,15 @@ public Void scan(AnnotatedTypeMirror t, AnnotationMirror qual) { } break; } + case TYPE_DECLARATION: + { + if (scope != null + && (scope.getKind().isClass() || scope.getKind().isInterface()) + && t == type) { + addAnnotation(t, qual); + } + break; + } case OTHERWISE: case ALL: { From dfc93f4dc27e6dbc017e6184fcb8054f8cec3796 Mon Sep 17 00:00:00 2001 From: tamier Date: Sun, 16 Oct 2016 12:11:19 -0400 Subject: [PATCH 02/13] Refactor the implementation --- .../common/basetype/BaseTypeValidator.java | 673 +++++++----------- .../framework/util/BoundType.java | 35 + .../framework/util/BoundTypeUtil.java | 105 +++ 3 files changed, 390 insertions(+), 423 deletions(-) create mode 100644 framework/src/org/checkerframework/framework/util/BoundType.java create mode 100644 framework/src/org/checkerframework/framework/util/BoundTypeUtil.java diff --git a/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java b/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java index 6c3a7eddb02c..782d43c8e1e5 100644 --- a/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java +++ b/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java @@ -49,6 +49,8 @@ import org.checkerframework.framework.type.QualifierHierarchy; import org.checkerframework.framework.type.visitor.AnnotatedTypeScanner; import org.checkerframework.framework.util.AnnotatedTypes; +import org.checkerframework.framework.util.BoundType; +import org.checkerframework.framework.util.BoundTypeUtil; import org.checkerframework.javacutil.AnnotationUtils; import org.checkerframework.javacutil.CollectionUtils; import org.checkerframework.javacutil.ErrorReporter; @@ -63,7 +65,7 @@ public class BaseTypeValidator extends AnnotatedTypeScanner implemen protected final BaseTypeChecker checker; protected final BaseTypeVisitor visitor; protected final AnnotatedTypeFactory atypeFactory; - protected final TypeUseLocationValidator locationValidator; + protected final TargetLocationValidator locationValidator; // TODO: clean up coupling between components public BaseTypeValidator( BaseTypeChecker checker, @@ -72,7 +74,7 @@ public BaseTypeValidator( this.checker = checker; this.visitor = visitor; this.atypeFactory = atypeFactory; - locationValidator = new TypeUseLocationValidator(this); + locationValidator = new TargetLocationValidator(this); } /** @@ -532,182 +534,231 @@ public boolean checkConflictingPrimaryAnnos(final AnnotatedTypeMirror type, fina } } -class TypeUseLocationValidator { +class TargetLocationValidator { - TypeUseLocationValidatorImpl impl; - AnnotatedTypeMirror typeToValidateItsMainModifier; + TargetLocationValidatorImpl impl; BaseTypeValidator validator; + private boolean printDebug = false; + int count = 0; - TypeUseLocationValidator(BaseTypeValidator validator) { - impl = new TypeUseLocationValidatorImpl(validator); + TargetLocationValidator(BaseTypeValidator validator) { + this.validator = validator; + impl = new TargetLocationValidatorImpl(); } void validate(AnnotatedTypeMirror type, Tree tree) { - typeToValidateItsMainModifier = type; + if (printDebug) { + System.out.println( + "\n\n##################### [" + + count++ + + "] Entry: type: " + + type + + " kind: " + + type.getKind() + + " tree: " + + tree + + " treeKinid: " + + tree.getKind()); + } // scan is different from visit on not reseting first(reset includes cleaning the visitedNodes map, even though if // we use IdentityHashMap, it didn't act like an effective cache.) - impl.scan(typeToValidateItsMainModifier, tree); + validateMainModifier(type, tree); + impl.scan(type, tree); }; - // Only validates annotation against TypeUseLocatioins defined in TypeUseLocations enum. - // If in other locations, they are used, it's the type system specific validator's job to - // raise error. So, it's not TypeUseLocationValidator's job - class TypeUseLocationValidatorImpl extends AnnotatedTypeScanner { - - Set> checkedLocations; - private boolean isCheckingTypeArgument; - private boolean isCheckingArrayComponent; - // are we currently defaulting the lower bound of a type variable or wildcard - private boolean isLowerBound = false; - // are we currently defaulting the upper bound of a type variable or wildcard - private boolean isUpperBound = false; - // the bound type of the current wildcard or type variable being defaulted - private BoundType boundType = BoundType.UNBOUND; - private int count = 0; - private boolean printDebug = true; - private BaseTypeValidator typeValidator; - private static final int CACHE_SIZE = 300; - private final Map elementToBoundType = - CollectionUtils.createLRUCache(CACHE_SIZE); - Map visitedTypes; - - public TypeUseLocationValidatorImpl(BaseTypeValidator typeValidator) { - this.typeValidator = typeValidator; - checkedLocations = new HashSet<>(); - visitedTypes = new HashMap<>(); + private void validateMainModifier(AnnotatedTypeMirror type, Tree tree) { + Element elt = getElement(tree); + ElementKind elementKind = elt.getKind(); + switch (elementKind) { + case FIELD: + // Actual location IS Field! Need to check TypeUseLocation.FIELD is + //inside declared TypeUseLocation of the annotations on this element + checkValidLocation(type, tree, TypeUseLocation.FIELD); + break; + case LOCAL_VARIABLE: + checkValidLocation(type, tree, TypeUseLocation.LOCAL_VARIABLE); + break; + case RESOURCE_VARIABLE: + checkValidLocation(type, tree, TypeUseLocation.RESOURCE_VARIABLE); + break; + case EXCEPTION_PARAMETER: + checkValidLocation(type, tree, TypeUseLocation.EXCEPTION_PARAMETER); + break; + case PARAMETER: + // TODO method receciver and return type + if (elt.getSimpleName().contentEquals("this")) { + checkValidLocation(type, tree, TypeUseLocation.RECEIVER); + } else { + checkValidLocation(type, tree, TypeUseLocation.PARAMETER); + } + break; + case CONSTRUCTOR: + case METHOD: + // Upper bounf of type parameter declared in generic method after getting + // nearest enclosing element is also METHOD element. BUT its tree is no + // method tree. So, we add additional restriction: only when the tree is also + // method tree, they use is seen to be on method return. + if (tree.getKind() == Tree.Kind.METHOD) { + checkValidLocation(type, tree, TypeUseLocation.RETURN); + } + break; + case CLASS: + case INTERFACE: + case ANNOTATION_TYPE: + case ENUM: + // Not covered since type validator doesn't pass in class tree and a type to validate + //System.out.println("@@@tree: " + p); + // TODO validate class tree also in BaseTypeVisitor + //TODO: we get CLASS element kind for both type parameter tree and annotated type tree. + // The two tress are correct, and consistent with type. BUT, the Element gotton from tree + // has errors. And we use Element to process each location checking, so there are false + // warnings. Originally, these four cases are not supported, so didn't encountered this problem + // Update: this isClassTree if statement is basically because for upper bounds trees, it will + // extract class_name as element, and its kind is type_declaration. The major reason is that: + // getElement() methods returns wrong element for like List as Locations. So, we need + // to make sure we are using class trees, to ensure that we are really at type declaration position. + if (TreeUtils.isClassTree(tree)) { + checkValidLocation(type, tree, TypeUseLocation.TYPE_DECLARATION); + } + break; + default: + break; + } + } + + private Element getElement(Tree tree) { + Element elt; + switch (tree.getKind()) { + case MEMBER_SELECT: + elt = TreeUtils.elementFromUse((MemberSelectTree) tree); + break; + + case IDENTIFIER: + elt = TreeUtils.elementFromUse((IdentifierTree) tree); + break; + + case METHOD_INVOCATION: + elt = TreeUtils.elementFromUse((MethodInvocationTree) tree); + break; + + // TODO cases for array access, etc. -- every expression tree + // (The above probably means that we should use defaults in the + // scope of the declaration of the array. Is that right? -MDE) + + default: + // If no associated symbol was found, use the tree's (lexical) + // scope. + elt = nearestEnclosingExceptLocal(tree); } + return elt; + } - private Element getElement(Tree tree) { - Element elt; - switch (tree.getKind()) { - case MEMBER_SELECT: - elt = TreeUtils.elementFromUse((MemberSelectTree) tree); - break; - - case IDENTIFIER: - elt = TreeUtils.elementFromUse((IdentifierTree) tree); - break; - - case METHOD_INVOCATION: - elt = TreeUtils.elementFromUse((MethodInvocationTree) tree); - break; - - // TODO cases for array access, etc. -- every expression tree - // (The above probably means that we should use defaults in the - // scope of the declaration of the array. Is that right? -MDE) - /*case TYPE_PARAMETER: - elt = TreeUtils.elementFromDeclaration((TypeParameterTree) tree);*/ - default: - // If no associated symbol was found, use the tree's (lexical) - // scope. - elt = nearestEnclosingExceptLocal(tree); - // elt = nearestEnclosing(tree); + private Element nearestEnclosingExceptLocal(Tree tree) { + TreePath path = validator.atypeFactory.getPath(tree); + if (path == null) { + Element method = validator.atypeFactory.getEnclosingMethod(tree); + if (method != null) { + return method; + } else { + return InternalUtils.symbol(tree); } - return elt; } - private Element nearestEnclosingExceptLocal(Tree tree) { - TreePath path = typeValidator.atypeFactory.getPath(tree); - if (path == null) { - Element method = typeValidator.atypeFactory.getEnclosingMethod(tree); - if (method != null) { - return method; - } else { - return InternalUtils.symbol(tree); - } - } + Tree prev = null; - Tree prev = null; - - for (Tree t : path) { - switch (t.getKind()) { - case VARIABLE: - VariableTree vtree = (VariableTree) t; - ExpressionTree vtreeInit = vtree.getInitializer(); - if (vtreeInit != null && prev == vtreeInit) { - Element elt = TreeUtils.elementFromDeclaration((VariableTree) t); - DefaultQualifier d = elt.getAnnotation(DefaultQualifier.class); - DefaultQualifiers ds = elt.getAnnotation(DefaultQualifiers.class); - - if (d == null && ds == null) { - break; - } - } - if (prev != null && prev.getKind() == Tree.Kind.MODIFIERS) { - // Annotations are modifiers. We do not want to apply the local variable default to - // annotations. Without this, test fenum/TestSwitch failed, because the default for - // an argument became incompatible with the declared type. + for (Tree t : path) { + switch (t.getKind()) { + case VARIABLE: + VariableTree vtree = (VariableTree) t; + ExpressionTree vtreeInit = vtree.getInitializer(); + // TODO: evaluate comment-out of these lines + /*if (vtreeInit != null && prev == vtreeInit) { + Element elt = TreeUtils.elementFromDeclaration((VariableTree) t); + DefaultQualifier d = elt.getAnnotation(DefaultQualifier.class); + DefaultQualifiers ds = elt.getAnnotation(DefaultQualifiers.class); + + if (d == null && ds == null) { break; } - return TreeUtils.elementFromDeclaration((VariableTree) t); - case METHOD: - return TreeUtils.elementFromDeclaration((MethodTree) t); - case CLASS: - case ENUM: - case INTERFACE: - case ANNOTATION_TYPE: - //System.out.println("Hit!"); - return TreeUtils.elementFromDeclaration((ClassTree) t); - //case TYPE_PARAMETER: - //return TreeUtils.elementFromUse((TypeParameterTree)t); - default: // Do nothing. continue finding parent paths - } - prev = t; + }*/ + if (prev != null && prev.getKind() == Tree.Kind.MODIFIERS) { + // Annotations are modifiers. We do not want to apply the local variable default to + // annotations. Without this, test fenum/TestSwitch failed, because the default for + // an argument became incompatible with the declared type. + break; + } + return TreeUtils.elementFromDeclaration((VariableTree) t); + case METHOD: + return TreeUtils.elementFromDeclaration((MethodTree) t); + case CLASS: + case ENUM: + case INTERFACE: + case ANNOTATION_TYPE: + //System.out.println("Hit!"); + return TreeUtils.elementFromDeclaration((ClassTree) t); + //case TYPE_PARAMETER: + //return TreeUtils.elementFromUse((TypeParameterTree)t); + default: // Do nothing. continue finding parent paths } - // Seems like dead code because there must be a matching case in the for loop and return immediately - return null; + prev = t; } + // Seems like dead code because there must be a matching case in the for loop and return immediately + return null; + } - private void checkValidLocation( - AnnotatedTypeMirror type, Tree tree, TypeUseLocation location) { - //System.out.println("AnnotatedTypeVariable: " + type); - for (AnnotationMirror am : type.getAnnotations()) { - //System.out.println("Other than type variable hits"); - Element elementOfAnnotation = am.getAnnotationType().asElement(); - TargetLocations declLocations = - elementOfAnnotation.getAnnotation(TargetLocations.class); - // Null means no TargetLocations specified => Any use is correct. - if (declLocations != null) { - Set set = new HashSet<>(Arrays.asList(declLocations.value())); - if (set.contains(TypeUseLocation.ALL)) continue; - //System.out.println("contain?: " + set.contains(location) + "\n"); - //System.out.println("location: " + location); - //System.out.println("^^^^^^^^^^^^^^^^location is: " + location); - if (((location == TypeUseLocation.EXPLICIT_LOWER_BOUND) - || (location == TypeUseLocation.IMPLICIT_LOWER_BOUND)) - && set.contains(TypeUseLocation.LOWER_BOUND)) { - // TypeUseLocation.LOWER_BOUND already covers both explicit and implicit lower bounds, so no need to check containment - continue; - } else if (((location == TypeUseLocation.EXPLICIT_UPPER_BOUND) - || (location == TypeUseLocation.IMPLICIT_UPPER_BOUND)) - && set.contains(TypeUseLocation.UPPER_BOUND)) { - // TypeUseLocation.UPPER_BOUND already covers both explicit and implicit lower bounds, so no need to check containment - continue; - } else if (!set.contains(location)) reportLocationError(type, tree, location); - } + private void checkValidLocation(AnnotatedTypeMirror type, Tree tree, TypeUseLocation location) { + for (AnnotationMirror am : type.getAnnotations()) { + Element elementOfAnnotation = am.getAnnotationType().asElement(); + TargetLocations declLocations = + elementOfAnnotation.getAnnotation(TargetLocations.class); + // Null means no TargetLocations specified => Any use is correct. + if (declLocations != null) { + Set set = new HashSet<>(Arrays.asList(declLocations.value())); + if (set.contains(TypeUseLocation.ALL)) continue; + if (((location == TypeUseLocation.EXPLICIT_LOWER_BOUND) + || (location == TypeUseLocation.IMPLICIT_LOWER_BOUND)) + && set.contains(TypeUseLocation.LOWER_BOUND)) { + // TypeUseLocation.LOWER_BOUND already covers both explicit and implicit lower bounds, so no need to check containment + continue; + } else if (((location == TypeUseLocation.EXPLICIT_UPPER_BOUND) + || (location == TypeUseLocation.IMPLICIT_UPPER_BOUND)) + && set.contains(TypeUseLocation.UPPER_BOUND)) { + // TypeUseLocation.UPPER_BOUND already covers both explicit and implicit lower bounds, so no need to check containment + continue; + } else if (!set.contains(location)) reportLocationError(type, tree, location); } } + } - private void reportLocationError( - AnnotatedTypeMirror type, Tree tree, TypeUseLocation location) { - //System.out.println("Error! => type: " + type + " tree: " + tree + " location: " + location); - //System.out.println("Error: " + location.toString().toLowerCase()); - com.sun.tools.javac.util.Pair target = - new com.sun.tools.javac.util.Pair<>(type, tree); - if (checkedLocations.contains(target)) return; - typeValidator.reportValidityResult( - location.toString().toLowerCase() + ".annotation.forbidden", type, tree); - checkedLocations.add(target); - typeValidator.isValid = false; - } + private void reportLocationError( + AnnotatedTypeMirror type, Tree tree, TypeUseLocation location) { + if (printDebug) { + System.out.println( + "-----!!!----- Error => type: " + + type + + " \ntree: " + + tree + + " location: " + + location.toString().toLowerCase()); + } + // TODO check the effect of removing cache for type and tree while reporting error + validator.reportValidityResult( + location.toString().toLowerCase() + ".annotation.forbidden", type, tree); + validator.isValid = false; + } + + // Only validates annotation against TypeUseLocatioins defined in TypeUseLocations enum. + // If in other locations, they are used, it's the type system specific validator's job to + // raise error. So, it's not TypeUseLocationValidator's job + class TargetLocationValidatorImpl extends AnnotatedTypeScanner { + + private int count = 0; @Override protected Void scan(AnnotatedTypeMirror type, Tree p) { // The "type" here is constantly changing while visiting different types, like type arguments, // component, upper/lower bound. The "p" parameter is always passed the same from the entry of // visitXXX method from the top construct until the last any visitXXX method. - Element elt = getElement(p); - ElementKind elementKind = elt.getKind(); if (printDebug) { System.out.println( "\n===>" @@ -717,163 +768,59 @@ protected Void scan(AnnotatedTypeMirror type, Tree p) { + type + " kind: " + type.getKind()); + Element elt = getElement(p); System.out.println("elt: " + elt + " resulteltkind: " + elt.getKind()); System.out.println("tree is: " + p + " usedtreekind: " + p.getKind()); - if (visitedTypes.containsKey(type)) { + if (visitedNodes.containsKey(type)) { System.out.println("--- Skipped because visited"); } } - if (isCheckingTypeArgument) { - checkValidLocation(type, p, TypeUseLocation.TYPE_ARGUMENT); - } - if (isCheckingArrayComponent) { - checkValidLocation(type, p, TypeUseLocation.ARRAY_COMPONENT); - } - if (isCheckingTypeArgument || isCheckingArrayComponent) { - return super.scan(type, p); - } - if (p instanceof TypeParameterTree || p instanceof ClassTree) { - if (isUpperBound && boundType.isOneOf(BoundType.UPPER, BoundType.UNKNOWN)) { - //Explicit upper bound - checkValidLocation(type, p, TypeUseLocation.EXPLICIT_UPPER_BOUND); - } else if (isUpperBound && boundType.isOneOf(BoundType.UNBOUND, BoundType.LOWER)) { - // Implicit upper bound => Do nothing - // Do nothing - } else if (isUpperBound) { - // Upper bound - checkValidLocation(type, p, TypeUseLocation.UPPER_BOUND); - } - if (isLowerBound && boundType.isOneOf(BoundType.LOWER)) { - // Explicit lower bound - checkValidLocation(type, p, TypeUseLocation.EXPLICIT_LOWER_BOUND); - } else if (isLowerBound - && boundType.isOneOf( - BoundType.UNBOUND, BoundType.UPPER, BoundType.UNKNOWN)) { - // Implicit lower bound - // Do nothing - } else if (isLowerBound) { - checkValidLocation(type, p, TypeUseLocation.LOWER_BOUND); - } - if (isUpperBound || isLowerBound) { - return super.scan(type, p); - } - } else if (p instanceof AnnotatedTypeTree) { - return super.scan(type, p); - } - if (type == typeToValidateItsMainModifier) { - switch (elementKind) { - case FIELD: - // Actual location IS Field! Need to check TypeUseLocation.FIELD is - //inside declared TypeUseLocation of the annotations on this element - checkValidLocation(type, p, TypeUseLocation.FIELD); - break; - case LOCAL_VARIABLE: - checkValidLocation(type, p, TypeUseLocation.LOCAL_VARIABLE); - break; - case RESOURCE_VARIABLE: - checkValidLocation(type, p, TypeUseLocation.RESOURCE_VARIABLE); - break; - case EXCEPTION_PARAMETER: - checkValidLocation(type, p, TypeUseLocation.EXCEPTION_PARAMETER); - break; - case PARAMETER: - // TODO method receciver and return type - if (elt.getSimpleName().contentEquals("this")) { - checkValidLocation(type, p, TypeUseLocation.RECEIVER); - } else { - checkValidLocation(type, p, TypeUseLocation.PARAMETER); - } - break; - case CONSTRUCTOR: - case METHOD: - checkValidLocation(type, p, TypeUseLocation.RETURN); - break; - case CLASS: - case INTERFACE: - case ANNOTATION_TYPE: - case ENUM: - // Not covered since type validator doesn't pass in class tree and a type to validate - //System.out.println("@@@tree: " + p); - // TODO validate class tree also in BaseTypeVisitor - //TODO: we get CLASS element kind for both type parameter tree and annotated type tree. - // The two tress are correct, and consistent with type. BUT, the Element gotton from tree - // has errors. And we use Element to process each location checking, so there are false - // warnings. Originally, these four cases are not supported, so didn't encountered this problem - // Update: this isClassTree if statement is basically because for upper bounds trees, it will - // extract class_name as element, and its kind is type_declaration. The major reason is that: - // getElement() methods returns wrong element for like List as Locations. So, we need - // to make sure we are using class trees, to ensure that we are really at type declaration position. - if (TreeUtils.isClassTree(p)) { - checkValidLocation(type, p, TypeUseLocation.TYPE_DECLARATION); - } - break; - default: - break; - } - } return super.scan(type, p); } - @Override - public void reset() { - // We override one method not only it might be explicitly called in this subclass, but also may be - // a method which is called in superclass. Overriding this method will change the behaviour even if - // it's not explicitly called in overriding subclass. - super.reset(); - visitedTypes.clear(); - resetStates(); - } - - private void resetStates() { - isLowerBound = false; - isUpperBound = false; - boundType = BoundType.UNBOUND; - isCheckingTypeArgument = false; - isCheckingArrayComponent = false; - } - @Override public Void visitDeclared(AnnotatedDeclaredType type, Tree p) { - if (visitedTypes.containsKey(type)) { - return visitedTypes.get(type); + if (visitedNodes.containsKey(type)) { + return visitedNodes.get(type); + } + visitedNodes.put(type, null); + // Not check if tree p is a "wide" class declaration tree + if (!TreeUtils.isClassTree(p)) { + for (AnnotatedTypeMirror tArg : type.getTypeArguments()) { + checkValidLocation(tArg, p, TypeUseLocation.TYPE_ARGUMENT); + } + scan(type.getTypeArguments(), p); } - visitedTypes.put(type, null); - resetStates(); // Reset to check in clean environment without the correlating effect of fact like isCheckingArrayComponent is true, to avoid reporting error in wrong case - isCheckingTypeArgument = true; - scan(type.getTypeArguments(), p); - isCheckingTypeArgument = false; return null; } @Override public Void visitArray(AnnotatedArrayType type, Tree p) { - resetStates(); // Reset to check in immediate context - isCheckingArrayComponent = true; // Begin to check array component + checkValidLocation(type.getComponentType(), p, TypeUseLocation.ARRAY_COMPONENT); scan(type.getComponentType(), p); - isCheckingArrayComponent = false; return null; } @Override public Void visitTypeVariable(AnnotatedTypeVariable type, Tree p) { - if (visitedTypes.containsKey(type)) { - return visitedTypes.get(type); + if (visitedNodes.containsKey(type)) { + return visitedNodes.get(type); + } + visitedNodes.put(type, null); + if (type.isDeclaration()) { + visitBounds(type, type.getUpperBound(), type.getLowerBound(), p); } - visitedTypes.put(type, null); - resetStates(); - visitBounds(type, type.getUpperBound(), type.getLowerBound(), p); return null; } @Override public Void visitWildcard(AnnotatedWildcardType type, Tree p) { - if (visitedTypes.containsKey(type)) { - return visitedTypes.get(type); + if (visitedNodes.containsKey(type)) { + return visitedNodes.get(type); } - visitedTypes.put(type, null); - resetStates(); + visitedNodes.put(type, null); visitBounds(type, type.getExtendsBound(), type.getSuperBound(), p); return null; } @@ -888,162 +835,42 @@ protected void visitBounds( AnnotatedTypeMirror upperBound, AnnotatedTypeMirror lowerBound, Tree p) { - //System.out.println("upper bound: " + upperBound); - final boolean prevIsUpperBound = isUpperBound; - final boolean prevIsLowerBound = isLowerBound; - final BoundType prevBoundType = boundType; - - boundType = getBoundType(boundedType, typeValidator.atypeFactory); - - try { - isLowerBound = true; - isUpperBound = false; - - scanAndReduce(lowerBound, p, null); - - //visitedTypes.put(boundedType, null); - - isLowerBound = false; - isUpperBound = true; - scanAndReduce(upperBound, p, null); - - //visitedTypes.put(boundedType, null); - - } finally { - isUpperBound = prevIsUpperBound; - isLowerBound = prevIsLowerBound; - boundType = prevBoundType; - } - } - /** - * @param type the type whose boundType is returned. - * type must be an AnnotatedWildcardType or AnnotatedTypeVariable - * @return the boundType for type - */ - private BoundType getBoundType( - final AnnotatedTypeMirror type, final AnnotatedTypeFactory typeFactory) { - if (type instanceof AnnotatedTypeVariable) { - return getTypeVarBoundType((AnnotatedTypeVariable) type, typeFactory); - } - - if (type instanceof AnnotatedWildcardType) { - return getWildcardBoundType((AnnotatedWildcardType) type, typeFactory); - } - - ErrorReporter.errorAbort("Unexpected type kind: type=" + type); - return null; // dead code - } - - /** - * @return the bound type of the input typeVar - */ - private BoundType getTypeVarBoundType( - final AnnotatedTypeVariable typeVar, final AnnotatedTypeFactory typeFactory) { - return getTypeVarBoundType( - (TypeParameterElement) typeVar.getUnderlyingType().asElement(), typeFactory); - } - - /** - * @return the boundType (UPPER, UNBOUND, or UNKNOWN) of the declaration of typeParamElem - */ - // Results are cached in {@link elementToBoundType}. - private BoundType getTypeVarBoundType( - final TypeParameterElement typeParamElem, final AnnotatedTypeFactory typeFactory) { - final BoundType prev = elementToBoundType.get(typeParamElem); - if (prev != null) { - return prev; - } - - TreePath declaredTypeVarEle = typeFactory.getTreeUtils().getPath(typeParamElem); - Tree typeParamDecl = declaredTypeVarEle == null ? null : declaredTypeVarEle.getLeaf(); - final BoundType boundType; - if (typeParamDecl == null) { - // This is not only for elements from binaries, but also - // when the compilation unit is no longer available. - boundType = BoundType.UNKNOWN; - - } else { - if (typeParamDecl.getKind() == Tree.Kind.TYPE_PARAMETER) { - final TypeParameterTree tptree = (TypeParameterTree) typeParamDecl; - - List bnds = tptree.getBounds(); - if (bnds != null && !bnds.isEmpty()) { - boundType = BoundType.UPPER; - } else { - boundType = BoundType.UNBOUND; - } + BoundType boundType = BoundTypeUtil.getBoundType(boundedType, validator.atypeFactory); + // TODO Is this correct to use this as condition to check if it's type parameter declaration + // Checking lower bound + if (p.getKind() == Tree.Kind.TYPE_PARAMETER) { + if (boundType.isOneOf(BoundType.LOWER)) { + // Explicit lower bound + checkValidLocation(lowerBound, p, TypeUseLocation.EXPLICIT_LOWER_BOUND); + } else if (boundType.isOneOf( + BoundType.UNBOUND, BoundType.UPPER, BoundType.UNKNOWN)) { + // Implicit lower bound + // Do nothing } else { - ErrorReporter.errorAbort( - "Unexpected tree type for typeVar Element:\n" - + "typeParamElem=" - + typeParamElem - + "\n" - + typeParamDecl); - boundType = null; // dead code + checkValidLocation(lowerBound, p, TypeUseLocation.LOWER_BOUND); } } + // We only need to validate explicit main annotation on lower/upper bounds. So no need to + // visit recursively. They will be scan afterwards in different trees + //scanAndReduce(lowerBound, p, null); - elementToBoundType.put(typeParamElem, boundType); - return boundType; - } - - /** - * @return the BoundType of annotatedWildcard. If it is unbounded, use the type parameter to - * which its an argument - */ - public BoundType getWildcardBoundType( - final AnnotatedWildcardType annotatedWildcard, - final AnnotatedTypeFactory typeFactory) { - - final WildcardType wildcard = (WildcardType) annotatedWildcard.getUnderlyingType(); - - final BoundType boundType; - if (wildcard.isUnbound() && wildcard.bound != null) { - boundType = - getTypeVarBoundType( - (TypeParameterElement) wildcard.bound.asElement(), typeFactory); - - } else { - // note: isSuperBound will be true for unbounded and lowers, but the unbounded case is already handled - boundType = wildcard.isSuperBound() ? BoundType.LOWER : BoundType.UPPER; - } - - return boundType; - } - } - - enum BoundType { - - /** - * Indicates an upper bounded type variable or wildcard - */ - UPPER, - - /** - * Indicates a lower bounded type variable or wildcard - */ - LOWER, - - /** - * Neither bound is specified, BOTH are implicit - */ - UNBOUND, - - /** - * For bytecode, or trees for which we no longer have the compilation unit. - * We treat UNKNOWN bounds as if they are an UPPER bound. - */ - UNKNOWN; - - public boolean isOneOf(final BoundType... choices) { - for (final BoundType choice : choices) { - if (this == choice) { - return true; + // Checking upper bound + if (p.getKind() == Tree.Kind.TYPE_PARAMETER) { + if (boundType.isOneOf(BoundType.UPPER, BoundType.UNKNOWN)) { + //Explicit upper bound + checkValidLocation(upperBound, p, TypeUseLocation.EXPLICIT_UPPER_BOUND); + } else if (boundType.isOneOf(BoundType.UNBOUND, BoundType.LOWER)) { + // Implicit upper bound => Do nothing + // Do nothing + } else { + // Upper bound + checkValidLocation(upperBound, p, TypeUseLocation.UPPER_BOUND); } } - - return false; + // We only need to validate explicit main annotation on lower/upper bounds. So no need to + // visit recursively. They will be scan afterwards in different trees + //scanAndReduce(upperBound, p, null); } } } diff --git a/framework/src/org/checkerframework/framework/util/BoundType.java b/framework/src/org/checkerframework/framework/util/BoundType.java new file mode 100644 index 000000000000..4dc2883ce8b0 --- /dev/null +++ b/framework/src/org/checkerframework/framework/util/BoundType.java @@ -0,0 +1,35 @@ +package org.checkerframework.framework.util; + +public enum BoundType { + + /** + * Indicates an upper bounded type variable or wildcard + */ + UPPER, + + /** + * Indicates a lower bounded type variable or wildcard + */ + LOWER, + + /** + * Neither bound is specified, BOTH are implicit + */ + UNBOUND, + + /** + * For bytecode, or trees for which we no longer have the compilation unit. + * We treat UNKNOWN bounds as if they are an UPPER bound. + */ + UNKNOWN; + + public boolean isOneOf(final BoundType... choices) { + for (final BoundType choice : choices) { + if (this == choice) { + return true; + } + } + + return false; + } +} diff --git a/framework/src/org/checkerframework/framework/util/BoundTypeUtil.java b/framework/src/org/checkerframework/framework/util/BoundTypeUtil.java new file mode 100644 index 000000000000..df1dba81a928 --- /dev/null +++ b/framework/src/org/checkerframework/framework/util/BoundTypeUtil.java @@ -0,0 +1,105 @@ +package org.checkerframework.framework.util; + +import com.sun.source.tree.Tree; +import com.sun.source.tree.TypeParameterTree; +import com.sun.source.util.TreePath; +import com.sun.tools.javac.code.Type.WildcardType; +import java.util.List; +import javax.lang.model.element.TypeParameterElement; +import org.checkerframework.framework.type.AnnotatedTypeFactory; +import org.checkerframework.framework.type.AnnotatedTypeMirror; +import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedTypeVariable; +import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedWildcardType; +import org.checkerframework.javacutil.ErrorReporter; + +public class BoundTypeUtil { + /** + * @param type the type whose boundType is returned. + * type must be an AnnotatedWildcardType or AnnotatedTypeVariable + * @return the boundType for type + */ + public static BoundType getBoundType( + final AnnotatedTypeMirror type, final AnnotatedTypeFactory typeFactory) { + if (type instanceof AnnotatedTypeVariable) { + return getTypeVarBoundType((AnnotatedTypeVariable) type, typeFactory); + } + + if (type instanceof AnnotatedWildcardType) { + return getWildcardBoundType((AnnotatedWildcardType) type, typeFactory); + } + + ErrorReporter.errorAbort("Unexpected type kind: type=" + type); + return null; // dead code + } + + /** + * @return the bound type of the input typeVar + */ + public static BoundType getTypeVarBoundType( + final AnnotatedTypeVariable typeVar, final AnnotatedTypeFactory typeFactory) { + return getTypeVarBoundType( + (TypeParameterElement) typeVar.getUnderlyingType().asElement(), typeFactory); + } + + /** + * @return the boundType (UPPER, UNBOUND, or UNKNOWN) of the declaration of typeParamElem + */ + public static BoundType getTypeVarBoundType( + final TypeParameterElement typeParamElem, final AnnotatedTypeFactory typeFactory) { + + TreePath declaredTypeVarEle = typeFactory.getTreeUtils().getPath(typeParamElem); + Tree typeParamDecl = declaredTypeVarEle == null ? null : declaredTypeVarEle.getLeaf(); + + final BoundType boundType; + if (typeParamDecl == null) { + // This is not only for elements from binaries, but also + // when the compilation unit is no longer available. + boundType = BoundType.UNKNOWN; + + } else { + if (typeParamDecl.getKind() == Tree.Kind.TYPE_PARAMETER) { + final TypeParameterTree tptree = (TypeParameterTree) typeParamDecl; + + List bnds = tptree.getBounds(); + if (bnds != null && !bnds.isEmpty()) { + boundType = BoundType.UPPER; + } else { + boundType = BoundType.UNBOUND; + } + } else { + ErrorReporter.errorAbort( + "Unexpected tree type for typeVar Element:\n" + + "typeParamElem=" + + typeParamElem + + "\n" + + typeParamDecl); + boundType = null; // dead code + } + } + + return boundType; + } + + /** + * @return the BoundType of annotatedWildcard. If it is unbounded, use the type parameter to + * which its an argument + */ + public static BoundType getWildcardBoundType( + final AnnotatedWildcardType annotatedWildcard, final AnnotatedTypeFactory typeFactory) { + + final WildcardType wildcard = (WildcardType) annotatedWildcard.getUnderlyingType(); + + final BoundType boundType; + if (wildcard.isUnbound() && wildcard.bound != null) { + boundType = + getTypeVarBoundType( + (TypeParameterElement) wildcard.bound.asElement(), typeFactory); + + } else { + // note: isSuperBound will be true for unbounded and lowers, but the unbounded case is already handled + boundType = wildcard.isSuperBound() ? BoundType.LOWER : BoundType.UPPER; + } + + return boundType; + } +} From 1108f8f91209568c4790daf079640cff4708b291 Mon Sep 17 00:00:00 2001 From: tamier Date: Sun, 16 Oct 2016 12:12:00 -0400 Subject: [PATCH 03/13] Validate class tree and don't validate method tree again --- .../common/basetype/BaseTypeVisitor.java | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java index fc2b68834457..e6876aa4bd93 100644 --- a/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -293,6 +293,27 @@ public final Void visitClass(ClassTree classTree, Void p) { visitorState.setMethodTree(null); visitorState.setAssignmentContext(null); try { + if (!TreeUtils.hasExplicitConstructor(classTree)) { + checkDefaultConstructor(classTree); + } + + // We validate class tree to see if the annotation on it is allowed on type declaration location + validateTypeOf(classTree); + /* Visit the extends and implements clauses. + * The superclass also visits them, but only calls visitParameterizedType, which + * loses a main modifier. + */ + Tree ext = classTree.getExtendsClause(); + if (ext != null) { + validateTypeOf(ext); + } + + List impls = classTree.getImplementsClause(); + if (impls != null) { + for (Tree im : impls) { + validateTypeOf(im); + } + } processClassTree(classTree); atypeFactory.postProcessClassTree(classTree); } finally { @@ -1312,10 +1333,9 @@ public Void visitReturn(ReturnTree node, Void p) { if (enclosing.getKind() == Tree.Kind.METHOD) { MethodTree enclosingMethod = TreeUtils.enclosingMethod(getCurrentPath()); - boolean valid = validateTypeOf(enclosing); - if (valid) { - ret = atypeFactory.getMethodReturnType(enclosingMethod, node); - } + // We don't validate enclosingMethod again, because it's already been validated during visitMethod + ret = atypeFactory.getMethodReturnType(enclosingMethod, node); + } else { Pair result = atypeFactory.getFnInterfaceFromTree((LambdaExpressionTree) enclosing); @@ -1871,6 +1891,7 @@ protected void commonAssignmentCheck( if (shouldSkipUses(valueExp)) { return; } + if (varType.getKind() == TypeKind.ARRAY && valueExp instanceof NewArrayTree && ((NewArrayTree) valueExp).getType() == null) { From bbcd872972ac457430026f672c7cc7d17b78cd9e Mon Sep 17 00:00:00 2001 From: tamier Date: Mon, 17 Oct 2016 15:15:36 -0400 Subject: [PATCH 04/13] Remove unncessary inner class TargetLocationValidatorImpl --- .../common/basetype/BaseTypeValidator.java | 225 +++++++++--------- 1 file changed, 108 insertions(+), 117 deletions(-) diff --git a/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java b/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java index 782d43c8e1e5..6d7e81d12a28 100644 --- a/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java +++ b/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java @@ -534,23 +534,22 @@ public boolean checkConflictingPrimaryAnnos(final AnnotatedTypeMirror type, fina } } -class TargetLocationValidator { +class TargetLocationValidator extends AnnotatedTypeScanner { - TargetLocationValidatorImpl impl; BaseTypeValidator validator; private boolean printDebug = false; - int count = 0; + int countEntry = 0; + private int countScan = 0; TargetLocationValidator(BaseTypeValidator validator) { this.validator = validator; - impl = new TargetLocationValidatorImpl(); } void validate(AnnotatedTypeMirror type, Tree tree) { if (printDebug) { System.out.println( "\n\n##################### [" - + count++ + + countEntry++ + "] Entry: type: " + type + " kind: " @@ -563,7 +562,7 @@ void validate(AnnotatedTypeMirror type, Tree tree) { // scan is different from visit on not reseting first(reset includes cleaning the visitedNodes map, even though if // we use IdentityHashMap, it didn't act like an effective cache.) validateMainModifier(type, tree); - impl.scan(type, tree); + scan(type, tree); }; private void validateMainModifier(AnnotatedTypeMirror type, Tree tree) { @@ -747,130 +746,122 @@ private void reportLocationError( validator.isValid = false; } - // Only validates annotation against TypeUseLocatioins defined in TypeUseLocations enum. - // If in other locations, they are used, it's the type system specific validator's job to - // raise error. So, it's not TypeUseLocationValidator's job - class TargetLocationValidatorImpl extends AnnotatedTypeScanner { - - private int count = 0; - - @Override - protected Void scan(AnnotatedTypeMirror type, Tree p) { - // The "type" here is constantly changing while visiting different types, like type arguments, - // component, upper/lower bound. The "p" parameter is always passed the same from the entry of - // visitXXX method from the top construct until the last any visitXXX method. - if (printDebug) { - System.out.println( - "\n===>" - + count++ - + ") " - + "Visiting " - + type - + " kind: " - + type.getKind()); - Element elt = getElement(p); - System.out.println("elt: " + elt + " resulteltkind: " + elt.getKind()); - System.out.println("tree is: " + p + " usedtreekind: " + p.getKind()); - if (visitedNodes.containsKey(type)) { - System.out.println("--- Skipped because visited"); - } + @Override + protected Void scan(AnnotatedTypeMirror type, Tree p) { + // The "type" here is constantly changing while visiting different types, like type arguments, + // component, upper/lower bound. The "p" parameter is always passed the same from the entry of + // visitXXX method from the top construct until the last any visitXXX method. + if (printDebug) { + System.out.println( + "\n===>" + + countScan++ + + ") " + + "Visiting " + + type + + " kind: " + + type.getKind()); + Element elt = getElement(p); + System.out.println("elt: " + elt + " resulteltkind: " + elt.getKind()); + System.out.println("tree is: " + p + " usedtreekind: " + p.getKind()); + if (visitedNodes.containsKey(type)) { + System.out.println("--- Skipped because visited"); } - - return super.scan(type, p); } - @Override - public Void visitDeclared(AnnotatedDeclaredType type, Tree p) { - if (visitedNodes.containsKey(type)) { - return visitedNodes.get(type); - } - visitedNodes.put(type, null); - // Not check if tree p is a "wide" class declaration tree - if (!TreeUtils.isClassTree(p)) { - for (AnnotatedTypeMirror tArg : type.getTypeArguments()) { - checkValidLocation(tArg, p, TypeUseLocation.TYPE_ARGUMENT); - } - scan(type.getTypeArguments(), p); + return super.scan(type, p); + } + + @Override + public Void visitDeclared(AnnotatedDeclaredType type, Tree p) { + if (visitedNodes.containsKey(type)) { + return visitedNodes.get(type); + } + visitedNodes.put(type, null); + // Not check if tree p is a "wide" class declaration tree + if (!TreeUtils.isClassTree(p)) { + for (AnnotatedTypeMirror tArg : type.getTypeArguments()) { + checkValidLocation(tArg, p, TypeUseLocation.TYPE_ARGUMENT); } - return null; + scan(type.getTypeArguments(), p); } + return null; + } - @Override - public Void visitArray(AnnotatedArrayType type, Tree p) { - // Begin to check array component - checkValidLocation(type.getComponentType(), p, TypeUseLocation.ARRAY_COMPONENT); - scan(type.getComponentType(), p); - return null; - } + @Override + public Void visitArray(AnnotatedArrayType type, Tree p) { + // Begin to check array component + checkValidLocation(type.getComponentType(), p, TypeUseLocation.ARRAY_COMPONENT); + scan(type.getComponentType(), p); + return null; + } - @Override - public Void visitTypeVariable(AnnotatedTypeVariable type, Tree p) { - if (visitedNodes.containsKey(type)) { - return visitedNodes.get(type); - } - visitedNodes.put(type, null); - if (type.isDeclaration()) { - visitBounds(type, type.getUpperBound(), type.getLowerBound(), p); - } - return null; + @Override + public Void visitTypeVariable(AnnotatedTypeVariable type, Tree p) { + if (visitedNodes.containsKey(type)) { + return visitedNodes.get(type); + } + visitedNodes.put(type, null); + if (type.isDeclaration()) { + visitBounds(type, type.getUpperBound(), type.getLowerBound(), p); } + return null; + } - @Override - public Void visitWildcard(AnnotatedWildcardType type, Tree p) { - if (visitedNodes.containsKey(type)) { - return visitedNodes.get(type); - } - visitedNodes.put(type, null); - visitBounds(type, type.getExtendsBound(), type.getSuperBound(), p); - return null; + @Override + public Void visitWildcard(AnnotatedWildcardType type, Tree p) { + if (visitedNodes.containsKey(type)) { + return visitedNodes.get(type); } + visitedNodes.put(type, null); + visitBounds(type, type.getExtendsBound(), type.getSuperBound(), p); + return null; + } - /** - * Visit the bounds of a type variable or a wildcard and potentially apply qual to those - * bounds. This method will also update the boundType, isLowerBound, and isUpperbound - * fields. - */ - protected void visitBounds( - AnnotatedTypeMirror boundedType, - AnnotatedTypeMirror upperBound, - AnnotatedTypeMirror lowerBound, - Tree p) { - - BoundType boundType = BoundTypeUtil.getBoundType(boundedType, validator.atypeFactory); - // TODO Is this correct to use this as condition to check if it's type parameter declaration - // Checking lower bound - if (p.getKind() == Tree.Kind.TYPE_PARAMETER) { - if (boundType.isOneOf(BoundType.LOWER)) { - // Explicit lower bound - checkValidLocation(lowerBound, p, TypeUseLocation.EXPLICIT_LOWER_BOUND); - } else if (boundType.isOneOf( - BoundType.UNBOUND, BoundType.UPPER, BoundType.UNKNOWN)) { - // Implicit lower bound - // Do nothing - } else { - checkValidLocation(lowerBound, p, TypeUseLocation.LOWER_BOUND); - } + /** + * Visit the bounds of a type variable or a wildcard and potentially apply qual to those + * bounds. This method will also update the boundType, isLowerBound, and isUpperbound + * fields. + */ + protected void visitBounds( + AnnotatedTypeMirror boundedType, + AnnotatedTypeMirror upperBound, + AnnotatedTypeMirror lowerBound, + Tree p) { + + BoundType boundType = BoundTypeUtil.getBoundType(boundedType, validator.atypeFactory); + // TODO Is this correct to use this as condition to check if it's type parameter declaration + // Checking lower bound + if (p.getKind() == Tree.Kind.TYPE_PARAMETER) { + if (boundType.isOneOf(BoundType.LOWER)) { + // Explicit lower bound + checkValidLocation(lowerBound, p, TypeUseLocation.EXPLICIT_LOWER_BOUND); + } else if (boundType.isOneOf(BoundType.UNBOUND, BoundType.UPPER, BoundType.UNKNOWN)) { + // Implicit lower bound + // Do nothing + } else { + checkValidLocation(lowerBound, p, TypeUseLocation.LOWER_BOUND); } - // We only need to validate explicit main annotation on lower/upper bounds. So no need to - // visit recursively. They will be scan afterwards in different trees - //scanAndReduce(lowerBound, p, null); - - // Checking upper bound - if (p.getKind() == Tree.Kind.TYPE_PARAMETER) { - if (boundType.isOneOf(BoundType.UPPER, BoundType.UNKNOWN)) { - //Explicit upper bound - checkValidLocation(upperBound, p, TypeUseLocation.EXPLICIT_UPPER_BOUND); - } else if (boundType.isOneOf(BoundType.UNBOUND, BoundType.LOWER)) { - // Implicit upper bound => Do nothing - // Do nothing - } else { - // Upper bound - checkValidLocation(upperBound, p, TypeUseLocation.UPPER_BOUND); - } + } + // We only need to validate explicit main annotation on lower/upper bounds. So no need to + // visit recursively. They will be scan afterwards in different trees + //scanAndReduce(lowerBound, p, null); + + // Checking upper bound + if (p.getKind() == Tree.Kind.TYPE_PARAMETER) { + if (boundType.isOneOf(BoundType.UPPER, BoundType.UNKNOWN)) { + //Explicit upper bound + checkValidLocation(upperBound, p, TypeUseLocation.EXPLICIT_UPPER_BOUND); + } else if (boundType.isOneOf(BoundType.UNBOUND, BoundType.LOWER)) { + // Implicit upper bound => Do nothing + // Do nothing + } else { + // Upper bound + checkValidLocation(upperBound, p, TypeUseLocation.UPPER_BOUND); } - // We only need to validate explicit main annotation on lower/upper bounds. So no need to - // visit recursively. They will be scan afterwards in different trees - //scanAndReduce(upperBound, p, null); } + // We only need to validate explicit main annotation on lower/upper bounds. So no need to + // visit recursively. They will be scan afterwards in different trees from which the deeper + // types can be validated + //scanAndReduce(upperBound, p, null); } } From f2dc30ee790ba1e875f354dfab6a17ce47ec0df3 Mon Sep 17 00:00:00 2001 From: tamier Date: Tue, 1 Nov 2016 12:35:57 -0400 Subject: [PATCH 05/13] Refactor LocationValidator logic --- .../common/basetype/BaseTypeValidator.java | 182 +++++++++++------- .../common/basetype/BaseTypeVisitor.java | 4 +- .../common/basetype/messages.properties | 6 + .../framework/qual/TypeUseLocation.java | 22 ++- 4 files changed, 139 insertions(+), 75 deletions(-) diff --git a/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java b/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java index 6d7e81d12a28..cff41a34e9e3 100644 --- a/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java +++ b/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java @@ -8,12 +8,16 @@ import com.sun.source.tree.ClassTree; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.IdentifierTree; +import com.sun.source.tree.InstanceOfTree; import com.sun.source.tree.MemberSelectTree; import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.MethodTree; import com.sun.source.tree.NewClassTree; import com.sun.source.tree.ParameterizedTypeTree; +import com.sun.source.tree.ThrowTree; import com.sun.source.tree.Tree; +import com.sun.source.tree.Tree.Kind; +import com.sun.source.tree.TypeCastTree; import com.sun.source.tree.TypeParameterTree; import com.sun.source.tree.VariableTree; import com.sun.source.util.TreePath; @@ -566,7 +570,68 @@ void validate(AnnotatedTypeMirror type, Tree tree) { }; private void validateMainModifier(AnnotatedTypeMirror type, Tree tree) { + Kind treeKind = tree.getKind(); + // Cases where only tree is enough to determine its TypeUseLocation + boolean treeIsEnough = false; + switch (treeKind) { + //Additional handling for variable tree initializer + /*case VARIABLE: + Tree initializerTree = ((VariableTree)tree).getInitializer(); + if(initializerTree != null){ + AnnotatedTypeMirror initializerType = this.validator.atypeFactory.getAnnotatedType(initializerTree); + validateMainModifier(initializerType, initializerTree); + } + // treeIsEnough remains false => need additional checks for main modifier + break;*/ + //Additional handling for method tree throw tree + case METHOD: + List throwTrees = ((MethodTree) tree).getThrows(); + for (Tree throwTree : throwTrees) { + AnnotatedTypeMirror throwType = + this.validator.atypeFactory.getAnnotatedType(throwTree); + checkValidLocation(throwType, throwTree, TypeUseLocation.THROWS); + } + break; + // We don't validate throw tree separately. Instead, we do it in MethodTree + /*case THROW: + Tree throwTree = ((ThrowTree)tree).getExpression(); + AnnotatedTypeMirror throwType = this.validator.atypeFactory.getAnnotatedType(throwTree); + checkValidLocation(throwType, throwTree, TypeUseLocation.THROWS); + treeIsEnough = true; + break;*/ + case INSTANCE_OF: + Tree typeTree = ((InstanceOfTree) tree).getType(); + AnnotatedTypeMirror instanceOfType = + this.validator.atypeFactory.getAnnotatedType(typeTree); + checkValidLocation(instanceOfType, typeTree, TypeUseLocation.INSTANCEOF); + treeIsEnough = true; + break; + case NEW_CLASS: + //if(printDebug) System.out.println("Examining new class tree type : " + type); + checkValidLocation(type, tree, TypeUseLocation.NEW); + treeIsEnough = true; + break; + case TYPE_CAST: + Tree castTree = ((TypeCastTree) tree).getType(); + AnnotatedTypeMirror castType = + this.validator.atypeFactory.getAnnotatedType(castTree); + checkValidLocation(castType, castTree, TypeUseLocation.CAST); + treeIsEnough = true; + break; + // Don't need this, because if there is overriding annotation, they will be treated like normal annotated type, + // which might be in top level or recursive deep levels(array component or type argument) + /*case TYPE_PARAMETER: + checkValidLocation(type, tree, TypeUseLocation.TYPE_PARAMETER_OVERRIDE); + break;*/ + default: + break; + } + if (treeIsEnough) return; + // If tree can't be used to determine the TypeUseLocation, use the Element, but only with a specific set of Trees: + // namely, VariableTree, MethodTree, ClassTree Element elt = getElement(tree); + // Skip trees we don't want to validate explicit annotation + if (elt == null) return; ElementKind elementKind = elt.getKind(); switch (elementKind) { case FIELD: @@ -584,7 +649,7 @@ private void validateMainModifier(AnnotatedTypeMirror type, Tree tree) { checkValidLocation(type, tree, TypeUseLocation.EXCEPTION_PARAMETER); break; case PARAMETER: - // TODO method receciver and return type + // TODO method receiver and return type if (elt.getSimpleName().contentEquals("this")) { checkValidLocation(type, tree, TypeUseLocation.RECEIVER); } else { @@ -593,7 +658,7 @@ private void validateMainModifier(AnnotatedTypeMirror type, Tree tree) { break; case CONSTRUCTOR: case METHOD: - // Upper bounf of type parameter declared in generic method after getting + // Upper bound of type parameter declared in generic method after getting // nearest enclosing element is also METHOD element. BUT its tree is no // method tree. So, we add additional restriction: only when the tree is also // method tree, they use is seen to be on method return. @@ -628,83 +693,27 @@ private void validateMainModifier(AnnotatedTypeMirror type, Tree tree) { private Element getElement(Tree tree) { Element elt; switch (tree.getKind()) { - case MEMBER_SELECT: - elt = TreeUtils.elementFromUse((MemberSelectTree) tree); + case VARIABLE: + elt = TreeUtils.elementFromDeclaration((VariableTree) tree); break; - - case IDENTIFIER: - elt = TreeUtils.elementFromUse((IdentifierTree) tree); + case METHOD: + elt = TreeUtils.elementFromDeclaration((MethodTree) tree); break; - - case METHOD_INVOCATION: - elt = TreeUtils.elementFromUse((MethodInvocationTree) tree); + case CLASS: + case ENUM: + case INTERFACE: + case ANNOTATION_TYPE: + elt = TreeUtils.elementFromDeclaration((ClassTree) tree); break; - - // TODO cases for array access, etc. -- every expression tree - // (The above probably means that we should use defaults in the - // scope of the declaration of the array. Is that right? -MDE) - default: - // If no associated symbol was found, use the tree's (lexical) - // scope. - elt = nearestEnclosingExceptLocal(tree); + // We don't care about other trees, since they trees other than the above don't need Element to determine its + // location/ these trees don't contains a top level main modifier, thus no need to validate + elt = null; + break; } return elt; } - private Element nearestEnclosingExceptLocal(Tree tree) { - TreePath path = validator.atypeFactory.getPath(tree); - if (path == null) { - Element method = validator.atypeFactory.getEnclosingMethod(tree); - if (method != null) { - return method; - } else { - return InternalUtils.symbol(tree); - } - } - - Tree prev = null; - - for (Tree t : path) { - switch (t.getKind()) { - case VARIABLE: - VariableTree vtree = (VariableTree) t; - ExpressionTree vtreeInit = vtree.getInitializer(); - // TODO: evaluate comment-out of these lines - /*if (vtreeInit != null && prev == vtreeInit) { - Element elt = TreeUtils.elementFromDeclaration((VariableTree) t); - DefaultQualifier d = elt.getAnnotation(DefaultQualifier.class); - DefaultQualifiers ds = elt.getAnnotation(DefaultQualifiers.class); - - if (d == null && ds == null) { - break; - } - }*/ - if (prev != null && prev.getKind() == Tree.Kind.MODIFIERS) { - // Annotations are modifiers. We do not want to apply the local variable default to - // annotations. Without this, test fenum/TestSwitch failed, because the default for - // an argument became incompatible with the declared type. - break; - } - return TreeUtils.elementFromDeclaration((VariableTree) t); - case METHOD: - return TreeUtils.elementFromDeclaration((MethodTree) t); - case CLASS: - case ENUM: - case INTERFACE: - case ANNOTATION_TYPE: - //System.out.println("Hit!"); - return TreeUtils.elementFromDeclaration((ClassTree) t); - //case TYPE_PARAMETER: - //return TreeUtils.elementFromUse((TypeParameterTree)t); - default: // Do nothing. continue finding parent paths - } - prev = t; - } - // Seems like dead code because there must be a matching case in the for loop and return immediately - return null; - } - private void checkValidLocation(AnnotatedTypeMirror type, Tree tree, TypeUseLocation location) { for (AnnotationMirror am : type.getAnnotations()) { Element elementOfAnnotation = am.getAnnotationType().asElement(); @@ -733,7 +742,7 @@ private void reportLocationError( AnnotatedTypeMirror type, Tree tree, TypeUseLocation location) { if (printDebug) { System.out.println( - "-----!!!----- Error => type: " + "\n-----!!!----- Error => type: " + type + " \ntree: " + tree @@ -761,7 +770,12 @@ protected Void scan(AnnotatedTypeMirror type, Tree p) { + " kind: " + type.getKind()); Element elt = getElement(p); - System.out.println("elt: " + elt + " resulteltkind: " + elt.getKind()); + if (elt != null) { + System.out.println("elt: " + elt + " resulteltkind: " + elt.getKind()); + } else { + System.out.println( + "~~~~~~~~~~~~Unhandle tree case: skip when element is null for the tree below:"); + } System.out.println("tree is: " + p + " usedtreekind: " + p.getKind()); if (visitedNodes.containsKey(type)) { System.out.println("--- Skipped because visited"); @@ -784,6 +798,28 @@ public Void visitDeclared(AnnotatedDeclaredType type, Tree p) { } scan(type.getTypeArguments(), p); } + if (TreeUtils.isClassTree(p)) { + Tree extendsTree = ((ClassTree) p).getExtendsClause(); + if (extendsTree != null) { + AnnotatedTypeMirror extendsType = + this.validator.atypeFactory.getAnnotatedType(extendsTree); + /*if(printDebug) { + System.out.println("Extends tree is: " + extendsTree + " treekind: " + extendsTree.getKind()); + System.out.println("Extends type is: " + extendsType); + }*/ + checkValidLocation(extendsType, extendsTree, TypeUseLocation.EXTENDS); + } + List implementsTrees = ((ClassTree) p).getImplementsClause(); + for (Tree implementsTree : implementsTrees) { + AnnotatedTypeMirror implmentsType = + this.validator.atypeFactory.getAnnotatedType(implementsTree); + /*if(printDebug) { + System.out.println("Implements tree is: " + implementsTree + " treekind: " + implementsTree.getKind()); + System.out.println("Implementss type is: " + implmentsType); + }*/ + checkValidLocation(implmentsType, implementsTree, TypeUseLocation.IMPLEMENTS); + } + } return null; } diff --git a/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java index e6876aa4bd93..f0eb0197e9cf 100644 --- a/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -1674,6 +1674,7 @@ public Void visitTypeCast(TypeCastTree node, Void p) { @Override public Void visitInstanceOf(InstanceOfTree node, Void p) { + validateTypeOf(node); validateTypeOf(node.getType()); return super.visitInstanceOf(node, p); } @@ -1901,6 +1902,7 @@ protected void commonAssignmentCheck( : "array initializers are not expected to be null in: " + valueExp; checkArrayInitialization(compType, arrayTree.getInitializers()); } + // Causes duplicate warnings on new, instanceof if (!validateTypeOf(valueExp)) { return; } @@ -3420,7 +3422,7 @@ public boolean validateTypeOf(Tree tree) { // Nothing to do for void methods. // Note that for a constructor the AnnotatedExecutableType does // not use void as return type. - return true; + return typeValidator.isValid(type, tree); } break; default: diff --git a/framework/src/org/checkerframework/common/basetype/messages.properties b/framework/src/org/checkerframework/common/basetype/messages.properties index 58a37b332cbb..13b5078d6bc6 100644 --- a/framework/src/org/checkerframework/common/basetype/messages.properties +++ b/framework/src/org/checkerframework/common/basetype/messages.properties @@ -106,3 +106,9 @@ implicit_upper_bound.annotation.forbidden= %s is forbidden on implicit upper bou type_declaration.annotation.forbidden= %s is forbidden on type declaration! type_argument.annotation.forbidden= %s is forbidden on type argument! array_component.annotation.forbidden= %s is forbidden on array component! +extends.annotation.forbidden= %s is forbidden on extend clauses! +implements.annotation.forbidden= %s is forbidden on implement clauses! +new.annotation.forbidden= %s is forbidden on new instance creation! +throws.annotation.forbidden= %s is forbidden on thrown exception declaration! +instanceof.annotation.forbidden= %s is forbidden on instanceof type! +cast.annotation.forbidden= %s is forbidden on type cast! \ No newline at end of file diff --git a/framework/src/org/checkerframework/framework/qual/TypeUseLocation.java b/framework/src/org/checkerframework/framework/qual/TypeUseLocation.java index 4873965464e7..a600fa70b3a2 100644 --- a/framework/src/org/checkerframework/framework/qual/TypeUseLocation.java +++ b/framework/src/org/checkerframework/framework/qual/TypeUseLocation.java @@ -96,7 +96,27 @@ public enum TypeUseLocation { * {@code @AC2 String [] @AC1 []} */ ARRAY_COMPONENT, - /** Apply if nothing more concrete is provided. TODO: clarify relation to ALL. */ + + /** + * TODO is this documentation correct? Or does it really represent interface extends case? + * Represents extends location of a class/interface/enum/annotation type + */ + EXTENDS, + + IMPLEMENTS, + + THROWS, + + INSTANCEOF, + + NEW, + + CAST, + + /** + * Apply if nothing more concrete is provided. + * TODO: clarify relation to ALL. + */ OTHERWISE, /** From a331ce572d604a47a610a6c834f344ef4694edd9 Mon Sep 17 00:00:00 2001 From: tamier Date: Sun, 9 Jul 2017 00:48:30 -0400 Subject: [PATCH 06/13] Move qualified location validation logic into BaseTypeVisitor --- framework/build.xml | 7 + .../common/basetype/BaseTypeValidator.java | 455 +++--------------- .../common/basetype/BaseTypeVisitor.java | 96 ++++ .../framework/qual/QualifiedLocations.java | 19 + .../framework/qual/TypeUseLocation.java | 19 +- .../framework/util/BoundType.java | 16 +- .../framework/util/BoundTypeUtil.java | 16 +- .../qualifiedlocations/ArrayComponent.java | 12 + framework/tests/qualifiedlocations/Cast.java | 10 + .../ExceptionParameter.java | 14 + .../ExplicitLowerBound.java | 12 + .../ExplicitUpperBound.java | 12 + .../tests/qualifiedlocations/Extends.java | 6 + framework/tests/qualifiedlocations/Field.java | 8 + .../tests/qualifiedlocations/Implements.java | 14 + .../ImplicitLowerBound.java | 9 + .../ImplicitUpperBound.java | 9 + .../tests/qualifiedlocations/InstanceOf.java | 11 + .../qualifiedlocations/LocalVariable.java | 11 + .../tests/qualifiedlocations/Locations.java | 40 ++ framework/tests/qualifiedlocations/New.java | 13 + .../tests/qualifiedlocations/Parameter.java | 9 + .../tests/qualifiedlocations/Receiver.java | 9 + .../tests/qualifiedlocations/Return.java | 11 + .../tests/qualifiedlocations/Throws.java | 9 + .../qualifiedlocations/TypeArgument.java | 9 + .../qualifiedlocations/TypeDeclaration.java | 6 + ...ualifiedLocationsAnnotatedTypeFactory.java | 33 ++ .../QualifiedLocationsChecker.java | 6 + .../QualifiedLocationsVisitor.java | 25 + .../qualifiedlocations/qual/Bottom.java | 20 + .../testlib/qualifiedlocations/qual/Top.java | 17 + .../src/tests/QualifiedLocationsTest.java | 20 + 33 files changed, 555 insertions(+), 428 deletions(-) create mode 100644 framework/src/org/checkerframework/framework/qual/QualifiedLocations.java create mode 100644 framework/tests/qualifiedlocations/ArrayComponent.java create mode 100644 framework/tests/qualifiedlocations/Cast.java create mode 100644 framework/tests/qualifiedlocations/ExceptionParameter.java create mode 100644 framework/tests/qualifiedlocations/ExplicitLowerBound.java create mode 100644 framework/tests/qualifiedlocations/ExplicitUpperBound.java create mode 100644 framework/tests/qualifiedlocations/Extends.java create mode 100644 framework/tests/qualifiedlocations/Field.java create mode 100644 framework/tests/qualifiedlocations/Implements.java create mode 100644 framework/tests/qualifiedlocations/ImplicitLowerBound.java create mode 100644 framework/tests/qualifiedlocations/ImplicitUpperBound.java create mode 100644 framework/tests/qualifiedlocations/InstanceOf.java create mode 100644 framework/tests/qualifiedlocations/LocalVariable.java create mode 100644 framework/tests/qualifiedlocations/Locations.java create mode 100644 framework/tests/qualifiedlocations/New.java create mode 100644 framework/tests/qualifiedlocations/Parameter.java create mode 100644 framework/tests/qualifiedlocations/Receiver.java create mode 100644 framework/tests/qualifiedlocations/Return.java create mode 100644 framework/tests/qualifiedlocations/Throws.java create mode 100644 framework/tests/qualifiedlocations/TypeArgument.java create mode 100644 framework/tests/qualifiedlocations/TypeDeclaration.java create mode 100644 framework/tests/src/testlib/qualifiedlocations/QualifiedLocationsAnnotatedTypeFactory.java create mode 100644 framework/tests/src/testlib/qualifiedlocations/QualifiedLocationsChecker.java create mode 100644 framework/tests/src/testlib/qualifiedlocations/QualifiedLocationsVisitor.java create mode 100644 framework/tests/src/testlib/qualifiedlocations/qual/Bottom.java create mode 100644 framework/tests/src/testlib/qualifiedlocations/qual/Top.java create mode 100644 framework/tests/src/tests/QualifiedLocationsTest.java diff --git a/framework/build.xml b/framework/build.xml index aea57151c4a2..e689bf76363a 100644 --- a/framework/build.xml +++ b/framework/build.xml @@ -488,6 +488,13 @@ + + + + + + diff --git a/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java b/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java index cff41a34e9e3..149b51eb8511 100644 --- a/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java +++ b/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java @@ -5,40 +5,19 @@ */ import com.sun.source.tree.AnnotatedTypeTree; -import com.sun.source.tree.ClassTree; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.IdentifierTree; -import com.sun.source.tree.InstanceOfTree; -import com.sun.source.tree.MemberSelectTree; -import com.sun.source.tree.MethodInvocationTree; -import com.sun.source.tree.MethodTree; import com.sun.source.tree.NewClassTree; import com.sun.source.tree.ParameterizedTypeTree; -import com.sun.source.tree.ThrowTree; import com.sun.source.tree.Tree; import com.sun.source.tree.Tree.Kind; -import com.sun.source.tree.TypeCastTree; -import com.sun.source.tree.TypeParameterTree; import com.sun.source.tree.VariableTree; -import com.sun.source.util.TreePath; -import com.sun.tools.javac.code.Type.WildcardType; -import java.util.Arrays; -import java.util.HashMap; -import java.util.HashSet; -import java.util.IdentityHashMap; import java.util.List; -import java.util.Map; import java.util.Set; import javax.lang.model.element.AnnotationMirror; -import javax.lang.model.element.Element; -import javax.lang.model.element.ElementKind; import javax.lang.model.element.TypeElement; -import javax.lang.model.element.TypeParameterElement; import javax.lang.model.type.TypeKind; -import org.checkerframework.framework.qual.DefaultQualifier; -import org.checkerframework.framework.qual.DefaultQualifiers; import org.checkerframework.framework.qual.PolyAll; -import org.checkerframework.framework.qual.TargetLocations; import org.checkerframework.framework.qual.TypeUseLocation; import org.checkerframework.framework.source.Result; import org.checkerframework.framework.type.AnnotatedTypeFactory; @@ -56,9 +35,7 @@ import org.checkerframework.framework.util.BoundType; import org.checkerframework.framework.util.BoundTypeUtil; import org.checkerframework.javacutil.AnnotationUtils; -import org.checkerframework.javacutil.CollectionUtils; import org.checkerframework.javacutil.ErrorReporter; -import org.checkerframework.javacutil.InternalUtils; import org.checkerframework.javacutil.Pair; import org.checkerframework.javacutil.TreeUtils; @@ -69,7 +46,7 @@ public class BaseTypeValidator extends AnnotatedTypeScanner implemen protected final BaseTypeChecker checker; protected final BaseTypeVisitor visitor; protected final AnnotatedTypeFactory atypeFactory; - protected final TargetLocationValidator locationValidator; + // TODO: clean up coupling between components public BaseTypeValidator( BaseTypeChecker checker, @@ -78,7 +55,6 @@ public BaseTypeValidator( this.checker = checker; this.visitor = visitor; this.atypeFactory = atypeFactory; - locationValidator = new TargetLocationValidator(this); } /** @@ -95,10 +71,6 @@ public BaseTypeValidator( public boolean isValid(AnnotatedTypeMirror type, Tree tree) { this.isValid = true; visit(type, tree); - // TODO doesn't passin class tree!!! - //System.out.println("Tree " + tree + " is being checked:"); - //System.out.println("\nEntry: type: " + type + " kind: " + type.getKind() + "\n"); - locationValidator.validate(type, tree); return this.isValid; } @@ -228,6 +200,10 @@ public Void visitDeclared(AnnotatedDeclaredType type, Tree tree) { : "size mismatch for type arguments: " + type + " and " + typeArgTree; for (int i = 0; i < tatypes.size(); ++i) { + visitor.checkQualifiedLocation( + tatypes.get(i), + typeArgTree.getTypeArguments().get(i), + TypeUseLocation.TYPE_ARGUMENT); scan(tatypes.get(i), typeArgTree.getTypeArguments().get(i)); } } @@ -329,6 +305,7 @@ public Void visitArray(AnnotatedArrayType type, Tree tree) { AnnotatedTypeMirror comp = type; do { comp = ((AnnotatedArrayType) comp).getComponentType(); + visitor.checkQualifiedLocation(comp, tree, TypeUseLocation.ARRAY_COMPONENT); } while (comp.getKind() == TypeKind.ARRAY); if (comp.getKind() == TypeKind.DECLARED @@ -385,6 +362,11 @@ public Void visitTypeVariable(AnnotatedTypeVariable type, Tree tree) { reportInvalidBounds(type, tree); } + if (type.isDeclaration() && tree.getKind() == Kind.TYPE_PARAMETER) { + validateQualifiedLocationsOnBounds( + type, type.getUpperBound(), type.getLowerBound(), tree); + } + // Keep in sync with visitWildcard Set onVar = type.getAnnotations(); if (!onVar.isEmpty()) { @@ -437,6 +419,9 @@ public Void visitWildcard(AnnotatedWildcardType type, Tree tree) { reportInvalidBounds(type, tree); } + validateQualifiedLocationsOnBounds( + type, type.getExtendsBound(), type.getSuperBound(), tree); + // Keep in sync with visitTypeVariable Set onVar = type.getAnnotations(); if (!onVar.isEmpty()) { @@ -511,6 +496,54 @@ public boolean areBoundsValid( return true; } + /** + * Visit the bounds of a type variable or a wildcard and potentially apply qual to those bounds. + * This method will also update the boundType, isLowerBound, and isUpperbound fields. + */ + protected void validateQualifiedLocationsOnBounds( + AnnotatedTypeMirror boundedType, + AnnotatedTypeMirror upperBound, + AnnotatedTypeMirror lowerBound, + Tree p) { + + BoundType boundType = BoundTypeUtil.getBoundType(boundedType, atypeFactory); + // TODO Is this correct to use this as condition to check if it's type parameter declaration + + // We only need to validate explicit main annotation on lower/upper bounds. So no need to + // visit recursively. They will be scan afterwards in different trees from which the deeper + // types can be validated + //scanAndReduce(upperBound, p, null); + // Checking upper bound + //if (p.getKind() == Tree.Kind.TYPE_PARAMETER) { + if (boundType.isOneOf(BoundType.UPPER)) { + //Explicit upper bound + visitor.checkQualifiedLocation(upperBound, p, TypeUseLocation.EXPLICIT_UPPER_BOUND); + } else if (boundType.isOneOf(BoundType.UNBOUND, BoundType.LOWER, BoundType.UNKNOWN)) { + // Implicit upper bound => Do nothing + visitor.checkQualifiedLocation(upperBound, p, TypeUseLocation.IMPLICIT_UPPER_BOUND); + } else { + // Upper bound + visitor.checkQualifiedLocation(upperBound, p, TypeUseLocation.UPPER_BOUND); + } + //} + + // We only need to validate explicit main annotation on lower/upper bounds. So no need to + // visit recursively. They will be scan afterwards in different trees + //scanAndReduce(lowerBound, p, null); + // Checking lower bound + //if (p.getKind() == Tree.Kind.TYPE_PARAMETER) { + if (boundType.isOneOf(BoundType.LOWER)) { + // Explicit lower bound + visitor.checkQualifiedLocation(lowerBound, p, TypeUseLocation.EXPLICIT_LOWER_BOUND); + } else if (boundType.isOneOf(BoundType.UNBOUND, BoundType.UPPER, BoundType.UNKNOWN)) { + // Implicit lower bound + visitor.checkQualifiedLocation(lowerBound, p, TypeUseLocation.IMPLICIT_LOWER_BOUND); + } else { + visitor.checkQualifiedLocation(lowerBound, p, TypeUseLocation.LOWER_BOUND); + } + //} + } + /** * Determines if there are multiple qualifiers from a single hierarchy in type's primary * annotations. If so, report an error. @@ -537,367 +570,3 @@ public boolean checkConflictingPrimaryAnnos(final AnnotatedTypeMirror type, fina return error; } } - -class TargetLocationValidator extends AnnotatedTypeScanner { - - BaseTypeValidator validator; - private boolean printDebug = false; - int countEntry = 0; - private int countScan = 0; - - TargetLocationValidator(BaseTypeValidator validator) { - this.validator = validator; - } - - void validate(AnnotatedTypeMirror type, Tree tree) { - if (printDebug) { - System.out.println( - "\n\n##################### [" - + countEntry++ - + "] Entry: type: " - + type - + " kind: " - + type.getKind() - + " tree: " - + tree - + " treeKinid: " - + tree.getKind()); - } - // scan is different from visit on not reseting first(reset includes cleaning the visitedNodes map, even though if - // we use IdentityHashMap, it didn't act like an effective cache.) - validateMainModifier(type, tree); - scan(type, tree); - }; - - private void validateMainModifier(AnnotatedTypeMirror type, Tree tree) { - Kind treeKind = tree.getKind(); - // Cases where only tree is enough to determine its TypeUseLocation - boolean treeIsEnough = false; - switch (treeKind) { - //Additional handling for variable tree initializer - /*case VARIABLE: - Tree initializerTree = ((VariableTree)tree).getInitializer(); - if(initializerTree != null){ - AnnotatedTypeMirror initializerType = this.validator.atypeFactory.getAnnotatedType(initializerTree); - validateMainModifier(initializerType, initializerTree); - } - // treeIsEnough remains false => need additional checks for main modifier - break;*/ - //Additional handling for method tree throw tree - case METHOD: - List throwTrees = ((MethodTree) tree).getThrows(); - for (Tree throwTree : throwTrees) { - AnnotatedTypeMirror throwType = - this.validator.atypeFactory.getAnnotatedType(throwTree); - checkValidLocation(throwType, throwTree, TypeUseLocation.THROWS); - } - break; - // We don't validate throw tree separately. Instead, we do it in MethodTree - /*case THROW: - Tree throwTree = ((ThrowTree)tree).getExpression(); - AnnotatedTypeMirror throwType = this.validator.atypeFactory.getAnnotatedType(throwTree); - checkValidLocation(throwType, throwTree, TypeUseLocation.THROWS); - treeIsEnough = true; - break;*/ - case INSTANCE_OF: - Tree typeTree = ((InstanceOfTree) tree).getType(); - AnnotatedTypeMirror instanceOfType = - this.validator.atypeFactory.getAnnotatedType(typeTree); - checkValidLocation(instanceOfType, typeTree, TypeUseLocation.INSTANCEOF); - treeIsEnough = true; - break; - case NEW_CLASS: - //if(printDebug) System.out.println("Examining new class tree type : " + type); - checkValidLocation(type, tree, TypeUseLocation.NEW); - treeIsEnough = true; - break; - case TYPE_CAST: - Tree castTree = ((TypeCastTree) tree).getType(); - AnnotatedTypeMirror castType = - this.validator.atypeFactory.getAnnotatedType(castTree); - checkValidLocation(castType, castTree, TypeUseLocation.CAST); - treeIsEnough = true; - break; - // Don't need this, because if there is overriding annotation, they will be treated like normal annotated type, - // which might be in top level or recursive deep levels(array component or type argument) - /*case TYPE_PARAMETER: - checkValidLocation(type, tree, TypeUseLocation.TYPE_PARAMETER_OVERRIDE); - break;*/ - default: - break; - } - if (treeIsEnough) return; - // If tree can't be used to determine the TypeUseLocation, use the Element, but only with a specific set of Trees: - // namely, VariableTree, MethodTree, ClassTree - Element elt = getElement(tree); - // Skip trees we don't want to validate explicit annotation - if (elt == null) return; - ElementKind elementKind = elt.getKind(); - switch (elementKind) { - case FIELD: - // Actual location IS Field! Need to check TypeUseLocation.FIELD is - //inside declared TypeUseLocation of the annotations on this element - checkValidLocation(type, tree, TypeUseLocation.FIELD); - break; - case LOCAL_VARIABLE: - checkValidLocation(type, tree, TypeUseLocation.LOCAL_VARIABLE); - break; - case RESOURCE_VARIABLE: - checkValidLocation(type, tree, TypeUseLocation.RESOURCE_VARIABLE); - break; - case EXCEPTION_PARAMETER: - checkValidLocation(type, tree, TypeUseLocation.EXCEPTION_PARAMETER); - break; - case PARAMETER: - // TODO method receiver and return type - if (elt.getSimpleName().contentEquals("this")) { - checkValidLocation(type, tree, TypeUseLocation.RECEIVER); - } else { - checkValidLocation(type, tree, TypeUseLocation.PARAMETER); - } - break; - case CONSTRUCTOR: - case METHOD: - // Upper bound of type parameter declared in generic method after getting - // nearest enclosing element is also METHOD element. BUT its tree is no - // method tree. So, we add additional restriction: only when the tree is also - // method tree, they use is seen to be on method return. - if (tree.getKind() == Tree.Kind.METHOD) { - checkValidLocation(type, tree, TypeUseLocation.RETURN); - } - break; - case CLASS: - case INTERFACE: - case ANNOTATION_TYPE: - case ENUM: - // Not covered since type validator doesn't pass in class tree and a type to validate - //System.out.println("@@@tree: " + p); - // TODO validate class tree also in BaseTypeVisitor - //TODO: we get CLASS element kind for both type parameter tree and annotated type tree. - // The two tress are correct, and consistent with type. BUT, the Element gotton from tree - // has errors. And we use Element to process each location checking, so there are false - // warnings. Originally, these four cases are not supported, so didn't encountered this problem - // Update: this isClassTree if statement is basically because for upper bounds trees, it will - // extract class_name as element, and its kind is type_declaration. The major reason is that: - // getElement() methods returns wrong element for like List as Locations. So, we need - // to make sure we are using class trees, to ensure that we are really at type declaration position. - if (TreeUtils.isClassTree(tree)) { - checkValidLocation(type, tree, TypeUseLocation.TYPE_DECLARATION); - } - break; - default: - break; - } - } - - private Element getElement(Tree tree) { - Element elt; - switch (tree.getKind()) { - case VARIABLE: - elt = TreeUtils.elementFromDeclaration((VariableTree) tree); - break; - case METHOD: - elt = TreeUtils.elementFromDeclaration((MethodTree) tree); - break; - case CLASS: - case ENUM: - case INTERFACE: - case ANNOTATION_TYPE: - elt = TreeUtils.elementFromDeclaration((ClassTree) tree); - break; - default: - // We don't care about other trees, since they trees other than the above don't need Element to determine its - // location/ these trees don't contains a top level main modifier, thus no need to validate - elt = null; - break; - } - return elt; - } - - private void checkValidLocation(AnnotatedTypeMirror type, Tree tree, TypeUseLocation location) { - for (AnnotationMirror am : type.getAnnotations()) { - Element elementOfAnnotation = am.getAnnotationType().asElement(); - TargetLocations declLocations = - elementOfAnnotation.getAnnotation(TargetLocations.class); - // Null means no TargetLocations specified => Any use is correct. - if (declLocations != null) { - Set set = new HashSet<>(Arrays.asList(declLocations.value())); - if (set.contains(TypeUseLocation.ALL)) continue; - if (((location == TypeUseLocation.EXPLICIT_LOWER_BOUND) - || (location == TypeUseLocation.IMPLICIT_LOWER_BOUND)) - && set.contains(TypeUseLocation.LOWER_BOUND)) { - // TypeUseLocation.LOWER_BOUND already covers both explicit and implicit lower bounds, so no need to check containment - continue; - } else if (((location == TypeUseLocation.EXPLICIT_UPPER_BOUND) - || (location == TypeUseLocation.IMPLICIT_UPPER_BOUND)) - && set.contains(TypeUseLocation.UPPER_BOUND)) { - // TypeUseLocation.UPPER_BOUND already covers both explicit and implicit lower bounds, so no need to check containment - continue; - } else if (!set.contains(location)) reportLocationError(type, tree, location); - } - } - } - - private void reportLocationError( - AnnotatedTypeMirror type, Tree tree, TypeUseLocation location) { - if (printDebug) { - System.out.println( - "\n-----!!!----- Error => type: " - + type - + " \ntree: " - + tree - + " location: " - + location.toString().toLowerCase()); - } - // TODO check the effect of removing cache for type and tree while reporting error - validator.reportValidityResult( - location.toString().toLowerCase() + ".annotation.forbidden", type, tree); - validator.isValid = false; - } - - @Override - protected Void scan(AnnotatedTypeMirror type, Tree p) { - // The "type" here is constantly changing while visiting different types, like type arguments, - // component, upper/lower bound. The "p" parameter is always passed the same from the entry of - // visitXXX method from the top construct until the last any visitXXX method. - if (printDebug) { - System.out.println( - "\n===>" - + countScan++ - + ") " - + "Visiting " - + type - + " kind: " - + type.getKind()); - Element elt = getElement(p); - if (elt != null) { - System.out.println("elt: " + elt + " resulteltkind: " + elt.getKind()); - } else { - System.out.println( - "~~~~~~~~~~~~Unhandle tree case: skip when element is null for the tree below:"); - } - System.out.println("tree is: " + p + " usedtreekind: " + p.getKind()); - if (visitedNodes.containsKey(type)) { - System.out.println("--- Skipped because visited"); - } - } - - return super.scan(type, p); - } - - @Override - public Void visitDeclared(AnnotatedDeclaredType type, Tree p) { - if (visitedNodes.containsKey(type)) { - return visitedNodes.get(type); - } - visitedNodes.put(type, null); - // Not check if tree p is a "wide" class declaration tree - if (!TreeUtils.isClassTree(p)) { - for (AnnotatedTypeMirror tArg : type.getTypeArguments()) { - checkValidLocation(tArg, p, TypeUseLocation.TYPE_ARGUMENT); - } - scan(type.getTypeArguments(), p); - } - if (TreeUtils.isClassTree(p)) { - Tree extendsTree = ((ClassTree) p).getExtendsClause(); - if (extendsTree != null) { - AnnotatedTypeMirror extendsType = - this.validator.atypeFactory.getAnnotatedType(extendsTree); - /*if(printDebug) { - System.out.println("Extends tree is: " + extendsTree + " treekind: " + extendsTree.getKind()); - System.out.println("Extends type is: " + extendsType); - }*/ - checkValidLocation(extendsType, extendsTree, TypeUseLocation.EXTENDS); - } - List implementsTrees = ((ClassTree) p).getImplementsClause(); - for (Tree implementsTree : implementsTrees) { - AnnotatedTypeMirror implmentsType = - this.validator.atypeFactory.getAnnotatedType(implementsTree); - /*if(printDebug) { - System.out.println("Implements tree is: " + implementsTree + " treekind: " + implementsTree.getKind()); - System.out.println("Implementss type is: " + implmentsType); - }*/ - checkValidLocation(implmentsType, implementsTree, TypeUseLocation.IMPLEMENTS); - } - } - return null; - } - - @Override - public Void visitArray(AnnotatedArrayType type, Tree p) { - // Begin to check array component - checkValidLocation(type.getComponentType(), p, TypeUseLocation.ARRAY_COMPONENT); - scan(type.getComponentType(), p); - return null; - } - - @Override - public Void visitTypeVariable(AnnotatedTypeVariable type, Tree p) { - if (visitedNodes.containsKey(type)) { - return visitedNodes.get(type); - } - visitedNodes.put(type, null); - if (type.isDeclaration()) { - visitBounds(type, type.getUpperBound(), type.getLowerBound(), p); - } - return null; - } - - @Override - public Void visitWildcard(AnnotatedWildcardType type, Tree p) { - if (visitedNodes.containsKey(type)) { - return visitedNodes.get(type); - } - visitedNodes.put(type, null); - visitBounds(type, type.getExtendsBound(), type.getSuperBound(), p); - return null; - } - - /** - * Visit the bounds of a type variable or a wildcard and potentially apply qual to those - * bounds. This method will also update the boundType, isLowerBound, and isUpperbound - * fields. - */ - protected void visitBounds( - AnnotatedTypeMirror boundedType, - AnnotatedTypeMirror upperBound, - AnnotatedTypeMirror lowerBound, - Tree p) { - - BoundType boundType = BoundTypeUtil.getBoundType(boundedType, validator.atypeFactory); - // TODO Is this correct to use this as condition to check if it's type parameter declaration - // Checking lower bound - if (p.getKind() == Tree.Kind.TYPE_PARAMETER) { - if (boundType.isOneOf(BoundType.LOWER)) { - // Explicit lower bound - checkValidLocation(lowerBound, p, TypeUseLocation.EXPLICIT_LOWER_BOUND); - } else if (boundType.isOneOf(BoundType.UNBOUND, BoundType.UPPER, BoundType.UNKNOWN)) { - // Implicit lower bound - // Do nothing - } else { - checkValidLocation(lowerBound, p, TypeUseLocation.LOWER_BOUND); - } - } - // We only need to validate explicit main annotation on lower/upper bounds. So no need to - // visit recursively. They will be scan afterwards in different trees - //scanAndReduce(lowerBound, p, null); - - // Checking upper bound - if (p.getKind() == Tree.Kind.TYPE_PARAMETER) { - if (boundType.isOneOf(BoundType.UPPER, BoundType.UNKNOWN)) { - //Explicit upper bound - checkValidLocation(upperBound, p, TypeUseLocation.EXPLICIT_UPPER_BOUND); - } else if (boundType.isOneOf(BoundType.UNBOUND, BoundType.LOWER)) { - // Implicit upper bound => Do nothing - // Do nothing - } else { - // Upper bound - checkValidLocation(upperBound, p, TypeUseLocation.UPPER_BOUND); - } - } - // We only need to validate explicit main annotation on lower/upper bounds. So no need to - // visit recursively. They will be scan afterwards in different trees from which the deeper - // types can be validated - //scanAndReduce(upperBound, p, null); - } -} diff --git a/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java index f0eb0197e9cf..2c0f16eb5c00 100644 --- a/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -76,6 +76,8 @@ import org.checkerframework.framework.flow.CFAbstractStore; import org.checkerframework.framework.flow.CFAbstractValue; import org.checkerframework.framework.qual.DefaultQualifier; +import org.checkerframework.framework.qual.QualifiedLocations; +import org.checkerframework.framework.qual.TypeUseLocation; import org.checkerframework.framework.qual.Unused; import org.checkerframework.framework.source.Result; import org.checkerframework.framework.source.SourceVisitor; @@ -299,6 +301,10 @@ public final Void visitClass(ClassTree classTree, Void p) { // We validate class tree to see if the annotation on it is allowed on type declaration location validateTypeOf(classTree); + checkQualifiedLocation( + atypeFactory.getAnnotatedType(classTree), + classTree, + TypeUseLocation.TYPE_DECLARATION); /* Visit the extends and implements clauses. * The superclass also visits them, but only calls visitParameterizedType, which * loses a main modifier. @@ -306,12 +312,16 @@ public final Void visitClass(ClassTree classTree, Void p) { Tree ext = classTree.getExtendsClause(); if (ext != null) { validateTypeOf(ext); + checkQualifiedLocation( + atypeFactory.getAnnotatedType(ext), ext, TypeUseLocation.EXTENDS); } List impls = classTree.getImplementsClause(); if (impls != null) { for (Tree im : impls) { validateTypeOf(im); + checkQualifiedLocation( + atypeFactory.getAnnotatedType(im), im, TypeUseLocation.IMPLEMENTS); } } processClassTree(classTree); @@ -558,9 +568,13 @@ public Void visitMethod(MethodTree node, Void p) { // Passing the whole method/constructor validates the return type validateTypeOf(node); + checkQualifiedLocation( + atypeFactory.getMethodReturnType(node), node, TypeUseLocation.RETURN); // Validate types in throws clauses for (ExpressionTree thr : node.getThrows()) { + checkQualifiedLocation( + atypeFactory.getAnnotatedType(thr), thr, TypeUseLocation.THROWS); validateTypeOf(thr); } @@ -835,12 +849,23 @@ public Void visitTypeParameter(TypeParameterTree node, Void p) { return super.visitTypeParameter(node, p); } + /* @Override + public Void visitArrayType(ArrayTypeTree node, Void aVoid) { + // Why this doesn't work? arrayType didn't include explicit annotations! Why? + AnnotatedArrayType arrayType = (AnnotatedArrayType) atypeFactory.getAnnotatedTypeFromTypeTree(node); + Tree componenetTree = node.getType(); + checkQualifiedLocation(arrayType.getComponentType(), componenetTree, TypeUseLocation.ARRAY_COMPONENT); + return super.visitArrayType(node, aVoid); + }*/ + // ********************************************************************** // Assignment checkers and pseudo-assignments // ********************************************************************** @Override public Void visitVariable(VariableTree node, Void p) { + validateQualifiedLocationForVariableTree(node); + Pair preAssCtxt = visitorState.getAssignmentContext(); visitorState.setAssignmentContext( Pair.of((Tree) node, atypeFactory.getAnnotatedType(node))); @@ -865,6 +890,32 @@ public Void visitVariable(VariableTree node, Void p) { } } + private void validateQualifiedLocationForVariableTree(VariableTree node) { + AnnotatedTypeMirror type = atypeFactory.getAnnotatedType(node); + Element element = TreeUtils.elementFromDeclaration(node); + switch (element.getKind()) { + case FIELD: + checkQualifiedLocation(type, node, TypeUseLocation.FIELD); + break; + case LOCAL_VARIABLE: + checkQualifiedLocation(type, node, TypeUseLocation.LOCAL_VARIABLE); + break; + case RESOURCE_VARIABLE: + checkQualifiedLocation(type, node, TypeUseLocation.RESOURCE_VARIABLE); + break; + case EXCEPTION_PARAMETER: + checkQualifiedLocation(type, node, TypeUseLocation.EXCEPTION_PARAMETER); + break; + case PARAMETER: + if (element.getSimpleName().contentEquals("this")) { + checkQualifiedLocation(type, node, TypeUseLocation.RECEIVER); + } else { + checkQualifiedLocation(type, node, TypeUseLocation.PARAMETER); + } + break; + } + } + /** * Performs two checks: subtyping and assignability checks, using {@link * #commonAssignmentCheck(Tree, ExpressionTree, String)}. @@ -1252,6 +1303,8 @@ public Void visitNewClass(NewClassTree node, Void p) { checkTypeArguments(node, paramBounds, typeargs, node.getTypeArguments()); + checkQualifiedLocation(atypeFactory.getAnnotatedType(node), node, TypeUseLocation.NEW); + boolean valid = validateTypeOf(node); if (valid) { @@ -1519,6 +1572,8 @@ public Void visitCompoundAssignment(CompoundAssignmentTree node, Void p) { public Void visitNewArray(NewArrayTree node, Void p) { boolean valid = validateTypeOf(node); + checkQualifiedLocation(atypeFactory.getAnnotatedType(node), node, TypeUseLocation.NEW); + if (valid && node.getType() != null) { AnnotatedArrayType arrayType = atypeFactory.getAnnotatedType(node); if (atypeFactory.getDependentTypesHelper() != null) { @@ -1658,6 +1713,9 @@ private boolean isTypeCastSafe(AnnotatedTypeMirror castType, AnnotatedTypeMirror @Override public Void visitTypeCast(TypeCastTree node, Void p) { + Tree castTree = node.getType(); + AnnotatedTypeMirror castType = atypeFactory.getAnnotatedType(castTree); + checkQualifiedLocation(castType, castTree, TypeUseLocation.CAST); // validate "node" instead of "node.getType()" to prevent duplicate errors. boolean valid = validateTypeOf(node) && validateTypeOf(node.getExpression()); if (valid) { @@ -1674,6 +1732,9 @@ public Void visitTypeCast(TypeCastTree node, Void p) { @Override public Void visitInstanceOf(InstanceOfTree node, Void p) { + Tree typeTree = node.getType(); + AnnotatedTypeMirror instanceOfType = atypeFactory.getAnnotatedType(typeTree); + checkQualifiedLocation(instanceOfType, typeTree, TypeUseLocation.INSTANCEOF); validateTypeOf(node); validateTypeOf(node.getType()); return super.visitInstanceOf(node, p); @@ -2621,6 +2682,41 @@ private boolean checkMethodReferenceInference( return false; } + protected void checkQualifiedLocation( + AnnotatedTypeMirror type, Tree tree, TypeUseLocation location) { + for (AnnotationMirror am : type.getAnnotations()) { + Element elementOfAnnotation = am.getAnnotationType().asElement(); + QualifiedLocations declLocations = + elementOfAnnotation.getAnnotation(QualifiedLocations.class); + // Null means no TargetLocations specified => Any use is correct. + if (declLocations != null) { + Set set = new HashSet<>(Arrays.asList(declLocations.value())); + if (set.contains(TypeUseLocation.ALL)) continue; + if (((location == TypeUseLocation.EXPLICIT_LOWER_BOUND) + || (location == TypeUseLocation.IMPLICIT_LOWER_BOUND)) + && set.contains(TypeUseLocation.LOWER_BOUND)) { + // TypeUseLocation.LOWER_BOUND already covers both explicit and implicit lower bounds, so no need to check containment + continue; + } else if (((location == TypeUseLocation.EXPLICIT_UPPER_BOUND) + || (location == TypeUseLocation.IMPLICIT_UPPER_BOUND)) + && set.contains(TypeUseLocation.UPPER_BOUND)) { + // TypeUseLocation.UPPER_BOUND already covers both explicit and implicit lower bounds, so no need to check containment + continue; + } else if (!set.contains(location)) reportLocationError(type, tree, location); + } + } + } + + private void reportLocationError( + AnnotatedTypeMirror type, Tree tree, TypeUseLocation location) { + checker.report( + Result.failure( + location.toString().toLowerCase() + ".annotation.forbidden", + type.getAnnotations(), + type.toString()), + tree); + } + /** * Class to perform method override and method reference checks. * diff --git a/framework/src/org/checkerframework/framework/qual/QualifiedLocations.java b/framework/src/org/checkerframework/framework/qual/QualifiedLocations.java new file mode 100644 index 000000000000..f134f9e47ecb --- /dev/null +++ b/framework/src/org/checkerframework/framework/qual/QualifiedLocations.java @@ -0,0 +1,19 @@ +package org.checkerframework.framework.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; + +@Documented +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.ANNOTATION_TYPE) +/** + * Applied to the declaration of a type qualifier. It specifies the qualifier is qualified to be + * used on the specified locations. It will be enforced towards all kinds of annotation sources - + * explicitly written, implicit, defaulted etc. + */ +public @interface QualifiedLocations { + TypeUseLocation[] value(); +} diff --git a/framework/src/org/checkerframework/framework/qual/TypeUseLocation.java b/framework/src/org/checkerframework/framework/qual/TypeUseLocation.java index a600fa70b3a2..b585c0082b13 100644 --- a/framework/src/org/checkerframework/framework/qual/TypeUseLocation.java +++ b/framework/src/org/checkerframework/framework/qual/TypeUseLocation.java @@ -81,20 +81,14 @@ public enum TypeUseLocation { */ IMPLICIT_UPPER_BOUND, - /** - * Apply default annotations to unannotated type declarations: - * {@code @HERE class Demo{}} - */ + /** Apply default annotations to unannotated type declarations: {@code @HERE class Demo{}} */ TYPE_DECLARATION, /** - * Represents type argument location in parameterized type - * {@code List<@TA1 ArrayList<@TA2 String>>} + * Represents type argument location in parameterized type {@code List<@TA1 ArrayList<@TA2 + * String>>} */ TYPE_ARGUMENT, - /** - * Represents array component location - * {@code @AC2 String [] @AC1 []} - */ + /** Represents array component location {@code @AC2 String [] @AC1 []} */ ARRAY_COMPONENT, /** @@ -113,10 +107,7 @@ public enum TypeUseLocation { CAST, - /** - * Apply if nothing more concrete is provided. - * TODO: clarify relation to ALL. - */ + /** Apply if nothing more concrete is provided. TODO: clarify relation to ALL. */ OTHERWISE, /** diff --git a/framework/src/org/checkerframework/framework/util/BoundType.java b/framework/src/org/checkerframework/framework/util/BoundType.java index 4dc2883ce8b0..7e25ca287f5a 100644 --- a/framework/src/org/checkerframework/framework/util/BoundType.java +++ b/framework/src/org/checkerframework/framework/util/BoundType.java @@ -2,24 +2,18 @@ public enum BoundType { - /** - * Indicates an upper bounded type variable or wildcard - */ + /** Indicates an upper bounded type variable or wildcard */ UPPER, - /** - * Indicates a lower bounded type variable or wildcard - */ + /** Indicates a lower bounded type variable or wildcard */ LOWER, - /** - * Neither bound is specified, BOTH are implicit - */ + /** Neither bound is specified, BOTH are implicit */ UNBOUND, /** - * For bytecode, or trees for which we no longer have the compilation unit. - * We treat UNKNOWN bounds as if they are an UPPER bound. + * For bytecode, or trees for which we no longer have the compilation unit. We treat UNKNOWN + * bounds as if they are an UPPER bound. */ UNKNOWN; diff --git a/framework/src/org/checkerframework/framework/util/BoundTypeUtil.java b/framework/src/org/checkerframework/framework/util/BoundTypeUtil.java index df1dba81a928..942973eb6742 100644 --- a/framework/src/org/checkerframework/framework/util/BoundTypeUtil.java +++ b/framework/src/org/checkerframework/framework/util/BoundTypeUtil.java @@ -14,8 +14,8 @@ public class BoundTypeUtil { /** - * @param type the type whose boundType is returned. - * type must be an AnnotatedWildcardType or AnnotatedTypeVariable + * @param type the type whose boundType is returned. type must be an AnnotatedWildcardType or + * AnnotatedTypeVariable * @return the boundType for type */ public static BoundType getBoundType( @@ -32,18 +32,14 @@ public static BoundType getBoundType( return null; // dead code } - /** - * @return the bound type of the input typeVar - */ + /** @return the bound type of the input typeVar */ public static BoundType getTypeVarBoundType( final AnnotatedTypeVariable typeVar, final AnnotatedTypeFactory typeFactory) { return getTypeVarBoundType( (TypeParameterElement) typeVar.getUnderlyingType().asElement(), typeFactory); } - /** - * @return the boundType (UPPER, UNBOUND, or UNKNOWN) of the declaration of typeParamElem - */ + /** @return the boundType (UPPER, UNBOUND, or UNKNOWN) of the declaration of typeParamElem */ public static BoundType getTypeVarBoundType( final TypeParameterElement typeParamElem, final AnnotatedTypeFactory typeFactory) { @@ -81,8 +77,8 @@ public static BoundType getTypeVarBoundType( } /** - * @return the BoundType of annotatedWildcard. If it is unbounded, use the type parameter to - * which its an argument + * @return the BoundType of annotatedWildcard. If it is unbounded, use the type parameter to + * which its an argument */ public static BoundType getWildcardBoundType( final AnnotatedWildcardType annotatedWildcard, final AnnotatedTypeFactory typeFactory) { diff --git a/framework/tests/qualifiedlocations/ArrayComponent.java b/framework/tests/qualifiedlocations/ArrayComponent.java new file mode 100644 index 000000000000..ee85016666ee --- /dev/null +++ b/framework/tests/qualifiedlocations/ArrayComponent.java @@ -0,0 +1,12 @@ +package qualifiedlocations; + +import testlib.qualifiedlocations.qual.Bottom; + +public class ArrayComponent { + //:: error: (array_component.annotation.forbidden) + @Bottom Object[] array; + //:: error: (array_component.annotation.forbidden) + @Bottom Object[] foo() { + return null; + } +} diff --git a/framework/tests/qualifiedlocations/Cast.java b/framework/tests/qualifiedlocations/Cast.java new file mode 100644 index 000000000000..8cb39f507252 --- /dev/null +++ b/framework/tests/qualifiedlocations/Cast.java @@ -0,0 +1,10 @@ +package qualifiedlocations; + +import testlib.qualifiedlocations.qual.Bottom; + +public class Cast { + { + //:: error: (cast.annotation.forbidden) + ((@Bottom Object) new Object()).toString(); + } +} diff --git a/framework/tests/qualifiedlocations/ExceptionParameter.java b/framework/tests/qualifiedlocations/ExceptionParameter.java new file mode 100644 index 000000000000..0397da45f680 --- /dev/null +++ b/framework/tests/qualifiedlocations/ExceptionParameter.java @@ -0,0 +1,14 @@ +package qualifiedlocations; + +import testlib.qualifiedlocations.qual.Bottom; + +public class ExceptionParameter { + + void foo() { + try { + //:: error: (exception_parameter.annotation.forbidden) + } catch (@Bottom Exception e) { + + } + } +} diff --git a/framework/tests/qualifiedlocations/ExplicitLowerBound.java b/framework/tests/qualifiedlocations/ExplicitLowerBound.java new file mode 100644 index 000000000000..4c659e8b6250 --- /dev/null +++ b/framework/tests/qualifiedlocations/ExplicitLowerBound.java @@ -0,0 +1,12 @@ +package qualifiedlocations; + +import java.util.Set; +import testlib.qualifiedlocations.qual.Bottom; + +public class ExplicitLowerBound { + + //:: error: (explicit_lower_bound.annotation.forbidden) + Set foo() { + return null; + } +} diff --git a/framework/tests/qualifiedlocations/ExplicitUpperBound.java b/framework/tests/qualifiedlocations/ExplicitUpperBound.java new file mode 100644 index 000000000000..03e6d1b956a3 --- /dev/null +++ b/framework/tests/qualifiedlocations/ExplicitUpperBound.java @@ -0,0 +1,12 @@ +package qualifiedlocations; + +import java.util.Set; +import testlib.qualifiedlocations.qual.Bottom; + +//:: error: (explicit_upper_bound.annotation.forbidden) +public class ExplicitUpperBound { + //:: error: (explicit_upper_bound.annotation.forbidden) + Set foo() { + return null; + } +} diff --git a/framework/tests/qualifiedlocations/Extends.java b/framework/tests/qualifiedlocations/Extends.java new file mode 100644 index 000000000000..cac448a665c1 --- /dev/null +++ b/framework/tests/qualifiedlocations/Extends.java @@ -0,0 +1,6 @@ +package qualifiedlocations; + +import testlib.qualifiedlocations.qual.Bottom; + +//:: error: (extends.annotation.forbidden) :: error: (type_declaration.annotation.forbidden) +public class Extends extends @Bottom Object {} diff --git a/framework/tests/qualifiedlocations/Field.java b/framework/tests/qualifiedlocations/Field.java new file mode 100644 index 000000000000..a3217ea76507 --- /dev/null +++ b/framework/tests/qualifiedlocations/Field.java @@ -0,0 +1,8 @@ +package qualifiedlocations; + +import testlib.qualifiedlocations.qual.Bottom; + +public class Field { + //:: error: (field.annotation.forbidden) + @Bottom Object o; +} diff --git a/framework/tests/qualifiedlocations/Implements.java b/framework/tests/qualifiedlocations/Implements.java new file mode 100644 index 000000000000..49a421ba7a65 --- /dev/null +++ b/framework/tests/qualifiedlocations/Implements.java @@ -0,0 +1,14 @@ +package qualifiedlocations; + +import testlib.qualifiedlocations.qual.Bottom; + +interface Test { + void foo(); +} + +//:: error: (implements.annotation.forbidden) :: error: (type_declaration.annotation.forbidden) +public class Implements implements @Bottom Test { + public void foo() { + return; + } +} diff --git a/framework/tests/qualifiedlocations/ImplicitLowerBound.java b/framework/tests/qualifiedlocations/ImplicitLowerBound.java new file mode 100644 index 000000000000..da9eaa7a29eb --- /dev/null +++ b/framework/tests/qualifiedlocations/ImplicitLowerBound.java @@ -0,0 +1,9 @@ +package qualifiedlocations; + +import org.checkerframework.framework.qual.DefaultQualifier; +import org.checkerframework.framework.qual.TypeUseLocation; +import testlib.qualifiedlocations.qual.Top; + +@DefaultQualifier(value = Top.class, locations = TypeUseLocation.UPPER_BOUND) +//:: error: (implicit_lower_bound.annotation.forbidden) +public class ImplicitLowerBound {} diff --git a/framework/tests/qualifiedlocations/ImplicitUpperBound.java b/framework/tests/qualifiedlocations/ImplicitUpperBound.java new file mode 100644 index 000000000000..b4b5f1e06330 --- /dev/null +++ b/framework/tests/qualifiedlocations/ImplicitUpperBound.java @@ -0,0 +1,9 @@ +package qualifiedlocations; + +import org.checkerframework.framework.qual.DefaultQualifier; +import org.checkerframework.framework.qual.TypeUseLocation; +import testlib.qualifiedlocations.qual.Bottom; + +@DefaultQualifier(value = Bottom.class, locations = TypeUseLocation.UPPER_BOUND) +//:: error: (implicit_upper_bound.annotation.forbidden) +public class ImplicitUpperBound {} diff --git a/framework/tests/qualifiedlocations/InstanceOf.java b/framework/tests/qualifiedlocations/InstanceOf.java new file mode 100644 index 000000000000..155146c360f3 --- /dev/null +++ b/framework/tests/qualifiedlocations/InstanceOf.java @@ -0,0 +1,11 @@ +package qualifiedlocations; + +import testlib.qualifiedlocations.qual.Bottom; + +public class InstanceOf { + + void foo(Object o) { + //:: error: (instanceof.annotation.forbidden) + if (o instanceof @Bottom Object) {} + } +} diff --git a/framework/tests/qualifiedlocations/LocalVariable.java b/framework/tests/qualifiedlocations/LocalVariable.java new file mode 100644 index 000000000000..14d39eed6c24 --- /dev/null +++ b/framework/tests/qualifiedlocations/LocalVariable.java @@ -0,0 +1,11 @@ +package qualifiedlocations; + +import testlib.qualifiedlocations.qual.Bottom; + +public class LocalVariable { + + void foo() { + //:: error: (local_variable.annotation.forbidden) + @Bottom Object lo; + } +} diff --git a/framework/tests/qualifiedlocations/Locations.java b/framework/tests/qualifiedlocations/Locations.java new file mode 100644 index 000000000000..e3aae4641244 --- /dev/null +++ b/framework/tests/qualifiedlocations/Locations.java @@ -0,0 +1,40 @@ +package qualifiedlocations; + +import java.util.ArrayList; +import java.util.List; +import testlib.qualifiedlocations.qual.Bottom; + +//:: error: (array_component.annotation.forbidden) :: error: (type_argument.annotation.forbidden) :: error: (explicit_upper_bound.annotation.forbidden) +public class Locations> + //:: error: (type_argument.annotation.forbidden) :: error: (extends.annotation.forbidden) :: error: (implements.annotation.forbidden) + extends @Bottom ArrayList<@Bottom Object> implements @Bottom Iterable<@Bottom Object> { + //:: error: (field.annotation.forbidden) + @Bottom T t; + //:: error: (field.annotation.forbidden) :: error: (type_argument.annotation.forbidden) + @Bottom List<@Bottom ArrayList<@Bottom String>> l; + //:: error: (type_argument.annotation.forbidden) + List<@Bottom String @Bottom [] @Bottom []> + f; // It's strange that array component doesn't show error + + //:: error: (throws.annotation.forbidden) + void foo() throws @Bottom Exception { + //:: error: (local_variable.annotation.forbidden) :: error: (type_argument.annotation.forbidden) :: error: (new.annotation.forbidden) + @Bottom Object l = new @Bottom ArrayList<@Bottom Object>(); + //:: error: (instanceof.annotation.forbidden) :: error: (cast.annotation.forbidden) + boolean b = (@Bottom Object) l instanceof @Bottom List @Bottom []; + } + + //:: error: (parameter.annotation.forbidden) + void bar(@Bottom Object p) { + try { + foo(); + //:: error: (exception_parameter.annotation.forbidden) + } catch (@Bottom Exception e) { + } + } + + //:: error: (return.annotation.forbidden) :: error: (explicit_upper_bound.annotation.forbidden) :: error: (type_argument.annotation.forbidden) :: error: (receiver.annotation.forbidden) + > @Bottom Object hey(Locations<@Bottom T> this, S s) { + return null; + } +} diff --git a/framework/tests/qualifiedlocations/New.java b/framework/tests/qualifiedlocations/New.java new file mode 100644 index 000000000000..4ebd169de123 --- /dev/null +++ b/framework/tests/qualifiedlocations/New.java @@ -0,0 +1,13 @@ +package qualifiedlocations; + +import testlib.qualifiedlocations.qual.Bottom; +import testlib.qualifiedlocations.qual.Top; + +public class New { + { + //:: error: (new.annotation.forbidden) + new @Bottom Object(); + //:: error: (new.annotation.forbidden) + int a = new @Top String @Bottom [] {"string"}.length; + } +} diff --git a/framework/tests/qualifiedlocations/Parameter.java b/framework/tests/qualifiedlocations/Parameter.java new file mode 100644 index 000000000000..07c59e257253 --- /dev/null +++ b/framework/tests/qualifiedlocations/Parameter.java @@ -0,0 +1,9 @@ +package qualifiedlocations; + +import testlib.qualifiedlocations.qual.Bottom; + +public class Parameter { + + //:: error: (parameter.annotation.forbidden) + void foo(@Bottom Object p) {} +} diff --git a/framework/tests/qualifiedlocations/Receiver.java b/framework/tests/qualifiedlocations/Receiver.java new file mode 100644 index 000000000000..cd9715ec6405 --- /dev/null +++ b/framework/tests/qualifiedlocations/Receiver.java @@ -0,0 +1,9 @@ +package qualifiedlocations; + +import testlib.qualifiedlocations.qual.Bottom; + +public class Receiver { + + //:: error: (receiver.annotation.forbidden) + void foo(@Bottom Receiver this) {} +} diff --git a/framework/tests/qualifiedlocations/Return.java b/framework/tests/qualifiedlocations/Return.java new file mode 100644 index 000000000000..841e3ab5c41d --- /dev/null +++ b/framework/tests/qualifiedlocations/Return.java @@ -0,0 +1,11 @@ +package qualifiedlocations; + +import testlib.qualifiedlocations.qual.Bottom; + +public class Return { + + //:: error: (return.annotation.forbidden) + @Bottom Object foo() { + return null; + } +} diff --git a/framework/tests/qualifiedlocations/Throws.java b/framework/tests/qualifiedlocations/Throws.java new file mode 100644 index 000000000000..af09a3839ea0 --- /dev/null +++ b/framework/tests/qualifiedlocations/Throws.java @@ -0,0 +1,9 @@ +package qualifiedlocations; + +import testlib.qualifiedlocations.qual.Bottom; + +public class Throws { + + //:: error: (throws.annotation.forbidden) + void foo() throws @Bottom Exception {} +} diff --git a/framework/tests/qualifiedlocations/TypeArgument.java b/framework/tests/qualifiedlocations/TypeArgument.java new file mode 100644 index 000000000000..73cd509c6c89 --- /dev/null +++ b/framework/tests/qualifiedlocations/TypeArgument.java @@ -0,0 +1,9 @@ +package qualifiedlocations; + +import java.util.List; +import testlib.qualifiedlocations.qual.Bottom; + +public class TypeArgument { + //:: error: (type_argument.annotation.forbidden) + List<@Bottom Integer> list; +} diff --git a/framework/tests/qualifiedlocations/TypeDeclaration.java b/framework/tests/qualifiedlocations/TypeDeclaration.java new file mode 100644 index 000000000000..5cc733ae55ff --- /dev/null +++ b/framework/tests/qualifiedlocations/TypeDeclaration.java @@ -0,0 +1,6 @@ +package qualifiedlocations; + +import testlib.qualifiedlocations.qual.Bottom; + +//:: error: (type_declaration.annotation.forbidden) +@Bottom public class TypeDeclaration {} diff --git a/framework/tests/src/testlib/qualifiedlocations/QualifiedLocationsAnnotatedTypeFactory.java b/framework/tests/src/testlib/qualifiedlocations/QualifiedLocationsAnnotatedTypeFactory.java new file mode 100644 index 000000000000..cc35605a06e5 --- /dev/null +++ b/framework/tests/src/testlib/qualifiedlocations/QualifiedLocationsAnnotatedTypeFactory.java @@ -0,0 +1,33 @@ +package testlib.qualifiedlocations; + +import java.lang.annotation.Annotation; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; +import javax.lang.model.element.AnnotationMirror; +import org.checkerframework.common.basetype.BaseAnnotatedTypeFactory; +import org.checkerframework.common.basetype.BaseTypeChecker; +import org.checkerframework.javacutil.AnnotationUtils; +import testlib.qualifiedlocations.qual.Bottom; +import testlib.qualifiedlocations.qual.Top; + +/** Created by mier on 05/07/17. */ +public class QualifiedLocationsAnnotatedTypeFactory extends BaseAnnotatedTypeFactory { + + public AnnotationMirror TOP, BOTTOM; + + public QualifiedLocationsAnnotatedTypeFactory(BaseTypeChecker checker) { + super(checker); + TOP = AnnotationUtils.fromClass(elements, Top.class); + BOTTOM = AnnotationUtils.fromClass(elements, Bottom.class); + postInit(); + } + + @Override + protected Set> createSupportedTypeQualifiers() { + Set> annotations = + new HashSet<>(Arrays.asList(Top.class, Bottom.class)); + return Collections.unmodifiableSet(annotations); + } +} diff --git a/framework/tests/src/testlib/qualifiedlocations/QualifiedLocationsChecker.java b/framework/tests/src/testlib/qualifiedlocations/QualifiedLocationsChecker.java new file mode 100644 index 000000000000..9fc19bd2d5aa --- /dev/null +++ b/framework/tests/src/testlib/qualifiedlocations/QualifiedLocationsChecker.java @@ -0,0 +1,6 @@ +package testlib.qualifiedlocations; + +import org.checkerframework.common.basetype.BaseTypeChecker; + +/** Created by mier on 05/07/17. */ +public class QualifiedLocationsChecker extends BaseTypeChecker {} diff --git a/framework/tests/src/testlib/qualifiedlocations/QualifiedLocationsVisitor.java b/framework/tests/src/testlib/qualifiedlocations/QualifiedLocationsVisitor.java new file mode 100644 index 000000000000..1650f2d9ff76 --- /dev/null +++ b/framework/tests/src/testlib/qualifiedlocations/QualifiedLocationsVisitor.java @@ -0,0 +1,25 @@ +package testlib.qualifiedlocations; + +import com.sun.source.tree.TypeCastTree; +import java.util.Set; +import javax.lang.model.element.AnnotationMirror; +import org.checkerframework.common.basetype.BaseTypeChecker; +import org.checkerframework.common.basetype.BaseTypeVisitor; + +/** Created by mier on 05/07/17. */ +public class QualifiedLocationsVisitor + extends BaseTypeVisitor { + public QualifiedLocationsVisitor(BaseTypeChecker checker) { + super(checker); + } + + @Override + protected void checkTypecastSafety(TypeCastTree node, Void p) { + return; + } + + @Override + protected Set getExceptionParameterLowerBoundAnnotations() { + return atypeFactory.getQualifierHierarchy().getBottomAnnotations(); + } +} diff --git a/framework/tests/src/testlib/qualifiedlocations/qual/Bottom.java b/framework/tests/src/testlib/qualifiedlocations/qual/Bottom.java new file mode 100644 index 000000000000..115a096e5966 --- /dev/null +++ b/framework/tests/src/testlib/qualifiedlocations/qual/Bottom.java @@ -0,0 +1,20 @@ +package testlib.qualifiedlocations.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; +import org.checkerframework.framework.qual.ImplicitFor; +import org.checkerframework.framework.qual.LiteralKind; +import org.checkerframework.framework.qual.QualifiedLocations; +import org.checkerframework.framework.qual.SubtypeOf; + +/** Created by mier on 05/07/17. */ +@SubtypeOf({Top.class}) +@Documented +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.TYPE_USE, ElementType.TYPE_PARAMETER}) +@QualifiedLocations({}) +@ImplicitFor(literals = {LiteralKind.NULL}) +public @interface Bottom {} diff --git a/framework/tests/src/testlib/qualifiedlocations/qual/Top.java b/framework/tests/src/testlib/qualifiedlocations/qual/Top.java new file mode 100644 index 000000000000..29b84b05a7b6 --- /dev/null +++ b/framework/tests/src/testlib/qualifiedlocations/qual/Top.java @@ -0,0 +1,17 @@ +package testlib.qualifiedlocations.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; +import org.checkerframework.framework.qual.DefaultQualifierInHierarchy; +import org.checkerframework.framework.qual.SubtypeOf; + +/** Created by mier on 05/07/17. */ +@SubtypeOf({}) +@Documented +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.TYPE_USE, ElementType.TYPE_PARAMETER}) +@DefaultQualifierInHierarchy +public @interface Top {} diff --git a/framework/tests/src/tests/QualifiedLocationsTest.java b/framework/tests/src/tests/QualifiedLocationsTest.java new file mode 100644 index 000000000000..09036a3eee15 --- /dev/null +++ b/framework/tests/src/tests/QualifiedLocationsTest.java @@ -0,0 +1,20 @@ +package tests; + +import java.io.File; +import java.util.List; +import org.checkerframework.framework.test.CheckerFrameworkPerDirectoryTest; +import org.junit.runners.Parameterized.Parameters; +import testlib.qualifiedlocations.QualifiedLocationsChecker; + +/** Created by mier on 06/07/17. */ +public class QualifiedLocationsTest extends CheckerFrameworkPerDirectoryTest { + + public QualifiedLocationsTest(List testFiles) { + super(testFiles, QualifiedLocationsChecker.class, "qualifiedlocations", "-Anomsgtext"); + } + + @Parameters + public static String[] getTestDirs() { + return new String[] {"qualifiedlocations"}; + } +} From 91b44e362b7b7d2380fd4ce13755db3060128975 Mon Sep 17 00:00:00 2001 From: tamier Date: Sun, 9 Jul 2017 11:44:26 -0400 Subject: [PATCH 07/13] Don't validate void methods --- .../org/checkerframework/common/basetype/BaseTypeVisitor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java index 2c0f16eb5c00..bab8c226113f 100644 --- a/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -3518,7 +3518,7 @@ public boolean validateTypeOf(Tree tree) { // Nothing to do for void methods. // Note that for a constructor the AnnotatedExecutableType does // not use void as return type. - return typeValidator.isValid(type, tree); + return true; } break; default: From 95cd83ed7a326fc66cc178f3a6c03506dc3bf45a Mon Sep 17 00:00:00 2001 From: tamier Date: Sun, 9 Jul 2017 14:24:34 -0400 Subject: [PATCH 08/13] Fix build error --- .../common/basetype/BaseTypeVisitor.java | 45 ++++++------------- .../util/defaults/QualifierDefaults.java | 12 +---- .../qualifiedlocations/ArrayComponent.java | 2 + .../tests/qualifiedlocations/Locations.java | 4 +- 4 files changed, 18 insertions(+), 45 deletions(-) diff --git a/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java index bab8c226113f..8137eb28eb71 100644 --- a/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -295,35 +295,6 @@ public final Void visitClass(ClassTree classTree, Void p) { visitorState.setMethodTree(null); visitorState.setAssignmentContext(null); try { - if (!TreeUtils.hasExplicitConstructor(classTree)) { - checkDefaultConstructor(classTree); - } - - // We validate class tree to see if the annotation on it is allowed on type declaration location - validateTypeOf(classTree); - checkQualifiedLocation( - atypeFactory.getAnnotatedType(classTree), - classTree, - TypeUseLocation.TYPE_DECLARATION); - /* Visit the extends and implements clauses. - * The superclass also visits them, but only calls visitParameterizedType, which - * loses a main modifier. - */ - Tree ext = classTree.getExtendsClause(); - if (ext != null) { - validateTypeOf(ext); - checkQualifiedLocation( - atypeFactory.getAnnotatedType(ext), ext, TypeUseLocation.EXTENDS); - } - - List impls = classTree.getImplementsClause(); - if (impls != null) { - for (Tree im : impls) { - validateTypeOf(im); - checkQualifiedLocation( - atypeFactory.getAnnotatedType(im), im, TypeUseLocation.IMPLEMENTS); - } - } processClassTree(classTree); atypeFactory.postProcessClassTree(classTree); } finally { @@ -348,6 +319,11 @@ public void processClassTree(ClassTree classTree) { checkDefaultConstructor(classTree); } + checkQualifiedLocation( + atypeFactory.getAnnotatedType(classTree), + classTree, + TypeUseLocation.TYPE_DECLARATION); + /* Visit the extends and implements clauses. * The superclass also visits them, but only calls visitParameterizedType, which * loses a main modifier. @@ -355,12 +331,16 @@ public void processClassTree(ClassTree classTree) { Tree ext = classTree.getExtendsClause(); if (ext != null) { validateTypeOf(ext); + checkQualifiedLocation( + atypeFactory.getAnnotatedType(ext), ext, TypeUseLocation.EXTENDS); } List impls = classTree.getImplementsClause(); if (impls != null) { for (Tree im : impls) { validateTypeOf(im); + checkQualifiedLocation( + atypeFactory.getAnnotatedType(im), im, TypeUseLocation.IMPLEMENTS); } } super.visitClass(classTree, null); @@ -1386,8 +1366,10 @@ public Void visitReturn(ReturnTree node, Void p) { if (enclosing.getKind() == Tree.Kind.METHOD) { MethodTree enclosingMethod = TreeUtils.enclosingMethod(getCurrentPath()); - // We don't validate enclosingMethod again, because it's already been validated during visitMethod - ret = atypeFactory.getMethodReturnType(enclosingMethod, node); + boolean valid = validateTypeOf(enclosing); + if (valid) { + ret = atypeFactory.getMethodReturnType(enclosingMethod, node); + } } else { Pair result = @@ -1735,7 +1717,6 @@ public Void visitInstanceOf(InstanceOfTree node, Void p) { Tree typeTree = node.getType(); AnnotatedTypeMirror instanceOfType = atypeFactory.getAnnotatedType(typeTree); checkQualifiedLocation(instanceOfType, typeTree, TypeUseLocation.INSTANCEOF); - validateTypeOf(node); validateTypeOf(node.getType()); return super.visitInstanceOf(node, p); } diff --git a/framework/src/org/checkerframework/framework/util/defaults/QualifierDefaults.java b/framework/src/org/checkerframework/framework/util/defaults/QualifierDefaults.java index 83383810ee2d..6d311459124c 100644 --- a/framework/src/org/checkerframework/framework/util/defaults/QualifierDefaults.java +++ b/framework/src/org/checkerframework/framework/util/defaults/QualifierDefaults.java @@ -108,8 +108,7 @@ public class QualifierDefaults { TypeUseLocation.LOCAL_VARIABLE, TypeUseLocation.RESOURCE_VARIABLE, TypeUseLocation.EXCEPTION_PARAMETER, - TypeUseLocation.IMPLICIT_UPPER_BOUND, - TypeUseLocation.TYPE_DECLARATION + TypeUseLocation.IMPLICIT_UPPER_BOUND }; /** CLIMB locations whose standard default is bottom for a given type system. */ @@ -976,15 +975,6 @@ public Void scan(AnnotatedTypeMirror t, AnnotationMirror qual) { } break; } - case TYPE_DECLARATION: - { - if (scope != null - && (scope.getKind().isClass() || scope.getKind().isInterface()) - && t == type) { - addAnnotation(t, qual); - } - break; - } case OTHERWISE: case ALL: { diff --git a/framework/tests/qualifiedlocations/ArrayComponent.java b/framework/tests/qualifiedlocations/ArrayComponent.java index ee85016666ee..e6bba82013f4 100644 --- a/framework/tests/qualifiedlocations/ArrayComponent.java +++ b/framework/tests/qualifiedlocations/ArrayComponent.java @@ -6,6 +6,8 @@ public class ArrayComponent { //:: error: (array_component.annotation.forbidden) @Bottom Object[] array; //:: error: (array_component.annotation.forbidden) + @Bottom Number[] @Bottom [] twoDimensionArray; + //:: error: (array_component.annotation.forbidden) @Bottom Object[] foo() { return null; } diff --git a/framework/tests/qualifiedlocations/Locations.java b/framework/tests/qualifiedlocations/Locations.java index e3aae4641244..ba2b08231460 100644 --- a/framework/tests/qualifiedlocations/Locations.java +++ b/framework/tests/qualifiedlocations/Locations.java @@ -4,8 +4,8 @@ import java.util.List; import testlib.qualifiedlocations.qual.Bottom; -//:: error: (array_component.annotation.forbidden) :: error: (type_argument.annotation.forbidden) :: error: (explicit_upper_bound.annotation.forbidden) -public class Locations> +//:: error: (type_declaration.annotation.forbidden) :: error: (type_argument.annotation.forbidden) :: error: (explicit_upper_bound.annotation.forbidden) +public class Locations> //:: error: (type_argument.annotation.forbidden) :: error: (extends.annotation.forbidden) :: error: (implements.annotation.forbidden) extends @Bottom ArrayList<@Bottom Object> implements @Bottom Iterable<@Bottom Object> { //:: error: (field.annotation.forbidden) From f6269315cf8c7f3bb207fc5cc6b71919b985c17c Mon Sep 17 00:00:00 2001 From: tamier Date: Sun, 9 Jul 2017 18:54:28 -0400 Subject: [PATCH 09/13] Fix build error --- .../tests/qualifiedlocations/ExplicitUpperBound.java | 4 ++-- framework/tests/qualifiedlocations/Extends.java | 2 +- framework/tests/qualifiedlocations/Implements.java | 2 +- .../tests/qualifiedlocations/ImplicitUpperBound.java | 2 +- framework/tests/qualifiedlocations/Locations.java | 10 +++++----- .../tests/qualifiedlocations/TypeDeclaration.java | 2 +- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/framework/tests/qualifiedlocations/ExplicitUpperBound.java b/framework/tests/qualifiedlocations/ExplicitUpperBound.java index 03e6d1b956a3..3ca77d0d95e5 100644 --- a/framework/tests/qualifiedlocations/ExplicitUpperBound.java +++ b/framework/tests/qualifiedlocations/ExplicitUpperBound.java @@ -3,9 +3,9 @@ import java.util.Set; import testlib.qualifiedlocations.qual.Bottom; -//:: error: (explicit_upper_bound.annotation.forbidden) +//:: error: (explicit_upper_bound.annotation.forbidden) :: error: (implicit_lower_bound.annotation.forbidden) public class ExplicitUpperBound { - //:: error: (explicit_upper_bound.annotation.forbidden) + //:: error: (explicit_upper_bound.annotation.forbidden) :: error: (implicit_lower_bound.annotation.forbidden) Set foo() { return null; } diff --git a/framework/tests/qualifiedlocations/Extends.java b/framework/tests/qualifiedlocations/Extends.java index cac448a665c1..1f067b953da2 100644 --- a/framework/tests/qualifiedlocations/Extends.java +++ b/framework/tests/qualifiedlocations/Extends.java @@ -2,5 +2,5 @@ import testlib.qualifiedlocations.qual.Bottom; -//:: error: (extends.annotation.forbidden) :: error: (type_declaration.annotation.forbidden) +//:: error: (extends.annotation.forbidden) :: error: (type_declaration.annotation.forbidden) :: error: (type.invalid) public class Extends extends @Bottom Object {} diff --git a/framework/tests/qualifiedlocations/Implements.java b/framework/tests/qualifiedlocations/Implements.java index 49a421ba7a65..c37428d8dbd4 100644 --- a/framework/tests/qualifiedlocations/Implements.java +++ b/framework/tests/qualifiedlocations/Implements.java @@ -6,7 +6,7 @@ interface Test { void foo(); } -//:: error: (implements.annotation.forbidden) :: error: (type_declaration.annotation.forbidden) +//:: error: (implements.annotation.forbidden) :: error: (type_declaration.annotation.forbidden) :: error: (type.invalid) public class Implements implements @Bottom Test { public void foo() { return; diff --git a/framework/tests/qualifiedlocations/ImplicitUpperBound.java b/framework/tests/qualifiedlocations/ImplicitUpperBound.java index b4b5f1e06330..dd2e37c73463 100644 --- a/framework/tests/qualifiedlocations/ImplicitUpperBound.java +++ b/framework/tests/qualifiedlocations/ImplicitUpperBound.java @@ -5,5 +5,5 @@ import testlib.qualifiedlocations.qual.Bottom; @DefaultQualifier(value = Bottom.class, locations = TypeUseLocation.UPPER_BOUND) -//:: error: (implicit_upper_bound.annotation.forbidden) +//:: error: (implicit_upper_bound.annotation.forbidden) :: error: (implicit_lower_bound.annotation.forbidden) public class ImplicitUpperBound {} diff --git a/framework/tests/qualifiedlocations/Locations.java b/framework/tests/qualifiedlocations/Locations.java index ba2b08231460..b790ac3547cd 100644 --- a/framework/tests/qualifiedlocations/Locations.java +++ b/framework/tests/qualifiedlocations/Locations.java @@ -4,7 +4,7 @@ import java.util.List; import testlib.qualifiedlocations.qual.Bottom; -//:: error: (type_declaration.annotation.forbidden) :: error: (type_argument.annotation.forbidden) :: error: (explicit_upper_bound.annotation.forbidden) +//:: error: (type_declaration.annotation.forbidden) :: error: (type_argument.annotation.forbidden) :: error: (explicit_upper_bound.annotation.forbidden) :: error: (implicit_lower_bound.annotation.forbidden) :: error: (type.invalid) public class Locations> //:: error: (type_argument.annotation.forbidden) :: error: (extends.annotation.forbidden) :: error: (implements.annotation.forbidden) extends @Bottom ArrayList<@Bottom Object> implements @Bottom Iterable<@Bottom Object> { @@ -12,7 +12,7 @@ public class Locations> @Bottom T t; //:: error: (field.annotation.forbidden) :: error: (type_argument.annotation.forbidden) @Bottom List<@Bottom ArrayList<@Bottom String>> l; - //:: error: (type_argument.annotation.forbidden) + //:: error: (type_argument.annotation.forbidden) :: error: (array_component.annotation.forbidden) List<@Bottom String @Bottom [] @Bottom []> f; // It's strange that array component doesn't show error @@ -20,8 +20,8 @@ public class Locations> void foo() throws @Bottom Exception { //:: error: (local_variable.annotation.forbidden) :: error: (type_argument.annotation.forbidden) :: error: (new.annotation.forbidden) @Bottom Object l = new @Bottom ArrayList<@Bottom Object>(); - //:: error: (instanceof.annotation.forbidden) :: error: (cast.annotation.forbidden) - boolean b = (@Bottom Object) l instanceof @Bottom List @Bottom []; + //:: error: (instanceof.annotation.forbidden) :: error: (cast.annotation.forbidden) :: error: (array_component.annotation.forbidden) + boolean b = (@Bottom Object) l instanceof @Bottom Object @Bottom []; } //:: error: (parameter.annotation.forbidden) @@ -33,7 +33,7 @@ void bar(@Bottom Object p) { } } - //:: error: (return.annotation.forbidden) :: error: (explicit_upper_bound.annotation.forbidden) :: error: (type_argument.annotation.forbidden) :: error: (receiver.annotation.forbidden) + //:: error: (return.annotation.forbidden) :: error: (explicit_upper_bound.annotation.forbidden) :: error: (type_argument.annotation.forbidden) :: error: (receiver.annotation.forbidden) :: error: (implicit_lower_bound.annotation.forbidden) > @Bottom Object hey(Locations<@Bottom T> this, S s) { return null; } diff --git a/framework/tests/qualifiedlocations/TypeDeclaration.java b/framework/tests/qualifiedlocations/TypeDeclaration.java index 5cc733ae55ff..e8dcc591e026 100644 --- a/framework/tests/qualifiedlocations/TypeDeclaration.java +++ b/framework/tests/qualifiedlocations/TypeDeclaration.java @@ -2,5 +2,5 @@ import testlib.qualifiedlocations.qual.Bottom; -//:: error: (type_declaration.annotation.forbidden) +//:: error: (type_declaration.annotation.forbidden) :: error: (type.invalid) @Bottom public class TypeDeclaration {} From 5131ad60859dfdf5cbb151eb0770753286b4aa7d Mon Sep 17 00:00:00 2001 From: tamier Date: Sun, 9 Jul 2017 19:38:35 -0400 Subject: [PATCH 10/13] Fix code style and compiler message string issue --- .../common/basetype/BaseTypeVisitor.java | 9 +++------ .../checkerframework/common/basetype/messages.properties | 4 ++-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java index 8137eb28eb71..50a9ecd2bf6f 100644 --- a/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -2690,12 +2690,9 @@ protected void checkQualifiedLocation( private void reportLocationError( AnnotatedTypeMirror type, Tree tree, TypeUseLocation location) { - checker.report( - Result.failure( - location.toString().toLowerCase() + ".annotation.forbidden", - type.getAnnotations(), - type.toString()), - tree); + /*@CompilerMessageKey*/ String errorMessage = + location.toString().toLowerCase() + ".annotation.forbidden"; + checker.report(Result.failure(errorMessage, type.getAnnotations(), type.toString()), tree); } /** diff --git a/framework/src/org/checkerframework/common/basetype/messages.properties b/framework/src/org/checkerframework/common/basetype/messages.properties index 13b5078d6bc6..f145059a6fca 100644 --- a/framework/src/org/checkerframework/common/basetype/messages.properties +++ b/framework/src/org/checkerframework/common/basetype/messages.properties @@ -109,6 +109,6 @@ array_component.annotation.forbidden= %s is forbidden on array component! extends.annotation.forbidden= %s is forbidden on extend clauses! implements.annotation.forbidden= %s is forbidden on implement clauses! new.annotation.forbidden= %s is forbidden on new instance creation! -throws.annotation.forbidden= %s is forbidden on thrown exception declaration! +throws.annotation.forbidden= %s is forbidden on throws! instanceof.annotation.forbidden= %s is forbidden on instanceof type! -cast.annotation.forbidden= %s is forbidden on type cast! \ No newline at end of file +cast.annotation.forbidden= %s is forbidden on type cast! From f35537c0b7c57db1023f1b5d8c16d22637f51218 Mon Sep 17 00:00:00 2001 From: tamier Date: Sun, 9 Jul 2017 21:21:47 -0400 Subject: [PATCH 11/13] fix build error --- .../org/checkerframework/common/basetype/BaseTypeVisitor.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java index 50a9ecd2bf6f..70480ec49ad4 100644 --- a/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -893,6 +893,8 @@ private void validateQualifiedLocationForVariableTree(VariableTree node) { checkQualifiedLocation(type, node, TypeUseLocation.PARAMETER); } break; + default: + break; } } @@ -2690,6 +2692,7 @@ protected void checkQualifiedLocation( private void reportLocationError( AnnotatedTypeMirror type, Tree tree, TypeUseLocation location) { + @SuppressWarnings("CompilerMessages") /*@CompilerMessageKey*/ String errorMessage = location.toString().toLowerCase() + ".annotation.forbidden"; checker.report(Result.failure(errorMessage, type.getAnnotations(), type.toString()), tree); From c95413cdf42f2edddb506e739053e98d481600ce Mon Sep 17 00:00:00 2001 From: tamier Date: Mon, 10 Jul 2017 10:47:49 -0400 Subject: [PATCH 12/13] Clean up the code and add some documentation --- .../common/basetype/BaseTypeValidator.java | 25 +++++-------------- .../common/basetype/BaseTypeVisitor.java | 14 +---------- .../common/basetype/messages.properties | 1 + .../framework/qual/TypeUseLocation.java | 12 ++++++--- .../framework/util/BoundTypeUtil.java | 1 + 5 files changed, 17 insertions(+), 36 deletions(-) diff --git a/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java b/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java index 149b51eb8511..906c15d9e7fe 100644 --- a/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java +++ b/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java @@ -497,8 +497,8 @@ public boolean areBoundsValid( } /** - * Visit the bounds of a type variable or a wildcard and potentially apply qual to those bounds. - * This method will also update the boundType, isLowerBound, and isUpperbound fields. + * Validates the annotation are qualified to be used on the bounds of a type variable or a + * wildcard. */ protected void validateQualifiedLocationsOnBounds( AnnotatedTypeMirror boundedType, @@ -507,31 +507,18 @@ protected void validateQualifiedLocationsOnBounds( Tree p) { BoundType boundType = BoundTypeUtil.getBoundType(boundedType, atypeFactory); - // TODO Is this correct to use this as condition to check if it's type parameter declaration - - // We only need to validate explicit main annotation on lower/upper bounds. So no need to - // visit recursively. They will be scan afterwards in different trees from which the deeper - // types can be validated - //scanAndReduce(upperBound, p, null); - // Checking upper bound - //if (p.getKind() == Tree.Kind.TYPE_PARAMETER) { + if (boundType.isOneOf(BoundType.UPPER)) { //Explicit upper bound visitor.checkQualifiedLocation(upperBound, p, TypeUseLocation.EXPLICIT_UPPER_BOUND); } else if (boundType.isOneOf(BoundType.UNBOUND, BoundType.LOWER, BoundType.UNKNOWN)) { - // Implicit upper bound => Do nothing + // Implicit upper bound visitor.checkQualifiedLocation(upperBound, p, TypeUseLocation.IMPLICIT_UPPER_BOUND); } else { - // Upper bound + // Dead code visitor.checkQualifiedLocation(upperBound, p, TypeUseLocation.UPPER_BOUND); } - //} - // We only need to validate explicit main annotation on lower/upper bounds. So no need to - // visit recursively. They will be scan afterwards in different trees - //scanAndReduce(lowerBound, p, null); - // Checking lower bound - //if (p.getKind() == Tree.Kind.TYPE_PARAMETER) { if (boundType.isOneOf(BoundType.LOWER)) { // Explicit lower bound visitor.checkQualifiedLocation(lowerBound, p, TypeUseLocation.EXPLICIT_LOWER_BOUND); @@ -539,9 +526,9 @@ protected void validateQualifiedLocationsOnBounds( // Implicit lower bound visitor.checkQualifiedLocation(lowerBound, p, TypeUseLocation.IMPLICIT_LOWER_BOUND); } else { + // Dead code visitor.checkQualifiedLocation(lowerBound, p, TypeUseLocation.LOWER_BOUND); } - //} } /** diff --git a/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java index 70480ec49ad4..904b1eec774c 100644 --- a/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -829,15 +829,6 @@ public Void visitTypeParameter(TypeParameterTree node, Void p) { return super.visitTypeParameter(node, p); } - /* @Override - public Void visitArrayType(ArrayTypeTree node, Void aVoid) { - // Why this doesn't work? arrayType didn't include explicit annotations! Why? - AnnotatedArrayType arrayType = (AnnotatedArrayType) atypeFactory.getAnnotatedTypeFromTypeTree(node); - Tree componenetTree = node.getType(); - checkQualifiedLocation(arrayType.getComponentType(), componenetTree, TypeUseLocation.ARRAY_COMPONENT); - return super.visitArrayType(node, aVoid); - }*/ - // ********************************************************************** // Assignment checkers and pseudo-assignments // ********************************************************************** @@ -1372,7 +1363,6 @@ public Void visitReturn(ReturnTree node, Void p) { if (valid) { ret = atypeFactory.getMethodReturnType(enclosingMethod, node); } - } else { Pair result = atypeFactory.getFnInterfaceFromTree((LambdaExpressionTree) enclosing); @@ -1936,7 +1926,6 @@ protected void commonAssignmentCheck( if (shouldSkipUses(valueExp)) { return; } - if (varType.getKind() == TypeKind.ARRAY && valueExp instanceof NewArrayTree && ((NewArrayTree) valueExp).getType() == null) { @@ -1946,7 +1935,6 @@ protected void commonAssignmentCheck( : "array initializers are not expected to be null in: " + valueExp; checkArrayInitialization(compType, arrayTree.getInitializers()); } - // Causes duplicate warnings on new, instanceof if (!validateTypeOf(valueExp)) { return; } @@ -2671,7 +2659,7 @@ protected void checkQualifiedLocation( Element elementOfAnnotation = am.getAnnotationType().asElement(); QualifiedLocations declLocations = elementOfAnnotation.getAnnotation(QualifiedLocations.class); - // Null means no TargetLocations specified => Any use is correct. + // Null means no QualifiedLocations annotation => Any use is correct. if (declLocations != null) { Set set = new HashSet<>(Arrays.asList(declLocations.value())); if (set.contains(TypeUseLocation.ALL)) continue; diff --git a/framework/src/org/checkerframework/common/basetype/messages.properties b/framework/src/org/checkerframework/common/basetype/messages.properties index f145059a6fca..8194887d0568 100644 --- a/framework/src/org/checkerframework/common/basetype/messages.properties +++ b/framework/src/org/checkerframework/common/basetype/messages.properties @@ -83,6 +83,7 @@ contracts.conditional.postcondition.false.methodref.invalid=Conditional postcond lambda.unimplemented=This version of the Checker Framework does not type-check lambda expressions. methodref.inference.unimplemented=This version of the Checker Framework does not type-check method references with implicit type arguments. + field.invariant.not.found=the field invariant annotation refers to fields not found in a superclass\nfields not found: %s field.invariant.not.final=the field invariant annotation refers to fields that are not final\nfields not final: %s field.invariant.not.subtype=the qualifier for field %s is not a subtype of the declared type\nfound: %s\ndeclared type: %s diff --git a/framework/src/org/checkerframework/framework/qual/TypeUseLocation.java b/framework/src/org/checkerframework/framework/qual/TypeUseLocation.java index b585c0082b13..5b4e7847b4ad 100644 --- a/framework/src/org/checkerframework/framework/qual/TypeUseLocation.java +++ b/framework/src/org/checkerframework/framework/qual/TypeUseLocation.java @@ -83,28 +83,32 @@ public enum TypeUseLocation { /** Apply default annotations to unannotated type declarations: {@code @HERE class Demo{}} */ TYPE_DECLARATION, + /** * Represents type argument location in parameterized type {@code List<@TA1 ArrayList<@TA2 * String>>} */ TYPE_ARGUMENT, + /** Represents array component location {@code @AC2 String [] @AC1 []} */ ARRAY_COMPONENT, - /** - * TODO is this documentation correct? Or does it really represent interface extends case? - * Represents extends location of a class/interface/enum/annotation type - */ + /** Represents extends location of a class or interface: {@code class B extends @HERE A {}} */ EXTENDS, + /** Represents implements location of a class: {@code class B implements @HERE I {}} */ IMPLEMENTS, + /** Represents throws location of a method: {@code void foo throws @HERE Exception {}} */ THROWS, + /** Represents instanceof location: {@code o instanceof @HERE Object {}} */ INSTANCEOF, + /** Represents new expression location: {@code new @HERE Object()} */ NEW, + /** Represents casts location: {@code (@HERE Object)o} */ CAST, /** Apply if nothing more concrete is provided. TODO: clarify relation to ALL. */ diff --git a/framework/src/org/checkerframework/framework/util/BoundTypeUtil.java b/framework/src/org/checkerframework/framework/util/BoundTypeUtil.java index 942973eb6742..d057c0b36468 100644 --- a/framework/src/org/checkerframework/framework/util/BoundTypeUtil.java +++ b/framework/src/org/checkerframework/framework/util/BoundTypeUtil.java @@ -12,6 +12,7 @@ import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedWildcardType; import org.checkerframework.javacutil.ErrorReporter; +/** Utility class to get {@link BoundType} of a type variable or wildcard */ public class BoundTypeUtil { /** * @param type the type whose boundType is returned. type must be an AnnotatedWildcardType or From 335deb98bfc9b43e7d3d02c158ff0320a21a5d89 Mon Sep 17 00:00:00 2001 From: tamier Date: Wed, 29 Nov 2017 15:46:57 -0500 Subject: [PATCH 13/13] t --- .../common/basetype/BaseTypeValidator.java | 23 +-- .../common/basetype/BaseTypeVisitor.java | 48 ++++-- .../framework/qual/TypeUseLocation.java | 11 +- .../framework/util/BoundType.java | 24 +-- .../framework/util/BoundTypeUtil.java | 55 +++++- .../util/defaults/QualifierDefaults.java | 157 +----------------- 6 files changed, 102 insertions(+), 216 deletions(-) diff --git a/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java b/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java index 906c15d9e7fe..949282df218e 100644 --- a/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java +++ b/framework/src/org/checkerframework/common/basetype/BaseTypeValidator.java @@ -200,10 +200,6 @@ public Void visitDeclared(AnnotatedDeclaredType type, Tree tree) { : "size mismatch for type arguments: " + type + " and " + typeArgTree; for (int i = 0; i < tatypes.size(); ++i) { - visitor.checkQualifiedLocation( - tatypes.get(i), - typeArgTree.getTypeArguments().get(i), - TypeUseLocation.TYPE_ARGUMENT); scan(tatypes.get(i), typeArgTree.getTypeArguments().get(i)); } } @@ -305,7 +301,6 @@ public Void visitArray(AnnotatedArrayType type, Tree tree) { AnnotatedTypeMirror comp = type; do { comp = ((AnnotatedArrayType) comp).getComponentType(); - visitor.checkQualifiedLocation(comp, tree, TypeUseLocation.ARRAY_COMPONENT); } while (comp.getKind() == TypeKind.ARRAY); if (comp.getKind() == TypeKind.DECLARED @@ -508,26 +503,22 @@ protected void validateQualifiedLocationsOnBounds( BoundType boundType = BoundTypeUtil.getBoundType(boundedType, atypeFactory); - if (boundType.isOneOf(BoundType.UPPER)) { - //Explicit upper bound + // Upper bounds + if (BoundTypeUtil.isOneOf(boundType, BoundType.UPPER)) { + // Explicit upper bound visitor.checkQualifiedLocation(upperBound, p, TypeUseLocation.EXPLICIT_UPPER_BOUND); - } else if (boundType.isOneOf(BoundType.UNBOUND, BoundType.LOWER, BoundType.UNKNOWN)) { + } else { // Implicit upper bound visitor.checkQualifiedLocation(upperBound, p, TypeUseLocation.IMPLICIT_UPPER_BOUND); - } else { - // Dead code - visitor.checkQualifiedLocation(upperBound, p, TypeUseLocation.UPPER_BOUND); } - if (boundType.isOneOf(BoundType.LOWER)) { + // Lower bounds + if (BoundTypeUtil.isOneOf(boundType, BoundType.LOWER)) { // Explicit lower bound visitor.checkQualifiedLocation(lowerBound, p, TypeUseLocation.EXPLICIT_LOWER_BOUND); - } else if (boundType.isOneOf(BoundType.UNBOUND, BoundType.UPPER, BoundType.UNKNOWN)) { + } else { // Implicit lower bound visitor.checkQualifiedLocation(lowerBound, p, TypeUseLocation.IMPLICIT_LOWER_BOUND); - } else { - // Dead code - visitor.checkQualifiedLocation(lowerBound, p, TypeUseLocation.LOWER_BOUND); } } diff --git a/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java index 904b1eec774c..968c2475cb89 100644 --- a/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -62,6 +62,7 @@ import javax.lang.model.type.TypeVariable; import javax.lang.model.util.ElementFilter; import javax.tools.Diagnostic.Kind; +import org.checkerframework.checker.compilermsgs.qual.CompilerMessageKey; import org.checkerframework.dataflow.analysis.FlowExpressions; import org.checkerframework.dataflow.analysis.FlowExpressions.Receiver; import org.checkerframework.dataflow.analysis.TransferResult; @@ -1708,7 +1709,7 @@ public Void visitTypeCast(TypeCastTree node, Void p) { public Void visitInstanceOf(InstanceOfTree node, Void p) { Tree typeTree = node.getType(); AnnotatedTypeMirror instanceOfType = atypeFactory.getAnnotatedType(typeTree); - checkQualifiedLocation(instanceOfType, typeTree, TypeUseLocation.INSTANCEOF); + checkQualifiedLocation(instanceOfType, typeTree, TypeUseLocation.INSTANCE_OF); validateTypeOf(node.getType()); return super.visitInstanceOf(node, p); } @@ -2657,23 +2658,37 @@ protected void checkQualifiedLocation( AnnotatedTypeMirror type, Tree tree, TypeUseLocation location) { for (AnnotationMirror am : type.getAnnotations()) { Element elementOfAnnotation = am.getAnnotationType().asElement(); - QualifiedLocations declLocations = + QualifiedLocations qualifiedLocations = elementOfAnnotation.getAnnotation(QualifiedLocations.class); - // Null means no QualifiedLocations annotation => Any use is correct. - if (declLocations != null) { - Set set = new HashSet<>(Arrays.asList(declLocations.value())); - if (set.contains(TypeUseLocation.ALL)) continue; - if (((location == TypeUseLocation.EXPLICIT_LOWER_BOUND) - || (location == TypeUseLocation.IMPLICIT_LOWER_BOUND)) - && set.contains(TypeUseLocation.LOWER_BOUND)) { - // TypeUseLocation.LOWER_BOUND already covers both explicit and implicit lower bounds, so no need to check containment + // Null means no QualifiedLocations annotation => Any usage is correct. + if (qualifiedLocations == null) { + continue; + } + + Set set = new HashSet<>(Arrays.asList(qualifiedLocations.value())); + + if (set.contains(TypeUseLocation.ALL)) continue; + + if (set.contains(TypeUseLocation.LOWER_BOUND)) { + if (location == TypeUseLocation.EXPLICIT_LOWER_BOUND + || location == TypeUseLocation.IMPLICIT_LOWER_BOUND) { + // TypeUseLocation.LOWER_BOUND already covers both explicit and implicit lower + // bounds, so no need to check continue; - } else if (((location == TypeUseLocation.EXPLICIT_UPPER_BOUND) - || (location == TypeUseLocation.IMPLICIT_UPPER_BOUND)) - && set.contains(TypeUseLocation.UPPER_BOUND)) { - // TypeUseLocation.UPPER_BOUND already covers both explicit and implicit lower bounds, so no need to check containment + } + } + + if (set.contains(TypeUseLocation.UPPER_BOUND)) { + // TypeUseLocation.UPPER_BOUND already covers both explicit and implicit upper + // bounds, so no need to check + if (location == TypeUseLocation.EXPLICIT_UPPER_BOUND + || location == TypeUseLocation.IMPLICIT_UPPER_BOUND) { continue; - } else if (!set.contains(location)) reportLocationError(type, tree, location); + } + } + + if (!set.contains(location)) { + reportLocationError(type, tree, location); } } } @@ -2681,8 +2696,7 @@ protected void checkQualifiedLocation( private void reportLocationError( AnnotatedTypeMirror type, Tree tree, TypeUseLocation location) { @SuppressWarnings("CompilerMessages") - /*@CompilerMessageKey*/ String errorMessage = - location.toString().toLowerCase() + ".annotation.forbidden"; + @CompilerMessageKey String errorMessage = location.toString().toLowerCase() + ".annotation.forbidden"; checker.report(Result.failure(errorMessage, type.getAnnotations(), type.toString()), tree); } diff --git a/framework/src/org/checkerframework/framework/qual/TypeUseLocation.java b/framework/src/org/checkerframework/framework/qual/TypeUseLocation.java index 5b4e7847b4ad..ec4d707d2d40 100644 --- a/framework/src/org/checkerframework/framework/qual/TypeUseLocation.java +++ b/framework/src/org/checkerframework/framework/qual/TypeUseLocation.java @@ -84,15 +84,6 @@ public enum TypeUseLocation { /** Apply default annotations to unannotated type declarations: {@code @HERE class Demo{}} */ TYPE_DECLARATION, - /** - * Represents type argument location in parameterized type {@code List<@TA1 ArrayList<@TA2 - * String>>} - */ - TYPE_ARGUMENT, - - /** Represents array component location {@code @AC2 String [] @AC1 []} */ - ARRAY_COMPONENT, - /** Represents extends location of a class or interface: {@code class B extends @HERE A {}} */ EXTENDS, @@ -103,7 +94,7 @@ public enum TypeUseLocation { THROWS, /** Represents instanceof location: {@code o instanceof @HERE Object {}} */ - INSTANCEOF, + INSTANCE_OF, /** Represents new expression location: {@code new @HERE Object()} */ NEW, diff --git a/framework/src/org/checkerframework/framework/util/BoundType.java b/framework/src/org/checkerframework/framework/util/BoundType.java index 7e25ca287f5a..072c1e5d3635 100644 --- a/framework/src/org/checkerframework/framework/util/BoundType.java +++ b/framework/src/org/checkerframework/framework/util/BoundType.java @@ -1,5 +1,9 @@ package org.checkerframework.framework.util; +/** + * Specifies whether the type variable or wildcard has an explicit upper bound (UPPER), an explicit + * lower bound (LOWER), or no explicit bounds (UNBOUNDED). + */ public enum BoundType { /** Indicates an upper bounded type variable or wildcard */ @@ -8,22 +12,10 @@ public enum BoundType { /** Indicates a lower bounded type variable or wildcard */ LOWER, - /** Neither bound is specified, BOTH are implicit */ - UNBOUND, - /** - * For bytecode, or trees for which we no longer have the compilation unit. We treat UNKNOWN - * bounds as if they are an UPPER bound. + * Neither bound is specified, BOTH are implicit. (If a type variable is declared in bytecode + * and the type of the upper bound is Object, then the checker assumes that the bound was not + * explicitly written in source code.) */ - UNKNOWN; - - public boolean isOneOf(final BoundType... choices) { - for (final BoundType choice : choices) { - if (this == choice) { - return true; - } - } - - return false; - } + UNBOUNDED } diff --git a/framework/src/org/checkerframework/framework/util/BoundTypeUtil.java b/framework/src/org/checkerframework/framework/util/BoundTypeUtil.java index d057c0b36468..3572283bc6ea 100644 --- a/framework/src/org/checkerframework/framework/util/BoundTypeUtil.java +++ b/framework/src/org/checkerframework/framework/util/BoundTypeUtil.java @@ -5,18 +5,39 @@ import com.sun.source.util.TreePath; import com.sun.tools.javac.code.Type.WildcardType; import java.util.List; +import java.util.Map; +import javax.lang.model.element.Element; import javax.lang.model.element.TypeParameterElement; import org.checkerframework.framework.type.AnnotatedTypeFactory; import org.checkerframework.framework.type.AnnotatedTypeMirror; import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedTypeVariable; import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedWildcardType; +import org.checkerframework.javacutil.CollectionUtils; import org.checkerframework.javacutil.ErrorReporter; +import org.checkerframework.javacutil.TypesUtils; /** Utility class to get {@link BoundType} of a type variable or wildcard */ public class BoundTypeUtil { + + /** Mapping from an Element to the source Tree of the declaration. */ + private static final int CACHE_SIZE = 300; + + protected static final Map elementToBoundType = + CollectionUtils.createLRUCache(CACHE_SIZE); + + public static boolean isOneOf(final BoundType target, final BoundType... choices) { + for (final BoundType choice : choices) { + if (target == choice) { + return true; + } + } + + return false; + } + /** * @param type the type whose boundType is returned. type must be an AnnotatedWildcardType or - * AnnotatedTypeVariable + * AnnotatedTypeVariable. * @return the boundType for type */ public static BoundType getBoundType( @@ -34,15 +55,20 @@ public static BoundType getBoundType( } /** @return the bound type of the input typeVar */ - public static BoundType getTypeVarBoundType( + private static BoundType getTypeVarBoundType( final AnnotatedTypeVariable typeVar, final AnnotatedTypeFactory typeFactory) { return getTypeVarBoundType( (TypeParameterElement) typeVar.getUnderlyingType().asElement(), typeFactory); } - /** @return the boundType (UPPER, UNBOUND, or UNKNOWN) of the declaration of typeParamElem */ - public static BoundType getTypeVarBoundType( + /** @return the boundType (UPPER or UNBOUNDED) of the declaration of typeParamElem */ + // Results are cached in {@link elementToBoundType}. + private static BoundType getTypeVarBoundType( final TypeParameterElement typeParamElem, final AnnotatedTypeFactory typeFactory) { + final BoundType prev = elementToBoundType.get(typeParamElem); + if (prev != null) { + return prev; + } TreePath declaredTypeVarEle = typeFactory.getTreeUtils().getPath(typeParamElem); Tree typeParamDecl = declaredTypeVarEle == null ? null : declaredTypeVarEle.getLeaf(); @@ -50,8 +76,17 @@ public static BoundType getTypeVarBoundType( final BoundType boundType; if (typeParamDecl == null) { // This is not only for elements from binaries, but also - // when the compilation unit is no longer available. - boundType = BoundType.UNKNOWN; + // when the compilation unit is no-longer available. + if (typeParamElem.getBounds().size() == 1 + && TypesUtils.isObject(typeParamElem.getBounds().get(0))) { + // If the bound was Object, then it may or may not have been explicitly written. + // Assume that it was not. + boundType = BoundType.UNBOUNDED; + } else { + // The bound is not Object, so it must have been explicitly written and thus the + // type variable has an upper bound. + boundType = BoundType.UPPER; + } } else { if (typeParamDecl.getKind() == Tree.Kind.TYPE_PARAMETER) { @@ -61,7 +96,7 @@ public static BoundType getTypeVarBoundType( if (bnds != null && !bnds.isEmpty()) { boundType = BoundType.UPPER; } else { - boundType = BoundType.UNBOUND; + boundType = BoundType.UNBOUNDED; } } else { ErrorReporter.errorAbort( @@ -74,12 +109,13 @@ public static BoundType getTypeVarBoundType( } } + elementToBoundType.put(typeParamElem, boundType); return boundType; } /** * @return the BoundType of annotatedWildcard. If it is unbounded, use the type parameter to - * which its an argument + * which its an argument. */ public static BoundType getWildcardBoundType( final AnnotatedWildcardType annotatedWildcard, final AnnotatedTypeFactory typeFactory) { @@ -93,7 +129,8 @@ public static BoundType getWildcardBoundType( (TypeParameterElement) wildcard.bound.asElement(), typeFactory); } else { - // note: isSuperBound will be true for unbounded and lowers, but the unbounded case is already handled + // note: isSuperBound will be true for unbounded and lowers, but the unbounded case is + // already handled boundType = wildcard.isSuperBound() ? BoundType.LOWER : BoundType.UPPER; } diff --git a/framework/src/org/checkerframework/framework/util/defaults/QualifierDefaults.java b/framework/src/org/checkerframework/framework/util/defaults/QualifierDefaults.java index 6d311459124c..b190922d0c35 100644 --- a/framework/src/org/checkerframework/framework/util/defaults/QualifierDefaults.java +++ b/framework/src/org/checkerframework/framework/util/defaults/QualifierDefaults.java @@ -7,10 +7,8 @@ import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.MethodTree; import com.sun.source.tree.Tree; -import com.sun.source.tree.TypeParameterTree; import com.sun.source.tree.VariableTree; import com.sun.source.util.TreePath; -import com.sun.tools.javac.code.Type.WildcardType; import java.lang.annotation.Annotation; import java.util.EnumSet; import java.util.IdentityHashMap; @@ -21,7 +19,6 @@ import javax.lang.model.element.Element; import javax.lang.model.element.ElementKind; import javax.lang.model.element.PackageElement; -import javax.lang.model.element.TypeParameterElement; import javax.lang.model.type.MirroredTypeException; import javax.lang.model.type.TypeKind; import javax.lang.model.util.Elements; @@ -41,16 +38,16 @@ import org.checkerframework.framework.type.GenericAnnotatedTypeFactory; import org.checkerframework.framework.type.QualifierHierarchy; import org.checkerframework.framework.type.visitor.AnnotatedTypeScanner; +import org.checkerframework.framework.util.BoundType; +import org.checkerframework.framework.util.BoundTypeUtil; import org.checkerframework.framework.util.CheckerMain; import org.checkerframework.framework.util.PluginUtil; import org.checkerframework.javacutil.AnnotationBuilder; import org.checkerframework.javacutil.AnnotationUtils; -import org.checkerframework.javacutil.CollectionUtils; import org.checkerframework.javacutil.ElementUtils; import org.checkerframework.javacutil.ErrorReporter; import org.checkerframework.javacutil.InternalUtils; import org.checkerframework.javacutil.TreeUtils; -import org.checkerframework.javacutil.TypesUtils; /** * Determines the default qualifiers on a type. Default qualifiers are specified via the {@link @@ -87,12 +84,6 @@ public class QualifierDefaults { private final DefaultSet checkedCodeDefaults = new DefaultSet(); private final DefaultSet uncheckedCodeDefaults = new DefaultSet(); - /** Mapping from an Element to the source Tree of the declaration. */ - private static final int CACHE_SIZE = 300; - - protected static final Map elementToBoundType = - CollectionUtils.createLRUCache(CACHE_SIZE); - /** * Defaults that apply for a certain Element. On the one hand this is used for caching (an * earlier name for the field was "qualifierCache"). It can also be used by type systems to set @@ -931,7 +922,8 @@ public Void scan(AnnotatedTypeMirror t, AnnotationMirror qual) { case IMPLICIT_LOWER_BOUND: { if (isLowerBound - && boundType.isOneOf(BoundType.UNBOUNDED, BoundType.UPPER)) { + && BoundTypeUtil.isOneOf( + boundType, BoundType.UNBOUNDED, BoundType.UPPER)) { addAnnotation(t, qual); } break; @@ -939,7 +931,7 @@ public Void scan(AnnotatedTypeMirror t, AnnotationMirror qual) { case EXPLICIT_LOWER_BOUND: { - if (isLowerBound && boundType.isOneOf(BoundType.LOWER)) { + if (isLowerBound && BoundTypeUtil.isOneOf(boundType, BoundType.LOWER)) { addAnnotation(t, qual); } break; @@ -956,14 +948,15 @@ public Void scan(AnnotatedTypeMirror t, AnnotationMirror qual) { case IMPLICIT_UPPER_BOUND: { if (isUpperBound - && boundType.isOneOf(BoundType.UNBOUNDED, BoundType.LOWER)) { + && BoundTypeUtil.isOneOf( + boundType, BoundType.UNBOUNDED, BoundType.LOWER)) { addAnnotation(t, qual); } break; } case EXPLICIT_UPPER_BOUND: { - if (isUpperBound && boundType.isOneOf(BoundType.UPPER)) { + if (isUpperBound && BoundTypeUtil.isOneOf(boundType, BoundType.UPPER)) { addAnnotation(t, qual); } break; @@ -1046,7 +1039,7 @@ protected void visitBounds( final boolean prevIsLowerBound = isLowerBound; final BoundType prevBoundType = boundType; - boundType = getBoundType(boundedType, atypeFactory); + boundType = BoundTypeUtil.getBoundType(boundedType, atypeFactory); try { isLowerBound = true; @@ -1069,136 +1062,4 @@ protected void visitBounds( } } } - - /** - * Specifies whether the type variable or wildcard has an explicit upper bound (UPPER), an - * explicit lower bound (LOWER), or no explicit bounds (UNBOUNDED). - */ - enum BoundType { - - /** Indicates an upper bounded type variable or wildcard */ - UPPER, - - /** Indicates a lower bounded type variable or wildcard */ - LOWER, - - /** - * Neither bound is specified, BOTH are implicit. (If a type variable is declared in - * bytecode and the type of the upper bound is Object, then the checker assumes that the - * bound was not explicitly written in source code.) - */ - UNBOUNDED; - - public boolean isOneOf(final BoundType... choices) { - for (final BoundType choice : choices) { - if (this == choice) { - return true; - } - } - - return false; - } - } - - /** - * @param type the type whose boundType is returned. type must be an AnnotatedWildcardType or - * AnnotatedTypeVariable. - * @return the boundType for type - */ - private static BoundType getBoundType( - final AnnotatedTypeMirror type, final AnnotatedTypeFactory typeFactory) { - if (type instanceof AnnotatedTypeVariable) { - return getTypeVarBoundType((AnnotatedTypeVariable) type, typeFactory); - } - - if (type instanceof AnnotatedWildcardType) { - return getWildcardBoundType((AnnotatedWildcardType) type, typeFactory); - } - - ErrorReporter.errorAbort("Unexpected type kind: type=" + type); - return null; // dead code - } - - /** @return the bound type of the input typeVar */ - private static BoundType getTypeVarBoundType( - final AnnotatedTypeVariable typeVar, final AnnotatedTypeFactory typeFactory) { - return getTypeVarBoundType( - (TypeParameterElement) typeVar.getUnderlyingType().asElement(), typeFactory); - } - - /** @return the boundType (UPPER or UNBOUNDED) of the declaration of typeParamElem */ - // Results are cached in {@link elementToBoundType}. - private static BoundType getTypeVarBoundType( - final TypeParameterElement typeParamElem, final AnnotatedTypeFactory typeFactory) { - final BoundType prev = elementToBoundType.get(typeParamElem); - if (prev != null) { - return prev; - } - - TreePath declaredTypeVarEle = typeFactory.getTreeUtils().getPath(typeParamElem); - Tree typeParamDecl = declaredTypeVarEle == null ? null : declaredTypeVarEle.getLeaf(); - - final BoundType boundType; - if (typeParamDecl == null) { - // This is not only for elements from binaries, but also - // when the compilation unit is no-longer available. - if (typeParamElem.getBounds().size() == 1 - && TypesUtils.isObject(typeParamElem.getBounds().get(0))) { - // If the bound was Object, then it may or may not have been explicitly written. - // Assume that it was not. - boundType = BoundType.UNBOUNDED; - } else { - // The bound is not Object, so it must have been explicitly written and thus the - // type variable has an upper bound. - boundType = BoundType.UPPER; - } - - } else { - if (typeParamDecl.getKind() == Tree.Kind.TYPE_PARAMETER) { - final TypeParameterTree tptree = (TypeParameterTree) typeParamDecl; - - List bnds = tptree.getBounds(); - if (bnds != null && !bnds.isEmpty()) { - boundType = BoundType.UPPER; - } else { - boundType = BoundType.UNBOUNDED; - } - } else { - ErrorReporter.errorAbort( - "Unexpected tree type for typeVar Element:\n" - + "typeParamElem=" - + typeParamElem - + "\n" - + typeParamDecl); - boundType = null; // dead code - } - } - - elementToBoundType.put(typeParamElem, boundType); - return boundType; - } - - /** - * @return the BoundType of annotatedWildcard. If it is unbounded, use the type parameter to - * which its an argument. - */ - public static BoundType getWildcardBoundType( - final AnnotatedWildcardType annotatedWildcard, final AnnotatedTypeFactory typeFactory) { - - final WildcardType wildcard = (WildcardType) annotatedWildcard.getUnderlyingType(); - - final BoundType boundType; - if (wildcard.isUnbound() && wildcard.bound != null) { - boundType = - getTypeVarBoundType( - (TypeParameterElement) wildcard.bound.asElement(), typeFactory); - - } else { - // note: isSuperBound will be true for unbounded and lowers, but the unbounded case is - // already handled - boundType = wildcard.isSuperBound() ? BoundType.LOWER : BoundType.UPPER; - } - - return boundType; - } }