From cd1b42f98c9660ad3c36392ceac385a3170978fd Mon Sep 17 00:00:00 2001
From: Kristian Nedrevold-Hansen <kristian@krined.no>
Date: Thu, 19 Dec 2024 01:09:26 +0100
Subject: [PATCH] Better error reporting for the linking stage and NoOp for
 linking JVM

When linking a JVM project the link task now simply noops. This lets
clients send all build projects to the link task and the task will only
link where possible. This way the client does not need to be explicit
when linking (although still possible).

Errors when linking are now accumulated and error logged. Bsp returns an
error code when encountering linking errors.
---
 .../scala/bloop/bsp/BloopBspServices.scala    | 27 ++++++++++---------
 .../main/scala/bloop/engine/Interpreter.scala |  1 +
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/frontend/src/main/scala/bloop/bsp/BloopBspServices.scala b/frontend/src/main/scala/bloop/bsp/BloopBspServices.scala
index b89288601..ee3f4f559 100644
--- a/frontend/src/main/scala/bloop/bsp/BloopBspServices.scala
+++ b/frontend/src/main/scala/bloop/bsp/BloopBspServices.scala
@@ -436,7 +436,7 @@ final class BloopBspServices(
     def doLink(
         project: Project,
         newState: State
-    ) =
+    ): Task[(State, Right[Nothing, CompileResult])] =
       project.platform match {
         case platform @ Js(config, _, _) =>
           val cmd = Commands.Link(List(project.name))
@@ -449,7 +449,9 @@ final class BloopBspServices(
             }
             (linkState, result)
           }
-        case Jvm(_, _, _, _, _, _) => ???
+        case Jvm(_, _, _, _, _, _) =>
+          // We can just NoOp here as we have already run the compile step before doLink
+          Task.now((newState, Right(bsp.CompileResult(originId, bsp.StatusCode.Ok, None, None))))
         case platform @ Native(config, _, userMainClass) =>
           val cmd = Commands.Link(List(project.name))
           val target = ScalaNativeToolchain.linkTargetFrom(project, config)
@@ -470,17 +472,14 @@ final class BloopBspServices(
 
         linkExecution.materialize.map {
           case Success(linkRunsSeq) =>
-            linkRunsSeq
-              .map(_._2)
-              .foldLeft(Option.empty[CompileResult])((_, res) => // TODO propogate stuff better here
-                res match {
-                  case Left(_) => None
-                  case Right(value) => Some(value)
-                }
-              ) match {
-              case None =>
-                (newState, Right(bsp.CompileResult(originId, bsp.StatusCode.Error, None, None)))
-              case Some(result) => (newState, Right(result))
+            val errors = linkRunsSeq.collect { case (_, Left(err)) => err }
+            if (errors.nonEmpty) {
+              logger.error(
+                s"Encountered errors when liking\n${errors.map(_.getMessage()).mkString("\n")}"
+              )
+              (newState, Right(bsp.CompileResult(originId, bsp.StatusCode.Error, None, None)))
+            } else {
+              linkRunsSeq.last
             }
           case Failure(e) =>
             val errorMessage =
@@ -488,6 +487,8 @@ final class BloopBspServices(
             (newState, Left(errorMessage))
         }
 
+      case (newState, Right(CompileResult(_, errorCode, _, _))) =>
+        Task.now((newState, Right(bsp.CompileResult(originId, errorCode, None, None))))
       case (newState, Left(error)) =>
         Task.now((newState, Left(error)))
     }
diff --git a/frontend/src/main/scala/bloop/engine/Interpreter.scala b/frontend/src/main/scala/bloop/engine/Interpreter.scala
index bc514fed9..02a827bc9 100644
--- a/frontend/src/main/scala/bloop/engine/Interpreter.scala
+++ b/frontend/src/main/scala/bloop/engine/Interpreter.scala
@@ -524,6 +524,7 @@ object Interpreter {
               case platform @ Platform.Native(config, _, _) =>
                 val target = ScalaNativeToolchain.linkTargetFrom(project, config)
                 LinkTask.linkNative(cmd, project, state, None, target, platform)
+              case _ => Task.now(state)
             }
 
           case Right(mainClass) =>