From 4ed16c96ce051630234ca5c43fbd1ba7619a1414 Mon Sep 17 00:00:00 2001 From: Anatolii Kmetiuk Date: Sat, 27 Jun 2026 01:36:42 +0900 Subject: [PATCH] [2.x] Fix publishDiagnostics propagation (#9376) **Problem** Test is based on https://github.com/sbt/sbt/issues/9345#issuecomment-4718229113 which gives us the following sequence: 1. Metals sends buildTarget/compile. 2. sbt publishes real non-empty diagnostics. 3. Metals sends buildTarget/scalaMainClasses. 4. During that request, sbt emits build/publishDiagnostics with diagnostics: [] and reset: true. 5. The following build/taskFinish still reports errors: 1. Previously, errors for diagnostics reporting via bsp were collected from a live compilation run. In the sequence above, that is triggered by buildTarget/compile. Then, buildTarget/scalaMainClasses does not trigger such a run for the second time, it uses the cached compilation result. Therefore, the diagnostics is not populated. **Solution** The proposed fix modifies sendFailureReport to accept an optional CompileFailed object that contains the diagnostics even in case the actual compiler did not run because the cached result was used. If no problems were found for a file via default means, this CompileFailed object is queried to see if it has any information about problems in a given file. --- build.sbt | 4 + main/src/main/scala/sbt/Defaults.scala | 2 +- .../internal/server/BuildServerReporter.scala | 39 +++++++- .../test/scala/testpkg/BuildServerTest.scala | 92 +++++++++++++++++++ 4 files changed, 133 insertions(+), 4 deletions(-) diff --git a/build.sbt b/build.sbt index 427cc5eb0..796972723 100644 --- a/build.sbt +++ b/build.sbt @@ -801,6 +801,10 @@ lazy val mainProj = (project in file("main")) exclude[DirectMissingMethodProblem]("sbt.internal.ClassStamper.stampVf"), exclude[DirectMissingMethodProblem]("sbt.internal.CompileInputs2.*"), exclude[DirectMissingMethodProblem]("sbt.internal.IncrementalTest.cacheInput"), + // Added optional CompileFailed context for BSP failure diagnostics (sbt#9345) + exclude[ReversedMissingMethodProblem]( + "sbt.internal.server.BuildServerReporter.sendFailureReport" + ), ), ) .dependsOn(lmCore, lmCoursierShadedPublishing) diff --git a/main/src/main/scala/sbt/Defaults.scala b/main/src/main/scala/sbt/Defaults.scala index f89587923..07e22218f 100644 --- a/main/src/main/scala/sbt/Defaults.scala +++ b/main/src/main/scala/sbt/Defaults.scala @@ -2259,7 +2259,7 @@ object Defaults extends BuildCommon with DefExtra { res case Result.Inc(cause) => val compileFailed = cause.directCause.collect { case c: CompileFailed => c } - reporter.sendFailureReport(ci.options.sources) + reporter.sendFailureReport(ci.options.sources, compileFailed) bspTask.notifyFailure(compileFailed) throw cause }, diff --git a/main/src/main/scala/sbt/internal/server/BuildServerReporter.scala b/main/src/main/scala/sbt/internal/server/BuildServerReporter.scala index c7748ee71..b959c8231 100644 --- a/main/src/main/scala/sbt/internal/server/BuildServerReporter.scala +++ b/main/src/main/scala/sbt/internal/server/BuildServerReporter.scala @@ -14,6 +14,7 @@ import sbt.internal.util.ManagedLogger import sbt.internal.server.BuildServerProtocol.BspCompileState import xsbti.compile.CompileAnalysis import xsbti.{ + CompileFailed, FileConverter, Problem, Reporter, @@ -47,6 +48,11 @@ sealed trait BuildServerReporter extends Reporter { def sendFailureReport(sources: Array[VirtualFile]): Unit + def sendFailureReport( + sources: Array[VirtualFile], + failure: Option[CompileFailed] + ): Unit + override def reset(): Unit = underlying.reset() override def hasErrors: Boolean = underlying.hasErrors @@ -113,9 +119,30 @@ final class BuildServerReporterImpl( notifyFirstReport() } - override def sendFailureReport(sources: Array[VirtualFile]): Unit = { + override def sendFailureReport(sources: Array[VirtualFile]): Unit = + sendFailureReport(sources, None) + + override def sendFailureReport( + sources: Array[VirtualFile], + failure: Option[CompileFailed] + ): Unit = { + val fallbackByFile: Map[Path, Vector[Problem]] = failure match + case Some(failed) => + failed + .problems() + .toVector + .flatMap { problem => + problem.position.sourcePath.toScala.map { id => + converter.toPath(VirtualFileRef.of(id)) -> problem + } + } + .groupMap(_._1)(_._2) + case None => + Map.empty + for (source <- sources) { - val problems = problemsByFile.getOrElse(converter.toPath(source), Vector.empty) + val path = converter.toPath(source) + val problems = problemsByFile.getOrElse(path, fallbackByFile.getOrElse(path, Vector.empty)) sendReport(source, problems) } notifyFirstReport() @@ -245,7 +272,13 @@ final class BuildServerForwarder( analysis: CompileAnalysis, ): Unit = () - override def sendFailureReport(sources: Array[VirtualFile]): Unit = () + override def sendFailureReport(sources: Array[VirtualFile]): Unit = + sendFailureReport(sources, None) + + override def sendFailureReport( + sources: Array[VirtualFile], + failure: Option[CompileFailed] + ): Unit = () protected override def publishDiagnostic(problem: Problem): Unit = () } diff --git a/server-test/src/test/scala/testpkg/BuildServerTest.scala b/server-test/src/test/scala/testpkg/BuildServerTest.scala index 61b75c30f..679f83f0a 100644 --- a/server-test/src/test/scala/testpkg/BuildServerTest.scala +++ b/server-test/src/test/scala/testpkg/BuildServerTest.scala @@ -11,6 +11,7 @@ import sbt.internal.bsp.* import sbt.internal.bsp.codec.JsonProtocol.given import sbt.internal.langserver.{ ErrorCodes, LogMessageParams } import sbt.internal.langserver.codec.JsonProtocol.given +import sbt.internal.protocol.JsonRpcNotificationMessage import sbt.IO import sjsonnew.JsonWriter import sjsonnew.support.scalajson.unsafe.{ CompactPrinter, Converter } @@ -18,7 +19,9 @@ import sjsonnew.support.scalajson.unsafe.{ CompactPrinter, Converter } import java.io.File import java.net.URI import java.nio.file.{ Files, Paths } +import java.util.concurrent.TimeoutException import scala.concurrent.duration.* +import scala.util.{ Failure, Success } // starts svr using server-test/buildserver and perform custom server tests class BuildServerTest extends AbstractServerTest { @@ -278,6 +281,95 @@ class BuildServerTest extends AbstractServerTest { } } + // 1. Cause a real compile error and observe non-empty diagnostics. + // 2. Request buildTarget/scalaMainClasses. + // 3. Watch any notifications emitted while that request is processed. + // 4. Fail if one of them is the forbidden empty diagnostic reset. + test("buildTarget/scalaMainClasses does not clear compile diagnostics (#9345)") { + def isForbiddenDiagnosticReset(n: JsonRpcNotificationMessage): Boolean = + n.method == "build/publishDiagnostics" && + n.params + .flatMap(Converter.fromJson[PublishDiagnosticsParams](_).toOption) + .exists(p => + p.textDocument.uri.toString.contains("Diagnostics.scala") && + p.reset && + p.diagnostics.isEmpty + ) + + def failIfForbiddenDiagnosticReset(n: JsonRpcNotificationMessage): Unit = + if (isForbiddenDiagnosticReset(n)) + fail( + "buildTarget/scalaMainClasses must not publish empty reset=true " + + "diagnostics for Diagnostics.scala after a failed compile (#9345)" + ) + + def drainQueuedNotificationsAndFailOnForbiddenReset(): Unit = + svr.session.waitForNotificationMsg(Duration.Zero)(_ => true) match { + case Success(n) => + failIfForbiddenDiagnosticReset(n) + drainQueuedNotificationsAndFailOnForbiddenReset() + case Failure(_: TimeoutException) => () + case Failure(e) => throw e + } + + val buildTarget = buildTargetUri("diagnostics", "Compile") + val mainFile = new File(svr.baseDirectory, "diagnostics/src/main/scala/Diagnostics.scala") + val original = IO.read(mainFile) + try { + IO.write( + mainFile, + """|object Diagnostics { + | private val a: Int = "" + |}""".stripMargin + ) + + val compileId = compile(buildTarget) + val res = svr.session.waitForResultInResponseMsg[BspCompileResult](30.seconds, compileId).get + assert( + res.statusCode == StatusCode.Error, + s"expected StatusCode.Error, got ${res.statusCode}" + ) + + svr.session + .waitForParamsInNotificationMsg[PublishDiagnosticsParams](30.seconds) { p => + p.textDocument.uri.toString.contains("Diagnostics.scala") && + p.diagnostics.exists(d => + d.severity.contains(DiagnosticSeverity.Error) && + (d.message.contains("type mismatch") || + d.message.contains("Found:") || + d.message.contains("Required:")) + ) + } + .get + + svr.session + .waitForParamsInNotificationMsg[TaskFinishParams](30.seconds) { p => + p.message.contains("Compiled diagnostics") + } + .get + + val targets = Vector(BuildTargetIdentifier(buildTarget)) + val mainClassesId = + sendRequest("buildTarget/scalaMainClasses", ScalaMainClassesParams(targets, None)) + + svr.session + .waitForNotificationMsg(30.seconds) { n => + failIfForbiddenDiagnosticReset(n) + n.method == "build/taskFinish" && + n.params + .flatMap(Converter.fromJson[TaskFinishParams](_).toOption) + .exists(_.message.contains("Compiled diagnostics")) + } + .get + + svr.session.waitForResponseMsg(30.seconds, mainClassesId).get + + drainQueuedNotificationsAndFailOnForbiddenReset() + } finally { + IO.write(mainFile, original) + } + } + test("buildTarget/compile: Java diagnostics") { val buildTarget = buildTargetUri("javaProj", "Compile")