From 0b12e272ec0683c4b8ec6d5949387a7cddcb8e50 Mon Sep 17 00:00:00 2001 From: Viktor De Pasquale Date: Fri, 2 Feb 2024 15:02:43 +0100 Subject: [PATCH] Add incremental validation Validation is modified when using `useIncrementalValidation=true` to not fail when encountering additions, which are known to be backwards compatible. Instead spits out warnings to allow developers commiting the file early with the changes they have made. The use case is to pair it with CI-run :check (:apiDump) task and commit updated API after merge - considering all steps are automated. In cases where the API is not compatible, the task fails similarly to `useIncrementalValidation=false` or simply _the default_. It's acknowledged that using `useIncrementalValidation=true` might cause issues when building upon a feature (or PR) in which cases the .api file might be left out in its previous state, not commited to the VCS. In which case the developer might prefer leaving the option in the default state (false). To the naked eye it might be apparent that checking only removals (leading `-`) is naive, but since the api diff add two distinct lines for changes (one leading `-`, other `+`), this naive approach proves working for all considered use-cases. --- README.md | 10 ++ api/binary-compatibility-validator.api | 4 + .../validation/test/IncrementalTest.kt | 149 ++++++++++++++++++ .../examples/classes/Incremental.dump | 16 ++ .../examples/classes/IncrementalAddition.kt | 17 ++ .../examples/classes/IncrementalBase.kt | 14 ++ .../classes/IncrementalModification.kt | 14 ++ .../examples/classes/IncrementalRemoval.kt | 8 + .../incremental/incremental.gradle.kts | 8 + src/main/kotlin/ApiValidationExtension.kt | 14 +- .../BinaryCompatibilityValidatorPlugin.kt | 1 + src/main/kotlin/IncrementalVerification.kt | 32 ++++ src/main/kotlin/KotlinApiCompareTask.kt | 25 +-- src/main/kotlin/Verification.kt | 10 ++ src/main/kotlin/VerificationStrict.kt | 15 ++ 15 files changed, 322 insertions(+), 15 deletions(-) create mode 100644 src/functionalTest/kotlin/kotlinx/validation/test/IncrementalTest.kt create mode 100644 src/functionalTest/resources/examples/classes/Incremental.dump create mode 100644 src/functionalTest/resources/examples/classes/IncrementalAddition.kt create mode 100644 src/functionalTest/resources/examples/classes/IncrementalBase.kt create mode 100644 src/functionalTest/resources/examples/classes/IncrementalModification.kt create mode 100644 src/functionalTest/resources/examples/classes/IncrementalRemoval.kt create mode 100644 src/functionalTest/resources/examples/gradle/configuration/incremental/incremental.gradle.kts create mode 100644 src/main/kotlin/IncrementalVerification.kt create mode 100644 src/main/kotlin/Verification.kt create mode 100644 src/main/kotlin/VerificationStrict.kt diff --git a/README.md b/README.md index 594ee5fd..a8ca74b7 100644 --- a/README.md +++ b/README.md @@ -92,6 +92,11 @@ apiValidation { */ validationDisabled = true + /** + * Flag to allow incremental validation, ie. apiCheck task will not fail for incremental changes. + */ + useIncrementalValidation = true + /** * A path to a subdirectory inside the project root directory where dumps should be stored. */ @@ -131,6 +136,11 @@ apiValidation { */ validationDisabled = false + /** + * Flag to allow incremental validation, ie. apiCheck task will not fail for incremental changes. + */ + useIncrementalValidation = true + /** * A path to a subdirectory inside the project root directory where dumps should be stored. */ diff --git a/api/binary-compatibility-validator.api b/api/binary-compatibility-validator.api index 07d3bc3d..67662460 100644 --- a/api/binary-compatibility-validator.api +++ b/api/binary-compatibility-validator.api @@ -10,6 +10,7 @@ public class kotlinx/validation/ApiValidationExtension { public final fun getPublicClasses ()Ljava/util/Set; public final fun getPublicMarkers ()Ljava/util/Set; public final fun getPublicPackages ()Ljava/util/Set; + public final fun getUseIncrementalValidation ()Z public final fun getValidationDisabled ()Z public final fun klib (Lkotlin/jvm/functions/Function1;)V public final fun setAdditionalSourceSets (Ljava/util/Set;)V @@ -21,6 +22,7 @@ public class kotlinx/validation/ApiValidationExtension { public final fun setPublicClasses (Ljava/util/Set;)V public final fun setPublicMarkers (Ljava/util/Set;)V public final fun setPublicPackages (Ljava/util/Set;)V + public final fun setUseIncrementalValidation (Z)V public final fun setValidationDisabled (Z)V } @@ -80,8 +82,10 @@ public class kotlinx/validation/KotlinApiCompareTask : org/gradle/api/DefaultTas public field projectApiFile Ljava/io/File; public fun (Lorg/gradle/api/model/ObjectFactory;)V public final fun getGeneratedApiFile ()Ljava/io/File; + public final fun getIncremental ()Z public final fun getProjectApiFile ()Ljava/io/File; public final fun setGeneratedApiFile (Ljava/io/File;)V + public final fun setIncremental (Z)V public final fun setProjectApiFile (Ljava/io/File;)V } diff --git a/src/functionalTest/kotlin/kotlinx/validation/test/IncrementalTest.kt b/src/functionalTest/kotlin/kotlinx/validation/test/IncrementalTest.kt new file mode 100644 index 00000000..be89c580 --- /dev/null +++ b/src/functionalTest/kotlin/kotlinx/validation/test/IncrementalTest.kt @@ -0,0 +1,149 @@ +/* + * Copyright 2016-2024 JetBrains s.r.o. + * Use of this source code is governed by the Apache 2.0 License that can be found in the LICENSE.txt file. + */ + +package kotlinx.validation.test + +import kotlinx.validation.api.* +import org.junit.* + +class IncrementalTest : BaseKotlinGradleTest() { + + @Test + fun `fails when removing source lines`() { + val runner = test { + buildGradleKts { + resolve("/examples/gradle/base/withPlugin.gradle.kts") + resolve("/examples/gradle/configuration/incremental/incremental.gradle.kts") + } + kotlin("IncrementalBase.kt") { + resolve("/examples/classes/IncrementalRemoval.kt") + } + apiFile(rootProjectDir.name) { + resolve("/examples/classes/Incremental.dump") + } + runner { + arguments.add(":apiCheck") + } + } + runner.buildAndFail().apply { + assertTaskFailure(":apiCheck") + } + } + + @Test + fun `fails when modifying source lines`() { + val runner = test { + buildGradleKts { + resolve("/examples/gradle/base/withPlugin.gradle.kts") + resolve("/examples/gradle/configuration/incremental/incremental.gradle.kts") + } + kotlin("IncrementalBase.kt") { + resolve("/examples/classes/IncrementalModification.kt") + } + apiFile(rootProjectDir.name) { + resolve("/examples/classes/Incremental.dump") + } + runner { + arguments.add(":apiCheck") + } + } + runner.buildAndFail().apply { + assertTaskFailure(":apiCheck") + } + } + + @Test + fun `succeeds when adding source lines`() { + val runner = test { + buildGradleKts { + resolve("/examples/gradle/base/withPlugin.gradle.kts") + resolve("/examples/gradle/configuration/incremental/incremental.gradle.kts") + } + kotlin("IncrementalBase.kt") { + resolve("/examples/classes/IncrementalAddition.kt") + } + apiFile(rootProjectDir.name) { + resolve("/examples/classes/Incremental.dump") + } + runner { + arguments.add(":apiCheck") + } + } + runner.build().apply { + assertTaskSuccess(":apiCheck") + } + } + + @Test + fun `does not dump when removing source lines`() { + val runner = test { + buildGradleKts { + resolve("/examples/gradle/base/withPlugin.gradle.kts") + resolve("/examples/gradle/configuration/incremental/incremental.gradle.kts") + } + kotlin("IncrementalBase.kt") { + resolve("/examples/classes/IncrementalRemoval.kt") + } + apiFile(rootProjectDir.name) { + resolve("/examples/classes/Incremental.dump") + } + runner { + arguments.add(":apiCheck") + } + } + runner.buildAndFail().apply { + assertTaskFailure(":apiCheck") + assertTaskNotRun(":apiDump") + } + } + + @Test + fun `does not dump when modifying source lines`() { + val runner = test { + buildGradleKts { + resolve("/examples/gradle/base/withPlugin.gradle.kts") + resolve("/examples/gradle/configuration/incremental/incremental.gradle.kts") + } + kotlin("IncrementalBase.kt") { + resolve("/examples/classes/IncrementalModification.kt") + } + apiFile(rootProjectDir.name) { + resolve("/examples/classes/Incremental.dump") + } + runner { + arguments.add(":apiCheck") + } + } + runner.buildAndFail().apply { + assertTaskFailure(":apiCheck") + assertTaskNotRun(":apiDump") + } + } + + @Ignore + @Test + fun `updates dump when adding source lines`() { + val runner = test { + buildGradleKts { + resolve("/examples/gradle/base/withPlugin.gradle.kts") + resolve("/examples/gradle/configuration/incremental/incremental.gradle.kts") + } + kotlin("IncrementalBase.kt") { + resolve("/examples/classes/IncrementalAddition.kt") + } + apiFile(rootProjectDir.name) { + resolve("/examples/classes/Incremental.dump") + } + runner { + arguments.add(":apiCheck") + } + } + runner.build().apply { + assertTaskSuccess(":apiCheck") + assertTaskSuccess(":apiDump") + } + } + +} \ No newline at end of file diff --git a/src/functionalTest/resources/examples/classes/Incremental.dump b/src/functionalTest/resources/examples/classes/Incremental.dump new file mode 100644 index 00000000..36ad9a6b --- /dev/null +++ b/src/functionalTest/resources/examples/classes/Incremental.dump @@ -0,0 +1,16 @@ +public final class foo/Incremental { + public fun (Ljava/lang/Object;)V + public final fun component1 ()Ljava/lang/Object; + public final fun copy (Ljava/lang/Object;)Lfoo/Incremental; + public static synthetic fun copy$default (Lfoo/Incremental;Ljava/lang/Object;ILjava/lang/Object;)Lfoo/Incremental; + public fun equals (Ljava/lang/Object;)Z + public final fun getId ()Ljava/lang/Object; + public fun hashCode ()I + public final fun integer ()I + public fun toString ()Ljava/lang/String; +} + +public final class foo/IncrementalBaseKt { + public static final fun getId ()Ljava/lang/Object; + public static final fun sum ([I)I +} \ No newline at end of file diff --git a/src/functionalTest/resources/examples/classes/IncrementalAddition.kt b/src/functionalTest/resources/examples/classes/IncrementalAddition.kt new file mode 100644 index 00000000..2f6500bb --- /dev/null +++ b/src/functionalTest/resources/examples/classes/IncrementalAddition.kt @@ -0,0 +1,17 @@ +/* + * Copyright 2016-2024 JetBrains s.r.o. + * Use of this source code is governed by the Apache 2.0 License that can be found in the LICENSE.txt file. + */ + +package foo + +data class Incremental(val id: Any) { + fun integer() = 42 + fun double() = 4.2 +} + +fun sum(vararg integers: Int) = integers.sum() +fun mus(vararg integers: Int) = integers.sum() + +val id: Any = 42 +val id2: Any = 24 \ No newline at end of file diff --git a/src/functionalTest/resources/examples/classes/IncrementalBase.kt b/src/functionalTest/resources/examples/classes/IncrementalBase.kt new file mode 100644 index 00000000..953b3a9b --- /dev/null +++ b/src/functionalTest/resources/examples/classes/IncrementalBase.kt @@ -0,0 +1,14 @@ +/* + * Copyright 2016-2024 JetBrains s.r.o. + * Use of this source code is governed by the Apache 2.0 License that can be found in the LICENSE.txt file. + */ + +package foo + +data class Incremental(val id: Any) { + fun integer() = 42 +} + +fun sum(vararg integers: Int) = integers.sum() + +val id: Any = 42 \ No newline at end of file diff --git a/src/functionalTest/resources/examples/classes/IncrementalModification.kt b/src/functionalTest/resources/examples/classes/IncrementalModification.kt new file mode 100644 index 00000000..bee9f7ec --- /dev/null +++ b/src/functionalTest/resources/examples/classes/IncrementalModification.kt @@ -0,0 +1,14 @@ +/* + * Copyright 2016-2024 JetBrains s.r.o. + * Use of this source code is governed by the Apache 2.0 License that can be found in the LICENSE.txt file. + */ + +package foo + +data class Incremental(val id: String) { + fun integer() = 42.0 +} + +fun sumOfInts(vararg integers: Int) = integers.sum() + +val id: Int = 42 \ No newline at end of file diff --git a/src/functionalTest/resources/examples/classes/IncrementalRemoval.kt b/src/functionalTest/resources/examples/classes/IncrementalRemoval.kt new file mode 100644 index 00000000..fc44b5a2 --- /dev/null +++ b/src/functionalTest/resources/examples/classes/IncrementalRemoval.kt @@ -0,0 +1,8 @@ +/* + * Copyright 2016-2024 JetBrains s.r.o. + * Use of this source code is governed by the Apache 2.0 License that can be found in the LICENSE.txt file. + */ + +package foo + +class Incremental \ No newline at end of file diff --git a/src/functionalTest/resources/examples/gradle/configuration/incremental/incremental.gradle.kts b/src/functionalTest/resources/examples/gradle/configuration/incremental/incremental.gradle.kts new file mode 100644 index 00000000..9955575b --- /dev/null +++ b/src/functionalTest/resources/examples/gradle/configuration/incremental/incremental.gradle.kts @@ -0,0 +1,8 @@ +/* + * Copyright 2016-2024 JetBrains s.r.o. + * Use of this source code is governed by the Apache 2.0 License that can be found in the LICENSE.txt file. + */ + +configure { + useIncrementalValidation = true +} diff --git a/src/main/kotlin/ApiValidationExtension.kt b/src/main/kotlin/ApiValidationExtension.kt index 7a879775..abb3dba7 100644 --- a/src/main/kotlin/ApiValidationExtension.kt +++ b/src/main/kotlin/ApiValidationExtension.kt @@ -14,6 +14,12 @@ public open class ApiValidationExtension { */ public var validationDisabled: Boolean = false + /** + * Replaces hard errors during validation with warnings if only additions have been made since the last public API + * snapshot. This can prove useful when publishing incremental API changes without additional human interaction. + */ + public var useIncrementalValidation: Boolean = false + /** * Fully qualified package names that are not consider public API. * For example, it could be `kotlinx.coroutines.internal` or `kotlinx.serialization.implementation`. @@ -38,17 +44,17 @@ public open class ApiValidationExtension { public var ignoredClasses: MutableSet = HashSet() /** - * Fully qualified names of annotations that can be used to explicitly mark public declarations. + * Fully qualified names of annotations that can be used to explicitly mark public declarations. * If at least one of [publicMarkers], [publicPackages] or [publicClasses] is defined, - * all declarations not covered by any of them will be considered non-public. + * all declarations not covered by any of them will be considered non-public. * [ignoredPackages], [ignoredClasses] and [nonPublicMarkers] can be used for additional filtering. */ public var publicMarkers: MutableSet = HashSet() /** - * Fully qualified package names that contain public declarations. + * Fully qualified package names that contain public declarations. * If at least one of [publicMarkers], [publicPackages] or [publicClasses] is defined, - * all declarations not covered by any of them will be considered non-public. + * all declarations not covered by any of them will be considered non-public. * [ignoredPackages], [ignoredClasses] and [nonPublicMarkers] can be used for additional filtering. */ public var publicPackages: MutableSet = HashSet() diff --git a/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt b/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt index 1548c4db..b10067c3 100644 --- a/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt +++ b/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt @@ -305,6 +305,7 @@ private fun Project.configureCheckTasks( isEnabled = apiCheckEnabled(projectName, extension) && apiBuild.map { it.enabled }.getOrElse(true) group = "verification" description = "Checks signatures of public API against the golden value in API folder for $projectName" + incremental = extension.useIncrementalValidation projectApiFile = apiCheckDir.get().resolve(jvmDumpFileName) generatedApiFile = apiBuildDir.get().resolve(jvmDumpFileName) dependsOn(apiBuild) diff --git a/src/main/kotlin/IncrementalVerification.kt b/src/main/kotlin/IncrementalVerification.kt new file mode 100644 index 00000000..e8f8f672 --- /dev/null +++ b/src/main/kotlin/IncrementalVerification.kt @@ -0,0 +1,32 @@ +/* + * Copyright 2016-2024 JetBrains s.r.o. + * Use of this source code is governed by the Apache 2.0 License that can be found in the LICENSE.txt file. + */ + +package kotlinx.validation + +import org.gradle.api.logging.Logger + +internal class IncrementalVerification(private val subject: String, private val logger: Logger): Verification { + override fun verify(diffSet: Collection) { + var containsAdditions = false + var containsRemovals = false + out@ for (diff in diffSet) { + for (line in diff.split("\n")) { + when { + line.startsWith("+++") || line.startsWith("---") -> continue + line.startsWith("-") -> containsRemovals = true + line.startsWith("+") -> containsAdditions = true + } + if (containsRemovals) break@out + } + } + check(!containsRemovals) { + val diffText = diffSet.joinToString("\n\n") + "Incremental API check failed for project $subject.\n$diffText\n\n You can run :$subject:apiDump task to overwrite API declarations. These changes likely break compatibility with existing consumers using library '$subject', consider incrementing major version code for your next release" + } + if (containsAdditions) { + logger.warn("API is incrementally compatible with previous version, however is not identical to the API file provided.") + } + } +} \ No newline at end of file diff --git a/src/main/kotlin/KotlinApiCompareTask.kt b/src/main/kotlin/KotlinApiCompareTask.kt index 88e693ef..a047993d 100644 --- a/src/main/kotlin/KotlinApiCompareTask.kt +++ b/src/main/kotlin/KotlinApiCompareTask.kt @@ -5,18 +5,21 @@ package kotlinx.validation -import com.github.difflib.DiffUtils -import com.github.difflib.UnifiedDiffUtils -import java.io.* -import java.util.TreeMap -import javax.inject.Inject +import com.github.difflib.* import org.gradle.api.* import org.gradle.api.file.* -import org.gradle.api.model.ObjectFactory +import org.gradle.api.model.* import org.gradle.api.tasks.* +import org.gradle.api.tasks.Optional +import java.io.* +import java.util.* +import javax.inject.* public open class KotlinApiCompareTask @Inject constructor(private val objects: ObjectFactory): DefaultTask() { + @Input + public var incremental: Boolean = false + @InputFiles @PathSensitive(PathSensitivity.RELATIVE) public lateinit var projectApiFile: File @@ -25,7 +28,6 @@ public open class KotlinApiCompareTask @Inject constructor(private val objects: public lateinit var generatedApiFile: File private val projectName = project.name - private val rootDir = project.rootProject.rootDir @TaskAction @@ -40,6 +42,10 @@ public open class KotlinApiCompareTask @Inject constructor(private val objects: error("Expected folder with generate API declarations '$buildApiDir' does not exist.") } val subject = projectName + val verification = when { + incremental -> IncrementalVerification(projectName, logger) + else -> VerificationStrict(projectName) + } /* * We use case-insensitive comparison to workaround issues with case-insensitive OSes @@ -79,10 +85,7 @@ public open class KotlinApiCompareTask @Inject constructor(private val objects: val actualFile = actualApiDeclaration.getFile(buildApiDir) val diff = compareFiles(expectedFile, actualFile) if (diff != null) diffSet.add(diff) - if (diffSet.isNotEmpty()) { - val diffText = diffSet.joinToString("\n\n") - error("API check failed for project $subject.\n$diffText\n\n You can run :$subject:apiDump task to overwrite API declarations") - } + verification.verify(diffSet) } private fun File.relativeDirPath(): String { diff --git a/src/main/kotlin/Verification.kt b/src/main/kotlin/Verification.kt new file mode 100644 index 00000000..35b5deb9 --- /dev/null +++ b/src/main/kotlin/Verification.kt @@ -0,0 +1,10 @@ +/* + * Copyright 2016-2024 JetBrains s.r.o. + * Use of this source code is governed by the Apache 2.0 License that can be found in the LICENSE.txt file. + */ + +package kotlinx.validation + +internal interface Verification { + fun verify(diffSet: Collection) +} \ No newline at end of file diff --git a/src/main/kotlin/VerificationStrict.kt b/src/main/kotlin/VerificationStrict.kt new file mode 100644 index 00000000..a97cd1a3 --- /dev/null +++ b/src/main/kotlin/VerificationStrict.kt @@ -0,0 +1,15 @@ +/* + * Copyright 2016-2024 JetBrains s.r.o. + * Use of this source code is governed by the Apache 2.0 License that can be found in the LICENSE.txt file. + */ + +package kotlinx.validation + +internal class VerificationStrict(private val subject: String): Verification { + override fun verify(diffSet: Collection) { + check(diffSet.isEmpty()) { + val diffText = diffSet.joinToString("\n\n") + "API check failed for project $subject.\n$diffText\n\n You can run :$subject:apiDump task to overwrite API declarations" + } + } +} \ No newline at end of file