Skip to content

Commit

Permalink
Add build/taskStart and taskFinish to the exception reporting BSP mec…
Browse files Browse the repository at this point in the history
…hanism (#1821)
  • Loading branch information
MaciejG604 committed Feb 9, 2023
1 parent 2a1c30e commit f64512a
Show file tree
Hide file tree
Showing 5 changed files with 210 additions and 17 deletions.
42 changes: 34 additions & 8 deletions modules/build/src/main/scala/scala/build/bsp/BspClient.scala
Expand Up @@ -2,7 +2,7 @@ package scala.build.bsp

import ch.epfl.scala.{bsp4j => b}

import java.lang.{Boolean => JBoolean}
import java.lang.Boolean as JBoolean
import java.net.URI
import java.nio.file.Paths
import java.util.concurrent.{ConcurrentHashMap, ExecutorService}
Expand Down Expand Up @@ -155,12 +155,11 @@ class BspClient(
case None =>
logger.debug(s"Not reporting $ex to users (no build target id)")
case Some(targetId) =>
val allExceptions = ex match {
case c: CompositeBuildException => c.exceptions
case _ => Seq(ex)
}
val touchedFiles =
allExceptions.flatMap(reportDiagnosticForFiles(targetId, reset = reset)).toSet
val touchedFiles = (ex match {
case c: CompositeBuildException =>
reportDiagnosticsForFiles(targetId, c.exceptions, reset = reset)
case _ => reportDiagnosticsForFiles(targetId, Seq(ex), reset = reset)
}).toSet

// Small chance of us wiping some Bloop diagnostics, if these happen
// between the call to remove and the call to actualBuildPublishDiagnostics.
Expand All @@ -182,7 +181,34 @@ class BspClient(
}
}

def reportDiagnosticForFiles(
def reportDiagnosticsForFiles(
targetId: b.BuildTargetIdentifier,
diags: Seq[Diagnostic],
reset: Boolean = true
): Seq[os.Path] =
if reset then // send diagnostic with reset only once for every file path
diags.flatMap { diag =>
diag.positions.map { position =>
Diagnostic(diag.message, diag.severity, Seq(position), diag.textEdit)
}
}
.groupBy(_.positions.headOption match
case Some(File(Right(path), _, _)) => Some(path)
case _ => None
)
.filter(_._1.isDefined)
.values
.toSeq
.flatMap {
case head :: tail =>
reportDiagnosticForFiles(targetId, reset = reset)(head)
++ tail.flatMap(reportDiagnosticForFiles(targetId))
case _ => Nil
}
else
diags.flatMap(reportDiagnosticForFiles(targetId))

private def reportDiagnosticForFiles(
targetId: b.BuildTargetIdentifier,
reset: Boolean = false
)(diag: Diagnostic): Seq[os.Path] =
Expand Down
50 changes: 44 additions & 6 deletions modules/build/src/main/scala/scala/build/bsp/BspImpl.scala
Expand Up @@ -7,14 +7,20 @@ import org.eclipse.lsp4j.jsonrpc
import org.eclipse.lsp4j.jsonrpc.messages.ResponseError

import java.io.{InputStream, OutputStream}
import java.util.UUID
import java.util.concurrent.{CompletableFuture, Executor}

import scala.build.EitherCps.{either, value}
import scala.build.*
import scala.build.actionable.ActionablePreprocessor
import scala.build.bloop.BloopServer
import scala.build.compiler.BloopCompiler
import scala.build.errors.{BuildException, Diagnostic, ParsingInputsException}
import scala.build.errors.{
BuildException,
CompositeBuildException,
Diagnostic,
ParsingInputsException
}
import scala.build.input.Inputs
import scala.build.internal.{Constants, CustomCodeWrapper}
import scala.build.options.{BuildOptions, Scope}
Expand Down Expand Up @@ -287,10 +293,42 @@ final class BspImpl(

preBuild.thenCompose {
case Left((ex, scope)) =>
actualLocalClient.reportBuildException(
currentBloopSession.bspServer.targetScopeIdOpt(scope),
ex
)
val taskId = new b.TaskId(UUID.randomUUID().toString)

for targetId <- currentBloopSession.bspServer.targetScopeIdOpt(scope) do {
val target = targetId.getUri match {
case s"$_?id=$targetId" => targetId
case targetIdUri => targetIdUri
}

val taskStartParams = new b.TaskStartParams(taskId)
taskStartParams.setEventTime(System.currentTimeMillis())
taskStartParams.setMessage(s"Preprocessing '$target'")
taskStartParams.setDataKind(b.TaskDataKind.COMPILE_TASK)
taskStartParams.setData(new b.CompileTask(targetId))

actualLocalClient.onBuildTaskStart(taskStartParams)

actualLocalClient.reportBuildException(
Some(targetId),
ex
)

val taskFinishParams = new b.TaskFinishParams(taskId, b.StatusCode.ERROR)
taskFinishParams.setEventTime(System.currentTimeMillis())
taskFinishParams.setMessage(s"Preprocessed '$target'")
taskFinishParams.setDataKind(b.TaskDataKind.COMPILE_REPORT)

val errorSize = ex match {
case c: CompositeBuildException => c.exceptions.size
case _ => 1
}

taskFinishParams.setData(new b.CompileReport(targetId, errorSize, 0))

actualLocalClient.onBuildTaskFinish(taskFinishParams)
}

CompletableFuture.completedFuture(
new b.CompileResult(b.StatusCode.ERROR)
)
Expand All @@ -299,7 +337,7 @@ final class BspImpl(
actualLocalClient.resetBuildExceptionDiagnostics(targetId)

val targetId = currentBloopSession.bspServer.targetIds.head
params.diagnostics.foreach(actualLocalClient.reportDiagnosticForFiles(targetId))
actualLocalClient.reportDiagnosticsForFiles(targetId, params.diagnostics, reset = false)

doCompile().thenCompose { res =>
def doPostProcess(data: PreBuildData, scope: Scope): Unit =
Expand Down
Expand Up @@ -20,12 +20,14 @@ object Diagnostic {
private case class ADiagnostic(
message: String,
severity: Severity,
positions: Seq[Position]
positions: Seq[Position],
override val textEdit: Option[TextEdit]
) extends Diagnostic

def apply(
message: String,
severity: Severity,
positions: Seq[Position] = Nil
): Diagnostic = ADiagnostic(message, severity, positions)
positions: Seq[Position] = Nil,
textEdit: Option[TextEdit] = None
): Diagnostic = ADiagnostic(message, severity, positions, textEdit)
}
Expand Up @@ -1306,6 +1306,110 @@ abstract class BspTestDefinitions(val scalaVersionOpt: Option[String])
}
}
}

test("bsp should report errors with taskStart and taskFinish for ammonite imports") {
val fileName = "Hello.scala"
val inputs = TestInputs(
os.rel / fileName ->
s"""import $$ivy.`com.lihaoyi::os-lib:0.7.8`
|
|object Hello extends App {
| println("Hello")
|}
|""".stripMargin
)
withBsp(inputs, Seq(".", "--actions")) {
(_, localClient, remoteServer) =>
async {
// prepare build
val buildTargetsResp = await(remoteServer.workspaceBuildTargets().asScala)
// build code
val targets = buildTargetsResp.getTargets.asScala.map(_.getId()).asJava

await(remoteServer.buildTargetCompile(new b.CompileParams(targets)).asScala)

val taskMessages = localClient.getTaskDiagnostics()

expect(taskMessages.size == 1)
expect(taskMessages.head.taskMessages.size == 1)

val taskFinishData = new Gson().fromJson[b.CompileReport](
taskMessages.head.finishParams.getData.asInstanceOf[JsonElement],
classOf[b.CompileReport]
)

expect(taskFinishData.getErrors == 1)

checkDiagnostic(
taskMessages.head.taskMessages.head.getDiagnostics.get(0),
expectedMessage =
"Ammonite imports using \"$ivy\" and \"$dep\" are no longer supported, switch to 'using lib' directive",
expectedSeverity = b.DiagnosticSeverity.ERROR,
expectedStartLine = 0,
expectedStartCharacter = 0,
expectedEndLine = 0,
expectedEndCharacter = 39,
expectedSource = Some("scala-cli")
)
}
}
}

test(
"bsp should report errors with taskStart and taskFinish for ammonite imports in multiple files"
) {
val inputs = TestInputs(
os.rel / "Hello.scala" ->
s"""import $$ivy.`com.lihaoyi::os-lib:0.7.8`
|import $$dep.`com.lihaoyi::pprint:0.6.6`
|import $$dep.`com.lihaoyi::utest::0.7.10`
|import $$dep.`com.lihaoyi::utest::0.7.10`
|
|object Hello extends App {
| println("Hello")
|}
|""".stripMargin,
os.rel / "Foo.scala" ->
s"""import $$dep.`com.lihaoyi::utest::0.7.10`
|import $$ivy.`com.lihaoyi::pprint:0.6.6`
|
|object Foo {
| def foo() = println("Hello")
|}
|""".stripMargin
)
withBsp(inputs, Seq(".", "--actions")) {
(_, localClient, remoteServer) =>
async {
// prepare build
val buildTargetsResp = await(remoteServer.workspaceBuildTargets().asScala)
// build code
val targets = buildTargetsResp.getTargets.asScala.map(_.getId()).asJava

await(remoteServer.buildTargetCompile(new b.CompileParams(targets)).asScala)

val taskMessages = localClient.getTaskDiagnostics()

expect(taskMessages.size == 1)
expect(taskMessages.head.taskMessages.size == 6)

val taskFinishData = new Gson().fromJson[b.CompileReport](
taskMessages.head.finishParams.getData.asInstanceOf[JsonElement],
classOf[b.CompileReport]
)

expect(taskFinishData.getErrors == 6)

val diagnosticsByPath = taskMessages.head.taskMessages.groupBy(_.getTextDocument)

for ((_, values) <- diagnosticsByPath) {
expect(values.head.getReset)
expect(values.tail.forall(!_.getReset))
}
}
}
}

private def checkIfBloopProjectIsInitialised(
root: os.Path,
buildTargetsResp: b.WorkspaceBuildTargetsResult
Expand Down
Expand Up @@ -43,6 +43,29 @@ class TestBspClient extends b.BuildClient {
}
}

case class TaskStartFinishDiagnostics(
startParams: b.TaskStartParams,
finishParams: b.TaskFinishParams,
taskMessages: Seq[b.PublishDiagnosticsParams]
)

def getTaskDiagnostics(): Seq[TaskStartFinishDiagnostics] =
lock.synchronized {
messages0.foldRight(
Option.empty[TaskStartFinishDiagnostics],
List.empty[TaskStartFinishDiagnostics]
) {
case (msg: b.TaskFinishParams, (None, acc)) =>
(Some(TaskStartFinishDiagnostics(null, msg, Nil)), acc)
case (msg: b.TaskStartParams, (Some(tsfm), acc))
if tsfm.finishParams.getTaskId == msg.getTaskId =>
(None, tsfm.copy(startParams = msg) :: acc)
case (msg: b.PublishDiagnosticsParams, (Some(tsfm), acc)) =>
(Some(tsfm.copy(taskMessages = msg +: tsfm.taskMessages)), acc)
case (_, accTuple) => accTuple
}._2
}

def onBuildLogMessage(params: b.LogMessageParams): Unit =
addMessage(params)
def onBuildShowMessage(params: b.ShowMessageParams): Unit =
Expand Down

0 comments on commit f64512a

Please sign in to comment.