Merge pull request #2490 from tgodzik/compile-existing

bugfix: Compile only existing classes

backend/src/main/scala/sbt/internal/inc/bloop/internal/BloopIncremental.scala

@@ -115,6 +115,7 @@ object BloopIncremental {
       val doCompile = (srcs: Set[VirtualFile], changes: DependencyChanges) => {
         for {
           callback <- Task.now(callbackBuilder())
+          _ = incremental.log.debug(s"Compiling $srcs")
           _ <- compile(srcs, changes, callback, manager)
         } yield callback.get
       }

backend/src/main/scala/sbt/internal/inc/bloop/internal/BloopNameHashing.scala

@@ -90,59 +90,66 @@ private final class BloopNameHashing(
           case ref: VirtualFileRef => PlainVirtualFileConverter.converter.toVirtualFile(ref)
         }
 
-      recompileClasses(invalidatedSources, binaryChanges, previous, compileTask, manager).flatMap {
-        current =>
-          // Return immediate analysis as all sources have been recompiled
-          if (invalidatedSources == allSources) Task.now(current)
-          else {
-            val recompiledClasses: Set[String] = {
-              // Represents classes detected as changed externally and internally (by a previous cycle)
-              classesToRecompile ++
-                // Maps the changed sources by the user to class names we can count as invalidated
-                initialChangedSources.flatMap(previous.relations.classNames) ++
-                initialChangedSources.flatMap(current.relations.classNames)
-            }
+      recompileClasses(
+        invalidatedSources.filter(allSources),
+        invalidatedSources,
+        binaryChanges,
+        previous,
+        compileTask,
+        manager
+      ).flatMap { current =>
+        // Return immediate analysis as all sources have been recompiled
+        if (invalidatedSources == allSources) Task.now(current)
+        else {
+          val recompiledClasses: Set[String] = {
+            // Represents classes detected as changed externally and internally (by a previous cycle)
+            classesToRecompile ++
+              // Maps the changed sources by the user to class names we can count as invalidated
+              initialChangedSources.flatMap(previous.relations.classNames) ++
+              initialChangedSources.flatMap(current.relations.classNames)
+          }
 
-            val newApiChanges =
-              detectAPIChanges(
-                recompiledClasses,
-                previous.apis.internalAPI,
-                current.apis.internalAPI
-              )
-            debug("\nChanges:\n" + newApiChanges)
-            val nextInvalidations = invalidateAfterInternalCompilation(
-              current,
-              newApiChanges,
+          val newApiChanges =
+            detectAPIChanges(
               recompiledClasses,
-              cycleNum >= options.transitiveStep,
-              IncrementalCommon.comesFromScalaSource(previous.relations, Some(current.relations))
+              previous.apis.internalAPI,
+              current.apis.internalAPI
             )
+          debug("\nChanges:\n" + newApiChanges)
+          val nextInvalidations = invalidateAfterInternalCompilation(
+            current,
+            newApiChanges,
+            recompiledClasses,
+            cycleNum >= options.transitiveStep,
+            IncrementalCommon.comesFromScalaSource(previous.relations, Some(current.relations))
+          )
+          debug(s"Next invalidations: $nextInvalidations")
 
-            val continue = lookup.shouldDoIncrementalCompilation(nextInvalidations, current)
+          val continue = lookup.shouldDoIncrementalCompilation(nextInvalidations, current)
 
-            profiler.registerCycle(
-              invalidatedClasses,
-              invalidatedByPackageObjects,
-              initialChangedSources,
-              invalidatedSources,
-              recompiledClasses,
-              newApiChanges,
-              nextInvalidations,
-              continue
-            )
+          profiler.registerCycle(
+            invalidatedClasses,
+            invalidatedByPackageObjects,
+            initialChangedSources,
+            invalidatedSources,
+            recompiledClasses,
+            newApiChanges,
+            nextInvalidations,
+            continue
+          )
 
-            entrypoint(
-              if (continue) nextInvalidations else Set.empty,
-              Set.empty,
-              allSources,
-              IncrementalCommon.emptyChanges,
-              lookup,
-              current,
-              compileTask,
-              manager,
-              cycleNum + 1
-            )
-          }
+          entrypoint(
+            if (continue) nextInvalidations else Set.empty,
+            Set.empty,
+            allSources,
+            IncrementalCommon.emptyChanges,
+            lookup,
+            current,
+            compileTask,
+            manager,
+            cycleNum + 1
+          )
+        }
       }
     }
   }
@@ -231,21 +238,22 @@ private final class BloopNameHashing(
   }
 
   def recompileClasses(
-      sources: Set[VirtualFile],
+      currentInvalidatedSources: Set[VirtualFile],
+      invalidatedSources: Set[VirtualFile],
       binaryChanges: DependencyChanges,
       previous: Analysis,
       compileTask: (Set[VirtualFile], DependencyChanges) => Task[Analysis],
       classfileManager: ClassFileManager
   ): Task[Analysis] = {
     val pruned =
       IncrementalCommon.pruneClassFilesOfInvalidations(
-        sources,
+        invalidatedSources,
         previous,
         classfileManager,
         PlainVirtualFileConverter.converter
       )
     debug("********* Pruned: \n" + pruned.relations + "\n*********")
-    compileTask(sources, binaryChanges).map { fresh =>
+    compileTask(currentInvalidatedSources, binaryChanges).map { fresh =>
       debug("********* Fresh: \n" + fresh.relations + "\n*********")
 
       /* This is required for both scala compilation and forked java compilation, despite

frontend/src/test/scala/bloop/bsp/BspCompileSpec.scala

@@ -349,6 +349,81 @@ class BspCompileSpec(
     }
   }
 
+  test("compile incrementally renamed file") {
+    TestUtil.withinWorkspace { workspace =>
+      def path(suffix: String) = s"/main/scala/scala/meta/internal/metals/Foo$suffix.scala"
+      object Sources {
+        val `Foo0.scala` =
+          s"""${path("0")}
+             |package scala.meta.internal.metals
+             |
+             |final class Foo0(conn: () => String) {
+             |  def foo(s: String): String = s
+             |}
+          """.stripMargin
+      }
+
+      val logger = new RecordingLogger(ansiCodesSupported = false)
+      val `A` = TestProject(workspace, "a", List(Sources.`Foo0.scala`))
+      val projects = List(`A`)
+      val fooFilePath = `A`.srcFor(path("0"))
+      writeFile(fooFilePath, Sources.`Foo0.scala`)
+
+      def assertCorrectArtifacts(compiledState: ManagedBspTestState, suffix: String): Unit = {
+
+        val buildProject = compiledState.toTestState.getProjectFor(`A`)
+        val externalClassesDirA =
+          compiledState.client.getUniqueClassesDirFor(buildProject, forceGeneration = true)
+
+        val classFilesAfterSuccess = takeDirectorySnapshot(externalClassesDirA)
+        assertEquals(
+          classFilesAfterSuccess.map { classfile =>
+            val pathString = classfile.path.toString()
+            // drop drive letter and replace backslashes with forward slashes
+            if (isWindows) pathString.drop(2).replace("\\", "/") else pathString
+          },
+          List(s"/scala/meta/internal/metals/Foo$suffix.class")
+        )
+      }
+
+      loadBspState(workspace, projects, logger) { state =>
+        val firstCompiledState = state.compile(`A`)
+        assertExitStatus(firstCompiledState, ExitStatus.Ok)
+        assertValidCompilationState(firstCompiledState, projects)
+        assertCorrectArtifacts(firstCompiledState, "0")
+
+        // try to rename 10 times the same file
+        // VS Code will first change the file, then rename it
+        val last = (1 to 10).foldLeft(firstCompiledState) { (state, num) =>
+          val previousFile = `A`.srcFor(path(s"${num - 1}"))
+          val newContents = Sources.`Foo0.scala`.replace(s"Foo0", s"Foo$num")
+          // rename class
+          writeFile(
+            previousFile,
+            newContents
+          )
+          val compiledStateChangedFile = state.compile(`A`)
+          assertExitStatus(compiledStateChangedFile, ExitStatus.Ok)
+          assertValidCompilationState(compiledStateChangedFile, projects)
+          assertCorrectArtifacts(compiledStateChangedFile, s"$num")
+
+          // rename file
+          val newFile = fooFilePath.getParent.resolve(s"Foo$num.scala")
+          Files.move(previousFile.underlying, newFile.underlying)
+
+          val compiledStateRenameFile = compiledStateChangedFile.compile(`A`)
+          assertExitStatus(compiledStateRenameFile, ExitStatus.Ok)
+          assertValidCompilationState(compiledStateRenameFile, projects)
+
+          assertCorrectArtifacts(compiledStateRenameFile, s"$num")
+          compiledStateRenameFile
+        }
+
+        assertValidCompilationState(last, projects)
+      }
+    }
+  }
+
   /**
    * Checks several variants regarding the previous execution of post
    * compilation tasks when the compile result is a success and when it is a