Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explain the need having three validateTargetLocationsXX methods #798

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
+92 −66
Open

Explain the need having three validateTargetLocationsXX methods #798

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
632493f
improve comments and readability
AndrewShf Oct 3, 2023
cc14e53
improve comments and readability
AndrewShf Oct 3, 2023
58b3f05
Apply suggestions from code review
AndrewShf Oct 23, 2023
633f95e
clarify the implementation details of having three validating methods
AndrewShf Oct 23, 2023
14e18e3
Merge branch 'master' into haifeng-targetlocation
Ao-senXiong Aug 19, 2024
ac3361b
Merge branch 'master' into haifeng-targetlocation
Ao-senXiong Sep 3, 2024
16e180e
Merge branch 'master' into haifeng-targetlocation
Ao-senXiong Sep 8, 2024
028cf63
Merge branch 'master' into haifeng-targetlocation
Ao-senXiong Dec 14, 2024
b145a93
Use fully qualified name to see if fix the javadoc error
Ao-senXiong Dec 15, 2024
876a91e
Try with linkplain
Ao-senXiong Dec 17, 2024
7368004
Don't use any link
Ao-senXiong Dec 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions framework/src/main/java/org/checkerframework/common/basetype/BaseTypeValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -729,9 +729,17 @@ public boolean areBoundsValid(AnnotatedTypeMirror upperBound, AnnotatedTypeMirro
}

