improvement: Show crashes more visibly for the user

Previously, we would only show it in the build server status, which might not always be visible to the user.

Now, we show a new diagnotic pointing to the actual error report. It will get removed when the next compilation runs.

Author: tgodzik

metals/src/main/scala/scala/meta/internal/builds/BSPErrorHandler.scala

@@ -5,24 +5,30 @@ import java.util.Optional
 
 import scala.meta.internal.bsp.BspSession
 import scala.meta.internal.bsp.ConnectionBspStatus
+import scala.meta.internal.metals.Diagnostics
 import scala.meta.internal.metals.MetalsEnrichments._
 import scala.meta.internal.metals.Report
 import scala.meta.internal.metals.ReportContext
 import scala.meta.internal.metals.Tables
+import scala.meta.io.AbsolutePath
 
 import com.google.common.io.BaseEncoding
 
 class BspErrorHandler(
     currentSession: () => Option[BspSession],
     tables: Tables,
     bspStatus: ConnectionBspStatus,
+    diagnostics: Diagnostics,
 )(implicit reportContext: ReportContext) {
   def onError(message: String): Unit = {
     if (shouldShowBspError) {
       for {
         report <- createReport(message).asScala
         if !tables.dismissedNotifications.BspErrors.isDismissed
-      } bspStatus.showError(message, report)
+      } {
+        bspStatus.showError(message, report)
+        diagnostics.onBuildTargetCompilationCrash(AbsolutePath(report), message)
+      }
     } else logError(message)
   }
 

metals/src/main/scala/scala/meta/internal/metals/Diagnostics.scala

@@ -25,6 +25,7 @@ import org.eclipse.{lsp4j => l}
 private final case class CompilationStatus(
     code: bsp4j.StatusCode,
     errors: Int,
+    originId: String,
 )
 
 case class DiagnosticWithOrigin(diagnostic: Diagnostic, originId: String)
