diff --git a/metals/src/main/scala/scala/meta/internal/bsp/ConnectionBspStatus.scala b/metals/src/main/scala/scala/meta/internal/bsp/ConnectionBspStatus.scala index 0aab3d7f22f..d2c748449d4 100644 --- a/metals/src/main/scala/scala/meta/internal/bsp/ConnectionBspStatus.scala +++ b/metals/src/main/scala/scala/meta/internal/bsp/ConnectionBspStatus.scala @@ -1,9 +1,12 @@ package scala.meta.internal.bsp -import java.util.concurrent.atomic.AtomicBoolean +import java.nio.file.Path +import java.util.concurrent.atomic.AtomicReference import scala.meta.internal.metals.BspStatus import scala.meta.internal.metals.Icons +import scala.meta.internal.metals.MetalsEnrichments._ +import scala.meta.internal.metals.ReportContext import scala.meta.internal.metals.ServerCommands import scala.meta.internal.metals.clients.language.MetalsStatusParams import scala.meta.internal.metals.clients.language.StatusType @@ -13,25 +16,94 @@ class ConnectionBspStatus( bspStatus: BspStatus, folderPath: AbsolutePath, icons: Icons, -) { - private val isServerResponsive = new AtomicBoolean(false) - val status: MetalsStatusParams => Unit = bspStatus.status(folderPath, _) - - def connected(serverName: String): Unit = - if (isServerResponsive.compareAndSet(false, true)) - status(ConnectionBspStatus.connectedParams(serverName, icons)) - def noResponse(serverName: String): Unit = - if (isServerResponsive.compareAndSet(true, false)) { - scribe.debug("server liveness monitor detected no response") - status(ConnectionBspStatus.noResponseParams(serverName, icons)) - } +)(implicit rc: ReportContext) { + private val status = new AtomicReference[BspStatusState]( + BspStatusState(Disconnected, None, None, shouldShow = false) + ) + + /** Paths to all bsp error reports from this session (bsp connection). */ + private val currentSessionErrors = new AtomicReference[Set[String]](Set()) + + def connected(serverName: String): Unit = changeState(Connected(serverName)) + + def noResponse(): Unit = { + scribe.debug("server liveness monitor detected no response") + changeState(NoResponse) + } def disconnected(): Unit = { - isServerResponsive.set(false) - status(ConnectionBspStatus.disconnectedParams) + currentSessionErrors.set(Set.empty) + changeState(Disconnected) + } + + def showError(message: String, pathToReport: Path): Unit = { + val updatedSet = + currentSessionErrors.updateAndGet(_ + pathToReport.toUri().toString()) + changeState(ErrorMessage(message), updatedSet) + } + + def onReportsUpdate(): Unit = { + status.get().currentState match { + case ErrorMessage(_) => showState(status.get()) + case _ => + } + } + + def isBuildServerResponsive: Option[Boolean] = + status.get().currentState match { + case NoResponse => Some(false) + case Disconnected => None + case _ => Some(true) + } + + private def changeState( + newState: BspServerState, + errorReports: Set[String] = currentSessionErrors.get(), + ) = { + val newServerState = status.updateAndGet(_.changeState(newState)) + if (newServerState.shouldShow) { + showState(newServerState, errorReports) + } } - def isBuildServerResponsive: Boolean = isServerResponsive.get() + private def showState( + statusState: BspStatusState, + errorReports: Set[String] = currentSessionErrors.get(), + ) = { + val showParams = + statusState.currentState match { + case Disconnected => ConnectionBspStatus.disconnectedParams + case NoResponse => + ConnectionBspStatus.noResponseParams(statusState.serverName, icons) + case Connected(serverName) => + ConnectionBspStatus.connectedParams(serverName, icons) + case ErrorMessage(message) => + val currentSessionReports = syncWithReportContext(errorReports) + if (currentSessionReports.isEmpty) + ConnectionBspStatus.connectedParams(statusState.serverName, icons) + else + ConnectionBspStatus.bspErrorParams( + statusState.serverName, + icons, + message, + currentSessionReports.size, + ) + } + bspStatus.status(folderPath, showParams) + } + + /** + * To get the actual number of error reports we take an intersection + * of this session's error reports with the reports in `.metals/.reports/bloop`, + * this allows for two things: + * 1. When user deletes the report from file system the warning will disappear. + * 2. Error deduplication. When for a perticular error a report already exists, we remove the old report. + * For reports management details look [[scala.meta.internal.metals.StdReporter]] + */ + private def syncWithReportContext(errorReports: Set[String]) = + errorReports.intersect( + rc.bloop.getReports().map(_.toPath.toUri().toString()).toSet + ) } object ConnectionBspStatus { @@ -55,4 +127,80 @@ object ConnectionBspStatus { command = ServerCommands.ConnectBuildServer.id, commandTooltip = "Reconnect.", ).withStatusType(StatusType.bsp) + + def bspErrorParams( + serverName: String, + icons: Icons, + message: String, + errorsNumber: Int, + ): MetalsStatusParams = + MetalsStatusParams( + s"$serverName $errorsNumber ${icons.alert}", + "warn", + show = true, + tooltip = message.trimTo(TOOLTIP_MAX_LENGTH), + command = ServerCommands.RunDoctor.id, + commandTooltip = "Open doctor.", + ).withStatusType(StatusType.bsp) + + private val TOOLTIP_MAX_LENGTH = 150 +} + +trait BspServerState +case object Disconnected extends BspServerState +case class Connected(serverName: String) extends BspServerState +case class ErrorMessage(message: String) extends BspServerState +case object NoResponse extends BspServerState + +case class BspStatusState( + currentState: BspServerState, + lastError: Option[ErrorMessage], + optServerName: Option[String], + shouldShow: Boolean, +) { + val serverName: String = optServerName.getOrElse("bsp") + def changeState( + newState: BspServerState + ): BspStatusState = { + newState match { + case Disconnected if currentState != Disconnected => moveTo(Disconnected) + case NoResponse if currentState != NoResponse => moveTo(NoResponse) + case ErrorMessage(msg) => + currentState match { + case NoResponse => + BspStatusState( + NoResponse, + Some(ErrorMessage(msg)), + optServerName, + shouldShow = false, + ) + case _ => moveTo(ErrorMessage(msg)) + } + case Connected(serverName) => + currentState match { + case Disconnected => moveTo(Connected(serverName)) + case NoResponse => + lastError match { + case Some(error) => moveTo(error) + case _ => moveTo(Connected(serverName)) + } + case _ => this.copy(shouldShow = false) + } + case _ => this.copy(shouldShow = false) + } + } + + def moveTo( + newState: BspServerState + ): BspStatusState = { + val newServerName = + newState match { + case Connected(serverName) => Some(serverName) + case _ => optServerName + } + val lastError = Some(currentState).collect { case error: ErrorMessage => + error + } + BspStatusState(newState, lastError, newServerName, shouldShow = true) + } } diff --git a/metals/src/main/scala/scala/meta/internal/builds/BSPErrorHandler.scala b/metals/src/main/scala/scala/meta/internal/builds/BSPErrorHandler.scala index 9b817dd7bae..546262bdd02 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BSPErrorHandler.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BSPErrorHandler.scala @@ -1,49 +1,26 @@ package scala.meta.internal.builds -import java.nio.file.Files -import java.util.concurrent.atomic.AtomicReference - -import scala.concurrent.ExecutionContext -import scala.concurrent.Future -import scala.util.control.NonFatal +import java.security.MessageDigest import scala.meta.internal.bsp.BspSession -import scala.meta.internal.metals.ClientCommands -import scala.meta.internal.metals.ConcurrentHashSet -import scala.meta.internal.metals.Directories +import scala.meta.internal.bsp.ConnectionBspStatus import scala.meta.internal.metals.MetalsEnrichments._ +import scala.meta.internal.metals.Report +import scala.meta.internal.metals.ReportContext import scala.meta.internal.metals.Tables -import scala.meta.internal.metals.clients.language.MetalsLanguageClient -import scala.meta.io.AbsolutePath - -import org.eclipse.{lsp4j => l} class BspErrorHandler( - languageClient: MetalsLanguageClient, - workspaceFolder: AbsolutePath, currentSession: () => Option[BspSession], tables: Tables, -)(implicit context: ExecutionContext) { - - protected def logsPath: AbsolutePath = - workspaceFolder.resolve(Directories.log) - private val lastError = new AtomicReference[String]("") - private val dismissedErrors = ConcurrentHashSet.empty[String] - - def onError(message: String): Future[Unit] = { - if ( - !tables.dismissedNotifications.BspErrors.isDismissed && - shouldShowBspError && - !dismissedErrors.contains(message) - ) { - val previousError = lastError.getAndSet(message) - if (message != previousError) { - showError(message) - } else Future.successful(()) - } else { - logError(message) - Future.successful(()) - } + bspStatus: ConnectionBspStatus, +)(implicit reportContext: ReportContext) { + def onError(message: String): Unit = { + if (shouldShowBspError) { + for { + report <- createReport(message) + if !tables.dismissedNotifications.BspErrors.isDismissed + } bspStatus.showError(message, report) + } else logError(message) } def shouldShowBspError: Boolean = currentSession().exists(session => @@ -52,96 +29,23 @@ class BspErrorHandler( protected def logError(message: String): Unit = scribe.error(message) - private def showError(message: String): Future[Unit] = { - val bspError = s"${BspErrorHandler.errorInBsp} $message" - logError(bspError) - val params = BspErrorHandler.makeShowMessage(message) - languageClient.showMessageRequest(params).asScala.flatMap { - case BspErrorHandler.goToLogs => - val errorMsgStartLine = - bspError.linesIterator.headOption - .flatMap(findLine(_)) - .getOrElse(0) - Future.successful(gotoLogs(errorMsgStartLine)) - case BspErrorHandler.dismiss => - Future.successful(dismissedErrors.add(message)).ignoreValue - case BspErrorHandler.doNotShowErrors => - Future.successful { - tables.dismissedNotifications.BspErrors.dismissForever - }.ignoreValue - case _ => Future.successful(()) - } - } - - private def findLine(line: String): Option[Int] = - try { - val lineNumber = - Files - .readAllLines(logsPath.toNIO) - .asScala - .lastIndexWhere(_.contains(line)) - if (lineNumber >= 0) Some(lineNumber) else None - } catch { - case NonFatal(_) => None - } - - private def gotoLogs(line: Int) = { - val pos = new l.Position(line, 0) - val location = new l.Location( - logsPath.toURI.toString(), - new l.Range(pos, pos), - ) - languageClient.metalsExecuteClientCommand( - ClientCommands.GotoLocation.toExecuteCommandParams( - ClientCommands.WindowLocation( - location.getUri(), - location.getRange(), - ) + private def createReport(message: String) = { + val id = MessageDigest + .getInstance("MD5") + .digest(message.getBytes) + .map(_.toChar) + .mkString + val sanitized = reportContext.bloop.sanitize(message) + reportContext.bloop.create( + Report( + sanitized.trimTo(20), + s"""|### Bloop error: + | + |$message""".stripMargin, + shortSummary = sanitized.trimTo(100), + path = None, + id = Some(id), ) ) } } - -object BspErrorHandler { - def makeShowMessage( - message: String - ): l.ShowMessageRequestParams = { - val (msg, actions) = - if (message.length() <= MESSAGE_MAX_LENGTH) { - ( - makeShortMessage(message), - List(dismiss, doNotShowErrors), - ) - } else { - ( - makeLongMessage(message), - List(goToLogs, dismiss, doNotShowErrors), - ) - } - val params = new l.ShowMessageRequestParams() - params.setType(l.MessageType.Error) - params.setMessage(msg) - params.setActions(actions.asJava) - params - } - - def makeShortMessage(message: String): String = - s"""|$errorHeader - |$message""".stripMargin - - def makeLongMessage(message: String): String = - s"""|${makeShortMessage(s"${message.take(MESSAGE_MAX_LENGTH)}...")} - |$gotoLogsToSeeFull""".stripMargin - - val goToLogs = new l.MessageActionItem("Go to logs.") - val dismiss = new l.MessageActionItem("Dismiss.") - val doNotShowErrors = new l.MessageActionItem("Stop showing bsp errors.") - - val errorHeader = "Encountered an error in the build server:" - private val gotoLogsToSeeFull = "Go to logs to see the full error" - - val errorInBsp = "Build server error:" - - private val MESSAGE_MAX_LENGTH = 150 - -} diff --git a/metals/src/main/scala/scala/meta/internal/metals/BuildServerConnection.scala b/metals/src/main/scala/scala/meta/internal/metals/BuildServerConnection.scala index 051e8a6a223..fcc150eadca 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/BuildServerConnection.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/BuildServerConnection.scala @@ -478,14 +478,6 @@ class BuildServerConnection private ( } } - def isBuildServerResponsive: Future[Option[Boolean]] = { - val original = connection - original.map( - _.optLivenessMonitor - .map(_.isBuildServerResponsive) - ) - } - } object BuildServerConnection { @@ -557,7 +549,6 @@ object BuildServerConnection { config.metalsToIdleTime, config.pingInterval, bspStatus, - serverName, ) LauncherConnection( diff --git a/metals/src/main/scala/scala/meta/internal/metals/Configs.scala b/metals/src/main/scala/scala/meta/internal/metals/Configs.scala index 283fecbbc89..4e9b040388c 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Configs.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Configs.scala @@ -37,6 +37,9 @@ object Configs { new FileSystemWatcher( Either.forLeft(s"$root/project/build.properties") ), + new FileSystemWatcher( + Either.forLeft(s"$root/.metals/.reports/bloop/*/*") + ), ).asJava ) } diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala index 26652e7088f..44aa20d0c85 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala @@ -383,15 +383,17 @@ class MetalsLspService( ) ) + private val connectionBspStatus = + new ConnectionBspStatus(bspStatus, folder, clientConfig.icons()) + private val bspErrorHandler: BspErrorHandler = new BspErrorHandler( - languageClient, - folder, () => bspSession, tables, + connectionBspStatus, ) - private val buildClient: ForwardingMetalsBuildClient = + val buildClient: ForwardingMetalsBuildClient = new ForwardingMetalsBuildClient( languageClient, diagnostics, @@ -426,9 +428,6 @@ class MetalsLspService( clientConfig.initialConfig, ) - private val connectionBspStatus = - new ConnectionBspStatus(bspStatus, folder, clientConfig.icons()) - private val bspServers: BspServers = new BspServers( folder, charset, @@ -765,6 +764,7 @@ class MetalsLspService( maybeJdkVersion, getVisibleName, buildTools, + connectionBspStatus, ) val gitHubIssueFolderInfo = new GitHubIssueFolderInfo( @@ -1299,7 +1299,13 @@ class MetalsLspService( .toSeq val (deleteEvents, changeAndCreateEvents) = importantEvents.partition(_.getType().equals(FileChangeType.Deleted)) - deleteEvents.map(_.getUri().toAbsolutePath).foreach(onDelete) + val (bloopReportDelete, otherDeleteEvents) = + deleteEvents.partition( + _.getUri().toAbsolutePath.toNIO + .startsWith(reports.bloop.maybeReportsDir) + ) + if (bloopReportDelete.nonEmpty) connectionBspStatus.onReportsUpdate() + otherDeleteEvents.map(_.getUri().toAbsolutePath).foreach(onDelete) onChange(changeAndCreateEvents.map(_.getUri().toAbsolutePath)) } @@ -1327,7 +1333,6 @@ class MetalsLspService( ): CompletableFuture[Unit] = { val path = AbsolutePath(event.path) val isScalaOrJava = path.isScalaOrJava - event.eventType match { case EventType.CreateOrModify if path.isInBspDirectory(folder) && path.extension == "json" diff --git a/metals/src/main/scala/scala/meta/internal/metals/ServerLivenessMonitor.scala b/metals/src/main/scala/scala/meta/internal/metals/ServerLivenessMonitor.scala index 9acaa9ae1ba..e19d161fecd 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ServerLivenessMonitor.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ServerLivenessMonitor.scala @@ -58,7 +58,6 @@ class ServerLivenessMonitor( metalsIdleInterval: Duration, pingInterval: Duration, bspStatus: ConnectionBspStatus, - serverName: String, ) { @volatile private var lastPing: Long = 0 val scheduler: ScheduledExecutorService = Executors.newScheduledThreadPool(1) @@ -74,7 +73,7 @@ class ServerLivenessMonitor( def notResponding = lastIncoming > (pingInterval.toMillis * 2) if (!metalsIsIdle) { if (lastPingOk && notResponding) { - bspStatus.noResponse(serverName) + bspStatus.noResponse() } scribe.debug("server liveness monitor: pinging build server...") lastPing = now @@ -99,8 +98,6 @@ class ServerLivenessMonitor( TimeUnit.MILLISECONDS, ) - def isBuildServerResponsive: Boolean = bspStatus.isBuildServerResponsive - def shutdown(): Unit = { scribe.debug("shutting down server liveness monitor") scheduled.cancel(true) diff --git a/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala b/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala index 4ce840c85c0..cfd9a673117 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala @@ -8,13 +8,12 @@ import java.util.Date import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicBoolean -import scala.concurrent.Await import scala.concurrent.ExecutionContext -import scala.concurrent.duration.Duration import scala.util.Try import scala.meta.internal.bsp.BspResolvedResult import scala.meta.internal.bsp.BspSession +import scala.meta.internal.bsp.ConnectionBspStatus import scala.meta.internal.bsp.ResolvedBloop import scala.meta.internal.bsp.ResolvedBspOne import scala.meta.internal.bsp.ResolvedMultiple @@ -66,6 +65,7 @@ final class Doctor( maybeJDKVersion: Option[JdkVersion], folderName: String, buildTools: BuildTools, + bspStatus: ConnectionBspStatus, )(implicit ec: ExecutionContext, rc: ReportContext) { private val hasProblems = new AtomicBoolean(false) private val problemResolver = @@ -345,7 +345,7 @@ final class Doctor( } val targetIds = allTargetIds() - val errorReports = getErrorReports().groupBy(_.buildTarget) + val errorReports = getErrorReports().groupBy(ErrorReportsGroup.from) if (targetIds.isEmpty) { html .element("p")( @@ -376,7 +376,13 @@ final class Doctor( .element("th")(_.text("Recommendation")) ) ).element("tbody")(html => - buildTargetRows(html, allTargetsInfo, errorReports) + buildTargetRows( + html, + allTargetsInfo, + errorReports.collect { case (BuildTarget(name) -> v) => + (name -> v) + }, + ) ) ) @@ -394,22 +400,20 @@ final class Doctor( private def addErrorReportsInfo( html: HtmlBuilder, - errorReports: Map[Option[String], List[ErrorReportInfo]], + errorReports: Map[ErrorReportsGroup, List[ErrorReportInfo]], ) = { html.element("h2")(_.text("Error reports:")) errorReports.toVector .sortWith { - case (Some(v1) -> _, Some(v2) -> _) => v1 < v2 - case (None -> _, _ -> _) => false - case (_ -> _, None -> _) => true + case (BuildTarget(v1) -> _, BuildTarget(v2) -> _) => v1 < v2 + case (v1 -> _, v2 -> _) => v1.orderingNumber < v2.orderingNumber } - .foreach { case (optBuildTarget, reports) => - def name(default: String) = optBuildTarget.getOrElse(default) + .foreach { case (group, reports) => html.element("details")(details => details - .element("summary", s"id=reports-${name("other")}")( + .element("summary", s"id=reports-${group.id}")( _.element("b")( - _.text(s"${name("Other error reports")} (${reports.length}):") + _.text(s"${group.name} (${reports.length}):") ) ) .element("table") { table => @@ -443,14 +447,14 @@ final class Doctor( private def buildTargetRows( html: HtmlBuilder, infos: Seq[DoctorTargetInfo], - errorReports: Map[Option[String], List[ErrorReportInfo]], + errorReports: Map[String, List[ErrorReportInfo]], ): Unit = { infos .sortBy(f => (f.baseDirectory, f.name, f.dataKind)) .foreach { targetInfo => val center = "style='text-align: center'" def addErrorReportText(html: HtmlBuilder) = - errorReports.getOrElse(Some(targetInfo.name), List.empty) match { + errorReports.getOrElse(targetInfo.name, List.empty) match { case Nil => html.text(Icons.unicode.check) case _ => html.link(s"#reports-${targetInfo.name}", Icons.unicode.alert) @@ -546,10 +550,7 @@ final class Doctor( } private def isServerResponsive: Option[Boolean] = - currentBuildServer().flatMap { conn => - val isResponsiveFuture = conn.main.isBuildServerResponsive - Try(Await.result(isResponsiveFuture, Duration("1s"))).toOption.flatten - } + bspStatus.isBuildServerResponsive private def extractScalaTargetInfo( scalaTarget: ScalaTarget, @@ -667,3 +668,34 @@ object Doctor { .mkString("\n") } } + +sealed trait ErrorReportsGroup { + def orderingNumber: Int + def id: String + def name: String +} + +case object Bloop extends ErrorReportsGroup { + def orderingNumber = 1 + def id: String = "bloop" + def name: String = "Bloop error reports" +} +case class BuildTarget(name: String) extends ErrorReportsGroup { + def orderingNumber = 2 + def id: String = name +} +case object Other extends ErrorReportsGroup { + def orderingNumber = 3 + def id: String = "other" + def name: String = "Other error reports" +} + +object ErrorReportsGroup { + def from(info: ErrorReportInfo): ErrorReportsGroup = { + info.buildTarget match { + case Some(bt) => BuildTarget(bt) + case None if info.errorReportType == "bloop" => Bloop + case _ => Other + } + } +} diff --git a/mtags-shared/src/main/scala/scala/meta/internal/metals/ReportContext.scala b/mtags-shared/src/main/scala/scala/meta/internal/metals/ReportContext.scala index dc48d9b7f98..e1e6370060f 100644 --- a/mtags-shared/src/main/scala/scala/meta/internal/metals/ReportContext.scala +++ b/mtags-shared/src/main/scala/scala/meta/internal/metals/ReportContext.scala @@ -3,6 +3,8 @@ package scala.meta.internal.metals import java.nio.file.Files import java.nio.file.Path import java.nio.file.Paths +import java.util.concurrent.atomic.AtomicBoolean +import java.util.concurrent.atomic.AtomicReference import scala.util.matching.Regex @@ -30,6 +32,7 @@ trait Reporter { ): List[TimestampedFile] def getReports(): List[TimestampedFile] def deleteAll(): Unit + def sanitize(message: String) = message } class StdReportContext( @@ -84,7 +87,7 @@ class StdReporter( level: ReportLevel, override val name: String ) extends Reporter { - private lazy val maybeReportsDir: Path = + val maybeReportsDir: Path = workspace.resolve(pathToReports).resolve(name) private lazy val reportsDir = maybeReportsDir.createDirectories() private val limitedFilesManager = @@ -97,20 +100,21 @@ class StdReporter( private lazy val userHome = Option(System.getProperty("user.home")) - private var initialized = false - private var reported = Set.empty[String] + private val initialized = new AtomicBoolean(false) + private val reported = new AtomicReference(Map[String, Path]()) def readInIds(): Unit = { - reported = getReports().flatMap { report => + val reports = getReports().flatMap { report => val lines = Files.readAllLines(report.file.toPath()) if (lines.size() > 0) { lines.get(0) match { case id if id.startsWith(Report.idPrefix) => - Some(id.stripPrefix(Report.idPrefix)) + Some((id.stripPrefix(Report.idPrefix) -> report.toPath)) case _ => None } } else None - }.toSet + }.toMap + reported.updateAndGet(_ ++ reports) } override def create( @@ -119,22 +123,29 @@ class StdReporter( ): Option[Path] = if (ifVerbose && !level.isVerbose) None else { - if (!initialized) { + if (initialized.compareAndSet(false, true)) { readInIds() - initialized = true } val sanitizedId = report.id.map(sanitize) - if (sanitizedId.isDefined && reported.contains(sanitizedId.get)) None - else { - val path = reportPath(report) - path.getParent.createDirectories() - sanitizedId.foreach(reported += _) + val path = reportPath(report) + + val optDuplicate = + for { + id <- sanitizedId + reportedMap = reported.getAndUpdate(map => + if (map.contains(id)) map else map + (id -> path) + ) + duplicate <- reportedMap.get(id) + } yield duplicate + + optDuplicate.orElse { + path.createDirectories() path.writeText(sanitize(report.fullText(withIdAndSummary = true))) Some(path) } } - private def sanitize(text: String) = { + override def sanitize(text: String): String = { val textAfterWokspaceReplace = text.replace(workspace.toString(), StdReportContext.WORKSPACE_STR) userHome diff --git a/mtags-shared/src/main/scala/scala/meta/internal/mtags/CommonMtagsEnrichments.scala b/mtags-shared/src/main/scala/scala/meta/internal/mtags/CommonMtagsEnrichments.scala index 7b88c1d3252..930d897923a 100644 --- a/mtags-shared/src/main/scala/scala/meta/internal/mtags/CommonMtagsEnrichments.scala +++ b/mtags-shared/src/main/scala/scala/meta/internal/mtags/CommonMtagsEnrichments.scala @@ -379,4 +379,11 @@ trait CommonMtagsEnrichments { } } } + + implicit class XtensionText(text: String) { + def trimTo(maxLength: Int): String = + if (text.length() <= maxLength) text + else s"${text.take(maxLength)}..." + } + } diff --git a/tests/unit/src/main/scala/tests/TestingClient.scala b/tests/unit/src/main/scala/tests/TestingClient.scala index 762abc85734..a8a2d8ebf77 100644 --- a/tests/unit/src/main/scala/tests/TestingClient.scala +++ b/tests/unit/src/main/scala/tests/TestingClient.scala @@ -13,7 +13,6 @@ import scala.concurrent.Promise import scala.meta.inputs.Input import scala.meta.internal.bsp.ConnectionBspStatus -import scala.meta.internal.builds.BspErrorHandler import scala.meta.internal.builds.BuildTool import scala.meta.internal.builds.BuildTools import scala.meta.internal.decorations.PublishDecorationsParams @@ -91,7 +90,6 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) var importScalaCliScript = new MessageActionItem(ImportScalaScript.dismiss) var resetWorkspace = new MessageActionItem(ResetWorkspace.cancel) var regenerateAndRestartScalaCliBuildSever = FileOutOfScalaCliBspScope.ignore - var bspError = BspErrorHandler.dismiss val resources = new ResourceOperations(buffers) val diagnostics: TrieMap[AbsolutePath, Seq[Diagnostic]] = @@ -189,6 +187,8 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) clientCommands.asScala.toList.map(_.getCommand) } + def pollStatusBar(): String = statusParams.poll().text + def statusBarHistory: String = { statusParams.asScala .map { params => @@ -370,10 +370,6 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) regenerateAndRestartScalaCliBuildSever } else if (params.getMessage() == choicesMessage) { params.getActions().asScala.head - } else if ( - params.getMessage().startsWith(BspErrorHandler.errorHeader) - ) { - bspError } else if ( params.getMessage() == ConnectionBspStatus .noResponseParams("Bill", Icons.default) diff --git a/tests/unit/src/test/scala/tests/BspErrorHandlerSuite.scala b/tests/unit/src/test/scala/tests/BspErrorHandlerSuite.scala deleted file mode 100644 index 720deb23f0c..00000000000 --- a/tests/unit/src/test/scala/tests/BspErrorHandlerSuite.scala +++ /dev/null @@ -1,110 +0,0 @@ -package tests - -import java.util.concurrent.Executors - -import scala.concurrent.ExecutionContext - -import scala.meta.internal.builds.BspErrorHandler -import scala.meta.internal.metals.Buffers -import scala.meta.internal.metals.Directories -import scala.meta.internal.metals.MetalsEnrichments._ -import scala.meta.internal.metals.Tables -import scala.meta.io.AbsolutePath - -import com.google.gson.JsonObject -import org.eclipse.lsp4j.MessageActionItem - -class BspErrorHandlerSuite extends BaseTablesSuite { - implicit val ec: ExecutionContext = - ExecutionContext.fromExecutorService(Executors.newCachedThreadPool()) - - val client = new TestingClient(workspace, Buffers()) - val exampleError1 = "an error" - val exampleError2 = "a different error" - val longError: String = - """|This is a long error. - |Such a long error will not fully fit in the - |message box to be show to the user. - |It will get trimmed to maximum 150 characters. - |The full message will be available in the metals logs. - |""".stripMargin - - test("handle-bsp-error") { - val errorHandler = new TestBspErrorHandler(client, workspace, tables) - - FileLayout.fromString( - s"""|/.metals/metals.log - | - |""".stripMargin, - workspace, - ) - - client.bspError = new MessageActionItem("OK") - - for { - _ <- errorHandler.onError(exampleError1) - _ <- errorHandler.onError(exampleError1) - _ <- errorHandler.onError(exampleError2) - _ = client.bspError = BspErrorHandler.dismiss - _ <- errorHandler.onError(exampleError1) - _ = client.bspError = BspErrorHandler.goToLogs - _ <- errorHandler.onError(longError) - _ <- errorHandler.onError(exampleError1) - _ = client.bspError = BspErrorHandler.doNotShowErrors - _ <- errorHandler.onError(exampleError2) - _ <- errorHandler.onError(longError) - _ <- errorHandler.onError(exampleError2) - } yield { - assertNoDiff( - client.workspaceMessageRequests, - s"""|${BspErrorHandler.makeShortMessage(exampleError1)} - |${BspErrorHandler.makeShortMessage(exampleError2)} - |${BspErrorHandler.makeShortMessage(exampleError1)} - |${BspErrorHandler.makeLongMessage(longError)} - |${BspErrorHandler.makeShortMessage(exampleError2)} - |""".stripMargin, - ) - assert(client.clientCommands.asScala.nonEmpty) - assertNoDiff( - client.clientCommands.asScala.head.getCommand(), - "metals-goto-location", - ) - val params = client.clientCommands.asScala.head - .getArguments() - .asScala - .head - .asInstanceOf[JsonObject] - assertEquals( - params.get("uri").getAsString(), - workspace.resolve(Directories.log).toURI.toString(), - ) - assertEquals( - params - .getAsJsonObject("range") - .getAsJsonObject("start") - .get("line") - .getAsInt(), - 4, - ) - workspace.resolve(Directories.log).delete() - } - } - -} - -class TestBspErrorHandler( - val languageClient: TestingClient, - workspaceFolder: AbsolutePath, - tables: Tables, -)(implicit context: ExecutionContext) - extends BspErrorHandler( - languageClient, - workspaceFolder, - () => None, - tables, - ) { - override def shouldShowBspError: Boolean = true - - override def logError(message: String): Unit = - logsPath.appendText(s"$message\n") -} diff --git a/tests/unit/src/test/scala/tests/BspStatusSuite.scala b/tests/unit/src/test/scala/tests/BspStatusSuite.scala index b0525ef79e5..e0e38824786 100644 --- a/tests/unit/src/test/scala/tests/BspStatusSuite.scala +++ b/tests/unit/src/test/scala/tests/BspStatusSuite.scala @@ -1,13 +1,28 @@ package tests +import java.nio.file.Files import java.nio.file.Paths import scala.meta.internal.metals.BspStatus +import scala.meta.internal.metals.Icons +import scala.meta.internal.metals.MetalsEnrichments._ +import scala.meta.internal.metals.MetalsServerConfig +import scala.meta.internal.metals.StatusBarConfig import scala.meta.internal.metals.clients.language.MetalsStatusParams import scala.meta.internal.metals.clients.language.NoopLanguageClient import scala.meta.io.AbsolutePath -class BspStatusSuite extends BaseSuite { +import bill.Bill +import org.eclipse.lsp4j.DidChangeWatchedFilesParams +import org.eclipse.lsp4j.FileChangeType +import org.eclipse.lsp4j.FileEvent +import org.eclipse.lsp4j.MessageParams +import org.eclipse.lsp4j.MessageType + +class BspStatusSuite extends BaseLspSuite("bsp-status-suite") { + + override def serverConfig: MetalsServerConfig = + MetalsServerConfig.default.copy(bspStatusBar = StatusBarConfig.on) test("basic") { val client = new StatusClient @@ -45,6 +60,63 @@ class BspStatusSuite extends BaseSuite { assertEquals(client.status, "some other other text") } + test("bsp-error") { + cleanWorkspace() + Bill.installWorkspace(workspace.resolve("billWorkspace").toNIO, "Bill") + def bloopReports = server.server.reports.bloop.getReports() + for { + _ <- initialize( + Map( + "bloopWorkspace" -> + """|/metals.json + |{ "a": { } } + | + |/a/src/main/scala/Main.scala + |object Main { + | val x: Int = 1 + |} + |""".stripMargin, + "billWorkspace" -> + """|/src/com/App.scala + |object App { + | val x: Int = 1 + |} + |""".stripMargin, + ), + expectError = false, + ) + _ <- server.didOpen("bloopWorkspace/a/src/main/scala/Main.scala") + _ = assertNoDiff(client.pollStatusBar(), s"Bloop ${Icons.default.link}") + _ = client.statusParams.clear() + _ <- server.didOpen("billWorkspace/src/com/App.scala") + _ = assertNoDiff(client.pollStatusBar(), s"Bill ${Icons.default.link}") + _ = client.statusParams.clear() + _ = server.server.buildClient.onBuildLogMessage( + new MessageParams(MessageType.Error, "This is an error.") + ) + _ = assert(client.statusParams.isEmpty()) + _ <- server.didFocus("bloopWorkspace/a/src/main/scala/Main.scala") + _ = assertNoDiff( + client.pollStatusBar(), + s"Bloop 1 ${Icons.default.alert}", + ) + reports = bloopReports + _ = assert(reports.nonEmpty) + reportUri = reports.head.file.toURI().toString + newPath = (reportUri ++ ".seen").toAbsolutePath.toNIO + _ = Files.move(reports.head.toPath, newPath) + _ <- server.fullServer + .didChangeWatchedFiles( + new DidChangeWatchedFilesParams( + List(new FileEvent(reportUri, FileChangeType.Deleted)).asJava + ) + ) + .asScala + _ = assertNoDiff(client.pollStatusBar(), s"Bloop ${Icons.default.link}") + _ = assert(bloopReports.nonEmpty) + } yield () + } + } class StatusClient extends NoopLanguageClient { diff --git a/tests/unit/src/test/scala/tests/ReportsSuite.scala b/tests/unit/src/test/scala/tests/ReportsSuite.scala index 0df57467ac8..7450e47aace 100644 --- a/tests/unit/src/test/scala/tests/ReportsSuite.scala +++ b/tests/unit/src/test/scala/tests/ReportsSuite.scala @@ -115,9 +115,11 @@ class ReportsSuite extends BaseSuite { test("save-with-id") { val testId = "test-id" - val path = reportsProvider.incognito.create( - Report("test_error", exampleText(), "Test error", id = Some(testId)) - ) + val path = reportsProvider.incognito + .create( + Report("test_error", exampleText(), "Test error", id = Some(testId)) + ) + .map(_.toRealPath()) val obtained = new String(Files.readAllBytes(path.get), StandardCharsets.UTF_8) assertNoDiff( @@ -132,13 +134,16 @@ class ReportsSuite extends BaseSuite { val none1 = reportsProvider.incognito.create( Report("test_error_again", exampleText(), "Test error", id = Some(testId)) ) - assert(none1.isEmpty) + assertEquals( + none1.map(_.toRealPath()), + path, + ) // check that it returns the path to the original report val newReportsProvider = new StdReportContext(workspace.toNIO, _ => Some("buildTarget")) val none2 = newReportsProvider.incognito.create( Report("test_error_again", exampleText(), "Test error", id = Some(testId)) ) - assert(none2.isEmpty) + assertEquals(none2.map(_.toRealPath()), path) val reports = newReportsProvider.incognito.getReports() reports match { case head :: Nil => assert(head.file.getName == path.get.toFile.getName) diff --git a/tests/unit/src/test/scala/tests/ServerLivenessMonitorSuite.scala b/tests/unit/src/test/scala/tests/ServerLivenessMonitorSuite.scala index 14757dcd04f..ae04d404d0f 100644 --- a/tests/unit/src/test/scala/tests/ServerLivenessMonitorSuite.scala +++ b/tests/unit/src/test/scala/tests/ServerLivenessMonitorSuite.scala @@ -12,6 +12,7 @@ import scala.concurrent.duration.Duration import scala.meta.internal.bsp.ConnectionBspStatus import scala.meta.internal.metals.BspStatus import scala.meta.internal.metals.Icons +import scala.meta.internal.metals.LoggerReportContext import scala.meta.internal.metals.RequestMonitor import scala.meta.internal.metals.ServerLivenessMonitor import scala.meta.internal.metals.clients.language.MetalsStatusParams @@ -25,20 +26,18 @@ class ServerLivenessMonitorSuite extends BaseSuite { test("basic") { val pingInterval = Duration("3s") val server = new ResponsiveServer(pingInterval) - val client = new CountMessageRequestsClient - val bspStatus = new BspStatus(client, isBspStatusProvider = true) + val bspStatus = new CountMessageRequestsBspStatus val connectionBspStatus = new ConnectionBspStatus( bspStatus, AbsolutePath(Paths.get(".")), Icons.default, - ) + )(LoggerReportContext) val livenessMonitor = new ServerLivenessMonitor( server, () => server.sendRequest(true), metalsIdleInterval = pingInterval * 4, pingInterval, connectionBspStatus, - "responsive-server", ) connectionBspStatus.connected("responsive-server") Thread.sleep(pingInterval.toMillis * 3 / 2) @@ -56,7 +55,7 @@ class ServerLivenessMonitorSuite extends BaseSuite { server.sendRequest(false) assert(!livenessMonitor.metalsIsIdle) assert(livenessMonitor.lastPingOk) - assert(client.showMessageRequests == 0) + assert(bspStatus.noResponseMessages == 0) livenessMonitor.shutdown() } } @@ -93,16 +92,16 @@ class ResponsiveServer(pingInterval: Duration) extends RequestMonitor { } } -class CountMessageRequestsClient extends NoopLanguageClient { - var showMessageRequests = 0 - - override def metalsStatus(params: MetalsStatusParams): Unit = +class CountMessageRequestsBspStatus + extends BspStatus(NoopLanguageClient, true) { + var noResponseMessages = 0 + override def status(folder: AbsolutePath, params: MetalsStatusParams): Unit = if ( params == ConnectionBspStatus.noResponseParams( "responsive-server", Icons.default, ) ) { - showMessageRequests += 1 + noResponseMessages += 1 } }