KT-41456: Incremental KAPT - check compiled sources before running incrementally

Because incremental KAPT tries to reuse .class files produced by kotlinc
and javac, it should check for their existence before starting
an incremental run. Otherwise, annotation processors that perform type
validation will fail to run.

Current check counts the number of declared types in processed .java
sources, and it makes sure the total number of .class files in compiled
sources dirs is equal or higher. Otherwise, KAPT runs non-incrementally.

Tests: KaptIncrementalWithIsolatingApt.testMissingKotlinOutputForcesNonIncrementalRun

^KT-41456 In Progress

libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/KaptIncrementalWithIsolatingApt.kt

@@ -403,6 +403,23 @@ class KaptIncrementalWithIsolatingApt : KaptIncrementalIT() {
             }
         }
     }
+
+    @DisplayName("KT-41456: detect missing kotlin compilation output")
+    @GradleTest
+    @DisabledOnOs(OS.WINDOWS, disabledReason = "https://youtrack.jetbrains.com/issue/KTI-405")
+    fun testMissingKotlinOutputForcesNonIncrementalRun(gradleVersion: GradleVersion) {
+        kaptProject(gradleVersion) {
+            build("clean", "assemble")
+
+            // Explicitly remove all kotlinc generated .class files
+            projectPath.resolve("build/classes/kotlin/main").toFile().listFiles()!!.forEach { it.deleteRecursively() }
+            javaSourcesDir().resolve("bar/useB.kt").modify { current -> "$current\nfun otherFunction() {}" }
+
+            build("assemble") {
+                assertOutputContains("Unable to run incrementally, processing all sources.")
+            }
+        }
+    }
 }
 
 private const val patternApt = "Processing java sources with annotation processors:"

plugins/kapt3/kapt3-base/src/org/jetbrains/kotlin/kapt3/base/KaptContext.kt

