Merge pull request #2111 from EngineHub/feature/abi-more-like-allthin…

…gs-break-intermittenly

Add automatic ABI checking

buildSrc/build.gradle.kts

@@ -48,6 +48,7 @@ dependencies {
     implementation(gradleApi())
     implementation("gradle.plugin.org.cadixdev.gradle:licenser:0.6.1")
     implementation("org.ajoberstar.grgit:grgit-gradle:4.1.1")
+    implementation("me.champeau.gradle:japicmp-gradle-plugin:0.4.0")
     implementation("gradle.plugin.com.github.johnrengelman:shadow:7.1.2")
     implementation("org.jfrog.buildinfo:build-info-extractor-gradle:4.27.1")
     implementation("org.spongepowered:spongegradle-plugin-development:2.0.1")

buildSrc/src/main/kotlin/japicmp/accept/AbstractAcceptingRule.kt

@@ -0,0 +1,52 @@
+package japicmp.accept
+
+import com.google.gson.Gson
+import com.google.gson.reflect.TypeToken
+import com.google.gson.stream.JsonWriter
+import japicmp.model.JApiCompatibility
+import me.champeau.gradle.japicmp.report.AbstractContextAwareViolationRule
+import me.champeau.gradle.japicmp.report.Violation
+import java.io.StringWriter
+
+abstract class AbstractAcceptingRule : AbstractContextAwareViolationRule() {
+    fun checkAcceptance(
+        member: JApiCompatibility,
+        changes: List<String>,
+        rejection: Violation
+    ): Violation {
+        val changeParams = context.changeParams
+        val seenApiChanges = context.seenApiChanges
+        val change = ApiChange(
+            context.className,
+            Violation.describe(member),
+            changes
+        )
+        val reason = changeParams.changeToReason.get(change)
+        if (reason != null) {
+            seenApiChanges.add(change)
+            return Violation.accept(
+                member,
+                "${rejection.humanExplanation}. Reason for accepting this: <b>$reason</b>"
+            )
+        }
+        return Violation.error(
+            member,
+            rejection.humanExplanation + """.
+                <br>
+                <p>
+                In order to accept this change add the following to <code>verification/src/changes/${changeParams.changeFileName}</code>:
+                <pre>${prettyPrintJson(mapOf("[REASON CHANGE IS OKAY]" to listOf(change)))}</pre>
+                </p>
+            """.trimIndent()
+        )
+    }
+
+    private fun prettyPrintJson(acceptanceJson: Any): String {
+        val stringWriter = StringWriter()
+        JsonWriter(stringWriter).use {
+            it.setIndent("    ")
+            Gson().toJson(acceptanceJson, object : TypeToken<ApiChangesDiskFormat>() {}.type, it)
+        }
+        return stringWriter.toString()
+    }
+}

buildSrc/src/main/kotlin/japicmp/accept/AcceptedRegressionsRulePostProcess.kt

@@ -0,0 +1,17 @@
+package japicmp.accept
+
+import me.champeau.gradle.japicmp.report.PostProcessViolationsRule
+import me.champeau.gradle.japicmp.report.ViolationCheckContextWithViolations
+
+class AcceptedRegressionsRulePostProcess : PostProcessViolationsRule {
+    override fun execute(context: ViolationCheckContextWithViolations) {
+        val changeParams = context.changeParams
+        val seenApiChanges = context.seenApiChanges
+        val left = HashSet(changeParams.changeToReason.keys)
+        left.removeAll(seenApiChanges)
+        if (!left.isEmpty()) {
+            val formattedLeft: String = left.joinToString(separator = "\n") { it.toString() }
+            throw RuntimeException("The following regressions are declared as accepted, but didn't match any rule:\n\n$formattedLeft")
+        }
+    }
+}

buildSrc/src/main/kotlin/japicmp/accept/AcceptingSetupRule.kt

@@ -0,0 +1,28 @@
+package japicmp.accept
+
+import me.champeau.gradle.japicmp.report.SetupRule
+import me.champeau.gradle.japicmp.report.ViolationCheckContext
+import java.nio.file.Path
+import java.nio.file.Paths
+
+class AcceptingSetupRule(private val params: Map<String, String>) : SetupRule {
+    companion object {
+        fun createParams(changeFile: Path): Map<String, String> {
+            return mapOf(
+                "changeFile" to changeFile.toAbsolutePath().toString(),
+                "fileName" to changeFile.fileName.toString()
+            )
+        }
+    }
+
+    override fun execute(t: ViolationCheckContext) {
+        @Suppress("UNCHECKED_CAST")
+        val userData = t.userData as MutableMap<String, Any>
+        userData["changeParams"] = ChangeParams(
+            changeToReason = loadAcceptedApiChanges(Paths.get(params["changeFile"]!!)),
+            changeFileName = params["fileName"]!!,
+        )
+        userData["seenApiChanges"] = HashSet<ApiChange>()
+    }
+
+}

buildSrc/src/main/kotlin/japicmp/accept/BinaryCompatRule.kt

@@ -0,0 +1,38 @@
+package japicmp.accept
+
+import japicmp.model.*
+import me.champeau.gradle.japicmp.report.Violation
+
+
+class BinaryCompatRule() : AbstractAcceptingRule() {
+    private val IGNORED_CHANGE_TYPES: List<JApiCompatibilityChange> = listOf(
+        JApiCompatibilityChange.METHOD_REMOVED_IN_SUPERCLASS,  // the removal of the method will be reported
+        JApiCompatibilityChange.INTERFACE_REMOVED,  // the removed methods will be reported
+        JApiCompatibilityChange.INTERFACE_ADDED, // the added methods will be reported
+    )
+
+    override fun maybeViolation(member: JApiCompatibility): Violation? {
+        if (member.isBinaryCompatible) {
+            return null
+        }
+        if (member is JApiClass && member.compatibilityChanges.isEmpty()) {
+            // A member of the class breaks binary compatibility.
+            // That will be handled when the member is passed to `maybeViolation`.
+            return null
+        }
+        if (member is JApiImplementedInterface) {
+            // The changes about the interface's methods will be reported already
+            return null
+        }
+        for (change in member.compatibilityChanges) {
+            if (IGNORED_CHANGE_TYPES.contains(change)) {
+                return null
+            }
+        }
+        return checkAcceptance(
+            member,
+            member.compatibilityChanges.map { it.name },
+            Violation.notBinaryCompatible(member),
+        )
+    }
+}

buildSrc/src/main/kotlin/japicmp/accept/ChangeParams.kt

@@ -0,0 +1,6 @@
+package japicmp.accept
+
+data class ChangeParams(
+    val changeToReason: Map<ApiChange, String>,
+    val changeFileName: String,
+)

buildSrc/src/main/kotlin/japicmp/accept/LICENSE.md

@@ -0,0 +1,3 @@
+Code in this package is mostly taken and adapted from
+[Gradle's binary compat checking](https://github.com/gradle/gradle/blob/master/build-logic/binary-compatibility),
+which is licensed under the Apache License 2.0.

buildSrc/src/main/kotlin/japicmp/accept/apichanges.kt

@@ -0,0 +1,22 @@
+package japicmp.accept
+
+import com.google.gson.Gson
+import com.google.gson.reflect.TypeToken
+import java.nio.file.Files
+import java.nio.file.Path
+
+typealias ApiChangesDiskFormat = Map<String, List<ApiChange>>;
+fun loadAcceptedApiChanges(path: Path): Map<ApiChange, String> {
+    val fromDisk: ApiChangesDiskFormat = Files.newBufferedReader(path).use {
+        Gson().fromJson(it, object : TypeToken<ApiChangesDiskFormat>() {}.type)
+    }
+    return fromDisk.asSequence().flatMap { (key, value) ->
+        value.asSequence().map { it to key }
+    }.toMap()
+}
+
+data class ApiChange(
+    val type: String,
+    val member: String,
+    val changes: List<String>,
+)

buildSrc/src/main/kotlin/japicmp/accept/userdata.kt

@@ -0,0 +1,10 @@
+package japicmp.accept
+
+import me.champeau.gradle.japicmp.report.ViolationCheckContext
+
+val ViolationCheckContext.changeParams
+    get() = userData["changeParams"] as ChangeParams
+
+@Suppress("UNCHECKED_CAST")
+val ViolationCheckContext.seenApiChanges
+    get() = userData["seenApiChanges"] as MutableSet<ApiChange>

settings.gradle.kts

@@ -14,3 +14,5 @@ include("worldedit-mod")
 include("worldedit-libs:core:ap")
 
 include("worldedit-core:doctools")
+
+include("verification")

verification/build.gradle.kts

@@ -0,0 +1,130 @@
+import japicmp.accept.AcceptingSetupRule
+import japicmp.accept.BinaryCompatRule
+import me.champeau.gradle.japicmp.JapicmpTask
+import org.gradle.internal.resolve.ModuleVersionNotFoundException
+import java.nio.charset.StandardCharsets
+import java.nio.file.Files
+import java.util.*
+
+plugins {
+    base
+    id("me.champeau.gradle.japicmp")
+}
+
+repositories {
+    maven {
+        name = "EngineHub Repository (Releases Only)"
+        url = uri("https://maven.enginehub.org/artifactory/libs-release-local/")
+    }
+    mavenCentral()
+}
+
+val resetAcceptedApiChangesFiles by tasks.registering {
+    group = "API Compatibility"
+    description = "Resets ALL the accepted API changes files"
+}
+
+val checkApiCompatibility by tasks.registering {
+    group = "API Compatibility"
+    description = "Checks ALL API compatibility"
+}
+
+tasks.check {
+    dependsOn(checkApiCompatibility)
+}
+
+// Generic setup for all tasks
+// Pull the version before our current version.
+val baseVersion = "(,${rootProject.version.toString().substringBefore("-SNAPSHOT")}["
+for (projectFragment in listOf("bukkit", "cli", "core", "fabric", "forge", "sponge")) {
+    val capitalizedFragment = projectFragment.capitalize(Locale.ROOT)
+    val proj = project(":worldedit-$projectFragment")
+    evaluationDependsOn(proj.path)
+
+    val changeFile = project.file("src/changes/accepted-$projectFragment-public-api-changes.json").toPath()
+
+    val resetChangeFileTask = tasks.register("reset${capitalizedFragment}AcceptedApiChangesFile") {
+        group = "API Compatibility"
+        description = "Reset the accepted API changes file for $projectFragment"
+
+        doFirst {
+            Files.newBufferedWriter(changeFile, StandardCharsets.UTF_8).use {
+                it.write("{\n}")
+            }
+        }
+    }
+    resetAcceptedApiChangesFiles {
+        dependsOn(resetChangeFileTask)
+    }
+
+    val conf = configurations.create("${projectFragment}OldJar") {
+        isCanBeResolved = true
+    }
+    val projPublication = proj.the<PublishingExtension>().publications.getByName<MavenPublication>("maven")
+    conf.dependencies.add(
+        dependencies.create("${projPublication.groupId}:${projPublication.artifactId}:$baseVersion").apply {
+            (this as? ModuleDependency)?.isTransitive = false
+        }
+    )
+    val checkApi = tasks.register<JapicmpTask>("check${capitalizedFragment}ApiCompatibility") {
+        group = "API Compatibility"
+        description = "Check API compatibility for $capitalizedFragment API"
+        richReport {
+            addSetupRule(
+                AcceptingSetupRule::class.java, AcceptingSetupRule.createParams(
+                    changeFile,
+                )
+            )
+            addRule(BinaryCompatRule::class.java)
+            reportName.set("api-compatibility-$projectFragment.html")
+        }
+
+        onlyIf {
+            // Only check if we have a jar to compare against
+            try {
+                conf.resolvedConfiguration.rethrowFailure()
+                true
+            } catch (e: ResolveException) {
+                if (e.cause is ModuleVersionNotFoundException) {
+                    it.logger.warn("Skipping check for $projectFragment API compatibility because there is no jar to compare against")
+                    false
+                } else {
+                    throw e
+                }
+            }
+        }
+
+        oldClasspath.from(conf)
+        newClasspath.from(proj.tasks.named("jar"))
+        onlyModified.set(false)
+        failOnModification.set(false) // report does the failing (so we can accept)
+        ignoreMissingClasses.set(true)
+
+        // Internals are not API
+        packageExcludes.add("com.sk89q.worldedit*.internal*")
+        // Mixins are not API
+        packageExcludes.add("com.sk89q.worldedit*.mixin*")
+    }
+
+    checkApiCompatibility {
+        dependsOn(checkApi)
+    }
+}
+
+// Specific project overrides
+tasks.named<JapicmpTask>("checkCoreApiCompatibility") {
+    // Commands are not API
+    packageExcludes.add("com.sk89q.worldedit.command*")
+}
+tasks.named<JapicmpTask>("checkBukkitApiCompatibility") {
+    // Internal Adapters are not API
+    packageExcludes.add("com.sk89q.worldedit.bukkit.adapter*")
+}
+tasks.named<JapicmpTask>("checkFabricApiCompatibility") {
+    // Need to check against the reobf JAR
+    newClasspath.setFrom(project(":worldedit-fabric").tasks.named("remapJar"))
+}
+tasks.named<JapicmpTask>("checkForgeApiCompatibility") {
+    // Need to check against the reobf JAR
+    newClasspath.builtBy(project(":worldedit-forge").tasks.named("reobfJar"))
+}

verification/src/changes/accepted-bukkit-public-api-changes.json

@@ -0,0 +1,2 @@
+{
+}

verification/src/changes/accepted-cli-public-api-changes.json

@@ -0,0 +1,2 @@
+{
+}