Exclude nested classes of private classes from ABI classes

2018-12-03 20:33:25 +03:00
parent 64380f55d2
commit d54ce2da6f
14 changed files with 323 additions and 9 deletions
@@ -26,6 +26,7 @@ import org.jetbrains.kotlin.protobuf.GeneratedMessageLite
 import org.jetbrains.kotlin.resolve.DescriptorUtils
 import org.jetbrains.kotlin.resolve.DescriptorUtils.isInterface
 import org.jetbrains.kotlin.resolve.descriptorUtil.classId
+import org.jetbrains.kotlin.resolve.descriptorUtil.isEffectivelyPrivateApi
 import org.jetbrains.kotlin.resolve.descriptorUtil.nonSourceAnnotations
 import org.jetbrains.kotlin.resolve.jvm.annotations.hasJvmDefaultAnnotation
 import org.jetbrains.kotlin.serialization.DescriptorSerializer
@@ -59,6 +60,10 @@ class JvmSerializerExtension(private val bindings: JvmSerializationBindings, sta
        return classBuilderMode != ClassBuilderMode.ABI || descriptor.visibility != Visibilities.PRIVATE
    }

+    override fun shouldSerializeNestedClass(descriptor: ClassDescriptor): Boolean {
+        return classBuilderMode != ClassBuilderMode.ABI || !descriptor.isEffectivelyPrivateApi
+    }
+
    override fun serializeClass(
            descriptor: ClassDescriptor,
            proto: ProtoBuf.Class.Builder,
@@ -119,6 +119,10 @@ class DescriptorSerializer private constructor(
                typeAliasProto(descriptor)?.let { builder.addTypeAlias(it) }
            }
            else {
+                if (descriptor is ClassDescriptor && !extension.shouldSerializeNestedClass(descriptor)) {
+                    continue
+                }
+
                val name = getSimpleNameIndex(descriptor.name)
                if (isEnumEntry(descriptor)) {
                    builder.addEnumEntry(enumEntryProto(descriptor as ClassDescriptor))
@@ -25,6 +25,7 @@ abstract class SerializerExtension {
    open fun shouldSerializeFunction(descriptor: FunctionDescriptor): Boolean = true
    open fun shouldSerializeProperty(descriptor: PropertyDescriptor): Boolean = true
    open fun shouldSerializeTypeAlias(descriptor: TypeAliasDescriptor): Boolean = true
+    open fun shouldSerializeNestedClass(descriptor: ClassDescriptor): Boolean = true

    interface ClassMembersProducer {
        fun getCallableMembers(classDescriptor: ClassDescriptor): Collection<CallableMemberDescriptor>
@@ -154,6 +154,7 @@ import org.jetbrains.kotlin.kapt.cli.test.AbstractArgumentParsingTest
 import org.jetbrains.kotlin.kapt.cli.test.AbstractKaptToolIntegrationTest
 import org.jetbrains.kotlin.jvm.abi.AbstractCompareJvmAbiTest
 import org.jetbrains.kotlin.jvm.abi.AbstractCompileAgainstJvmAbiTest
+import org.jetbrains.kotlin.jvm.abi.AbstractJvmAbiContentTest
 import org.jetbrains.kotlin.kapt3.test.AbstractClassFileToSourceStubConverterTest
 import org.jetbrains.kotlin.kapt3.test.AbstractKotlinKaptContextTest
 import org.jetbrains.kotlin.noarg.AbstractBlackBoxCodegenTestForNoArg
@@ -1076,6 +1077,10 @@ fun main(args: Array<String>) {
            model("compare", recursive = false, extension = null)
        }

+        testClass<AbstractJvmAbiContentTest> {
+            model("content", recursive = false, extension = null)
+        }
+
        testClass<AbstractCompileAgainstJvmAbiTest> {
            model("compile", recursive = false, extension = null)
        }
@@ -19,6 +19,7 @@ import org.jetbrains.kotlin.incremental.LocalFileKotlinClass
 import org.jetbrains.kotlin.incremental.isClassFile
 import org.jetbrains.kotlin.jvm.abi.asm.AbiClassBuilder
 import org.jetbrains.kotlin.jvm.abi.asm.FilterInnerClassesVisitor
+import org.jetbrains.kotlin.jvm.abi.asm.InnerClassesCollectingVisitor
 import org.jetbrains.kotlin.load.kotlin.FileBasedKotlinClass
 import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader
 import org.jetbrains.kotlin.metadata.ProtoBuf
@@ -33,6 +34,7 @@ import org.jetbrains.kotlin.resolve.jvm.diagnostics.JvmDeclarationOrigin
 import org.jetbrains.kotlin.resolve.jvm.extensions.AnalysisHandlerExtension
 import org.jetbrains.org.objectweb.asm.*
 import java.io.File
+import java.util.*

 class JvmAbiAnalysisHandlerExtension(
    private val compilerConfiguration: CompilerConfiguration
@@ -78,8 +80,22 @@ class JvmAbiAnalysisHandlerExtension(
     * Removes private or local classes from outputs
     */
    private fun removeUnneededClasses(outputs: Iterable<AbiOutput>) {
-        val removedClasses = HashSet<String>()
+        // maps internal names of classes: class -> inner classes
+        val innerClasses = HashMap<String, Collection<String>>()
+        val internalNameToFile = HashMap<String, File>()

+        for (output in outputs) {
+            if (!output.file.isClassFile()) continue
+
+            val visitor = InnerClassesCollectingVisitor()
+            output.accept(visitor)
+            val outputInternalName = visitor.ownInternalName!!
+            internalNameToFile[outputInternalName] = output.file
+            innerClasses[outputInternalName] = visitor.innerClasses
+        }
+
+        // internal names of removed files
+        val classesToRemoveQueue = ArrayDeque<String>()
        for (output in outputs) {
            if (!output.file.isClassFile()) continue

@@ -96,17 +112,33 @@ class JvmAbiAnalysisHandlerExtension(
            }

            if (!isNeededForAbi) {
-                output.delete()
                val jvmClassName = JvmClassName.byClassId(classData.classId)
-                removedClasses.add(jvmClassName.internalName)
+                classesToRemoveQueue.add(jvmClassName.internalName)
            }
        }

+        // we can remove inner classes of removed classes
+        val classesToRemove = HashSet<String>()
+        classesToRemove.addAll(classesToRemoveQueue)
+        while (classesToRemoveQueue.isNotEmpty()) {
+            val classToRemove = classesToRemoveQueue.removeFirst()
+            innerClasses[classToRemove]?.forEach {
+                if (classesToRemove.add(it)) {
+                    classesToRemoveQueue.add(it)
+                }
+            }
+        }
+
+        val classFilesToRemove = classesToRemove.mapTo(HashSet()) { internalNameToFile[it] }
        for (output in outputs) {
            if (!output.file.isClassFile()) continue

-            output.transform { writer ->
-                FilterInnerClassesVisitor(removedClasses, Opcodes.ASM6, writer)
+            if (output.file in classFilesToRemove) {
+                output.delete()
+            } else {
+                output.transform { writer ->
+                    FilterInnerClassesVisitor(classesToRemove, Opcodes.ASM6, writer)
+                }
            }
        }
    }
@@ -164,6 +196,12 @@ class JvmAbiAnalysisHandlerExtension(
            this.bytes = cw.toByteArray()
        }

+        fun accept(visitor: ClassVisitor) {
+            val bytes = bytes ?: return
+            val cr = ClassReader(bytes)
+            cr.accept(visitor, 0)
+        }
+
        fun flush() {
            bytes?.let { FileUtil.writeToFile(file, it) }
        }
@@ -0,0 +1,30 @@
+package org.jetbrains.kotlin.jvm.abi.asm
+
+import org.jetbrains.org.objectweb.asm.ClassVisitor
+import org.jetbrains.org.objectweb.asm.Opcodes
+
+internal class InnerClassesCollectingVisitor : ClassVisitor(Opcodes.ASM6) {
+    var ownInternalName: String? = null
+        private set
+
+    private val myInnerClasses = arrayListOf<String>()
+    val innerClasses: List<String>
+        get() = myInnerClasses
+
+    override fun visit(
+        version: Int,
+        access: Int,
+        name: String,
+        signature: String?,
+        superName: String?,
+        interfaces: Array<out String>?
+    ) {
+        super.visit(version, access, name, signature, superName, interfaces)
+        ownInternalName = name
+    }
+
+    override fun visitInnerClass(name: String, outerName: String?, innerName: String?, access: Int) {
+        super.visitInnerClass(name, outerName, innerName, access)
+        myInnerClasses.add(name)
+    }
+}
@@ -0,0 +1,94 @@
+package org.jetbrains.kotlin.jvm.abi
+
+import com.intellij.testFramework.UsefulTestCase
+import org.jetbrains.kotlin.incremental.isClassFile
+import org.jetbrains.kotlin.utils.Printer
+import org.jetbrains.org.objectweb.asm.*
+import java.io.File
+
+abstract class AbstractJvmAbiContentTest : BaseJvmAbiTest() {
+    fun doTest(path: String) {
+        val testDir = File(path)
+        val compilation = Compilation(testDir, name = null).also { make(it) }
+
+        val classToBytecode = hashMapOf<File, String>()
+        val baseDir = compilation.abiDir
+        val classFiles = baseDir.walk().filter { it.isFile && it.isClassFile() }
+        for (classFile in classFiles) {
+            val bytes = classFile.readBytes()
+            val reader = ClassReader(bytes)
+            val sb = StringBuilder()
+            val p = Printer(sb)
+            val visitor = PrintingClassVisitor(p)
+            reader.accept(visitor, 0)
+            classToBytecode[classFile] = sb.toString()
+        }
+
+        val actual = classToBytecode.entries
+            .sortedBy { it.key.relativeTo(baseDir) }
+            .joinToString("\n") { it.value }
+        val signaturesFile = testDir.resolve("signatures.txt")
+        if (!signaturesFile.exists()) {
+            signaturesFile.writeText("")
+        }
+        UsefulTestCase.assertSameLinesWithFile(signaturesFile.canonicalPath, actual)
+    }
+}
+
+private class PrintingClassVisitor(private val p: Printer) : ClassVisitor(Opcodes.ASM6) {
+    override fun visit(
+        version: Int,
+        access: Int,
+        name: String,
+        signature: String?,
+        superName: String?,
+        interfaces: Array<out String>?
+    ) {
+        val superTypes = linkedSetOf<String>()
+        superName?.let { superTypes.add(it) }
+        interfaces?.forEach { superTypes.add(it) }
+        val supertypeString =
+            if (superTypes.isEmpty()) ""
+            else " : ${superTypes.joinToString()}"
+        val classOrInterface = if (access and Opcodes.ACC_INTERFACE == Opcodes.ACC_INTERFACE) "interface" else "class"
+
+        p.println("$classOrInterface $name$supertypeString {")
+        p.pushIndent()
+
+        super.visit(version, access, name, signature, superName, interfaces)
+    }
+
+    override fun visitField(access: Int, name: String, desc: String, signature: String?, value: Any?): FieldVisitor? {
+        val flags = memberFlagsString(access)
+        p.println("$flags val $name $desc = $value")
+
+        return super.visitField(access, name, desc, signature, value)
+    }
+
+    override fun visitMethod(access: Int, name: String, desc: String, signature: String?, exceptions: Array<out String>?): MethodVisitor? {
+        val flags = memberFlagsString(access)
+        p.println("$flags fun $name $desc")
+
+        return super.visitMethod(access, name, desc, signature, exceptions)
+    }
+
+    override fun visitEnd() {
+        p.popIndent()
+        p.println("}")
+
+        super.visitEnd()
+    }
+
+    private fun memberFlagsString(flags: Int): String =
+        listOfNotNull(
+            flagToString(flags, Opcodes.ACC_PUBLIC, "public"),
+            flagToString(flags, Opcodes.ACC_PROTECTED, "protected"),
+            flagToString(flags, Opcodes.ACC_PRIVATE, "private"),
+            flagToString(flags, Opcodes.ACC_STATIC, "static"),
+            flagToString(flags, Opcodes.ACC_FINAL, "final"),
+            flagToString(flags, Opcodes.ACC_ABSTRACT, "abstract")
+        ).joinToString(" ")
+
+    private fun flagToString(flags: Int, flag: Int, flagName: String): String? =
+        flagName.takeIf { flags and flag == flag }
+}
@@ -32,17 +32,17 @@ abstract class BaseJvmAbiTest : TestCase() {

    inner class Compilation(
        val projectDir: File,
-        val name: String,
+        val name: String?,
        val dependencies: Collection<Compilation> = emptyList()
    ) {
        val srcDir: File
-            get() = projectDir.resolve(name)
+            get() = if (name == null) projectDir else projectDir.resolve(name)

        val destinationDir: File
-            get() = workingDir.resolve("$name/out")
+            get() = if (name == null) workingDir.resolve("out") else workingDir.resolve("$name/out")

        val abiDir: File
-            get() = workingDir.resolve("$name/abi")
+            get() = if (name == null) workingDir.resolve("abi") else workingDir.resolve("$name/abi")

        override fun toString(): String =
            "compilation '$name'"
@@ -64,6 +64,11 @@ public class CompareJvmAbiTestGenerated extends AbstractCompareJvmAbiTest {
        runTest("plugins/jvm-abi-gen/testData/compare/inlineFunctionBody/");
    }

+    @TestMetadata("innerClasses")
+    public void testInnerClasses() throws Exception {
+        runTest("plugins/jvm-abi-gen/testData/compare/innerClasses/");
+    }
+
    @TestMetadata("lambda")
    public void testLambda() throws Exception {
        runTest("plugins/jvm-abi-gen/testData/compare/lambda/");
@@ -0,0 +1,36 @@
+/*
+ * 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.jvm.abi;
+
+import com.intellij.testFramework.TestDataPath;
+import org.jetbrains.kotlin.test.JUnit3RunnerWithInners;
+import org.jetbrains.kotlin.test.KotlinTestUtils;
+import org.jetbrains.kotlin.test.TargetBackend;
+import org.jetbrains.kotlin.test.TestMetadata;
+import org.junit.runner.RunWith;
+
+import java.io.File;
+import java.util.regex.Pattern;
+
+/** This class is generated by {@link org.jetbrains.kotlin.generators.tests.TestsPackage}. DO NOT MODIFY MANUALLY */
+@SuppressWarnings("all")
+@TestMetadata("plugins/jvm-abi-gen/testData/content")
+@TestDataPath("$PROJECT_ROOT")
+@RunWith(JUnit3RunnerWithInners.class)
+public class JvmAbiContentTestGenerated extends AbstractJvmAbiContentTest {
+    private void runTest(String testDataFilePath) throws Exception {
+        KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath);
+    }
+
+    public void testAllFilesPresentInContent() throws Exception {
+        KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("plugins/jvm-abi-gen/testData/content"), Pattern.compile("^([^\\.]+)$"), TargetBackend.ANY, false);
+    }
+
+    @TestMetadata("class")
+    public void testClass() throws Exception {
+        runTest("plugins/jvm-abi-gen/testData/content/class/");
+    }
+}
@@ -0,0 +1,31 @@
+package test
+
+class A {
+    private inner class AInner {
+        inner class AInnerInner {}
+
+        fun aInnerMethod() {
+            class AInnerMethodLocal
+
+            val aInnerMethodLambda: (Int) -> Int = { it * it }
+        }
+    }
+
+    private class ANested {
+        inner class ANestedInner {}
+        class ANestedNested
+        object ANestedObject
+
+        fun aNestedMethod() {
+            class ANestedMethodLocal
+
+            val aNestedMethodLambda: (Int) -> Int = { it * it }
+        }
+    }
+}
+
+private class B {
+    inner class BInnerClass
+    class BNestedClass
+    object BObject {}
+}
@@ -0,0 +1,3 @@
+package test
+
+class A
@@ -0,0 +1,39 @@
+package test
+
+interface Interface {
+}
+
+open class BaseClass {
+    open val baseClassPublicVal: Int = 0
+    open fun baseClassPublicFun(): Int = 1
+
+    internal val baseClassInternalVal: Int = 2
+    internal fun baseClassInternalFun(): Int = 3
+
+    protected val baseClassProtectedVal: Int = 4
+    protected fun baseClassProtectedFun(): Int = 5
+
+    private val baseClassPrivateVal: Int = 6
+    private fun baseClassPrivateFun(): Int = 7
+
+    companion object {
+        const val basePublicConst: Int = 8
+        private const val basePrivateConst: Int = 9
+    }
+}
+
+class Class : BaseClass(), Interface {
+    fun classPublicMethod() {
+        class publicMethodLocalClass {
+            val x = 0
+        }
+
+        val publicMethodLambda: (Int) -> Int = { it * it }
+    }
+
+    private class NestedInnerClass() {
+        class NestedNestedInnerClass() {}
+    }
+}
+
+private class PrivateClass
@@ -0,0 +1,23 @@
+class test/BaseClass$Companion : java/lang/Object {
+    public fun <init> (Lkotlin/jvm/internal/DefaultConstructorMarker;)V
+}
+
+class test/BaseClass : java/lang/Object {
+    public static final val basePublicConst I = 8
+    public static final val Companion Ltest/BaseClass$Companion; = null
+    public fun getBaseClassPublicVal ()I
+    public fun baseClassPublicFun ()I
+    public final fun getBaseClassInternalVal$main ()I
+    public final fun baseClassInternalFun$main ()I
+    protected final fun getBaseClassProtectedVal ()I
+    protected final fun baseClassProtectedFun ()I
+    public fun <init> ()V
+}
+
+class test/Class : test/BaseClass, test/Interface {
+    public final fun classPublicMethod ()V
+    public fun <init> ()V
+}
+
+interface test/Interface : java/lang/Object {
+}