Skip to content

Commit 3297635

Browse files
mernstclaude
andcommitted
Fail when the Checker Framework is enabled too late to run
Enabling the Checker Framework after Gradle has built the task graph, as in a `gradle.taskGraph.whenReady` action, leaves the task that writes the manifest out of the graph, because the manifest directory is an input of a task only while the Checker Framework is enabled on it. The compilation then discovered no annotation processor and silently checked nothing. Fail instead, and check the manifest rather than when the task was enabled, because a task that is enabled too late is still checked if another task in the same build was enabled in time. Also make two tests assert. Each returns early on a Java version that the plugin under test does not support there, and then guarded its assertions with the same condition, so the assertions never ran and the tests only checked that the build failed, for any reason at all. Remove the guards, and take the version from the JDK that runs the test's build rather than from the JDK that runs the test, which is always 17. With the assertions running, the Error Prone test needed the javac arguments that Error Prone requires but that its Gradle plugin does not add. Tested on JDK 17, 21, and 25, and on Gradle 7.3.3, 8.2.1, and 9.2.1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent f53e692 commit 3297635

4 files changed

Lines changed: 98 additions & 25 deletions

File tree

src/functionalTest/kotlin/org/checkerframework/plugin/gradle/CFGroovyPluginFunctionalTest.kt

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,39 @@ class CFGroovyPluginFunctionalTest : GroovyPluginFunctionalTest() {
467467
assertThat(result.output).contains(NULLNESS_FAILURE)
468468
}
469469

