From 80e87531d35aaf730d4eead08901657bfb774017 Mon Sep 17 00:00:00 2001 From: Rikito Taniguchi Date: Fri, 18 Mar 2022 02:23:08 +0900 Subject: [PATCH 1/3] Don't fire publishDiagnostic if there's no problems both in current and previous compilation --- main/src/main/scala/sbt/Defaults.scala | 8 +++-- .../internal/server/BuildServerReporter.scala | 30 +++++++++++-------- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/main/src/main/scala/sbt/Defaults.scala b/main/src/main/scala/sbt/Defaults.scala index facf6637a..2e3f463f1 100644 --- a/main/src/main/scala/sbt/Defaults.scala +++ b/main/src/main/scala/sbt/Defaults.scala @@ -2310,10 +2310,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 @@ -2342,7 +2343,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) = { @@ -2364,7 +2366,7 @@ object Defaults extends BuildCommon { .withSetup(onProgress(setup)) try { val result = incCompiler.compile(i, s.log) - reporter.sendSuccessReport(result.getAnalysis) + reporter.sendSuccessReport(result.getAnalysis, prev) result } catch { case e: Throwable => diff --git a/main/src/main/scala/sbt/internal/server/BuildServerReporter.scala b/main/src/main/scala/sbt/internal/server/BuildServerReporter.scala index dba366578..8184b090c 100644 --- a/main/src/main/scala/sbt/internal/server/BuildServerReporter.scala +++ b/main/src/main/scala/sbt/internal/server/BuildServerReporter.scala @@ -38,7 +38,7 @@ sealed trait BuildServerReporter extends Reporter { protected def publishDiagnostic(problem: Problem): Unit - def sendSuccessReport(analysis: CompileAnalysis): Unit + def sendSuccessReport(analysis: CompileAnalysis, prev: CompileAnalysis): Unit def sendFailureReport(sources: Array[VirtualFile]): Unit @@ -86,20 +86,26 @@ 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): 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 + + if (!dontPublish) { + 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 +185,7 @@ final class BuildServerForwarder( protected override val underlying: Reporter ) extends BuildServerReporter { - override def sendSuccessReport(analysis: CompileAnalysis): Unit = () + override def sendSuccessReport(analysis: CompileAnalysis, prev: CompileAnalysis): Unit = () override def sendFailureReport(sources: Array[VirtualFile]): Unit = () From f118d2d73b85a883b37015be2bfd20086a67efeb Mon Sep 17 00:00:00 2001 From: Rikito Taniguchi Date: Fri, 18 Mar 2022 23:04:07 +0900 Subject: [PATCH 2/3] Re-publish warnings on BSP server startup Imitate https://github.com/scalacenter/bloop/commit/8aaf828b033c081f420c2c54879b0e28eeeae9df --- main/src/main/scala/sbt/Defaults.scala | 8 +++++-- .../internal/server/BuildServerProtocol.scala | 16 ++++++++++++++ .../internal/server/BuildServerReporter.scala | 21 +++++++++++++++---- 3 files changed, 39 insertions(+), 6 deletions(-) diff --git a/main/src/main/scala/sbt/Defaults.scala b/main/src/main/scala/sbt/Defaults.scala index 2e3f463f1..4b0fc2ad9 100644 --- a/main/src/main/scala/sbt/Defaults.scala +++ b/main/src/main/scala/sbt/Defaults.scala @@ -2329,7 +2329,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 { @@ -2366,7 +2366,11 @@ object Defaults extends BuildCommon { .withSetup(onProgress(setup)) try { val result = incCompiler.compile(i, s.log) - reporter.sendSuccessReport(result.getAnalysis, prev) + reporter.sendSuccessReport( + result.getAnalysis, + prev, + BuildServerProtocol.shouldReportAllPreviousProblems(task.targetId) + ) result } catch { case e: Throwable => diff --git a/main/src/main/scala/sbt/internal/server/BuildServerProtocol.scala b/main/src/main/scala/sbt/internal/server/BuildServerProtocol.scala index 0c6655e2d..1cfae2054 100644 --- a/main/src/main/scala/sbt/internal/server/BuildServerProtocol.scala +++ b/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", diff --git a/main/src/main/scala/sbt/internal/server/BuildServerReporter.scala b/main/src/main/scala/sbt/internal/server/BuildServerReporter.scala index 8184b090c..98d59d645 100644 --- a/main/src/main/scala/sbt/internal/server/BuildServerReporter.scala +++ b/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, prev: CompileAnalysis): Unit + def sendSuccessReport( + analysis: CompileAnalysis, + prev: CompileAnalysis, + reportAllPreviousProblems: Boolean + ): Unit def sendFailureReport(sources: Array[VirtualFile]): Unit @@ -86,7 +90,11 @@ final class BuildServerReporterImpl( if (ref.id().contains("<")) None else Some(converter.toPath(ref)) - override def sendSuccessReport(analysis: CompileAnalysis, prev: 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 @@ -94,8 +102,9 @@ final class BuildServerReporterImpl( } { val prevProblems = prevInfos.get(source).map(_.getReportedProblems()).getOrElse(Array.empty) val dontPublish = prevProblems.length == 0 && infos.getReportedProblems().length == 0 + val shouldPublish = reportAllPreviousProblems || !dontPublish - if (!dontPublish) { + if (shouldPublish) { val diagnostics = infos.getReportedProblems.toSeq.flatMap(toDiagnostic) val params = PublishDiagnosticsParams( textDocument = TextDocumentIdentifier(filePath.toUri), @@ -185,7 +194,11 @@ final class BuildServerForwarder( protected override val underlying: Reporter ) extends BuildServerReporter { - override def sendSuccessReport(analysis: CompileAnalysis, prev: CompileAnalysis): Unit = () + override def sendSuccessReport( + analysis: CompileAnalysis, + prev: CompileAnalysis, + reportAllPreviousProblems: Boolean + ): Unit = () override def sendFailureReport(sources: Array[VirtualFile]): Unit = () From 893cfbba3976a876e72608848cc5a1a0db1bb50c Mon Sep 17 00:00:00 2001 From: Rikito Taniguchi Date: Mon, 21 Mar 2022 21:35:11 +0900 Subject: [PATCH 3/3] Add test to verify server doesn't send notifications --- .../src/test/scala/testpkg/BuildServerTest.scala | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/server-test/src/test/scala/testpkg/BuildServerTest.scala b/server-test/src/test/scala/testpkg/BuildServerTest.scala index 9cbe43980..a6d900a16 100644 --- a/server-test/src/test/scala/testpkg/BuildServerTest.scala +++ b/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") { _ =>