From 83f4aae198f49d574d5f84b00dae9af6289a22d3 Mon Sep 17 00:00:00 2001 From: Pavel Kirpichenkov Date: Wed, 21 Dec 2022 17:38:17 +0200 Subject: [PATCH 1/2] Add support for defining public declarations explicitly If the added properties are used, all unmatched declarations will be excluded from the API check. If properties for both ignored and explicit markers are set, filtering of ignored declarations will happen after filtering of declarations not explicitly marked as public. --- .../validation/test/MixedMarkersTest.kt | 42 ++++++++ .../validation/test/PublicMarkersTest.kt | 46 +++++++++ .../examples/classes/ClassInPublicPackage.kt | 10 ++ .../classes/ClassWithPublicMarkers.dump | 23 +++++ .../classes/ClassWithPublicMarkers.kt | 32 ++++++ .../examples/classes/MixedAnnotations.dump | 5 + .../examples/classes/MixedAnnotations.kt | 49 ++++++++++ .../publicMarkers/markers.gradle.kts | 13 +++ .../publicMarkers/mixedMarkers.gradle.kts | 9 ++ src/main/kotlin/ApiValidationExtension.kt | 24 +++++ src/main/kotlin/KotlinApiBuildTask.kt | 21 +++- .../kotlin/api/KotlinSignaturesLoading.kt | 97 +++++++++++++------ 12 files changed, 342 insertions(+), 29 deletions(-) create mode 100644 src/functionalTest/kotlin/kotlinx/validation/test/MixedMarkersTest.kt create mode 100644 src/functionalTest/kotlin/kotlinx/validation/test/PublicMarkersTest.kt create mode 100644 src/functionalTest/resources/examples/classes/ClassInPublicPackage.kt create mode 100644 src/functionalTest/resources/examples/classes/ClassWithPublicMarkers.dump create mode 100644 src/functionalTest/resources/examples/classes/ClassWithPublicMarkers.kt create mode 100644 src/functionalTest/resources/examples/classes/MixedAnnotations.dump create mode 100644 src/functionalTest/resources/examples/classes/MixedAnnotations.kt create mode 100644 src/functionalTest/resources/examples/gradle/configuration/publicMarkers/markers.gradle.kts create mode 100644 src/functionalTest/resources/examples/gradle/configuration/publicMarkers/mixedMarkers.gradle.kts diff --git a/src/functionalTest/kotlin/kotlinx/validation/test/MixedMarkersTest.kt b/src/functionalTest/kotlin/kotlinx/validation/test/MixedMarkersTest.kt new file mode 100644 index 00000000..be9b83cf --- /dev/null +++ b/src/functionalTest/kotlin/kotlinx/validation/test/MixedMarkersTest.kt @@ -0,0 +1,42 @@ +/* + * Copyright 2016-2022 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 kotlinx.validation.api.buildGradleKts +import kotlinx.validation.api.kotlin +import kotlinx.validation.api.resolve +import kotlinx.validation.api.test +import org.junit.Test + +class MixedMarkersTest : BaseKotlinGradleTest() { + + @Test + fun testMixedMarkers() { + val runner = test { + buildGradleKts { + resolve("examples/gradle/base/withPlugin.gradle.kts") + resolve("examples/gradle/configuration/publicMarkers/mixedMarkers.gradle.kts") + } + + kotlin("MixedAnnotations.kt") { + resolve("examples/classes/MixedAnnotations.kt") + } + + apiFile(projectName = rootProjectDir.name) { + resolve("examples/classes/MixedAnnotations.dump") + } + + runner { + arguments.add(":apiCheck") + } + } + + runner.withDebug(true).build().apply { + assertTaskSuccess(":apiCheck") + } + } +} diff --git a/src/functionalTest/kotlin/kotlinx/validation/test/PublicMarkersTest.kt b/src/functionalTest/kotlin/kotlinx/validation/test/PublicMarkersTest.kt new file mode 100644 index 00000000..7da7010b --- /dev/null +++ b/src/functionalTest/kotlin/kotlinx/validation/test/PublicMarkersTest.kt @@ -0,0 +1,46 @@ +/* + * Copyright 2016-2022 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 kotlinx.validation.api.buildGradleKts +import kotlinx.validation.api.kotlin +import kotlinx.validation.api.resolve +import kotlinx.validation.api.test +import org.junit.Test + +class PublicMarkersTest : BaseKotlinGradleTest() { + + @Test + fun testPublicMarkers() { + val runner = test { + buildGradleKts { + resolve("examples/gradle/base/withPlugin.gradle.kts") + resolve("examples/gradle/configuration/publicMarkers/markers.gradle.kts") + } + + kotlin("ClassWithPublicMarkers.kt") { + resolve("examples/classes/ClassWithPublicMarkers.kt") + } + + kotlin("ClassInPublicPackage.kt") { + resolve("examples/classes/ClassInPublicPackage.kt") + } + + apiFile(projectName = rootProjectDir.name) { + resolve("examples/classes/ClassWithPublicMarkers.dump") + } + + runner { + arguments.add(":apiCheck") + } + } + + runner.withDebug(true).build().apply { + assertTaskSuccess(":apiCheck") + } + } +} diff --git a/src/functionalTest/resources/examples/classes/ClassInPublicPackage.kt b/src/functionalTest/resources/examples/classes/ClassInPublicPackage.kt new file mode 100644 index 00000000..fa93895a --- /dev/null +++ b/src/functionalTest/resources/examples/classes/ClassInPublicPackage.kt @@ -0,0 +1,10 @@ +/* + * Copyright 2016-2022 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.api + +class ClassInPublicPackage { + class Inner +} diff --git a/src/functionalTest/resources/examples/classes/ClassWithPublicMarkers.dump b/src/functionalTest/resources/examples/classes/ClassWithPublicMarkers.dump new file mode 100644 index 00000000..89b68dbe --- /dev/null +++ b/src/functionalTest/resources/examples/classes/ClassWithPublicMarkers.dump @@ -0,0 +1,23 @@ +public final class foo/ClassWithPublicMarkers { + public final fun getBar1 ()I + public final fun getBar2 ()I + public final fun setBar1 (I)V + public final fun setBar2 (I)V +} + +public final class foo/ClassWithPublicMarkers$MarkedClass { + public fun ()V + public final fun getBar1 ()I +} + +public abstract interface annotation class foo/PublicClass : java/lang/annotation/Annotation { +} + +public final class foo/api/ClassInPublicPackage { + public fun ()V +} + +public final class foo/api/ClassInPublicPackage$Inner { + public fun ()V +} + diff --git a/src/functionalTest/resources/examples/classes/ClassWithPublicMarkers.kt b/src/functionalTest/resources/examples/classes/ClassWithPublicMarkers.kt new file mode 100644 index 00000000..cb8f3843 --- /dev/null +++ b/src/functionalTest/resources/examples/classes/ClassWithPublicMarkers.kt @@ -0,0 +1,32 @@ +/* + * Copyright 2016-2022 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 + +@Target(AnnotationTarget.CLASS) +annotation class PublicClass + +@Target(AnnotationTarget.FIELD) +annotation class PublicField + +@Target(AnnotationTarget.PROPERTY) +annotation class PublicProperty + +public class ClassWithPublicMarkers { + @PublicField + var bar1 = 42 + + @PublicProperty + var bar2 = 42 + + @PublicClass + class MarkedClass { + val bar1 = 41 + } + + var notMarkedPublic = 42 + + class NotMarkedClass +} diff --git a/src/functionalTest/resources/examples/classes/MixedAnnotations.dump b/src/functionalTest/resources/examples/classes/MixedAnnotations.dump new file mode 100644 index 00000000..9d7982fc --- /dev/null +++ b/src/functionalTest/resources/examples/classes/MixedAnnotations.dump @@ -0,0 +1,5 @@ +public final class mixed/MarkedPublicWithPrivateMembers { + public fun ()V + public final fun otherFun ()V +} + diff --git a/src/functionalTest/resources/examples/classes/MixedAnnotations.kt b/src/functionalTest/resources/examples/classes/MixedAnnotations.kt new file mode 100644 index 00000000..b382a652 --- /dev/null +++ b/src/functionalTest/resources/examples/classes/MixedAnnotations.kt @@ -0,0 +1,49 @@ +/* + * Copyright 2016-2022 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 mixed + +@Target(AnnotationTarget.CLASS, AnnotationTarget.PROPERTY, AnnotationTarget.FIELD, AnnotationTarget.FUNCTION) +annotation class PublicApi + +@Target(AnnotationTarget.CLASS, AnnotationTarget.PROPERTY, AnnotationTarget.FIELD, AnnotationTarget.FUNCTION) +annotation class PrivateApi + +@PublicApi +class MarkedPublicWithPrivateMembers { + @PrivateApi + var private1 = 42 + + @field:PrivateApi + var private2 = 15 + + @PrivateApi + fun privateFun() = Unit + + @PublicApi + @PrivateApi + fun privateFun2() = Unit + + fun otherFun() = Unit +} + +// Member annotations should be ignored in explicitly private classes +@PrivateApi +class MarkedPrivateWithPublicMembers { + @PublicApi + var public1 = 42 + + @field:PublicApi + var public2 = 15 + + @PublicApi + fun publicFun() = Unit + + fun otherFun() = Unit +} + +@PrivateApi +@PublicApi +class PublicAndPrivateFilteredOut diff --git a/src/functionalTest/resources/examples/gradle/configuration/publicMarkers/markers.gradle.kts b/src/functionalTest/resources/examples/gradle/configuration/publicMarkers/markers.gradle.kts new file mode 100644 index 00000000..f0d67493 --- /dev/null +++ b/src/functionalTest/resources/examples/gradle/configuration/publicMarkers/markers.gradle.kts @@ -0,0 +1,13 @@ +/* + * Copyright 2016-2022 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 { + publicMarkers.add("foo.PublicClass") + publicMarkers.add("foo.PublicField") + publicMarkers.add("foo.PublicProperty") + + publicPackages.add("foo.api") + publicClasses.add("foo.PublicClass") +} diff --git a/src/functionalTest/resources/examples/gradle/configuration/publicMarkers/mixedMarkers.gradle.kts b/src/functionalTest/resources/examples/gradle/configuration/publicMarkers/mixedMarkers.gradle.kts new file mode 100644 index 00000000..c8c97481 --- /dev/null +++ b/src/functionalTest/resources/examples/gradle/configuration/publicMarkers/mixedMarkers.gradle.kts @@ -0,0 +1,9 @@ +/* + * Copyright 2016-2022 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 { + nonPublicMarkers.add("mixed.PrivateApi") + publicMarkers.add("mixed.PublicApi") +} diff --git a/src/main/kotlin/ApiValidationExtension.kt b/src/main/kotlin/ApiValidationExtension.kt index 7ebbbf35..3ca825bc 100644 --- a/src/main/kotlin/ApiValidationExtension.kt +++ b/src/main/kotlin/ApiValidationExtension.kt @@ -34,4 +34,28 @@ open class ApiValidationExtension { * Example of such a class could be `com.package.android.BuildConfig`. */ public var ignoredClasses: MutableSet = HashSet() + + /** + * 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. + * [ignoredPackages], [ignoredClasses] and [nonPublicMarkers] can be used for additional filtering. + */ + public var publicMarkers: MutableSet = HashSet() + + /** + * 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. + * [ignoredPackages], [ignoredClasses] and [nonPublicMarkers] can be used for additional filtering. + */ + public var publicPackages: MutableSet = HashSet() + + /** + * Fully qualified names of public classes. + * If at least one of [publicMarkers], [publicPackages] or [publicClasses] is defined, + * 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 publicClasses: MutableSet = HashSet() } diff --git a/src/main/kotlin/KotlinApiBuildTask.kt b/src/main/kotlin/KotlinApiBuildTask.kt index 1621c65b..e5f70fc3 100644 --- a/src/main/kotlin/KotlinApiBuildTask.kt +++ b/src/main/kotlin/KotlinApiBuildTask.kt @@ -53,6 +53,24 @@ open class KotlinApiBuildTask @Inject constructor( get() = _ignoredClasses ?: extension?.ignoredClasses ?: emptySet() set(value) { _ignoredClasses = value } + private var _publicPackages: Set? = null + @get:Input + var publicPackages: Set + get() = _publicPackages ?: extension?.publicPackages ?: emptySet() + set(value) { _publicPackages = value } + + private var _publicMarkers: Set? = null + @get:Input + var publicMarkers: Set + get() = _publicMarkers ?: extension?.publicMarkers ?: emptySet() + set(value) { _publicMarkers = value} + + private var _publicClasses: Set? = null + @get:Input + var publicClasses: Set + get() = _publicClasses ?: extension?.publicClasses ?: emptySet() + set(value) { _publicClasses = value } + @get:Internal internal val projectName = project.name @@ -79,8 +97,9 @@ open class KotlinApiBuildTask @Inject constructor( val filteredSignatures = signatures + .retainExplicitlyIncludedIfDeclared(publicPackages, publicClasses, publicMarkers) .filterOutNonPublic(ignoredPackages, ignoredClasses) - .filterOutAnnotated(nonPublicMarkers.map { it.replace(".", "/") }.toSet()) + .filterOutAnnotated(nonPublicMarkers.map(::replaceDots).toSet()) outputApiDir.resolve("$projectName.api").bufferedWriter().use { writer -> filteredSignatures diff --git a/src/main/kotlin/api/KotlinSignaturesLoading.kt b/src/main/kotlin/api/KotlinSignaturesLoading.kt index f0c401d4..3234d7f0 100644 --- a/src/main/kotlin/api/KotlinSignaturesLoading.kt +++ b/src/main/kotlin/api/KotlinSignaturesLoading.kt @@ -125,24 +125,38 @@ public fun List.filterOutAnnotated(targetAnnotations: Set< if (targetAnnotations.isEmpty()) return this return filter { it.annotations.all { ann -> !targetAnnotations.any { ann.refersToName(it) } } - }.map { - ClassBinarySignature( - it.name, - it.superName, - it.outerName, - it.supertypes, - it.memberSignatures.filter { - it.annotations.all { ann -> - !targetAnnotations.any { - ann.refersToName(it) - } + }.map { signature -> + val notAnnotatedMemberSignatures = signature.memberSignatures.filter { memberSignature -> + memberSignature.annotations.all { ann -> + !targetAnnotations.any { + ann.refersToName(it) } - }, - it.access, - it.isEffectivelyPublic, - it.isNotUsedWhenEmpty, - it.annotations - ) + } + } + + signature.copy(memberSignatures = notAnnotatedMemberSignatures) + } +} + +private fun List.filterOutNotAnnotated( + targetAnnotations: Set +): List { + if (targetAnnotations.isEmpty()) return this + return mapNotNull { classSignature -> + + /* If class is annotated: Return class and all its members */ + if (classSignature.annotations.any { annotation -> + targetAnnotations.any { annotation.refersToName(it) } + }) return@mapNotNull classSignature + + val annotatedMembers = classSignature.memberSignatures.filter { memberSignature -> + memberSignature.annotations.any { annotation -> + targetAnnotations.any { annotation.refersToName(it) } + } + } + + /* If some members are annotated, return class with only annotated members */ + if (annotatedMembers.isNotEmpty()) classSignature.copy(memberSignatures = annotatedMembers) else null } } @@ -151,19 +165,11 @@ public fun List.filterOutNonPublic( nonPublicPackages: Collection = emptyList(), nonPublicClasses: Collection = emptyList() ): List { - val pathMapper: (String) -> String = { it.replace('.', '/') + '/' } - val nonPublicPackagePaths = nonPublicPackages.map(pathMapper) - val excludedClasses = nonPublicClasses.map(pathMapper) + val nonPublicPackagePaths = nonPublicPackages.map(::toSlashSeparatedPath).toSet() + val excludedClasses = nonPublicClasses.map(::toSlashSeparatedPath).toSet() val classByName = associateBy { it.name } - fun ClassBinarySignature.isInNonPublicPackage() = - nonPublicPackagePaths.any { name.startsWith(it) } - - // checks whether class (e.g. com/company/BuildConfig) is in excluded class (e.g. com/company/BuildConfig/) - fun ClassBinarySignature.isInExcludedClasses() = - excludedClasses.any { it.startsWith(name) } - fun ClassBinarySignature.isPublicAndAccessible(): Boolean = isEffectivelyPublic && (outerName == null || classByName[outerName]?.let { outerClass -> @@ -189,11 +195,34 @@ public fun List.filterOutNonPublic( ) } - return filter { !it.isInNonPublicPackage() && !it.isInExcludedClasses() && it.isPublicAndAccessible() } + return filter { + !it.isInPackages(nonPublicPackagePaths) && !it.isInClasses(excludedClasses) && it.isPublicAndAccessible() + } .map { it.flattenNonPublicBases() } .filterNot { it.isNotUsedWhenEmpty && it.memberSignatures.isEmpty() } } +@ExternalApi +public fun List.retainExplicitlyIncludedIfDeclared( + publicPackages: Collection = emptyList(), + publicClasses: Collection = emptyList(), + publicMarkerAnnotations: Collection = emptyList(), +): List { + if (publicPackages.isEmpty() && publicClasses.isEmpty() && publicMarkerAnnotations.isEmpty()) return this + + val packagePaths = publicPackages.map(::toSlashSeparatedPath).toSet() + val classesPaths = publicClasses.map(::toSlashSeparatedPath).toSet() + val markerAnnotations = publicMarkerAnnotations.map(::replaceDots).toSet() + + val (includedByPackageOrClass, potentiallyAnnotated) = this.partition { signature -> + signature.isInClasses(classesPaths) || signature.isInPackages(packagePaths) + } + + val includedByMarkerAnnotations = potentiallyAnnotated.filterOutNotAnnotated(markerAnnotations) + + return includedByPackageOrClass + includedByMarkerAnnotations +} + @ExternalApi public fun List.dump() = dump(to = System.out) @@ -211,9 +240,21 @@ public fun List.dump(to: T): T { return to } +private fun ClassBinarySignature.isInPackages(packageNames: Collection): Boolean = + packageNames.any { packageName -> name.startsWith(packageName) } + +private fun ClassBinarySignature.isInClasses(classNames: Collection): Boolean = + classNames.any { className -> className.startsWith(name) } + private fun JarFile.classEntries() = Sequence { entries().iterator() }.filter { !it.isDirectory && it.name.endsWith(".class") && !it.name.startsWith("META-INF/") } +internal fun toSlashSeparatedPath(dotSeparated: String): String = + dotSeparated.replace('.', '/') + '/' + +internal fun replaceDots(dotSeparated: String): String = + dotSeparated.replace('.', '/') + internal fun annotations(l1: List?, l2: List?): List = ((l1 ?: emptyList()) + (l2 ?: emptyList())) From 5d3f4b77445d991485e15356f391dcadcc1bf05c Mon Sep 17 00:00:00 2001 From: Pavel Kirpichenkov Date: Thu, 22 Dec 2022 16:11:14 +0200 Subject: [PATCH 2/2] Support validation of non-main source sets for kotlin-jvm projects --- src/main/kotlin/ApiValidationExtension.kt | 6 ++++++ .../BinaryCompatibilityValidatorPlugin.kt | 18 +++++++++--------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/main/kotlin/ApiValidationExtension.kt b/src/main/kotlin/ApiValidationExtension.kt index 3ca825bc..deb85435 100644 --- a/src/main/kotlin/ApiValidationExtension.kt +++ b/src/main/kotlin/ApiValidationExtension.kt @@ -58,4 +58,10 @@ open class ApiValidationExtension { * [ignoredPackages], [ignoredClasses] and [nonPublicMarkers] can be used for additional filtering. */ public var publicClasses: MutableSet = HashSet() + + /** + * Non-default Gradle SourceSet names that should be validated. + * By default, only the `main` source set is checked. + */ + public var additionalSourceSets: MutableSet = HashSet() } diff --git a/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt b/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt index bd778e9a..c0217ae4 100644 --- a/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt +++ b/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt @@ -130,12 +130,7 @@ class BinaryCompatibilityValidatorPlugin : Plugin { project: Project, extension: ApiValidationExtension ) = configurePlugin("kotlin", project, extension) { - project.sourceSets.all { sourceSet -> - if (sourceSet.name != SourceSet.MAIN_SOURCE_SET_NAME) { - return@all - } - project.configureApiTasks(sourceSet, extension, TargetConfig(project)) - } + project.configureApiTasks(extension, TargetConfig(project)) } } @@ -225,19 +220,24 @@ fun apiCheckEnabled(projectName: String, extension: ApiValidationExtension): Boo projectName !in extension.ignoredProjects && !extension.validationDisabled private fun Project.configureApiTasks( - sourceSet: SourceSet, extension: ApiValidationExtension, targetConfig: TargetConfig = TargetConfig(this), ) { val projectName = project.name val apiBuildDir = targetConfig.apiDir.map { buildDir.resolve(it) } + val sourceSetsOutputsProvider = project.provider { + sourceSets + .filter { it.name == SourceSet.MAIN_SOURCE_SET_NAME || it.name in extension.additionalSourceSets } + .map { it.output.classesDirs } + } + val apiBuild = task(targetConfig.apiTaskName("Build")) { isEnabled = apiCheckEnabled(projectName, extension) // 'group' is not specified deliberately, so it will be hidden from ./gradlew tasks description = "Builds Kotlin API for 'main' compilations of $projectName. Complementary task and shouldn't be called manually" - inputClassesDirs = files(provider { if (isEnabled) sourceSet.output.classesDirs else emptyList() }) - inputDependencies = files(provider { if (isEnabled) sourceSet.output.classesDirs else emptyList() }) + inputClassesDirs = files(provider { if (isEnabled) sourceSetsOutputsProvider.get() else emptyList() }) + inputDependencies = files(provider { if (isEnabled) sourceSetsOutputsProvider.get() else emptyList() }) outputApiDir = apiBuildDir.get() }