Skip to content

Commit

Permalink
[ABI Validation] Enable strict explicit API mode
Browse files Browse the repository at this point in the history
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
Pull request Kotlin/binary-compatibility-validator#168

Moved from Kotlin/binary-compatibility-validator@07d46e7
  • Loading branch information
qwwdfsad authored and shanshin committed Dec 23, 2024
1 parent e01d2fc commit 23791d3
Show file tree
Hide file tree
Showing 18 changed files with 89 additions and 301 deletions.
208 changes: 0 additions & 208 deletions libraries/tools/abi-validation/api/binary-compatibility-validator.api

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions libraries/tools/abi-validation/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
*/

Expand All @@ -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)
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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))
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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")
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
*/

Expand All @@ -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",
Expand All @@ -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()
Expand All @@ -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)) {
Expand All @@ -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)
Loading

0 comments on commit 23791d3

Please sign in to comment.