diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/KaptIncrementalWithIsolatingApt.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/KaptIncrementalWithIsolatingApt.kt index 66edec8c525..fe111b75cb8 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/KaptIncrementalWithIsolatingApt.kt +++ b/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:" diff --git a/plugins/kapt3/kapt3-base/src/org/jetbrains/kotlin/kapt3/base/KaptContext.kt b/plugins/kapt3/kapt3-base/src/org/jetbrains/kotlin/kapt3/base/KaptContext.kt index 5cf85417ec6..8432f613ce1 100644 --- a/plugins/kapt3/kapt3-base/src/org/jetbrains/kotlin/kapt3/base/KaptContext.kt +++ b/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 diff --git a/plugins/kapt3/kapt3-base/src/org/jetbrains/kotlin/kapt3/base/incremental/cache.kt b/plugins/kapt3/kapt3-base/src/org/jetbrains/kotlin/kapt3/base/incremental/cache.kt index 3d25d87f9aa..94b658347a0 100644 --- a/plugins/kapt3/kapt3-base/src/org/jetbrains/kotlin/kapt3/base/incremental/cache.kt +++ b/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, dirtyClasspathJvmNames: Collection): SourcesToReprocess { + fun invalidateAndGetDirtyFiles( + changedSources: Collection, + dirtyClasspathJvmNames: Collection, + compiledSources: List + ): 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(dirtyClasspathJvmNames.size) dirtyClasspathJvmNames.forEach { dirtyClasspathFqNames.add(it.replace("$", ".").replace("/", ".")) diff --git a/plugins/kapt3/kapt3-base/src/org/jetbrains/kotlin/kapt3/base/incremental/classStructureCache.kt b/plugins/kapt3/kapt3-base/src/org/jetbrains/kotlin/kapt3/base/incremental/classStructureCache.kt index 8cdd14f6d1d..27dd111cfe8 100644 --- a/plugins/kapt3/kapt3-base/src/org/jetbrains/kotlin/kapt3/base/incremental/classStructureCache.kt +++ b/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 + } + } } diff --git a/plugins/kapt3/kapt3-base/test/org/jetbrains/kotlin/kapt3/base/incremental/IncrementalKaptTest.kt b/plugins/kapt3/kapt3-base/test/org/jetbrains/kotlin/kapt3/base/incremental/IncrementalKaptTest.kt index 5fdb35a877f..236b1afa84d 100644 --- a/plugins/kapt3/kapt3-base/test/org/jetbrains/kotlin/kapt3/base/incremental/IncrementalKaptTest.kt +++ b/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() diff --git a/plugins/kapt3/kapt3-base/test/org/jetbrains/kotlin/kapt3/base/incremental/JavaClassCacheManagerTest.kt b/plugins/kapt3/kapt3-base/test/org/jetbrains/kotlin/kapt3/base/incremental/JavaClassCacheManagerTest.kt index f3c704c24eb..998c600e66e 100644 --- a/plugins/kapt3/kapt3-base/test/org/jetbrains/kotlin/kapt3/base/incremental/JavaClassCacheManagerTest.kt +++ b/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 @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" \ No newline at end of file diff --git a/plugins/kapt3/kapt3-base/test/org/jetbrains/kotlin/kapt3/base/incremental/TestSimpleIncrementalAptCache.kt b/plugins/kapt3/kapt3-base/test/org/jetbrains/kotlin/kapt3/base/incremental/TestSimpleIncrementalAptCache.kt index abb1259d2d2..7e786c5cf9b 100644 --- a/plugins/kapt3/kapt3-base/test/org/jetbrains/kotlin/kapt3/base/incremental/TestSimpleIncrementalAptCache.kt +++ b/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 @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() } -} \ No newline at end of file +} + +private const val TEST_PACKAGE_NAME = "test" \ No newline at end of file