Merge pull request sbt#6854 from eed3si9n/bport/6847

[1.7.x] Do not fire `build/publishDiagnostics` if there are (and were) no problems

Author: eed3si9n

main/src/main/scala/sbt/Defaults.scala

@@ -2374,10 +2374,11 @@ object Defaults extends BuildCommon {
     val ci = (compile / compileInputs).value
     val ping = earlyOutputPing.value
     val reporter = (compile / bspReporter).value
+    val prevAnalysis = previousCompile.value.analysis.toOption.getOrElse(Analysis.empty)
     BspCompileTask.compute(bspTargetIdentifier.value, thisProjectRef.value, configuration.value) {
       task =>
         // TODO - Should readAnalysis + saveAnalysis be scoped by the compile task too?
-        compileIncrementalTaskImpl(task, s, ci, ping, reporter)
+        compileIncrementalTaskImpl(task, s, ci, ping, reporter, prevAnalysis)
     }
   }
   private val incCompiler = ZincUtil.defaultIncrementalCompiler
@@ -2392,7 +2393,7 @@ object Defaults extends BuildCommon {
         val result0 = incCompiler
           .asInstanceOf[sbt.internal.inc.IncrementalCompilerImpl]
           .compileAllJava(in, s.log)
-        reporter.sendSuccessReport(result0.analysis())
+        reporter.sendSuccessReport(result0.analysis(), Analysis.empty, false)
         result0.withHasModified(result0.hasModified || r.hasModified)
       } else r
     } catch {
@@ -2406,7 +2407,8 @@ object Defaults extends BuildCommon {
       s: TaskStreams,
       ci: Inputs,
       promise: PromiseWrap[Boolean],
-      reporter: BuildServerReporter
+      reporter: BuildServerReporter,
+      prev: CompileAnalysis
   ): CompileResult = {
     lazy val x = s.text(ExportStream)
     def onArgs(cs: Compilers) = {
@@ -2428,7 +2430,11 @@ object Defaults extends BuildCommon {
       .withSetup(onProgress(setup))
     try {
       val result = incCompiler.compile(i, s.log)
-      reporter.sendSuccessReport(result.getAnalysis)
+      reporter.sendSuccessReport(
+        result.getAnalysis,
+        prev,
+        BuildServerProtocol.shouldReportAllPreviousProblems(task.targetId)
+      )
       result
     } catch {
       case e: Throwable =>

main/src/main/scala/sbt/internal/server/BuildServerProtocol.scala

@@ -39,11 +39,26 @@ import scala.collection.mutable
 import scala.util.control.NonFatal
 import scala.util.{ Failure, Success, Try }
 import scala.annotation.nowarn
+import scala.collection.concurrent.TrieMap
 import sbt.testing.Framework
 
 object BuildServerProtocol {
   import sbt.internal.bsp.codec.JsonProtocol._
 
+  /**
+   * Keep track of those projects that were compiled at least once so that we can
+   * decide to enable fresh reporting for projects that are compiled for the first time.
+   *
+   * see: https://github.com/scalacenter/bloop/issues/726
+   */
+  private val compiledTargetsAtLeastOnce = new TrieMap[bsp.BuildTargetIdentifier, Boolean]()
+  def shouldReportAllPreviousProblems(id: bsp.BuildTargetIdentifier): Boolean = {
+    compiledTargetsAtLeastOnce.putIfAbsent(id, true) match {
+      case Some(_) => false
+      case None    => true
+    }
+  }
+
   private val capabilities = BuildServerCapabilities(
     CompileProvider(BuildServerConnection.languages),
     TestProvider(BuildServerConnection.languages),
@@ -354,6 +369,7 @@ object BuildServerProtocol {
           case r if r.method == Method.Initialize =>
             val params = Converter.fromJson[InitializeBuildParams](json(r)).get
             checkMetalsCompatibility(semanticdbEnabled, semanticdbVersion, params, callback.log)
+            compiledTargetsAtLeastOnce.clear()
 
             val response = InitializeBuildResult(
               "sbt",

main/src/main/scala/sbt/internal/server/BuildServerReporter.scala

@@ -38,7 +38,11 @@ sealed trait BuildServerReporter extends Reporter {
 
   protected def publishDiagnostic(problem: Problem): Unit
 
-  def sendSuccessReport(analysis: CompileAnalysis): Unit
+  def sendSuccessReport(
+      analysis: CompileAnalysis,
+      prev: CompileAnalysis,
+      reportAllPreviousProblems: Boolean
+  ): Unit
 
   def sendFailureReport(sources: Array[VirtualFile]): Unit
 
@@ -86,20 +90,31 @@ final class BuildServerReporterImpl(
     if (ref.id().contains("<")) None
     else Some(converter.toPath(ref))
 
-  override def sendSuccessReport(analysis: CompileAnalysis): Unit = {
+  override def sendSuccessReport(
+      analysis: CompileAnalysis,
+      prev: CompileAnalysis,
+      reportAllPreviousProblems: Boolean
+  ): Unit = {
+    val prevInfos = prev.readSourceInfos().getAllSourceInfos().asScala
     for {
       (source, infos) <- analysis.readSourceInfos.getAllSourceInfos.asScala
       filePath <- toSafePath(source)
     } {
-      val diagnostics = infos.getReportedProblems.toSeq.flatMap(toDiagnostic)
-      val params = PublishDiagnosticsParams(
-        textDocument = TextDocumentIdentifier(filePath.toUri),
-        buildTarget,
-        originId = None,
-        diagnostics.toVector,
-        reset = true
-      )
-      exchange.notifyEvent("build/publishDiagnostics", params)
+      val prevProblems = prevInfos.get(source).map(_.getReportedProblems()).getOrElse(Array.empty)
+      val dontPublish = prevProblems.length == 0 && infos.getReportedProblems().length == 0
+      val shouldPublish = reportAllPreviousProblems || !dontPublish
+
+      if (shouldPublish) {
+        val diagnostics = infos.getReportedProblems.toSeq.flatMap(toDiagnostic)
+        val params = PublishDiagnosticsParams(
+          textDocument = TextDocumentIdentifier(filePath.toUri),
+          buildTarget,
+          originId = None,
+          diagnostics.toVector,
+          reset = true
+        )
+        exchange.notifyEvent("build/publishDiagnostics", params)
+      }
     }
   }
 
@@ -179,7 +194,11 @@ final class BuildServerForwarder(
     protected override val underlying: Reporter
 ) extends BuildServerReporter {
 
-  override def sendSuccessReport(analysis: CompileAnalysis): Unit = ()
+  override def sendSuccessReport(
+      analysis: CompileAnalysis,
+      prev: CompileAnalysis,
+      reportAllPreviousProblems: Boolean
+  ): Unit = ()
 
   override def sendFailureReport(sources: Array[VirtualFile]): Unit = ()
 

server-test/src/test/scala/testpkg/BuildServerTest.scala

@@ -125,10 +125,25 @@ object BuildServerTest extends AbstractServerTest {
       s.contains(""""message":"Compiling runAndTest (100%)"""")
     })
 
+    assert(svr.waitForString(60.seconds) { s =>
+      s.contains("build/publishDiagnostics")
+      s.contains(""""diagnostics":[]""")
+    })
+
     assert(svr.waitForString(60.seconds) { s =>
       s.contains("build/taskFinish") &&
       s.contains(""""message":"Compiled runAndTest"""")
     })
+
+    svr.sendJsonRpc(
+      s"""{ "jsonrpc": "2.0", "id": "34", "method": "buildTarget/compile", "params": {
+         |  "targets": [{ "uri": "$buildTarget" }]
+         |} }""".stripMargin
+    )
+
+    assert(!svr.waitForString(30.seconds) { s =>
+      s.contains("build/publishDiagnostics")
+    }, "shouldn't send publishDiagnostics if there's no change in diagnostics")
   }
 
   test("buildTarget/scalacOptions") { _ =>