mirror of https://github.com/sbt/sbt.git
[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.
This commit is contained in:
parent
4766e4370d
commit
4ed16c96ce
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
},
|
||||
|
|
|
|||
|
|
@ -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 = ()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue