Fixes to KAPT classpath change detection

1) Fix tests to use canonical path for comparison because Mac was failing
2) Update current classpath snapshot only with the missing entries from the previous one
3) Clean-up code and style

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

@@ -45,10 +45,10 @@ class KaptIncrementalWithAggregatingApt : KaptIncrementalIT() {
 
             assertEquals(
                 setOf(
-                    fileInWorkingDir("build/tmp/kapt3/stubs/main/bar/UseBKt.java").absolutePath,
+                    fileInWorkingDir("build/tmp/kapt3/stubs/main/bar/UseBKt.java").canonicalPath,
-                    fileInWorkingDir("build/tmp/kapt3/stubs/main/bar/B.java").absolutePath,
+                    fileInWorkingDir("build/tmp/kapt3/stubs/main/bar/B.java").canonicalPath,
-                    fileInWorkingDir("build/tmp/kapt3/stubs/main/baz/UtilKt.java").absolutePath,
+                    fileInWorkingDir("build/tmp/kapt3/stubs/main/baz/UtilKt.java").canonicalPath,
-                    fileInWorkingDir("build/tmp/kapt3/stubs/main/foo/A.java").absolutePath
+                    fileInWorkingDir("build/tmp/kapt3/stubs/main/foo/A.java").canonicalPath
                 ), getProcessedSources(output)
             )
         }
