Enable strict explicit API mode (#168)

Rationale: We now have quite a lot of accidentally public entities while both promoting and using BCV as a standalone JAR dependency. Apart from that, we have quite an unfortunate package name 'api' that might imply all these methods are part of public API (when, in fact, it's all related to API validation). It would be nice to explicitly confine our visibilities and be more deliberate about that
Kotlin · Jan 15, 2024 · 07d46e7 · 07d46e7
1 parent f62caa2
commit 07d46e7
Show file tree

Hide file tree

Showing 18 changed files with 89 additions and 301 deletions.
diff --git a/api/binary-compatibility-validator.api b/api/binary-compatibility-validator.api
diff --git a/build.gradle.kts b/build.gradle.kts
@@ -76,6 +76,7 @@ dependencies {
 
 tasks.compileKotlin {
     compilerOptions {
+        freeCompilerArgs.add("-Xexplicit-api=strict")
         allWarningsAsErrors.set(true)
         @Suppress("DEPRECATION") // Compatibility with Gradle 7 requires Kotlin 1.4
         languageVersion.set(KotlinVersion.KOTLIN_1_4)

diff --git a/src/functionalTest/kotlin/kotlinx/validation/api/BaseKotlinGradleTest.kt b/src/functionalTest/kotlin/kotlinx/validation/api/BaseKotlinGradleTest.kt
@@ -5,7 +5,6 @@
 
 package kotlinx.validation.api
 
-import kotlinx.validation.API_DIR
 import org.junit.Rule
 import org.junit.rules.TemporaryFolder
 import java.io.File

diff --git a/src/functionalTest/kotlin/kotlinx/validation/api/TestDsl.kt b/src/functionalTest/kotlin/kotlinx/validation/api/TestDsl.kt
@@ -6,10 +6,11 @@
 package kotlinx.validation.api
 
 import java.io.*
-import kotlinx.validation.API_DIR
 import org.gradle.testkit.runner.GradleRunner
 import org.intellij.lang.annotations.Language
 
+public const val API_DIR: String = "api"
+
 internal fun BaseKotlinGradleTest.test(fn: BaseKotlinScope.() -> Unit): GradleRunner {
     val baseKotlinScope = BaseKotlinScope()
     fn(baseKotlinScope)

diff --git a/src/functionalTest/kotlin/kotlinx/validation/test/DefaultConfigTests.kt b/src/functionalTest/kotlin/kotlinx/validation/test/DefaultConfigTests.kt
@@ -5,7 +5,6 @@
 
 package kotlinx.validation.test
 
-import kotlinx.validation.API_DIR
 import kotlinx.validation.api.*
 import org.assertj.core.api.*
 import org.junit.Test

diff --git a/src/functionalTest/kotlin/kotlinx/validation/test/MultiPlatformSingleJvmTargetTest.kt b/src/functionalTest/kotlin/kotlinx/validation/test/MultiPlatformSingleJvmTargetTest.kt
@@ -5,7 +5,6 @@
 
 package kotlinx.validation.test
 
-import kotlinx.validation.API_DIR
 import kotlinx.validation.api.*
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.Test

diff --git a/src/functionalTest/kotlin/kotlinx/validation/test/MultipleJvmTargetsTest.kt b/src/functionalTest/kotlin/kotlinx/validation/test/MultipleJvmTargetsTest.kt
@@ -5,7 +5,6 @@
 
 package kotlinx.validation.test
 
-import kotlinx.validation.API_DIR
 import kotlinx.validation.api.*
 import org.assertj.core.api.Assertions.assertThat
 import org.gradle.testkit.runner.GradleRunner

diff --git a/src/functionalTest/kotlin/kotlinx/validation/test/SubprojectsWithPluginOnRootTests.kt b/src/functionalTest/kotlin/kotlinx/validation/test/SubprojectsWithPluginOnRootTests.kt
@@ -5,7 +5,6 @@
 
 package kotlinx.validation.test
 
-import kotlinx.validation.API_DIR
 import kotlinx.validation.api.*
 import kotlinx.validation.api.BaseKotlinGradleTest
 import kotlinx.validation.api.assertTaskSuccess

diff --git a/src/functionalTest/kotlin/kotlinx/validation/test/SubprojectsWithPluginOnSubTests.kt b/src/functionalTest/kotlin/kotlinx/validation/test/SubprojectsWithPluginOnSubTests.kt
@@ -5,7 +5,6 @@
 
 package kotlinx.validation.test
 
-import kotlinx.validation.API_DIR
 import kotlinx.validation.api.*
 import kotlinx.validation.api.BaseKotlinGradleTest
 import kotlinx.validation.api.assertTaskSuccess

diff --git a/src/main/kotlin/ApiValidationExtension.kt b/src/main/kotlin/ApiValidationExtension.kt
@@ -5,12 +5,12 @@
 
 package kotlinx.validation
 
-open class ApiValidationExtension {
+public open class ApiValidationExtension {
 
     /**
      * Disables API validation checks completely.
      */
-    public var validationDisabled = false
+    public var validationDisabled: Boolean = false
 
     /**
      * Fully qualified package names that are not consider public API.

diff --git a/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt b/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt
@@ -1,5 +1,5 @@
 /*
- * Copyright 2016-2020 JetBrains s.r.o.
+ * Copyright 2016-2023 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.
  */
 
@@ -13,9 +13,9 @@ import org.jetbrains.kotlin.gradle.dsl.*
 import org.jetbrains.kotlin.gradle.plugin.*
 import java.io.*
 
-const val API_DIR = "api"
+private const val API_DIR = "api" // Mirrored in functional tests
 
-class BinaryCompatibilityValidatorPlugin : Plugin<Project> {
+public class BinaryCompatibilityValidatorPlugin : Plugin<Project> {
 
     override fun apply(target: Project): Unit = with(target) {
         val extension = extensions.create("apiValidation", ApiValidationExtension::class.java)
@@ -216,7 +216,7 @@ internal val Project.apiValidationExtensionOrNull: ApiValidationExtension?
             .map { it.extensions.findByType(ApiValidationExtension::class.java) }
             .firstOrNull { it != null }
 
-fun apiCheckEnabled(projectName: String, extension: ApiValidationExtension): Boolean =
+private fun apiCheckEnabled(projectName: String, extension: ApiValidationExtension): Boolean =
     projectName !in extension.ignoredProjects && !extension.validationDisabled
 
 private fun Project.configureApiTasks(
@@ -283,7 +283,7 @@ private fun Project.configureCheckTasks(
     }
 }
 
-inline fun <reified T : Task> Project.task(
+private inline fun <reified T : Task> Project.task(
     name: String,
     noinline configuration: T.() -> Unit,
 ): TaskProvider<T> = tasks.register(name, T::class.java, Action(configuration))
diff --git a/src/main/kotlin/ExternalApi.kt b/src/main/kotlin/ExternalApi.kt
@@ -9,4 +9,4 @@ package kotlinx.validation
  * API that is used externally and programmatically by binary-compatibility-validator tool in Kotlin standard library
  */
 @Retention(AnnotationRetention.SOURCE)
-annotation class ExternalApi
+public annotation class ExternalApi
diff --git a/src/main/kotlin/KotlinApiBuildTask.kt b/src/main/kotlin/KotlinApiBuildTask.kt
@@ -13,69 +13,69 @@ import java.io.*
 import java.util.jar.JarFile
 import javax.inject.Inject
 
-open class KotlinApiBuildTask @Inject constructor(
+public open class KotlinApiBuildTask @Inject constructor(
 ) : DefaultTask() {
 
     private val extension = project.apiValidationExtensionOrNull
 
     @InputFiles
     @Optional
     @PathSensitive(PathSensitivity.RELATIVE)
-    var inputClassesDirs: FileCollection? = null
+    public var inputClassesDirs: FileCollection? = null
 
     @InputFile
     @Optional
     @PathSensitive(PathSensitivity.RELATIVE)
-    val inputJar: RegularFileProperty = this.project.objects.fileProperty()
+    public val inputJar: RegularFileProperty = this.project.objects.fileProperty()
 
     @InputFiles
     @PathSensitive(PathSensitivity.RELATIVE)
-    lateinit var inputDependencies: FileCollection
+    public lateinit var inputDependencies: FileCollection
 
     @OutputDirectory
-    lateinit var outputApiDir: File
+    public lateinit var outputApiDir: File
 
     private var _ignoredPackages: Set<String>? = null
     @get:Input
-    var ignoredPackages : Set<String>
+    internal var ignoredPackages : Set<String>
         get() = _ignoredPackages ?: extension?.ignoredPackages ?: emptySet()
         set(value) { _ignoredPackages = value }
 
     private var _nonPublicMarkes: Set<String>? = null
     @get:Input
-    var nonPublicMarkers : Set<String>
+    internal var nonPublicMarkers : Set<String>
         get() = _nonPublicMarkes ?: extension?.nonPublicMarkers ?: emptySet()
         set(value) { _nonPublicMarkes = value }
 
     private var _ignoredClasses: Set<String>? = null
     @get:Input
-    var ignoredClasses : Set<String>
+    internal var ignoredClasses : Set<String>
         get() = _ignoredClasses ?: extension?.ignoredClasses ?: emptySet()
         set(value) { _ignoredClasses = value }
 
     private var _publicPackages: Set<String>? = null
     @get:Input
-    var publicPackages: Set<String>
+    internal var publicPackages: Set<String>
         get() = _publicPackages ?: extension?.publicPackages ?: emptySet()
         set(value) { _publicPackages = value }
 
     private var _publicMarkers: Set<String>? = null
     @get:Input
-    var publicMarkers: Set<String>
+    internal var publicMarkers: Set<String>
         get() = _publicMarkers ?: extension?.publicMarkers ?: emptySet()
         set(value) { _publicMarkers = value}
 
     private var _publicClasses: Set<String>? = null
     @get:Input
-    var publicClasses: Set<String>
+    internal var publicClasses: Set<String>
         get() = _publicClasses ?: extension?.publicClasses ?: emptySet()
         set(value) { _publicClasses = value }
 
     @get:Internal
     internal val projectName = project.name
 
     @TaskAction
-    fun generate() {
+    internal fun generate() {
         cleanup(outputApiDir)
         outputApiDir.mkdirs()
 

diff --git a/src/main/kotlin/KotlinApiCompareTask.kt b/src/main/kotlin/KotlinApiCompareTask.kt
@@ -15,7 +15,7 @@ import org.gradle.api.file.*
 import org.gradle.api.model.ObjectFactory
 import org.gradle.api.tasks.*
 
-open class KotlinApiCompareTask @Inject constructor(private val objects: ObjectFactory): DefaultTask() {
+public open class KotlinApiCompareTask @Inject constructor(private val objects: ObjectFactory): DefaultTask() {
 
     /*
      * Nullability and optionality is a workaround for
@@ -26,14 +26,14 @@ open class KotlinApiCompareTask @Inject constructor(private val objects: ObjectF
     @Optional
     @InputDirectory
     @PathSensitive(PathSensitivity.RELATIVE)
-    var projectApiDir: File? = null
+    public var projectApiDir: File? = null
 
     // Used for diagnostic error message when projectApiDir doesn't exist
     @Input
     @Optional
-    var nonExistingProjectApiDir: String? = null
+    public var nonExistingProjectApiDir: String? = null
 
-    fun compareApiDumps(apiReferenceDir: File, apiBuildDir: File) {
+    internal fun compareApiDumps(apiReferenceDir: File, apiBuildDir: File) {
         if (apiReferenceDir.exists()) {
             projectApiDir = apiReferenceDir
         } else {
@@ -45,19 +45,19 @@ open class KotlinApiCompareTask @Inject constructor(private val objects: ObjectF
 
     @InputDirectory
     @PathSensitive(PathSensitivity.RELATIVE)
-    lateinit var apiBuildDir: File
+    public lateinit var apiBuildDir: File
 
     @OutputFile
     @Optional
     @Suppress("unused")
-    val dummyOutputFile: File? = null
+    public val dummyOutputFile: File? = null
 
     private val projectName = project.name
 
     private val rootDir = project.rootProject.rootDir
 
     @TaskAction
-    fun verify() {
+    internal fun verify() {
         val projectApiDir = projectApiDir
             ?: error("Expected folder with API declarations '$nonExistingProjectApiDir' does not exist.\n" +
                     "Please ensure that ':apiDump' was executed in order to get API dump to compare the build against")

diff --git a/src/main/kotlin/api/AsmMetadataLoading.kt b/src/main/kotlin/api/AsmMetadataLoading.kt
@@ -1,5 +1,5 @@
 /*
- * Copyright 2016-2020 JetBrains s.r.o.
+ * Copyright 2016-2023 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.
  */
 
@@ -9,7 +9,7 @@ import kotlinx.metadata.jvm.*
 import org.objectweb.asm.*
 import org.objectweb.asm.tree.*
 
-val ACCESS_NAMES = mapOf(
+internal val ACCESS_NAMES = mapOf(
     Opcodes.ACC_PUBLIC to "public",
     Opcodes.ACC_PROTECTED to "protected",
     Opcodes.ACC_PRIVATE to "private",
@@ -21,13 +21,13 @@ val ACCESS_NAMES = mapOf(
     Opcodes.ACC_ANNOTATION to "annotation"
 )
 
-fun isPublic(access: Int) = access and Opcodes.ACC_PUBLIC != 0 || access and Opcodes.ACC_PROTECTED != 0
-fun isProtected(access: Int) = access and Opcodes.ACC_PROTECTED != 0
-fun isStatic(access: Int) = access and Opcodes.ACC_STATIC != 0
-fun isFinal(access: Int) = access and Opcodes.ACC_FINAL != 0
-fun isSynthetic(access: Int) = access and Opcodes.ACC_SYNTHETIC != 0
+internal fun isPublic(access: Int) = access and Opcodes.ACC_PUBLIC != 0 || access and Opcodes.ACC_PROTECTED != 0
+internal fun isProtected(access: Int) = access and Opcodes.ACC_PROTECTED != 0
+internal fun isStatic(access: Int) = access and Opcodes.ACC_STATIC != 0
+internal fun isFinal(access: Int) = access and Opcodes.ACC_FINAL != 0
+internal fun isSynthetic(access: Int) = access and Opcodes.ACC_SYNTHETIC != 0
 
-fun ClassNode.isEffectivelyPublic(classVisibility: ClassVisibility?) =
+internal fun ClassNode.isEffectivelyPublic(classVisibility: ClassVisibility?) =
     isPublic(access)
             && !isLocal()
             && !isWhenMappings()
@@ -36,27 +36,26 @@ fun ClassNode.isEffectivelyPublic(classVisibility: ClassVisibility?) =
             && (classVisibility?.isPublic(isPublishedApi()) ?: true)
 
 
-val ClassNode.innerClassNode: InnerClassNode? get() = innerClasses.singleOrNull { it.name == name }
-fun ClassNode.isLocal() = outerMethod != null
-fun ClassNode.isInner() = innerClassNode != null
-fun ClassNode.isWhenMappings() = isSynthetic(access) && name.endsWith("\$WhenMappings")
-fun ClassNode.isSyntheticAnnotationClass() = isSynthetic(access) && name.contains("\$annotationImpl\$")
-fun ClassNode.isEnumEntriesMappings() = isSynthetic(access) && name.endsWith("\$EntriesMappings")
+private val ClassNode.innerClassNode: InnerClassNode? get() = innerClasses.singleOrNull { it.name == name }
+private fun ClassNode.isLocal() = outerMethod != null
+private fun ClassNode.isInner() = innerClassNode != null
+private fun ClassNode.isWhenMappings() = isSynthetic(access) && name.endsWith("\$WhenMappings")
+private fun ClassNode.isSyntheticAnnotationClass() = isSynthetic(access) && name.contains("\$annotationImpl\$")
+private fun ClassNode.isEnumEntriesMappings() = isSynthetic(access) && name.endsWith("\$EntriesMappings")
 
-val ClassNode.effectiveAccess: Int get() = innerClassNode?.access ?: access
-val ClassNode.outerClassName: String? get() = innerClassNode?.outerName
+internal val ClassNode.effectiveAccess: Int get() = innerClassNode?.access ?: access
+internal val ClassNode.outerClassName: String? get() = innerClassNode?.outerName
 
 
-const val publishedApiAnnotationName = "kotlin/PublishedApi"
-fun ClassNode.isPublishedApi() = findAnnotation(publishedApiAnnotationName, includeInvisible = true) != null
-fun List<AnnotationNode>.isPublishedApi() = firstOrNull { it.refersToName(publishedApiAnnotationName) } != null
+private const val publishedApiAnnotationName = "kotlin/PublishedApi"
+private fun ClassNode.isPublishedApi() = findAnnotation(publishedApiAnnotationName, includeInvisible = true) != null
+internal fun List<AnnotationNode>.isPublishedApi() = firstOrNull { it.refersToName(publishedApiAnnotationName) } != null
 
-fun ClassNode.isDefaultImpls(metadata: KotlinClassMetadata?) = isInner() && name.endsWith("\$DefaultImpls") && metadata.isSyntheticClass()
+internal fun ClassNode.isDefaultImpls(metadata: KotlinClassMetadata?) = isInner() && name.endsWith("\$DefaultImpls") && metadata.isSyntheticClass()
 
-
-fun ClassNode.findAnnotation(annotationName: String, includeInvisible: Boolean = false) =
+internal fun ClassNode.findAnnotation(annotationName: String, includeInvisible: Boolean = false) =
     findAnnotation(annotationName, visibleAnnotations, invisibleAnnotations, includeInvisible)
-operator fun AnnotationNode.get(key: String): Any? = values.annotationValue(key)
+internal operator fun AnnotationNode.get(key: String): Any? = values.annotationValue(key)
 
 private fun List<Any>.annotationValue(key: String): Any? {
     for (index in (0 until size / 2)) {
@@ -75,5 +74,5 @@ private fun findAnnotation(
     visibleAnnotations?.firstOrNull { it.refersToName(annotationName) }
         ?: if (includeInvisible) invisibleAnnotations?.firstOrNull { it.refersToName(annotationName) } else null
 
-fun AnnotationNode.refersToName(name: String) =
+internal fun AnnotationNode.refersToName(name: String) =
     desc.startsWith('L') && desc.endsWith(';') && desc.regionMatches(1, name, 0, name.length)