JDK 16 compatibility (#4677)

Author: neilccbrown

.gitignore

@@ -125,6 +125,8 @@ checker/tests/nullness-extra/*.class
 checker/tests/nullness-extra/compat/Out.txt
 checker/tests/nullness-extra/compat/javax/annotation/Nullable.class
 checker/tests/nullness-extra/compat/lib/Lib.class
+checker/tests/nullness-extra/issue3597/testpkg/Issue3597A.class
+checker/tests/nullness-extra/issue3597/testpkg/Issue3597B.class
 checker/tests/nullness-extra/issue502/Issue502.class
 checker/tests/nullness-extra/issue502/Out.txt
 checker/tests/nullness-extra/issue594/Out.txt

azure-pipelines.yml

@@ -34,6 +34,21 @@ jobs:
     fetchDepth: 25
   - bash: ./checker/bin-devel/test-cftests-junit.sh
     displayName: test-cftests-junit.sh
+- job: junit_tests_jdk16
+  dependsOn:
+   - junit_tests_jdk11
+   - nonjunit_tests_jdk11
+   - misc_jdk11
+   - typecheck_jdk11
+  pool:
+    vmImage: 'ubuntu-latest'
+  container: mdernst/cf-ubuntu-jdk16:latest
+  timeoutInMinutes: 70
+  steps:
+  - checkout: self
+    fetchDepth: 25
+  - bash: ./checker/bin-devel/test-cftests-junit.sh
+    displayName: test-cftests-junit.sh
 - job: nonjunit_tests_jdk8
   dependsOn:
    - junit_tests_jdk11
@@ -57,6 +72,20 @@ jobs:
     fetchDepth: 25
   - bash: ./checker/bin-devel/test-cftests-nonjunit.sh
     displayName: test-cftests-nonjunit.sh
+- job: nonjunit_tests_jdk16
+  dependsOn:
+   - junit_tests_jdk11
+   - nonjunit_tests_jdk11
+   - misc_jdk11
+   - typecheck_jdk11
+  pool:
+    vmImage: 'ubuntu-latest'
+  container: mdernst/cf-ubuntu-jdk16:latest
+  steps:
+  - checkout: self
+    fetchDepth: 25
+  - bash: ./checker/bin-devel/test-cftests-nonjunit.sh
+    displayName: test-cftests-nonjunit.sh
 - job: inference_tests_jdk8
   dependsOn:
    - junit_tests_jdk11
@@ -80,6 +109,20 @@ jobs:
     fetchDepth: 25
   - bash: ./checker/bin-devel/test-cftests-inference.sh
     displayName: test-cftests-inference.sh
+- job: inference_tests_jdk16
+  dependsOn:
+   - junit_tests_jdk11
+   - inference_tests_jdk11
+   - misc_jdk11
+   - typecheck_jdk11
+  pool:
+    vmImage: 'ubuntu-latest'
+  container: mdernst/cf-ubuntu-jdk16:latest
+  steps:
+  - checkout: self
+    fetchDepth: 25
+  - bash: ./checker/bin-devel/test-cftests-inference.sh
+    displayName: test-cftests-inference.sh
 - job: misc_jdk8
   ## The dependsOn is commented out because misc_jdk8 sometimes fails when misc_jdk11 does not.
   # dependsOn:
@@ -104,6 +147,20 @@ jobs:
     fetchDepth: 1000
   - bash: ./checker/bin-devel/test-misc.sh
     displayName: test-misc.sh
+- job: misc_jdk16
+  dependsOn:
+   - junit_tests_jdk11
+   - nonjunit_tests_jdk11
+   - misc_jdk11
+   - typecheck_jdk11
+  pool:
+    vmImage: 'ubuntu-latest'
+  container: mdernst/cf-ubuntu-jdk16-plus:latest
+  steps:
+  - checkout: self
+    fetchDepth: 1000
+  - bash: ./checker/bin-devel/test-misc.sh
+    displayName: test-misc.sh
 - job: typecheck_jdk8
   dependsOn:
    - junit_tests_jdk11
@@ -127,6 +184,20 @@ jobs:
     fetchDepth: 1000
   - bash: ./checker/bin-devel/test-typecheck.sh
     displayName: test-typecheck.sh
+- job: typecheck_jdk16
+  dependsOn:
+   - junit_tests_jdk11
+   - nonjunit_tests_jdk11
+   - misc_jdk11
+   - typecheck_jdk11
+  pool:
+    vmImage: 'ubuntu-latest'
+  container: mdernst/cf-ubuntu-jdk16-plus:latest
+  steps:
+  - checkout: self
+    fetchDepth: 1000
+  - bash: ./checker/bin-devel/test-typecheck.sh
+    displayName: test-typecheck.sh
 - job: daikon_jdk8
   dependsOn:
    - junit_tests_jdk11
@@ -154,6 +225,24 @@ jobs:
     fetchDepth: 25
   - bash: ./checker/bin-devel/test-daikon.sh
     displayName: test-daikon.sh
+## Daikon does not yet support JDK 16.  TODO: Make Daikon run under JDK 16.
+# - job: daikon_jdk16
+#   dependsOn:
+#    - junit_tests_jdk11
+#    - nonjunit_tests_jdk11
+#    - misc_jdk11
+#    - typecheck_jdk11
+#   #  ## Commented to reduce latency and eliminate the "daikon_jdk11 -> daikon_jdk16" critical path.
+#   #  # - daikon_jdk11
+#   pool:
+#     vmImage: 'ubuntu-latest'
+#   container: mdernst/cf-ubuntu-jdk16:latest
+#   timeoutInMinutes: 70
+#   steps:
+#   - checkout: self
+#     fetchDepth: 25
+#   - bash: ./checker/bin-devel/test-daikon.sh
+#     displayName: test-daikon.sh
 - job: guava_jdk8
   dependsOn:
    - junit_tests_jdk11
@@ -178,6 +267,21 @@ jobs:
     fetchDepth: 25
   - bash: ./checker/bin-devel/test-guava.sh
     displayName: test-guava.sh
+- job: guava_jdk16
+  dependsOn:
+   - junit_tests_jdk11
+   - nonjunit_tests_jdk11
+   - misc_jdk11
+   - typecheck_jdk11
+   - guava_jdk11
+  pool:
+    vmImage: 'ubuntu-latest'
+  container: mdernst/cf-ubuntu-jdk16:latest
+  steps:
+  - checkout: self
+    fetchDepth: 25
+  - bash: ./checker/bin-devel/test-guava.sh
+    displayName: test-guava.sh
 - job: plume_lib_jdk8
   dependsOn:
    - junit_tests_jdk11
@@ -202,6 +306,22 @@ jobs:
     fetchDepth: 25
   - bash: ./checker/bin-devel/test-plume-lib.sh
     displayName: test-plume-lib.sh
+- job: plume_lib_jdk16
+  dependsOn:
+   - junit_tests_jdk11
+   - nonjunit_tests_jdk11
+   - misc_jdk11
+   - typecheck_jdk11
+   - plume_lib_jdk11
+  pool:
+    vmImage: 'ubuntu-latest'
+  container: mdernst/cf-ubuntu-jdk16:latest
+  steps:
+  - checkout: self
+    fetchDepth: 25
+  - bash: ./checker/bin-devel/test-plume-lib.sh
+    displayName: test-plume-lib.sh
+## The downstream jobs are not currently needed because test-downstream.sh is empty.
 # - job: downstream_jdk8
 #   dependsOn:
 #    - junit_tests_jdk11
@@ -226,3 +346,18 @@ jobs:
 #     fetchDepth: 25
 #   - bash: ./checker/bin-devel/test-downstream.sh
 #     displayName: test-downstream.sh
+# - job: downstream_jdk16
+#   dependsOn:
+#    - junit_tests_jdk11
+#    - nonjunit_tests_jdk11
+#    - misc_jdk11
+#    - typecheck_jdk11
+#    - downstream_jdk11
+#   pool:
+#     vmImage: 'ubuntu-latest'
+#   container: mdernst/cf-ubuntu-jdk16:latest
+#   steps:
+#   - checkout: self
+#     fetchDepth: 25
+#   - bash: ./checker/bin-devel/test-downstream.sh
+#     displayName: test-downstream.sh

build.gradle

@@ -26,6 +26,8 @@ ext {
     // On a Java 9+ JVM, use the host javac, default source/target, and required module flags.
     isJava8 = JavaVersion.current() == JavaVersion.VERSION_1_8
 
+    isJava16 = JavaVersion.current() == JavaVersion.VERSION_16
+
     errorproneJavacVersion = '9+181-r4173-1'
 
     parentDir = file("${rootDir}/../").absolutePath
@@ -58,6 +60,7 @@ switch (JavaVersion.current()) {
         break;
     case JavaVersion.VERSION_1_8:
     case JavaVersion.VERSION_11:
+    case JavaVersion.VERSION_16:
         break; // Supported versions
     default:
         throw new GradleException("Build the Checker Framework with JDK 8 or JDK 11." +
@@ -354,8 +357,18 @@ def createCheckTypeTask(projectName, taskName, checker, args = []) {
         } else {
             options.fork = true
             options.forkOptions.jvmArgs += [
+                // These are required in Java 16+ because the --illegal-access option is set to deny
+                // by default.  None of these packages are accessed via reflection, so the module
+                // only needs to be exported, but not opened.
+                "--add-exports", "jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED",
+                "--add-exports", "jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED",
+                "--add-exports", "jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED",
+                "--add-exports", "jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED",
+                "--add-exports", "jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
+                "--add-exports", "jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED",
+                // Required because the Checker Framework reflectively accesses private members in com.sun.tools.javac.comp.
                 "--add-opens", "jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED",
-                ]
+            ]
        }
     }
 }
@@ -662,6 +675,9 @@ subprojects {
             }
             args += "${formatScriptsHome}/check-google-java-format.py"
             args += getJavaFilesToFormat(project.name)
+            // Since the scripts are downloaded from third-party Github, the environment variable
+            // is the only way to add the necessary open:
+            environment('JDK_JAVA_OPTIONS', '--add-opens jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED')
         }
         ignoreExitValue = true
         doLast {
@@ -819,7 +835,16 @@ subprojects {
                                     // uses the jdk.jdeps module.
                                     "-javacoptions:--add-modules jdk.jdeps",
                                     "-javacoptions:--add-exports=jdk.jdeps/com.sun.tools.classfile=ALL-UNNAMED",
+                                    "-vmoptions:--add-opens=jdk.jdeps/com.sun.tools.classfile=ALL-UNNAMED",
+                                    "-vmoptions:--add-opens=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED",
+                                    "-vmoptions:--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED",
                                     "-vmoptions:--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED",
+                                    "-vmoptions:--add-opens=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED",
+                                    "-vmoptions:--add-opens=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED",
+                                    "-vmoptions:--add-opens=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED",
+                                    "-vmoptions:--add-opens=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED",
+                                    "-vmoptions:--add-opens=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
+                                    "-vmoptions:--add-opens=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED",
                             ]
                         }
                         if (project.name.is('framework')) {
@@ -923,7 +948,12 @@ subprojects {
         tasks.create(name: 'inferenceTests', group: 'Verification') {
             description 'Run inference tests.'
             if (project.name.is('checker')) {
-                dependsOn('ainferTest', 'wpiManyTest', 'wpiPlumeLibTest')
+                // TODO JDK16: Use this line instead of the following lines.
+                // dependsOn('ainferTest', 'wpiManyTest', 'wpiPlumeLibTest')
+                dependsOn('ainferTest', 'wpiPlumeLibTest')
+                if (!isJava16) {
+                    dependsOn('wpiManyTest')
+                }
             }
         }
 

checker-util/src/main/java/org/checkerframework/checker/i18nformatter/util/I18nFormatUtil.java

@@ -300,7 +300,7 @@ private static void makeFormat(int offsetNumber, @Nullable StringBuilder[] textS
       argumentIndices.add(argumentNumber);
 
       // now get the format
-      I18nConversionCategory category = null;
+      final I18nConversionCategory category;
       if (segments[SEG_TYPE].length() != 0) {
         int type = findKeyword(segments[SEG_TYPE], TYPE_KEYWORDS);
         switch (type) {

checker/bin-devel/build.sh

@@ -69,7 +69,7 @@ version=$("$_java" -version 2>&1 | head -1 | cut -d'"' -f2 | sed '/^1\./s///' |
 if [[ "$version" -ge 9 ]]; then
   echo "Running:  (cd ../jspecify/ && ./gradlew build)"
   # If failure, retry in case the failure was due to network lossage.
-  (cd ../jspecify/ && ./gradlew build) || (sleep 60 && cd ../jspecify/ && ./gradlew build)
+  (cd ../jspecify/ && export JDK_JAVA_OPTIONS='--add-opens jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED --add-opens jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED' && (./gradlew build || (sleep 60 && ./gradlew build)))
   echo "... done: (cd ../jspecify/ && ./gradlew build)"
 fi
 

checker/bin-devel/test-plume-lib.sh

@@ -51,6 +51,6 @@ for PACKAGE in "${PACKAGES[@]}"; do
   echo "About to call ./gradlew --console=plain -PcfLocal compileJava"
   # Try twice in case of network lossage while downloading packages (e.g., from Maven Central).
   # A disadvantage is that if there is a real error in pluggable type-checking, this runs it twice
-  # and puts a delays in between.
+  # and puts a delay in between.
   (cd "${PACKAGEDIR}" && (./gradlew --console=plain -PcfLocal compileJava || (sleep 60 && ./gradlew --console=plain -PcfLocal compileJava)))
 done

checker/bin-devel/wpi-plumelib/test-wpi-plumelib.sh

@@ -45,6 +45,11 @@ clean_compile_output() {
 
     # Remove uninteresting output
     sed -i '/^warning: \[path\] bad path element /d' "$out"
+    sed -i '/^.*warning: Option --illegal-access is deprecated and will be removed in a future release./d' "$out"
+    sed -i '/^warning: \[options\] bootstrap class path not set/d' "$out"
+
+    # Remove warning count because it can differ between JDK 8 and later JDKs due to the bootstrap warning:
+    sed -i '/^[0-9]* warning/d' "$out"
 
     # Remove directory names and line numbers
     sed -i 's/^[^ ]*\///' "$out"

checker/bin/wpi-many.sh

@@ -65,6 +65,13 @@ else
   has_java11="yes"
 fi
 
+# shellcheck disable=SC2153 # testing for JAVA16_HOME, not a typo of JAVA_HOME
+if [ "x${JAVA16_HOME}" = "x" ]; then
+  has_java16="no"
+else
+  has_java16="yes"
+fi
+
 if [ "${has_java_home}" = "yes" ]; then
     java_version=$("${JAVA_HOME}"/bin/java -version 2>&1 | head -1 | cut -d'"' -f2 | sed '/^1\./s///' | cut -d'.' -f1)
     if [ "${has_java8}" = "no" ] && [ "${java_version}" = 8 ]; then
@@ -75,6 +82,10 @@ if [ "${has_java_home}" = "yes" ]; then
       export JAVA11_HOME="${JAVA_HOME}"
       has_java11="yes"
     fi
+    if [ "${has_java16}" = "no" ] && [ "${java_version}" = 16 ]; then
+      export JAVA16_HOME="${JAVA_HOME}"
+      has_java16="yes"
+    fi
 fi
 
 if [ "${has_java8}" = "yes" ] && [ ! -d "${JAVA8_HOME}" ]; then
@@ -87,8 +98,13 @@ if [ "${has_java11}" = "yes" ] && [ ! -d "${JAVA11_HOME}" ]; then
     exit 1
 fi
 
-if [ "${has_java8}" = "no" ] && [ "${has_java11}" = "no" ]; then
-    echo "No Java 8 or 11 JDKs found. At least one of JAVA_HOME, JAVA8_HOME, or JAVA11_HOME must be set."
+if [ "${has_java16}" = "yes" ] && [ ! -d "${JAVA16_HOME}" ]; then
+    echo "JAVA16_HOME is set to a non-existent directory ${JAVA16_HOME}"
+    exit 1
+fi
+
+if [ "${has_java8}" = "no" ] && [ "${has_java11}" = "no" ] && [ "${has_java16}" = "no" ]; then
+    echo "No Java 8, 11, or 16 JDKs found. At least one of JAVA_HOME, JAVA8_HOME, JAVA11_HOME, or JAVA16_HOME must be set."
     exit 1
 fi