Skip to content

Commit e252bc2

Browse files
committed
Merge ../checker-framework-gradle-plugin-branch-config-cache-7 into config-cache-8
2 parents 00a1421 + 164c6a9 commit e252bc2

4 files changed

Lines changed: 170 additions & 41 deletions

File tree

.github/workflows/gradle.yml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ jobs:
2020

2121
steps:
2222
- uses: actions/checkout@v7
23+
with:
24+
fetch-depth: 1
25+
show-progress: false
26+
persist-credentials: false
2327
- name: Set up JDK 17
2428
uses: actions/setup-java@v4
2529
with:
@@ -42,6 +46,10 @@ jobs:
4246

4347
steps:
4448
- uses: actions/checkout@v7
49+
with:
50+
fetch-depth: 1
51+
show-progress: false
52+
persist-credentials: false
4553
- name: Set up JDK 17
4654
uses: actions/setup-java@v4
4755
with:
@@ -65,6 +73,10 @@ jobs:
6573
gradle: ['9.7.0', '9.6.1', '9.5.1', '9.4.1', '9.3.1', '9.2.1', '9.1.0', '9.0.0', '8.14.5', '8.13', '8.12.1', '8.11.1', '8.10.2', '8.9', '8.8', '8.7', '8.6', '8.5', '8.4', '8.3', '8.2.1', '8.1.1', '8.0.2', '7.6.6', '7.5.1', '7.4.2', '7.3.3']
6674
steps:
6775
- uses: actions/checkout@v7
76+
with:
77+
fetch-depth: 1
78+
show-progress: false
79+
persist-credentials: false
6880

6981
- uses: actions/setup-java@v6
7082
with:
@@ -89,6 +101,10 @@ jobs:
89101
java: [ 17, 21, 25 ]
90102
steps:
91103
- uses: actions/checkout@v7
104+
with:
105+
fetch-depth: 1
106+
show-progress: false
107+
persist-credentials: false
92108

93109
- uses: actions/setup-java@v6
94110
with:

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

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -737,18 +737,53 @@ class CFGroovyPluginFunctionalTest : GroovyPluginFunctionalTest() {
737737
testProjectDir.writeEmptyClass()
738738

739739
// when
740-
val result = testProjectDir.buildWithArgs("compileJava")
740+
val result = testProjectDir.buildWithArgs("compileJava", "--info")
741741

742742
// then
743743
// Forking was requested at configuration time, while the Checker Framework was still enabled.
744744
// Because this compilation does not run the Checker Framework after all, it does not fork.
745745
assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
746746
assertThat(result.output).contains("COMPILE_JAVA_FORK=false")
747-
// The user sees, at the default log level, that a fork was discarded.
747+
// The discarded fork is this plugin's own, so discarding it is logged at the info level.
748748
assertThat(result.output)
749749
.contains("Not forking :compileJava because the Checker Framework will not run")
750750
}
751751