@@ -67,6 +68,14 @@ final class Diagnostics(
   private val compilationStatus =
     TrieMap.empty[BuildTargetIdentifier, CompilationStatus]
 
+  /*
+   * A map of build crashes diagnostics.
+   * The key is the path of the markdown file that contains the build crash.
+   * The diagnostics will be removed at the end of the next compilation.
+   */
+  private val buildErrorDiagnostics =
+    TrieMap.empty[AbsolutePath, Diagnostic]
+
   def forFile(path: AbsolutePath): Seq[Diagnostic] = {
     diagnostics
       .getOrElse(path, new ConcurrentLinkedQueue[DiagnosticWithOrigin]())
@@ -83,12 +92,14 @@ final class Diagnostics(
   def reset(): Unit = {
     val keys = diagnostics.keys
     diagnostics.clear()
+    buildErrorDiagnostics.clear()
     keys.foreach { key => publishDiagnostics(key) }
   }
 
   def reset(paths: Seq[AbsolutePath]): Unit =
     for (path <- paths if diagnostics.contains(path)) {
       diagnostics.remove(path)
+      buildErrorDiagnostics.remove(path)
       publishDiagnostics(path)
     }
 
@@ -117,6 +128,8 @@ final class Diagnostics(
       downstreamTargets.remove(target)
     }
 
+    removeStaleBuildErrorDiagnostics()
+
     // Bazel doesn't clean diagnostics for paths with no errors, so instead we remove everything
     // from previous compilations.
     val isBazel = buildTargets.buildServerOf(target).exists(_.isBazel)
@@ -140,7 +153,7 @@ final class Diagnostics(
     publishDiagnosticsBuffer()
 
     compileTimer.remove(target)
-    val status = CompilationStatus(statusCode, report.getErrors())
+    val status = CompilationStatus(statusCode, report.getErrors(), originId)
     compilationStatus.update(target, status)
   }
 
@@ -440,4 +453,34 @@ final class Diagnostics(
 
   private def shouldAdjustWithinToken(diagnostic: l.Diagnostic): Boolean =
     diagnostic.getSource() == "scala-cli"
+
+  def onBuildTargetCompilationCrash(
+      reportPath: AbsolutePath,
+      message: String,
+  ): Unit = {
+    val diagnostic = new l.Diagnostic(
+      new l.Range(new l.Position(0, 0), new l.Position(0, 0)),
+      message,
+      l.DiagnosticSeverity.Error,
+      "build-server",
+    )
+    buildErrorDiagnostics(reportPath) = diagnostic
+
+    languageClient.publishDiagnostics(
+      new PublishDiagnosticsParams(
+        reportPath.toURI.toString(),
+        List(diagnostic).asJava,
+      )
+    )
+  }
+
+  private def removeStaleBuildErrorDiagnostics(): Unit = {
+    val all = buildErrorDiagnostics.keySet
+    all.foreach { path =>
+      languageClient.publishDiagnostics(
+        new PublishDiagnosticsParams(path.toURI.toString(), List().asJava)
+      )
+      buildErrorDiagnostics.remove(path)
+    }
+  }
 }

metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala

@@ -286,6 +286,7 @@ abstract class MetalsLspService(
       () => bspSession,
       tables,
       connectionBspStatus,
+      diagnostics,
     )
 
   val workspaceSymbols: WorkspaceSymbolProvider =

tests/unit/src/main/scala/tests/TestingClient.scala

@@ -300,7 +300,9 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers)
   }
   def workspaceDiagnostics: String = {
     val paths = diagnostics.keys.toList
-      .filter(f => f.isScalaOrJava || f.extension == "conf")
+      .filter(f =>
+        f.isScalaOrJava || f.extension == "conf" || f.extension == "md"
+      )
       .sortBy(_.toURI.toString)
     paths.map(pathDiagnostics).mkString
   }

tests/unit/src/test/scala/tests/DiagnosticsLspSuite.scala

@@ -555,6 +555,57 @@ class DiagnosticsLspSuite extends BaseLspSuite("diagnostics") {
           |""".stripMargin
   }
 
+  test("build-error-diagnostics") {
+    cleanWorkspace()
+    for {
+      _ <- initialize(
+        """|/metals.json
+           |{
+           |  "a": {},
+           |  "b": {
+           |    "dependsOn": ["a"]
+           |  }
+           |}
+           |/a/src/main/scala/a/A.scala
+           |package foo
+           |
+           |import scala.language.experimental.macros
+           |import scala.reflect.macros.blackbox
+           |
+           |object FooMacro {
+           |  def crashNow: Int = macro crashNowImpl
+           |
+           |  def crashNowImpl(c: blackbox.Context): c.Expr[Int] = {
+           |    import c.universe._
+           |    val badSymbol = c.internal.newTermSymbol(NoSymbol, TermName("badSymbol"))
+           |    val badTree = Ident(badSymbol)
+           |    c.Expr[Int](badTree)
+           |  }
+           |}
+           |
+           |/b/src/main/scala/b/B.scala
+           |package example
+           |import foo.FooMacro
+           |
+           |object Bar extends App {
+           |  FooMacro.crashNow
+           |} 
+           |""".stripMargin
+      )
+      _ <- server.didOpen("b/src/main/scala/b/B.scala")
+      _ <- server.didSave("b/src/main/scala/b/B.scala")
+      _ = assertContains(
+        client.workspaceDiagnostics,
+        "error: Unexpected error when compiling b: java.lang.AssertionError: assertion failed:",
+      )
+      _ <- server.didChange("b/src/main/scala/b/B.scala") {
+        _.replace("FooMacro.crashNow", "// FooMacro.crashNow")
+      }
+      _ <- server.didSave("b/src/main/scala/b/B.scala")
+      _ = assertNoDiagnostics()
+    } yield ()
+  }
+
   class Basic(name: String) {
     val path: String = s"$name/src/main/scala/$name/${name.toUpperCase()}.scala"
     def content(tpe: String, value: String): String =