bugfix: Fix publishing warnings

Previously, no (Scala) warnings would be reported since they would come inside stderr in a progress build event. Now, they are handled via that event and cleared correctly when needed.

MODULE.bazel

@@ -26,8 +26,8 @@ bazel_dep(
 )
 git_override(
     module_name = "bsp-testkit2",
-    commit = "a53e52ed415d087b63a40e796374c7819d9f76a7",
-    remote = "https://github.com/build-server-protocol/bsp-testkit2.git",
+    commit = "4134f87a14321c0ddde9abd5c4b0de9426b471e3",
+    remote = "https://github.com/tgodzik/bsp-testkit2.git",
 )
 
 bazel_dep(

e2e/src/main/kotlin/org/jetbrains/bsp/bazel/BazelBspScalaProjectTest.kt

@@ -1,14 +1,9 @@
 package org.jetbrains.bsp.bazel
 
-import ch.epfl.scala.bsp4j.BuildTarget
-import ch.epfl.scala.bsp4j.BuildTargetCapabilities
-import ch.epfl.scala.bsp4j.BuildTargetIdentifier
-import ch.epfl.scala.bsp4j.ScalacOptionsItem
-import ch.epfl.scala.bsp4j.ScalacOptionsParams
-import ch.epfl.scala.bsp4j.ScalacOptionsResult
-import ch.epfl.scala.bsp4j.WorkspaceBuildTargetsResult
+import ch.epfl.scala.bsp4j.*
 import org.jetbrains.bsp.bazel.base.BazelBspTestBaseScenario
 import org.jetbrains.bsp.bazel.base.BazelBspTestScenarioStep
+import java.util.UUID
 import kotlin.time.Duration.Companion.seconds
 
 object BazelBspScalaProjectTest : BazelBspTestBaseScenario() {
@@ -19,44 +14,49 @@ object BazelBspScalaProjectTest : BazelBspTestBaseScenario() {
 
   override fun scenarioSteps(): List<BazelBspTestScenarioStep> = listOf(
     scalaOptionsResults(),
+    compileWithWarnings(),
   )
 
   override fun expectedWorkspaceBuildTargetsResult(): WorkspaceBuildTargetsResult {
-    return WorkspaceBuildTargetsResult(
-      listOf(
-        BuildTarget(
-          BuildTargetIdentifier("@//scala_targets:library"),
-          listOf("library"),
-          listOf("scala"),
-          emptyList(),
-          BuildTargetCapabilities()
-        )
-      )
-    )
+    return WorkspaceBuildTargetsResult(listOf(BuildTarget(BuildTargetIdentifier("@//scala_targets:library"), listOf("library"), listOf("scala"), emptyList(), BuildTargetCapabilities())))
   }
 
 
   private fun scalaOptionsResults(): BazelBspTestScenarioStep {
     val expectedTargetIdentifiers = expectedTargetIdentifiers().filter { it.uri != "bsp-workspace-root" }
     val expectedScalaOptionsItems = expectedTargetIdentifiers.map {
-      ScalacOptionsItem(
-        it,
-        emptyList(),
-        listOf(
-          "file://\$BAZEL_OUTPUT_BASE_PATH/execroot/__main__/bazel-out/k8-fastbuild/bin/external/io_bazel_rules_scala_scala_library/io_bazel_rules_scala_scala_library.stamp/scala-library-2.13.6-stamped.jar",
-          "file://\$BAZEL_OUTPUT_BASE_PATH/execroot/__main__/bazel-out/k8-fastbuild/bin/external/io_bazel_rules_scala_scala_reflect/io_bazel_rules_scala_scala_reflect.stamp/scala-reflect-2.13.6-stamped.jar"
-        ),
-        "file://\$BAZEL_OUTPUT_BASE_PATH/execroot/__main__/bazel-out/k8-fastbuild/bin/scala_targets/library.jar"
-      )
+      ScalacOptionsItem(it, emptyList(), listOf("file://\$BAZEL_OUTPUT_BASE_PATH/execroot/__main__/bazel-out/k8-fastbuild/bin/external/io_bazel_rules_scala_scala_library/io_bazel_rules_scala_scala_library.stamp/scala-library-2.13.6-stamped.jar", "file://\$BAZEL_OUTPUT_BASE_PATH/execroot/__main__/bazel-out/k8-fastbuild/bin/external/io_bazel_rules_scala_scala_reflect/io_bazel_rules_scala_scala_reflect.stamp/scala-reflect-2.13.6-stamped.jar"), "file://\$BAZEL_OUTPUT_BASE_PATH/execroot/__main__/bazel-out/k8-fastbuild/bin/scala_targets/library.jar")
     }
     val expectedScalaOptionsResult = ScalacOptionsResult(expectedScalaOptionsItems)
     val scalaOptionsParams = ScalacOptionsParams(expectedTargetIdentifiers)
-
-    return BazelBspTestScenarioStep(
-      "scalaOptions results"
-    ) {
+    return BazelBspTestScenarioStep("scalaOptions results") {
       testClient.testScalacOptions(60.seconds, scalaOptionsParams, expectedScalaOptionsResult)
     }
   }
 
+  private fun compileWithWarnings(): BazelBspTestScenarioStep {
+    val expectedTargetIdentifiers = expectedTargetIdentifiers().filter { it.uri != "bsp-workspace-root" }
+    val compileParams = CompileParams(expectedTargetIdentifiers);
+    compileParams.originId = UUID.randomUUID().toString()
+
+    val expectedCompilerResult = CompileResult(StatusCode.OK)
+    val expectedDiagnostic = Diagnostic(
+      Range(Position(2, 4), Position(2, 4)),
+      "\"match may not be exhaustive.\\nIt would fail on the following input: C(_)\\n  aa match {\\n  ^\""
+    )
+    expectedDiagnostic.severity = DiagnosticSeverity.WARNING
+    val expectedDocumentId = TextDocumentIdentifier("file://\$WORKSPACE/scala_targets/Example.scala")
+    val expectedDiagnosticsParam = PublishDiagnosticsParams(
+      expectedDocumentId,
+      expectedTargetIdentifiers[0],
+      listOf(expectedDiagnostic),
+      true,
+    )
+    expectedDiagnosticsParam.originId = compileParams.originId
+
+    return BazelBspTestScenarioStep("compile results") {
+      testClient.testCompile(60.seconds, compileParams, expectedCompilerResult, listOf(expectedDiagnosticsParam))
+    }
+  }
+
 }

e2e/test-resources/enabled-rules-project/scala_targets/Example.scala

@@ -1,3 +1,12 @@
 package example
 
-object Example extends App{}
+object Example extends App{
+  val aa: A = ???
+  aa match {
+    case B(_) =>
+  }
+}
+
+sealed trait A
+case class B(b: Int) extends A
+case class C(c: Int) extends A

server/src/main/kotlin/org/jetbrains/bsp/bazel/server/bep/BepServer.kt

@@ -165,6 +165,9 @@ class BepServer(
 
   private fun processProgressEvent(event: BuildEventStreamProtos.BuildEvent) {
     if (event.hasProgress()) {
+      if (target != null) {
+        processDiagnosticText(event.progress.stderr, target.uri, true)
+      }
       // TODO https://youtrack.jetbrains.com/issue/BAZEL-622
       // bepLogger.onProgress(event.getProgress());
     }
@@ -250,32 +253,34 @@ class BepServer(
         try {
           val path = Paths.get(URI.create(actionEvent.stderr.uri))
           val stdErrText = Files.readString(path)
-          processDiagnosticText(stdErrText, label)
+          processDiagnosticText(stdErrText, label, false)
         } catch (e: FileSystemNotFoundException) {
           LOGGER.warn(e)
         } catch (e: IOException) {
           LOGGER.warn(e)
         }
       }
       BuildEventStreamProtos.File.FileCase.CONTENTS -> {
-        processDiagnosticText(actionEvent.stderr.contents.toStringUtf8(), label)
+        processDiagnosticText(actionEvent.stderr.contents.toStringUtf8(), label, false)
       }
       else -> {
-        processDiagnosticText("", label)
+        processDiagnosticText("", label, false)
       }
     }
   }
 
-  private fun processDiagnosticText(stdErrText: String, targetLabel: String) {
-    val events =
-      diagnosticsService.extractDiagnostics(
-        stdErrText, targetLabel, startedEvents.first.value
-      )
-    events.forEach(Consumer { publishDiagnosticsParams: PublishDiagnosticsParams? ->
-      bspClient.onBuildPublishDiagnostics(
-        publishDiagnosticsParams
-      )
-    })
+  private fun processDiagnosticText(stdErrText: String, targetLabel: String, diagnosticsFromProgress: Boolean) {
+    if (startedEvents.isNotEmpty() && stdErrText.isNotEmpty()) {
+      val events =
+        diagnosticsService.extractDiagnostics(
+          stdErrText, targetLabel, startedEvents.first.value, diagnosticsFromProgress
+        )
+      events.forEach(Consumer { publishDiagnosticsParams: PublishDiagnosticsParams? ->
+        bspClient.onBuildPublishDiagnostics(
+            publishDiagnosticsParams
+        )
+      })
+    }
   }
 
   private fun consumeCompletedEvent(event: BuildEventStreamProtos.BuildEvent) {

server/src/main/kotlin/org/jetbrains/bsp/bazel/server/diagnostics/DiagnosticsParser.kt

@@ -2,9 +2,9 @@ package org.jetbrains.bsp.bazel.server.diagnostics
 
 class DiagnosticsParser {
 
-  fun parse(bazelOutput: String, target: String): List<Diagnostic> {
+  fun parse(bazelOutput: String, target: String, onlyKnownFiles: Boolean): List<Diagnostic> {
     val output = prepareOutput(bazelOutput, target)
-    val diagnostics = collectDiagnostics(output)
+    val diagnostics = collectDiagnostics(output, onlyKnownFiles)
     return deduplicate(diagnostics)
   }
 
@@ -14,7 +14,7 @@ class DiagnosticsParser {
     return Output(relevantLines, target)
   }
 
-  private fun collectDiagnostics(output: Output): List<Diagnostic> {
+  private fun collectDiagnostics(output: Output, onlyKnownFiles: Boolean): List<Diagnostic> {
     val diagnostics = mutableListOf<Diagnostic>()
     while (output.nonEmpty()) {
       for (parser in Parsers) {
@@ -26,7 +26,7 @@ class DiagnosticsParser {
       }
     }
 
-    if (diagnostics.isEmpty()) {
+    if (diagnostics.isEmpty() && !onlyKnownFiles) {
       diagnostics.add(
         Diagnostic(
           position = Position(0, 0),

server/src/main/kotlin/org/jetbrains/bsp/bazel/server/diagnostics/DiagnosticsService.kt

@@ -1,11 +1,9 @@
 package org.jetbrains.bsp.bazel.server.diagnostics
 
 import ch.epfl.scala.bsp4j.BuildTargetIdentifier
-import ch.epfl.scala.bsp4j.Diagnostic
 import ch.epfl.scala.bsp4j.PublishDiagnosticsParams
 import ch.epfl.scala.bsp4j.TextDocumentIdentifier
 import java.nio.file.Path
-import java.util.concurrent.ConcurrentHashMap
 import java.util.Collections
 
 /**
@@ -17,23 +15,31 @@ class DiagnosticsService(workspaceRoot: Path, private val hasAnyProblems: Mutabl
 
     private val parser = DiagnosticsParser()
     private val mapper = DiagnosticBspMapper(workspaceRoot)
+    /* Warnings are reported before the target completed event, when everything is cleared. so we want to avoid removing them */
+    private val updatedInThisRun = mutableSetOf<PublishDiagnosticsParams>()
 
     fun getBspState() = Collections.unmodifiableMap(hasAnyProblems)
 
     fun extractDiagnostics(
         bazelOutput: String,
         targetLabel: String,
-        originId: String?
+        originId: String?,
+        diagnosticsFromProgress: Boolean
     ): List<PublishDiagnosticsParams> {
-        val parsedDiagnostics = parser.parse(bazelOutput, targetLabel)
+        val parsedDiagnostics = parser.parse(bazelOutput, targetLabel, diagnosticsFromProgress)
         val events = mapper.createDiagnostics(parsedDiagnostics, originId)
+        if (diagnosticsFromProgress) updatedInThisRun.addAll(events)
         updateProblemState(events)
         return events
     }
 
     fun clearFormerDiagnostics(targetLabel: String): List<PublishDiagnosticsParams> {
 	    val docs = hasAnyProblems[targetLabel]
 	    hasAnyProblems.remove(targetLabel)
+        if (updatedInThisRun.isNotEmpty()) {
+            hasAnyProblems[targetLabel] = updatedInThisRun.map { it.textDocument }.toSet()
+            updatedInThisRun.clear()
+        }
 	    return docs
 	      ?.map { PublishDiagnosticsParams(it, BuildTargetIdentifier(targetLabel), emptyList(), true)}
 	      .orEmpty()

server/src/test/kotlin/org/jetbrains/bsp/bazel/server/diagnostics/DiagnosticsServiceTest.kt

@@ -638,7 +638,7 @@ class DiagnosticsServiceTest {
         diagnosticsBeforeError.shouldBeEmpty()
 
         // Extract the diagnostics from BEP, there's an error in `Test.scala` of "//path/to/package:test".
-        service.extractDiagnostics(output, "//path/to/package:test", null)
+        service.extractDiagnostics(output, "//path/to/package:test", null, false)
 
         // Assert that state is updated
         service.getBspState().keys.shouldHaveSize(1)
@@ -726,8 +726,8 @@ class DiagnosticsServiceTest {
             )
         )
 
-        service.extractDiagnostics(output1, "//path/to/package:test", null)
-        service.extractDiagnostics(output2, "//path/to/package2:test", null)
+        service.extractDiagnostics(output1, "//path/to/package:test", null, false)
+        service.extractDiagnostics(output2, "//path/to/package2:test", null, false)
 
         // Assert that state is updated
         service.getBspState().keys shouldContainExactlyInAnyOrder listOf(
@@ -775,6 +775,6 @@ class DiagnosticsServiceTest {
     }
 
     private fun extractDiagnostics(output: String, buildTarget: String): List<PublishDiagnosticsParams>? {
-        return DiagnosticsService(workspacePath, ConcurrentHashMap()).extractDiagnostics(output, buildTarget, null)
+        return DiagnosticsService(workspacePath, ConcurrentHashMap()).extractDiagnostics(output, buildTarget, null, false)
     }
 }