Skip to content

Commit

Permalink
bugfix: Don't republish old errors on successful compilation
Browse files Browse the repository at this point in the history
We need to report all problems if a BSP client just connected to the server and never compiled.
`reportAllPreviousProblems` will be set to true in that case.

However, if previously we had an error, which was reverted and got back to the state before the
error, we will get a successfull noop compilation, which will not produce any problems.

In this case we don't want to republish everything again, since diagnostics will contain the
error which was reverted.

Connected to VirtusLab/scala-cli#2226 where CLI always connected anew.
  • Loading branch information
tgodzik committed Sep 11, 2024
1 parent b9170ed commit 0727b16
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 3 deletions.
25 changes: 22 additions & 3 deletions frontend/src/main/scala/bloop/reporter/BspProjectReporter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -301,14 +301,31 @@ final class BspProjectReporter(
): Unit = {
val problemsInPreviousAnalysisPerFile = Reporter.groupProblemsByFile(previousSuccessfulProblems)

/**
* We need to report all problems if a BSP client just connected to the server and never compiled.
* `reportAllPreviousProblems` will be set to true in that case.
*
* However, if previously we had an error, which was reverted and got back to the state before the
* error, we will get a successfull noop compilation, which will not produce any problems.
*
* In this case we don't want to republish everything again, since diagnostics will contain the
* error which was reverted.
*
* Connected to https://github.com/VirtusLab/scala-cli/issues/2226 where CLI always connected anew.
*/
def shouldPublishAllProblems =
reportAllPreviousProblems && !wasPreviousSuccessful && code != bsp.StatusCode.Ok
def mockNoOpCompileEventsAndEnd: Option[CompilationEvent.EndCompilation] = {
recentlyReportProblemsPerFile.foreach {
case (sourceFile, problemsPerFile) if reportAllPreviousProblems =>
case (sourceFile, problemsPerFile) if shouldPublishAllProblems =>
reportAllProblems(sourceFile, problemsPerFile)
case (sourceFile, problemsPerFile) =>
problemsInPreviousAnalysisPerFile.get(sourceFile) match {
case Some(problemsInPreviousAnalysis) =>
if (problemsInPreviousAnalysis.map(_.problem) == problemsPerFile.map(_.problem)) {
if (
problemsInPreviousAnalysis
.map(_.problem) == problemsPerFile.map(_.problem) && !reportAllPreviousProblems
) {
// If problems are the same, diagnostics in the editor are up-to-date, do nothing
()
} else {
Expand All @@ -322,8 +339,10 @@ final class BspProjectReporter(
}
}

case None =>
// there is nothing to clear if we need to rereport all problems
case None if !reportAllPreviousProblems =>
logger.noDiagnostic(CompilationEvent.NoDiagnostic(project.bspUri, sourceFile))
case _ =>
}
}
if (wasPreviousSuccessful) None
Expand Down
59 changes: 59 additions & 0 deletions frontend/src/test/scala/bloop/bsp/BspCompileSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -748,6 +748,65 @@ class BspCompileSpec(
}
}

test("no-op compile and not publish diagnostics from a previous failed CLI compilation") {
TestUtil.withinWorkspace { workspace =>
object Sources {
val `App.scala` =
"""/main/scala/App.scala
|object App {
| val a: String = ""
|}
""".stripMargin
}

val bspLogger = new RecordingLogger(ansiCodesSupported = false)
val `A` = TestProject(workspace, "a", List(Sources.`App.scala`))
val projects = List(`A`)

// compile successfully in a separate bsp connection and then break
loadBspState(workspace, projects, bspLogger) { state =>
val compiledState = state.compile(`A`)
assertExitStatus(compiledState, ExitStatus.Ok)
assertValidCompilationState(compiledState, projects)

writeFile(
`A`.srcFor("/main/scala/App.scala"),
"""|object App {
| val a: String = 1
|}
""".stripMargin
)
val secondCliCompiledState = compiledState.compile(`A`)
assertExitStatus(secondCliCompiledState, ExitStatus.CompilationError)
assertValidCompilationState(secondCliCompiledState, projects)
}

// connecting and compiling should not get any previous and no longer valid diagnostics
loadBspState(workspace, projects, bspLogger) { state =>
// Remove the error and get back to the previous state
writeFile(
`A`.srcFor("/main/scala/App.scala"),
Sources.`App.scala`
)
val compiledState = state.compile(`A`)
assertExitStatus(compiledState, ExitStatus.Ok)
assertValidCompilationState(compiledState, projects)

assertNoDiff(
compiledState.lastDiagnostics(`A`),
"""|#1: task start 1
| -> Msg: Start no-op compilation for a
| -> Data kind: compile-task
|#1: task finish 1
| -> errors 0, warnings 0
| -> Msg: Compiled 'a'
| -> Data kind: compile-report
|""".stripMargin
)
}
}
}

test("compile incrementally and publish warnings from a previous CLI compilation") {
TestUtil.withinWorkspace { workspace =>
object Sources {
Expand Down

0 comments on commit 0727b16

Please sign in to comment.