752+
@Test
753+
fun `test forking is not undone if fork options are set after configuration`() {
754+
buildFile.appendText(
755+
"""
756+
checkerFramework {
757+
version = "$TEST_CF_VERSION"
758+
checkers = ["org.checkerframework.checker.nullness.NullnessChecker"]
759+
}
760+
gradle.taskGraph.whenReady {
761+
tasks.compileJava.options.checkerFrameworkCompile.enabled = false
762+
tasks.compileJava.options.fork = true
763+
tasks.compileJava.options.forkOptions.memoryMaximumSize = "1g"
764+
}
765+
tasks.compileJava.doLast {
766+
println "COMPILE_JAVA_FORK=" + options.fork
767+
println "COMPILE_JAVA_MEMORY=" + options.forkOptions.memoryMaximumSize
768+
}
769+
"""
770+
.trimIndent()
771+
)
772+
// given
773+
testProjectDir.writeEmptyClass()
774+
775+
// when
776+
val result = testProjectDir.buildWithArgs("compileJava")
777+
778+
// then
779+
// The fork options were set after this plugin requested the fork, so the fork is one that the
780+
// build script wants, even though the Checker Framework does not run on this compilation.
781+
assertThat(result.task(":compileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
782+
assertThat(result.output).contains("COMPILE_JAVA_FORK=true")
783+
assertThat(result.output).contains("COMPILE_JAVA_MEMORY=1g")
784+
assertThat(result.output).doesNotContain("Not forking :compileJava")
785+
}
786+
752787
@Test
753788
fun `test forking is undone if annotation processing is disabled after configuration`() {
754789
buildFile.appendText(
@@ -770,7 +805,7 @@ class CFGroovyPluginFunctionalTest : GroovyPluginFunctionalTest() {
770805
testProjectDir.writeEmptyClass()
771806

772807
// when
773-
val result = testProjectDir.buildWithArgs("compileJava")
808+
val result = testProjectDir.buildWithArgs("compileJava", "--info")
774809

775810
// then
776811
// A null annotationProcessorPath means that annotation processing is disabled, so no checker

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,48 @@ class OtherPluginsFunctionalTest : KotlinPluginFunctionalTest() {
123123
assertThat(result.output).contains("CHECK_DELOMBOK_FORK=true")
124124
}
125125

126+
@Test
127+
fun `test forking is undone with lombok when annotation processing is disabled`() {
128+
buildFile.appendText(
129+
"""
130+
plugins {
131+
`java-library`
132+
id("org.checkerframework")
133+
id("io.freefair.lombok").version("9.2.0")
134+
}
135+
136+
configure<CheckerFrameworkExtension> {
137+
version = "$TEST_CF_VERSION"
138+
checkers = listOf("org.checkerframework.checker.nullness.NullnessChecker")
139+
}
140+
tasks.withType<JavaCompile>().configureEach {
141+
options.annotationProcessorPath = configurations.getByName("annotationProcessor")
142+
}
143+
gradle.taskGraph.whenReady {
144+
tasks.named<JavaCompile>("checkDelombokCompileJava").get().options.annotationProcessorPath =
145+
null
146+
}
147+
tasks.named<JavaCompile>("checkDelombokCompileJava") {
148+
doLast { logger.lifecycle("CHECK_DELOMBOK_FORK=" + options.isFork) }
149+
}
150+
"""
151+
.trimIndent()
152+
)
153+
// given
154+
testProjectDir.writeCorrectLombokExample()
155+
156+
// when
157+
val result = testProjectDir.buildWithArgs("checkDelombokCompileJava")
158+
159+
// then
160+
// The build script's own configureEach gives the checkDelombokCompileJava task an
161+
// annotationProcessorPath before this plugin copies one onto it, so this plugin requests the
162+
// fork the first of the two times that it tries to. That request must still be recorded when
163+
// the task turns out not to run the Checker Framework, so that the fork is undone.
164+
assertThat(result.task(":checkDelombokCompileJava")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
165+
assertThat(result.output).contains("CHECK_DELOMBOK_FORK=false")
166+
}
167+
126168
@Test
127169
fun `test disabling CF with lombok `() {
128170
buildFile.appendText(

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

Lines changed: 74 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import org.gradle.api.tasks.PathSensitivity
2828
import org.gradle.api.tasks.SourceSet
2929
import org.gradle.api.tasks.SourceSetContainer
3030
import org.gradle.api.tasks.TaskProvider
31+
import org.gradle.api.tasks.compile.ForkOptions
3132
import org.gradle.api.tasks.compile.JavaCompile
3233
import org.gradle.kotlin.dsl.getByName
3334
import org.gradle.kotlin.dsl.getByType
@@ -43,6 +44,25 @@ class CheckerFrameworkPlugin @Inject constructor() : Plugin<Project> {
4344
companion object {
4445
const val PLUGIN_ID = "org.checkerframework"
4546
const val CONFIGURATION_NAME = "checkerFramework"
47+
48+
/**
49+
* Returns a description of the given fork options, for determining whether the fork options
50+
* have changed since some earlier moment. JVM argument providers are not described, because
51+
* this plugin adds one of its own.
52+
*
53+
* @param forkOptions the fork options to describe
54+
* @return a description of the fork options
55+
*/
56+
private fun forkOptionsDescription(forkOptions: ForkOptions): String =
57+
listOf(
58+
forkOptions.javaHome,
59+
forkOptions.executable,
60+
forkOptions.tempDir,
61+
forkOptions.memoryInitialSize,
62+
forkOptions.memoryMaximumSize,
63+
forkOptions.jvmArgs,
64+
)
65+
.toString()
4666
}
4767

4868
override fun apply(project: Project) {
@@ -94,10 +114,10 @@ class CheckerFrameworkPlugin @Inject constructor() : Plugin<Project> {
94114
// configureJavaCompileTasks leaves the task alone, is not compiled with the Checker Framework.
95115
val cfEnabled = HashMap<String, Property<Boolean>>()
96116

97-
// Whether this plugin requested forking for a [JavaCompile] task, by task name.
98-
// requestFork sets each value; a task whose value is never set, because
99-
// configureJavaCompileTasks leaves the task alone, was not made to fork by this plugin.
100-
val cfRequestedFork = HashMap<String, Property<Boolean>>()
117+
// The fork that this plugin requested for a [JavaCompile] task, by task name. Each value is
118+
// the task's fork options as of the request. requestFork sets a value only for a task that it
119+
// makes fork, so a property with no value means that this plugin did not make the task fork.
120+
val cfForkRequest = HashMap<String, Property<String>>()
101121

102122
project.tasks.withType<JavaCompile>().configureEach {
103123
(options as ExtensionAware)
@@ -111,10 +131,10 @@ class CheckerFrameworkPlugin @Inject constructor() : Plugin<Project> {
111131
// property, whose value configureJavaCompileTasks sets after the build script has run.
112132
val enabled = project.objects.property(Boolean::class.java)
113133
cfEnabled[name] = enabled
114-
val requestedFork = project.objects.property(Boolean::class.java)
115-
cfRequestedFork[name] = requestedFork
134+
val forkRequest = project.objects.property(String::class.java)
135+
cfForkRequest[name] = forkRequest
116136
doFirst(
117-
ApplyCheckerFrameworkOptions(enabled, cfExtension.checkers, cfManifestFiles, requestedFork)
137+
ApplyCheckerFrameworkOptions(enabled, cfExtension.checkers, cfManifestFiles, forkRequest)
118138
)
119139
}
120140

@@ -134,15 +154,15 @@ class CheckerFrameworkPlugin @Inject constructor() : Plugin<Project> {
134154
// Configure after the build script has run, so that the values of the extensions and of the
135155
// project properties are the ones the user requested, no matter when a task is realized.
136156
afterEvaluateOrNow(project) {
137-
configureJavaCompileTasks(project, cfExtension, cfManifestFiles, cfEnabled, cfRequestedFork)
157+
configureJavaCompileTasks(project, cfExtension, cfManifestFiles, cfEnabled, cfForkRequest)
138158
}
139159

140160
// Handle Lombok
141161
project.pluginManager.withPlugin("io.freefair.lombok") {
142162
val javaPluginExtension: JavaPluginExtension =
143163
project.extensions.getByType(JavaPluginExtension::class.java)
144164
javaPluginExtension.sourceSets.configureEach {
145-
addCheckDelombokTask(this, project, cfEnabled, cfRequestedFork)
165+
addCheckDelombokTask(this, project, cfEnabled, cfForkRequest)
146166
}
147167
}
148168
}
@@ -273,15 +293,15 @@ class CheckerFrameworkPlugin @Inject constructor() : Plugin<Project> {
273293
* @param cfManifestFiles the Checker Framework manifest directory
274294
* @param cfEnabled for each task, the property that says whether to run the Checker Framework on
275295
* it, which this method sets
276-
* @param cfRequestedFork for each task, the property that says whether this plugin made the task
277-
* fork, which this method sets
296+
* @param cfForkRequest for each task, the property that records the fork that this plugin
297+
* requested, which this method sets
278298
*/
279299
private fun configureJavaCompileTasks(
280300
project: Project,
281301
cfExtension: CheckerFrameworkExtension,
282302
cfManifestFiles: FileCollection,
283303
cfEnabled: Map<String, Property<Boolean>>,
284-
cfRequestedFork: Map<String, Property<Boolean>>,
304+
cfForkRequest: Map<String, Property<String>>,
285305
) {
286306
project.tasks.withType<JavaCompile>().configureEach {
287307
// The "skipCheckerFramework" project property is read here, rather than once outside this
@@ -324,7 +344,7 @@ class CheckerFrameworkPlugin @Inject constructor() : Plugin<Project> {
324344
)
325345
options.forkOptions.jvmArgumentProviders.add(CheckerFrameworkJvmArgumentProvider(enabled))
326346

327-
requestFork(this, enabled.get(), cfRequestedFork)
347+
requestFork(this, enabled.get(), cfForkRequest)
328348

329349
// The manifest directory, or no files if the Checker Framework is disabled. The manifest
330350
// directory carries a dependency on the task that writes it, so that task runs only if some
@@ -356,8 +376,8 @@ class CheckerFrameworkPlugin @Inject constructor() : Plugin<Project> {
356376
}
357377

358378
/**
359-
* Makes the given task fork, if the Checker Framework will run on it, and records whether this
360-
* plugin made the task fork.
379+
* Makes the given task fork, if the Checker Framework will run on it, and records the fork that
380+
* this plugin requested.
361381
*
362382
* Forking is necessary for the JVM arguments to be applied. It is requested at configuration
363383
* time, rather than only by [ApplyCheckerFrameworkOptions], because `isFork` is a task input and
@@ -369,21 +389,28 @@ class CheckerFrameworkPlugin @Inject constructor() : Plugin<Project> {
369389
* that a compilation that the user enables later forks after all; and it undoes this request if
370390
* the Checker Framework will not run on the task after all.
371391
*
392+
* This method does nothing for a task that this plugin has already made fork, so that calling it
393+
* again does not discard the record of the earlier request.
394+
*
372395
* @param task the task to make fork
373396
* @param enabled whether to run the Checker Framework on the task
374-
* @param cfRequestedFork for each task, the property that says whether this plugin made the task
375-
* fork, which this method sets
397+
* @param cfForkRequest for each task, the property that records the fork that this plugin
398+
* requested, which this method sets
376399
*/
377400
private fun requestFork(
378401
task: JavaCompile,
379402
enabled: Boolean,
380-
cfRequestedFork: Map<String, Property<Boolean>>,
403+
cfForkRequest: Map<String, Property<String>>,
381404
) {
405+
val forkRequest = cfForkRequest.getValue(task.name)
406+
if (forkRequest.isPresent) {
407+
// This plugin has already made the task fork.
408+
return
409+
}
382410
val options = task.options
383-
val requestedFork = enabled && !options.isFork && options.annotationProcessorPath != null
384-
cfRequestedFork.getValue(task.name).set(requestedFork)
385-
if (requestedFork) {
411+
if (enabled && !options.isFork && options.annotationProcessorPath != null) {
386412
options.isFork = true
413+
forkRequest.set(forkOptionsDescription(options.forkOptions))
387414
}
388415
}
389416

@@ -395,14 +422,14 @@ class CheckerFrameworkPlugin @Inject constructor() : Plugin<Project> {
395422
* @param project current project
396423
* @param cfEnabled for each task, the property that says whether to run the Checker Framework on
397424
* it
398-
* @param cfRequestedFork for each task, the property that says whether this plugin made the task
399-
* fork, which this method sets for the checkDelombokCompileJava task
425+
* @param cfForkRequest for each task, the property that records the fork that this plugin
426+
* requested, which this method sets for the checkDelombokCompileJava task
400427
*/
401428
private fun addCheckDelombokTask(
402429
sourceSet: SourceSet,
403430
project: Project,
404431
cfEnabled: Map<String, Property<Boolean>>,
405-
cfRequestedFork: Map<String, Property<Boolean>>,
432+
cfForkRequest: Map<String, Property<String>>,
406433
) {
407434

408435
val checkerTaskProvider: TaskProvider<JavaCompile> =
@@ -447,8 +474,9 @@ class CheckerFrameworkPlugin @Inject constructor() : Plugin<Project> {
447474

448475
// Request forking now, rather than relying on configureJavaCompileTasks to have done so.
449476
// configureJavaCompileTasks ran when this method realized the task, above, at which time the
450-
// task's annotationProcessorPath was still null and forking was therefore not requested.
451-
requestFork(checkerTask, enabled.getOrElse(false), cfRequestedFork)
477+
// task's annotationProcessorPath was usually still null and forking was therefore usually not
478+
// requested.
479+
requestFork(checkerTask, enabled.getOrElse(false), cfForkRequest)
452480
project.tasks.named("build").configure { dependsOn(checkerTask) }
453481
}
454482
}
@@ -653,13 +681,14 @@ class CheckerFrameworkPlugin @Inject constructor() : Plugin<Project> {
653681
* the property has no value, which means that this plugin left the task alone
654682
* @param checkers the checkers to run
655683
* @param cfManifestFiles the Checker Framework manifest directory
656-
* @param requestedFork whether this plugin made the task fork
684+
* @param forkRequest the task's fork options as of when this plugin made the task fork, or no
685+
* value if this plugin did not make the task fork
657686
*/
658687
internal class ApplyCheckerFrameworkOptions(
659688
private val enabled: Provider<Boolean>,
660689
private val checkers: ListProperty<String>,
661690
private val cfManifestFiles: FileCollection,
662-
private val requestedFork: Provider<Boolean>,
691+
private val forkRequest: Provider<String>,
663692
) : Action<Task> {
664693
override fun execute(task: Task) {
665694
val options = (task as JavaCompile).options
@@ -726,21 +755,28 @@ class CheckerFrameworkPlugin @Inject constructor() : Plugin<Project> {
726755
/**
727756
* Undoes the forking that configuration time requested, when the Checker Framework was still
728757
* going to run on the task, so that a compilation that does not run the Checker Framework does
729-
* not fork needlessly. Forking that this plugin did not request is left alone; a request that
730-
* the user makes after this plugin's cannot be distinguished from this plugin's, and is undone
731-
* as well.
758+
* not fork needlessly. Forking that this plugin did not request is left alone, as is forking
759+
* whose options other configuration set after this plugin's request, because such a fork is one
760+
* that something other than this plugin wants. A bare request to fork, with no fork options,
761+
* that the user makes after this plugin's cannot be distinguished from this plugin's, and is
762+
* undone as well.
732763
*
733764
* @param task the task that will not run the Checker Framework
734765
*/
735766
private fun undoFork(task: JavaCompile) {
736-
if (requestedFork.getOrElse(false)) {
737-
// The undoing is logged, at a log level that the user sees by default, because it discards
738-
// a fork that the user may have asked for, along with its fork options.
739-
task.logger.lifecycle(
740-
"Not forking ${task.path} because the Checker Framework will not run on that task."
741-
)
742-
task.options.isFork = false
767+
val requestedForkOptions = forkRequest.orNull ?: return
768+
val options = task.options
769+
if (requestedForkOptions != forkOptionsDescription(options.forkOptions)) {
770+
// Other configuration set fork options after this plugin requested the fork, so the fork is
771+
// wanted for its own sake.
772+
return
743773
}
774+
// The undoing is logged, at a log level that the user does not see by default, because the
775+
// fork that it discards is this plugin's own.
776+
task.logger.info(
777+
"Not forking ${task.path} because the Checker Framework will not run on that task."
778+
)
779+
options.isFork = false
744780
}
745781
}
746782

0 commit comments

Comments
 (0)