From d4217a47c68d89d2178fd49126683831f8a33d7c Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Mon, 16 Oct 2023 16:18:44 +0200 Subject: [PATCH 1/7] typetools/checker-framework 3.38.0 release (#600) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Chris Povirk Co-authored-by: Michael Ernst Co-authored-by: Suzanne Millstein Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Calvin Loncaric Co-authored-by: Michael Ernst Co-authored-by: Martin Kellogg Co-authored-by: Manu Sridharan Co-authored-by: Nicholas Breen Co-authored-by: Narges Shadab <54193416+Nargeshdb@users.noreply.github.com> Co-authored-by: Chris Povirk --- .editorconfig | 4 +- azure-pipelines.yml | 3 + build.gradle | 5 +- .../qual/EnsuresCalledMethods.java | 13 +- .../signedness/qual/SignedPositive.java | 10 +- .../qual/SignedPositiveFromUnsigned.java | 24 - .../signedness/qual/SignednessBottom.java | 2 +- .../signedness/qual/SignednessGlb.java | 3 - ...suppressions => count-suppression-reasons} | 12 +- checker/bin/query-github.sh | 4 +- .../builder/BuilderFrameworkSupport.java | 4 +- .../checker/index/IndexUtil.java | 13 +- .../checker/interning/InterningVisitor.java | 2 +- .../MustCallConsistencyAnalyzer.java | 23 +- .../SignednessAnnotatedTypeFactory.java | 419 +++--------------- .../checker/signedness/SignednessShifts.java | 304 +++++++++++++ .../checker/signedness/SignednessVisitor.java | 14 +- .../TaintingAnnotatedTypeFactory.java | 38 ++ checker/tests/nullness/Issue2564.java | 4 +- checker/tests/nullness/Issue2587.java | 5 +- .../AccumulationValueFieldTest.java | 40 ++ checker/tests/tainting/Issue6110.java | 23 + checker/tests/tainting/Issue6116.java | 14 + .../dataflow/cfg/builder/ExtendedNode.java | 3 + .../dataflow/cfg/builder/Label.java | 3 + docs/CHANGELOG.md | 19 +- docs/checker-framework-webpage.html | 8 +- docs/developer/developer-manual.html | 64 ++- docs/examples/MavenExample/pom.xml | 66 ++- docs/manual/called-methods-checker.tex | 3 +- docs/manual/contributors.tex | 1 + docs/manual/external-tools.tex | 122 ++--- docs/manual/figures/signedness.svg | 107 ++--- docs/manual/manual.tex | 4 +- docs/manual/signedness-checker.tex | 9 +- framework-test/build.gradle | 2 + .../CheckerFrameworkPerDirectoryTest.java | 20 +- .../test/CheckerFrameworkPerFileTest.java | 20 +- .../test/CheckerFrameworkRootedTest.java | 33 ++ .../framework/test/PerDirectorySuite.java | 6 +- .../framework/test/PerFileSuite.java | 31 +- .../framework/test/RootedSuite.java | 40 ++ .../framework/test/TestRootDirectory.java | 20 + .../framework/test/TestUtilities.java | 20 + .../junit/AlternateTestRootPerDirTest.java | 64 +++ .../AlternateTestRootPerFileWithDirsTest.java | 63 +++ ...AlternateTestRootPerFileWithFilesTest.java | 60 +++ framework-test/tests-alt/README | 4 + .../tests-alt/alt-dir-a/Issue6125A.java | 6 + .../tests-alt/alt-dir-b/Issue6125B.java | 6 + .../common/basetype/BaseTypeChecker.java | 6 +- .../common/basetype/BaseTypeVisitor.java | 8 +- .../common/basetype/messages.properties | 1 + .../common/reflection/ReflectionResolver.java | 2 +- .../util/count/AnnotationStatistics.java | 10 +- .../common/value/ValueMethodIdentifier.java | 20 +- .../common/value/util/Range.java | 14 +- ...holeProgramInferenceJavaParserStorage.java | 17 +- .../framework/source/SourceChecker.java | 20 +- .../framework/type/AnnotatedTypeFactory.java | 49 +- .../framework/type/AnnotatedTypeMirror.java | 7 +- .../framework/type/DefaultTypeHierarchy.java | 6 +- .../framework/type/SupertypeFinder.java | 7 +- .../framework/util/AnnotatedTypes.java | 54 +-- .../util/DefaultQualifierKindHierarchy.java | 3 +- .../framework/util/Heuristics.java | 2 +- framework/tests/all-systems/Issue3791.java | 3 +- framework/tests/value/DoubleRounding.java | 11 + .../checkerframework/javacutil/TreeUtils.java | 4 +- .../javacutil/TypesUtils.java | 62 ++- .../javacutil/trees/TreeBuilder.java | 7 +- 71 files changed, 1329 insertions(+), 771 deletions(-) delete mode 100644 checker-qual/src/main/java/org/checkerframework/checker/signedness/qual/SignedPositiveFromUnsigned.java rename checker/bin-devel/{count-suppressions => count-suppression-reasons} (91%) create mode 100644 checker/src/main/java/org/checkerframework/checker/signedness/SignednessShifts.java create mode 100644 checker/src/main/java/org/checkerframework/checker/tainting/TaintingAnnotatedTypeFactory.java create mode 100644 checker/tests/resourceleak/AccumulationValueFieldTest.java create mode 100644 checker/tests/tainting/Issue6110.java create mode 100644 checker/tests/tainting/Issue6116.java create mode 100644 framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkRootedTest.java create mode 100644 framework-test/src/main/java/org/checkerframework/framework/test/RootedSuite.java create mode 100644 framework-test/src/main/java/org/checkerframework/framework/test/TestRootDirectory.java create mode 100644 framework-test/src/test/java/org/checkerframework/framework/test/test/junit/AlternateTestRootPerDirTest.java create mode 100644 framework-test/src/test/java/org/checkerframework/framework/test/test/junit/AlternateTestRootPerFileWithDirsTest.java create mode 100644 framework-test/src/test/java/org/checkerframework/framework/test/test/junit/AlternateTestRootPerFileWithFilesTest.java create mode 100644 framework-test/tests-alt/README create mode 100644 framework-test/tests-alt/alt-dir-a/Issue6125A.java create mode 100644 framework-test/tests-alt/alt-dir-b/Issue6125B.java create mode 100644 framework/tests/value/DoubleRounding.java diff --git a/.editorconfig b/.editorconfig index 2207b5df3cd..7402f5bef1f 100644 --- a/.editorconfig +++ b/.editorconfig @@ -32,11 +32,13 @@ ij_any_spaces_around_logical_operators = true ij_any_spaces_around_multiplicative_operators = true ij_any_spaces_around_relational_operators = true ij_any_spaces_around_shift_operators = true +ij_java_align_multiline_parameters = false ij_java_if_brace_force = always ij_java_indent_case_from_switch = true +ij_java_names_count_to_use_import_on_demand = 9999 +ij_java_class_count_to_use_import_on_demand = 9999 ij_java_space_after_colon = true ij_java_space_before_colon = true ij_java_ternary_operation_signs_on_next_line = true ij_java_use_single_class_imports = true ij_java_wrap_long_lines = true -ij_java_align_multiline_parameters = false diff --git a/azure-pipelines.yml b/azure-pipelines.yml index e03f90947ab..a30b2c61f72 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -342,6 +342,7 @@ jobs: container: wmdietl/cf-ubuntu-jdk11-plus:latest steps: - checkout: self + fetchDepth: 25 - bash: ./checker/bin-devel/test-misc.sh displayName: test-misc.sh - job: misc_jdk17 @@ -350,6 +351,7 @@ jobs: container: wmdietl/cf-ubuntu-jdk17-plus:latest steps: - checkout: self + fetchDepth: 25 - bash: ./checker/bin-devel/test-misc.sh displayName: test-misc.sh - job: misc_jdk_latest @@ -361,6 +363,7 @@ jobs: container: wmdietl/cf-ubuntu-jdk-latest-plus:latest steps: - checkout: self + fetchDepth: 25 - bash: ./checker/bin-devel/test-misc.sh displayName: test-misc.sh - job: misc_jdk_next diff --git a/build.gradle b/build.gradle index a388e87f1e7..d689f27da86 100644 --- a/build.gradle +++ b/build.gradle @@ -70,7 +70,7 @@ ext { // afu = "${annotationTools}/annotation-file-utilities" stubparser = "${parentDir}/stubparser" - stubparserVersion = "3.25.3" + stubparserVersion = "3.25.5" stubparserJar = "${stubparser}/javaparser-core/target/stubparser-${stubparserVersion}.jar" jtregHome = "${parentDir}/jtreg" @@ -163,7 +163,7 @@ allprojects { // * any new checkers have been added, or // * backward-incompatible changes have been made to APIs or elsewhere. // To make a snapshot release: ./gradlew publish - version '3.37.0' + version '3.38.0' tasks.withType(JavaCompile).configureEach { options.fork = true @@ -252,6 +252,7 @@ allprojects { // If you add any formatters to this block that require dependencies, then you must also // add them to spotlessPredeclare block. def doNotFormat = [ + 'checker/bin-devel/.plume-scripts/**', 'checker/tests/ainfer-*/annotated/*', 'dataflow/manual/examples/', '**/nullness-javac-errors/*', diff --git a/checker-qual/src/main/java/org/checkerframework/checker/calledmethods/qual/EnsuresCalledMethods.java b/checker-qual/src/main/java/org/checkerframework/checker/calledmethods/qual/EnsuresCalledMethods.java index 95a88a8c06f..6025b3dee54 100644 --- a/checker-qual/src/main/java/org/checkerframework/checker/calledmethods/qual/EnsuresCalledMethods.java +++ b/checker-qual/src/main/java/org/checkerframework/checker/calledmethods/qual/EnsuresCalledMethods.java @@ -13,7 +13,10 @@ /** * Indicates that the method, if it terminates successfully, always invokes the given methods on the - * given expressions. This annotation is repeatable. + * given expressions. This annotation is repeatable, which means that users can write more than one + * instance of it on the same method (users should NOT manually write an + * {@code @EnsuresCalledMethods.List} annotation, which the checker will create from multiple copies + * of this annotation automatically). * *

Consider the following method: * @@ -61,10 +64,10 @@ String[] methods(); /** - * A wrapper annotation that makes the {@link EnsuresCalledMethods} annotation repeatable. - * - *

Programmers generally do not need to write this. It is created by Java when a programmer - * writes more than one {@link EnsuresCalledMethods} annotation at the same location. + * A wrapper annotation that makes the {@link EnsuresCalledMethods} annotation repeatable. This + * annotation is an implementation detail: programmers generally do not need to write this. It + * is created automatically by Java when a programmer writes more than one {@link + * EnsuresCalledMethods} annotation at the same location. */ @Documented @Retention(RetentionPolicy.RUNTIME) diff --git a/checker-qual/src/main/java/org/checkerframework/checker/signedness/qual/SignedPositive.java b/checker-qual/src/main/java/org/checkerframework/checker/signedness/qual/SignedPositive.java index f91e659378d..52416f01445 100644 --- a/checker-qual/src/main/java/org/checkerframework/checker/signedness/qual/SignedPositive.java +++ b/checker-qual/src/main/java/org/checkerframework/checker/signedness/qual/SignedPositive.java @@ -1,5 +1,7 @@ package org.checkerframework.checker.signedness.qual; +import org.checkerframework.framework.qual.SubtypeOf; + import java.lang.annotation.Documented; import java.lang.annotation.ElementType; import java.lang.annotation.Retention; @@ -7,17 +9,14 @@ import java.lang.annotation.Target; /** - * The expression's value is in the signed positive range; that is, its most significant bit is not - * set. The value has the same interpretation as {@code @}{@link Signed} and {@code @}{@link + * The expression's value is in the signed positive range; that is, its most significant bit is + * zero. The value has the same interpretation as {@code @}{@link Signed} and {@code @}{@link * Unsigned} — both interpretations are equivalent. * *

Programmers should rarely write {@code @SignedPositive}. Instead, the programmer should write * {@code @}{@link Signed} or {@code @}{@link Unsigned} to indicate how the programmer intends the * value to be interpreted. * - *

Internally, this is translated to the {@code @}{@link SignednessGlb} annotation. This means - * that programmers do not see this annotation in error messages. - * *

{@code @SignedPositive} corresponds to {@code @}{@link * org.checkerframework.checker.index.qual.NonNegative NonNegative} in the Index Checker's type * system. @@ -28,4 +27,5 @@ @Documented @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.TYPE_USE, ElementType.TYPE_PARAMETER}) +@SubtypeOf(SignednessGlb.class) public @interface SignedPositive {} diff --git a/checker-qual/src/main/java/org/checkerframework/checker/signedness/qual/SignedPositiveFromUnsigned.java b/checker-qual/src/main/java/org/checkerframework/checker/signedness/qual/SignedPositiveFromUnsigned.java deleted file mode 100644 index 2aee2fb5f18..00000000000 --- a/checker-qual/src/main/java/org/checkerframework/checker/signedness/qual/SignedPositiveFromUnsigned.java +++ /dev/null @@ -1,24 +0,0 @@ -package org.checkerframework.checker.signedness.qual; - -import org.checkerframework.framework.qual.SubtypeOf; - -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; - -/** - * The expression is {@code @}{@link SignedPositive}, and its value came from widening a value that - * is allowed to be interpreted as unsigned. - * - *

Programmers should rarely write this annotation. - * - * @see SignednessGlb - * @checker_framework.manual #signedness-checker Signedness Checker - */ -@Documented -@Retention(RetentionPolicy.RUNTIME) -@Target({ElementType.TYPE_USE, ElementType.TYPE_PARAMETER}) -@SubtypeOf({SignednessGlb.class}) -public @interface SignedPositiveFromUnsigned {} diff --git a/checker-qual/src/main/java/org/checkerframework/checker/signedness/qual/SignednessBottom.java b/checker-qual/src/main/java/org/checkerframework/checker/signedness/qual/SignednessBottom.java index 33ec7578e0e..2cb269fbf62 100644 --- a/checker-qual/src/main/java/org/checkerframework/checker/signedness/qual/SignednessBottom.java +++ b/checker-qual/src/main/java/org/checkerframework/checker/signedness/qual/SignednessBottom.java @@ -25,5 +25,5 @@ TypeUseLocation.LOWER_BOUND, TypeUseLocation.UPPER_BOUND, }) -@SubtypeOf({SignedPositiveFromUnsigned.class}) +@SubtypeOf({SignedPositive.class}) public @interface SignednessBottom {} diff --git a/checker-qual/src/main/java/org/checkerframework/checker/signedness/qual/SignednessGlb.java b/checker-qual/src/main/java/org/checkerframework/checker/signedness/qual/SignednessGlb.java index 33a33869fb8..36f8e10c7ee 100644 --- a/checker-qual/src/main/java/org/checkerframework/checker/signedness/qual/SignednessGlb.java +++ b/checker-qual/src/main/java/org/checkerframework/checker/signedness/qual/SignednessGlb.java @@ -1,7 +1,5 @@ package org.checkerframework.checker.signedness.qual; -import org.checkerframework.framework.qual.LiteralKind; -import org.checkerframework.framework.qual.QualifierForLiterals; import org.checkerframework.framework.qual.SubtypeOf; import java.lang.annotation.Documented; @@ -39,5 +37,4 @@ @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.TYPE_USE, ElementType.TYPE_PARAMETER}) @SubtypeOf({Unsigned.class, Signed.class}) -@QualifierForLiterals({LiteralKind.INT, LiteralKind.LONG, LiteralKind.CHAR}) public @interface SignednessGlb {} diff --git a/checker/bin-devel/count-suppressions b/checker/bin-devel/count-suppression-reasons similarity index 91% rename from checker/bin-devel/count-suppressions rename to checker/bin-devel/count-suppression-reasons index 1ca1e143740..f4326b591a9 100755 --- a/checker/bin-devel/count-suppressions +++ b/checker/bin-devel/count-suppression-reasons @@ -3,7 +3,7 @@ # This command counts the approximate frequency of each distinct reason for # warning suppressions, in all files under the current directory. # To invoke it, pass a type system name; for example: -# count-suppressions nullness +# count-suppression-reasons nullness # The argument to this script is actually a regular expression. # The "reason" for a warning suppression is the Java line comment after it: @@ -62,8 +62,8 @@ fi # Diagnostics # echo "regex=${regex}" -greplines=$(mktemp /tmp/count-suppressions."$(date +%Y%m%d-%H%M%S)"-XXX) -countedreasons=$(mktemp /tmp/count-suppressions."$(date +%Y%m%d-%H%M%S)"-XXX) +greplines=$(mktemp /tmp/count-suppression-reasons."$(date +%Y%m%d-%H%M%S)"-XXX) +countedreasons=$(mktemp /tmp/count-suppression-reasons."$(date +%Y%m%d-%H%M%S)"-XXX) # These are the two types of matching lines: # * "checkername" or "chekername:..." @@ -71,10 +71,10 @@ countedreasons=$(mktemp /tmp/count-suppressions."$(date +%Y%m%d-%H%M%S)"-XXX) # include "@SuppressWarnings" because it might appear on the previous line. # * @AssumeAssertion(checkername) # This grep command captures a few stray lines; users should ignore them. -# This grep command assumes that tests are not annotated, and it hard-codes ignoring "annotated-jdk", "jdk", "true positive", "// TP" (as an alias for "true positive"), and "count-suppressions-ignore". +# This grep command assumes that tests are not annotated, and it hard-codes ignoring "annotated-jdk", "jdk", "true positive", "// TP" (as an alias for "true positive"), and "count-suppression-reasons-ignore". ${GREP} -n --recursive --include='*.java' "\"${regex}[:\"]\(.*[^;]\)\?\(\$\|//\)\|@AssumeAssertion(${regex})" \ | grep -v "@AnnotatedFor" | grep -v "/tests/" \ - | grep -v "/annotated-jdk/" | grep -v "/jdk/" | grep -v "^jdk/" | grep -v "true positive" | grep -v "// TP" | grep -v "count-suppressions-ignore" > "${greplines}" + | grep -v "/annotated-jdk/" | grep -v "/jdk/" | grep -v "^jdk/" | grep -v "true positive" | grep -v "// TP" | grep -v "count-suppression-reasons-ignore" > "${greplines}" total=$(wc -l < "${greplines}") ## Don't output a total, to avoid people using this approximate count. @@ -87,7 +87,7 @@ cat "${greplines}" \ | ${SED} 's/ \+$//' \ | sort | uniq -c | sort -rg > "${countedreasons}" -# Add leading percentages to `uniq -c` output. Note that it rounds down to the nearest integer. +# Add leading percentages to `uniq -c` output. Note that it rounds *down* to the nearest integer. # (Digits afert the decimal don't make a practical difference.) while read -r line; do count=$(echo "$line" | cut -f1 -d " "); diff --git a/checker/bin/query-github.sh b/checker/bin/query-github.sh index 49d5d5906b2..752d789a508 100755 --- a/checker/bin/query-github.sh +++ b/checker/bin/query-github.sh @@ -11,8 +11,8 @@ # $1 is the query file, which should contain the literal string to use # as the github search. REQUIRED, no default. # -# $2 is the number of GitHub search pages. Default 1. Each page contains 10 results. GitHub only returns -# the first 1000 results, so 100 is the maximum useful number of search pages. +# $2 is the number of GitHub search pages. Default 1. Each page contains 10 results. GitHub only +# returns the first 1000 results, so 100 is the maximum useful number of search pages. # # This script outputs a list of projects. The underlying GitHub search is for code snippets, and this # script eliminates duplicates (i.e. different code snippets from the same project are combined into diff --git a/checker/src/main/java/org/checkerframework/checker/calledmethods/builder/BuilderFrameworkSupport.java b/checker/src/main/java/org/checkerframework/checker/calledmethods/builder/BuilderFrameworkSupport.java index 555677ed745..40aa4a06202 100644 --- a/checker/src/main/java/org/checkerframework/checker/calledmethods/builder/BuilderFrameworkSupport.java +++ b/checker/src/main/java/org/checkerframework/checker/calledmethods/builder/BuilderFrameworkSupport.java @@ -20,7 +20,7 @@ public interface BuilderFrameworkSupport { /** - * Determines if a method is a {@code toBuilder} method on a type generated by the builder + * Returns true if a method is a {@code toBuilder} method on a type generated by the builder * framework. * * @param candidateToBuilderElement a method @@ -42,7 +42,7 @@ public interface BuilderFrameworkSupport { void handleToBuilderMethod(AnnotatedExecutableType toBuilderType); /** - * Determines if a method is a {@code build} method on a {@code Builder} type for the builder + * Returns true if a method is a {@code build} method on a {@code Builder} type for the builder * framework. * * @param candidateBuildElement a method diff --git a/checker/src/main/java/org/checkerframework/checker/index/IndexUtil.java b/checker/src/main/java/org/checkerframework/checker/index/IndexUtil.java index 80c7c7370a6..9f86692a4e4 100644 --- a/checker/src/main/java/org/checkerframework/checker/index/IndexUtil.java +++ b/checker/src/main/java/org/checkerframework/checker/index/IndexUtil.java @@ -15,7 +15,18 @@ /** A collection of utility functions used by several Index Checker subcheckers. */ public class IndexUtil { - /** Determines whether the type is a sequence supported by this checker. */ + + /** Do not instantiate IndexUtil. */ + private IndexUtil() { + throw new Error("Do not instantiate IndexUtil."); + } + + /** + * Returns true if the type is a sequence supported by this checker. + * + * @param type a type + * @return true if the type is a sequence supported by this checker + */ public static boolean isSequenceType(TypeMirror type) { return type.getKind() == TypeKind.ARRAY || TypesUtils.isString(type); } diff --git a/checker/src/main/java/org/checkerframework/checker/interning/InterningVisitor.java b/checker/src/main/java/org/checkerframework/checker/interning/InterningVisitor.java index 0982bcb6140..dc72963e72c 100644 --- a/checker/src/main/java/org/checkerframework/checker/interning/InterningVisitor.java +++ b/checker/src/main/java/org/checkerframework/checker/interning/InterningVisitor.java @@ -937,7 +937,7 @@ private boolean classIsAnnotated(AnnotatedTypeMirror type) { } /** - * Determines whether or not the given element overrides the named method in the named class. + * Returns true if the given element overrides the named method in the named class. * * @param e an element for a method * @param clazz the class diff --git a/checker/src/main/java/org/checkerframework/checker/resourceleak/MustCallConsistencyAnalyzer.java b/checker/src/main/java/org/checkerframework/checker/resourceleak/MustCallConsistencyAnalyzer.java index c53290aa00b..52557668104 100644 --- a/checker/src/main/java/org/checkerframework/checker/resourceleak/MustCallConsistencyAnalyzer.java +++ b/checker/src/main/java/org/checkerframework/checker/resourceleak/MustCallConsistencyAnalyzer.java @@ -863,9 +863,9 @@ private void updateObligationsWithInvocationResult(Set obligations, } /** - * Determines if the result of the given method or constructor invocation node should be tracked - * in {@code obligations}. In some cases, there is no need to track the result because the - * must-call obligations are already satisfied in some other way or there cannot possibly be + * Returns true if the result of the given method or constructor invocation node should be + * tracked in {@code obligations}. In some cases, there is no need to track the result because + * the must-call obligations are already satisfied in some other way or there cannot possibly be * must-call obligations because of the structure of the code. * *

