Add specific support for JDK 16

tbroyer · tbroyer · commit 89eeb78aaaa7 · 2021-03-21T18:01:37.000+01:00
JDK 16 is then tested with both Gradle 6 and Gradle 7
diff --git a/.github/workflows/gradle.yaml b/.github/workflows/gradle.yaml
@@ -104,7 +104,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        java: [ 11, 15 ]
+        java: [ 11, 16 ]
     steps:
       - uses: actions/checkout@v2
 
@@ -141,3 +141,41 @@ jobs:
         with:
           name: jdk-${{ matrix.java }}-tests
           path: build/reports/tests/test/
+
+  test-jdk16-gradle7:
+    needs: linux
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+
+      - name: Setup JDK 16
+        id: setup-test-java
+        uses: actions/setup-java@v1
+        with:
+          java-version: 16
+
+      - name: Setup JAVA_HOME_16_X64 environment variable
+        run: echo "JAVA_HOME_16_X64=${{ steps.setup-test-java.outputs.path }}" | tee -a $GITHUB_ENV
+
+      - name: Setup environment
+        run: |
+          echo "JAVA_HOME=${JAVA_HOME_8_X64}" | tee -a $GITHUB_ENV
+          echo "org.gradle.java.installations.auto-download=false" | tee -a gradle.properties
+          echo "org.gradle.java.installations.fromEnv=JAVA_HOME_16_X64" | tee -a gradle.properties
+
+      - name: Build with Gradle
+        id: gradle-build
+        uses: eskatos/gradle-command-action@v1
+        with:
+          arguments: --scan test -Ptest.java-toolchain=16 -Ptest.gradle-version=7.0-milestone-3
+          wrapper-cache-enabled: true
+          dependencies-cache-enabled: true
+          dependencies-cache-key: '**/*gradle.lockfile'
+          dependencies-cache-exact: true
+
+      - name: Store reports
+        if: always() && (steps.gradle-build.outcome == 'success' || steps.gradle-build.outcome == 'failure')
+        uses: actions/upload-artifact@v2
+        with:
+          name: jdk16-gradle7-tests
+          path: build/reports/tests/test/
diff --git a/README.md b/README.md
@@ -196,6 +196,23 @@ someTask.configure {
 [BaseForkOptions.getJvmArgs]: https://docs.gradle.org/current/javadoc/org/gradle/api/tasks/compile/BaseForkOptions.html#getJvmArgs--
 [ForkOptions.setJavaHome]: https://docs.gradle.org/current/javadoc/org/gradle/api/tasks/compile/ForkOptions.html#setJavaHome-java.io.File-
 
+## JDK 16+ support
+
+Starting with JDK 16, due to [JEP 396: Strongly Encapsulate JDK Internals by Default][jep396],
+`--add-opens` and `--add-exports` arguments need to be passed to the compiler's JVM.
+
+The plugin will automatically [use a forking compiler][CompileOptions.fork]
+and pass the necessary [JVM arguments][BaseForkOptions.getJvmArgs]
+whenever it detects such a JDK is being used and ErrorProne is enabled.
+
+That detection will only take into account the [toolchain][gradle-toolchains] used by the `JavaCompile` task,
+or the JDK used to run Gradle in case no toolchain is being used.
+This should also include toolchains set using `forkOptions.executable` or `forkOptions.javaHome`,
+but only in Gradle 6.7+ where Gradle should expose those as ad-hoc toolchains,
+and this hasn't been tested.
+
+[jep396]: https://openjdk.java.net/jeps/396
+
 ## Migration from [versions 0.0._x_]
 
 <details>
diff --git a/src/main/kotlin/net/ltgt/gradle/errorprone/ErrorPronePlugin.kt b/src/main/kotlin/net/ltgt/gradle/errorprone/ErrorPronePlugin.kt
@@ -54,6 +54,19 @@ Add a dependency to com.google.errorprone:javac with the appropriate version cor
         private val HAS_TOOLCHAINS = GradleVersion.current().baseVersion >= GradleVersion.version("6.7")
 
         internal const val TOO_OLD_TOOLCHAIN_ERROR_MESSAGE = "Must not enable ErrorProne when compiling with JDK < 8"
+
+        internal val JVM_ARGS_STRONG_ENCAPSULATION = listOf(
+            "--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED",
+            "--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED",
+            "--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED",
+            "--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
+            "--add-exports=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED",
+            "--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED",
+            "--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED",
+            "--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED",
+            "--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED",
+            "--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED"
+        )
     }
 
     override fun apply(project: Project) {
@@ -101,7 +114,7 @@ Add a dependency to com.google.errorprone:javac with the appropriate version cor
                 .compilerArgumentProviders
                 .add(ErrorProneCompilerArgumentProvider(errorproneOptions))
 
-            if (HAS_TOOLCHAINS || JavaVersion.current().isJava8) {
+            if (HAS_TOOLCHAINS || JavaVersion.current().run { isJava8 || isJava16Compatible }) {
                 inputs.files(
                     providers.provider {
                         when {
@@ -125,10 +138,13 @@ Add a dependency to com.google.errorprone:javac with the appropriate version cor
                             when {
                                 targetVersion < 8 -> throw UnsupportedOperationException(TOO_OLD_TOOLCHAIN_ERROR_MESSAGE)
                                 targetVersion == 8 -> configureErrorProneJavac()
+                                targetVersion >= 16 -> configureForJava16plus()
                             }
                         }
                         JavaVersion.current().isJava8 && (!options.isFork || (options.forkOptions.javaHome == null && options.forkOptions.executable == null)) ->
                             configureErrorProneJavac()
+                        JavaVersion.current().isJava16Compatible && (!options.isFork || (options.forkOptions.javaHome == null && options.forkOptions.executable == null)) ->
+                            configureForJava16plus()
                     }
                 }
             }
@@ -174,6 +190,16 @@ Add a dependency to com.google.errorprone:javac with the appropriate version cor
             }
         }
     }
