move class handling changed: classes that directly depend on removed sources are now compiled in the first compilation round (IDEA-165813)

java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound.log

@@ -0,0 +1,12 @@
+Cleaning output files:
+out/production/CompileDependenciesOnMovedClassesInFirstRound/qqq/C.class
+End of files
+Cleaning output files:
+out/production/CompileDependenciesOnMovedClassesInFirstRound/ppp/A.class
+out/production/CompileDependenciesOnMovedClassesInFirstRound/ppp/B.class
+End of files
+Compiling files:
+src/ppp/A.java
+src/ppp/B.java
+src/ppp/C.java
+End of files

java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/ppp/A.java

@@ -0,0 +1,14 @@
+package ppp;
+import qqq.*;
+
+public class A {
+  private C delegate;
+
+  public A(C delegate) {
+    this.delegate = delegate;
+  }
+
+  void foo() {
+    B.util(delegate);
+  }
+}

java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/ppp/A.java.new

@@ -0,0 +1,15 @@
+package ppp;
+import qqq.*;
+
+public class A {
+
+  private C delegate;
+
+  public A(C delegate) {
+    this.delegate = delegate;
+  }
+
+  void foo() {
+    B.util(delegate);
+  }
+}

java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/ppp/B.java

@@ -0,0 +1,8 @@
+package ppp;
+import qqq.*;
+
+public class B {
+  public static void util(C iface) {
+    iface.execute();
+  }
+}

java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/ppp/C.java.new

@@ -0,0 +1,5 @@
+package ppp;
+
+public interface C {
+  void execute();
+}

java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/qqq/C.java

@@ -0,0 +1,5 @@
+package qqq;
+
+public interface C {
+  void execute();
+}

java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/qqq/Placeholder.java

@@ -0,0 +1,4 @@
+package qqq;
+
+public class Placeholder{
+}

java/java-tests/testData/compileServer/incremental/common/moveClassToAnotherRoot.log

@@ -1,12 +1,10 @@
 Cleaning output files:
 out/production/moveClassToAnotherRoot/pkg/A.class
 End of files
-Compiling files:
-src2/pkg/A.java
-End of files
 Cleaning output files:
 out/production/moveClassToAnotherRoot/pkg/Client.class
 End of files
 Compiling files:
 src/pkg/Client.java
-End of files
+src2/pkg/A.java
+End of files

java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies.log

@@ -4,7 +4,11 @@ End of files
 Cleaning output files:
 out/production/RecompileTwinDependencies/package1/C.class
 End of files
+Cleaning output files:
+out/production/RecompileTwinDependencies/com/B.class
+End of files
 Compiling files:
+src/com/B.java
 src/package2/A.java
 src/package2/C.java
-End of files
+End of files

java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package2/C.java.new

@@ -5,7 +5,8 @@ import com.B;
 public class C {
     {
         new B().get(); // should be resolved to "com/B.get:()Lpackage2/A;"
-        // but package2/C.java is first compiled when B.class still contains "public package1.A get();"
-        // and compiler returns an error
+        // If B.class were not compiled in the very first compilation round, it would still contain "public package1.A get();" and compiler would return an error
+        // Because of special logic, all classes that depend on moved or deleted classes, are marked dirty before compilation starts,
+        // so they are compiled in the first round and no errors should occur.
     }
 }

jps/jps-builders/src/org/jetbrains/jps/builders/java/JavaBuilderUtil.java

