Improve JapiCmp: avoid misses, improve reporting and exclusions (#3198)

This PR improves the japicmp integration in the build in several ways:

1) Add a finalizedBy task that prints a filtered report
The basic text report uses * and ! as markers for binary and source
incompatible changes respectively. Additionally, a MODIFIED class is
marked with ***. This task attempts at pinpointing the incompatible
changes only in the report, and printing these lines. This is called out
in the ci job when prepare step fails fast.

2) Ensure sourceModified changes are considered
See reactor/reactor#722.
If a change is binary compatible but not source compatible, using
`onlyBinaryCompatibleModified = true` will mask the issue. This
change uses `onlyModified = true` instead. This is slightly more
verbose, but this is alleviated by (1).

3) Bump to japicmp plugin v0.4.1, exclude NEW_DEFAULT_METHOD as a whole
The new `compatibilityChangeExcludes` parameter allows us to ignore all
occurrences of a default method added to an interface, instead of
having to exclude each such method one by one.

.github/workflows/ci.yml

@@ -33,7 +33,10 @@ jobs:
           echo -e "\n - \u001b[1mSpotless (license headers)\u001b[0m failures on touched java files \033[38;5;255;48;5;0m\u001b[1mcan be automatically fixed by running\u001b[0m:"
           echo -e "   \033[38;5;0;48;5;255m ./gradlew spotlessApply \033[0m"
           echo -e "\n - \u001b[1mAPI Compatibility\u001b[0m failures should be considered carefully and \033[38;5;255;48;5;0m\u001b[1mdiscussed with maintainers in the PR\u001b[0m"
-          echo "   If there are failures, the detail should be available in the logs of the api compatibility step above"
+          echo "   If there are failures, the detail should be available in the step's log:"
+          echo -e "   Look for the \033[38;5;0;48;5;255m API compatibility failures \033[0m block(s)."
+          echo "   Alternatively, locally run the following command to get access to the full report:"
+          echo -e "   \033[38;5;0;48;5;255m ./gradlew japicmp \033[0m"
           echo ""
           exit -1
   core-fast:

gradle/libs.versions.toml

