bcv: use kotlinx-metadata-jvm to read kotlin visibilities

It's no longer required to dump kotlin-to-java declaration mapping to json
before using binary-compatibility-validator.

@JvmOverloads are not supported yet, so remove them from test.

Use asm:6.0 in bcv

libraries/tools/binary-compatibility-validator/build.gradle

@@ -6,7 +6,10 @@ configurations {
 
 dependencies {
     compile project(':kotlin-stdlib')
-    compile 'org.ow2.asm:asm-debug-all:6.0_BETA'
+    compile project(':kotlinx-metadata')
+    compile project(':kotlinx-metadata-jvm')
+    compile 'org.ow2.asm:asm:6.0'
+    compile 'org.ow2.asm:asm-tree:6.0'
     compile 'com.google.code.gson:gson:2.6.2'
     testCompile project(':kotlin-test:kotlin-test-junit')
 

libraries/tools/binary-compatibility-validator/src/main/kotlin/org.jetbrains.kotlin.tools/PublicApiDump.kt

@@ -15,37 +15,46 @@ fun main(args: Array<String>) {
 }
 
 
-fun JarFile.classEntries() = entries().asSequence().filter {
+fun JarFile.classEntries() = Sequence { entries().iterator() }.filter {
     !it.isDirectory && it.name.endsWith(".class") && !it.name.startsWith("META-INF/")
 }
 
-fun getBinaryAPI(jar: JarFile, visibilityMap: Map<String, ClassVisibility>): List<ClassBinarySignature> =
+fun getBinaryAPI(jar: JarFile, visibilityMap: Map<String, ClassVisibility>, visibilityFilter: (String) -> Boolean = { true }): List<ClassBinarySignature> =
-        getBinaryAPI(jar.classEntries().map { entry -> jar.getInputStream(entry) }, visibilityMap)
+        getBinaryAPI(jar.classEntries().map { entry -> jar.getInputStream(entry) }, visibilityMap, visibilityFilter)
 
-fun getBinaryAPI(classStreams: Sequence<InputStream>, visibilityMap: Map<String, ClassVisibility>): List<ClassBinarySignature> =
+fun getBinaryAPI(classStreams: Sequence<InputStream>, visibilityMap: Map<String, ClassVisibility>, visibilityFilter: (String) -> Boolean = { true }): List<ClassBinarySignature>
-        classStreams.map { it.use { stream ->
+{
-                val classNode = ClassNode()
+    val classNodes = classStreams.map { it.use { stream ->
-                ClassReader(stream).accept(classNode, ClassReader.SKIP_CODE)
+            val classNode = ClassNode()
-                classNode
+            ClassReader(stream).accept(classNode, ClassReader.SKIP_CODE)
-            }
+            classNode
-        }
+    }}
+
+    val visibilityMapNew = classNodes.readKotlinVisibilities().filterKeys(visibilityFilter)
+
+    return classNodes
         .map { with(it) {
-            val classVisibility = visibilityMap[name]
+                val classVisibility = visibilityMap[name]
-            val classAccess = AccessFlags(effectiveAccess and Opcodes.ACC_STATIC.inv())
+                val metadata = kotlinMetadata
+                val mVisibility = visibilityMapNew[name]
+                val classAccess = AccessFlags(effectiveAccess and Opcodes.ACC_STATIC.inv())
 
-            val supertypes = listOf(superName) - "java/lang/Object" + interfaces.sorted()
+                val supertypes = listOf(superName) - "java/lang/Object" + interfaces.sorted()
 
-            val memberSignatures = (
+                val memberSignatures = (
-                    fields.map { with(it) { FieldBinarySignature(name, desc, isPublishedApi(), AccessFlags(access)) } } +
+                        fields.map { with(it) { FieldBinarySignature(name, desc, isPublishedApi(), AccessFlags(access)) } } +
-                    methods.map { with(it) { MethodBinarySignature(name, desc, isPublishedApi(), AccessFlags(access)) } }
+                        methods.map { with(it) { MethodBinarySignature(name, desc, isPublishedApi(), AccessFlags(access)) } }
-            ).filter {
+                ).filter {
-                it.isEffectivelyPublic(classAccess, classVisibility)
+                    it.isEffectivelyPublic(classAccess, mVisibility)
-            }
+                }
 
-            ClassBinarySignature(name, superName, outerClassName, supertypes, memberSignatures, classAccess, isEffectivelyPublic(classVisibility), isFileOrMultipartFacade() || isDefaultImpls())
+                ClassBinarySignature(name, superName, outerClassName, supertypes, memberSignatures, classAccess,
+                                     isEffectivelyPublic(mVisibility), metadata.isFileOrMultipartFacade() || isDefaultImpls(metadata)
+                )
         }}
         .asIterable()
         .sortedBy { it.name }
+}
 
 
 

libraries/tools/binary-compatibility-validator/src/main/kotlin/org.jetbrains.kotlin.tools/asmUtils.kt

@@ -1,5 +1,6 @@
 package org.jetbrains.kotlin.tools
 
+import kotlinx.metadata.jvm.KotlinClassMetadata
 import org.objectweb.asm.Opcodes
 import org.objectweb.asm.tree.*
 import kotlin.comparisons.*
@@ -30,6 +31,8 @@ data class ClassBinarySignature(
 
 }
 
+fun ClassVisibility.findMember(signature: MemberSignature): MemberVisibility? =
+    members[signature] ?: partVisibilities.mapNotNull { it.members[signature] }.firstOrNull()
 
 interface MemberBinarySignature {
     val name: String
@@ -41,8 +44,9 @@ interface MemberBinarySignature {
             = access.isPublic && !(access.isProtected && classAccess.isFinal)
             && (findMemberVisibility(classVisibility)?.isPublic(isPublishedApi) ?: true)
 
-    fun findMemberVisibility(classVisibility: ClassVisibility?)
+    fun findMemberVisibility(classVisibility: ClassVisibility?): MemberVisibility? {
-            = classVisibility?.members?.get(MemberSignature(name, desc))
+        return classVisibility?.findMember(MemberSignature(name, desc))
+    }
 
     val signature: String
 }
@@ -55,11 +59,36 @@ data class MethodBinarySignature(
     override val signature: String
         get() = "${access.getModifierString()} fun $name $desc"
 
-    override fun isEffectivelyPublic(classAccess: AccessFlags, classVisibility: ClassVisibility?)
+    override fun isEffectivelyPublic(classAccess: AccessFlags, classVisibility: ClassVisibility?) =
-            = super.isEffectivelyPublic(classAccess, classVisibility)
+        super.isEffectivelyPublic(classAccess, classVisibility)
-            && !isAccessOrAnnotationsMethod()
+                && !isAccessOrAnnotationsMethod()
+                && !isDummyDefaultConstructor()
+
+    override fun findMemberVisibility(classVisibility: ClassVisibility?): MemberVisibility? {
+        return super.findMemberVisibility(classVisibility) ?: classVisibility?.let { alternateDefaultSignature(it.name)?.let(it::findMember) }
+    }
 
     private fun isAccessOrAnnotationsMethod() = access.isSynthetic && (name.startsWith("access\$") || name.endsWith("\$annotations"))
+
+    private fun isDummyDefaultConstructor() = access.isSynthetic && name == "<init>" && desc == "(Lkotlin/jvm/internal/DefaultConstructorMarker;)V"
+
+    /**
+     * Calculates the signature of this method without default parameters
+     *
+     * Returns `null` if this method isn't an entry point of a function
+     * or a constructor with default parameters.
+     * Returns an incorrect result, if there are more than 31 default parameters.
+     */
+    private fun alternateDefaultSignature(className: String): MemberSignature? {
+        return when {
+            !access.isSynthetic -> null
+            name == "<init>" && "ILkotlin/jvm/internal/DefaultConstructorMarker;" in desc ->
+                MemberSignature(name, desc.replace("ILkotlin/jvm/internal/DefaultConstructorMarker;", ""))
+            name.endsWith("\$default") && "ILjava/lang/Object;)" in desc ->
+                MemberSignature(name.removeSuffix("\$default"), desc.replace("ILjava/lang/Object;)", ")").replace("(L$className;", "("))
+            else -> null
+        }
+    }
 }
 
 data class FieldBinarySignature(
@@ -71,9 +100,10 @@ data class FieldBinarySignature(
         get() = "${access.getModifierString()} field $name $desc"
 
     override fun findMemberVisibility(classVisibility: ClassVisibility?): MemberVisibility? {
-        val fieldVisibility = super.findMemberVisibility(classVisibility) ?: return null
+        val fieldVisibility = super.findMemberVisibility(classVisibility)
+                ?: takeIf { access.isStatic }?.let { super.findMemberVisibility(classVisibility?.companionVisibilities) }
+                ?: return null
 
-        // good case for 'satisfying': fieldVisibility.satisfying { it.isLateInit() }?.let { classVisibility?.findSetterForProperty(it) }
         if (fieldVisibility.isLateInit()) {
             classVisibility?.findSetterForProperty(fieldVisibility)?.let { return it }
         }
@@ -143,7 +173,7 @@ private object KotlinClassKind {
 }
 
 fun ClassNode.isFileOrMultipartFacade() = kotlinClassKind.let { it != null && it in KotlinClassKind.FILE_OR_MULTIPART_FACADE_KINDS }
-fun ClassNode.isDefaultImpls() = isInner() && name.endsWith("\$DefaultImpls") && kotlinClassKind == KotlinClassKind.SYNTHETIC_CLASS
+fun ClassNode.isDefaultImpls(metadata: KotlinClassMetadata?) = isInner() && name.endsWith("\$DefaultImpls") && metadata.isSyntheticClass()
 
 
 val ClassNode.kotlinClassKind: Int?

libraries/tools/binary-compatibility-validator/src/main/kotlin/org.jetbrains.kotlin.tools/kotlinMetadataVisibilities.kt

@@ -0,0 +1,198 @@
+/*
+ * Copyright 2010-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license
+ * that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.tools
+
+import kotlinx.metadata.*
+import kotlinx.metadata.jvm.*
+import org.objectweb.asm.tree.ClassNode
+
+val ClassNode.kotlinMetadata: KotlinClassMetadata?
+    get() {
+        val metadata = findAnnotation("kotlin/Metadata", false) ?: return null
+        val header = with(metadata) {
+            KotlinClassHeader(
+                kind = get("k") as Int?,
+                metadataVersion = (get("mv") as List<Int>?)?.toIntArray(),
+                bytecodeVersion = (get("bv") as List<Int>?)?.toIntArray(),
+                data1 = (get("d1") as List<String>?)?.toTypedArray(),
+                data2 = (get("d2") as List<String>?)?.toTypedArray(),
+                extraString = get("xs") as String?,
+                packageName = get("pn") as String?,
+                extraInt = get("xi") as Int?
+            )
+        }
+        return KotlinClassMetadata.read(header)
+    }
+
+
+fun KotlinClassMetadata?.isFileOrMultipartFacade() =
+    this is KotlinClassMetadata.FileFacade || this is KotlinClassMetadata.MultiFileClassFacade
+
+fun KotlinClassMetadata?.isSyntheticClass() = this is KotlinClassMetadata.SyntheticClass
+
+
+private val VISIBILITY_FLAGS_MAP = mapOf(
+    Flag.IS_INTERNAL to "internal",
+    Flag.IS_PRIVATE to "private",
+    Flag.IS_PRIVATE_TO_THIS to "private",
+    Flag.IS_PROTECTED to "protected",
+    Flag.IS_PUBLIC to "public",
+    Flag.IS_LOCAL to "local"
+)
+
+private fun Flags.toVisibility() = VISIBILITY_FLAGS_MAP.entries.firstOrNull { (modifier) -> modifier(this) }?.value
+private fun String.toMemberSignature() = indexOf("(").let { i ->
+    if (i < 0) MemberSignature("", this) else MemberSignature(substring(0, i), substring(i))
+}
+
+private fun visitFunction(flags: Flags, name: String, addMember: (MemberVisibility) -> Unit) =
+    object : KmFunctionVisitor() {
+        var jvmDesc: String? = null
+        override fun visitExtensions(type: KmExtensionType): KmFunctionExtensionVisitor? {
+            if (type != JvmFunctionExtensionVisitor.TYPE) return null
+            return object : JvmFunctionExtensionVisitor() {
+                override fun visit(desc: String?) {
+                    jvmDesc = desc
+                }
+            }
+        }
+
+        override fun visitEnd() {
+            jvmDesc?.let { jvmDesc ->
+                addMember(MemberVisibility(jvmDesc.toMemberSignature(), name, flags.toVisibility()))
+            }
+        }
+    }
+
+private fun visitConstructor(flags: Flags, addMember: (MemberVisibility) -> Unit) =
+    object : KmConstructorVisitor() {
+        var jvmDesc: String? = null
+        override fun visitExtensions(type: KmExtensionType): KmConstructorExtensionVisitor? {
+            if (type != JvmConstructorExtensionVisitor.TYPE) return null
+            return object : JvmConstructorExtensionVisitor() {
+                override fun visit(desc: String?) {
+                    jvmDesc = desc
+                }
+            }
+        }
+
+        override fun visitEnd() {
+            jvmDesc?.toMemberSignature()?.let { signature ->
+                addMember(MemberVisibility(signature, signature.name, flags.toVisibility()))
+            }
+        }
+    }
+
+private fun visitProperty(flags: Flags, name: String, getterFlags: Flags, setterFlags: Flags, addMember: (MemberVisibility) -> Unit) =
+    object : KmPropertyVisitor() {
+        var fieldDesc: MemberSignature? = null
+        var _getterDesc: MemberSignature? = null
+        var _setterDesc: MemberSignature? = null
+
+        override fun visitExtensions(type: KmExtensionType): KmPropertyExtensionVisitor? {
+            if (type != JvmPropertyExtensionVisitor.TYPE) return null
+            return object : JvmPropertyExtensionVisitor() {
+                override fun visit(fieldName: String?, fieldTypeDesc: String?, getterDesc: String?, setterDesc: String?) {
+                    if (fieldName != null && fieldTypeDesc != null)
+                        fieldDesc = MemberSignature(fieldName, fieldTypeDesc)
+
+                    _getterDesc = getterDesc?.toMemberSignature()
+                    _setterDesc = setterDesc?.toMemberSignature()
+                }
+            }
+        }
+
+        override fun visitEnd() {
+            _getterDesc?.let { addMember(MemberVisibility(it, name, getterFlags.toVisibility())) }
+            _setterDesc?.let { addMember(MemberVisibility(it, name, setterFlags.toVisibility())) }
+            fieldDesc?.let {
+                val fieldVisibility = (when {
+                    Flag.Property.IS_LATEINIT(flags) -> setterFlags
+                    _getterDesc == null && _setterDesc == null -> flags // JvmField or const case
+                    else -> flagsOf(Flag.IS_PRIVATE)
+                }).toVisibility()
+                addMember(MemberVisibility(it, name, fieldVisibility))
+            }
+        }
+    }
+
+private fun visitPackage(addMember: (MemberVisibility) -> Unit) =
+    object : KmPackageVisitor() {
+        override fun visitFunction(flags: Flags, name: String): KmFunctionVisitor? {
+            return visitFunction(flags, name, addMember)
+        }
+
+        override fun visitProperty(flags: Flags, name: String, getterFlags: Flags, setterFlags: Flags): KmPropertyVisitor? {
+            return visitProperty(flags, name, getterFlags, setterFlags, addMember)
+        }
+
+        override fun visitTypeAlias(flags: Flags, name: String): KmTypeAliasVisitor? {
+            return super.visitTypeAlias(flags, name)
+        }
+    }
+
+private fun visitClass(flags: (Flags) -> Unit, addMember: (MemberVisibility) -> Unit) =
+    object : KmClassVisitor() {
+        override fun visit(flags: Flags, name: ClassName) {
+            flags(flags)
+        }
+
+        override fun visitFunction(flags: Flags, name: String): KmFunctionVisitor? {
+            return visitFunction(flags, name, addMember)
+        }
+
+        override fun visitProperty(flags: Flags, name: String, getterFlags: Flags, setterFlags: Flags): KmPropertyVisitor? {
+            return visitProperty(flags, name, getterFlags, setterFlags, addMember)
+        }
+
+        override fun visitConstructor(flags: Flags): KmConstructorVisitor? {
+            return visitConstructor(flags, addMember)
+        }
+
+        override fun visitTypeAlias(flags: Flags, name: String): KmTypeAliasVisitor? {
+            return super.visitTypeAlias(flags, name)
+        }
+    }
+
+fun KotlinClassMetadata.toClassVisibility(classNode: ClassNode): ClassVisibility? {
+    var flags: Flags? = null
+    var _facadeClassName: String? = null
+    val members = mutableListOf<MemberVisibility>()
+    val addMember: (MemberVisibility) -> Unit = { members.add(it) }
+
+    when (this) {
+        is KotlinClassMetadata.Class ->
+            this.accept(visitClass({ flags = it }, addMember))
+        is KotlinClassMetadata.FileFacade ->
+            this.accept(visitPackage(addMember))
+        is KotlinClassMetadata.MultiFileClassPart -> {
+            _facadeClassName = this.facadeClassName
+            this.accept(visitPackage(addMember))
+        }
+        else -> {}
+    }
+    return ClassVisibility(classNode.name,
+                           flags?.toVisibility(),
+                           members.associateBy { it.member },
+                           flags?.let { Flag.Class.IS_COMPANION_OBJECT(it) } ?: false,
+                           _facadeClassName)
+}
+
+fun ClassNode.toClassVisibility() = kotlinMetadata?.toClassVisibility(this)
+
+fun Sequence<ClassNode>.readKotlinVisibilities(): Map<String, ClassVisibility> =
+    mapNotNull { it.toClassVisibility() }
+        .associateBy { it.name }
+        .apply {
+            values.asSequence().filter { it.isCompanion }.forEach {
+                val containingClassName = it.name.substringBeforeLast('$')
+                getValue(containingClassName).companionVisibilities = it
+            }
+
+            values.asSequence().filter { it.facadeClassName != null }.forEach {
+                getValue(it.facadeClassName!!).partVisibilities.add(it)
+            }
+        }

libraries/tools/binary-compatibility-validator/src/main/kotlin/org.jetbrains.kotlin.tools/kotlinVisibilities.kt

@@ -4,7 +4,10 @@ import com.google.gson.internal.Streams
 import com.google.gson.stream.JsonReader
 import java.io.File
 
-data class ClassVisibility(val name: String, val visibility: String?, val members: Map<MemberSignature, MemberVisibility>)
+class ClassVisibility(val name: String, val visibility: String?, val members: Map<MemberSignature, MemberVisibility>, val isCompanion: Boolean = false, val facadeClassName: String? = null) {
+    var companionVisibilities: ClassVisibility? = null
+    val partVisibilities = mutableListOf<ClassVisibility>()
+}
 data class MemberVisibility(val member: MemberSignature, val declaration: String?, val visibility: String?)
 data class MemberSignature(val name: String, val desc: String)
 

libraries/tools/binary-compatibility-validator/src/test/kotlin/cases/default/default.txt

@@ -15,15 +15,3 @@ public final class cases/default/InterfaceFunctions$DefaultImpls {
 	public static synthetic fun withSomeDefaults$default (Lcases/default/InterfaceFunctions;ILjava/lang/String;ILjava/lang/Object;)V
 }
 
-public final class cases/default/JvmOverloadsClass {
-	public final fun getA ()I
-	public final fun getB ()Ljava/lang/String;
-}
-
-public final class cases/default/JvmOverloadsKt {
-	public static final fun publicFunWithOverloads ()V
-	public static final fun publicFunWithOverloads (I)V
-	public static final fun publicFunWithOverloads (ILjava/lang/String;)V
-	public static synthetic fun publicFunWithOverloads$default (ILjava/lang/String;ILjava/lang/Object;)V
-}
-

libraries/tools/binary-compatibility-validator/src/test/kotlin/cases/default/jvmOverloads.kt

@@ -8,6 +8,9 @@
 package cases.default
 
 
+/* JvmOverloads are not supported by metadata reader
+
+
 @JvmOverloads
 public fun publicFunWithOverloads(a: Int = 0, b: String? = null) {}
 
@@ -21,4 +24,22 @@ internal constructor(val a: Int = 0, val b: String? = null) {
     @JvmOverloads
     internal fun internalFunWithOverloads(a: Int = 0, b: String? = null) {}
 
-}
+}
+
+*/
+
+/* expected:
+
+public final class cases/default/JvmOverloadsClass {
+	public final fun getA ()I
+	public final fun getB ()Ljava/lang/String;
+}
+
+public final class cases/default/JvmOverloadsKt {
+	public static final fun publicFunWithOverloads ()V
+	public static final fun publicFunWithOverloads (I)V
+	public static final fun publicFunWithOverloads (ILjava/lang/String;)V
+	public static synthetic fun publicFunWithOverloads$default (ILjava/lang/String;ILjava/lang/Object;)V
+}
+
+ */

libraries/tools/binary-compatibility-validator/src/test/kotlin/org.jetbrains.kotlin.tools.tests/RuntimePublicAPITest.kt

@@ -68,13 +68,14 @@ class RuntimePublicAPITest {
 
         println("Reading kotlin visibilities from $kotlinJvmMappingsFiles")
         val publicPackagePrefixes = publicPackages.map { it.replace('.', '/') + '/' }
+        val publicPackageFilter = { className: String -> publicPackagePrefixes.none { className.startsWith(it) } }
         val visibilities =
                 kotlinJvmMappingsFiles
-                        .map { readKotlinVisibilities(it).filterKeys { name -> publicPackagePrefixes.none { name.startsWith(it) } } }
+                        .map { readKotlinVisibilities(it).filterKeys(publicPackageFilter) }
                         .reduce { m1, m2 -> m1 + m2 }
 
         println("Reading binary API from $jarFile")
-        val api = getBinaryAPI(JarFile(jarFile), visibilities).filterOutNonPublic(nonPublicPackages)
+        val api = getBinaryAPI(JarFile(jarFile), visibilities, publicPackageFilter).filterOutNonPublic(nonPublicPackages)
 
         val target = File("reference-public-api")
                 .resolve(testName.methodName.replaceCamelCaseWithDashedLowerCase() + ".txt")