[Gradle] Make diagnostic reporting/handling logic more consistent (3/3)
Use two separate instances of the KotlinToolingDiagnosticCollector:
one for configuration phase, another for execution phase.
The reason why we have to go through such a weird hoop is because Gradle
properly serializes BuildService.Parameters only if they were configured
in registerIfAbsent-action. All consequent mutations to will not be
serialized for execution-phase. Therefore, after the configuration phase
we'd lose:
a) all diagnostics (making correct deduplication impossible)
b) isTransparent flag (making diagnostics reported from tasks to be
swallowed)
So, instead we collect all the diagnostics during the configuration
phase in one instance of KotlinToolingDiagnosticCollector, and then
configure the second instance that pulls data from the first one.
diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/mpp/MppDiagnosticsIt.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/mpp/MppDiagnosticsIt.kt
index 8c5dc3d..ec5fb5a 100644
--- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/mpp/MppDiagnosticsIt.kt
+++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/mpp/MppDiagnosticsIt.kt
@@ -155,6 +155,21 @@
}
@GradleTest
+ fun testDiagnosticReportsFromTasksWithCC(gradleVersion: GradleVersion) {
+ project("diagnosticReportsFromTasksWithCC", gradleVersion) {
+ build("assemble", "--rerun-tasks", buildOptions = buildOptions.copy(configurationCache = true)) {
+ assertConfigurationCacheStored()
+ assertEqualsToFile(expectedOutputFile("first-run"), extractProjectsAndTheirDiagnostics())
+ }
+
+ build("assemble", "--rerun-tasks", buildOptions = buildOptions.copy(configurationCache = true)) {
+ assertConfigurationCacheReused()
+ assertEqualsToFile(expectedOutputFile("cache-reused"), extractProjectsAndTheirDiagnostics())
+ }
+ }
+ }
+
+ @GradleTest
fun testEarlyTasksMaterializationDoesntBreakReports(gradleVersion: GradleVersion) {
project("earlyTasksMaterializationDoesntBreakReports", gradleVersion) {
buildAndFail("assemble") {
diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/diagnosticReportsFromTasksWithCC/build.gradle.kts b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/diagnosticReportsFromTasksWithCC/build.gradle.kts
new file mode 100644
index 0000000..40352ec
--- /dev/null
+++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/diagnosticReportsFromTasksWithCC/build.gradle.kts
@@ -0,0 +1,11 @@
+plugins {
+ id("org.jetbrains.kotlin.jvm")
+}
+
+repositories {
+ mavenLocal()
+ mavenCentral()
+}
+
+kotlin.jvmToolchain(11)
+kotlin.compilerOptions.jvmTarget.set(org.jetbrains.kotlin.gradle.dsl.JvmTarget.JVM_17)
diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/diagnosticReportsFromTasksWithCC/expectedOutput-cache-reused.txt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/diagnosticReportsFromTasksWithCC/expectedOutput-cache-reused.txt
new file mode 100644
index 0000000..7bc5321
--- /dev/null
+++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/diagnosticReportsFromTasksWithCC/expectedOutput-cache-reused.txt
@@ -0,0 +1,6 @@
+> Task :checkKotlinGradlePluginConfigurationErrors
+w: [InconsistentTargetCompatibilityForKotlinAndJavaTasks | WARNING] Inconsistent JVM-target compatibility detected for tasks 'compileJava' (11) and 'compileKotlin' (17).
+This will become an error in Gradle 8.0.
+Consider using JVM Toolchain: https://kotl.in/gradle/jvm/toolchain
+Learn more about JVM-target validation: https://kotl.in/gradle/jvm/target-validation
+#diagnostic-end
diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/diagnosticReportsFromTasksWithCC/expectedOutput-first-run.txt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/diagnosticReportsFromTasksWithCC/expectedOutput-first-run.txt
new file mode 100644
index 0000000..f836982
--- /dev/null
+++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/diagnosticReportsFromTasksWithCC/expectedOutput-first-run.txt
@@ -0,0 +1,8 @@
+> Configure project :
+
+> Task :checkKotlinGradlePluginConfigurationErrors
+w: [InconsistentTargetCompatibilityForKotlinAndJavaTasks | WARNING] Inconsistent JVM-target compatibility detected for tasks 'compileJava' (11) and 'compileKotlin' (17).
+This will become an error in Gradle 8.0.
+Consider using JVM Toolchain: https://kotl.in/gradle/jvm/toolchain
+Learn more about JVM-target validation: https://kotl.in/gradle/jvm/target-validation
+#diagnostic-end
diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/diagnosticReportsFromTasksWithCC/gradle.properties b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/diagnosticReportsFromTasksWithCC/gradle.properties
new file mode 100644
index 0000000..45559a9
--- /dev/null
+++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/diagnosticReportsFromTasksWithCC/gradle.properties
@@ -0,0 +1 @@
+kotlin.jvm.target.validation.mode = warning
diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/diagnosticReportsFromTasksWithCC/src/main/kotlin/org/jetbrains/Main.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/diagnosticReportsFromTasksWithCC/src/main/kotlin/org/jetbrains/Main.kt
new file mode 100644
index 0000000..2978f28
--- /dev/null
+++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/diagnosticReportsFromTasksWithCC/src/main/kotlin/org/jetbrains/Main.kt
@@ -0,0 +1,11 @@
+/*
+ * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains
+
+// Need to have some sources so that compileKotlin tasks actually run and report something
+fun main() {
+
+}
diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinPluginWrapper.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinPluginWrapper.kt
index b14e609d..c24370b 100644
--- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinPluginWrapper.kt
+++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinPluginWrapper.kt
@@ -34,7 +34,7 @@
import org.jetbrains.kotlin.gradle.plugin.diagnostics.*
import org.jetbrains.kotlin.gradle.plugin.diagnostics.ToolingDiagnosticRenderingOptions
import org.jetbrains.kotlin.gradle.plugin.diagnostics.UsesKotlinToolingDiagnostics
-import org.jetbrains.kotlin.gradle.plugin.diagnostics.kotlinToolingDiagnosticsCollectorProvider
+import org.jetbrains.kotlin.gradle.plugin.diagnostics.kotlinToolingDiagnosticsCollectorForConfiguration
import org.jetbrains.kotlin.gradle.plugin.diagnostics.launchKotlinGradleProjectCheckers
import org.jetbrains.kotlin.gradle.plugin.internal.*
import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinMultiplatformPlugin
@@ -274,13 +274,13 @@
}
private fun Project.setupDiagnosticsChecksAndReporting() {
- val collectorProvider = kotlinToolingDiagnosticsCollectorProvider
+ val collectorProvider = kotlinToolingDiagnosticsCollectorForConfiguration
val diagnosticRenderingOptions = ToolingDiagnosticRenderingOptions.forProject(this)
// Setup reporting from tasks
tasks.withType(UsesKotlinToolingDiagnostics::class.java).configureEach {
- it.usesService(collectorProvider)
- it.toolingDiagnosticsCollector.value(collectorProvider)
+ it.usesService(kotlinToolingDiagnosticsCollectorForExecution)
+ it.toolingDiagnosticsCollector.value(kotlinToolingDiagnosticsCollectorForExecution)
it.diagnosticRenderingOptions.set(diagnosticRenderingOptions)
}
diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/diagnostics/KotlinToolingDiagnosticsCollector.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/diagnostics/KotlinToolingDiagnosticsCollector.kt
index 5fdf482..4a87e1b 100644
--- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/diagnostics/KotlinToolingDiagnosticsCollector.kt
+++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/diagnostics/KotlinToolingDiagnosticsCollector.kt
@@ -7,28 +7,45 @@
import org.gradle.api.Project
import org.gradle.api.logging.Logger
+import org.gradle.api.provider.MapProperty
+import org.gradle.api.provider.Property
import org.gradle.api.provider.Provider
+import org.gradle.api.provider.SetProperty
import org.gradle.api.services.BuildService
import org.gradle.api.services.BuildServiceParameters
import org.jetbrains.kotlin.gradle.utils.registerClassLoaderScopedBuildService
-import java.util.*
-import java.util.concurrent.ConcurrentHashMap
+import java.io.Serializable
private typealias ToolingDiagnosticId = String
-internal abstract class KotlinToolingDiagnosticsCollector : BuildService<BuildServiceParameters.None> {
- /**
- * When collector is in transparent mode, any diagnostics received will be immediately rendered
- * instead of collected
- */
- @Volatile
- private var isTransparent: Boolean = false
+internal abstract class KotlinToolingDiagnosticsCollector : BuildService<KotlinToolingDiagnosticsCollector.Parameters> {
- private val rawDiagnosticsFromProject: MutableMap<ToolingDiagnostic.Location, MutableList<ToolingDiagnostic>> = ConcurrentHashMap()
- private val reportedIds: MutableSet<ToolingDiagnosticId> = Collections.newSetFromMap(ConcurrentHashMap())
+ abstract class Parameters : BuildServiceParameters, Serializable {
+ /**
+ * When collector is in transparent mode, any diagnostics received will be immediately rendered
+ * instead of collected
+ */
+ abstract val transparent: Property<Boolean>
- fun getDiagnosticsForLocation(location: ToolingDiagnostic.Location): Collection<ToolingDiagnostic> =
- rawDiagnosticsFromProject[location] ?: emptyList()
+ abstract val rawDiagnosticsFromProject: MapProperty<ToolingDiagnostic.Location, MutableList<ToolingDiagnostic>>
+ abstract val reportedIds: SetProperty<ToolingDiagnosticId>
+
+ fun copyStateFrom(other: Parameters) {
+ this.rawDiagnosticsFromProject.putAll(
+ // .map { it } is a workaround for a weird Gradle exception/bug:
+ //
+ // java.lang.IllegalArgumentException: Cannot set the value of a property of type java.util.Map with key type
+ // org.jetbrains.kotlin.gradle.plugin.diagnostics.ToolingDiagnostic$Location and value type java.util.List using a
+ // provider with key type java.lang.Object and value type java.lang.Object.
+ other.rawDiagnosticsFromProject.map { it }
+ )
+ this.reportedIds.addAll(other.reportedIds)
+ this.transparent.set(other.transparent)
+ }
+ }
+
+ fun getDiagnosticsForLocation(location: ToolingDiagnostic.Location): Collection<ToolingDiagnostic> =
+ parameters.rawDiagnosticsFromProject.get()[location] ?: emptyList()
fun report(project: Project, diagnostic: ToolingDiagnostic) {
val location = project.toLocation()
@@ -56,7 +73,7 @@
}
fun switchToTransparentMode() {
- isTransparent = true
+ parameters.transparent.set(true)
}
private fun handleDiagnostic(
@@ -68,30 +85,47 @@
) {
// 1. Check suppression or duplicated reporting. Reporting a suppressed or duplicated diagnostic shouldn't cause any side-effects,
// so we're returning right away if it is suppressed
- if (diagnostic.isSuppressed(options) || deduplicationKey != null && !reportedIds.add(deduplicationKey)) return
+ if (diagnostic.isSuppressed(options) || deduplicationKey != null && parameters.reportedIds.get().contains(deduplicationKey)) return
// 2. Store diagnostic. Note that we don't care about any external user-visible effects this diagnostic causes.
// As a specific consequence, stored diagnostics can be FATAL. This shouldn't make any difference on production, but is convenient
// for tests
- rawDiagnosticsFromProject.compute(location) { _, previousListIfAny ->
- previousListIfAny?.apply { add(diagnostic) } ?: mutableListOf(diagnostic)
- }
+ if (deduplicationKey != null) parameters.reportedIds.add(deduplicationKey)
+
+ val updatedValue = parameters.rawDiagnosticsFromProject
+ .getting(location)
+ .getOrElse(mutableListOf())
+ .apply { add(diagnostic) }
+ parameters.rawDiagnosticsFromProject.put(location, updatedValue)
// 3. Produce user-visible effects if necessary
when {
diagnostic.severity == ToolingDiagnostic.Severity.FATAL -> throw diagnostic.createAnExceptionForFatalDiagnostic(options)
- isTransparent -> renderReportedDiagnostic(diagnostic, logger, options)
+ parameters.transparent.get() -> renderReportedDiagnostic(diagnostic, logger, options)
}
}
}
-internal val Project.kotlinToolingDiagnosticsCollectorProvider: Provider<KotlinToolingDiagnosticsCollector>
- get() = gradle.registerClassLoaderScopedBuildService(KotlinToolingDiagnosticsCollector::class)
+internal val Project.kotlinToolingDiagnosticsCollectorForConfiguration: Provider<KotlinToolingDiagnosticsCollector>
+ get() = gradle.registerClassLoaderScopedBuildService(KotlinToolingDiagnosticsCollector::class) {
+ it.parameters.apply {
+ reportedIds.set(emptySet())
+ rawDiagnosticsFromProject.set(emptyMap())
+ transparent.set(false)
+ }
+ }
+internal val Project.kotlinToolingDiagnosticsCollectorForExecution: Provider<KotlinToolingDiagnosticsCollector>
+ get() {
+ val kClass = KotlinToolingDiagnosticsCollector::class
+ return gradle.sharedServices.registerIfAbsent("${kClass.simpleName}_EXECUTION_${kClass.java.classLoader.hashCode()}", kClass.java) {
+ it.parameters.copyStateFrom(kotlinToolingDiagnosticsCollector.parameters)
+ }
+ }
internal val Project.kotlinToolingDiagnosticsCollector: KotlinToolingDiagnosticsCollector
- get() = kotlinToolingDiagnosticsCollectorProvider.get()
+ get() = kotlinToolingDiagnosticsCollectorForConfiguration.get()
internal fun Project.reportDiagnostic(diagnostic: ToolingDiagnostic) {
kotlinToolingDiagnosticsCollector.report(this, diagnostic)
diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/diagnostics/ToolingDiagnostic.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/diagnostics/ToolingDiagnostic.kt
index 2179547..80689fd 100644
--- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/diagnostics/ToolingDiagnostic.kt
+++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/diagnostics/ToolingDiagnostic.kt
@@ -8,6 +8,7 @@
import org.gradle.api.Project
import org.gradle.api.Task
import org.jetbrains.kotlin.gradle.InternalKotlinGradlePluginApi
+import java.io.Serializable
/**
* Represents a reported instance of a diagnostic.
@@ -24,7 +25,7 @@
@InternalKotlinGradlePluginApi // used in integration tests
data class ToolingDiagnostic(
val factoryId: String, val message: String, val severity: Severity
-) {
+) : Serializable {
/**
* Stacktrace pointing where the original cause of the diagnostic happened. Note that it is not necessarily
* the stacktrace of where the diagnostic has been reported
@@ -92,7 +93,7 @@
/**
* [path] is a standard fully qualified Gradle path
*/
- sealed class Location {
+ sealed class Location : Serializable {
data class GradleProject(val path: String) : Location()
data class GradleTask(val path: String) : Location()
}
