Skip to content

Commit

Permalink
Merge pull request #2427 from tgodzik/fix-republish-diags
Browse files Browse the repository at this point in the history
bugfix: Don't republish old errors on successful compilation
  • Loading branch information
tgodzik authored Sep 17, 2024
2 parents 27e1725 + 0727b16 commit 4d9d369
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 4d9d369

Please sign in to comment.