KT-45777: Take snapshots and compute changes for Kotlin classes

Reuse the existing IncrementalJvmCache to take snapshots and compute changes for Kotlin classes. Java classes will be handled next. Bug: KT-45777 Test: New ClasspathSnapshotterTest, ClasspathChangesComputerTest
2021-07-29 11:58:40 +03:00
parent 7e04bb4bf1
commit 206457d9ff
14 changed files with 387 additions and 29 deletions
@@ -25,6 +25,7 @@ import org.jetbrains.annotations.TestOnly
 import org.jetbrains.kotlin.build.GeneratedJvmClass
 import org.jetbrains.kotlin.incremental.storage.*
 import org.jetbrains.kotlin.inline.inlineFunctionsJvmNames
+import org.jetbrains.kotlin.load.kotlin.FileBasedKotlinClass
 import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader
 import org.jetbrains.kotlin.load.kotlin.incremental.components.IncrementalCache
 import org.jetbrains.kotlin.load.kotlin.incremental.components.JvmPackagePartProto
@@ -113,7 +114,7 @@ open class IncrementalJvmCache(
    }

    open fun saveFileToCache(generatedClass: GeneratedJvmClass, changesCollector: ChangesCollector) {
-        saveClassToCache(KotlinClassInfo(generatedClass.outputClass), generatedClass.sourceFiles, changesCollector)
+        saveClassToCache(KotlinClassInfo.createFrom(generatedClass.outputClass), generatedClass.sourceFiles, changesCollector)
    }

    /**
@@ -612,16 +613,6 @@ class KotlinClassInfo private constructor(
    val inlineFunctionsMap: LinkedHashMap<String, Long>
 ) {

-    constructor(kotlinClass: LocalFileKotlinClass) : this(
-        kotlinClass.classId,
-        kotlinClass.classHeader.kind,
-        kotlinClass.classHeader.data,
-        kotlinClass.classHeader.strings,
-        kotlinClass.classHeader.multifileClassName,
-        getConstantsMap(kotlinClass.fileContents),
-        getInlineFunctionsMap(kotlinClass.classHeader, kotlinClass.fileContents)
-    )
-
    val className: JvmClassName by lazy { JvmClassName.byClassId(classId) }

    fun scopeFqName(companion: Boolean = false) = when (classKind) {
@@ -630,6 +621,36 @@ class KotlinClassInfo private constructor(
        }
        else -> className.packageFqName
    }
+
+    companion object {
+
+        fun createFrom(kotlinClass: LocalFileKotlinClass): KotlinClassInfo {
+            return KotlinClassInfo(
+                kotlinClass.classId,
+                kotlinClass.classHeader.kind,
+                kotlinClass.classHeader.data,
+                kotlinClass.classHeader.strings,
+                kotlinClass.classHeader.multifileClassName,
+                getConstantsMap(kotlinClass.fileContents),
+                getInlineFunctionsMap(kotlinClass.classHeader, kotlinClass.fileContents)
+            )
+        }
+
+        /** Creates [KotlinClassInfo] from the given classContents, or returns `null` if the class is not a kotlinc-generated class. */
+        fun tryCreateFrom(classContents: ByteArray): KotlinClassInfo? {
+            return FileBasedKotlinClass.create(classContents) { classId, _, classHeader, _ ->
+                KotlinClassInfo(
+                    classId,
+                    classHeader.kind,
+                    classHeader.data,
+                    classHeader.strings,
+                    classHeader.multifileClassName,
+                    getConstantsMap(classContents),
+                    getInlineFunctionsMap(classHeader, classContents)
+                )
+            }
+        }
+    }
 }

 private fun getConstantsMap(bytes: ByteArray): LinkedHashMap<String, Any> {
@@ -694,4 +715,4 @@ private fun getInlineFunctionsMap(header: KotlinClassHeader, bytes: ByteArray):
    }, 0)

    return result
-}
+}
@@ -5,6 +5,8 @@

 package org.jetbrains.kotlin.gradle.incremental

+import com.intellij.openapi.util.io.FileUtil
+import org.jetbrains.annotations.TestOnly
 import org.jetbrains.kotlin.build.report.BuildReporter
 import org.jetbrains.kotlin.build.report.ICReporter
 import org.jetbrains.kotlin.build.report.metrics.BuildAttribute
