From 0727b16efd58928abc730bd3bf29ed86af4737f0 Mon Sep 17 00:00:00 2001
From: Tomasz Godzik <tgodzik@virtuslab.com>
Date: Tue, 10 Sep 2024 19:43:05 +0200
Subject: [PATCH] bugfix: Don't republish old errors on successful compilation

We need to report all problems if a BSP client just connected to the server and never compiled.
`reportAllPreviousProblems` will be set to true in that case.

However, if previously we had an error, which was reverted and got back to the state before the
error, we will get a successfull noop compilation, which will not produce any problems.

In this case we don't want to republish everything again, since diagnostics will contain the
error which was reverted.

Connected to https://github.com/VirtusLab/scala-cli/issues/2226 where CLI always connected anew.
---
 .../bloop/reporter/BspProjectReporter.scala   | 25 +++++++-
 .../test/scala/bloop/bsp/BspCompileSpec.scala | 59 +++++++++++++++++++
 2 files changed, 81 insertions(+), 3 deletions(-)

diff --git a/frontend/src/main/scala/bloop/reporter/BspProjectReporter.scala b/frontend/src/main/scala/bloop/reporter/BspProjectReporter.scala
index f489caca0..0e52a7321 100644
--- a/frontend/src/main/scala/bloop/reporter/BspProjectReporter.scala
+++ b/frontend/src/main/scala/bloop/reporter/BspProjectReporter.scala
@@ -301,14 +301,31 @@ final class BspProjectReporter(
   ): Unit = {
     val problemsInPreviousAnalysisPerFile = Reporter.groupProblemsByFile(previousSuccessfulProblems)
 
+    /**
+     * We need to report all problems if a BSP client just connected to the server and never compiled.
+     * `reportAllPreviousProblems` will be set to true in that case.
+     *
+     * However, if previously we had an error, which was reverted and got back to the state before the
+     * error, we will get a successfull noop compilation, which will not produce any problems.
+     *
+     * In this case we don't want to republish everything again, since diagnostics will contain the
+     * error which was reverted.
+     *
+     * Connected to https://github.com/VirtusLab/scala-cli/issues/2226 where CLI always connected anew.
+     */
+    def shouldPublishAllProblems =
+      reportAllPreviousProblems && !wasPreviousSuccessful && code != bsp.StatusCode.Ok
     def mockNoOpCompileEventsAndEnd: Option[CompilationEvent.EndCompilation] = {
       recentlyReportProblemsPerFile.foreach {
-        case (sourceFile, problemsPerFile) if reportAllPreviousProblems =>
+        case (sourceFile, problemsPerFile) if shouldPublishAllProblems =>
           reportAllProblems(sourceFile, problemsPerFile)
         case (sourceFile, problemsPerFile) =>
           problemsInPreviousAnalysisPerFile.get(sourceFile) match {
             case Some(problemsInPreviousAnalysis) =>
-              if (problemsInPreviousAnalysis.map(_.problem) == problemsPerFile.map(_.problem)) {
+              if (
+                problemsInPreviousAnalysis
+                  .map(_.problem) == problemsPerFile.map(_.problem) && !reportAllPreviousProblems
+              ) {
                 // If problems are the same, diagnostics in the editor are up-to-date, do nothing
                 ()
               } else {
@@ -322,8 +339,10 @@ final class BspProjectReporter(
                 }
               }
 
-            case None =>
+            // there is nothing to clear if we need to rereport all problems
+            case None if !reportAllPreviousProblems =>
               logger.noDiagnostic(CompilationEvent.NoDiagnostic(project.bspUri, sourceFile))
+            case _ =>
           }
       }
       if (wasPreviousSuccessful) None
diff --git a/frontend/src/test/scala/bloop/bsp/BspCompileSpec.scala b/frontend/src/test/scala/bloop/bsp/BspCompileSpec.scala
index 3687ba015..e2f36d760 100644
--- a/frontend/src/test/scala/bloop/bsp/BspCompileSpec.scala
+++ b/frontend/src/test/scala/bloop/bsp/BspCompileSpec.scala
@@ -748,6 +748,65 @@ class BspCompileSpec(
     }
   }
 
+  test("no-op compile and not publish diagnostics from a previous failed CLI compilation") {
+    TestUtil.withinWorkspace { workspace =>
+      object Sources {
+        val `App.scala` =
+          """/main/scala/App.scala
+            |object App {
+            |  val a: String = ""
+            |}
+          """.stripMargin
+      }
+
+      val bspLogger = new RecordingLogger(ansiCodesSupported = false)
+      val `A` = TestProject(workspace, "a", List(Sources.`App.scala`))
+      val projects = List(`A`)
+
+      // compile successfully in a separate bsp connection and then break
+      loadBspState(workspace, projects, bspLogger) { state =>
+        val compiledState = state.compile(`A`)
+        assertExitStatus(compiledState, ExitStatus.Ok)
+        assertValidCompilationState(compiledState, projects)
+
+        writeFile(
+          `A`.srcFor("/main/scala/App.scala"),
+          """|object App {
+             |  val a: String = 1
+             |}
+          """.stripMargin
+        )
+        val secondCliCompiledState = compiledState.compile(`A`)
+        assertExitStatus(secondCliCompiledState, ExitStatus.CompilationError)
+        assertValidCompilationState(secondCliCompiledState, projects)
+      }
+
+      // connecting and compiling should not get any previous and no longer valid diagnostics
+      loadBspState(workspace, projects, bspLogger) { state =>
+        // Remove the error and get back to the previous state
+        writeFile(
+          `A`.srcFor("/main/scala/App.scala"),
+          Sources.`App.scala`
+        )
+        val compiledState = state.compile(`A`)
+        assertExitStatus(compiledState, ExitStatus.Ok)
+        assertValidCompilationState(compiledState, projects)
+
+        assertNoDiff(
+          compiledState.lastDiagnostics(`A`),
+          """|#1: task start 1
+             |  -> Msg: Start no-op compilation for a
+             |  -> Data kind: compile-task
+             |#1: task finish 1
+             |  -> errors 0, warnings 0
+             |  -> Msg: Compiled 'a'
+             |  -> Data kind: compile-report
+             |""".stripMargin
+        )
+      }
+    }
+  }
+
   test("compile incrementally and publish warnings from a previous CLI compilation") {
     TestUtil.withinWorkspace { workspace =>
       object Sources {