@@ -74,10 +74,14 @@ open class KaptContext(val options: KaptOptions, val withJdk: Boolean, val logge
             sourcesToReprocess = run {
                 val start = System.currentTimeMillis()
                 cacheManager?.invalidateAndGetDirtyFiles(
-                    options.changedFiles, options.classpathChanges
-                ).also {
+                    options.changedFiles, options.classpathChanges, options.compiledSources
+                ).also { result ->
                     if (logger.isVerbose) {
-                        logger.info("Computing sources to reprocess took ${System.currentTimeMillis() - start}[ms].")
+                        if (result == SourcesToReprocess.FullRebuild) {
+                            logger.info("Unable to run incrementally, processing all sources.")
+                        } else {
+                            logger.info("Computing sources to reprocess took ${System.currentTimeMillis() - start}[ms].")
+                        }
                     }
                 }
             }?: SourcesToReprocess.FullRebuild

plugins/kapt3/kapt3-base/src/org/jetbrains/kotlin/kapt3/base/incremental/cache.kt

@@ -36,11 +36,35 @@ class JavaClassCacheManager(val file: File) : Closeable {
      * From set of changed sources, get list of files to recompile using structural information and dependency information from
      * annotation processing.
      */
-    fun invalidateAndGetDirtyFiles(changedSources: Collection<File>, dirtyClasspathJvmNames: Collection<String>): SourcesToReprocess {
+    fun invalidateAndGetDirtyFiles(
+        changedSources: Collection<File>,
+        dirtyClasspathJvmNames: Collection<String>,
+        compiledSources: List<File>
+    ): SourcesToReprocess {
         if (!aptCache.isIncremental) {
             return SourcesToReprocess.FullRebuild
         }
 
+        /**
+         * Incremental annotation processing tries to limit the number of .java files that are passes to annotation processors, and it
+         * uses already existing .class files to resolve types. This relies on outputs of kotlinc and javac being present.
+         *
+         * The check below verifies that the number of types defined in the processed .java files matches the number of .class files
+         * in the compiled sources. This check does not guarantee that .class file will indeed exist for every defined type, but
+         * even if such check is introduced, there is no guarantee that the actual content of the source file matches the .class file. E.g:
+         * - clean build i.e non-incremental KAPT
+         * - modify B.java with error in the method body: KAPT runs successfully, but java compilation will fail (assume old B.class is kept)
+         * - add C.java that references B as method parameter type: KAPT wil only process C.java, and it will use old version of B
+         * To properly fix this, KAPT needs to keep track of kotlinc and javac compilation outcome, and there is no clean API to do that.
+         *
+         * However, comparing total numbers is probably good enough, and it should eliminate most of the issues. See KT-41456 for details.
+         */
+        val totalCompiledClasses = compiledSources.sumOf { it.walk().filter { it.extension == "class" }.count() }
+        val totalDeclaredTypes = javaCache.getSourceFileDefinedTypesCount()
+        if (totalCompiledClasses < totalDeclaredTypes) {
+            return SourcesToReprocess.FullRebuild
+        }
+
         val dirtyClasspathFqNames = HashSet<String>(dirtyClasspathJvmNames.size)
         dirtyClasspathJvmNames.forEach {
             dirtyClasspathFqNames.add(it.replace("$", ".").replace("/", "."))

plugins/kapt3/kapt3-base/src/org/jetbrains/kotlin/kapt3/base/incremental/classStructureCache.kt

@@ -158,6 +158,18 @@ class JavaClassCache() : Serializable {
 
         allSources.forEach { sourceCache.remove(it) }
     }
+
+    /** Returns total number of declared types in .java source files that were processed. */
+    fun getSourceFileDefinedTypesCount(): Int {
+        return sourceCache.values.sumOf {
+            val structure = it as? SourceFileStructure ?: return@sumOf 0
+            if (structure.declaredTypes.size == 1 && structure.declaredTypes.single() == "error.NonExistentClass") {
+                // never report package for error.NonExistentClass, as it is never compiled by javac/kotlinc
+                return@sumOf 0
+            }
+            return@sumOf structure.declaredTypes.size
+        }
+    }
 }
 
 

plugins/kapt3/kapt3-base/test/org/jetbrains/kotlin/kapt3/base/incremental/IncrementalKaptTest.kt

@@ -129,7 +129,10 @@ class IncrementalKaptTest {
 
         val classesOutput = tmp.newFolder()
         compileSources(sourcesDir.listFiles().asIterable(), classesOutput)
-        compileSources(listOf(outputDir.resolve("test/UserGenerated.java")), classesOutput)
+        compileSources(
+            listOf(outputDir.resolve("test/UserGenerated.java"), outputDir.resolve("test/AddressGenerated.java")),
+            classesOutput
+        )
 
         val optionsForSecondRun = KaptOptions.Builder().apply {
             projectBaseDir = tmp.newFolder()

plugins/kapt3/kapt3-base/test/org/jetbrains/kotlin/kapt3/base/incremental/JavaClassCacheManagerTest.kt

@@ -21,10 +21,12 @@ class JavaClassCacheManagerTest {
 
     private lateinit var cache: JavaClassCacheManager
     private lateinit var cacheDir: File
+    private lateinit var compiledSources: List<File>
 
     @Before
     fun setUp() {
         cacheDir = tmp.newFolder()
+        compiledSources = listOf(tmp.newFolder().also { it.resolve(TEST_PACKAGE_NAME).mkdir() })
         cache = JavaClassCacheManager(cacheDir)
     }
 
@@ -42,21 +44,24 @@ class JavaClassCacheManagerTest {
         SourceFileStructure(File("Mentioned.java").toURI()).also {
             it.addDeclaredType("test.Mentioned")
             cache.javaCache.addSourceStructure(it)
+            compiledSources.single().resolve("test/Mentioned.class").createNewFile()
         }
         SourceFileStructure(File("Src.java").toURI()).also {
             it.addDeclaredType("test.Src")
             it.addMentionedType("test.Mentioned")
             cache.javaCache.addSourceStructure(it)
+            compiledSources.single().resolve("test/Src.class").createNewFile()
         }
         SourceFileStructure(File("ReferencesSrc.java").toURI()).also {
             it.addDeclaredType("test.ReferencesSrc")
             it.addPrivateType("test.Src")
             cache.javaCache.addSourceStructure(it)
+            compiledSources.single().resolve("test/ReferencesSrc.class").createNewFile()
         }
         prepareForIncremental()
 
         val dirtyFiles =
-            cache.invalidateAndGetDirtyFiles(listOf(File("Mentioned.java").absoluteFile), emptyList()) as SourcesToReprocess.Incremental
+            cache.invalidateAndGetDirtyFiles(listOf(File("Mentioned.java").absoluteFile), emptyList(), compiledSources) as SourcesToReprocess.Incremental
         assertEquals(
             listOf(
                 File("Mentioned.java").absoluteFile,
@@ -71,21 +76,24 @@ class JavaClassCacheManagerTest {
         SourceFileStructure(File("Mentioned.java").toURI()).also {
             it.addDeclaredType("test.Mentioned")
             cache.javaCache.addSourceStructure(it)
+            compiledSources.single().resolve("test/Mentioned.class").createNewFile()
         }
         SourceFileStructure(File("Src.java").toURI()).also {
             it.addDeclaredType("test.Src")
             it.addPrivateType("test.Mentioned")
             cache.javaCache.addSourceStructure(it)
+            compiledSources.single().resolve("test/Src.class").createNewFile()
         }
         SourceFileStructure(File("ReferencesSrc.java").toURI()).also {
             it.addDeclaredType("test.ReferencesSrc")
             it.addPrivateType("test.Src")
             cache.javaCache.addSourceStructure(it)
+            compiledSources.single().resolve("test/ReferencesSrc.class").createNewFile()
         }
         prepareForIncremental()
 
         val dirtyFiles =
-            cache.invalidateAndGetDirtyFiles(listOf(File("Mentioned.java").absoluteFile), emptyList()) as SourcesToReprocess.Incremental
+            cache.invalidateAndGetDirtyFiles(listOf(File("Mentioned.java").absoluteFile), emptyList(), compiledSources) as SourcesToReprocess.Incremental
         assertEquals(
             listOf(
                 File("Mentioned.java").absoluteFile,
@@ -100,21 +108,25 @@ class JavaClassCacheManagerTest {
             it.addDeclaredType("test.TwoTypes")
             it.addDeclaredType("test.AnotherType")
             cache.javaCache.addSourceStructure(it)
+            compiledSources.single().resolve("test/TwoTypes.class").createNewFile()
+            compiledSources.single().resolve("test/AnotherType.class").createNewFile()
         }
         SourceFileStructure(File("ReferencesTwoTypes.java").toURI()).also {
             it.addDeclaredType("test.ReferencesTwoTypes")
             it.addPrivateType("test.TwoTypes")
             cache.javaCache.addSourceStructure(it)
+            compiledSources.single().resolve("test/ReferencesTwoTypes.class").createNewFile()
         }
         SourceFileStructure(File("ReferencesAnotherType.java").toURI()).also {
             it.addDeclaredType("test.ReferencesAnotherType")
             it.addPrivateType("test.AnotherType")
             cache.javaCache.addSourceStructure(it)
+            compiledSources.single().resolve("test/ReferencesAnotherType.class").createNewFile()
         }
         prepareForIncremental()
 
         val dirtyFiles =
-            cache.invalidateAndGetDirtyFiles(listOf(File("TwoTypes.java").absoluteFile), emptyList()) as SourcesToReprocess.Incremental
+            cache.invalidateAndGetDirtyFiles(listOf(File("TwoTypes.java").absoluteFile), emptyList(), compiledSources) as SourcesToReprocess.Incremental
         assertEquals(
             listOf(
                 File("TwoTypes.java").absoluteFile,
@@ -130,10 +142,11 @@ class JavaClassCacheManagerTest {
             it.addDeclaredType("test.Src")
             it.addMentionedType("test.Mentioned")
             cache.javaCache.addSourceStructure(it)
+            compiledSources.single().resolve("test/Src.class").createNewFile()
         }
         prepareForIncremental()
 
-        val dirtyFiles = cache.invalidateAndGetDirtyFiles(listOf(), listOf("test/Mentioned")) as SourcesToReprocess.Incremental
+        val dirtyFiles = cache.invalidateAndGetDirtyFiles(listOf(), listOf("test/Mentioned"), compiledSources) as SourcesToReprocess.Incremental
         assertEquals(listOf(File("Src.java").absoluteFile), dirtyFiles.toReprocess)
     }
 
@@ -142,22 +155,25 @@ class JavaClassCacheManagerTest {
         SourceFileStructure(File("Constants.java").toURI()).also {
             it.addDeclaredType("test.Constants")
             cache.javaCache.addSourceStructure(it)
+            compiledSources.single().resolve("test/Constants.class").createNewFile()
         }
         SourceFileStructure(File("MentionsConst.java").toURI()).also {
             it.addDeclaredType("test.MentionsConst")
             it.addMentionedConstant("test.Constants", "CONST")
             cache.javaCache.addSourceStructure(it)
+            compiledSources.single().resolve("test/MentionsConst.class").createNewFile()
         }
         SourceFileStructure(File("MentionsOtherConst.java").toURI()).also {
             it.addDeclaredType("test.MentionsOtherConst")
             it.addMentionedConstant("test.OtherConstants", "CONST")
             cache.javaCache.addSourceStructure(it)
+            compiledSources.single().resolve("test/MentionsOtherConst.class").createNewFile()
         }
         prepareForIncremental()
 
         val dirtyFiles =
             cache.invalidateAndGetDirtyFiles(
-                listOf(File("Constants.java").absoluteFile), emptyList()
+                listOf(File("Constants.java").absoluteFile), emptyList(), compiledSources
             ) as SourcesToReprocess.Incremental
         assertEquals(
             listOf(File("Constants.java").absoluteFile, File("MentionsConst.java").absoluteFile),
@@ -170,3 +186,5 @@ class JavaClassCacheManagerTest {
         cache = JavaClassCacheManager(cacheDir)
     }
 }
+
+private const val TEST_PACKAGE_NAME = "test"

plugins/kapt3/kapt3-base/test/org/jetbrains/kotlin/kapt3/base/incremental/TestSimpleIncrementalAptCache.kt

@@ -5,10 +5,7 @@
 
 package org.jetbrains.kotlin.kapt.base.test.org.jetbrains.kotlin.kapt3.base.incremental
 
-import org.jetbrains.kotlin.kapt3.base.incremental.IncrementalProcessor
-import org.jetbrains.kotlin.kapt3.base.incremental.JavaClassCacheManager
-import org.jetbrains.kotlin.kapt3.base.incremental.MentionedTypesTaskListener
-import org.jetbrains.kotlin.kapt3.base.incremental.SourcesToReprocess
+import org.jetbrains.kotlin.kapt3.base.incremental.*
 import org.junit.Assert.*
 import org.junit.Before
 import org.junit.Rule
@@ -24,11 +21,13 @@ class TestSimpleIncrementalAptCache {
 
     private lateinit var cache: JavaClassCacheManager
     private lateinit var generatedSources: File
+    private lateinit var compiledSources: List<File>
 
     @Before
     fun setUp() {
         cache = JavaClassCacheManager(tmp.newFolder())
         generatedSources = tmp.newFolder()
+        compiledSources = listOf(tmp.newFolder().also { it.resolve(TEST_PACKAGE_NAME).mkdir() })
         cache.close()
     }
 
@@ -36,7 +35,11 @@ class TestSimpleIncrementalAptCache {
     fun testAggregatingAnnotations() {
         runProcessor(SimpleProcessor().toAggregating())
 
-        val dirtyFiles = cache.invalidateAndGetDirtyFiles(listOf(TEST_DATA_DIR.resolve("User.java").absoluteFile), emptyList()) as SourcesToReprocess.Incremental
+        val dirtyFiles = cache.invalidateAndGetDirtyFiles(
+            listOf(TEST_DATA_DIR.resolve("User.java").absoluteFile),
+            emptyList(),
+            compiledSources
+        ) as SourcesToReprocess.Incremental
         assertEquals(
             listOf(TEST_DATA_DIR.resolve("User.java").absoluteFile, TEST_DATA_DIR.resolve("Address.java").absoluteFile),
             dirtyFiles.toReprocess
@@ -49,7 +52,11 @@ class TestSimpleIncrementalAptCache {
     fun testIsolatingAnnotations() {
         runProcessor(SimpleProcessor().toIsolating())
 
-        val dirtyFiles = cache.invalidateAndGetDirtyFiles(listOf(TEST_DATA_DIR.resolve("User.java").absoluteFile), emptyList()) as SourcesToReprocess.Incremental
+        val dirtyFiles = cache.invalidateAndGetDirtyFiles(
+            listOf(TEST_DATA_DIR.resolve("User.java").absoluteFile),
+            emptyList(),
+            compiledSources
+        ) as SourcesToReprocess.Incremental
         assertFalse(generatedSources.resolve("test/UserGenerated.java").exists())
         assertEquals(
             listOf(TEST_DATA_DIR.resolve("User.java").absoluteFile),
@@ -61,7 +68,7 @@ class TestSimpleIncrementalAptCache {
     fun testNonIncremental() {
         runProcessor(SimpleProcessor().toNonIncremental())
 
-        val dirtyFiles = cache.invalidateAndGetDirtyFiles(listOf(TEST_DATA_DIR.resolve("User.java").absoluteFile), emptyList())
+        val dirtyFiles = cache.invalidateAndGetDirtyFiles(listOf(TEST_DATA_DIR.resolve("User.java").absoluteFile), emptyList(), compiledSources)
         assertTrue(dirtyFiles is SourcesToReprocess.FullRebuild)
     }
 
@@ -73,5 +80,14 @@ class TestSimpleIncrementalAptCache {
             generatedSources
         ) { elementUtils, trees -> MentionedTypesTaskListener(cache.javaCache, elementUtils, trees) }
         cache.updateCache(listOf(processor), false)
+
+        // add mock compiled source files
+        compiledSources.single().resolve("test/User.class").createNewFile()
+        compiledSources.single().resolve("test/Address.class").createNewFile()
+        compiledSources.single().resolve("test/Observable.class").createNewFile()
+        compiledSources.single().resolve("test/UserGenerated.class").createNewFile()
+        compiledSources.single().resolve("test/AddressGenerated.class").createNewFile()
     }
-}
+}
+
+private const val TEST_PACKAGE_NAME = "test"