@@ -44,7 +44,7 @@ asciidoctor-convert = { id = "org.asciidoctor.jvm.convert", version.ref = "ascii
 asciidoctor-pdf = { id = "org.asciidoctor.jvm.pdf", version.ref = "asciidoctor" }
 bnd = { id = "biz.aQute.bnd.builder", version = "6.3.1" }
 download = { id = "de.undercouch.download", version = "5.1.2" }
-japicmp = { id = "me.champeau.gradle.japicmp", version = "0.4.0" }
+japicmp = { id = "me.champeau.gradle.japicmp", version = "0.4.1" }
 jcstress = { id = "io.github.reyerizo.gradle.jcstress", version = "0.8.13" }
 kotlin = { id = "org.jetbrains.kotlin.jvm", version.ref = "kotlin" }
 nohttp = { id = "io.spring.nohttp", version = "0.0.10" }

reactor-core/build.gradle

@@ -135,6 +135,28 @@ task downloadBaseline(type: Download) {
 	dest "${buildDir}/baselineLibs/reactor-core-${libs.versions.baseline.core.api.get()}.jar"
 }
 
+def japicmpReport = tasks.register('japicmpReport') {
+	onlyIf {
+		japicmp.state.failure != null
+	}
+	doLast {
+		def reportFile = file("${project.buildDir}/reports/japi.txt")
+		if (reportFile.exists()) {
+			println "\n **********************************"
+			println " * /!\\ API compatibility failures *"
+			println " **********************************"
+			println "Japicmp report was filtered and interpreted to find the following incompatibilities:"
+			reportFile.eachLine {
+				if (it.contains("*") && (!it.contains("***") || it.contains("****")))
+					println "source incompatible change: $it"
+				else if (it.contains("!"))
+					println "binary incompatible change: $it"
+			}
+		}
+		else println "No incompatible change to report"
+	}
+}
+
 task japicmp(type: JapicmpTask) {
 	if (project.gradle.startParameter.isOffline()) {
 		println "Offline: skipping downloading of baseline and JAPICMP"
@@ -148,16 +170,21 @@ task japicmp(type: JapicmpTask) {
 		println "Will download and perform baseline comparison with ${libs.versions.baseline.core.api.get()}"
 		dependsOn(downloadBaseline)
 		dependsOn(jar)
+		finalizedBy(japicmpReport)
 	}
 
 	oldClasspath.from(files("${buildDir}/baselineLibs/reactor-core-${libs.versions.baseline.core.api.get()}.jar"))
 	newClasspath.from(files(jar.archiveFile))
-	onlyBinaryIncompatibleModified = true
+	// these onlyXxx parameters result in a report that is slightly too noisy, but better than
+	// onlyBinaryIncompatibleModified = true which masks source-incompatible-only changes
+	onlyBinaryIncompatibleModified = false
+	onlyModified = true
 	failOnModification = true
 	failOnSourceIncompatibility = true
 	txtOutputFile = file("${project.buildDir}/reports/japi.txt")
 	ignoreMissingClasses = true
 	includeSynthetic = true
+	compatibilityChangeExcludes = [ "METHOD_NEW_DEFAULT" ]
 
 	// TODO after a .0 release, bump the gradle.properties baseline
 	// TODO after a .0 release, remove the reactor-core exclusions below if any

reactor-test/build.gradle

@@ -64,28 +64,55 @@ task downloadBaseline(type: Download) {
 	finalizedBy { japicmp }
 }
 
+def japicmpReport = tasks.register('japicmpReport') {
+	onlyIf {
+		japicmp.state.failure != null
+	}
+	doLast {
+		def reportFile = file("${project.buildDir}/reports/japi.txt")
+		if (reportFile.exists()) {
+			println "\n *********************************"
+			println " * /!\\ API compatibility failures *"
+			println " **********************************"
+			println "Japicmp report was filtered and interpreted to find the following incompatibilities:"
+			reportFile.eachLine {
+				if (it.contains("*") && (!it.contains("***") || it.contains("****")))
+					println "source incompatible change: $it"
+				else if (it.contains("!"))
+					println "binary incompatible change: $it"
+			}
+		}
+		else println "No incompatible change to report"
+	}
+}
+
 task japicmp(type: JapicmpTask) {
 	if (project.gradle.startParameter.isOffline()) {
 		println "Offline: skipping downloading of baseline and JAPICMP"
-	  	enabled = false
+		enabled = false
 	}
 	else if ("${libs.versions.baseline.core.api.get()}" == "SKIP") {
 		println "SKIP: Instructed to skip the baseline comparison"
-	  	enabled = false
+		enabled = false
 	}
 	else {
 		println "Will download and perform baseline comparison with ${libs.versions.baseline.core.api.get()}"
-	  	dependsOn(downloadBaseline)
+		dependsOn(downloadBaseline)
+		finalizedBy(japicmpReport)
 	}
 
 	oldClasspath.from(files("${buildDir}/baselineLibs/reactor-test-${libs.versions.baseline.core.api.get()}.jar"))
 	newClasspath.from(files(jar.archiveFile))
-	onlyBinaryIncompatibleModified = true
+	// these onlyXxx parameters result in a report that is slightly too noisy, but better than
+	// onlyBinaryIncompatibleModified = true which masks source-incompatible-only changes
+	onlyBinaryIncompatibleModified = false
+	onlyModified = true
 	failOnModification = true
 	failOnSourceIncompatibility = true
 	txtOutputFile = file("${project.buildDir}/reports/japi.txt")
 	ignoreMissingClasses = true
 	includeSynthetic = true
+	compatibilityChangeExcludes = [ "METHOD_NEW_DEFAULT" ]
 
 	// TODO after a .0 release, remove the reactor-test exclusions below if any
 	methodExcludes = [ ]