Specifically, an invocation result does NOT need to be tracked if any of the following is @@ -1505,12 +1505,17 @@ && varTrackedInObligations(obligations, (LocalVariableNode) receiver)) AccumulationStore cmStoreBefore = typeFactory.getStoreBefore(rhs); AccumulationValue cmValue = cmStoreBefore == null ? null : cmStoreBefore.getValue(lhs); AnnotationMirror cmAnno = null; - if (cmValue != null) { - for (AnnotationMirror anno : cmValue.getAnnotations()) { - if (AnnotationUtils.areSameByName( - anno, "org.checkerframework.checker.calledmethods.qual.CalledMethods")) { - cmAnno = anno; - break; + if (cmValue != null) { // When store contains the lhs + Set accumulatedValues = cmValue.getAccumulatedValues(); + if (accumulatedValues != null) { // type variable or wildcard type + cmAnno = typeFactory.createCalledMethods(accumulatedValues.toArray(new String[0])); + } else { + for (AnnotationMirror anno : cmValue.getAnnotations()) { + if (AnnotationUtils.areSameByName( + anno, + "org.checkerframework.checker.calledmethods.qual.CalledMethods")) { + cmAnno = anno; + } } } } diff --git a/checker/src/main/java/org/checkerframework/checker/signedness/SignednessAnnotatedTypeFactory.java b/checker/src/main/java/org/checkerframework/checker/signedness/SignednessAnnotatedTypeFactory.java index 5245de90dd0..71f29f5183d 100644 --- a/checker/src/main/java/org/checkerframework/checker/signedness/SignednessAnnotatedTypeFactory.java +++ b/checker/src/main/java/org/checkerframework/checker/signedness/SignednessAnnotatedTypeFactory.java @@ -1,21 +1,17 @@ package org.checkerframework.checker.signedness; -import com.sun.source.tree.AnnotatedTypeTree; import com.sun.source.tree.BinaryTree; import com.sun.source.tree.CompoundAssignmentTree; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.LiteralTree; -import com.sun.source.tree.PrimitiveTypeTree; import com.sun.source.tree.Tree; import com.sun.source.tree.TypeCastTree; import com.sun.source.util.TreePath; -import org.checkerframework.checker.interning.qual.InternedDistinct; import org.checkerframework.checker.nullness.qual.Nullable; import org.checkerframework.checker.signedness.qual.PolySigned; import org.checkerframework.checker.signedness.qual.Signed; import org.checkerframework.checker.signedness.qual.SignedPositive; -import org.checkerframework.checker.signedness.qual.SignedPositiveFromUnsigned; import org.checkerframework.checker.signedness.qual.SignednessBottom; import org.checkerframework.checker.signedness.qual.SignednessGlb; import org.checkerframework.checker.signedness.qual.Unsigned; @@ -40,17 +36,12 @@ import org.checkerframework.javacutil.AnnotationBuilder; import org.checkerframework.javacutil.AnnotationMirrorSet; import org.checkerframework.javacutil.AnnotationUtils; -import org.checkerframework.javacutil.TreePathUtil; import org.checkerframework.javacutil.TreeUtils; import org.checkerframework.javacutil.TypeKindUtils; -import org.checkerframework.javacutil.TypeSystemError; import org.checkerframework.javacutil.TypesUtils; -import org.plumelib.util.IPair; import java.io.Serializable; -import java.lang.annotation.Annotation; import java.util.List; -import java.util.Set; import javax.annotation.processing.ProcessingEnvironment; import javax.lang.model.element.AnnotationMirror; @@ -76,9 +67,9 @@ public class SignednessAnnotatedTypeFactory extends BaseAnnotatedTypeFactory { protected final AnnotationMirror SIGNEDNESS_GLB = AnnotationBuilder.fromClass(elements, SignednessGlb.class); - /** The @SignedPositiveFromUnsigned annotation. */ - protected final AnnotationMirror SIGNED_POSITIVE_FROM_UNSIGNED = - AnnotationBuilder.fromClass(elements, SignedPositiveFromUnsigned.class); + /** The @SignedPositive annotation. */ + protected final AnnotationMirror SIGNED_POSITIVE = + AnnotationBuilder.fromClass(elements, SignedPositive.class); /** The @SignednessBottom annotation. */ protected final AnnotationMirror SIGNEDNESS_BOTTOM = @@ -122,88 +113,95 @@ public class SignednessAnnotatedTypeFactory extends BaseAnnotatedTypeFactory { public SignednessAnnotatedTypeFactory(BaseTypeChecker checker) { super(checker); - addAliasedTypeAnnotation(SignedPositive.class, SIGNEDNESS_GLB); - addAliasedTypeAnnotation("jdk.jfr.Unsigned", UNSIGNED); postInit(); } - @Override - protected Set> createSupportedTypeQualifiers() { - Set> result = getBundledTypeQualifiers(); - result.remove(SignedPositive.class); // this method should not return aliases - return result; - } - @Override protected void addComputedTypeAnnotations( Tree tree, AnnotatedTypeMirror type, boolean iUseFlow) { - if (!isComputingAnnotatedTypeMirrorOfLhs()) { - addSignednessGlbAnnotation(tree, type); + Tree.Kind treeKind = tree.getKind(); + if (treeKind == Tree.Kind.INT_LITERAL) { + int literalValue = (int) ((LiteralTree) tree).getValue(); + if (literalValue >= 0) { + type.replaceAnnotation(SIGNED_POSITIVE); + } else { + type.replaceAnnotation(SIGNEDNESS_GLB); + } + } else if (treeKind == Tree.Kind.LONG_LITERAL) { + long literalValue = (long) ((LiteralTree) tree).getValue(); + if (literalValue >= 0) { + type.replaceAnnotation(SIGNED_POSITIVE); + } else { + type.replaceAnnotation(SIGNEDNESS_GLB); + } + } else if (!isComputingAnnotatedTypeMirrorOfLhs()) { + addSignedPositiveAnnotation(tree, type); } super.addComputedTypeAnnotations(tree, type, iUseFlow); } /** - * Refines an integer expression to @SignednessGlb if its value is within the signed positive + * Refines an integer expression to @SignedPositive if its value is within the signed positive * range (i.e. its MSB is zero). Does not refine the type of cast expressions. * * @param tree an AST node, whose type may be refined * @param type the type of the tree */ - private void addSignednessGlbAnnotation(Tree tree, AnnotatedTypeMirror type) { + private void addSignedPositiveAnnotation(Tree tree, AnnotatedTypeMirror type) { if (tree.getKind() == Tree.Kind.TYPE_CAST) { return; } TypeMirror javaType = type.getUnderlyingType(); TypeKind javaTypeKind = javaType.getKind(); - if (tree.getKind() != Tree.Kind.VARIABLE) { - if (javaTypeKind == TypeKind.BYTE - || javaTypeKind == TypeKind.CHAR - || javaTypeKind == TypeKind.SHORT - || javaTypeKind == TypeKind.INT - || javaTypeKind == TypeKind.LONG) { - ValueAnnotatedTypeFactory valueFactory = - getTypeFactoryOfSubchecker(ValueChecker.class); - AnnotatedTypeMirror valueATM = valueFactory.getAnnotatedType(tree); - // These annotations are trusted rather than checked. Maybe have an option to - // disable using them? - if ((valueATM.hasAnnotation(INT_RANGE_FROM_NON_NEGATIVE) - || valueATM.hasAnnotation(INT_RANGE_FROM_POSITIVE)) - && type.hasAnnotation(SIGNED)) { - type.replaceAnnotation(SIGNEDNESS_GLB); - } else { - Range treeRange = ValueCheckerUtils.getPossibleValues(valueATM, valueFactory); - - if (treeRange != null) { - switch (javaType.getKind()) { - case BYTE: - case CHAR: - if (treeRange.isWithin(0, Byte.MAX_VALUE)) { - type.replaceAnnotation(SIGNEDNESS_GLB); - } - break; - case SHORT: - if (treeRange.isWithin(0, Short.MAX_VALUE)) { - type.replaceAnnotation(SIGNEDNESS_GLB); - } - break; - case INT: - if (treeRange.isWithin(0, Integer.MAX_VALUE)) { - type.replaceAnnotation(SIGNEDNESS_GLB); - } - break; - case LONG: - if (treeRange.isWithin(0, Long.MAX_VALUE)) { - type.replaceAnnotation(SIGNEDNESS_GLB); - } - break; - default: - // Nothing + if (tree.getKind() == Tree.Kind.VARIABLE) { + return; + } + if (!(javaTypeKind == TypeKind.BYTE + || javaTypeKind == TypeKind.CHAR + || javaTypeKind == TypeKind.SHORT + || javaTypeKind == TypeKind.INT + || javaTypeKind == TypeKind.LONG)) { + return; + } + ValueAnnotatedTypeFactory valueFactory = getTypeFactoryOfSubchecker(ValueChecker.class); + AnnotatedTypeMirror valueATM = valueFactory.getAnnotatedType(tree); + // These annotations are trusted rather than checked. Maybe have an option to + // disable using them? + if ((valueATM.hasAnnotation(INT_RANGE_FROM_NON_NEGATIVE) + || valueATM.hasAnnotation(INT_RANGE_FROM_POSITIVE)) + && type.hasAnnotation(SIGNED)) { + type.replaceAnnotation(SIGNED_POSITIVE); + } else { + Range treeRange = ValueCheckerUtils.getPossibleValues(valueATM, valueFactory); + + if (treeRange != null) { + switch (javaType.getKind()) { + case BYTE: + case CHAR: + if (treeRange.isWithin(0, Byte.MAX_VALUE)) { + type.replaceAnnotation(SIGNED_POSITIVE); } - } + break; + case SHORT: + if (treeRange.isWithin(0, Short.MAX_VALUE)) { + type.replaceAnnotation(SIGNED_POSITIVE); + } + break; + case INT: + if (treeRange.isWithin(0, Integer.MAX_VALUE)) { + type.replaceAnnotation(SIGNED_POSITIVE); + } + break; + case LONG: + if (treeRange.isWithin(0, Long.MAX_VALUE)) { + type.replaceAnnotation(SIGNED_POSITIVE); + } + break; + default: + // Nothing } } } @@ -225,7 +223,7 @@ public AnnotationMirrorSet getWidenedAnnotations( } if ((widenedTypeKind == TypeKind.INT || widenedTypeKind == TypeKind.LONG) && typeKind == TypeKind.CHAR) { - result.add(SIGNED_POSITIVE_FROM_UNSIGNED); + result.add(SIGNED_POSITIVE); return result; } return annos; @@ -286,9 +284,10 @@ public Void visitBinary(BinaryTree tree, AnnotatedTypeMirror type) { case UNSIGNED_RIGHT_SHIFT: TreePath path = getPath(tree); if (path != null - && (isMaskedShiftEitherSignedness(tree, path) - || isCastedShiftEitherSignedness(tree, path))) { - type.replaceAnnotation(SIGNEDNESS_GLB); + && (SignednessShifts.isMaskedShiftEitherSignedness(tree, path) + || SignednessShifts.isCastedShiftEitherSignedness( + tree, path))) { + type.replaceAnnotation(SIGNED_POSITIVE); } else { AnnotatedTypeMirror lht = getAnnotatedType(tree.getLeftOperand()); type.replaceAnnotations(lht.getAnnotations()); @@ -395,280 +394,6 @@ protected void addAnnotationsFromDefaultForType( } } - // The remainder of this file contains code to special-case shifts whose result does not depend - // on the MSB of the first argument, due to subsequent masking or casts. - - /** - * Returns true iff the given tree node is a mask operation (& or |). - * - * @param tree a tree to test - * @return true iff node is a mask operation (& or |) - */ - private boolean isMask(Tree tree) { - Tree.Kind kind = tree.getKind(); - - return kind == Tree.Kind.AND || kind == Tree.Kind.OR; - } - - // TODO: Return a TypeKind rather than a PrimitiveTypeTree? - /** - * Returns the type of a primitive cast, or null if the argument is not a cast to a primitive. - * - * @param tree a tree that might be a cast to a primitive - * @return type of a primitive cast, or null if not a cast to a primitive - */ - private @Nullable PrimitiveTypeTree primitiveTypeCast(Tree tree) { - if (tree.getKind() != Tree.Kind.TYPE_CAST) { - return null; - } - - TypeCastTree cast = (TypeCastTree) tree; - Tree castType = cast.getType(); - - Tree underlyingType; - if (castType.getKind() == Tree.Kind.ANNOTATED_TYPE) { - underlyingType = ((AnnotatedTypeTree) castType).getUnderlyingType(); - } else { - underlyingType = castType; - } - - if (underlyingType.getKind() != Tree.Kind.PRIMITIVE_TYPE) { - return null; - } - - return (PrimitiveTypeTree) underlyingType; - } - - /** - * Returns true iff the given tree is a literal. - * - * @param expr a tree to test - * @return true iff expr is a literal - */ - private boolean isLiteral(ExpressionTree expr) { - return expr instanceof LiteralTree; - } - - /** - * Returns the long value of an Integer or a Long - * - * @param obj either an Integer or a Long - * @return the long value of obj - */ - private long getLong(Object obj) { - return ((Number) obj).longValue(); - } - - /** - * Given a masking operation of the form {@code expr & maskLit} or {@code expr | maskLit}, - * return true iff the masking operation results in the same output regardless of the value of - * the shiftAmount most significant bits of expr. This is if the shiftAmount most significant - * bits of mask are 0 for AND, and 1 for OR. For example, assuming that shiftAmount is 4, the - * following is true about AND and OR masks: - * - *

{@code expr & 0x0[anything] == 0x0[something] ;} - * - *

{@code expr | 0xF[anything] == 0xF[something] ;} - * - * @param maskKind the kind of mask (AND or OR) - * @param shiftAmountLit the LiteralTree whose value is shiftAmount - * @param maskLit the LiteralTree whose value is mask - * @param shiftedTypeKind the type of shift operation; int or long - * @return true iff the shiftAmount most significant bits of mask are 0 for AND, and 1 for OR - */ - private boolean maskIgnoresMSB( - Tree.Kind maskKind, - LiteralTree shiftAmountLit, - LiteralTree maskLit, - TypeKind shiftedTypeKind) { - long shiftAmount = getLong(shiftAmountLit.getValue()); - - // Shift of zero is a nop - if (shiftAmount == 0) { - return true; - } - - long mask = getLong(maskLit.getValue()); - // Shift the shiftAmount most significant bits to become the shiftAmount least significant - // bits, zeroing out the rest. - if (shiftedTypeKind == TypeKind.INT) { - mask <<= 32; - } - mask >>>= (64 - shiftAmount); - - if (maskKind == Tree.Kind.AND) { - // Check that the shiftAmount most significant bits of the mask were 0. - return mask == 0; - } else if (maskKind == Tree.Kind.OR) { - // Check that the shiftAmount most significant bits of the mask were 1. - return mask == (1 << shiftAmount) - 1; - } else { - throw new TypeSystemError("Invalid Masking Operation"); - } - } - - /** - * Given a casted right shift of the form {@code (type) (baseExpr >> shiftAmount)} or {@code - * (type) (baseExpr >>> shiftAmount)}, return true iff the expression's value is the same - * regardless of the type of right shift (signed or unsigned). This is true if the cast ignores - * the shiftAmount most significant bits of the shift result -- that is, if the cast ignores all - * the new bits that the right shift introduced on the left. - * - *

For example, the function returns true for - * - * 

{@code (short) (myInt >> 16)}
- * - * and for - * - * 
{@code (short) (myInt >>> 16)}
- * - * because these two expressions are guaranteed to have the same result. - * - * @param shiftTypeKind the kind of the type of the shift literal (BYTE, CHAR, SHORT, INT, or - * LONG) - * @param castTypeKind the kind of the cast target type (BYTE, CHAR, SHORT, INT, or LONG) - * @param shiftAmountLit the LiteralTree whose value is shiftAmount - * @return true iff introduced bits are discarded - */ - private boolean castIgnoresMSB( - TypeKind shiftTypeKind, TypeKind castTypeKind, LiteralTree shiftAmountLit) { - - // Determine number of bits in the shift type, note shifts upcast to int. - // Also determine the shift amount as it is dependent on the shift type. - long shiftBits; - long shiftAmount; - switch (shiftTypeKind) { - case INT: - shiftBits = 32; - // When the LHS of the shift is an int, the 5 lower order bits of the RHS are used. - shiftAmount = 0x1F & getLong(shiftAmountLit.getValue()); - break; - case LONG: - shiftBits = 64; - // When the LHS of the shift is a long, the 6 lower order bits of the RHS are used. - shiftAmount = 0x3F & getLong(shiftAmountLit.getValue()); - break; - default: - throw new TypeSystemError("Invalid shift type"); - } - - // Determine number of bits in the cast type - long castBits; - switch (castTypeKind) { - case BYTE: - castBits = 8; - break; - case CHAR: - castBits = 8; - break; - case SHORT: - castBits = 16; - break; - case INT: - castBits = 32; - break; - case LONG: - castBits = 64; - break; - default: - throw new TypeSystemError("Invalid cast target"); - } - - long bitsDiscarded = shiftBits - castBits; - - return shiftAmount <= bitsDiscarded || shiftAmount == 0; - } - - /** - * Determines if a right shift operation, {@code >>} or {@code >>>}, is masked with a masking - * operation of the form {@code shiftExpr & maskLit} or {@code shiftExpr | maskLit} such that - * the mask renders the shift signedness ({@code >>} vs {@code >>>}) irrelevant by destroying - * the bits duplicated into the shift result. For example, the following pairs of right shifts - * on {@code byte b} both produce the same results under any input, because of their masks: - * - *

{@code (b >> 4) & 0x0F == (b >>> 4) & 0x0F;} - * - *

{@code (b >> 4) | 0xF0 == (b >>> 4) | 0xF0;} - * - * @param shiftExpr a right shift expression: {@code expr1 >> expr2} or {@code expr1 >>> expr2} - * @param path the path to {@code shiftExpr} - * @return true iff the right shift is masked such that a signed or unsigned right shift has the - * same effect - */ - /*package-private*/ boolean isMaskedShiftEitherSignedness(BinaryTree shiftExpr, TreePath path) { - IPair enclosingPair = TreePathUtil.enclosingNonParen(path); - // enclosing immediately contains shiftExpr or a parenthesized version of shiftExpr - Tree enclosing = enclosingPair.first; - // enclosingChild is a child of enclosing: shiftExpr or a parenthesized version of it. - @SuppressWarnings("interning:assignment.type.incompatible") // comparing AST nodes - @InternedDistinct Tree enclosingChild = enclosingPair.second; - - if (!isMask(enclosing)) { - return false; - } - - BinaryTree maskExpr = (BinaryTree) enclosing; - ExpressionTree shiftAmountExpr = shiftExpr.getRightOperand(); - - // Determine which child of maskExpr leads to shiftExpr. The other one is the mask. - ExpressionTree mask = - maskExpr.getRightOperand() == enclosingChild - ? maskExpr.getLeftOperand() - : maskExpr.getRightOperand(); - - // Strip away the parentheses from the mask if any exist - mask = TreeUtils.withoutParens(mask); - - if (!isLiteral(shiftAmountExpr) || !isLiteral(mask)) { - return false; - } - - LiteralTree shiftLit = (LiteralTree) shiftAmountExpr; - LiteralTree maskLit = (LiteralTree) mask; - - return maskIgnoresMSB( - maskExpr.getKind(), shiftLit, maskLit, TreeUtils.typeOf(shiftExpr).getKind()); - } - - /** - * Determines if a right shift operation, {@code >>} or {@code >>>}, is type casted such that - * the cast renders the shift signedness ({@code >>} vs {@code >>>}) irrelevant by discarding - * the bits duplicated into the shift result. For example, the following pair of right shifts on - * {@code short s} both produce the same results under any input, because of type casting: - * - *

{@code (byte)(s >> 8) == (byte)(b >>> 8);} - * - * @param shiftExpr a right shift expression: {@code expr1 >> expr2} or {@code expr1 >>> expr2} - * @param path the path to {@code shiftExpr} - * @return true iff the right shift is type casted such that a signed or unsigned right shift - * has the same effect - */ - /*package-private*/ boolean isCastedShiftEitherSignedness(BinaryTree shiftExpr, TreePath path) { - // enclosing immediately contains shiftExpr or a parenthesized version of shiftExpr - Tree enclosing = TreePathUtil.enclosingNonParen(path).first; - - PrimitiveTypeTree castPrimitiveType = primitiveTypeCast(enclosing); - if (castPrimitiveType == null) { - return false; - } - TypeKind castTypeKind = castPrimitiveType.getPrimitiveTypeKind(); - - // Determine the type of the shift result - TypeKind shiftTypeKind = TreeUtils.typeOf(shiftExpr).getKind(); - - // Determine shift literal - ExpressionTree shiftAmountExpr = shiftExpr.getRightOperand(); - if (!isLiteral(shiftAmountExpr)) { - return false; - } - LiteralTree shiftLit = (LiteralTree) shiftAmountExpr; - - boolean result = castIgnoresMSB(shiftTypeKind, castTypeKind, shiftLit); - return result; - } - - // End of special-case code for shifts that do not depend on the MSB of the first argument. - /** * Requires that, when two formal parameter types are annotated with {@code @PolySigned}, the * two arguments must have the same signedness type annotation. diff --git a/checker/src/main/java/org/checkerframework/checker/signedness/SignednessShifts.java b/checker/src/main/java/org/checkerframework/checker/signedness/SignednessShifts.java new file mode 100644 index 00000000000..19206e8f557 --- /dev/null +++ b/checker/src/main/java/org/checkerframework/checker/signedness/SignednessShifts.java @@ -0,0 +1,304 @@ +package org.checkerframework.checker.signedness; + +import com.sun.source.tree.AnnotatedTypeTree; +import com.sun.source.tree.BinaryTree; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.LiteralTree; +import com.sun.source.tree.PrimitiveTypeTree; +import com.sun.source.tree.Tree; +import com.sun.source.tree.TypeCastTree; +import com.sun.source.util.TreePath; + +import org.checkerframework.checker.interning.qual.InternedDistinct; +import org.checkerframework.checker.nullness.qual.Nullable; +import org.checkerframework.javacutil.TreePathUtil; +import org.checkerframework.javacutil.TreeUtils; +import org.checkerframework.javacutil.TypeSystemError; +import org.plumelib.util.IPair; + +import javax.lang.model.type.TypeKind; + +/** + * This file contains code to special-case shifts whose result does not depend on the MSB of the + * first argument, due to subsequent masking or casts. + * + * @checker_framework.manual #signedness-checker Signedness Checker + */ +public class SignednessShifts { + + /** Do not instantiate SignednessShifts. */ + private SignednessShifts() { + throw new Error("Do not instantiate SignednessShifts"); + } + + /** + * Returns true iff the given tree node is a mask operation (& or |). + * + * @param tree a tree to test + * @return true iff node is a mask operation (& or |) + */ + private static boolean isMask(Tree tree) { + Tree.Kind kind = tree.getKind(); + + return kind == Tree.Kind.AND || kind == Tree.Kind.OR; + } + + // TODO: Return a TypeKind rather than a PrimitiveTypeTree? + /** + * Returns the type of a primitive cast, or null if the argument is not a cast to a primitive. + * + * @param tree a tree that might be a cast to a primitive + * @return type of a primitive cast, or null if not a cast to a primitive + */ + private static @Nullable PrimitiveTypeTree primitiveTypeCast(Tree tree) { + if (tree.getKind() != Tree.Kind.TYPE_CAST) { + return null; + } + + TypeCastTree cast = (TypeCastTree) tree; + Tree castType = cast.getType(); + + Tree underlyingType; + if (castType.getKind() == Tree.Kind.ANNOTATED_TYPE) { + underlyingType = ((AnnotatedTypeTree) castType).getUnderlyingType(); + } else { + underlyingType = castType; + } + + if (underlyingType.getKind() != Tree.Kind.PRIMITIVE_TYPE) { + return null; + } + + return (PrimitiveTypeTree) underlyingType; + } + + /** + * Returns true iff the given tree is a literal. + * + * @param expr a tree to test + * @return true iff expr is a literal + */ + private static boolean isLiteral(ExpressionTree expr) { + return expr instanceof LiteralTree; + } + + /** + * Returns the long value of an Integer or a Long + * + * @param obj either an Integer or a Long + * @return the long value of obj + */ + private static long getLong(Object obj) { + return ((Number) obj).longValue(); + } + + /** + * Given a masking operation of the form {@code expr & maskLit} or {@code expr | maskLit}, + * return true iff the masking operation results in the same output regardless of the value of + * the shiftAmount most significant bits of expr. This is if the shiftAmount most significant + * bits of mask are 0 for AND, and 1 for OR. For example, assuming that shiftAmount is 4, the + * following is true about AND and OR masks: + * + *

{@code expr & 0x0[anything] == 0x0[something] ;} + * + *

{@code expr | 0xF[anything] == 0xF[something] ;} + * + * @param maskKind the kind of mask (AND or OR) + * @param shiftAmountLit the LiteralTree whose value is shiftAmount + * @param maskLit the LiteralTree whose value is mask + * @param shiftedTypeKind the type of shift operation; int or long + * @return true iff the shiftAmount most significant bits of mask are 0 for AND, and 1 for OR + */ + private static boolean maskIgnoresMSB( + Tree.Kind maskKind, + LiteralTree shiftAmountLit, + LiteralTree maskLit, + TypeKind shiftedTypeKind) { + long shiftAmount = getLong(shiftAmountLit.getValue()); + + // Shift of zero is a nop + if (shiftAmount == 0) { + return true; + } + + long mask = getLong(maskLit.getValue()); + // Shift the shiftAmount most significant bits to become the shiftAmount least significant + // bits, zeroing out the rest. + if (shiftedTypeKind == TypeKind.INT) { + mask <<= 32; + } + mask >>>= (64 - shiftAmount); + + if (maskKind == Tree.Kind.AND) { + // Check that the shiftAmount most significant bits of the mask were 0. + return mask == 0; + } else if (maskKind == Tree.Kind.OR) { + // Check that the shiftAmount most significant bits of the mask were 1. + return mask == (1 << shiftAmount) - 1; + } else { + throw new TypeSystemError("Invalid Masking Operation"); + } + } + + /** + * Given a casted right shift of the form {@code (type) (baseExpr >> shiftAmount)} or {@code + * (type) (baseExpr >>> shiftAmount)}, return true iff the expression's value is the same + * regardless of the type of right shift (signed or unsigned). This is true if the cast ignores + * the shiftAmount most significant bits of the shift result -- that is, if the cast ignores all + * the new bits that the right shift introduced on the left. + * + *

For example, the function returns true for + * + * 

{@code (short) (myInt >> 16)}
+ * + * and for + * + * 
{@code (short) (myInt >>> 16)}
+ * + * because these two expressions are guaranteed to have the same result. + * + * @param shiftTypeKind the kind of the type of the shift literal (BYTE, CHAR, SHORT, INT, or + * LONG) + * @param castTypeKind the kind of the cast target type (BYTE, CHAR, SHORT, INT, or LONG) + * @param shiftAmountLit the LiteralTree whose value is shiftAmount + * @return true iff introduced bits are discarded + */ + private static boolean castIgnoresMSB( + TypeKind shiftTypeKind, TypeKind castTypeKind, LiteralTree shiftAmountLit) { + + // Determine number of bits in the shift type, note shifts upcast to int. + // Also determine the shift amount as it is dependent on the shift type. + long shiftBits; + long shiftAmount; + switch (shiftTypeKind) { + case INT: + shiftBits = 32; + // When the LHS of the shift is an int, the 5 lower order bits of the RHS are used. + shiftAmount = 0x1F & getLong(shiftAmountLit.getValue()); + break; + case LONG: + shiftBits = 64; + // When the LHS of the shift is a long, the 6 lower order bits of the RHS are used. + shiftAmount = 0x3F & getLong(shiftAmountLit.getValue()); + break; + default: + throw new TypeSystemError("Invalid shift type"); + } + + // Determine number of bits in the cast type + long castBits; + switch (castTypeKind) { + case BYTE: + castBits = 8; + break; + case CHAR: + castBits = 8; + break; + case SHORT: + castBits = 16; + break; + case INT: + castBits = 32; + break; + case LONG: + castBits = 64; + break; + default: + throw new TypeSystemError("Invalid cast target"); + } + + long bitsDiscarded = shiftBits - castBits; + + return shiftAmount <= bitsDiscarded || shiftAmount == 0; + } + + /** + * Returns true if a right shift operation, {@code >>} or {@code >>>}, is masked with a masking + * operation of the form {@code shiftExpr & maskLit} or {@code shiftExpr | maskLit} such that + * the mask renders the shift signedness ({@code >>} vs {@code >>>}) irrelevant by destroying + * the bits duplicated into the shift result. For example, the following pairs of right shifts + * on {@code byte b} both produce the same results under any input, because of their masks: + * + *

{@code (b >> 4) & 0x0F == (b >>> 4) & 0x0F;} + * + *

{@code (b >> 4) | 0xF0 == (b >>> 4) | 0xF0;} + * + * @param shiftExpr a right shift expression: {@code expr1 >> expr2} or {@code expr1 >>> expr2} + * @param path the path to {@code shiftExpr} + * @return true iff the right shift is masked such that a signed or unsigned right shift has the + * same effect + */ + /*package-private*/ static boolean isMaskedShiftEitherSignedness( + BinaryTree shiftExpr, TreePath path) { + IPair enclosingPair = TreePathUtil.enclosingNonParen(path); + // enclosing immediately contains shiftExpr or a parenthesized version of shiftExpr + Tree enclosing = enclosingPair.first; + // enclosingChild is a child of enclosing: shiftExpr or a parenthesized version of it. + @SuppressWarnings("interning:assignment") // comparing AST nodes + @InternedDistinct Tree enclosingChild = enclosingPair.second; + + if (!isMask(enclosing)) { + return false; + } + + BinaryTree maskExpr = (BinaryTree) enclosing; + ExpressionTree shiftAmountExpr = shiftExpr.getRightOperand(); + + // Determine which child of maskExpr leads to shiftExpr. The other one is the mask. + ExpressionTree mask = + maskExpr.getRightOperand() == enclosingChild + ? maskExpr.getLeftOperand() + : maskExpr.getRightOperand(); + + // Strip away the parentheses from the mask if any exist + mask = TreeUtils.withoutParens(mask); + + if (!isLiteral(shiftAmountExpr) || !isLiteral(mask)) { + return false; + } + + LiteralTree shiftLit = (LiteralTree) shiftAmountExpr; + LiteralTree maskLit = (LiteralTree) mask; + + return maskIgnoresMSB( + maskExpr.getKind(), shiftLit, maskLit, TreeUtils.typeOf(shiftExpr).getKind()); + } + + /** + * Returns true if a right shift operation, {@code >>} or {@code >>>}, is type casted such that + * the cast renders the shift signedness ({@code >>} vs {@code >>>}) irrelevant by discarding + * the bits duplicated into the shift result. For example, the following pair of right shifts on + * {@code short s} both produce the same results under any input, because of type casting: + * + *

{@code (byte)(s >> 8) == (byte)(b >>> 8);} + * + * @param shiftExpr a right shift expression: {@code expr1 >> expr2} or {@code expr1 >>> expr2} + * @param path the path to {@code shiftExpr} + * @return true iff the right shift is type casted such that a signed or unsigned right shift + * has the same effect + */ + /*package-private*/ static boolean isCastedShiftEitherSignedness( + BinaryTree shiftExpr, TreePath path) { + // enclosing immediately contains shiftExpr or a parenthesized version of shiftExpr + Tree enclosing = TreePathUtil.enclosingNonParen(path).first; + + PrimitiveTypeTree castPrimitiveType = primitiveTypeCast(enclosing); + if (castPrimitiveType == null) { + return false; + } + TypeKind castTypeKind = castPrimitiveType.getPrimitiveTypeKind(); + + // Determine the type of the shift result + TypeKind shiftTypeKind = TreeUtils.typeOf(shiftExpr).getKind(); + + // Determine shift literal + ExpressionTree shiftAmountExpr = shiftExpr.getRightOperand(); + if (!isLiteral(shiftAmountExpr)) { + return false; + } + LiteralTree shiftLit = (LiteralTree) shiftAmountExpr; + + boolean result = castIgnoresMSB(shiftTypeKind, castTypeKind, shiftLit); + return result; + } +} diff --git a/checker/src/main/java/org/checkerframework/checker/signedness/SignednessVisitor.java b/checker/src/main/java/org/checkerframework/checker/signedness/SignednessVisitor.java index 740d4580afc..dc0f41b6624 100644 --- a/checker/src/main/java/org/checkerframework/checker/signedness/SignednessVisitor.java +++ b/checker/src/main/java/org/checkerframework/checker/signedness/SignednessVisitor.java @@ -39,7 +39,7 @@ public SignednessVisitor(BaseTypeChecker checker) { } /** - * Determines if an annotated type is annotated as {@link Unsigned} or {@link PolySigned} + * Returns true if an annotated type is annotated as {@link Unsigned} or {@link PolySigned} * * @param type the annotated type to be checked * @return true if the annotated type is annotated as {@link Unsigned} or {@link PolySigned} @@ -49,7 +49,7 @@ private boolean hasUnsignedAnnotation(AnnotatedTypeMirror type) { } /** - * Determines if an annotated type is annotated as {@link Signed} or {@link PolySigned} + * Returns true if an annotated type is annotated as {@link Signed} or {@link PolySigned} * * @param type the annotated type to be checked * @return true if the annotated type is annotated as {@link Signed} or {@link PolySigned} @@ -100,16 +100,18 @@ public Void visitBinary(BinaryTree tree, Void p) { case RIGHT_SHIFT: if (hasUnsignedAnnotation(leftOpType) - && !atypeFactory.isMaskedShiftEitherSignedness(tree, getCurrentPath()) - && !atypeFactory.isCastedShiftEitherSignedness(tree, getCurrentPath())) { + && !SignednessShifts.isMaskedShiftEitherSignedness(tree, getCurrentPath()) + && !SignednessShifts.isCastedShiftEitherSignedness( + tree, getCurrentPath())) { checker.reportError(leftOp, "shift.signed", kind, leftOpType, rightOpType); } break; case UNSIGNED_RIGHT_SHIFT: if (hasSignedAnnotation(leftOpType) - && !atypeFactory.isMaskedShiftEitherSignedness(tree, getCurrentPath()) - && !atypeFactory.isCastedShiftEitherSignedness(tree, getCurrentPath())) { + && !SignednessShifts.isMaskedShiftEitherSignedness(tree, getCurrentPath()) + && !SignednessShifts.isCastedShiftEitherSignedness( + tree, getCurrentPath())) { checker.reportError(leftOp, "shift.unsigned", kind, leftOpType, rightOpType); } break; diff --git a/checker/src/main/java/org/checkerframework/checker/tainting/TaintingAnnotatedTypeFactory.java b/checker/src/main/java/org/checkerframework/checker/tainting/TaintingAnnotatedTypeFactory.java new file mode 100644 index 00000000000..a92ec7a5f4f --- /dev/null +++ b/checker/src/main/java/org/checkerframework/checker/tainting/TaintingAnnotatedTypeFactory.java @@ -0,0 +1,38 @@ +package org.checkerframework.checker.tainting; + +import org.checkerframework.checker.tainting.qual.Untainted; +import org.checkerframework.common.basetype.BaseAnnotatedTypeFactory; +import org.checkerframework.common.basetype.BaseTypeChecker; +import org.checkerframework.javacutil.AnnotationBuilder; +import org.checkerframework.javacutil.AnnotationMirrorSet; + +import java.util.Set; + +import javax.lang.model.element.AnnotationMirror; + +/** Annotated type factory for the Tainting Checker. */ +public class TaintingAnnotatedTypeFactory extends BaseAnnotatedTypeFactory { + + /** The {@code @}{@link Untainted} annotation mirror. */ + private final AnnotationMirror UNTAINTED; + + /** A singleton set containing the {@code @}{@link Untainted} annotation mirror. */ + private final AnnotationMirrorSet setOfUntainted; + + /** + * Creates a {@link TaintingAnnotatedTypeFactory}. + * + * @param checker the tainting checker + */ + public TaintingAnnotatedTypeFactory(BaseTypeChecker checker) { + super(checker); + this.UNTAINTED = AnnotationBuilder.fromClass(getElementUtils(), Untainted.class); + this.setOfUntainted = AnnotationMirrorSet.singleton(UNTAINTED); + postInit(); + } + + @Override + protected Set getEnumConstructorQualifiers() { + return setOfUntainted; + } +} diff --git a/checker/tests/nullness/Issue2564.java b/checker/tests/nullness/Issue2564.java index 460499a0272..769f7442e0a 100644 --- a/checker/tests/nullness/Issue2564.java +++ b/checker/tests/nullness/Issue2564.java @@ -5,9 +5,9 @@ public abstract class Issue2564 { public enum EnumType { - // :: error: (assignment.type.incompatible) + // :: error: (enum.declaration.type.incompatible) @KeyFor("myMap") MY_KEY, - // :: error: (assignment.type.incompatible) + // :: error: (enum.declaration.type.incompatible) @KeyFor("enumMap") ENUM_KEY; private static final Map enumMap = new HashMap<>(); diff --git a/checker/tests/nullness/Issue2587.java b/checker/tests/nullness/Issue2587.java index 19ec9964323..bc1db838c01 100644 --- a/checker/tests/nullness/Issue2587.java +++ b/checker/tests/nullness/Issue2587.java @@ -3,7 +3,10 @@ import java.util.HashMap; import java.util.Map; -@SuppressWarnings("assignment.type.incompatible") // These warnings are not relevant +@SuppressWarnings({ + "enum.declaration.type.incompatible", + "assignment.type.incompatible" +}) // These warnings are not relevant public abstract class Issue2587 { public enum EnumType { // :: error: (expression.unparsable.type.invalid) diff --git a/checker/tests/resourceleak/AccumulationValueFieldTest.java b/checker/tests/resourceleak/AccumulationValueFieldTest.java new file mode 100644 index 00000000000..e145aa32426 --- /dev/null +++ b/checker/tests/resourceleak/AccumulationValueFieldTest.java @@ -0,0 +1,40 @@ +// This test checks the accumulation value for a field with a wildcard type. + +import org.checkerframework.checker.calledmethods.qual.*; +import org.checkerframework.checker.mustcall.qual.*; + +public class AccumulationValueFieldTest { + + @InheritableMustCall({"a"}) + class MCAB { + void a() {} + + void b() {} + } + + @InheritableMustCall({"a"}) + class FieldTest { + + @Owning + @MustCall({"a"}) T m = null; + + FieldTest(@Owning @MustCall({"a"}) T mcab) { + m = mcab; + } + + @RequiresCalledMethods( + value = {"this.m"}, + methods = {"a"}) + @CreatesMustCallFor("this") + void overwriteMCorrect(@Owning @MustCall({"a"}) T mcab) { + this.m = mcab; + } + + @EnsuresCalledMethods( + value = {"this.m"}, + methods = {"a"}) + void a() { + m.a(); + } + } +} diff --git a/checker/tests/tainting/Issue6110.java b/checker/tests/tainting/Issue6110.java new file mode 100644 index 00000000000..2e22c799320 --- /dev/null +++ b/checker/tests/tainting/Issue6110.java @@ -0,0 +1,23 @@ +import org.checkerframework.checker.tainting.qual.Tainted; +import org.checkerframework.checker.tainting.qual.Untainted; + +import java.util.EnumSet; + +class Issue6110 { + enum TestEnum { + ONE, + @Untainted TWO + } + + static void test(Enum<@Untainted TestEnum> o) { + + @Tainted TestEnum e = TestEnum.ONE; + o.compareTo(TestEnum.ONE); + o.compareTo(TestEnum.TWO); + + EnumSet<@Tainted TestEnum> s1 = EnumSet.of(TestEnum.ONE); + // :: error: (assignment.type.incompatible) + EnumSet<@Untainted TestEnum> s2 = EnumSet.of(TestEnum.ONE); + EnumSet<@Untainted TestEnum> s3 = EnumSet.of(TestEnum.TWO); + } +} diff --git a/checker/tests/tainting/Issue6116.java b/checker/tests/tainting/Issue6116.java new file mode 100644 index 00000000000..c4367118cca --- /dev/null +++ b/checker/tests/tainting/Issue6116.java @@ -0,0 +1,14 @@ +import java.util.Iterator; +import java.util.function.Consumer; + +class Issue6116 { + private final Iterator it; + + public Issue6116(Iterator iterator) { + this.it = iterator; + } + + public void test(Consumer action) { + it.forEachRemaining(action); + } +} diff --git a/dataflow/src/main/java/org/checkerframework/dataflow/cfg/builder/ExtendedNode.java b/dataflow/src/main/java/org/checkerframework/dataflow/cfg/builder/ExtendedNode.java index dbadba1b828..1c1c40f6e69 100644 --- a/dataflow/src/main/java/org/checkerframework/dataflow/cfg/builder/ExtendedNode.java +++ b/dataflow/src/main/java/org/checkerframework/dataflow/cfg/builder/ExtendedNode.java @@ -18,6 +18,9 @@ *

  • TWO_TARGET_CONDITIONAL_JUMP: {@link ConditionalJump}. A conditional jump with two * targets for both the 'then' and 'else' branch. * + * + * Note that this class is deliberately public, to enable users of the dataflow library to customize + * CFG construction. */ @SuppressWarnings("nullness") // TODO public abstract class ExtendedNode { diff --git a/dataflow/src/main/java/org/checkerframework/dataflow/cfg/builder/Label.java b/dataflow/src/main/java/org/checkerframework/dataflow/cfg/builder/Label.java index 3b28c14e22b..3ce9e3dcda3 100644 --- a/dataflow/src/main/java/org/checkerframework/dataflow/cfg/builder/Label.java +++ b/dataflow/src/main/java/org/checkerframework/dataflow/cfg/builder/Label.java @@ -4,6 +4,9 @@ * A label is used to refer to other extended nodes using a mapping from labels to extended nodes. * Labels get their names either from labeled statements in the source code or from internally * generated unique names. + * + *

    Note that this class is deliberately public, to enable users of the dataflow library to + * customize CFG construction. */ public class Label { diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index c3a25dca343..34a7e9ba80a 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -1,4 +1,4 @@ -Version 3.37.0-eisop1 (October ?, 2023) +Version 3.38.0-eisop1 (October ?, 2023) --------------------------------------- **User-visible changes:** @@ -48,6 +48,23 @@ Changed the return types of eisop#297, eisop#376, eisop#400, eisop#519, eisop#532, eisop#533, typetools#1590, typetools#1919. +Version 3.38.0 (September 1, 2023) +---------------------------------- + +**User-visible changes:** + +Eliminated the `@SignedPositiveFromUnsigned` annotation, which users were +advised against using. + +**Implementation details:** + +Renamed `SourceChecker.processArg()' to `processErrorMessageArg()`. + +**Closed issues:** + +#2156, #5672, #6110, #6111, #6116, #6125, #6129, #6136. + + Version 3.37.0 (August 1, 2023) ------------------------------- diff --git a/docs/checker-framework-webpage.html b/docs/checker-framework-webpage.html index 5c5d4d21a80..af9e37e1265 100644 --- a/docs/checker-framework-webpage.html +++ b/docs/checker-framework-webpage.html @@ -30,8 +30,8 @@

    The Checker Framework

    Installation instructions and tutorial.
  • - Download: checker-framework-3.37.0.zip - (1 Aug 2023); + Download: checker-framework-3.38.0.zip + (1 Sep 2023); includes source, platform-independent binary, tests, and documentation.
    Then, see the installation @@ -93,7 +93,7 @@

    The Checker Framework

    the .class file. The tools support both Java 5 declaration annotations and Java 8 type annotations.     @@ -479,12 +481,70 @@

    Making a Checker Framework release +

    Describing a case study

    + +

    + After you have performed a case study of applying some checker to an + open-source codebase, you should have: +

    + +
      +
    • + the unannotated code, as of the time/commit you started annotating it. + This might be in your forked repository, or in an upstream repository. +
      • +
    • + the annotated code. Typically, this is in a branch of your forked repository. + +

      + In the annotated code, each @SuppressWarnings annotation should have a brief justification, explaining why the code is correct and the warning is a false positive. + The justification should be a //-style comment, on the same line as the argument to @SuppressWarnings. For example: +

      + +
      +  @SuppressWarnings("nullness:assignment") // dynamic check: checked against null immediately above
+
+  @SuppressWarnings({
+    "nullness:assignment" // dynamic check: checked against null immediately above
+  })
+
      + + If there are more than about 10 warning suppressions, prefix each one by a + category followed by a colon (as with "dynamic check:") above, to aid in + computing statistics about the causes of false positive warnings. + +
      • +
    • + the command to run the type-checker(s) on the annotated code. +
      • +
    • + a list of all the bugs you fixed, with a brief description of each. The + description can point to pull requests, or commits, or just be text; provide + whatever is most helpful. +
      • +
    + + +

    Counting annotations

    + +

    +After you have annotated a project, you may wish to count the annotations that you have written. +These programs will help you do that: +

    + + + +

    Building a historical version of the Checker Framework

    The use of four different repositories, as explained in "Related repositories" above, means that you -cannot check out an old version of the Checker Framework and build it +cannot just check out an old version of the Checker Framework and build it with ./gradlew assemble. By default, the Checker Framework build uses the latest version of the other three repositories, which are rarely compatible with an old version of the Checker Framework. One symptom of the @@ -538,7 +598,7 @@

    Building a historical version of the Checker Framework - diff --git a/docs/examples/MavenExample/pom.xml b/docs/examples/MavenExample/pom.xml index 8404f00a0e0..2d9e8f21b34 100644 --- a/docs/examples/MavenExample/pom.xml +++ b/docs/examples/MavenExample/pom.xml @@ -12,8 +12,8 @@ UTF-8 - - ${com.google.errorprone:javac:jar} + 8 + 8 3.34.0-eisop1 @@ -50,18 +50,6 @@ - - - org.apache.maven.plugins - maven-dependency-plugin - - - - properties - - - - org.apache.maven.plugins maven-site-plugin @@ -83,10 +71,6 @@ 3.10.1 true - - 10000 - 10000 - io.github.eisop @@ -99,19 +83,16 @@ org.checkerframework.checker.nullness.NullnessChecker + -Xmaxerrs + 10000 + -Xmaxwarns + 10000 - - - io.github.eisop - checker - ${checkerFrameworkVersion} - - @@ -123,25 +104,33 @@ 9+181-r4173-1 - - - com.google.errorprone - javac - 9+181-r4173-1 - - + + + org.apache.maven.plugins + maven-dependency-plugin + + + + copy + + process-sources + + com.google.errorprone:javac:${javac.version}:jar + ${project.build.directory}/javac + + + + org.apache.maven.plugins maven-compiler-plugin - 3.10.1 - true - 1.8 - 1.8 - -J-Xbootclasspath/p:${settings.localRepository}/com/google/errorprone/javac/${javac.version}/javac-${javac.version}.jar + -J-Xbootclasspath/p:${project.build.directory}/javac/javac-${javac.version}.jar @@ -159,10 +148,7 @@ org.apache.maven.plugins maven-compiler-plugin - 3.10.1 - true - 11 -J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED -J--add-exports=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED diff --git a/docs/manual/called-methods-checker.tex b/docs/manual/called-methods-checker.tex index a2bbae59c55..4056c798e7d 100644 --- a/docs/manual/called-methods-checker.tex +++ b/docs/manual/called-methods-checker.tex @@ -193,7 +193,8 @@ \label{called-methods-ensurescalledmethods} \item[\refqualclass{checker/calledmethods/qual}{EnsuresCalledMethods}] This declaration annotation specifies a post-condition on a method, indicating the methods it - guarantees to be called. + guarantees to be called. This annotation is repeatable, meaning that you can write + multiple copies of it (with different arguments) on the same method, and the checker will check all of them. For example, this specification: diff --git a/docs/manual/contributors.tex b/docs/manual/contributors.tex index 60faab7c00e..d822b4de5ea 100644 --- a/docs/manual/contributors.tex +++ b/docs/manual/contributors.tex @@ -93,6 +93,7 @@ Neil Brown, Nhat Dinh, Nhat Nguyen, +Nicholas Breen, Nikhil Shinde, Nima Karimipour, Nitin Kumar Das, diff --git a/docs/manual/external-tools.tex b/docs/manual/external-tools.tex index 82991e0f94a..7190cd9c887 100644 --- a/docs/manual/external-tools.tex +++ b/docs/manual/external-tools.tex @@ -948,7 +948,8 @@ If you use the \href{https://maven.apache.org/}{Maven} tool, then you can enable Checker Framework checkers by following the -instructions below. +instructions below. These instructions have been tested with +Maven 3.9.3, and they should work with any 3.x version of Maven. See the directory \code{docs/examples/MavenExample/} for examples of the use of Maven build files. @@ -1000,57 +1001,6 @@ mvn versions:use-latest-versions -Dincludes="io.github.eisop:*" \end{Verbatim} -\item If using JDK 8, use a Maven property to hold the location of the - Error Prone \. -To set the value of these properties automatically, you will use the Maven Dependency plugin. - -Add a dependency: - -\begin{alltt} - - ... existing items ... - - - com.google.errorprone - javac - 9+181-r4173-1 - - -\end{alltt} - -Java 11 and later do not need the \ -dependency, but it does no harm. -The need for the \ artifact when running under -JDK 8 is explained in Section~\ref{javac-jdk8}. - -Create the property in the \code{properties} section of the POM: - -\begin{alltt} - - - $\{com.google.errorprone:javac:jar\} - -\end{alltt} - -Change the reference to the \code{maven-dependency-plugin} within the \code{} -section, or add it if it is not present. - -% Use of 4 initial spaces improves pasteability into existing pom.xml files. -\begin{alltt} - - - org.apache.maven.plugins - maven-dependency-plugin - - - - properties - - - - -\end{alltt} - \item Direct the Maven compiler plugin to use the desired checkers by creating three new profiles as shown below (the example uses the Nullness Checker). If you do not use Java 8, then you only need two of the @@ -1085,10 +1035,6 @@ 3.10.1 true - - 10000 - 10000 - io.github.eisop @@ -1101,19 +1047,16 @@ org.checkerframework.checker.nullness.NullnessChecker + -Xmaxerrs + 10000 + -Xmaxwarns + 10000 - - - io.github.eisop - checker - \ReleaseVersion{} - - @@ -1125,22 +1068,33 @@ 9+181-r4173-1 - - - com.google.errorprone - javac - 9+181-r4173-1 - - + + + org.apache.maven.plugins + maven-dependency-plugin + + + + copy + + process-sources + + com.google.errorprone:javac:\$\{javac.version\}:jar + \$\{project.build.directory\}/javac + + + + org.apache.maven.plugins maven-compiler-plugin - true - -J-Xbootclasspath/p:$\{settings.localRepository\}/com/google/errorprone/javac/$\{javac.version\}/javac-$\{javac.version\}.jar + -J-Xbootclasspath/p:\$\{project.build.directory\}/javac/javac-\$\{javac.version\}.jar @@ -1159,7 +1113,6 @@ org.apache.maven.plugins maven-compiler-plugin - true -J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED -J--add-exports=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED @@ -1175,15 +1128,14 @@ - - - true - \end{alltt} \end{mysmall} +The need for the \ artifact when running under +JDK 8 is explained in Section~\ref{javac-jdk8}. + Now, building with Maven will run the checkers during every compilation that uses JDK 8 or higher. @@ -1200,7 +1152,7 @@ \end{Verbatim} To compile without using the Checker Framework, pass -\<-P !checkerframework> +\<-P '!checkerframework'> on the Maven command line. %% Per GitHub user danibs in %% https://github.com/typetools/checker-framework/issues/5086, @@ -1224,6 +1176,20 @@ \end{enumerate} +If you use \, you must upgrade to version 1.20 +or later. Version 1.19 and earlier are broken. If necessary, you can +disable animal sniffer by modifying your POM file's properties: + +\begin{mysmall} +\begin{alltt} + + ... existing items ... + + + true + +\end{alltt} +\end{mysmall} \subsectionAndLabel{Maven, with a locally-built version of the Checker Framework}{maven-locally-built} diff --git a/docs/manual/figures/signedness.svg b/docs/manual/figures/signedness.svg index 8f43eddf727..e745ed9cbae 100644 --- a/docs/manual/figures/signedness.svg +++ b/docs/manual/figures/signedness.svg @@ -1,19 +1,19 @@ + inkscape:version="1.2.2 (b0a8486541, 2022-12-01)" + sodipodi:docname="signedness.svg" + xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape" + xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd" + xmlns="http://www.w3.org/2000/svg" + xmlns:svg="http://www.w3.org/2000/svg" + xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#" + xmlns:cc="http://creativecommons.org/ns#" + xmlns:dc="http://purl.org/dc/elements/1.1/"> @@ -113,24 +113,28 @@ guidetolerance="10" inkscape:pageopacity="0" inkscape:pageshadow="2" - inkscape:window-width="2560" - inkscape:window-height="1534" + inkscape:window-width="1763" + inkscape:window-height="1011" id="namedview76" showgrid="false" inkscape:snap-page="true" inkscape:zoom="1.094736" - inkscape:cx="45.247074" - inkscape:cy="191.49037" - inkscape:window-x="0" - inkscape:window-y="29" - inkscape:window-maximized="1" - inkscape:current-layer="svg2" + inkscape:cx="50.697154" + inkscape:cy="86.778913" + inkscape:window-x="37" + inkscape:window-y="64" + inkscape:window-maximized="0" + inkscape:current-layer="g66-3" inkscape:snap-others="true" fit-margin-top="0" fit-margin-left="0" fit-margin-right="0" fit-margin-bottom="0" - inkscape:snap-global="false" /> + inkscape:snap-global="false" + inkscape:showpageshadow="2" + inkscape:pagecheckerboard="0" + inkscape:deskcolor="#d1d1d1" + inkscape:document-units="cm" /> @@ -153,12 +157,10 @@ style="font-style:normal;font-weight:normal;font-size:16px;line-height:0%;font-family:'courier new';text-anchor:middle;fill:#000000" x="700" y="345.85001" - id="text10"> - @Unsigned - + id="tspan12">@Unsigned - @Signed - + id="tspan22">@Signed - @UnknownSignedness - + id="tspan32">@UnknownSignedness - @SignedPositive - - - @SignednessGlb - + y="404.8703" + id="tspan48">@SignednessGlb - @SignednessBottom - + id="tspan74-4">@SignednessBottom - @SignedPositive- - - - FromUnsigned - + y="405.1297">@SignedPositive */ @RunWith(PerDirectorySuite.class) -public abstract class CheckerFrameworkPerDirectoryTest { +public abstract class CheckerFrameworkPerDirectoryTest extends CheckerFrameworkRootedTest { /** The files containing test code, which will be type-checked. */ protected final List testFiles; @@ -53,7 +53,10 @@ public abstract class CheckerFrameworkPerDirectoryTest { /** The binary names of the checkers to run. */ protected final List<@BinaryName String> checkerNames; - /** The path, relative to currentDir/test to the directory containing test inputs. */ + /** + * The path, relative to the test root directory (see {@link + * CheckerFrameworkRootedTest#resolveTestDirectory()}), to the directory containing test inputs. + */ protected final String testDir; /** Extra options to pass to javac when running the checker. */ @@ -70,7 +73,7 @@ public abstract class CheckerFrameworkPerDirectoryTest { * * @param testFiles the files containing test code, which will be type-checked * @param checker the class for the checker to use - * @param testDir the path to the directory of test inputs + * @param testDir the path, relative to currentDir/tests, to the directory of test inputs * @param checkerOptions options to pass to the compiler when running tests */ protected CheckerFrameworkPerDirectoryTest( @@ -89,7 +92,7 @@ protected CheckerFrameworkPerDirectoryTest( * * @param testFiles the files containing test code, which will be type-checked * @param checker the class for the checker to use - * @param testDir the path to the directory of test inputs + * @param testDir the path, relative to currentDir/tests, to the directory of test inputs * @param classpathExtra extra entries for the classpath, relative to a directory such as * checker-framework/checker * @param checkerOptions options to pass to the compiler when running tests @@ -120,7 +123,7 @@ protected CheckerFrameworkPerDirectoryTest( * * @param testFiles the files containing test code, which will be type-checked * @param checkerNames the binary names of the checkers to run - * @param testDir the path to the directory of test inputs + * @param testDir the path, relative to currentDir/tests, to the directory of test inputs * @param classpathExtra extra entries for the classpath, relative to a directory such as * checker-framework/checker * @param checkerOptions options to pass to the compiler when running tests @@ -131,9 +134,10 @@ protected CheckerFrameworkPerDirectoryTest( String testDir, List classpathExtra, String... checkerOptions) { + super(); this.testFiles = testFiles; this.checkerNames = checkerNames; - this.testDir = "tests" + File.separator + testDir; + this.testDir = testDir; this.classpathExtra = classpathExtra; this.checkerOptions = Arrays.asList(checkerOptions); } @@ -145,7 +149,7 @@ public void run() { customizeOptions(Collections.unmodifiableList(checkerOptions)); TestConfiguration config = TestConfigurationBuilder.buildDefaultConfiguration( - testDir, + new File(resolveTestDirectory(), testDir).getPath(), testFiles, classpathExtra, checkerNames, @@ -153,7 +157,7 @@ public void run() { shouldEmitDebugInfo); TypecheckResult testResult = new TypecheckExecutor().runTest(config); TypecheckResult adjustedTestResult = adjustTypecheckResult(testResult); - TestUtilities.assertTestDidNotFail(adjustedTestResult); + checkResult(adjustedTestResult); } /** diff --git a/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkPerFileTest.java b/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkPerFileTest.java index 9aec49f5f29..de1f9c7fb10 100644 --- a/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkPerFileTest.java +++ b/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkPerFileTest.java @@ -47,7 +47,7 @@ * */ @RunWith(PerFileSuite.class) -public abstract class CheckerFrameworkPerFileTest { +public abstract class CheckerFrameworkPerFileTest extends CheckerFrameworkRootedTest { /** The file containing test code, which will be type-checked. */ protected final File testFile; @@ -55,7 +55,10 @@ public abstract class CheckerFrameworkPerFileTest { /** The checker to use for tests. */ protected final Class checker; - /** The path, relative to currentDir/test to the directory containing test inputs. */ + /** + * The path, relative to the test root directory (see {@link + * CheckerFrameworkRootedTest#resolveTestDirectory()}), to the directory containing test inputs. + */ protected final String testDir; /** Extra options to pass to javac when running the checker. */ @@ -69,7 +72,7 @@ public abstract class CheckerFrameworkPerFileTest { * * @param testFile the file containing test code, which will be type-checked * @param checker the class for the checker to use - * @param testDir the path to the directory of test inputs + * @param testDir the path, relative to currentDir/tests, to the directory of test inputs * @param checkerOptions options to pass to the compiler when running tests */ protected CheckerFrameworkPerFileTest( @@ -77,9 +80,10 @@ protected CheckerFrameworkPerFileTest( Class checker, String testDir, String... checkerOptions) { + super(); this.testFile = testFile; this.checker = checker; - this.testDir = "tests" + File.separator + testDir; + this.testDir = testDir; this.checkerOptions = new ArrayList<>(Arrays.asList(checkerOptions)); } @@ -90,9 +94,13 @@ public void run() { customizeOptions(Collections.unmodifiableList(checkerOptions)); TestConfiguration config = TestConfigurationBuilder.buildDefaultConfiguration( - testDir, testFile, checker, customizedOptions, shouldEmitDebugInfo); + new File(resolveTestDirectory(), testDir).getPath(), + testFile, + checker, + customizedOptions, + shouldEmitDebugInfo); TypecheckResult testResult = new TypecheckExecutor().runTest(config); - TestUtilities.assertTestDidNotFail(testResult); + checkResult(testResult); } /** diff --git a/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkRootedTest.java b/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkRootedTest.java new file mode 100644 index 00000000000..53e974d7dfa --- /dev/null +++ b/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkRootedTest.java @@ -0,0 +1,33 @@ +package org.checkerframework.framework.test; + +import java.io.File; + +/** Encapsulates the directory root to search within for test files to compile. */ +abstract class CheckerFrameworkRootedTest { + + /** Constructs a test that will assert that can resolve its tests root directory. */ + public CheckerFrameworkRootedTest() {} + + /** + * Resolves the test root directory from the optional {@link TestRootDirectory} annotation or + * falls back to the default of {@code currentDir/tests}. + * + * @return the resolved directory + */ + protected File resolveTestDirectory() { + TestRootDirectory annotation = getClass().getAnnotation(TestRootDirectory.class); + if (annotation != null) { + return new File(annotation.value()); + } + return new File("tests"); + } + + /** + * Check that the {@link TypecheckResult} did not fail. + * + * @param typecheckResult result to check + */ + public void checkResult(TypecheckResult typecheckResult) { + TestUtilities.assertTestDidNotFail(typecheckResult); + } +} diff --git a/framework-test/src/main/java/org/checkerframework/framework/test/PerDirectorySuite.java b/framework-test/src/main/java/org/checkerframework/framework/test/PerDirectorySuite.java index 1576e64fa91..25c518e28e9 100644 --- a/framework-test/src/main/java/org/checkerframework/framework/test/PerDirectorySuite.java +++ b/framework-test/src/main/java/org/checkerframework/framework/test/PerDirectorySuite.java @@ -5,7 +5,6 @@ import org.junit.runner.notification.RunNotifier; import org.junit.runners.BlockJUnit4ClassRunner; import org.junit.runners.Parameterized.Parameters; -import org.junit.runners.Suite; import org.junit.runners.model.FrameworkMethod; import org.junit.runners.model.InitializationError; import org.junit.runners.model.Statement; @@ -35,8 +34,9 @@ * test against OR a {@code String []} where each String in the array is a directory in the tests * directory. */ -public class PerDirectorySuite extends Suite { +public class PerDirectorySuite extends RootedSuite { + /** Name */ @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.METHOD) public @interface Name {} @@ -80,7 +80,7 @@ private List> getParametersList(TestClass klass) throws Throwable { "@Parameters annotation on method that does not return an array: " + method); } String[] dirs = (String[]) method.invokeExplosively(null); - return TestUtilities.findJavaFilesPerDirectory(new File("tests"), dirs); + return TestUtilities.findJavaFilesPerDirectory(resolveTestDirectory(), dirs); } /** Returns method annotated @Parameters, typically the getTestDirs or getTestFiles method. */ diff --git a/framework-test/src/main/java/org/checkerframework/framework/test/PerFileSuite.java b/framework-test/src/main/java/org/checkerframework/framework/test/PerFileSuite.java index 276d7a6ec6e..7466ff2cc27 100644 --- a/framework-test/src/main/java/org/checkerframework/framework/test/PerFileSuite.java +++ b/framework-test/src/main/java/org/checkerframework/framework/test/PerFileSuite.java @@ -5,7 +5,6 @@ import org.junit.runner.notification.RunNotifier; import org.junit.runners.BlockJUnit4ClassRunner; import org.junit.runners.Parameterized.Parameters; -import org.junit.runners.Suite; import org.junit.runners.model.FrameworkMethod; import org.junit.runners.model.InitializationError; import org.junit.runners.model.Statement; @@ -36,8 +35,9 @@ * test against OR a {@code String []} where each String in the array is a directory in the tests * directory. */ -public class PerFileSuite extends Suite { +public class PerFileSuite extends RootedSuite { + /** Name */ @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.METHOD) public @interface Name {} @@ -62,7 +62,9 @@ public PerFileSuite(Class klass) throws Throwable { List parametersList = getParametersList(testClass); for (Object[] parameters : parametersList) { - runners.add(new PerParameterSetTestRunner(javaTestClass, parameters)); + runners.add( + new PerParameterSetTestRunner( + javaTestClass, resolveTestDirectory(), parameters)); } } @@ -82,7 +84,7 @@ private List getParametersList(TestClass klass) throws Throwable { } if (method.getReturnType().isArray()) { String[] dirs = (String[]) method.invokeExplosively(null); - javaFiles = TestUtilities.findNestedJavaTestFiles(dirs); + javaFiles = TestUtilities.findRelativeNestedJavaFiles(resolveTestDirectory(), dirs); } else { javaFiles = (List) method.invokeExplosively(null); } @@ -160,10 +162,24 @@ private FrameworkMethod getParametersMethod(TestClass testClass) { /** Runs the test class for the set of parameters passed in the constructor. */ private static class PerParameterSetTestRunner extends BlockJUnit4ClassRunner { + /** A directory prefix to remove from the test name. */ + private final File directoryPrefix; + + /** The parameters, java source file(s) under {@link #directoryPrefix}. */ private final Object[] parameters; - PerParameterSetTestRunner(Class type, Object[] parameters) throws InitializationError { + /** + * Creates a PerParameterSetTestRunner for the test class {@code type}. + * + * @param type the test class + * @param directoryPrefix directory prefix to remove from the test name + * @param parameters java source file(s) under {@link #directoryPrefix} + * @throws InitializationError if the test class is malformed + */ + PerParameterSetTestRunner(Class type, File directoryPrefix, Object[] parameters) + throws InitializationError { super(type); + this.directoryPrefix = directoryPrefix; this.parameters = parameters; } @@ -179,7 +195,10 @@ public Object createTest() throws Exception { */ String testCaseName() { File file = (File) parameters[0]; - String name = file.getPath().replace(".java", "").replace("tests" + File.separator, ""); + String name = + file.getPath() + .replace(".java", "") + .replace(directoryPrefix.getPath() + File.separator, ""); return name; } diff --git a/framework-test/src/main/java/org/checkerframework/framework/test/RootedSuite.java b/framework-test/src/main/java/org/checkerframework/framework/test/RootedSuite.java new file mode 100644 index 00000000000..37e2be289ed --- /dev/null +++ b/framework-test/src/main/java/org/checkerframework/framework/test/RootedSuite.java @@ -0,0 +1,40 @@ +package org.checkerframework.framework.test; + +import org.junit.runner.Runner; +import org.junit.runners.Suite; +import org.junit.runners.model.InitializationError; + +import java.io.File; +import java.util.List; + +/** + * Encapsulates the directory root to search within for test files to parameterise the test with. + */ +abstract class RootedSuite extends Suite { + + /** + * Called by this class and subclasses once the runners making up the suite have been + * determined. + * + * @param klass root of the suite + * @param runners for each class in the suite, a {@link Runner} + * @throws InitializationError malformed test suite + */ + public RootedSuite(Class klass, List runners) throws InitializationError { + super(klass, runners); + } + + /** + * Resolves the directory specified by {@link TestRootDirectory} or defaults to {@code + * currentDir/tests}. + * + * @return the resolved directory + */ + protected final File resolveTestDirectory() { + TestRootDirectory annotation = getTestClass().getAnnotation(TestRootDirectory.class); + if (annotation != null) { + return new File(annotation.value()); + } + return new File("tests"); + } +} diff --git a/framework-test/src/main/java/org/checkerframework/framework/test/TestRootDirectory.java b/framework-test/src/main/java/org/checkerframework/framework/test/TestRootDirectory.java new file mode 100644 index 00000000000..b288bc7ac2b --- /dev/null +++ b/framework-test/src/main/java/org/checkerframework/framework/test/TestRootDirectory.java @@ -0,0 +1,20 @@ +package org.checkerframework.framework.test; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** Defines the path to the directory which holds test java files. */ +@Target(ElementType.TYPE) +@Retention(RetentionPolicy.RUNTIME) +@Inherited +public @interface TestRootDirectory { + /** + * Path, relative to the current/module directory, within which to search for test sources. + * + * @return tests root directory + */ + String value(); +} diff --git a/framework-test/src/main/java/org/checkerframework/framework/test/TestUtilities.java b/framework-test/src/main/java/org/checkerframework/framework/test/TestUtilities.java index d05e76f8248..0c85c97e962 100644 --- a/framework-test/src/main/java/org/checkerframework/framework/test/TestUtilities.java +++ b/framework-test/src/main/java/org/checkerframework/framework/test/TestUtilities.java @@ -81,14 +81,34 @@ public class TestUtilities { compiler.run(null, null, err, "-version"); } + /** + * Find test java sources within currentDir/tests. + * + * @param dirNames subdirectories of currentDir/tests + * @return found files + */ public static List findNestedJavaTestFiles(String... dirNames) { return findRelativeNestedJavaFiles(new File("tests"), dirNames); } + /** + * Find test java sources within {@code parent}. + * + * @param parent directory to search within + * @param dirNames subdirectories of {@code parent} + * @return found files + */ public static List findRelativeNestedJavaFiles(String parent, String... dirNames) { return findRelativeNestedJavaFiles(new File(parent), dirNames); } + /** + * Find test java sources within {@code parent}. + * + * @param parent directory to search within + * @param dirNames subdirectories of {@code parent} + * @return found files + */ public static List findRelativeNestedJavaFiles(File parent, String... dirNames) { File[] dirs = new File[dirNames.length]; diff --git a/framework-test/src/test/java/org/checkerframework/framework/test/test/junit/AlternateTestRootPerDirTest.java b/framework-test/src/test/java/org/checkerframework/framework/test/test/junit/AlternateTestRootPerDirTest.java new file mode 100644 index 00000000000..08d5e7afc02 --- /dev/null +++ b/framework-test/src/test/java/org/checkerframework/framework/test/test/junit/AlternateTestRootPerDirTest.java @@ -0,0 +1,64 @@ +package org.checkerframework.framework.test.test.junit; + +import org.checkerframework.common.value.ValueChecker; +import org.checkerframework.framework.test.CheckerFrameworkPerDirectoryTest; +import org.checkerframework.framework.test.TestRootDirectory; +import org.checkerframework.framework.test.TypecheckResult; +import org.checkerframework.framework.test.diagnostics.TestDiagnosticUtils; +import org.hamcrest.CoreMatchers; +import org.hamcrest.MatcherAssert; +import org.junit.runners.Parameterized.Parameters; + +import java.io.File; +import java.util.List; + +/** Tests the explicit tests root configuration. */ +@TestRootDirectory("tests-alt") +public class AlternateTestRootPerDirTest extends CheckerFrameworkPerDirectoryTest { + + /** + * @param testFiles the files containing test code, which will be type-checked + */ + public AlternateTestRootPerDirTest(List testFiles) { + super(testFiles, ValueChecker.class, ""); + } + + @Parameters + public static String[] getTestDirs() { + return new String[] {"alt-dir-a", "alt-dir-b"}; + } + + @Override + public void checkResult(TypecheckResult typecheckResult) { + super.checkResult(typecheckResult); + MatcherAssert.assertThat( + "test check result has exactly one expected diagnostic", + typecheckResult.getExpectedDiagnostics().size(), + CoreMatchers.describedAs( + "singleton collection %0", + CoreMatchers.is(1), typecheckResult.getExpectedDiagnostics())); + MatcherAssert.assertThat( + "test check result has the expected diagnostic of one of the test files", + typecheckResult.getExpectedDiagnostics(), + CoreMatchers.either( + CoreMatchers.hasItem( + TestDiagnosticUtils.fromJavaFileComment( + "Issue6125A.java", 5, "error: (assignment)"))) + .or( + CoreMatchers.hasItem( + TestDiagnosticUtils.fromJavaFileComment( + "Issue6125B.java", 5, "error: (assignment)")))); + MatcherAssert.assertThat( + "test check result has exactly zero unexpected diagnostics", + typecheckResult.getUnexpectedDiagnostics().size(), + CoreMatchers.describedAs( + "zero length collection %0", + CoreMatchers.is(0), typecheckResult.getUnexpectedDiagnostics())); + MatcherAssert.assertThat( + "test check result has exactly zero missing diagnostics", + typecheckResult.getMissingDiagnostics().size(), + CoreMatchers.describedAs( + "zero length collection %0", + CoreMatchers.is(0), typecheckResult.getMissingDiagnostics())); + } +} diff --git a/framework-test/src/test/java/org/checkerframework/framework/test/test/junit/AlternateTestRootPerFileWithDirsTest.java b/framework-test/src/test/java/org/checkerframework/framework/test/test/junit/AlternateTestRootPerFileWithDirsTest.java new file mode 100644 index 00000000000..14cc945628a --- /dev/null +++ b/framework-test/src/test/java/org/checkerframework/framework/test/test/junit/AlternateTestRootPerFileWithDirsTest.java @@ -0,0 +1,63 @@ +package org.checkerframework.framework.test.test.junit; + +import org.checkerframework.common.value.ValueChecker; +import org.checkerframework.framework.test.CheckerFrameworkPerFileTest; +import org.checkerframework.framework.test.TestRootDirectory; +import org.checkerframework.framework.test.TypecheckResult; +import org.checkerframework.framework.test.diagnostics.TestDiagnosticUtils; +import org.hamcrest.CoreMatchers; +import org.hamcrest.MatcherAssert; +import org.junit.runners.Parameterized.Parameters; + +import java.io.File; + +/** Tests the explicit tests root configuration. */ +@TestRootDirectory("tests-alt") +public class AlternateTestRootPerFileWithDirsTest extends CheckerFrameworkPerFileTest { + + /** + * @param testFile the files containing test code, which will be type-checked + */ + public AlternateTestRootPerFileWithDirsTest(File testFile) { + super(testFile, ValueChecker.class, ""); + } + + @Parameters + public static String[] getTestDirs() { + return new String[] {"alt-dir-a", "alt-dir-b"}; + } + + @Override + public void checkResult(TypecheckResult typecheckResult) { + super.checkResult(typecheckResult); + MatcherAssert.assertThat( + "test check result has exactly one expected diagnostic", + typecheckResult.getExpectedDiagnostics().size(), + CoreMatchers.describedAs( + "singleton collection %0", + CoreMatchers.is(1), typecheckResult.getExpectedDiagnostics())); + MatcherAssert.assertThat( + "test check result has the expected diagnostic of one of the test files", + typecheckResult.getExpectedDiagnostics(), + CoreMatchers.either( + CoreMatchers.hasItem( + TestDiagnosticUtils.fromJavaFileComment( + "Issue6125A.java", 5, "error: (assignment)"))) + .or( + CoreMatchers.hasItem( + TestDiagnosticUtils.fromJavaFileComment( + "Issue6125B.java", 5, "error: (assignment)")))); + MatcherAssert.assertThat( + "test check result has exactly zero unexpected diagnostics", + typecheckResult.getUnexpectedDiagnostics().size(), + CoreMatchers.describedAs( + "zero length collection %0", + CoreMatchers.is(0), typecheckResult.getUnexpectedDiagnostics())); + MatcherAssert.assertThat( + "test check result has exactly zero missing diagnostics", + typecheckResult.getMissingDiagnostics().size(), + CoreMatchers.describedAs( + "zero length collection %0", + CoreMatchers.is(0), typecheckResult.getMissingDiagnostics())); + } +} diff --git a/framework-test/src/test/java/org/checkerframework/framework/test/test/junit/AlternateTestRootPerFileWithFilesTest.java b/framework-test/src/test/java/org/checkerframework/framework/test/test/junit/AlternateTestRootPerFileWithFilesTest.java new file mode 100644 index 00000000000..a28c0fcc921 --- /dev/null +++ b/framework-test/src/test/java/org/checkerframework/framework/test/test/junit/AlternateTestRootPerFileWithFilesTest.java @@ -0,0 +1,60 @@ +package org.checkerframework.framework.test.test.junit; + +import org.checkerframework.common.value.ValueChecker; +import org.checkerframework.framework.test.CheckerFrameworkPerFileTest; +import org.checkerframework.framework.test.TestRootDirectory; +import org.checkerframework.framework.test.TestUtilities; +import org.checkerframework.framework.test.TypecheckResult; +import org.checkerframework.framework.test.diagnostics.TestDiagnosticUtils; +import org.hamcrest.CoreMatchers; +import org.hamcrest.MatcherAssert; +import org.junit.runners.Parameterized.Parameters; + +import java.io.File; +import java.util.List; + +/** Tests the explicit tests root configuration. */ +@TestRootDirectory("tests-alt") +public class AlternateTestRootPerFileWithFilesTest extends CheckerFrameworkPerFileTest { + + /** + * @param testFile the files containing test code, which will be type-checked + */ + public AlternateTestRootPerFileWithFilesTest(File testFile) { + super(testFile, ValueChecker.class, ""); + } + + @Parameters + public static List getTestFiles() { + return TestUtilities.findRelativeNestedJavaFiles("tests-alt", "alt-dir-a"); + } + + @Override + public void checkResult(TypecheckResult typecheckResult) { + super.checkResult(typecheckResult); + MatcherAssert.assertThat( + "test check result has exactly one expected diagnostic", + typecheckResult.getExpectedDiagnostics().size(), + CoreMatchers.describedAs( + "singleton collection %0", + CoreMatchers.is(1), typecheckResult.getExpectedDiagnostics())); + MatcherAssert.assertThat( + "test check result has the expected diagnostic of the test file", + typecheckResult.getExpectedDiagnostics(), + CoreMatchers.hasItem( + TestDiagnosticUtils.fromJavaFileComment( + "Issue6125A.java", 5, "error: (assignment)"))); + MatcherAssert.assertThat( + "test check result has exactly zero unexpected diagnostics", + typecheckResult.getUnexpectedDiagnostics().size(), + CoreMatchers.describedAs( + "zero length collection %0", + CoreMatchers.is(0), typecheckResult.getUnexpectedDiagnostics())); + MatcherAssert.assertThat( + "test check result has exactly zero missing diagnostics", + typecheckResult.getMissingDiagnostics().size(), + CoreMatchers.describedAs( + "zero length collection %0", + CoreMatchers.is(0), typecheckResult.getMissingDiagnostics())); + } +} diff --git a/framework-test/tests-alt/README b/framework-test/tests-alt/README new file mode 100644 index 00000000000..ac02856cadf --- /dev/null +++ b/framework-test/tests-alt/README @@ -0,0 +1,4 @@ +Contains trivial tests to test annotating tests with @TestRootDirectory. + +Please do not place tests in this directory unless testing @TestRootDirectory. + diff --git a/framework-test/tests-alt/alt-dir-a/Issue6125A.java b/framework-test/tests-alt/alt-dir-a/Issue6125A.java new file mode 100644 index 00000000000..0b22a3e5b6b --- /dev/null +++ b/framework-test/tests-alt/alt-dir-a/Issue6125A.java @@ -0,0 +1,6 @@ +import org.checkerframework.common.value.qual.StringVal; + +public class Issue6125A { + // :: error: (assignment) + @StringVal("hello") String s = "goodbye"; +} diff --git a/framework-test/tests-alt/alt-dir-b/Issue6125B.java b/framework-test/tests-alt/alt-dir-b/Issue6125B.java new file mode 100644 index 00000000000..e57f47dff98 --- /dev/null +++ b/framework-test/tests-alt/alt-dir-b/Issue6125B.java @@ -0,0 +1,6 @@ +import org.checkerframework.common.value.qual.StringVal; + +public class Issue6125B { + // :: error: (assignment) + @StringVal("hello") String s = "world"; +} diff --git a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeChecker.java b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeChecker.java index 348c75c51a9..7969bab9303 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeChecker.java +++ b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeChecker.java @@ -880,16 +880,16 @@ public List getExtraStubFiles() { } @Override - protected Object processArg(Object arg) { + protected Object processErrorMessageArg(Object arg) { if (arg instanceof Collection) { Collection carg = (Collection) arg; - return CollectionsPlume.mapList(this::processArg, carg); + return CollectionsPlume.mapList(this::processErrorMessageArg, carg); } else if (arg instanceof AnnotationMirror && getTypeFactory() != null) { return getTypeFactory() .getAnnotationFormatter() .formatAnnotationMirror((AnnotationMirror) arg); } else { - return super.processArg(arg); + return super.processErrorMessageArg(arg); } } 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 61de45111d8..969a62819c7 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java +++ b/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeVisitor.java @@ -1612,8 +1612,12 @@ && getCurrentPath().getParentPath().getLeaf().getKind() } atypeFactory.getDependentTypesHelper().checkTypeForErrorExpressions(variableType, tree); - // If there's no assignment in this variable declaration, skip it. - if (tree.getInitializer() != null) { + Element varEle = TreeUtils.elementFromDeclaration(tree); + if (varEle.getKind() == ElementKind.ENUM_CONSTANT) { + commonAssignmentCheck( + tree, tree.getInitializer(), "enum.declaration.type.incompatible"); + } else if (tree.getInitializer() != null) { + // If there's no assignment in this variable declaration, skip it. commonAssignmentCheck(tree, tree.getInitializer(), "assignment.type.incompatible"); } else { // commonAssignmentCheck validates the type of `tree`, diff --git a/framework/src/main/java/org/checkerframework/common/basetype/messages.properties b/framework/src/main/java/org/checkerframework/common/basetype/messages.properties index ed00070da71..023231d98d0 100644 --- a/framework/src/main/java/org/checkerframework/common/basetype/messages.properties +++ b/framework/src/main/java/org/checkerframework/common/basetype/messages.properties @@ -5,6 +5,7 @@ type.anno.before.decl.anno=write type annotations %s immediately before type, af array.initializer.type.incompatible=incompatible types in array initializer.%nfound : %s%nrequired: %s assignment.type.incompatible=incompatible types in assignment.%nfound : %s%nrequired: %s +enum.declaration.type.incompatible=incompatible annotations on Enum constant declaration.%nfound : %s%nrequired: %s compound.assignment.type.incompatible=expression type incompatible with left-hand side in compound assignment.%nfound : %s%nrequired: %s unary.increment.type.incompatible=increment result incompatible with variable declared type.%nfound : %s%nrequired: %s unary.decrement.type.incompatible=decrement result incompatible with variable declared type.%nfound : %s%nrequired: %s diff --git a/framework/src/main/java/org/checkerframework/common/reflection/ReflectionResolver.java b/framework/src/main/java/org/checkerframework/common/reflection/ReflectionResolver.java index 022d4bbb5a6..7d9282da79a 100644 --- a/framework/src/main/java/org/checkerframework/common/reflection/ReflectionResolver.java +++ b/framework/src/main/java/org/checkerframework/common/reflection/ReflectionResolver.java @@ -26,7 +26,7 @@ public interface ReflectionResolver { public static final List INIT_LIST = Collections.singletonList(INIT); /** - * Determines whether the given tree represents a reflective method or constructor call. + * Returns true if the given tree represents a reflective method or constructor call. * * @return {@code true} iff tree is a reflective method invocation, {@code false} otherwise */ diff --git a/framework/src/main/java/org/checkerframework/common/util/count/AnnotationStatistics.java b/framework/src/main/java/org/checkerframework/common/util/count/AnnotationStatistics.java index cfc23fc38bf..ae0de6fbf74 100644 --- a/framework/src/main/java/org/checkerframework/common/util/count/AnnotationStatistics.java +++ b/framework/src/main/java/org/checkerframework/common/util/count/AnnotationStatistics.java @@ -34,7 +34,8 @@ import javax.tools.Diagnostic; /** - * An annotation processor for listing the potential locations of annotations. To invoke it, use + * An annotation processor for counting the annotations in a program and for listing the potential + * locations of annotations. To invoke it, use * * 
      * javac -proc:only -processor org.checkerframework.common.util.count.AnnotationStatistics MyFile.java ...
@@ -48,11 +49,12 @@
  *   
  • Count for only certain location types: use {@code grep}
  * 
  *
- * 
    By default, this utility displays annotation locations only. The following two options may be
- * used to adjust the output:
+ * 
    By default, this utility displays annotation locations only. The following options may be used
+ * to adjust the output:
  *
  * 
      
- *   
    • {@code -Aannotations}: prints information about the annotations
+ *   
    • {@code -Aannotations}: prints information about the annotations, such as whether it is in a
+ *       signature or in a body
  *   
    • {@code -Anolocations}: suppresses location output; only makes sense in conjunction with
  *       {@code -Aannotations}
  *   
    • {@code -Aannotationsummaryonly}: with both of the obove, only outputs a summary
diff --git a/framework/src/main/java/org/checkerframework/common/value/ValueMethodIdentifier.java b/framework/src/main/java/org/checkerframework/common/value/ValueMethodIdentifier.java
index 57515795991..262ca839d7c 100644
--- a/framework/src/main/java/org/checkerframework/common/value/ValueMethodIdentifier.java
+++ b/framework/src/main/java/org/checkerframework/common/value/ValueMethodIdentifier.java
@@ -72,13 +72,19 @@ public boolean isMathMax(Tree methodTree, ProcessingEnvironment processingEnv) {
         return TreeUtils.isMethodInvocation(methodTree, mathMaxMethods, processingEnv);
     }
 
-    /** Determines whether a tree is an invocation of the {@code String.length()} method. */
+    /**
+     * Returns true if a tree is an invocation of the {@code String.length()} method.
+     *
+     * @param tree a tree
+     * @param processingEnv the processing environment
+     * @return true if a tree is an invocation of the {@code String.length()} method
+     */
     public boolean isStringLengthInvocation(Tree tree, ProcessingEnvironment processingEnv) {
         return TreeUtils.isMethodInvocation(tree, lengthMethod, processingEnv);
     }
 
     /**
-     * Determines whether a tree is an invocation of the {@code Array.getLength()} method.
+     * Returns true if a tree is an invocation of the {@code Array.getLength()} method.
      *
      * @param tree tree to check
      * @param processingEnv the processing environment
@@ -89,7 +95,7 @@ public boolean isArrayGetLengthInvocation(Tree tree, ProcessingEnvironment proce
     }
 
     /**
-     * Determines whether a method is the {@code String.length()} method.
+     * Returns true if a method is the {@code String.length()} method.
      *
      * @param method the element to check
      * @return true iff the argument methid is {@code String.length()} method
@@ -100,7 +106,7 @@ public boolean isStringLengthMethod(ExecutableElement method) {
     }
 
     /**
-     * Determines whether a method is the {@code Array.getLength()} method.
+     * Returns true if a method is the {@code Array.getLength()} method.
      *
      * @param method the element to check
      * @return true iff the argument method is {@code Array.getLength()} method
@@ -111,7 +117,7 @@ public boolean isArrayGetLengthMethod(ExecutableElement method) {
     }
 
     /**
-     * Determines whether a method is the {@code String.startsWith(String)} method.
+     * Returns true if a method is the {@code String.startsWith(String)} method.
      *
      * @param method the element to check
      * @return true iff the argument method is {@code String.startsWith(String)} method
@@ -122,7 +128,7 @@ public boolean isStartsWithMethod(ExecutableElement method) {
     }
 
     /**
-     * Determines whether a method is the {@code String.endsWith(String)} method.
+     * Returns true if a method is the {@code String.endsWith(String)} method.
      *
      * @param method a method
      * @return true if the method is {@code String.endsWith(String)}
@@ -133,7 +139,7 @@ public boolean isEndsWithMethod(ExecutableElement method) {
     }
 
     /**
-     * Determines whether a tree is an invocation of the {@code Arrays.copyOf()} method.
+     * Returns true if a tree is an invocation of the {@code Arrays.copyOf()} method.
      *
      * @param tree tree to check
      * @param processingEnv the processing environment
diff --git a/framework/src/main/java/org/checkerframework/common/value/util/Range.java b/framework/src/main/java/org/checkerframework/common/value/util/Range.java
index 59d791216b4..2d490e276a5 100644
--- a/framework/src/main/java/org/checkerframework/common/value/util/Range.java
+++ b/framework/src/main/java/org/checkerframework/common/value/util/Range.java
@@ -1199,7 +1199,7 @@ public Range refineNotEqualTo(Range right) {
     }
 
     /**
-     * Determines if the range is wider than a given value, i.e., if the number of possible values
+     * Returns true if the range is wider than a given value, i.e., if the number of possible values
      * enclosed by this range is more than the given value.
      *
      * @param value the value to compare with
@@ -1220,13 +1220,17 @@ public boolean isWiderThan(long value) {
         }
     }
 
-    /** Determines if this range represents a constant value. */
+    /**
+     * Returns true if this range represents a constant value.
+     *
+     * @return true if this range represents a constant value
+     */
     public boolean isConstant() {
         return from == to;
     }
 
     /**
-     * Determines if this range is completely contained in the range specified by the given lower
+     * Returns true if this range is completely contained in the range specified by the given lower
      * bound inclusive and upper bound inclusive.
      *
      * @param lb lower bound for the range that might contain this one
@@ -1239,7 +1243,7 @@ public boolean isWithin(long lb, long ub) {
     }
 
     /**
-     * Determines if this range is contained inclusively between Long.MIN_VALUE/2 and
+     * Returns true if this range is contained inclusively between Long.MIN_VALUE/2 and
      * Long.MAX_VALUE/2. Note: Long.MIN_VALUE/2 != -Long.MAX_VALUE/2
      */
     private boolean isWithinHalfLong() {
@@ -1247,7 +1251,7 @@ private boolean isWithinHalfLong() {
     }
 
     /**
-     * Determines if this range is completely contained in the scope of the Integer type.
+     * Returns true if this range is completely contained in the scope of the Integer type.
      *
      * @return true if the range is contained within the Integer range inclusive
      */
diff --git a/framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceJavaParserStorage.java b/framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceJavaParserStorage.java
index d6ae8008040..c6291477a4c 100644
--- a/framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceJavaParserStorage.java
+++ b/framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceJavaParserStorage.java
@@ -777,21 +777,14 @@ private void addClass(
                             for (TypeElement supertypeElement :
                                     ElementUtils.getSuperTypes(classElt, elements)) {
                                 String supertypeName = ElementUtils.getBinaryName(supertypeElement);
-                                @SuppressWarnings({
-                                    "signature:assignment",
-                                    "signature:return"
-                                }) // #979?
-                                Set supertypeSet =
+                                Set<@BinaryName String> supertypeSet =
                                         supertypesMap.computeIfAbsent(
-                                                className, k -> new TreeSet<>());
+                                                className, k -> new TreeSet<@BinaryName String>());
                                 supertypeSet.add(supertypeName);
-                                @SuppressWarnings({
-                                    "signature:assignment",
-                                    "signature:return"
-                                }) // #979?
-                                Set subtypeSet =
+                                Set<@BinaryName String> subtypeSet =
                                         subtypesMap.computeIfAbsent(
-                                                supertypeName, k -> new TreeSet<>());
+                                                supertypeName,
+                                                k -> new TreeSet<@BinaryName String>());
                                 subtypeSet.add(className);
                             }
                         }
diff --git a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java
index 055ae1bbd19..11186f54159 100644
--- a/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java
+++ b/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java
@@ -464,7 +464,7 @@ public abstract class SourceChecker extends AbstractTypeProcessor implements Opt
 
     /**
      * Maps error keys to localized/custom error messages. Do not use directly; call {@link
-     * #fullMessageOf} or {@link #processArg}. Is set in {@link #initChecker}.
+     * #fullMessageOf} or {@link #processErrorMessageArg}. Is set in {@link #initChecker}.
      */
     protected Properties messagesProperties;
 
@@ -1203,7 +1203,7 @@ private void report(
 
         if (args != null) {
             for (int i = 0; i < args.length; ++i) {
-                args[i] = processArg(args[i]);
+                args[i] = processErrorMessageArg(args[i]);
             }
         }
 
@@ -1437,7 +1437,7 @@ protected String fullMessageOf(String messageKey, String defaultValue) {
      * @param arg the argument
      * @return the result after processing
      */
-    protected Object processArg(Object arg) {
+    protected Object processErrorMessageArg(Object arg) {
         // Check to see if the argument itself is a property to be expanded
         if (arg instanceof String) {
             return messagesProperties.getProperty((String) arg, (String) arg);
@@ -2219,7 +2219,7 @@ private boolean shouldSuppressWarnings(@Nullable Object src, String errKey) {
     }
 
     /**
-     * Determines whether all the warnings pertaining to a given tree should be suppressed. Returns
+     * Returns true if all the warnings pertaining to a given tree should be suppressed. Returns
      * true if the tree is within the scope of a @SuppressWarnings annotation, one of whose values
      * suppresses the checker's warning. Also, returns true if the {@code errKey} matches a string
      * in {@code -AsuppressWarnings}.
@@ -2251,7 +2251,7 @@ public boolean shouldSuppressWarnings(Tree tree, String errKey) {
     }
 
     /**
-     * Determines whether all the warnings pertaining to a given tree path should be suppressed.
+     * Returns true if all the warnings pertaining to a given tree path should be suppressed.
      * Returns true if the path is within the scope of a @SuppressWarnings annotation, one of whose
      * values suppresses the checker's warning.
      *
@@ -2349,9 +2349,9 @@ public boolean useConservativeDefault(String kindOfCode) {
     protected final Set elementsWithSuppressedWarnings = new HashSet<>();
 
     /**
-     * Determines whether all the warnings pertaining to a given element should be suppressed.
-     * Returns true if the element is within the scope of a @SuppressWarnings annotation, one of
-     * whose values suppresses all the checker's warnings.
+     * Returns true if all the warnings pertaining to a given element should be suppressed. Returns
+     * true if the element is within the scope of a @SuppressWarnings annotation, one of whose
+     * values suppresses all the checker's warnings.
      *
      * @param elt the Element that might be a source of, or related to, a warning
      * @param errKey the error key the checker is emitting
@@ -2386,8 +2386,8 @@ public boolean shouldSuppressWarnings(@Nullable Element elt, String errKey) {
     }
 
     /**
-     * Determines whether an error (whose message key is {@code messageKey}) should be suppressed.
-     * It is suppressed if any of the given SuppressWarnings strings suppresses it.
+     * Returns true if an error (whose message key is {@code messageKey}) should be suppressed. It
+     * is suppressed if any of the given SuppressWarnings strings suppresses it.
      *
      * 
      A SuppressWarnings string may be of the following pattern:
      *
diff --git a/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java b/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java
index e01c7829158..1de593e6046 100644
--- a/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java
+++ b/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java
@@ -314,14 +314,14 @@ public class AnnotatedTypeFactory implements AnnotationProvider {
      * Caches the supported type qualifier classes. Call {@link #getSupportedTypeQualifiers()}
      * instead of using this field directly, as it may not have been initialized.
      */
-    private final Set> supportedQuals;
+    private @MonotonicNonNull Set> supportedQuals = null;
 
     /**
      * Caches the fully-qualified names of the classes in {@link #supportedQuals}. Call {@link
      * #getSupportedTypeQualifierNames()} instead of using this field directly, as it may not have
      * been initialized.
      */
-    private final Set<@CanonicalName String> supportedQualNames;
+    private @MonotonicNonNull Set<@CanonicalName String> supportedQualNames = null;
 
     /** Parses stub files and stores annotations on public elements from stub files. */
     public final AnnotationFileElementTypes stubTypes;
@@ -565,8 +565,6 @@ public AnnotatedTypeFactory(BaseTypeChecker checker) {
         this.elements = processingEnv.getElementUtils();
         this.types = processingEnv.getTypeUtils();
 
-        this.supportedQuals = new HashSet<>();
-        this.supportedQualNames = new HashSet<>();
         this.stubTypes = new AnnotationFileElementTypes(this);
         this.ajavaTypes = new AnnotationFileElementTypes(this);
         this.currentFileAjavaTypes = null;
@@ -775,14 +773,14 @@ public AnnotatedTypeFactory(BaseTypeChecker checker) {
     }
 
     /**
-     * Requires that supportedQuals is non-empty and each element is a type qualifier. That is, no
-     * element has a {@code @Target} meta-annotation that contains something besides TYPE_USE or
-     * TYPE_PARAMETER. (@Target({}) is allowed.) @
+     * Requires that supportedQuals is non-null and non-empty and each element is a type qualifier.
+     * That is, no element has a {@code @Target} meta-annotation that contains something besides
+     * TYPE_USE or TYPE_PARAMETER. (@Target({}) is allowed.) @
      *
      * @throws BugInCF If supportedQuals is empty or contaions a non-type qualifier
      */
     private void checkSupportedQualsAreTypeQuals() {
-        if (supportedQuals.isEmpty()) {
+        if (supportedQuals == null || supportedQuals.isEmpty()) {
             throw new TypeSystemError("Found no supported qualifiers.");
         }
         for (Class annotationClass : supportedQuals) {
@@ -1334,11 +1332,11 @@ public AnnotationFormatter getAnnotationFormatter() {
      *     are supported
      */
     public final Set> getSupportedTypeQualifiers() {
-        if (this.supportedQuals.isEmpty()) {
-            supportedQuals.addAll(createSupportedTypeQualifiers());
+        if (this.supportedQuals == null) {
+            supportedQuals = createSupportedTypeQualifiers();
             checkSupportedQualsAreTypeQuals();
         }
-        return Collections.unmodifiableSet(supportedQuals);
+        return supportedQuals;
     }
 
     /**
@@ -1353,12 +1351,14 @@ public final Set> getSupportedTypeQualifiers() {
      *     are supported
      */
     public final Set<@CanonicalName String> getSupportedTypeQualifierNames() {
-        if (this.supportedQualNames.isEmpty()) {
+        if (this.supportedQualNames == null) {
+            supportedQualNames = new HashSet<>();
             for (Class clazz : getSupportedTypeQualifiers()) {
                 supportedQualNames.add(clazz.getCanonicalName());
             }
+            supportedQualNames = Collections.unmodifiableSet(supportedQualNames);
         }
-        return Collections.unmodifiableSet(supportedQualNames);
+        return supportedQualNames;
     }
 
     // **********************************************************************
@@ -2947,15 +2947,32 @@ public ParameterizedExecutableType constructorFromUse(NewClassTree tree) {
             // Reset the enclosing type because it can be substituted incorrectly.
             ((AnnotatedDeclaredType) con.getReturnType()).setEnclosingType(enclosingType);
         }
+
+        if (ctor.getEnclosingElement().getKind() == ElementKind.ENUM) {
+            Set enumAnnos = getEnumConstructorQualifiers();
+            con.getReturnType().replaceAnnotations(enumAnnos);
+        }
+
         // Adapt parameters, which makes parameters and arguments be the same size for later
         // checking. The vararg type of con has been already computed and stored when calling
         // typeVarSubstitutor.substitute.
         List parameters =
                 AnnotatedTypes.adaptParameters(this, con, tree.getArguments(), tree);
         con.setParameterTypes(parameters);
+
         return new ParameterizedExecutableType(con, typeargs);
     }
 
+    /**
+     * Returns the annotations that should be applied to enum constructors. This implementation
+     * returns an empty set. Subclasses can override to return a different set.
+     *
+     * @return the annotations that should be applied to enum constructors
+     */
+    protected Set getEnumConstructorQualifiers() {
+        return Collections.emptySet();
+    }
+
     /**
      * Creates an AnnotatedDeclaredType for a NewClassTree. Only adds explicit annotations, unless
      * newClassTree has a diamond operator. In that case, the annotations on the type arguments are
@@ -3549,7 +3566,7 @@ public final AnnotatedExecutableType fromElement(ExecutableElement elt) {
     // **********************************************************************
 
     /**
-     * Determines whether the given annotation is a part of the type system under which this type
+     * Returns true if the given annotation is a part of the type system under which this type
      * factory operates. Null is never a supported qualifier; the parameter is nullable to allow the
      * result of canonicalAnnotation to be passed in directly.
      *
@@ -3566,7 +3583,7 @@ public boolean isSupportedQualifier(@Nullable AnnotationMirror a) {
     }
 
     /**
-     * Determines whether the given class is a part of the type system under which this type factory
+     * Returns true if the given class is a part of the type system under which this type factory
      * operates.
      *
      * @param clazz annotation class
@@ -3578,7 +3595,7 @@ public boolean isSupportedQualifier(Class clazz) {
     }
 
     /**
-     * Determines whether the given class name is a part of the type system under which this type
+     * Returns true if the given class name is a part of the type system under which this type
      * factory operates.
      *
      * @param className fully-qualified annotation class name
diff --git a/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeMirror.java b/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeMirror.java
index f6e9e3396a3..95960617f61 100644
--- a/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeMirror.java
+++ b/framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeMirror.java
@@ -547,7 +547,7 @@ public boolean hasEffectiveAnnotation(AnnotationMirror a) {
     }
 
     /**
-     * Determines whether this type contains the given annotation explicitly written at declaration.
+     * Returns true if this type contains the given annotation explicitly written at declaration.
      * This method considers the annotation's values. If the type is {@code @A("s") @B(3) Object}, a
      * call with {@code @A("t")} or {@code @A} will return false, whereas a call with {@code @B(3)}
      * will return true.
@@ -603,9 +603,8 @@ public boolean hasExplicitAnnotationRelaxed(AnnotationMirror a) {
     }
 
     /**
-     * Determines whether this type contains an explicitly written annotation with the same
-     * annotation type as a particular annotation. This method does not consider an annotation's
-     * values.
+     * Returns true if this type contains an explicitly written annotation with the same annotation
+     * type as a particular annotation. This method does not consider an annotation's values.
      *
      * 
      See the documentation for {@link #getExplicitAnnotations()} for details on which explicit
      * annotations are not included.
diff --git a/framework/src/main/java/org/checkerframework/framework/type/DefaultTypeHierarchy.java b/framework/src/main/java/org/checkerframework/framework/type/DefaultTypeHierarchy.java
index d26754a463a..12592220cdc 100644
--- a/framework/src/main/java/org/checkerframework/framework/type/DefaultTypeHierarchy.java
+++ b/framework/src/main/java/org/checkerframework/framework/type/DefaultTypeHierarchy.java
@@ -1019,8 +1019,10 @@ public Boolean visitTypevar_Typevar(
             // This should be removed when 979 is fixed.
             // This case happens when the captured type variables should be the same type, but
             // aren't because type argument inference isn't implemented correctly.
-            return isContainedWithinBounds(
-                    subtype, supertype.getLowerBound(), supertype.getUpperBound(), false);
+            if (isContainedWithinBounds(
+                    subtype, supertype.getLowerBound(), supertype.getUpperBound(), false)) {
+                return true;
+            }
         }
 
         if (supertype.getLowerBound().getKind() != TypeKind.NULL) {
diff --git a/framework/src/main/java/org/checkerframework/framework/type/SupertypeFinder.java b/framework/src/main/java/org/checkerframework/framework/type/SupertypeFinder.java
index 9f91cd7b099..0fc1867cf7a 100644
--- a/framework/src/main/java/org/checkerframework/framework/type/SupertypeFinder.java
+++ b/framework/src/main/java/org/checkerframework/framework/type/SupertypeFinder.java
@@ -403,12 +403,7 @@ private AnnotatedDeclaredType createEnumSuperType(
                 // If the type argument of super is the same as the input type
                 if (atypeFactory.types.isSameType(
                         t.getUnderlyingType(), type.getUnderlyingType())) {
-                    AnnotationMirrorSet bounds =
-                            ((AnnotatedDeclaredType) atypeFactory.getAnnotatedType(dt.asElement()))
-                                    .typeArgs
-                                    .get(0)
-                                    .getEffectiveAnnotations();
-                    t.addAnnotations(bounds);
+                    t.addAnnotations(type.primaryAnnotations);
                 }
             }
             adt.addAnnotations(type.getAnnotations());
diff --git a/framework/src/main/java/org/checkerframework/framework/util/AnnotatedTypes.java b/framework/src/main/java/org/checkerframework/framework/util/AnnotatedTypes.java
index 88832851537..a38c078191f 100644
--- a/framework/src/main/java/org/checkerframework/framework/util/AnnotatedTypes.java
+++ b/framework/src/main/java/org/checkerframework/framework/util/AnnotatedTypes.java
@@ -6,7 +6,6 @@
 import com.sun.source.tree.NewClassTree;
 import com.sun.source.tree.Tree;
 import com.sun.tools.javac.code.Attribute;
-import com.sun.tools.javac.code.BoundKind;
 import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.code.Type;
 
@@ -133,7 +132,6 @@ public static  T asSuper(
     public static  T castedAsSuper(
             AnnotatedTypeFactory atypeFactory, AnnotatedTypeMirror subtype, T supertype) {
         Types types = atypeFactory.getProcessingEnv().getTypeUtils();
-        Elements elements = atypeFactory.getProcessingEnv().getElementUtils();
 
         if (subtype.getKind() == TypeKind.NULL) {
             // Make a copy of the supertype so that if supertype is a composite type, the
@@ -149,38 +147,6 @@ public static  T castedAsSuper(
 
         fixUpRawTypes(subtype, asSuperType, supertype, types);
 
-        // if we have a type for enum MyEnum {...}
-        // When the supertype is the declaration of java.lang.Enum, MyEnum values become
-        // Enum.  Where really, we would like an Enum with the annotations from
-        // Enum are transferred to Enum.  That is, if we have a type:
-        // @1 Enum<@2 MyEnum>
-        // asSuper should return:
-        // @1 Enum>
-        if (asSuperType != null
-                && AnnotatedTypes.isEnum(asSuperType)
-                && AnnotatedTypes.isDeclarationOfJavaLangEnum(types, elements, supertype)) {
-            AnnotatedDeclaredType resultAtd = ((AnnotatedDeclaredType) supertype).deepCopy();
-            resultAtd.clearAnnotations();
-            resultAtd.addAnnotations(asSuperType.getAnnotations());
-
-            AnnotatedDeclaredType asSuperAdt = (AnnotatedDeclaredType) asSuperType;
-            if (!resultAtd.getTypeArguments().isEmpty()
-                    && !asSuperAdt.getTypeArguments().isEmpty()) {
-                AnnotatedTypeMirror sourceTypeArg = asSuperAdt.getTypeArguments().get(0);
-                AnnotatedTypeMirror resultTypeArg = resultAtd.getTypeArguments().get(0);
-                resultTypeArg.clearAnnotations();
-                if (resultTypeArg.getKind() == TypeKind.TYPEVAR) {
-                    // Only change the upper bound of a type variable.
-                    AnnotatedTypeVariable resultTypeArgTV = (AnnotatedTypeVariable) resultTypeArg;
-                    resultTypeArgTV.getUpperBound().addAnnotations(sourceTypeArg.getAnnotations());
-                } else {
-                    resultTypeArg.addAnnotations(sourceTypeArg.getEffectiveAnnotations());
-                }
-                @SuppressWarnings("unchecked")
-                T result = (T) resultAtd;
-                return result;
-            }
-        }
         return asSuperType;
     }
 
@@ -1610,7 +1576,7 @@ public static AnnotationMirrorSet glbOfBounds(
      * @return true if the given card is an unbounded wildcard
      */
     public static boolean hasNoExplicitBound(AnnotatedTypeMirror wildcard) {
-        return ((Type.WildcardType) wildcard.getUnderlyingType()).kind == BoundKind.UNBOUND;
+        return TypesUtils.hasNoExplicitBound(wildcard.getUnderlyingType());
     }
 
     /**
@@ -1632,8 +1598,7 @@ public static boolean isExplicitlySuperBounded(AnnotatedWildcardType wildcardTyp
      * @return true if wildcard type is explicitly super bounded
      */
     public static boolean hasExplicitSuperBound(AnnotatedTypeMirror wildcardType) {
-        Type.WildcardType wc = (Type.WildcardType) wildcardType.getUnderlyingType();
-        return wc.isSuperBound() && wc.kind != BoundKind.UNBOUND;
+        return TypesUtils.hasExplicitSuperBound(wildcardType.getUnderlyingType());
     }
 
     /**
@@ -1655,28 +1620,27 @@ public static boolean isExplicitlyExtendsBounded(AnnotatedWildcardType wildcardT
      * @return true if wildcard type is explicitly extends bounded
      */
     public static boolean hasExplicitExtendsBound(AnnotatedTypeMirror wildcardType) {
-        Type.WildcardType wc = (Type.WildcardType) wildcardType.getUnderlyingType();
-        return wc.isExtendsBound() && wc.kind != BoundKind.UNBOUND;
+        return TypesUtils.hasExplicitExtendsBound(wildcardType.getUnderlyingType());
     }
 
     /**
      * Returns true if this type is super bounded or unbounded.
      *
-     * @param wildcardType the wildcard type to test
+     * @param wildcard the wildcard type to test
      * @return true if this type is super bounded or unbounded
      */
-    public static boolean isUnboundedOrSuperBounded(AnnotatedWildcardType wildcardType) {
-        return ((Type.WildcardType) wildcardType.getUnderlyingType()).isSuperBound();
+    public static boolean isUnboundedOrSuperBounded(AnnotatedWildcardType wildcard) {
+        return TypesUtils.isUnboundedOrSuperBounded(wildcard.getUnderlyingType());
     }
 
     /**
      * Returns true if this type is extends bounded or unbounded.
      *
-     * @param wildcardType the wildcard type to test
+     * @param wildcard the wildcard type to test
      * @return true if this type is extends bounded or unbounded
      */
-    public static boolean isUnboundedOrExtendsBounded(AnnotatedWildcardType wildcardType) {
-        return ((Type.WildcardType) wildcardType.getUnderlyingType()).isExtendsBound();
+    public static boolean isUnboundedOrExtendsBounded(AnnotatedWildcardType wildcard) {
+        return TypesUtils.isUnboundedOrExtendsBounded(wildcard.getUnderlyingType());
     }
 
     /**
diff --git a/framework/src/main/java/org/checkerframework/framework/util/DefaultQualifierKindHierarchy.java b/framework/src/main/java/org/checkerframework/framework/util/DefaultQualifierKindHierarchy.java
index 34f21de3e4f..ccb58e04cd7 100644
--- a/framework/src/main/java/org/checkerframework/framework/util/DefaultQualifierKindHierarchy.java
+++ b/framework/src/main/java/org/checkerframework/framework/util/DefaultQualifierKindHierarchy.java
@@ -290,7 +290,8 @@ protected Map> createDirectSuper
                 DefaultQualifierKind superQualifier = nameToQualifierKind.get(superName);
                 if (superQualifier == null) {
                     throw new TypeSystemError(
-                            "%s @Subtype argument %s isn't in the hierarchy. Qualifiers: [%s]",
+                            "In %s, @SubtypeOf(%s) argument isn't in the hierarchy."
+                                    + " Have you mis-defined createSupportedTypeQualifiers()? Qualifiers: [%s]",
                             qualifierKind, superName, StringsPlume.join(", ", qualifierKinds));
                 }
                 directSupers.add(superQualifier);
diff --git a/framework/src/main/java/org/checkerframework/framework/util/Heuristics.java b/framework/src/main/java/org/checkerframework/framework/util/Heuristics.java
index c3db5fc9244..001d5c68689 100644
--- a/framework/src/main/java/org/checkerframework/framework/util/Heuristics.java
+++ b/framework/src/main/java/org/checkerframework/framework/util/Heuristics.java
@@ -24,7 +24,7 @@
 public class Heuristics {
 
     /**
-     * Determines whether a tree has a particular set of direct parents, ignoring blocks and
+     * Returns true if a tree has a particular set of direct parents, ignoring blocks and
      * parentheses.
      *
      * 
      For example, to test whether an expression (specified by {@code path}) is immediately
diff --git a/framework/tests/all-systems/Issue3791.java b/framework/tests/all-systems/Issue3791.java
index cb4768bc750..e66066f8b12 100644
--- a/framework/tests/all-systems/Issue3791.java
+++ b/framework/tests/all-systems/Issue3791.java
@@ -11,7 +11,8 @@ abstract static class MyInterfaceMyClass param) {
         // TODO: Should we open an issue for this?
-        // See code is reject by Eclipse and should be reject by javac.
+        // This code is reject by Eclipse and should be reject by javac.
+        // See the javac bug report https://bugs.openjdk.org/browse/JDK-8265255.
         @SuppressWarnings({"unchecked", "type.argument"})
         MyInterfaceMyClass> local = (MyInterfaceMyClass>) param;
     }
diff --git a/framework/tests/value/DoubleRounding.java b/framework/tests/value/DoubleRounding.java
new file mode 100644
index 00000000000..1baff0c41ac
--- /dev/null
+++ b/framework/tests/value/DoubleRounding.java
@@ -0,0 +1,11 @@
+// Test case for https://github.com/typetools/checker-framework/issues/6141 .
+// @skip-test until the bug is fixed
+
+public class DoubleRounding {
+
+    final double FLOATING_POINT_DELTA = 1e-15;
+
+    void round() {
+        float f = (float) FLOATING_POINT_DELTA;
+    }
+}
diff --git a/javacutil/src/main/java/org/checkerframework/javacutil/TreeUtils.java b/javacutil/src/main/java/org/checkerframework/javacutil/TreeUtils.java
index 9e729f35a6b..911d2a9b25b 100644
--- a/javacutil/src/main/java/org/checkerframework/javacutil/TreeUtils.java
+++ b/javacutil/src/main/java/org/checkerframework/javacutil/TreeUtils.java
@@ -1767,8 +1767,8 @@ && getFieldName(tree).equals("length")) {
     }
 
     /**
-     * Determines whether or not the given {@link MethodTree} is an anonymous constructor (the
-     * constructor for an anonymous class).
+     * Returns true if the given {@link MethodTree} is an anonymous constructor (the constructor for
+     * an anonymous class).
      *
      * @param method a method tree that may be an anonymous constructor
      * @return true if the given path points to an anonymous constructor, false if it does not
diff --git a/javacutil/src/main/java/org/checkerframework/javacutil/TypesUtils.java b/javacutil/src/main/java/org/checkerframework/javacutil/TypesUtils.java
index 66c303b16c0..2037b5182b5 100644
--- a/javacutil/src/main/java/org/checkerframework/javacutil/TypesUtils.java
+++ b/javacutil/src/main/java/org/checkerframework/javacutil/TypesUtils.java
@@ -1,5 +1,6 @@
 package org.checkerframework.javacutil;
 
+import com.sun.tools.javac.code.BoundKind;
 import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.code.Symtab;
 import com.sun.tools.javac.code.Type;
@@ -728,8 +729,67 @@ public static Type wildLowerBound(TypeMirror tm, ProcessingEnvironment env) {
         return effectiveUpper;
     }
 
+    // For Wildcards, isSuperBound() and isExtendsBound() will return true if isUnbound() does.
+    // But don't use isUnbound(), because as of Java 18, it returns true for "? extends Object".
+
+    /**
+     * Returns true if {@code type} is an unbounded wildcard.
+     *
+     * @param type the type to check
+     * @return true if the given type is an unbounded wildcard
+     */
+    public static boolean hasNoExplicitBound(TypeMirror type) {
+        return type.getKind() == TypeKind.WILDCARD
+                && ((Type.WildcardType) type).kind == BoundKind.UNBOUND;
+    }
+
+    /**
+     * Returns true if {@code type} is a wildcard with an explicit super bound.
+     *
+     * @param type the {@code type} to test
+     * @return true if {@code type} is explicitly super bounded
+     */
+    public static boolean hasExplicitSuperBound(TypeMirror type) {
+        return type.getKind() == TypeKind.WILDCARD
+                && !hasNoExplicitBound(type)
+                && ((Type.WildcardType) type).isSuperBound();
+    }
+
+    /**
+     * Returns true if {@code type} is a wildcard with an explicit extends bound.
+     *
+     * @param type the type to test
+     * @return true if {@code type} is a wildcard with an explicit extends bound
+     */
+    public static boolean hasExplicitExtendsBound(TypeMirror type) {
+        return type.getKind() == TypeKind.WILDCARD
+                && !hasNoExplicitBound(type)
+                && ((Type.WildcardType) type).isExtendsBound();
+    }
+
+    /**
+     * Returns true if this type is super bounded or unbounded.
+     *
+     * @param wildcardType the wildcard type to test
+     * @return true if this type is super bounded or unbounded
+     */
+    public static boolean isUnboundedOrSuperBounded(WildcardType wildcardType) {
+        return ((Type.WildcardType) wildcardType).isSuperBound();
+    }
+
+    /**
+     * Returns true if this type is extends bounded or unbounded.
+     *
+     * @param wildcardType the wildcard type to test
+     * @return true if this type is extends bounded or unbounded
+     */
+    public static boolean isUnboundedOrExtendsBounded(WildcardType wildcardType) {
+        return ((Type.WildcardType) wildcardType).isExtendsBound();
+    }
+
     /**
-     * Returns true if the erased type of subtype is a subtype of the erased type of supertype.
+     * Returns true if the erased type of {@code subtype} is a subtype of the erased type of {@code
+     * supertype}.
      *
      * @param subtype possible subtype
      * @param supertype possible supertype
diff --git a/javacutil/src/main/java/org/checkerframework/javacutil/trees/TreeBuilder.java b/javacutil/src/main/java/org/checkerframework/javacutil/trees/TreeBuilder.java
index c2ebdf8dddf..d25a83c06c0 100644
--- a/javacutil/src/main/java/org/checkerframework/javacutil/trees/TreeBuilder.java
+++ b/javacutil/src/main/java/org/checkerframework/javacutil/trees/TreeBuilder.java
@@ -24,6 +24,7 @@
 import com.sun.tools.javac.util.Context;
 import com.sun.tools.javac.util.Names;
 
+import org.checkerframework.javacutil.BugInCF;
 import org.checkerframework.javacutil.TreeUtils;
 import org.checkerframework.javacutil.TypesUtils;
 import org.plumelib.util.CollectionsPlume;
@@ -151,11 +152,13 @@ public MemberSelectTree buildHasNextMethodAccess(ExpressionTree iteratorExpr) {
             if (method.getParameters().isEmpty()
                     && method.getSimpleName().contentEquals("hasNext")) {
                 hasNextMethod = (Symbol.MethodSymbol) method;
+                break;
             }
         }
 
-        assert hasNextMethod != null
-                : "@AssumeAssertion(nullness): no hasNext method declared for expression type";
+        if (hasNextMethod == null) {
+            throw new BugInCF("no hasNext method declared for " + exprElement);
+        }
 
         JCTree.JCFieldAccess hasNextAccess = TreeUtils.Select(maker, iteratorExpr, hasNextMethod);
         hasNextAccess.setType(hasNextMethod.asType());

From 4877eaf7a713ed363aa3da11db765c8fed63bf29 Mon Sep 17 00:00:00 2001
From: Werner Dietl 
Date: Mon, 16 Oct 2023 11:44:15 -0400
Subject: [PATCH 2/7] Use EISOP links

---
 docs/developer/developer-manual.html | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/docs/developer/developer-manual.html b/docs/developer/developer-manual.html
index 994cbe65ce9..ecb22571d8d 100644
--- a/docs/developer/developer-manual.html
+++ b/docs/developer/developer-manual.html
@@ -533,9 +533,9 @@ 
      Counting annotations
      
 
      
 
 
 
 

From ab301dc82ac5586ddac8e07cd0acc7c265da438d Mon Sep 17 00:00:00 2001
From: Werner Dietl 
Date: Mon, 16 Oct 2023 11:44:43 -0400
Subject: [PATCH 3/7] Expand suppression key

---
 .../checkerframework/checker/signedness/SignednessShifts.java   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/checker/src/main/java/org/checkerframework/checker/signedness/SignednessShifts.java b/checker/src/main/java/org/checkerframework/checker/signedness/SignednessShifts.java
index 19206e8f557..69758906971 100644
--- a/checker/src/main/java/org/checkerframework/checker/signedness/SignednessShifts.java
+++ b/checker/src/main/java/org/checkerframework/checker/signedness/SignednessShifts.java
@@ -234,7 +234,7 @@ private static boolean castIgnoresMSB(
         // enclosing immediately contains shiftExpr or a parenthesized version of shiftExpr
         Tree enclosing = enclosingPair.first;
         // enclosingChild is a child of enclosing:  shiftExpr or a parenthesized version of it.
-        @SuppressWarnings("interning:assignment") // comparing AST nodes
+        @SuppressWarnings("interning:assignment.type.incompatible") // comparing AST nodes
         @InternedDistinct Tree enclosingChild = enclosingPair.second;
 
         if (!isMask(enclosing)) {

From bafee37ad3c9b1b0bf0b28a946ff897881e32883 Mon Sep 17 00:00:00 2001
From: Werner Dietl 
Date: Mon, 16 Oct 2023 11:45:30 -0400
Subject: [PATCH 4/7] Fix sentence

---
 .../framework/test/CheckerFrameworkRootedTest.java              | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkRootedTest.java b/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkRootedTest.java
index 53e974d7dfa..8d2f358a21f 100644
--- a/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkRootedTest.java
+++ b/framework-test/src/main/java/org/checkerframework/framework/test/CheckerFrameworkRootedTest.java
@@ -5,7 +5,7 @@
 /** Encapsulates the directory root to search within for test files to compile. */
 abstract class CheckerFrameworkRootedTest {
 
-    /** Constructs a test that will assert that can resolve its tests root directory. */
+    /** Constructs a test that will assert that it can resolve its test root directory. */
     public CheckerFrameworkRootedTest() {}
 
     /**

From a596b468c3fe26e89d3485884bb3173ffbd71949 Mon Sep 17 00:00:00 2001
From: Werner Dietl 
Date: Mon, 16 Oct 2023 11:46:07 -0400
Subject: [PATCH 5/7] Uppercase enum looks odd in error message

---
 .../org/checkerframework/common/basetype/messages.properties    | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/framework/src/main/java/org/checkerframework/common/basetype/messages.properties b/framework/src/main/java/org/checkerframework/common/basetype/messages.properties
index 023231d98d0..7e07ed8e7f3 100644
--- a/framework/src/main/java/org/checkerframework/common/basetype/messages.properties
+++ b/framework/src/main/java/org/checkerframework/common/basetype/messages.properties
@@ -5,7 +5,7 @@ type.anno.before.decl.anno=write type annotations %s immediately before type, af
 
 array.initializer.type.incompatible=incompatible types in array initializer.%nfound   : %s%nrequired: %s
 assignment.type.incompatible=incompatible types in assignment.%nfound   : %s%nrequired: %s
-enum.declaration.type.incompatible=incompatible annotations on Enum constant declaration.%nfound   : %s%nrequired: %s
+enum.declaration.type.incompatible=incompatible annotations on enum constant declaration.%nfound   : %s%nrequired: %s
 compound.assignment.type.incompatible=expression type incompatible with left-hand side in compound assignment.%nfound   : %s%nrequired: %s
 unary.increment.type.incompatible=increment result incompatible with variable declared type.%nfound   : %s%nrequired: %s
 unary.decrement.type.incompatible=decrement result incompatible with variable declared type.%nfound   : %s%nrequired: %s

From 753e6872cf9d47c03293b66ce7add3fd87b0f978 Mon Sep 17 00:00:00 2001
From: Werner Dietl 
Date: Mon, 16 Oct 2023 11:47:03 -0400
Subject: [PATCH 6/7] Make parameter names consistent

---
 .../framework/util/AnnotatedTypes.java        | 26 +++++++++----------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/framework/src/main/java/org/checkerframework/framework/util/AnnotatedTypes.java b/framework/src/main/java/org/checkerframework/framework/util/AnnotatedTypes.java
index a38c078191f..c756feffb1e 100644
--- a/framework/src/main/java/org/checkerframework/framework/util/AnnotatedTypes.java
+++ b/framework/src/main/java/org/checkerframework/framework/util/AnnotatedTypes.java
@@ -1572,11 +1572,11 @@ public static AnnotationMirrorSet glbOfBounds(
     /**
      * This method identifies wildcard types that are unbound.
      *
-     * @param wildcard the type to check
+     * @param wildcardType the type to check
      * @return true if the given card is an unbounded wildcard
      */
-    public static boolean hasNoExplicitBound(AnnotatedTypeMirror wildcard) {
-        return TypesUtils.hasNoExplicitBound(wildcard.getUnderlyingType());
+    public static boolean hasNoExplicitBound(AnnotatedTypeMirror wildcardType) {
+        return TypesUtils.hasNoExplicitBound(wildcardType.getUnderlyingType());
     }
 
     /**
@@ -1626,21 +1626,21 @@ public static boolean hasExplicitExtendsBound(AnnotatedTypeMirror wildcardType)
     /**
      * Returns true if this type is super bounded or unbounded.
      *
-     * @param wildcard the wildcard type to test
+     * @param wildcardType the wildcard type to test
      * @return true if this type is super bounded or unbounded
      */
-    public static boolean isUnboundedOrSuperBounded(AnnotatedWildcardType wildcard) {
-        return TypesUtils.isUnboundedOrSuperBounded(wildcard.getUnderlyingType());
+    public static boolean isUnboundedOrSuperBounded(AnnotatedWildcardType wildcardType) {
+        return TypesUtils.isUnboundedOrSuperBounded(wildcardType.getUnderlyingType());
     }
 
     /**
      * Returns true if this type is extends bounded or unbounded.
      *
-     * @param wildcard the wildcard type to test
+     * @param wildcardType the wildcard type to test
      * @return true if this type is extends bounded or unbounded
      */
-    public static boolean isUnboundedOrExtendsBounded(AnnotatedWildcardType wildcard) {
-        return TypesUtils.isUnboundedOrExtendsBounded(wildcard.getUnderlyingType());
+    public static boolean isUnboundedOrExtendsBounded(AnnotatedWildcardType wildcardType) {
+        return TypesUtils.isUnboundedOrExtendsBounded(wildcardType.getUnderlyingType());
     }
 
     /**
@@ -1651,16 +1651,16 @@ public static boolean isUnboundedOrExtendsBounded(AnnotatedWildcardType wildcard
      *
      * @param atypeFactory type factory
      * @param returnType return type to copy annotations to
-     * @param constructor the ATM for the constructor
+     * @param constructorType the ATM for the constructor
      */
     public static void copyOnlyExplicitConstructorAnnotations(
             AnnotatedTypeFactory atypeFactory,
             AnnotatedDeclaredType returnType,
-            AnnotatedExecutableType constructor) {
+            AnnotatedExecutableType constructorType) {
 
         // TODO: There will be a nicer way to access this in 308 soon.
         List decall =
-                ((Symbol) constructor.getElement()).getRawTypeAttributes();
+                ((Symbol) constructorType.getElement()).getRawTypeAttributes();
         AnnotationMirrorSet decret = new AnnotationMirrorSet();
         for (Attribute.TypeCompound da : decall) {
             if (da.position.type == com.sun.tools.javac.code.TargetType.METHOD_RETURN) {
@@ -1678,7 +1678,7 @@ public static void copyOnlyExplicitConstructorAnnotations(
             }
         }
 
-        for (AnnotationMirror cta : constructor.getReturnType().getAnnotations()) {
+        for (AnnotationMirror cta : constructorType.getReturnType().getAnnotations()) {
             AnnotationMirror ctatop = qualHierarchy.getTopAnnotation(cta);
             if (returnType.hasAnnotationInHierarchy(cta)) {
                 continue;

From 5cd155d257378e4088751b26378135599dbbda86 Mon Sep 17 00:00:00 2001
From: Werner Dietl 
Date: Mon, 16 Oct 2023 11:47:30 -0400
Subject: [PATCH 7/7] Use consistent error key

---
 .../test/test/junit/AlternateTestRootPerDirTest.java      | 8 ++++++--
 .../test/junit/AlternateTestRootPerFileWithDirsTest.java  | 8 ++++++--
 .../test/junit/AlternateTestRootPerFileWithFilesTest.java | 2 +-
 framework-test/tests-alt/alt-dir-a/Issue6125A.java        | 2 +-
 framework-test/tests-alt/alt-dir-b/Issue6125B.java        | 2 +-
 5 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/framework-test/src/test/java/org/checkerframework/framework/test/test/junit/AlternateTestRootPerDirTest.java b/framework-test/src/test/java/org/checkerframework/framework/test/test/junit/AlternateTestRootPerDirTest.java
index 08d5e7afc02..0c6fd0a7778 100644
--- a/framework-test/src/test/java/org/checkerframework/framework/test/test/junit/AlternateTestRootPerDirTest.java
+++ b/framework-test/src/test/java/org/checkerframework/framework/test/test/junit/AlternateTestRootPerDirTest.java
@@ -43,11 +43,15 @@ public void checkResult(TypecheckResult typecheckResult) {
                 CoreMatchers.either(
                                 CoreMatchers.hasItem(
                                         TestDiagnosticUtils.fromJavaFileComment(
-                                                "Issue6125A.java", 5, "error: (assignment)")))
+                                                "Issue6125A.java",
+                                                5,
+                                                "error: (assignment.type.incompatible)")))
                         .or(
                                 CoreMatchers.hasItem(
                                         TestDiagnosticUtils.fromJavaFileComment(
-                                                "Issue6125B.java", 5, "error: (assignment)"))));
+                                                "Issue6125B.java",
+                                                5,
+                                                "error: (assignment.type.incompatible)"))));
         MatcherAssert.assertThat(
                 "test check result has exactly zero unexpected diagnostics",
                 typecheckResult.getUnexpectedDiagnostics().size(),
diff --git a/framework-test/src/test/java/org/checkerframework/framework/test/test/junit/AlternateTestRootPerFileWithDirsTest.java b/framework-test/src/test/java/org/checkerframework/framework/test/test/junit/AlternateTestRootPerFileWithDirsTest.java
index 14cc945628a..26c45913faa 100644
--- a/framework-test/src/test/java/org/checkerframework/framework/test/test/junit/AlternateTestRootPerFileWithDirsTest.java
+++ b/framework-test/src/test/java/org/checkerframework/framework/test/test/junit/AlternateTestRootPerFileWithDirsTest.java
@@ -42,11 +42,15 @@ public void checkResult(TypecheckResult typecheckResult) {
                 CoreMatchers.either(
                                 CoreMatchers.hasItem(
                                         TestDiagnosticUtils.fromJavaFileComment(
-                                                "Issue6125A.java", 5, "error: (assignment)")))
+                                                "Issue6125A.java",
+                                                5,
+                                                "error: (assignment.type.incompatible)")))
                         .or(
                                 CoreMatchers.hasItem(
                                         TestDiagnosticUtils.fromJavaFileComment(
-                                                "Issue6125B.java", 5, "error: (assignment)"))));
+                                                "Issue6125B.java",
+                                                5,
+                                                "error: (assignment.type.incompatible)"))));
         MatcherAssert.assertThat(
                 "test check result has exactly zero unexpected diagnostics",
                 typecheckResult.getUnexpectedDiagnostics().size(),
diff --git a/framework-test/src/test/java/org/checkerframework/framework/test/test/junit/AlternateTestRootPerFileWithFilesTest.java b/framework-test/src/test/java/org/checkerframework/framework/test/test/junit/AlternateTestRootPerFileWithFilesTest.java
index a28c0fcc921..c85dc0bcd54 100644
--- a/framework-test/src/test/java/org/checkerframework/framework/test/test/junit/AlternateTestRootPerFileWithFilesTest.java
+++ b/framework-test/src/test/java/org/checkerframework/framework/test/test/junit/AlternateTestRootPerFileWithFilesTest.java
@@ -43,7 +43,7 @@ public void checkResult(TypecheckResult typecheckResult) {
                 typecheckResult.getExpectedDiagnostics(),
                 CoreMatchers.hasItem(
                         TestDiagnosticUtils.fromJavaFileComment(
-                                "Issue6125A.java", 5, "error: (assignment)")));
+                                "Issue6125A.java", 5, "error: (assignment.type.incompatible)")));
         MatcherAssert.assertThat(
                 "test check result has exactly zero unexpected diagnostics",
                 typecheckResult.getUnexpectedDiagnostics().size(),
diff --git a/framework-test/tests-alt/alt-dir-a/Issue6125A.java b/framework-test/tests-alt/alt-dir-a/Issue6125A.java
index 0b22a3e5b6b..b1df9696a06 100644
--- a/framework-test/tests-alt/alt-dir-a/Issue6125A.java
+++ b/framework-test/tests-alt/alt-dir-a/Issue6125A.java
@@ -1,6 +1,6 @@
 import org.checkerframework.common.value.qual.StringVal;
 
 public class Issue6125A {
-    // :: error: (assignment)
+    // :: error: (assignment.type.incompatible)
     @StringVal("hello") String s = "goodbye";
 }
diff --git a/framework-test/tests-alt/alt-dir-b/Issue6125B.java b/framework-test/tests-alt/alt-dir-b/Issue6125B.java
index e57f47dff98..0a7aa611b5b 100644
--- a/framework-test/tests-alt/alt-dir-b/Issue6125B.java
+++ b/framework-test/tests-alt/alt-dir-b/Issue6125B.java
@@ -1,6 +1,6 @@
 import org.checkerframework.common.value.qual.StringVal;
 
 public class Issue6125B {
-    // :: error: (assignment)
+    // :: error: (assignment.type.incompatible)
     @StringVal("hello") String s = "world";
 }