From 632493fc556a92988113aefb3a08090f56303271 Mon Sep 17 00:00:00 2001 From: Haifeng Shi Date: Tue, 3 Oct 2023 12:48:46 -0400 Subject: [PATCH 1/8] improve comments and readability --- .../common/basetype/BaseTypeVisitor.java | 38 +++++++++++-------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java index 21f5bbaf089..a197b3dc4de 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -1670,15 +1670,20 @@ && 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)} - * - * @param tree annotations on this VariableTree will be validated + * Validate if the qualifiers 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)}. The three + * different validate methods in the visitor and validator file achieve the same goal and the + * need for having three is due to implementation reasons. We perform checks for types in + * VariableTree at {@link BaseTypeVisitor#visitVariable(VariableTree, Void)}; checks for types + * in wildcards at {@link BaseTypeValidator#visitWildcard(AnnotatedWildcardType, Tree)}; the + * rest done in the {@link BaseTypeVisitor#validateTypeOf(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; } @@ -1696,14 +1701,17 @@ protected void validateVariablesTargetLocation(Tree tree, AnnotatedTypeMirror ty boolean issueError = true; switch (elemKind) { case LOCAL_VARIABLE: - if (locations.contains(TypeUseLocation.LOCAL_VARIABLE)) issueError = false; + if (locations.contains(TypeUseLocation.LOCAL_VARIABLE)) { + issueError = false; + } break; case EXCEPTION_PARAMETER: - if (locations.contains(TypeUseLocation.EXCEPTION_PARAMETER)) + if (locations.contains(TypeUseLocation.EXCEPTION_PARAMETER)) { issueError = false; + } break; case PARAMETER: - if (((VariableTree) tree).getName().contentEquals("this")) { + if (tree.getName().contentEquals("this")) { if (locations.contains(TypeUseLocation.RECEIVER)) { issueError = false; } @@ -1744,13 +1752,13 @@ protected void validateVariablesTargetLocation(Tree tree, AnnotatedTypeMirror ty } /** - * 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 locations, which are specified by the + * meta-annotation @TargetLocations. * - * @param tree annotations on this VariableTree will be validated + * @param tree qualifiers on this VariableTree 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 (@TargetLocations), issue an error. */ protected void validateTargetLocation( Tree tree, AnnotatedTypeMirror type, TypeUseLocation required) { From cc14e536b35d6d2398cb85e5c0d02909e73dc5ce Mon Sep 17 00:00:00 2001 From: Haifeng Shi Date: Tue, 3 Oct 2023 15:56:02 -0400 Subject: [PATCH 2/8] improve comments and readability --- .../common/basetype/BaseTypeVisitor.java | 110 +++++++++--------- 1 file changed, 54 insertions(+), 56 deletions(-) diff --git a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java index a197b3dc4de..6645bd1cf19 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -1687,66 +1687,64 @@ protected void validateVariablesTargetLocation(VariableTree tree, AnnotatedTypeM if (ignoreTargetLocations) { return; } - Element element = TreeUtils.elementFromTree(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 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; - } - } 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)) { + Element element = TreeUtils.elementFromDeclaration(tree); + + ElementKind elemKind = element.getKind(); + // TypeUseLocation.java doesn't have ENUM type use location right now. + for (AnnotationMirror am : type.getAnnotations()) { + List 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()); } } } From 58b3f05b6f613cc9c60426413b51b4753ed2be24 Mon Sep 17 00:00:00 2001 From: Haifeng Shi Date: Mon, 23 Oct 2023 15:28:41 -0400 Subject: [PATCH 3/8] Apply suggestions from code review Co-authored-by: Werner Dietl --- .../org/checkerframework/common/basetype/BaseTypeVisitor.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java index 6645bd1cf19..aac277dcd23 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -1671,14 +1671,14 @@ && getCurrentPath().getParentPath().getLeaf().getKind() /** * Validate if the qualifiers on the VariableTree are at the right locations, which is specified - * by the meta-annotation @TargetLocations. The difference of this method between {@link + * by the meta-annotation @TargetLocations. The difference between this method and {@link * BaseTypeVisitor#validateTargetLocation(Tree, AnnotatedTypeMirror, TypeUseLocation)} is that * this one is only used in {@link BaseTypeVisitor#visitVariable(VariableTree, Void)}. The three * different validate methods in the visitor and validator file achieve the same goal and the * need for having three is due to implementation reasons. We perform checks for types in * VariableTree at {@link BaseTypeVisitor#visitVariable(VariableTree, Void)}; checks for types * in wildcards at {@link BaseTypeValidator#visitWildcard(AnnotatedWildcardType, Tree)}; the - * rest done in the {@link BaseTypeVisitor#validateTypeOf(Tree)}. + * rest is done in the {@link BaseTypeVisitor#validateTypeOf(Tree)}. * * @param tree qualifiers on this VariableTree will be validated * @param type the type of the tree From 633f95e935d214525655e2289b45d8842aa6c92f Mon Sep 17 00:00:00 2001 From: Haifeng Shi Date: Mon, 23 Oct 2023 16:54:15 -0400 Subject: [PATCH 4/8] clarify the implementation details of having three validating methods --- .../common/basetype/BaseTypeValidator.java | 14 +++++-- .../common/basetype/BaseTypeVisitor.java | 38 ++++++++++++------- 2 files changed, 36 insertions(+), 16 deletions(-) diff --git a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeValidator.java b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeValidator.java index 825bfb07400..f386f0941dd 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeValidator.java +++ b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeValidator.java @@ -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}. + * + *

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. + * + *

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 diff --git a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java index aac277dcd23..c6417d84c85 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -1670,15 +1670,17 @@ && getCurrentPath().getParentPath().getLeaf().getKind() } /** - * Validate if the qualifiers on the VariableTree are at the right locations, which is specified - * by the meta-annotation @TargetLocations. The difference between this method and {@link - * BaseTypeVisitor#validateTargetLocation(Tree, AnnotatedTypeMirror, TypeUseLocation)} is that - * this one is only used in {@link BaseTypeVisitor#visitVariable(VariableTree, Void)}. The three - * different validate methods in the visitor and validator file achieve the same goal and the - * need for having three is due to implementation reasons. We perform checks for types in - * VariableTree at {@link BaseTypeVisitor#visitVariable(VariableTree, Void)}; checks for types - * in wildcards at {@link BaseTypeValidator#visitWildcard(AnnotatedWildcardType, Tree)}; the - * rest is done in the {@link BaseTypeVisitor#validateTypeOf(Tree)}. + * 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}. + * + *

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. + * + *

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 {@link + * BaseTypeValidator#validateWildCardTargetLocation(AnnotatedWildcardType, Tree)}. * * @param tree qualifiers on this VariableTree will be validated * @param type the type of the tree @@ -1750,13 +1752,23 @@ protected void validateVariablesTargetLocation(VariableTree tree, AnnotatedTypeM } /** - * Validate if the qualifiers 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}. * - * @param tree qualifiers on this VariableTree will be validated + *

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. + * + *

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 BaseTypeValidator#validateWildCardTargetLocation(AnnotatedWildcardType, Tree)}. + * + * @param tree qualifiers on the tree will be validated * @param type the type of the tree * @param required if all the TypeUseLocations in {@code required} are not present in the - * specification of the meta-annotation (@TargetLocations), issue an error. + * specification of the meta-annotation ({@link + * org.checkerframework.framework.qual.TargetLocations}), issue an error. */ protected void validateTargetLocation( Tree tree, AnnotatedTypeMirror type, TypeUseLocation required) { From b145a93d140e1ece5e12c8efe8283b51451ec67b Mon Sep 17 00:00:00 2001 From: Aosen Xiong Date: Sat, 14 Dec 2024 16:15:06 -0800 Subject: [PATCH 5/8] Use fully qualified name to see if fix the javadoc error --- .../checkerframework/common/basetype/BaseTypeVisitor.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java index c6417d84c85..87cc86a7646 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -1680,7 +1680,8 @@ && getCurrentPath().getParentPath().getLeaf().getKind() *

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 {@link - * BaseTypeValidator#validateWildCardTargetLocation(AnnotatedWildcardType, Tree)}. + * org.checkerframework.common.basetype.BaseTypeValidator#validateWildCardTargetLocation(AnnotatedWildcardType, + * Tree)}. * * @param tree qualifiers on this VariableTree will be validated * @param type the type of the tree @@ -1762,7 +1763,9 @@ protected void validateVariablesTargetLocation(VariableTree tree, AnnotatedTypeM *

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 BaseTypeValidator#validateWildCardTargetLocation(AnnotatedWildcardType, Tree)}. + * {@link + * org.checkerframework.common.basetype.BaseTypeValidator#validateWildCardTargetLocation(AnnotatedWildcardType, + * Tree)}. * * @param tree qualifiers on the tree will be validated * @param type the type of the tree From 876a91e8e5d3ea15fb286658e868f4e42959d747 Mon Sep 17 00:00:00 2001 From: Aosen Xiong Date: Mon, 16 Dec 2024 20:47:47 -0500 Subject: [PATCH 6/8] Try with linkplain --- .../common/basetype/BaseTypeVisitor.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java index 87cc86a7646..5655e399ff0 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -1679,9 +1679,8 @@ && getCurrentPath().getParentPath().getLeaf().getKind() * *

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 {@link - * org.checkerframework.common.basetype.BaseTypeValidator#validateWildCardTargetLocation(AnnotatedWildcardType, - * Tree)}. + * AnnotatedTypeMirror, TypeUseLocation)} and {@linkplain + * BaseTypeValidator#validateWildCardTargetLocation(AnnotatedWildcardType, Tree)}. * * @param tree qualifiers on this VariableTree will be validated * @param type the type of the tree @@ -1763,9 +1762,7 @@ protected void validateVariablesTargetLocation(VariableTree tree, AnnotatedTypeM *

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 - * org.checkerframework.common.basetype.BaseTypeValidator#validateWildCardTargetLocation(AnnotatedWildcardType, - * Tree)}. + * {@linkplain BaseTypeValidator#validateWildCardTargetLocation(AnnotatedWildcardType, Tree)}. * * @param tree qualifiers on the tree will be validated * @param type the type of the tree From 7368004e5882806c473e120bf16b566ca1364e0c Mon Sep 17 00:00:00 2001 From: Aosen Xiong Date: Tue, 17 Dec 2024 07:49:23 -0800 Subject: [PATCH 7/8] Don't use any link --- .../checkerframework/common/basetype/BaseTypeVisitor.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java index 5655e399ff0..f1b24760582 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -1679,8 +1679,8 @@ && getCurrentPath().getParentPath().getLeaf().getKind() * *

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 {@linkplain - * BaseTypeValidator#validateWildCardTargetLocation(AnnotatedWildcardType, Tree)}. + * AnnotatedTypeMirror, TypeUseLocation)} and + * BaseTypeValidator#validateWildCardTargetLocation(AnnotatedWildcardType, Tree). * * @param tree qualifiers on this VariableTree will be validated * @param type the type of the tree @@ -1762,7 +1762,7 @@ protected void validateVariablesTargetLocation(VariableTree tree, AnnotatedTypeM *

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 - * {@linkplain BaseTypeValidator#validateWildCardTargetLocation(AnnotatedWildcardType, Tree)}. + * BaseTypeValidator#validateWildCardTargetLocation(AnnotatedWildcardType, Tree). * * @param tree qualifiers on the tree will be validated * @param type the type of the tree From c165f4c204106531af667252e5d455a230f635cc Mon Sep 17 00:00:00 2001 From: Aosen Xiong Date: Thu, 2 Jan 2025 16:55:11 -0800 Subject: [PATCH 8/8] Update Javadoc link based on previous observation for JDK8 --- .../common/basetype/BaseTypeVisitor.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java index f1b24760582..ed70a52a985 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -1679,8 +1679,9 @@ && getCurrentPath().getParentPath().getLeaf().getKind() * *

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). + * AnnotatedTypeMirror, TypeUseLocation)} and {@link + * BaseTypeValidator#validateWildCardTargetLocation(AnnotatedTypeMirror.AnnotatedWildcardType, + * Tree)}. * * @param tree qualifiers on this VariableTree will be validated * @param type the type of the tree @@ -1762,7 +1763,9 @@ protected void validateVariablesTargetLocation(VariableTree tree, AnnotatedTypeM *

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). + * {@link + * BaseTypeValidator#validateWildCardTargetLocation(AnnotatedTypeMirror.AnnotatedWildcardType, + * Tree)}. * * @param tree qualifiers on the tree will be validated * @param type the type of the tree