@@ -127,7 +127,7 @@ public class JavaBuilderUtil {
     SUCCESSFULLY_COMPILED_FILES_KEY.set(context, null);
     FileFilter filter = createOrFilter(SKIP_MARKING_DIRTY_FILTERS_KEY.get(context));
     SKIP_MARKING_DIRTY_FILTERS_KEY.set(context, null);
-    return updateMappings(context, delta, dirtyFilesHolder, chunk, compiledFiles, successfullyCompiled, filter);
+    return updateMappings(context, delta, dirtyFilesHolder, chunk, compiledFiles, successfullyCompiled, CompilationRound.NEXT, filter);
   }
 
   /**
@@ -142,21 +142,31 @@ public class JavaBuilderUtil {
                                        ModuleChunk chunk,
                                        Collection<File> filesToCompile,
                                        Collection<File> successfullyCompiled) throws IOException {
-    return updateMappings(context, delta, dirtyFilesHolder, chunk, filesToCompile, successfullyCompiled, null);
+    return updateMappings(context, delta, dirtyFilesHolder, chunk, filesToCompile, successfullyCompiled, CompilationRound.NEXT, null);
+  }
+
+  public static void markDirtyDependenciesForInitialRound(CompileContext context, DirtyFilesHolder<JavaSourceRootDescriptor, ModuleBuildTarget> dfh, ModuleChunk chunk) throws IOException {
+    if (hasRemovedPaths(chunk, dfh)) {
+      final Mappings delta = context.getProjectDescriptor().dataManager.getMappings().createDelta();
+      final Set<File> empty = Collections.emptySet();
+      updateMappings(context, delta, dfh, chunk, empty, empty, CompilationRound.CURRENT, null);
+    }
   }
 
   /**
-   * @param filesToCompile       files compiled in this round
+   * @param filesToCompile   files compiled in this round
+   * @param markDirtyRound   compilation round at which dirty files should be visible to builders
    * @return true if additional compilation pass is required, false otherwise
    * @throws Exception
    */
   private static boolean updateMappings(CompileContext context,
-                                       final Mappings delta,
-                                       DirtyFilesHolder<JavaSourceRootDescriptor, ModuleBuildTarget> dirtyFilesHolder,
-                                       ModuleChunk chunk,
-                                       Collection<File> filesToCompile,
-                                       Collection<File> successfullyCompiled,
-                                       @Nullable FileFilter skipMarkingDirtyFilter) throws IOException {
+                                        final Mappings delta,
+                                        DirtyFilesHolder<JavaSourceRootDescriptor, ModuleBuildTarget> dirtyFilesHolder,
+                                        ModuleChunk chunk,
+                                        Collection<File> filesToCompile,
+                                        Collection<File> successfullyCompiled,
+                                        final CompilationRound markDirtyRound,
+                                        @Nullable FileFilter skipMarkingDirtyFilter) throws IOException {
     try {
       boolean performIntegrate = true;
       boolean additionalPassRequired = false;
@@ -232,7 +242,7 @@ public class JavaBuilderUtil {
               }
 
               for (File file : newlyAffectedFiles) {
-                FSOperations.markDirtyIfNotDeleted(context, CompilationRound.NEXT, file);
+                FSOperations.markDirtyIfNotDeleted(context, markDirtyRound, file);
               }
               additionalPassRequired = isCompileJavaIncrementally(context) && chunkContainsAffectedFiles(context, chunk, newlyAffectedFiles);
             }
@@ -255,7 +265,7 @@ public class JavaBuilderUtil {
             }
 
             FileFilter toBeMarkedFilter = skipMarkingDirtyFilter == null ? null : new NegationFileFilter(skipMarkingDirtyFilter);
-            FSOperations.markDirtyRecursively(context, CompilationRound.NEXT, chunk, toBeMarkedFilter);
+            FSOperations.markDirtyRecursively(context, markDirtyRound, chunk, toBeMarkedFilter);
           }
         }
         else {
@@ -384,6 +394,17 @@ public class JavaBuilderUtil {
     return removed;
   }
 
+  private static boolean hasRemovedPaths(ModuleChunk chunk, DirtyFilesHolder<JavaSourceRootDescriptor, ModuleBuildTarget> dirtyFilesHolder) {
+    if (dirtyFilesHolder.hasRemovedFiles()) {
+      for (ModuleBuildTarget target : chunk.getTargets()) {
+        if (!dirtyFilesHolder.getRemovedFiles(target).isEmpty()) {
+          return true;
+        }
+      }
+    }
+    return false;
+  }
+
   public static void cleanupChunkResources(CompileContext context) {
     ALL_AFFECTED_FILES_KEY.set(context, null);
     ALL_COMPILED_FILES_KEY.set(context, null);

jps/jps-builders/src/org/jetbrains/jps/incremental/java/JavaBuilder.java

@@ -37,6 +37,7 @@ import org.jetbrains.jps.api.GlobalOptions;
 import org.jetbrains.jps.builders.BuildRootIndex;
 import org.jetbrains.jps.builders.DirtyFilesHolder;
 import org.jetbrains.jps.builders.FileProcessor;
+import org.jetbrains.jps.builders.impl.DirtyFilesHolderBase;
 import org.jetbrains.jps.builders.java.JavaBuilderExtension;
 import org.jetbrains.jps.builders.java.JavaBuilderUtil;
 import org.jetbrains.jps.builders.java.JavaCompilingTool;
@@ -64,8 +65,7 @@ import org.jetbrains.jps.model.serialization.PathMacroUtil;
 import org.jetbrains.jps.service.JpsServiceManager;
 import org.jetbrains.jps.service.SharedThreadPool;
 
-import javax.tools.Diagnostic;
-import javax.tools.JavaFileObject;
+import javax.tools.*;
 import java.io.*;
 import java.net.ServerSocket;
 import java.util.*;
@@ -157,6 +157,23 @@ public class JavaBuilder extends ModuleLevelBuilder {
     COMPILER_USAGE_STATISTICS.set(context, new ConcurrentHashMap<String, Collection<String>>());
   }
 
+  @Override
+  public void chunkBuildStarted(final CompileContext context, final ModuleChunk chunk) {
+    // before the first compilation round starts: find and mark dirty all classes that depend on removed or moved classes so
+    // that all such files are compiled in the first round.
+    try {
+      JavaBuilderUtil.markDirtyDependenciesForInitialRound(context, new DirtyFilesHolderBase<JavaSourceRootDescriptor, ModuleBuildTarget>(context) {
+        @Override
+        public void processDirtyFiles(@NotNull FileProcessor<JavaSourceRootDescriptor, ModuleBuildTarget> processor) throws IOException {
+          FSOperations.processFilesToRecompile(context, chunk, processor);
+        }
+      }, chunk);
+    }
+    catch (IOException e) {
+      throw new RuntimeException(e);
+    }
+  }
+
   public void buildFinished(CompileContext context) {
     final ConcurrentMap<String, Collection<String>> stats = COMPILER_USAGE_STATISTICS.get(context);
     if (stats.size() == 1) {

jps/jps-builders/testData/incremental/multipleSources/deleteFile.log

@@ -3,7 +3,6 @@ out/production/m/PackageFacade.class
 out/production/m/b.class
 End of files
 Cleaning output files:
-out/production/m/PackageFacade.class
 out/production/m/a.class
 End of files
 Compiling files:

jps/jps-builders/testSrc/org/jetbrains/ether/CommonTest.java

@@ -156,6 +156,10 @@ public class CommonTest extends IncrementalTestCase {
     doTestBuild(1).assertSuccessful();
   }
 
+  public void testCompileDependenciesOnMovedClassesInFirstRound() throws Exception {
+    doTest().assertSuccessful();
+  }
+
   public void testIntegrateOnSuperclassRemovedAndRestored() throws Exception {
     setupInitialProject();
 

jps/jps-builders/testSrc/org/jetbrains/ether/MarkDirtyTest.java

@@ -54,7 +54,7 @@ public class MarkDirtyTest extends IncrementalTestCase {
   }
   
   public void testRecompileTwinDependencies() {
-    doTest().assertFailed();
+    doTest().assertSuccessful();
   }
 
   public void testDoNotMarkDirtyCompiledChunks() {