Merge pull request #2395 from kasiaMarek/metals-i6656

fix: report compile progress when no-op after unsuccessful compilation

backend/src/main/scala/bloop/Compiler.scala

@@ -10,6 +10,8 @@ import scala.collection.mutable
 import scala.concurrent.Promise
 import scala.util.Try
 
+import bloop.Compiler.Result.Failed
+import bloop.Compiler.Result.Ok
 import bloop.io.AbsolutePath
 import bloop.io.ParallelOps
 import bloop.io.ParallelOps.CopyMode
@@ -20,9 +22,12 @@ import bloop.logging.ObservedLogger
 import bloop.reporter.ProblemPerPhase
 import bloop.reporter.Reporter
 import bloop.reporter.ZincReporter
+import bloop.rtexport.RtJarCache
 import bloop.task.Task
 import bloop.tracing.BraveTracer
 import bloop.util.AnalysisUtils
+import bloop.util.BestEffortUtils
+import bloop.util.BestEffortUtils.BestEffortProducts
 import bloop.util.CacheHashCode
 import bloop.util.JavaRuntime
 import bloop.util.UUIDUtil
@@ -41,11 +46,6 @@ import xsbti.T2
 import xsbti.VirtualFileRef
 import xsbti.compile._
 
-import bloop.Compiler.Result.Failed
-import bloop.util.BestEffortUtils
-import bloop.util.BestEffortUtils.BestEffortProducts
-import bloop.rtexport.RtJarCache
-
 case class CompileInputs(
     scalaInstance: ScalaInstance,
     compilerCache: CompilerCache,
@@ -366,7 +366,13 @@ object Compiler {
     }
 
     val uniqueInputs = compileInputs.uniqueInputs
-    reporter.reportStartCompilation(previousProblems)
+    val wasPreviousSuccessful =
+      compileInputs.previousCompilerResult match {
+        case Ok(_) => true
+        case _ => false
+      }
+
+    reporter.reportStartCompilation(previousProblems, wasPreviousSuccessful)
     val fileManager = newFileManager
 
     // Manually skip redundant best-effort compilations. This is necessary because compiler

backend/src/main/scala/bloop/reporter/ObservedReporter.scala

@@ -52,8 +52,11 @@ final class ObservedReporter(
     registerAction(ReporterAction.ReportCancelledCompilation)
   }
 
-  override def reportStartCompilation(previousProblems: List[ProblemPerPhase]): Unit = {
-    underlying.reportStartCompilation(previousProblems)
+  override def reportStartCompilation(
+      previousProblems: List[ProblemPerPhase],
+      wasPreviousSuccessful: Boolean
+  ): Unit = {
+    underlying.reportStartCompilation(previousProblems, wasPreviousSuccessful)
     registerAction(ReporterAction.ReportStartCompilation)
   }
 

backend/src/main/scala/bloop/reporter/Reporter.scala

@@ -239,7 +239,10 @@ trait ZincReporter extends xsbti.Reporter with ConfigurableReporter {
   def reportCancelledCompilation(): Unit
 
   /** A function called *always* at the very beginning of compilation. */
-  def reportStartCompilation(previousProblems: List[ProblemPerPhase]): Unit
+  def reportStartCompilation(
+      previousProblems: List[ProblemPerPhase],
+      wasPreviousSuccessful: Boolean
+  ): Unit
 
   /**
    * A function called at the very end of compilation that processes the end of

frontend/src/main/scala/bloop/engine/tasks/compilation/CompileGraph.scala

@@ -149,6 +149,10 @@ object CompileGraph {
         val analysis = runningCompilation.usedLastSuccessful.previous.analysis().toOption
         val previousSuccessfulProblems =
           Compiler.previousProblemsFromSuccessfulCompilation(analysis)
+        val wasPreviousSuccessful = bundle.latestResult match {
+          case Compiler.Result.Ok(_) => true
+          case _ => false
+        }
         val previousProblems =
           Compiler.previousProblemsFromResult(bundle.latestResult, previousSuccessfulProblems)
 
@@ -166,7 +170,7 @@ object CompileGraph {
                 case ReporterAction.EnableFatalWarnings =>
                   reporter.enableFatalWarnings()
                 case ReporterAction.ReportStartCompilation =>
-                  reporter.reportStartCompilation(previousProblems)
+                  reporter.reportStartCompilation(previousProblems, wasPreviousSuccessful)
                 case a: ReporterAction.ReportStartIncrementalCycle =>
                   reporter.reportStartIncrementalCycle(a.sources, a.outputDirs)
                 case a: ReporterAction.ReportProblem => reporter.log(a.problem)

frontend/src/main/scala/bloop/reporter/BspProjectReporter.scala

@@ -88,8 +88,13 @@ final class BspProjectReporter(
   }
 
   private var recentlyReportProblemsPerFile: Map[File, List[ProblemPerPhase]] = Map.empty
+  private var wasPreviousSuccessful: Boolean = false
 
-  override def reportStartCompilation(recentProblems: List[ProblemPerPhase]): Unit = {
+  override def reportStartCompilation(
+      recentProblems: List[ProblemPerPhase],
+      wasPreviousSuccessful: Boolean
+  ): Unit = {
+    this.wasPreviousSuccessful = wasPreviousSuccessful
     recentlyReportProblemsPerFile = Reporter.groupProblemsByFile(recentProblems)
   }
 
@@ -296,7 +301,7 @@ final class BspProjectReporter(
   ): Unit = {
     val problemsInPreviousAnalysisPerFile = Reporter.groupProblemsByFile(previousSuccessfulProblems)
 
-    def recheckProblems = {
+    def mockNoOpCompileEventsAndEnd: Option[CompilationEvent.EndCompilation] = {
       recentlyReportProblemsPerFile.foreach {
         case (sourceFile, problemsPerFile) if reportAllPreviousProblems =>
           reportAllProblems(sourceFile, problemsPerFile)
@@ -321,17 +326,43 @@ final class BspProjectReporter(
               logger.noDiagnostic(CompilationEvent.NoDiagnostic(project.bspUri, sourceFile))
           }
       }
+      if (wasPreviousSuccessful) None
+      else {
+        // When no-op after unsuccessful report the start and the end of compilation
+        val startMsg = s"Start no-op compilation for ${project.name}"
+        logger.publishCompilationStart(
+          CompilationEvent.StartCompilation(
+            project.name,
+            project.bspUri,
+            startMsg,
+            taskId
+          )
+        )
+        val liftedProblems = allProblems.toIterator.map(super.liftFatalWarning(_)).toList
+        Some(
+          CompilationEvent.EndCompilation(
+            project.name,
+            project.bspUri,
+            taskId,
+            liftedProblems,
+            code,
+            isNoOp = true,
+            isLastCycle = true,
+            clientClassesDir,
+            clientAnalysisOut
+          )
+        )
+      }
     }
     wasEndProcessed = true
-    endEvent = if (cycleCount.get == 0) {
-      recheckProblems
-      None
-    } else {
-      // Great, let's report the pending end incremental cycle as the last one
-      val inputs =
-        CycleInputs(true, problemsInPreviousAnalysisPerFile, clientClassesDir, clientAnalysisOut)
-      Some(processEndPreviousCycle(inputs, Some(code)))
-    }
+    endEvent =
+      if (cycleCount.get == 0) mockNoOpCompileEventsAndEnd
+      else {
+        // Great, let's report the pending end incremental cycle as the last one
+        val inputs =
+          CycleInputs(true, problemsInPreviousAnalysisPerFile, clientClassesDir, clientAnalysisOut)
+        Some(processEndPreviousCycle(inputs, Some(code)))
+      }
 
     // Clear the state of files with problems at the end of compilation
     clearedFilesForClient.clear()

frontend/src/main/scala/bloop/reporter/LogReporter.scala

@@ -74,7 +74,10 @@ final class LogReporter(
     logger.info(s"Compiled ${project.name} (${durationMs}ms)")
   }
 
-  override def reportStartCompilation(previousProblems: List[ProblemPerPhase]): Unit = ()
+  override def reportStartCompilation(
+      previousProblems: List[ProblemPerPhase],
+      wasPreviousSuccessful: Boolean
+  ): Unit = ()
 
   override def reportEndCompilation(): Unit = ()
 

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

@@ -573,6 +573,13 @@ class BspCompileSpec(
           """#3: a/src/A.scala
             |  -> List()
             |  -> reset = true
+            |#3: task start 3
+            |  -> Msg: Start no-op compilation for a
+            |  -> Data kind: compile-task
+            |#3: task finish 3
+            |  -> errors 0, warnings 0
+            |  -> Msg: Compiled 'a'
+            |  -> Data kind: compile-report
             """.stripMargin // no-op so it only gives the compile-report
         )
 
@@ -590,7 +597,7 @@ class BspCompileSpec(
         assertSameExternalClassesDirs(fourthCompiledState, compiledState, projects)
         assertNoDiff(
           fourthCompiledState.lastDiagnostics(`A`),
-          """#4: task start 3
+          """#4: task start 4
             |  -> Msg: Compiling a (1 Scala source)
             |  -> Data kind: compile-task
             |#4: a/src/A.scala
@@ -599,7 +606,7 @@ class BspCompileSpec(
             |#4: a/src/A.scala
             |  -> List(Diagnostic(Range(Position(0,0),Position(0,26)),Some(Warning),Some(_),Some(_),Unused import,None,None,Some({"actions":[]})))
             |  -> reset = false
-            |#4: task finish 3
+            |#4: task finish 4
             |  -> errors 1, warnings 1
             |  -> Msg: Compiled 'a'
             |  -> Data kind: compile-report
@@ -613,13 +620,13 @@ class BspCompileSpec(
         assertDifferentExternalClassesDirs(fifthCompiledState, compiledState, projects)
         assertNoDiff(
           fifthCompiledState.lastDiagnostics(`A`),
-          """#5: task start 4
+          """#5: task start 5
             |  -> Msg: Compiling a (1 Scala source)
             |  -> Data kind: compile-task
             |#5: a/src/A.scala
             |  -> List(Diagnostic(Range(Position(0,0),Position(0,26)),Some(Warning),Some(_),Some(_),Unused import,None,None,Some({"actions":[]})))
             |  -> reset = true
-            |#5: task finish 4
+            |#5: task finish 5
             |  -> errors 0, warnings 1
             |  -> Msg: Compiled 'a'
             |  -> Data kind: compile-report
@@ -639,17 +646,17 @@ class BspCompileSpec(
         assertSameExternalClassesDirs(sixthCompiledState, fifthCompiledState, projects)
         assertNoDiff(
           sixthCompiledState.lastDiagnostics(`A`),
-          """#6: task start 5
+          """#6: task start 6
             |  -> Msg: Compiling a (1 Scala source)
             |  -> Data kind: compile-task
             |#6: a/src/A.scala
             |  -> List()
             |  -> reset = true
-            |#6: task finish 5
+            |#6: task finish 6
             |  -> errors 0, warnings 0
             |  -> Msg: Compiled 'a'
             |  -> Data kind: compile-report
-            |#6: task start 5
+            |#6: task start 6
             |  -> Msg: Compiling a (1 Scala source)
             |  -> Data kind: compile-task
             |#6: a/src/A.scala
@@ -658,7 +665,7 @@ class BspCompileSpec(
             |#6: a/src/A.scala
             |  -> List(Diagnostic(Range(Position(1,0),Position(3,1)),Some(Error),Some(_),Some(_),object creation impossible, since value y in trait Base of type Int is not defined,None,None,Some({"actions":[]})))
             |  -> reset = false
-            |#6: task finish 5
+            |#6: task finish 6
             |  -> errors 1, warnings 1
             |  -> Msg: Compiled 'a'
             |  -> Data kind: compile-report
@@ -673,13 +680,13 @@ class BspCompileSpec(
 
         assertNoDiff(
           seventhCompiledState.lastDiagnostics(`A`),
-          """#7: task start 6
+          """#7: task start 7
             |  -> Msg: Compiling a (1 Scala source)
             |  -> Data kind: compile-task
             |#7: a/src/A.scala
             |  -> List(Diagnostic(Range(Position(0,0),Position(0,26)),Some(Warning),Some(_),Some(_),Unused import,None,None,Some({"actions":[]})))
             |  -> reset = true
-            |#7: task finish 6
+            |#7: task finish 7
             |  -> errors 0, warnings 0
             |  -> Msg: Compiled 'a'
             |  -> Data kind: compile-report
@@ -953,9 +960,16 @@ class BspCompileSpec(
 
         assertNoDiff(
           thirdCompiledState.lastDiagnostics(`A`),
-          """#3: a/src/main/scala/Bar.scala
-            |  -> List()
-            |  -> reset = true""".stripMargin
+          """|#3: a/src/main/scala/Bar.scala
+             |  -> List()
+             |  -> reset = true
+             |#3: task start 3
+             |  -> Msg: Start no-op compilation for a
+             |  -> Data kind: compile-task
+             |#3: task finish 3
+             |  -> errors 0, warnings 0
+             |  -> Msg: Compiled 'a'
+             |  -> Data kind: compile-report""".stripMargin
         )
       }
     }

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

@@ -283,7 +283,14 @@ class BspSbtClientSpec(
           thirdCompiledState.lastDiagnostics(`A`),
           """#3: a/src/main/scala/Foo.scala
             |  -> List()
-            |  -> reset = true""".stripMargin
+            |  -> reset = true
+            |#3: task start 3
+            |  -> Msg: Start no-op compilation for a
+            |  -> Data kind: compile-task
+            |#3: task finish 3
+            |  -> errors 0, warnings 0
+            |  -> Msg: Compiled 'a'
+            |  -> Data kind: compile-report""".stripMargin
         )
       }
     }