Skip to content

Commit

Permalink
bugfix: Fix publishing warnings
Browse files Browse the repository at this point in the history
Previously, no (Scala) warnings would be reported since they would come inside stderr in a progress build event. Now, they are handled via that event and cleared correctly when needed.
  • Loading branch information
tgodzik committed Mar 26, 2024
1 parent b1655d9 commit 4b25917
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 25 deletions.
Expand Up @@ -100,6 +100,9 @@ class BepServer(

private fun processProgressEvent(event: BuildEventStreamProtos.BuildEvent) {
if (event.hasProgress()) {
if (target != null) {
processDiagnosticText(event.progress.stderr, target.uri, true)
}
// TODO https://youtrack.jetbrains.com/issue/BAZEL-622
// bepLogger.onProgress(event.getProgress());
}
Expand Down Expand Up @@ -185,32 +188,34 @@ class BepServer(
try {
val path = Paths.get(URI.create(actionEvent.stderr.uri))
val stdErrText = Files.readString(path)
processDiagnosticText(stdErrText, label)
processDiagnosticText(stdErrText, label, false)
} catch (e: FileSystemNotFoundException) {
LOGGER.warn(e)
} catch (e: IOException) {
LOGGER.warn(e)
}
}
BuildEventStreamProtos.File.FileCase.CONTENTS -> {
processDiagnosticText(actionEvent.stderr.contents.toStringUtf8(), label)
processDiagnosticText(actionEvent.stderr.contents.toStringUtf8(), label, false)
}
else -> {
processDiagnosticText("", label)
processDiagnosticText("", label, false)
}
}
}

private fun processDiagnosticText(stdErrText: String, targetLabel: String) {
val events =
diagnosticsService.extractDiagnostics(
stdErrText, targetLabel, startedEvents.first.value
)
events.forEach(Consumer { publishDiagnosticsParams: PublishDiagnosticsParams? ->
bspClient.onBuildPublishDiagnostics(
publishDiagnosticsParams
)
})
private fun processDiagnosticText(stdErrText: String, targetLabel: String, diagnosticsFromProgress: Boolean) {
if (startedEvents.isNotEmpty() && stdErrText.isNotEmpty()) {
val events =
diagnosticsService.extractDiagnostics(
stdErrText, targetLabel, startedEvents.first.value, diagnosticsFromProgress
)
events.forEach(Consumer { publishDiagnosticsParams: PublishDiagnosticsParams? ->
bspClient.onBuildPublishDiagnostics(
publishDiagnosticsParams
)
})
}
}

private fun consumeCompletedEvent(event: BuildEventStreamProtos.BuildEvent) {
Expand Down
Expand Up @@ -2,9 +2,9 @@ package org.jetbrains.bsp.bazel.server.diagnostics

class DiagnosticsParser {

fun parse(bazelOutput: String, target: String): List<Diagnostic> {
fun parse(bazelOutput: String, target: String, onlyKnownFiles: Boolean): List<Diagnostic> {
val output = prepareOutput(bazelOutput, target)
val diagnostics = collectDiagnostics(output)
val diagnostics = collectDiagnostics(output, onlyKnownFiles)
return deduplicate(diagnostics)
}

Expand All @@ -14,7 +14,7 @@ class DiagnosticsParser {
return Output(relevantLines, target)
}

private fun collectDiagnostics(output: Output): List<Diagnostic> {
private fun collectDiagnostics(output: Output, onlyKnownFiles: Boolean): List<Diagnostic> {
val diagnostics = mutableListOf<Diagnostic>()
while (output.nonEmpty()) {
for (parser in Parsers) {
Expand All @@ -26,7 +26,7 @@ class DiagnosticsParser {
}
}

if (diagnostics.isEmpty()) {
if (diagnostics.isEmpty() && !onlyKnownFiles) {
diagnostics.add(
Diagnostic(
position = Position(0, 0),
Expand Down
@@ -1,11 +1,9 @@
package org.jetbrains.bsp.bazel.server.diagnostics

import ch.epfl.scala.bsp4j.BuildTargetIdentifier
import ch.epfl.scala.bsp4j.Diagnostic
import ch.epfl.scala.bsp4j.PublishDiagnosticsParams
import ch.epfl.scala.bsp4j.TextDocumentIdentifier
import java.nio.file.Path
import java.util.concurrent.ConcurrentHashMap
import java.util.Collections

/**
Expand All @@ -17,23 +15,31 @@ class DiagnosticsService(workspaceRoot: Path, private val hasAnyProblems: Mutabl

private val parser = DiagnosticsParser()
private val mapper = DiagnosticBspMapper(workspaceRoot)
/* Warnings are reported before the target completed event, when everything is cleared. so we want to avoid removing them */
private val updatedInThisRun = mutableSetOf<PublishDiagnosticsParams>()

fun getBspState() = Collections.unmodifiableMap(hasAnyProblems)

fun extractDiagnostics(
bazelOutput: String,
targetLabel: String,
originId: String?
originId: String?,
diagnosticsFromProgress: Boolean
): List<PublishDiagnosticsParams> {
val parsedDiagnostics = parser.parse(bazelOutput, targetLabel)
val parsedDiagnostics = parser.parse(bazelOutput, targetLabel, diagnosticsFromProgress)
val events = mapper.createDiagnostics(parsedDiagnostics, originId)
if (diagnosticsFromProgress) updatedInThisRun.addAll(events)
updateProblemState(events)
return events
}

fun clearFormerDiagnostics(targetLabel: String): List<PublishDiagnosticsParams> {
val docs = hasAnyProblems[targetLabel]
hasAnyProblems.remove(targetLabel)
if (updatedInThisRun.isNotEmpty()) {
hasAnyProblems[targetLabel] = updatedInThisRun.map { it.textDocument }.toSet()
updatedInThisRun.clear()
}
return docs
?.map { PublishDiagnosticsParams(it, BuildTargetIdentifier(targetLabel), emptyList(), true)}
.orEmpty()
Expand Down
Expand Up @@ -538,7 +538,7 @@ class DiagnosticsServiceTest {
diagnosticsBeforeError.shouldBeEmpty()

// Extract the diagnostics from BEP, there's an error in `Test.scala` of "//path/to/package:test".
service.extractDiagnostics(output, "//path/to/package:test", null)
service.extractDiagnostics(output, "//path/to/package:test", null, false)

// Assert that state is updated
service.getBspState().keys.shouldHaveSize(1)
Expand Down Expand Up @@ -626,8 +626,8 @@ class DiagnosticsServiceTest {
)
)

service.extractDiagnostics(output1, "//path/to/package:test", null)
service.extractDiagnostics(output2, "//path/to/package2:test", null)
service.extractDiagnostics(output1, "//path/to/package:test", null, false)
service.extractDiagnostics(output2, "//path/to/package2:test", null, false)

// Assert that state is updated
service.getBspState().keys shouldContainExactlyInAnyOrder listOf(
Expand Down Expand Up @@ -675,6 +675,6 @@ class DiagnosticsServiceTest {
}

private fun extractDiagnostics(output: String, buildTarget: String): List<PublishDiagnosticsParams>? {
return DiagnosticsService(workspacePath, ConcurrentHashMap()).extractDiagnostics(output, buildTarget, null)
return DiagnosticsService(workspacePath, ConcurrentHashMap()).extractDiagnostics(output, buildTarget, null, false)
}
}

0 comments on commit 4b25917

Please sign in to comment.