Refine dirty files computation in case of Java source changes

#KT-17621 In Progress

compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/ChangedJavaFilesProcessor.kt

@@ -1,81 +0,0 @@
-/*
- * Copyright 2010-2016 JetBrains s.r.o.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.jetbrains.kotlin.incremental
-
-import org.jetbrains.kotlin.cli.jvm.compiler.EnvironmentConfigFiles
-import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment
-import com.intellij.lang.java.JavaLanguage
-import com.intellij.openapi.util.Disposer
-import com.intellij.psi.PsiClass
-import com.intellij.psi.PsiFile
-import com.intellij.psi.PsiFileFactory
-import com.intellij.psi.PsiJavaFile
-import org.jetbrains.kotlin.config.CompilerConfiguration
-import java.io.File
-import java.util.*
-
-internal class ChangedJavaFilesProcessor(private val reporter: ICReporter) {
-    private val allSymbols = HashSet<LookupSymbol>()
-    private val javaLang = JavaLanguage.INSTANCE
-    private val psiFileFactory: PsiFileFactory by lazy {
-        val rootDisposable = Disposer.newDisposable()
-        val configuration = CompilerConfiguration()
-        val environment = KotlinCoreEnvironment.createForProduction(rootDisposable, configuration, EnvironmentConfigFiles.JVM_CONFIG_FILES)
-        val project = environment.project
-        PsiFileFactory.getInstance(project)
-    }
-
-    val allChangedSymbols: Collection<LookupSymbol>
-            get() = allSymbols
-
-    fun process(filesDiff: ChangedFiles.Known): ChangesEither {
-        val modifiedJava = filesDiff.modified.filter(File::isJavaFile)
-        val removedJava = filesDiff.removed.filter(File::isJavaFile)
-
-        if (removedJava.any()) {
-            reporter.report { "Some java files are removed: [${removedJava.joinToString()}]" }
-            return ChangesEither.Unknown()
-        }
-
-        val symbols = HashSet<LookupSymbol>()
-        for (javaFile in modifiedJava) {
-            assert(javaFile.extension.equals("java", ignoreCase = true))
-
-            val psiFile = javaFile.psiFile()
-            if (psiFile !is PsiJavaFile) {
-                reporter.report { "Expected PsiJavaFile, got ${psiFile?.javaClass}" }
-                return ChangesEither.Unknown()
-            }
-
-            psiFile.classes.forEach { it.addLookupSymbols(symbols) }
-        }
-        allSymbols.addAll(symbols)
-        return ChangesEither.Known(lookupSymbols = symbols)
-    }
-
-    private fun PsiClass.addLookupSymbols(symbols: MutableSet<LookupSymbol>) {
-        val fqn = qualifiedName.orEmpty()
-
-        symbols.add(LookupSymbol(name.orEmpty(), if (fqn == name) "" else fqn.removeSuffix("." + name!!)))
-        methods.forEach { symbols.add(LookupSymbol(it.name, fqn)) }
-        fields.forEach { symbols.add(LookupSymbol(it.name.orEmpty(), fqn)) }
-        innerClasses.forEach { it.addLookupSymbols(symbols) }
-    }
-
-    private fun File.psiFile(): PsiFile? =
-            psiFileFactory.createFileFromText(nameWithoutExtension, javaLang, readText())
-}

compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalCompilerRunner.kt

@@ -31,8 +31,7 @@ import org.jetbrains.kotlin.incremental.multiproject.ChangesRegistry
 import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.progress.CompilationCanceledStatus
 import java.io.File
-import java.util.ArrayList
-import java.util.HashSet
+import java.util.*
 
 abstract class IncrementalCompilerRunner<
     Args : CommonCompilerArguments,
@@ -143,8 +142,6 @@ abstract class IncrementalCompilerRunner<
     protected open fun postCompilationHook(exitCode: ExitCode) {}
     protected open fun additionalDirtyFiles(caches: CacheManager, generatedFiles: List<GeneratedFile>): Iterable<File> =
             emptyList()
-    protected open fun additionalDirtyLookupSymbols(): Iterable<LookupSymbol> =
-            emptyList()
 
     protected open fun makeServices(
             args: Args,
@@ -187,7 +184,7 @@ abstract class IncrementalCompilerRunner<
         var exitCode = ExitCode.OK
         val allGeneratedFiles = hashSetOf<GeneratedFile>()
 
-        while (dirtySources.any()) {
+        while (dirtySources.any() || runWithNoDirtyKotlinSources(caches)) {
             caches.platformCache.markDirty(dirtySources)
             caches.inputsCache.removeOutputForSourceFiles(dirtySources)
 
@@ -248,9 +245,6 @@ abstract class IncrementalCompilerRunner<
         if (exitCode == ExitCode.OK) {
             BuildInfo.write(currentBuildInfo, lastBuildInfoFile)
         }
-        if (exitCode == ExitCode.OK && compilationMode is CompilationMode.Incremental) {
-            buildDirtyLookupSymbols.addAll(additionalDirtyLookupSymbols())
-        }
 
         val dirtyData = DirtyData(buildDirtyLookupSymbols, buildDirtyFqNames)
         processChangesAfterBuild(compilationMode, currentBuildInfo, dirtyData)
@@ -262,6 +256,8 @@ abstract class IncrementalCompilerRunner<
         return exitCode
     }
 
+    open fun runWithNoDirtyKotlinSources(caches: CacheManager): Boolean = false
+
     protected open fun processChangesAfterBuild(compilationMode: CompilationMode, currentBuildInfo: BuildInfo, dirtyData: DirtyData) {
         if (changesRegistry == null) return
 
@@ -283,4 +279,4 @@ abstract class IncrementalCompilerRunner<
         override fun checkCanceled() {
         }
     }
-}
+}

compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalJvmCompilerRunner.kt

@@ -31,14 +31,12 @@ import org.jetbrains.kotlin.config.Services
 import org.jetbrains.kotlin.incremental.components.LookupTracker
 import org.jetbrains.kotlin.incremental.multiproject.ArtifactChangesProvider
 import org.jetbrains.kotlin.incremental.multiproject.ChangesRegistry
+import org.jetbrains.kotlin.load.java.JavaClassesTracker
 import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader
 import org.jetbrains.kotlin.load.kotlin.incremental.components.IncrementalCompilationComponents
 import org.jetbrains.kotlin.modules.TargetId
 import org.jetbrains.kotlin.name.FqName
-import org.jetbrains.kotlin.resolve.jvm.JvmClassName
 import java.io.File
-import java.util.*
-import kotlin.collections.HashSet
 
 fun makeIncrementally(
         cachesDir: File,
@@ -110,8 +108,6 @@ class IncrementalJvmCompilerRunner(
     override fun destinationDir(args: K2JVMCompilerArguments): File =
             args.destinationAsFile
 
-    private var javaFilesProcessor = ChangedJavaFilesProcessor(reporter)
-
     override fun calculateSourcesToCompile(caches: IncrementalJvmCachesManager, changedFiles: ChangedFiles.Known, args: K2JVMCompilerArguments): CompilationMode {
         val dirtyFiles = getDirtyFiles(changedFiles)
 
@@ -170,23 +166,22 @@ class IncrementalJvmCompilerRunner(
             return CompilationMode.Rebuild { "could not get changes from modified classpath entries: ${reporter.pathsAsString(modifiedClasspathEntries)}" }
         }
 
-        val javaFilesChanges = javaFilesProcessor.process(changedFiles)
-        val affectedJavaSymbols = when (javaFilesChanges) {
-            is ChangesEither.Known -> javaFilesChanges.lookupSymbols
-            is ChangesEither.Unknown -> return CompilationMode.Rebuild { "Could not get changes for java files" }
-        }
+        processChangedJava(changedFiles, caches)
 
         if ((changedFiles.modified + changedFiles.removed).any { it.extension.toLowerCase() == "xml" }) {
             return CompilationMode.Rebuild { "XML resource files were changed" }
         }
 
-        markDirtyBy(affectedJavaSymbols)
         markDirtyBy(classpathChanges.lookupSymbols)
         markDirtyBy(classpathChanges.fqNames)
 
         return CompilationMode.Incremental(dirtyFiles)
     }
 
+    private fun processChangedJava(changedFiles: ChangedFiles.Known, caches: IncrementalJvmCachesManager) {
+        caches.platformCache.markDirty((changedFiles.modified + changedFiles.removed).filter(File::isJavaFile))
+    }
+
     private fun getClasspathChanges(
             modifiedClasspath: List<File>,
             lastBuildInfo: BuildInfo?
@@ -249,9 +244,15 @@ class IncrementalJvmCompilerRunner(
             generatedFiles: List<GeneratedFile>,
             changesCollector: ChangesCollector
     ) {
-        updateIncrementalCache(generatedFiles, caches.platformCache, changesCollector)
+        updateIncrementalCache(
+                generatedFiles, caches.platformCache, changesCollector,
+                services[JavaClassesTracker::class.java] as? JavaClassesTrackerImpl
+        )
     }
 
+    override fun runWithNoDirtyKotlinSources(caches: IncrementalJvmCachesManager): Boolean =
+            caches.platformCache.getObsoleteJavaClasses().isNotEmpty()
+
     override fun additionalDirtyFiles(
             caches: IncrementalJvmCachesManager,
             generatedFiles: List<GeneratedFile>
@@ -292,9 +293,6 @@ class IncrementalJvmCompilerRunner(
         return result
     }
 
-    override fun additionalDirtyLookupSymbols(): Iterable<LookupSymbol> =
-            javaFilesProcessor.allChangedSymbols
-
     override fun processChangesAfterBuild(compilationMode: CompilationMode, currentBuildInfo: BuildInfo, dirtyData: DirtyData) {
         super.processChangesAfterBuild(compilationMode, currentBuildInfo, dirtyData)
 
@@ -323,6 +321,8 @@ class IncrementalJvmCompilerRunner(
             val targetToCache = mapOf(targetId to caches.platformCache)
             val incrementalComponents = IncrementalCompilationComponentsImpl(targetToCache)
             register(IncrementalCompilationComponents::class.java, incrementalComponents)
+            val changesTracker = JavaClassesTrackerImpl(caches.platformCache)
+            register(JavaClassesTracker::class.java, changesTracker)
         }
 
     override fun runCompiler(

compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/IncrementalJvmCompilerRunnerTestGenerated.java

@@ -1108,18 +1108,42 @@ public class IncrementalJvmCompilerRunnerTestGenerated extends AbstractIncrement
                 KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("jps-plugin/testData/incremental/withJava/javaUsedInKotlin"), Pattern.compile("^([^\\.]+)$"), TargetBackend.ANY, true);
             }
 
+            @TestMetadata("changeFieldType")
+            public void testChangeFieldType() throws Exception {
+                String fileName = KotlinTestUtils.navigationMetadata("jps-plugin/testData/incremental/withJava/javaUsedInKotlin/changeFieldType/");
+                doTest(fileName);
+            }
+
             @TestMetadata("changeNotUsedSignature")
             public void testChangeNotUsedSignature() throws Exception {
                 String fileName = KotlinTestUtils.navigationMetadata("jps-plugin/testData/incremental/withJava/javaUsedInKotlin/changeNotUsedSignature/");
                 doTest(fileName);
             }
 
+            @TestMetadata("changePropertyOverrideType")
+            public void testChangePropertyOverrideType() throws Exception {
+                String fileName = KotlinTestUtils.navigationMetadata("jps-plugin/testData/incremental/withJava/javaUsedInKotlin/changePropertyOverrideType/");
+                doTest(fileName);
+            }
+
             @TestMetadata("changeSignature")
             public void testChangeSignature() throws Exception {
                 String fileName = KotlinTestUtils.navigationMetadata("jps-plugin/testData/incremental/withJava/javaUsedInKotlin/changeSignature/");
                 doTest(fileName);
             }
 
+            @TestMetadata("changeSignaturePackagePrivate")
+            public void testChangeSignaturePackagePrivate() throws Exception {
+                String fileName = KotlinTestUtils.navigationMetadata("jps-plugin/testData/incremental/withJava/javaUsedInKotlin/changeSignaturePackagePrivate/");
+                doTest(fileName);
+            }
+
+            @TestMetadata("changeSignatureStatic")
+            public void testChangeSignatureStatic() throws Exception {
+                String fileName = KotlinTestUtils.navigationMetadata("jps-plugin/testData/incremental/withJava/javaUsedInKotlin/changeSignatureStatic/");
+                doTest(fileName);
+            }
+
             @TestMetadata("constantChanged")
             public void testConstantChanged() throws Exception {
                 String fileName = KotlinTestUtils.navigationMetadata("jps-plugin/testData/incremental/withJava/javaUsedInKotlin/constantChanged/");
@@ -1180,6 +1204,12 @@ public class IncrementalJvmCompilerRunnerTestGenerated extends AbstractIncrement
                 doTest(fileName);
             }
 
+            @TestMetadata("mixedInheritance")
+            public void testMixedInheritance() throws Exception {
+                String fileName = KotlinTestUtils.navigationMetadata("jps-plugin/testData/incremental/withJava/javaUsedInKotlin/mixedInheritance/");
+                doTest(fileName);
+            }
+
             @TestMetadata("notChangeSignature")
             public void testNotChangeSignature() throws Exception {
                 String fileName = KotlinTestUtils.navigationMetadata("jps-plugin/testData/incremental/withJava/javaUsedInKotlin/notChangeSignature/");
@@ -1472,4 +1502,31 @@ public class IncrementalJvmCompilerRunnerTestGenerated extends AbstractIncrement
             }
         }
     }
+
+    @TestMetadata("jps-plugin/testData/incremental/incrementalJvmCompilerOnly")
+    @TestDataPath("$PROJECT_ROOT")
+    @RunWith(JUnit3RunnerWithInners.class)
+    public static class IncrementalJvmCompilerOnly extends AbstractIncrementalJvmCompilerRunnerTest {
+        @TestMetadata("addAnnotationToJavaClass")
+        public void testAddAnnotationToJavaClass() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("jps-plugin/testData/incremental/incrementalJvmCompilerOnly/addAnnotationToJavaClass/");
+            doTest(fileName);
+        }
+
+        @TestMetadata("addNestedClass")
+        public void testAddNestedClass() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("jps-plugin/testData/incremental/incrementalJvmCompilerOnly/addNestedClass/");
+            doTest(fileName);
+        }
+
+        public void testAllFilesPresentInIncrementalJvmCompilerOnly() throws Exception {
+            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("jps-plugin/testData/incremental/incrementalJvmCompilerOnly"), Pattern.compile("^([^\\.]+)$"), TargetBackend.ANY, true);
+        }
+
+        @TestMetadata("changeAnnotationInJavaClass")
+        public void testChangeAnnotationInJavaClass() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("jps-plugin/testData/incremental/incrementalJvmCompilerOnly/changeAnnotationInJavaClass/");
+            doTest(fileName);
+        }
+    }
 }