+
+    private fun JavaCompile.configureForJava16plus() {
+        // https://github.com/google/error-prone/issues/1157#issuecomment-769289564
+        if (!options.isFork) {
+            options.isFork = true
+            // reset forkOptions in case they were configured
+            options.forkOptions = ForkOptions()
+        }
+        options.forkOptions.jvmArgs!!.addAll(JVM_ARGS_STRONG_ENCAPSULATION)
+    }
 }
 
 internal class ErrorProneCompilerArgumentProvider(
@@ -199,3 +225,6 @@ internal class ErrorProneCompilerArgumentProvider(
 
 internal val TEST_SOURCE_SET_NAME_REGEX =
     """^(t|.*T)est(\p{javaUpperCase}.*)?$""".toRegex()
+
+internal val JavaVersion.isJava16Compatible: Boolean
+    get() = this < JavaVersion.VERSION_HIGHER && this >= JavaVersion.toVersion(16)
diff --git a/src/test/kotlin/net/ltgt/gradle/errorprone/AndroidIntegrationTest.kt b/src/test/kotlin/net/ltgt/gradle/errorprone/AndroidIntegrationTest.kt
@@ -4,6 +4,7 @@ import com.google.common.truth.Truth.assertThat
 import com.google.common.truth.TruthJUnit.assume
 import java.io.File
 import java.util.Properties
+import org.gradle.api.JavaVersion
 import org.gradle.testkit.runner.TaskOutcome
 import org.gradle.util.GradleVersion
 import org.junit.Before
@@ -15,6 +16,7 @@ class AndroidIntegrationTest : AbstractPluginIntegrationTest() {
     @Before
     fun setupAndroid() {
         assume().that(GradleVersion.version(testGradleVersion)).isAtLeast(GradleVersion.version("6.5"))
+        assume().withMessage("isJava16Compatible").that(JavaVersion.current()).isLessThan(JavaVersion.VERSION_16)
         assertThat(androidSdkHome).isNotEmpty()
 
         Properties().apply {
diff --git a/src/test/kotlin/net/ltgt/gradle/errorprone/ErrorProneOptionsTest.kt b/src/test/kotlin/net/ltgt/gradle/errorprone/ErrorProneOptionsTest.kt
@@ -2,9 +2,11 @@ package net.ltgt.gradle.errorprone
 
 import com.google.common.truth.StringSubject
 import com.google.common.truth.Truth.assertThat
+import com.google.common.truth.TruthJUnit.assume
 import com.google.errorprone.ErrorProneOptions.Severity
 import com.google.errorprone.InvalidCommandLineOptionException
 import org.gradle.api.InvalidUserDataException
+import org.gradle.api.JavaVersion
 import org.gradle.api.model.ObjectFactory
 import org.gradle.api.provider.ProviderFactory
 import org.gradle.api.tasks.SourceSet
@@ -28,6 +30,9 @@ class ErrorProneOptionsTest {
 
         @JvmStatic @BeforeClass
         fun setup() {
+            // FIXME: remove when updating to Gradle 7
+            assume().that(JavaVersion.current()).isLessThan(JavaVersion.VERSION_16)
+
             ProjectBuilder.builder().withProjectDir(projectDir.root).build().let { project ->
                 objects = project.objects
                 providers = project.providers
diff --git a/src/test/kotlin/net/ltgt/gradle/errorprone/ErrorPronePluginIntegrationTest.kt b/src/test/kotlin/net/ltgt/gradle/errorprone/ErrorPronePluginIntegrationTest.kt
@@ -3,6 +3,7 @@ package net.ltgt.gradle.errorprone
 import com.google.common.truth.Truth.assertThat
 import com.google.common.truth.TruthJUnit.assume
 import java.io.File
+import org.gradle.api.JavaVersion
 import org.gradle.testkit.runner.TaskOutcome
 import org.gradle.util.GradleVersion
 import org.junit.Before
@@ -27,6 +28,19 @@ class ErrorPronePluginIntegrationTest : AbstractPluginIntegrationTest() {
             }
             """.trimIndent()
         )
+        if (JavaVersion.current().isJava16Compatible && GradleVersion.version(testGradleVersion) < GradleVersion.version("7.0-milestone-3")) {
+            // https://melix.github.io/blog/2021/03/gradle-java16.html
+            buildFile.appendText(
+                """
+
+                allprojects {
+                    tasks.withType<JavaCompile>().configureEach {
+                        options.isIncremental = false
+                    }
+                }
+                """.trimIndent()
+            )
+        }
     }
 
     @Test
@@ -155,6 +169,10 @@ class ErrorPronePluginIntegrationTest : AbstractPluginIntegrationTest() {
     @Test
     fun `is configuration-cache friendly`() {
         assume().that(GradleVersion.version(testGradleVersion)).isAtLeast(GradleVersion.version("6.6"))
+        assume().that(
+            JavaVersion.current().isJava16Compatible &&
+                GradleVersion.version(testGradleVersion) < GradleVersion.version("7.0-milestone-3")
+        ).isFalse()
 
         // given
         writeSuccessSource()
diff --git a/src/test/kotlin/net/ltgt/gradle/errorprone/GroovyDslIntegrationTest.kt b/src/test/kotlin/net/ltgt/gradle/errorprone/GroovyDslIntegrationTest.kt
@@ -1,11 +1,19 @@
 package net.ltgt.gradle.errorprone
 
 import com.google.common.truth.Truth.assertThat
+import com.google.common.truth.TruthJUnit.assume
+import org.gradle.api.JavaVersion
 import org.gradle.testkit.runner.TaskOutcome
+import org.gradle.util.GradleVersion
 import org.junit.Test
 
 class GroovyDslIntegrationTest : AbstractPluginIntegrationTest() {
     override fun setupProject() {
+        assume().that(
+            JavaVersion.current().isJava16Compatible &&
+                GradleVersion.version(testGradleVersion) < GradleVersion.version("7.0-milestone-3")
+        ).isFalse()
+
         settingsFile = testProjectDir.newFile("settings.gradle")
         buildFile = testProjectDir.newFile("build.gradle").apply {
             appendText(
diff --git a/src/test/kotlin/net/ltgt/gradle/errorprone/Java8IntegrationTest.kt b/src/test/kotlin/net/ltgt/gradle/errorprone/Java8IntegrationTest.kt
@@ -21,6 +21,7 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {
             """$JVM_ARG_BOOTCLASSPATH.*${fileSeparator}com\.google\.errorprone${fileSeparator}javac$fileSeparator$escapedErrorproneJavacVersion$fileSeparator.*${fileSeparator}javac-$escapedErrorproneJavacVersion.jar[${File.pathSeparator}${System.lineSeparator()}]"""
                 .toPattern()
         }
+        private val JVM_ARGS_STRONG_ENCAPSULATION = ErrorPronePlugin.JVM_ARGS_STRONG_ENCAPSULATION.joinToString(prefix = JVM_ARG, separator = JVM_ARG)
 
         private fun jvmArg(argPrefix: String) = "$JVM_ARG$argPrefix"
     }
@@ -53,12 +54,24 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {
             compileJava.finalizedBy(displayCompileJavaOptions)
             """.trimIndent()
         )
+        if (JavaVersion.current().isJava16Compatible && GradleVersion.version(testGradleVersion) < GradleVersion.version("7.0-milestone-3")) {
+            // https://melix.github.io/blog/2021/03/gradle-java16.html
+            buildFile.appendText(
+                """
+
+                tasks.withType<JavaCompile>().configureEach {
+                    options.isIncremental = false
+                }
+                """.trimIndent()
+            )
+        }
         writeSuccessSource()
     }
 
     @Test
-    fun `does not configure forking in non-Java 8 VM`() {
+    fun `does not configure forking in non-Java 8 or 16+ VM`() {
         assume().withMessage("isJava8").that(JavaVersion.current().isJava8).isFalse()
+        assume().withMessage("isJava16Compatible").that(JavaVersion.current()).isLessThan(JavaVersion.VERSION_16)
 
         // when
         buildWithArgs("compileJava").also { result ->
@@ -108,9 +121,30 @@ class Java8IntegrationTest : AbstractPluginIntegrationTest() {
         }
     }
 
+    @Test
+    fun `configure forking in Java 16+ VM`() {
+        assume().withMessage("isJava16Compatible").that(JavaVersion.current()).isAtLeast(JavaVersion.VERSION_16)
+
+        // when
+        buildWithArgs("compileJava").also { result ->
+            // then
+            assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
+            assertThat(result.output).contains(FORKED)
+            assertThat(result.output).contains(JVM_ARGS_STRONG_ENCAPSULATION)
+        }
+
+        // check that it doesn't mess with task avoidance
+
+        // when
+        buildWithArgs("compileJava").also { result ->
+            // then
+            assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.UP_TO_DATE)
+        }
+    }
+
     @Test
     fun `does not configure forking if Error Prone is disabled`() {
-        assume().withMessage("isJava8").that(JavaVersion.current().isJava8).isTrue()
+        assume().withMessage("isJava8Or16plus").that(JavaVersion.current().run { isJava8 || isJava16Compatible }).isTrue()
 
         // given
         buildFile.appendText(
diff --git a/src/test/kotlin/net/ltgt/gradle/errorprone/ManualConfigurationIntegrationTest.kt b/src/test/kotlin/net/ltgt/gradle/errorprone/ManualConfigurationIntegrationTest.kt
@@ -2,6 +2,7 @@ package net.ltgt.gradle.errorprone
 
 import com.google.common.truth.Truth.assertThat
 import com.google.common.truth.TruthJUnit.assume
+import org.gradle.api.JavaVersion
 import org.gradle.testkit.runner.TaskOutcome
 import org.gradle.util.GradleVersion
 import org.junit.Test
@@ -45,6 +46,17 @@ class ManualConfigurationIntegrationTest : AbstractPluginIntegrationTest() {
             }
             """.trimIndent()
         )
+        if (JavaVersion.current().isJava16Compatible && GradleVersion.version(testGradleVersion) < GradleVersion.version("7.0-milestone-3")) {
+            // https://melix.github.io/blog/2021/03/gradle-java16.html
+            buildFile.appendText(
+                """
+
+                tasks.withType<JavaCompile>().configureEach {
+                    options.isIncremental = false
+                }
+                """.trimIndent()
+            )
+        }
         writeFailureSource()
 
         // when
@@ -88,6 +100,17 @@ class ManualConfigurationIntegrationTest : AbstractPluginIntegrationTest() {
             }
             """.trimIndent()
         )
+        if (JavaVersion.current().isJava16Compatible && GradleVersion.version(testGradleVersion) < GradleVersion.version("7.0-milestone-3")) {
+            // https://melix.github.io/blog/2021/03/gradle-java16.html
+            buildFile.appendText(
+                """
+
+                tasks.withType<JavaCompile>().configureEach {
+                    options.isIncremental = false
+                }
+                """.trimIndent()
+            )
+        }
         writeFailureSource()
 
         // Error Prone is disabled by default, so compilation should succeed.
diff --git a/src/test/kotlin/net/ltgt/gradle/errorprone/ToolchainsIntegrationTest.kt b/src/test/kotlin/net/ltgt/gradle/errorprone/ToolchainsIntegrationTest.kt
