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..deb85435 100644 --- a/src/main/kotlin/ApiValidationExtension.kt +++ b/src/main/kotlin/ApiValidationExtension.kt @@ -34,4 +34,34 @@ 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() + + /** + * 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() } 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()))