470+
@Test
471+
fun `test enabling the Checker Framework after the task graph is built fails`() {
472+
buildFile.appendText(
473+
"""
474+
checkerFramework {
475+
version = "$TEST_CF_VERSION"
476+
checkers = ["org.checkerframework.checker.nullness.NullnessChecker"]
477+
}
478+
compileJava {
479+
options.checkerFrameworkCompile.enabled = false
480+
}
481+
gradle.taskGraph.whenReady {
482+
tasks.compileJava.options.checkerFrameworkCompile.enabled = true
483+
}
484+
"""
485+
.trimIndent()
486+
)
487+
// given
488+
testProjectDir.writeNullnessFailure()
489+
490+
// when
491+
val result = testProjectDir.buildWithArgsAndFail("compileJava")
492+
493+
// then
494+
// The task that writes the manifest was left out of the task graph, because the Checker
495+
// Framework was disabled when the graph was built, so no checker can run on this compilation.
496+
// The build fails rather than succeeding while checking nothing.
497+
assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.FAILED)
498+
assertThat(result.output)
499+
.contains("The Checker Framework was enabled on :compileJava too late for it to run")
500+
assertThat(result.output).doesNotContain(NULLNESS_FAILURE)
501+
}
502+
470503
@Test
471504
fun `test running the Checker Framework after a build that skipped it`() {
472505
buildFile.appendText(

src/functionalTest/kotlin/org/checkerframework/plugin/gradle/Fixtures.kt

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,14 @@ import org.gradle.testkit.runner.GradleRunner
66
import org.gradle.util.GradleVersion
77

88
val testJavaHome = System.getProperty("test.java-home", System.getProperty("java.home"))
9+
10+
/**
11+
* The major version of [testJavaHome], the Java version that runs the builds that the tests launch
12+
* and therefore compiles the code that the tests check. It is not necessarily the version that runs
13+
* the tests themselves, which is the version of this build's Kotlin toolchain.
14+
*/
15+
val testJavaVersion: Int =
16+
System.getProperty("test.java-version")?.toInt() ?: Runtime.version().feature()
917
val testGradleVersion =
1018
System.getProperty("test.gradle-version")?.let(GradleVersion::version) ?: GradleVersion.current()
1119

src/functionalTest/kotlin/org/checkerframework/plugin/gradle/OtherPluginsFunctionalTest.kt

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ class OtherPluginsFunctionalTest : KotlinPluginFunctionalTest() {
2020

2121
@Test
2222
fun `test lombok 8 12 1`() {
23-
val majorVersion = Runtime.version().feature()
24-
if (majorVersion >= 25) {
23+
// Lombok 8.12.1 does not support Java 25 and later.
24+
if (testJavaVersion >= 25) {
2525
return
2626
}
2727
buildFile.appendText(
@@ -46,16 +46,13 @@ class OtherPluginsFunctionalTest : KotlinPluginFunctionalTest() {
4646
// when
4747
val result = testProjectDir.buildWithArgsAndFail("build")
4848

49-
if (majorVersion >= 25) {
50-
51-
// then
52-
assertThat(result.output)
53-
.contains(
54-
"User.java:9: error: [argument] incompatible argument for parameter y of FooBuilder.y."
55-
)
56-
assertThat(result.output)
57-
.contains("Foo.java:12: error: [assignment] incompatible types in assignment.")
58-
}
49+
// then
50+
assertThat(result.output)
51+
.contains(
52+
"User.java:9: error: [argument] incompatible argument for parameter y of FooBuilder.y."
53+
)
54+
assertThat(result.output)
55+
.contains("Foo.java:12: error: [assignment] incompatible types in assignment.")
5956
}
6057

6158
@Test
@@ -281,8 +278,8 @@ class OtherPluginsFunctionalTest : KotlinPluginFunctionalTest() {
281278

282279
@Test
283280
fun `test errorprone latest`() {
284-
val majorVersion = Runtime.version().feature()
285-
if (majorVersion < 21) {
281+
// Error Prone 4.0.1 does not support Java versions before 21.
282+
if (testJavaVersion < 21) {
286283
return
287284
}
288285
buildFile.delete()
@@ -308,6 +305,12 @@ class OtherPluginsFunctionalTest : KotlinPluginFunctionalTest() {
308305
}
309306
tasks.withType<JavaCompile>().configureEach {
310307
options.errorprone.warn("CollectionIncompatibleType")
308+
// Error Prone requires these javac arguments, which its Gradle plugin does not add.
309+
options.compilerArgs.addAll(
310+
listOf(
311+
"-XDcompilePolicy=simple",
312+
"--should-stop=ifError=FLOW",
313+
"-XDaddTypeAnnotationsToSymbol=true"))
311314
}
312315
313316
configure<CheckerFrameworkExtension> {
@@ -324,16 +327,14 @@ class OtherPluginsFunctionalTest : KotlinPluginFunctionalTest() {
324327
// when
325328
val result = testProjectDir.buildWithArgsAndFail("build")
326329

327-
if (majorVersion < 21) {
328-
// then
329-
assertThat(result.output)
330-
.contains(
331-
"Demo.java:7: warning: [CollectionIncompatibleType] Argument 'i - 1' should not be passed to this method; its type int is not compatible with its collection's type argument Short"
332-
)
333-
assertThat(result.output)
334-
.contains(
335-
"Demo.java:8: error: [argument] incompatible argument for parameter arg0 of Set.add."
336-
)
337-
}
330+
// then
331+
assertThat(result.output)
332+
.contains(
333+
"Demo.java:7: warning: [CollectionIncompatibleType] Argument 'i - 1' should not be passed to this method; its type int is not compatible with its collection's type argument Short"
334+
)
335+
assertThat(result.output)
336+
.contains(
337+
"Demo.java:8: error: [argument] incompatible argument for parameter arg0 of Set.add."
338+
)
338339
}
339340
}

src/main/kotlin/org/checkerframework/plugin/gradle/CheckerFrameworkPlugin.kt

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -716,6 +716,7 @@ class CheckerFrameworkPlugin @Inject constructor() : Plugin<Project> {
716716
undoFork(task)
717717
return
718718
}
719+
requireManifest(task)
719720

720721
// Must fork for the JVM arguments to be applied. Configuration time requests forking if the
721722
// Checker Framework was enabled then, but this ensures that no other configuration has undone
@@ -746,6 +747,36 @@ class CheckerFrameworkPlugin @Inject constructor() : Plugin<Project> {
746747
}
747748
}
748749

750+
/**
751+
* Throws an exception if the manifest that makes javac discover the checkers has not been
752+
* written, which means that no checker would run on the given task.
753+
*
754+
* The manifest directory is an input of every task that this plugin enables, so the task that
755+
* writes it is in the task graph -- and has run by now -- if the Checker Framework was enabled
756+
* on any task when Gradle built the graph. If it was enabled only afterwards, for example by a
757+
* `gradle.taskGraph.whenReady` action, then adding the task that writes the manifest is no
758+
* longer possible. Without this check, the compilation would succeed while checking nothing,
759+
* because javac silently runs no annotation processor when it discovers none.
760+
*
761+
* The manifest is written once per project rather than once per task, so a task that is enabled
762+
* too late still finds the manifest, and is checked, if another task in the same build was
763+
* enabled in time. Hence this checks the manifest itself rather than when the task was enabled.
764+
*
765+
* @param task the task that the Checker Framework is enabled on
766+
*/
767+
private fun requireManifest(task: Task) {
768+
if (
769+
cfManifestFiles.files.any { File(it, WriteCheckerManifestTask.PROCESSOR_FILE_NAME).isFile }
770+
) {
771+
return
772+
}
773+
throw IllegalStateException(
774+
"The Checker Framework was enabled on ${task.path} too late for it to run: the manifest" +
775+
" that makes javac discover the checkers was not written. Enable the Checker Framework" +
776+
" while the build is being configured, no later than when Gradle builds the task graph."
777+
)
778+
}
779+
749780
/**
750781
* Undoes the forking that configuration time requested, when the Checker Framework was still
751782
* going to run on the task, so that a compilation that does not run the Checker Framework does

0 commit comments

Comments
 (0)