@@ -61,11 +61,11 @@ class KaptIncrementalWithAggregatingApt : KaptIncrementalIT() {
             assertSuccessful()
             assertEquals(
                 setOf(
-                    project.projectFile("JavaClass.java").absolutePath,
+                    project.projectFile("JavaClass.java").canonicalPath,
-                    fileInWorkingDir("build/tmp/kapt3/stubs/main/bar/UseBKt.java").absolutePath,
+                    fileInWorkingDir("build/tmp/kapt3/stubs/main/bar/UseBKt.java").canonicalPath,
-                    fileInWorkingDir("build/tmp/kapt3/stubs/main/bar/B.java").absolutePath,
+                    fileInWorkingDir("build/tmp/kapt3/stubs/main/bar/B.java").canonicalPath,
-                    fileInWorkingDir("build/tmp/kapt3/stubs/main/baz/UtilKt.java").absolutePath,
+                    fileInWorkingDir("build/tmp/kapt3/stubs/main/baz/UtilKt.java").canonicalPath,
-                    fileInWorkingDir("build/tmp/kapt3/stubs/main/foo/A.java").absolutePath
+                    fileInWorkingDir("build/tmp/kapt3/stubs/main/foo/A.java").canonicalPath
                 ),
                 getProcessedSources(output)
             )
@@ -115,13 +115,13 @@ class KaptIncrementalWithAggregatingApt : KaptIncrementalIT() {
 
             assertEquals(
                 setOf(
-                    fileInWorkingDir("app/build/tmp/kapt3/stubs/main/foo/AA.java").absolutePath,
+                    fileInWorkingDir("app/build/tmp/kapt3/stubs/main/foo/AA.java").canonicalPath,
-                    fileInWorkingDir("app/build/tmp/kapt3/stubs/main/foo/AAA.java").absolutePath,
+                    fileInWorkingDir("app/build/tmp/kapt3/stubs/main/foo/AAA.java").canonicalPath,
-                    fileInWorkingDir("app/build/tmp/kapt3/stubs/main/foo/BB.java").absolutePath,
+                    fileInWorkingDir("app/build/tmp/kapt3/stubs/main/foo/BB.java").canonicalPath,
-                    fileInWorkingDir("app/build/tmp/kapt3/stubs/main/foo/FooUseAKt.java").absolutePath,
+                    fileInWorkingDir("app/build/tmp/kapt3/stubs/main/foo/FooUseAKt.java").canonicalPath,
-                    fileInWorkingDir("app/build/tmp/kapt3/stubs/main/foo/FooUseBKt.java").absolutePath,
+                    fileInWorkingDir("app/build/tmp/kapt3/stubs/main/foo/FooUseBKt.java").canonicalPath,
-                    fileInWorkingDir("app/build/tmp/kapt3/stubs/main/foo/FooUseAAKt.java").absolutePath,
+                    fileInWorkingDir("app/build/tmp/kapt3/stubs/main/foo/FooUseAAKt.java").canonicalPath,
-                    fileInWorkingDir("app/build/tmp/kapt3/stubs/main/foo/FooUseBBKt.java").absolutePath
+                    fileInWorkingDir("app/build/tmp/kapt3/stubs/main/foo/FooUseBBKt.java").canonicalPath
 
                 ), getProcessedSources(output)
             )

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

@@ -47,7 +47,7 @@ class KaptIncrementalWithIsolatingApt : KaptIncrementalIT() {
         project.build("build") {
             assertSuccessful()
 
-            assertEquals(setOf(fileInWorkingDir("build/tmp/kapt3/stubs/main/bar/UseBKt.java").absolutePath), getProcessedSources(output))
+            assertEquals(setOf(fileInWorkingDir("build/tmp/kapt3/stubs/main/bar/UseBKt.java").canonicalPath), getProcessedSources(output))
         }
 
         project.projectFile("B.kt").modify { current ->
@@ -58,8 +58,8 @@ class KaptIncrementalWithIsolatingApt : KaptIncrementalIT() {
             assertSuccessful()
             assertEquals(
                 setOf(
-                    fileInWorkingDir("build/tmp/kapt3/stubs/main/bar/B.java").absolutePath,
+                    fileInWorkingDir("build/tmp/kapt3/stubs/main/bar/B.java").canonicalPath,
-                    fileInWorkingDir("build/tmp/kapt3/stubs/main/bar/UseBKt.java").absolutePath
+                    fileInWorkingDir("build/tmp/kapt3/stubs/main/bar/UseBKt.java").canonicalPath
                 ),
                 getProcessedSources(output)
             )

libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptTask.kt

@@ -167,48 +167,43 @@ abstract class KaptTask : ConventionTask(), TaskWithLocalState {
     protected fun getCompiledSources() = listOfNotNull(kotlinCompileTask.destinationDir, kotlinCompileTask.javaOutputDir)
 
     protected fun getIncrementalChanges(inputs: IncrementalTaskInputs): KaptIncrementalChanges {
-        val changedFiles = getChangedFiles(inputs)
+        if (!isIncremental) {
-
-        return if (isIncremental) {
-            val classpathChanges = if (changedFiles.isEmpty()) {
-                classpath.files
-            } else {
-                classpath.files.let { cp ->
-                    changedFiles.filter { cp.contains(it) }
-                }
-            }
-            val classpathStatus = findClasspathChanges(classpathChanges)
-            when (classpathStatus) {
-                is KaptClasspathChanges.Unknown -> KaptIncrementalChanges.Unknown
-                is KaptClasspathChanges.Known -> KaptIncrementalChanges.Known(
-                    changedFiles.filter { it.extension == "java" }.toSet(), classpathStatus.names
-                )
-            }
-        } else {
-            KaptIncrementalChanges.Unknown
-        }
-    }
-
-    private fun getChangedFiles(inputs: IncrementalTaskInputs): List<File> {
-        return if (!isIncremental || !inputs.isIncremental) {
             clearLocalState()
-            emptyList()
+            return KaptIncrementalChanges.Unknown
-        } else {
+        }
-            with(mutableSetOf<File>()) {
+        if (!inputs.isIncremental) {
-                inputs.outOfDate { this.add(it.file) }
+            clearLocalState()
-                inputs.removed { this.add(it.file) }
+            findClasspathChanges(classpath.files)
-                return@with this.toList()
+            return KaptIncrementalChanges.Unknown
-            }
+        }
+
+        val changedFiles = with(mutableSetOf<File>()) {
+            inputs.outOfDate { this.add(it.file) }
+            inputs.removed { this.add(it.file) }
+            return@with this.toList()
+        }
+
+        val classpathChanges = classpath.files.let { cp -> changedFiles.filter { cp.contains(it) } }
+
+        val classpathStatus = findClasspathChanges(classpathChanges)
+        return when (classpathStatus) {
+            is KaptClasspathChanges.Unknown -> KaptIncrementalChanges.Unknown
+            is KaptClasspathChanges.Known -> KaptIncrementalChanges.Known(
+                changedFiles.filter { it.extension == "java" }.toSet(), classpathStatus.names
+            )
         }
     }
 
     private fun findClasspathChanges(changedClasspath: Iterable<File>): KaptClasspathChanges {
-        incAptCache!!.mkdirs()
+        val incAptCacheDir = incAptCache!!
+        incAptCacheDir.mkdirs()
 
         val startTime = System.currentTimeMillis()
 
-        val previousSnapshot = ClasspathSnapshot.ClasspathSnapshotFactory.loadFrom(incAptCache!!)
+        val previousSnapshot = ClasspathSnapshot.ClasspathSnapshotFactory.loadFrom(incAptCacheDir)
-        val currentSnapshot = ClasspathSnapshot.ClasspathSnapshotFactory.createCurrent(incAptCache!!, classpath.files.toList(), classpathStructure!!.files)
+        val currentSnapshot = ClasspathSnapshot.ClasspathSnapshotFactory.createCurrent(
+            incAptCacheDir, classpath.files.toList(), classpathStructure!!.files
+        )
 
         val classpathChanges = currentSnapshot.diff(previousSnapshot, changedClasspath.toSet())
         currentSnapshot.writeToCache()

libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptWithKotlincTask.kt

@@ -75,15 +75,10 @@ open class KaptWithKotlincTask : KaptTask(), CompilerArgumentAwareWithInput<K2JV
         checkAnnotationProcessorClasspath()
 
         val incrementalChanges = getIncrementalChanges(inputs)
-        when (incrementalChanges) {
+        if (incrementalChanges is KaptIncrementalChanges.Known) {
-            is KaptIncrementalChanges.Known -> {
+            changedFiles = incrementalChanges.changedSources.toList()
-                changedFiles = incrementalChanges.changedSources.toList()
+            classpathChanges = incrementalChanges.changedClasspathJvmNames.toList()
-                classpathChanges = incrementalChanges.changedClasspathJvmNames.toList()
+            processIncrementally = true
-                processIncrementally = true
-            }
-            else -> {
-                // do nothing
-            }
         }
 
         val args = prepareCompilerArguments()

libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/KaptWithoutKotlincTask.kt

@@ -66,6 +66,7 @@ open class KaptWithoutKotlincTask @Inject constructor(private val workerExecutor
             if (isVerbose) add("VERBOSE")
             if (mapDiagnosticLocations) add("MAP_DIAGNOSTIC_LOCATIONS")
             if (includeCompileClasspath) add("INCLUDE_COMPILE_CLASSPATH")
+            if (incrementalChanges is KaptIncrementalChanges.Known) add("INCREMENTAL_APT")
         }
 
         val optionsForWorker = KaptOptionsForWorker(
@@ -77,7 +78,6 @@ open class KaptWithoutKotlincTask @Inject constructor(private val workerExecutor
             getCompiledSources(),
             incAptCache,
             classpathChanges.toList(),
-            incrementalChanges is KaptIncrementalChanges.Known,
 
             destinationDir,
             classesDir,
@@ -172,7 +172,6 @@ private class KaptExecution @Inject constructor(
             compiledSources,
             incAptCache,
             classpathChanges,
-            processIncrementally,
 
             sourcesOutputDir,
             classesOutputDir,
@@ -209,7 +208,6 @@ private data class KaptOptionsForWorker(
     val compiledSources: List<File>,
     val incAptCache: File?,
     val classpathChanges: List<String>,
-    val processIncrementally: Boolean,
 
     val sourcesOutputDir: File,
     val classesOutputDir: File,

libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/incremental/ClasspathSnapshot.kt

@@ -7,7 +7,6 @@ package org.jetbrains.kotlin.gradle.internal.kapt.incremental
 
 import java.io.*
 import java.util.*
-
 open class ClasspathSnapshot protected constructor(
     private val cacheDir: File,
     private val classpath: Iterable<File>,
@@ -20,7 +19,7 @@ open class ClasspathSnapshot protected constructor(
             val computedData = dataForFiles(missingFiles)
             computedClasspathData.putAll(computedData)
         }
-        computedClasspathData.filter { files.contains(it.key) }
+        computedClasspathData
     }
     private val computedClasspathData: MutableMap<File, ClasspathEntryData> = mutableMapOf()
 
@@ -95,10 +94,11 @@ open class ClasspathSnapshot protected constructor(
             }
         }
 
-        // We do not compute structural data for unchanged files of the current snapshot for performance reasons. That is why we
+        // We do not compute structural data for unchanged files of the current snapshot for performance reasons.
-        // update the previous snapshot as that one contains all entries.
+        // That is why we reuse the previous snapshot as that one contains all unchanged entries.
-        computedClasspathData.putAll(previousData)
+        previousData.filter { !computedClasspathData.containsKey(it.key) }.forEach {
-        computedClasspathData.putAll(currentData)
+            computedClasspathData[it.key] = it.value
+        }
 
         val allImpactedClasses = findAllImpacted(changedClasses)
 

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

@@ -59,7 +59,7 @@ open class KaptContext(val options: KaptOptions, val withJdk: Boolean, val logge
         cacheManager = options.incrementalCache?.let {
             JavaClassCacheManager(it)
         }
-        if (options.processIncrementally) {
+        if (options.flags[KaptFlag.INCREMENTAL_APT]) {
             sourcesToReprocess =
                 cacheManager?.invalidateAndGetDirtyFiles(
                     options.changedFiles, options.classpathChanges

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

@@ -18,7 +18,6 @@ class KaptOptions(
     val compiledSources: List<File>,
     val incrementalCache: File?,
     val classpathChanges: List<String>,
-    val processIncrementally: Boolean,
 
     val sourcesOutputDir: File,
     val classesOutputDir: File,
@@ -52,7 +51,6 @@ class KaptOptions(
         var classesOutputDir: File? = null
         var stubsOutputDir: File? = null
         var incrementalDataOutputDir: File? = null
-        var processIncrementally: Boolean = false
 
         val processingClasspath: MutableList<File> = mutableListOf()
         val processors: MutableList<String> = mutableListOf()
@@ -75,7 +73,7 @@ class KaptOptions(
 
             return KaptOptions(
                 projectBaseDir, compileClasspath, javaSourceRoots,
-                changedFiles, compiledSources, incrementalCache, classpathChanges, processIncrementally,
+                changedFiles, compiledSources, incrementalCache, classpathChanges,
                 sourcesOutputDir, classesOutputDir, stubsOutputDir, incrementalDataOutputDir,
                 processingClasspath, processors, processingOptions, javacOptions, KaptFlags.fromSet(flags),
                 mode, detectMemoryLeaks
@@ -106,7 +104,9 @@ enum class KaptFlag(val description: String) {
     CORRECT_ERROR_TYPES("Correct error types"),
     MAP_DIAGNOSTIC_LOCATIONS("Map diagnostic locations"),
     STRICT("Strict mode"),
-    INCLUDE_COMPILE_CLASSPATH("Detect annotation processors in compile classpath");
+    INCLUDE_COMPILE_CLASSPATH("Detect annotation processors in compile classpath"),
+    INCREMENTAL_APT("Incremental annotation processing (apt mode)"),
+    ;
 }
 
 interface KaptSelector {
@@ -172,5 +172,4 @@ fun KaptOptions.logString(additionalInfo: String = "") = buildString {
     appendln("[incremental apt] Compiled sources directories: ${compiledSources.joinToString()}")
     appendln("[incremental apt] Cache directory for incremental compilation: $incrementalCache")
     appendln("[incremental apt] Changed classpath names: ${classpathChanges.joinToString()}")
-    appendln("[incremental apt] If processing incrementally: $processIncrementally")
 }

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

@@ -211,9 +211,7 @@ class SourceFileStructure(
 
     fun addMentionedConstant(containingClass: String, name: String) {
         if (!declaredTypes.contains(containingClass)) {
-            val names = mentionedConstants[containingClass] ?: HashSet()
+            mentionedConstants.getOrPut(containingClass) { HashSet() }.add(name)
-            names.add(name)
-            mentionedConstants[containingClass] = names
         }
     }
 }

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

@@ -5,6 +5,7 @@
 
 package org.jetbrains.kotlin.kapt.base.test.org.jetbrains.kotlin.kapt3.base.incremental
 
+import org.jetbrains.kotlin.base.kapt3.KaptFlag
 import org.jetbrains.kotlin.base.kapt3.KaptOptions
 import org.jetbrains.kotlin.base.kapt3.collectJavaSourceFiles
 import org.jetbrains.kotlin.kapt3.base.KaptContext
@@ -70,7 +71,7 @@ class IncrementalKaptTest {
             incrementalCache = incrementalCacheDir
             compiledSources.add(classesOutput)
             changedFiles.add(sourcesDir.resolve("User.java"))
-            processIncrementally = true
+            flags.add(KaptFlag.INCREMENTAL_APT)
         }.build()
 
         KaptContext(optionsForSecondRun, true, logger).use {
@@ -144,7 +145,7 @@ class IncrementalKaptTest {
             incrementalCache = incrementalCacheDir
             compiledSources.add(classesOutput)
             changedFiles.add(sourcesDir.resolve("User.java"))
-            processIncrementally = true
+            flags.add(KaptFlag.INCREMENTAL_APT)
         }.build()
 
         KaptContext(optionsForSecondRun, true, logger).use {

plugins/kapt3/kapt3-compiler/src/org/jetbrains/kotlin/kapt3/Kapt3Plugin.kt

@@ -101,7 +101,7 @@ class Kapt3CommandLineProcessor : CommandLineProcessor {
             COMPILED_SOURCES_DIR -> compiledSources.addAll(value.split(File.pathSeparator).map { File(it) })
             INCREMENTAL_CACHE -> incrementalCache = File(value)
             CLASSPATH_CHANGES -> classpathChanges.addAll(value.split(File.pathSeparator).map { it })
-            PROCESS_INCREMENTALLY -> processIncrementally = value.toBoolean()
+            PROCESS_INCREMENTALLY -> setFlag(KaptFlag.INCREMENTAL_APT, value)
 
             ANNOTATION_PROCESSOR_CLASSPATH_OPTION -> processingClasspath += File(value)
             ANNOTATION_PROCESSORS_OPTION -> processors.addAll(value.split(',').map { it.trim() }.filter { it.isNotEmpty() })