@@ -16,6 +18,8 @@ import org.jetbrains.kotlin.incremental.*
 import java.io.File
 import org.jetbrains.kotlin.gradle.incremental.ChangesCollectorResult.Success
 import org.jetbrains.kotlin.gradle.incremental.ChangesCollectorResult.Failure
+import org.jetbrains.kotlin.incremental.storage.FileToCanonicalPathConverter
+import java.util.*

 /** Computes [ClasspathChanges] between two [ClasspathSnapshot]s .*/
 object ClasspathChangesComputer {
@@ -65,21 +69,39 @@ object ClasspathChangesComputer {
        for (key in current.classSnapshots.keys) {
            val currentSnapshot = current.classSnapshots[key]!!
            val previousSnapshot = previous.classSnapshots[key] ?: return Failure.AddedRemovedClasses
-            val result = collectClassChanges(currentSnapshot, previousSnapshot, changesCollector)
-            if (result is Failure) {
-                return result
+            if (currentSnapshot !is KotlinClassSnapshot || previousSnapshot !is KotlinClassSnapshot) {
+                return Failure.NotYetImplemented
            }
+            // TODO: Store results in changesCollector
+            collectKotlinClassChanges(currentSnapshot, previousSnapshot)
        }
        return Success
    }

-    @Suppress("UNUSED_PARAMETER")
-    private fun collectClassChanges(
-        current: ClassSnapshot,
-        previous: ClassSnapshot,
-        changesCollector: ChangesCollector
-    ): ChangesCollectorResult {
-        return Failure.NotYetImplemented
+    @TestOnly
+    internal fun collectKotlinClassChanges(current: KotlinClassSnapshot, previous: KotlinClassSnapshot): DirtyData {
+        // TODO Create IncrementalJvmCache early once and reuse it here
+        val workingDir =
+            FileUtil.createTempDirectory(this::class.java.simpleName, "_WorkingDir_${UUID.randomUUID()}", /* deleteOnExit */ true)
+        val incrementalJvmCache = IncrementalJvmCache(workingDir, /* targetOutputDir */ null, FileToCanonicalPathConverter)
+
+        // Store previous snapshot in incrementalJvmCache, the returned ChangesCollector result is not used.
+        incrementalJvmCache.saveClassToCache(
+            kotlinClassInfo = previous.classInfo,
+            sourceFiles = null,
+            changesCollector = ChangesCollector()
+        )
+
+        // Compute changes between the current snapshot and the previously stored snapshot, and store the result in changesCollector.
+        val changesCollector = ChangesCollector()
+        incrementalJvmCache.saveClassToCache(
+            kotlinClassInfo = current.classInfo,
+            sourceFiles = null,
+            changesCollector = changesCollector
+        )
+
+        workingDir.deleteRecursively()
+        return changesCollector.getDirtyData(listOf(incrementalJvmCache), NoOpBuildReporter.NoOpICReporter)
    }
 }

@@ -5,6 +5,7 @@

 package org.jetbrains.kotlin.gradle.incremental

+import org.jetbrains.kotlin.incremental.KotlinClassInfo
 import java.io.*

 /** Snapshot of a classpath. It consists of a list of [ClasspathEntrySnapshot]s. */
@@ -29,7 +30,23 @@ class ClasspathEntrySnapshot(
 * Snapshot of a class. It contains information to compute the source files that need to be recompiled during an incremental run of the
 * `KotlinCompile` task.
 */
-class ClassSnapshot : Serializable {
+abstract class ClassSnapshot : Serializable {
+
+    companion object {
+        private const val serialVersionUID = 0L
+    }
+}
+
+/** [ClassSnapshot] of a kotlinc-generated class. */
+class KotlinClassSnapshot(val classInfo: KotlinClassInfo) : ClassSnapshot(), Serializable {
+
+    companion object {
+        private const val serialVersionUID = 0L
+    }
+}
+
+/** [ClassSnapshot] of a non-kotlinc-generated class. */
+class JavaClassSnapshot : ClassSnapshot(), Serializable {

    // TODO WORK-IN-PROGRESS

@@ -6,6 +6,7 @@
 package org.jetbrains.kotlin.gradle.incremental

 import org.jetbrains.kotlin.gradle.incremental.ClasspathEntryContentsReader.Companion.DEFAULT_CLASS_FILTER
+import org.jetbrains.kotlin.incremental.KotlinClassInfo
 import java.io.File
 import java.util.zip.ZipInputStream

@@ -18,8 +19,8 @@ object ClasspathEntrySnapshotter {
            ClasspathEntryContentsReader.from(classpathEntry).readContents(DEFAULT_CLASS_FILTER)

        val pathsToSnapshots = LinkedHashMap<String, ClassSnapshot>()
-        pathsToContents.mapValuesTo(pathsToSnapshots) { (invariantSeparatorsRelativePath, classContents) ->
-            ClassSnapshotter.snapshot(invariantSeparatorsRelativePath, classContents)
+        pathsToContents.mapValuesTo(pathsToSnapshots) { (_, classContents) ->
+            ClassSnapshotter.snapshot(classContents)
        }

        return ClasspathEntrySnapshot(pathsToSnapshots)
@@ -30,10 +31,10 @@ object ClasspathEntrySnapshotter {
@Suppress("SpellCheckingInspection")
 object ClassSnapshotter {

-    @Suppress("UNUSED_PARAMETER")
-    fun snapshot(invariantSeparatorsRelativePath: String, classContents: ByteArray): ClassSnapshot {
-        // TODO WORK-IN-PROGRESS
-        return ClassSnapshot()
+    fun snapshot(classContents: ByteArray): ClassSnapshot {
+        // TODO Add custom serialization to KotlinClassInfo then return it here. For now, use JavaClassSnapshot.
+        KotlinClassInfo.tryCreateFrom(classContents)?.let { KotlinClassSnapshot(it) }
+        return JavaClassSnapshot()
    }
 }

@@ -0,0 +1,69 @@
+/*
+ * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * 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.gradle.incremental
+
+import org.jetbrains.kotlin.incremental.DirtyData
+import org.jetbrains.kotlin.incremental.LookupSymbol
+import org.jetbrains.kotlin.name.FqName
+import org.jetbrains.kotlin.resolve.sam.SAM_LOOKUP_NAME
+import org.junit.Assert.assertEquals
+import org.junit.Before
+import org.junit.Test
+
+abstract class ClasspathChangesComputerTest : ClasspathSnapshotTestCommon() {
+
+    protected abstract val testSourceFile: ChangeableTestSourceFile
+
+    private lateinit var originalSnapshot: ClassSnapshot
+
+    @Before
+    fun setUp() {
+        originalSnapshot = testSourceFile.compileAndSnapshot()
+    }
+
+    // TODO Add more test cases:
+    //   - private/non-private fields
+    //   - inline functions
+    //   - changing supertype by adding somethings that changes/does not change the supertype ABI
+    //   - adding an annotation
+
+    @Test
+    fun testCollectClassChanges_changedPublicMethodSignature() {
+        val updatedSnapshot = testSourceFile.changePublicMethodSignature().compileAndSnapshot()
+        val dirtyData = ClasspathChangesComputer.collectKotlinClassChanges(
+            updatedSnapshot as KotlinClassSnapshot,
+            originalSnapshot as KotlinClassSnapshot
+        )
+
+        assertEquals(
+            DirtyData(
+                dirtyLookupSymbols = setOf(
+                    LookupSymbol(name = SAM_LOOKUP_NAME.asString(), scope = "com.example.SimpleKotlinClass"),
+                    LookupSymbol(name = "publicMethod", scope = "com.example.SimpleKotlinClass"),
+                    LookupSymbol(name = "changedPublicMethod", scope = "com.example.SimpleKotlinClass")
+                ),
+                dirtyClassesFqNames = setOf(FqName("com.example.SimpleKotlinClass")),
+                dirtyClassesFqNamesForceRecompile = emptySet()
+            ),
+            dirtyData
+        )
+    }
+
+    @Test
+    fun testCollectClassChanges_changedMethodImplementation() {
+        val updatedSnapshot = testSourceFile.changeMethodImplementation().compileAndSnapshot()
+        val dirtyData = ClasspathChangesComputer.collectKotlinClassChanges(
+            updatedSnapshot as KotlinClassSnapshot,
+            originalSnapshot as KotlinClassSnapshot
+        )
+
+        assertEquals(DirtyData(emptySet(), emptySet(), emptySet()), dirtyData)
+    }
+}
+
+class KotlinClassesClasspathChangesComputerTest : ClasspathChangesComputerTest() {
+    override val testSourceFile = SimpleKotlinClass(tmpDir)
+}
@@ -0,0 +1,103 @@
+/*
+ * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * 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.gradle.incremental
+
+import com.google.gson.GsonBuilder
+import org.jetbrains.kotlin.incremental.KotlinClassInfo
+import org.junit.Rule
+import org.junit.rules.TemporaryFolder
+import java.io.File
+
+abstract class ClasspathSnapshotTestCommon {
+
+    companion object {
+        val testDataDir = File("libraries/tools/kotlin-gradle-plugin/src/testData/kotlin.incremental.useClasspathSnapshot")
+    }
+
+    @get:Rule
+    val tmpDir = TemporaryFolder()
+
+    abstract class RelativeFile(val baseDir: File, val relativePath: String) {
+        fun asFile() = File(baseDir, relativePath)
+    }
+
+    class SourceFile(baseDir: File, relativePath: String) : RelativeFile(baseDir, relativePath) {
+        init {
+            check(relativePath.endsWith(".kt") || relativePath.endsWith(".java"))
+        }
+    }
+
+    class ClassFile(baseDir: File, relativePath: String) : RelativeFile(baseDir, relativePath) {
+        init {
+            check(relativePath.endsWith(".class"))
+        }
+
+        fun snapshot() = KotlinClassSnapshot(KotlinClassInfo.tryCreateFrom(asFile().readBytes())!!)
+    }
+
+    open class TestSourceFile(val sourceFile: SourceFile, val tmpDir: TemporaryFolder) {
+
+        fun replace(oldValue: String, newValue: String): TestSourceFile {
+            val fileContents = sourceFile.asFile().readText()
+            check(fileContents.contains(oldValue)) { "String '$oldValue' not found in file '${sourceFile.asFile().path}'" }
+
+            val newSourceFile = SourceFile(tmpDir.newFolder(), sourceFile.relativePath).also { it.asFile().parentFile.mkdirs() }
+            newSourceFile.asFile().writeText(fileContents.replace(oldValue, newValue))
+            return TestSourceFile(newSourceFile, tmpDir)
+        }
+
+        fun compile(): ClassFile {
+            return when {
+                sourceFile.relativePath.endsWith(".kt") -> compileKotlin()
+                else -> error("Unexpected file name extension: '${sourceFile.asFile().path}'")
+            }
+        }
+
+        private fun compileKotlin(): ClassFile {
+            // TODO: Call Kotlin compiler to generate classes (see https://github.com/JetBrains/kotlin/pull/4512#discussion_r679432232)
+            return ClassFile(
+                File(sourceFile.baseDir.path.substringBeforeLast("src") + "classes" + sourceFile.baseDir.path.substringAfterLast("src")),
+                sourceFile.relativePath.substringBeforeLast('.') + ".class"
+            )
+        }
+
+        fun compileAndSnapshot(): ClassSnapshot = compile().snapshot()
+    }
+
+    abstract class ChangeableTestSourceFile(sourceFile: SourceFile, tmpDir: TemporaryFolder) : TestSourceFile(sourceFile, tmpDir) {
+
+        abstract fun changePublicMethodSignature(): TestSourceFile
+
+        abstract fun changeMethodImplementation(): TestSourceFile
+    }
+
+    class SimpleKotlinClass(tmpDir: TemporaryFolder) : ChangeableTestSourceFile(
+        SourceFile(File(testDataDir, "src/original"), "com/example/SimpleKotlinClass.kt"), tmpDir
+    ) {
+
+        override fun changePublicMethodSignature(): TestSourceFile {
+            return TestSourceFile(
+                SourceFile(
+                    File(sourceFile.baseDir.path.replace("original", "changedPublicMethodSignature")),
+                    sourceFile.relativePath
+                ), tmpDir
+            )
+        }
+
+        override fun changeMethodImplementation(): TestSourceFile {
+            return TestSourceFile(
+                SourceFile(
+                    File(sourceFile.baseDir.path.replace("original", "changedMethodImplementation")),
+                    sourceFile.relativePath
+                ), tmpDir
+            )
+        }
+    }
+
+    // Use Gson to compare objects
+    private val gson by lazy { GsonBuilder().setPrettyPrinting().create() }
+    protected fun Any.toGson(): String = gson.toJson(this)
+}
@@ -0,0 +1,54 @@
+/*
+ * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+@file:Suppress("SpellCheckingInspection")
+
+package org.jetbrains.kotlin.gradle.incremental
+
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotEquals
+import org.junit.Before
+import org.junit.Test
+import java.io.File
+
+abstract class ClasspathSnapshotterTest : ClasspathSnapshotTestCommon() {
+
+    protected abstract val testSourceFile: ChangeableTestSourceFile
+
+    private lateinit var testClassSnapshot: ClassSnapshot
+
+    @Before
+    fun setUp() {
+        testClassSnapshot = testSourceFile.compileAndSnapshot()
+    }
+
+    @Test
+    fun `test ClassSnapshotter's result against expected snapshot`() {
+        val expectedSnapshot = File("${testSourceFile.sourceFile.asFile().path.substringBeforeLast('.')}-expected-snapshot.json").readText()
+        assertEquals(expectedSnapshot, testClassSnapshot.toGson())
+    }
+
+    @Test
+    fun `test ClassSnapshotter extracts ABI info from a class`() {
+        // Change public method signature
+        val updatedSnapshot = testSourceFile.changePublicMethodSignature().compileAndSnapshot()
+
+        // The snapshot must change
+        assertNotEquals(testClassSnapshot.toGson(), updatedSnapshot.toGson())
+    }
+
+    @Test
+    fun `test ClassSnapshotter does not extract non-ABI info from a class`() {
+        // Change method implementation
+        val updatedSnapshot = testSourceFile.changeMethodImplementation().compileAndSnapshot()
+
+        // The snapshot must not change
+        assertEquals(testClassSnapshot.toGson(), updatedSnapshot.toGson())
+    }
+}
+
+class KotlinClassesClasspathSnapshotterTest : ClasspathSnapshotterTest() {
+    override val testSourceFile = SimpleKotlinClass(tmpDir)
+}
@@ -0,0 +1,12 @@
+package com.example
+
+class SimpleKotlinClass {
+
+    fun publicMethod() {
+        println("This method implementation has changed!")
+    }
+
+    private fun privateMethod() {
+        println("I'm in a private method")
+    }
+}
@@ -0,0 +1,12 @@
+package com.example
+
+class SimpleKotlinClass {
+
+    fun changedPublicMethod() {
+        println("I'm in a public method")
+    }
+
+    private fun privateMethod() {
+        println("I'm in a private method")
+    }
+}
@@ -0,0 +1,35 @@
+{
+  "classInfo": {
+    "classId": {
+      "packageFqName": {
+        "fqName": {
+          "fqName": "com.example"
+        }
+      },
+      "relativeClassName": {
+        "fqName": {
+          "fqName": "SimpleKotlinClass"
+        }
+      },
+      "local": false
+    },
+    "classKind": "CLASS",
+    "classHeaderData": [
+      "\u0000\u0012\n\u0002\u0018\u0002\n\u0002\u0010\u0000\n\u0002\b\u0002\n\u0002\u0010\u0002\n\u0000\u0018\u00002\u00020\u0001B\u0005¢\u0006\u0002\u0010\u0002J\b\u0010\u0003\u001a\u00020\u0004H\u0002J\u0006\u0010\u0005\u001a\u00020\u0004"
+    ],
+    "classHeaderStrings": [
+      "Lcom/example/SimpleKotlinClass;",
+      "",
+      "()V",
+      "privateMethod",
+      "",
+      "publicMethod"
+    ],
+    "constantsMap": {},
+    "inlineFunctionsMap": {},
+    "className$delegate": {
+      "initializer": {},
+      "_value": {}
+    }
+  }
+}
@@ -0,0 +1,12 @@
+package com.example
+
+class SimpleKotlinClass {
+
+    fun publicMethod() {
+        println("I'm in a public method")
+    }
+
+    private fun privateMethod() {
+        println("I'm in a private method")
+    }
+}