/**
* Validate if qualifiers on wildcard are permitted by {@link
* org.checkerframework.framework.qual.TargetLocations}. Report an error if the actual use of
* this annotation is not listed in the declared TypeUseLocations in this meta-annotation.
* Validate if the qualifiers on the tree are at the right type-use locations, which is
* specified by the meta-annotation {@link org.checkerframework.framework.qual.TargetLocations}.
*
* <p>More specifically, this method only checks qualifiers on the WildcardTree and thus checks
* for these following type-use locations: (EXPLICIT/IMPLICIT) LOWER_BOUND and
* (EXPLICIT/IMPLICIT) UPPER_BOUND.
*
* <p>The other two validate methods achieve the same goal but perform checks on different trees
* and different type-use locations. See {@link
* BaseTypeVisitor#validateVariablesTargetLocation(VariableTree, AnnotatedTypeMirror)} and
* {@link BaseTypeVisitor#validateTargetLocation(Tree, AnnotatedTypeMirror, TypeUseLocation)}.
*
* @param type the type to check
* @param tree the tree of this type
Expand Down
144 changes: 81 additions & 63 deletions framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1670,87 +1670,105 @@ && getCurrentPath().getParentPath().getLeaf().getKind()
}

/**
* Validate if the annotations on the VariableTree are at the right locations, which is
* specified by the meta-annotation @TargetLocations. The difference of this method between
* {@link BaseTypeVisitor#validateTargetLocation(Tree, AnnotatedTypeMirror, TypeUseLocation)} is
* that this one is only used in {@link BaseTypeVisitor#visitVariable(VariableTree, Void)}
* Validate if the qualifiers on the tree are at the right type-use locations, which is
* specified by the meta-annotation {@link org.checkerframework.framework.qual.TargetLocations}.
*
* @param tree annotations on this VariableTree will be validated
* <p>More specifically, this method only checks qualifiers on the VariableTree and thus checks
* for these following type-use locations: FIELD, LOCAL_VARIABLE, RESOURCE_VARIABLE,
* EXCEPTION_PARAMETER, PARAMETER, RECEIVER and CONSTRUCTOR_RESULT.
*
* <p>The other two validate methods achieve the same goal but perform checks on different trees
* and different type-use locations. See {@link BaseTypeVisitor#validateTargetLocation(Tree,
* AnnotatedTypeMirror, TypeUseLocation)} and
* BaseTypeValidator#validateWildCardTargetLocation(AnnotatedWildcardType, Tree).
*
* @param tree qualifiers on this VariableTree will be validated
* @param type the type of the tree
*/
protected void validateVariablesTargetLocation(Tree tree, AnnotatedTypeMirror type) {
protected void validateVariablesTargetLocation(VariableTree tree, AnnotatedTypeMirror type) {
if (ignoreTargetLocations) {
return;
}
Element element = TreeUtils.elementFromTree(tree);
Element element = TreeUtils.elementFromDeclaration(tree);

if (element != null) {
ElementKind elemKind = element.getKind();
// TypeUseLocation.java doesn't have ENUM type use location right now.
for (AnnotationMirror am : type.getAnnotations()) {
List<TypeUseLocation> locations =
qualAllowedLocations.get(AnnotationUtils.annotationName(am));
if (locations == null || locations.contains(TypeUseLocation.ALL)) {
continue;
}
boolean issueError = true;
switch (elemKind) {
case LOCAL_VARIABLE:
if (locations.contains(TypeUseLocation.LOCAL_VARIABLE)) issueError = false;
break;
case EXCEPTION_PARAMETER:
if (locations.contains(TypeUseLocation.EXCEPTION_PARAMETER))
issueError = false;
break;
case PARAMETER:
if (((VariableTree) tree).getName().contentEquals("this")) {
if (locations.contains(TypeUseLocation.RECEIVER)) {
issueError = false;
}
} else {
if (locations.contains(TypeUseLocation.PARAMETER)) {
issueError = false;
}
}
break;
case RESOURCE_VARIABLE:
if (locations.contains(TypeUseLocation.RESOURCE_VARIABLE)) {
issueError = false;
}
break;
case FIELD:
if (locations.contains(TypeUseLocation.FIELD)) {
ElementKind elemKind = element.getKind();
// TypeUseLocation.java doesn't have ENUM type use location right now.
for (AnnotationMirror am : type.getAnnotations()) {
List<TypeUseLocation> locations =
qualAllowedLocations.get(AnnotationUtils.annotationName(am));
if (locations == null || locations.contains(TypeUseLocation.ALL)) {
continue;
}
boolean issueError = true;
switch (elemKind) {
case LOCAL_VARIABLE:
if (locations.contains(TypeUseLocation.LOCAL_VARIABLE)) {
issueError = false;
}
break;
case EXCEPTION_PARAMETER:
if (locations.contains(TypeUseLocation.EXCEPTION_PARAMETER)) {
issueError = false;
}
break;
case PARAMETER:
if (tree.getName().contentEquals("this")) {
if (locations.contains(TypeUseLocation.RECEIVER)) {
issueError = false;
}
break;
case ENUM_CONSTANT:
if (locations.contains(TypeUseLocation.FIELD)
|| locations.contains(TypeUseLocation.CONSTRUCTOR_RESULT)) {
} else {
if (locations.contains(TypeUseLocation.PARAMETER)) {
issueError = false;
}
break;
default:
throw new BugInCF("Location not matched");
}
if (issueError) {
checker.reportError(
tree,
"type.invalid.annotations.on.location",
am.toString(),
element.getKind().name());
}
}
break;
case RESOURCE_VARIABLE:
if (locations.contains(TypeUseLocation.RESOURCE_VARIABLE)) {
issueError = false;
}
break;
case FIELD:
if (locations.contains(TypeUseLocation.FIELD)) {
issueError = false;
}
break;
case ENUM_CONSTANT:
if (locations.contains(TypeUseLocation.FIELD)
|| locations.contains(TypeUseLocation.CONSTRUCTOR_RESULT)) {
issueError = false;
}
break;
default:
throw new BugInCF("Location not matched");
}
if (issueError) {
checker.reportError(
tree,
"type.invalid.annotations.on.location",
am.toString(),
element.getKind().name());
}
}
}

/**
* Validate if the annotations on the tree are at the right locations, which are specified by
* the meta-annotation @TargetLocations.
* Validate if the qualifiers on the tree are at the right type-use locations, which is
* specified by the meta-annotation {@link org.checkerframework.framework.qual.TargetLocations}.
*
* <p>More specifically, this method only checks qualifiers on the TypeParameter and Method tree
* and thus checks for these following type-use locations: LOWER_BOUND, UPPER_BOUND,
* CONSTRUCTOR_RESULT and RETURN.
*
* <p>The other two validate methods achieve the same goal but perform checks on different trees
* and different type-use locations. See {@link
* BaseTypeVisitor#validateVariablesTargetLocation(VariableTree, AnnotatedTypeMirror)} and
* BaseTypeValidator#validateWildCardTargetLocation(AnnotatedWildcardType, Tree).
*
* @param tree annotations on this VariableTree will be validated
* @param tree qualifiers on the tree will be validated
* @param type the type of the tree
* @param required if all of the TypeUseLocations in {@code required} are not present in the
* specification of the annotation (@TargetLocations), issue an error.
* @param required if all the TypeUseLocations in {@code required} are not present in the
* specification of the meta-annotation ({@link
* org.checkerframework.framework.qual.TargetLocations}), issue an error.
*/
protected void validateTargetLocation(
Tree tree, AnnotatedTypeMirror type, TypeUseLocation required) {
Expand Down
Loading