From 044cc69c18193a17497996846fccdfb2d14f6e2b Mon Sep 17 00:00:00 2001 From: kasiaMarek Date: Thu, 12 Dec 2024 11:23:34 +0100 Subject: [PATCH] try to propagate cancel --- .../internal/bsp/BspConfigGenerator.scala | 3 +- .../meta/internal/bsp/BspConnector.scala | 1 + .../meta/internal/builds/BloopInstall.scala | 28 +++-- .../builds/BloopInstallProvider.scala | 7 +- .../internal/builds/BuildServerProvider.scala | 13 ++- .../internal/builds/NewProjectProvider.scala | 1 + .../meta/internal/builds/SbtBuildTool.scala | 4 +- .../internal/builds/ScalaCliBuildTool.scala | 11 +- .../meta/internal/builds/ShellRunner.scala | 8 +- .../internal/metals/CancelableFuture.scala | 2 +- .../internal/metals/ConnectionProvider.scala | 108 ++++++++---------- .../internal/metals/FileDecoderProvider.scala | 2 + .../meta/internal/metals/Interruptable.scala | 71 ++++++++---- .../scala/tests/bazel/BazelLspSuite.scala | 22 ++-- 14 files changed, 158 insertions(+), 123 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/bsp/BspConfigGenerator.scala b/metals/src/main/scala/scala/meta/internal/bsp/BspConfigGenerator.scala index 8f083a8ba8e..6e8ffe082d3 100644 --- a/metals/src/main/scala/scala/meta/internal/bsp/BspConfigGenerator.scala +++ b/metals/src/main/scala/scala/meta/internal/bsp/BspConfigGenerator.scala @@ -10,6 +10,7 @@ import scala.util.control.NonFatal import scala.meta.internal.bsp.BspConfigGenerationStatus._ import scala.meta.internal.builds.BuildServerProvider import scala.meta.internal.builds.ShellRunner +import scala.meta.internal.metals.CancelableFuture import scala.meta.internal.metals.Directories import scala.meta.internal.metals.Messages.BspProvider import scala.meta.internal.metals.MetalsEnrichments._ @@ -31,7 +32,7 @@ final class BspConfigGenerator( def runUnconditionally( buildTool: BuildServerProvider, args: List[String], - ): Future[BspConfigGenerationStatus] = + ): CancelableFuture[BspConfigGenerationStatus] = shellRunner .run( s"${buildTool.buildServerName} bspConfig", diff --git a/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala b/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala index f321e141098..0d0a73d5348 100644 --- a/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala +++ b/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala @@ -159,6 +159,7 @@ class BspConnector( args => bspConfigGenerator.runUnconditionally(bsp, args), statusBar, ) + .future .flatMap { _ => connect( projectRoot, diff --git a/metals/src/main/scala/scala/meta/internal/builds/BloopInstall.scala b/metals/src/main/scala/scala/meta/internal/builds/BloopInstall.scala index 9b4ba9bea41..a3b17f3991a 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BloopInstall.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BloopInstall.scala @@ -4,10 +4,15 @@ import java.util.concurrent.TimeUnit import scala.concurrent.ExecutionContext import scala.concurrent.Future +import scala.concurrent.Promise import scala.meta.internal.builds.Digest.Status import scala.meta.internal.metals.BuildInfo +import scala.meta.internal.metals.CancelSwitch +import scala.meta.internal.metals.CancelableFuture import scala.meta.internal.metals.Confirmation +import scala.meta.internal.metals.Interruptable +import scala.meta.internal.metals.Interruptable._ import scala.meta.internal.metals.Messages._ import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.Tables @@ -37,14 +42,14 @@ final class BloopInstall( def runUnconditionally( buildTool: BloopInstallProvider - ): Future[WorkspaceLoadedStatus] = { + ): CancelableFuture[WorkspaceLoadedStatus] = { buildTool.bloopInstall( workspace, args => { scribe.info(s"running '${args.mkString(" ")}'") val process = runArgumentsUnconditionally(buildTool, args, userConfig().javaHome) - process.foreach { e => + process.future.foreach { e => if (e.isFailed) { // Record the exact command that failed to help troubleshooting. scribe.error(s"$buildTool command failed: ${args.mkString(" ")}") @@ -59,7 +64,7 @@ final class BloopInstall( buildTool: BloopInstallProvider, args: List[String], javaHome: Option[String], - ): Future[WorkspaceLoadedStatus] = { + ): CancelableFuture[WorkspaceLoadedStatus] = { persistChecksumStatus(Status.Started, buildTool) val processFuture = shellRunner .run( @@ -81,7 +86,7 @@ final class BloopInstall( case ExitCodes.Cancel => WorkspaceLoadedStatus.Cancelled case result => WorkspaceLoadedStatus.Failed(result) } - processFuture.foreach { result => + processFuture.future.foreach { result => try result.toChecksumStatus.foreach(persistChecksumStatus(_, buildTool)) catch { case _: InterruptedException => @@ -112,35 +117,36 @@ final class BloopInstall( def runIfApproved( buildTool: BloopInstallProvider, digest: String, - ): Future[WorkspaceLoadedStatus] = + ): CancelableFuture[WorkspaceLoadedStatus] = synchronized { oldInstallResult(digest) match { case Some(result) if result != WorkspaceLoadedStatus.Duplicate(Status.Requested) => scribe.info(s"skipping build import with status '${result.name}'") - Future.successful(result) + CancelableFuture.successful(result) case _ => if (userConfig().shouldAutoImportNewProject) { runUnconditionally(buildTool) } else { scribe.debug("Awaiting user response...") - for { + implicit val cancelSwitch = CancelSwitch(Promise[Unit]()) + (for { userResponse <- requestImport( buildTools, buildTool, languageClient, digest, - ) + ).withInterrupt installResult <- { if (userResponse.isYes) { - runUnconditionally(buildTool) + runUnconditionally(buildTool).withInterrupt } else { // Don't spam the user with requests during rapid build changes. notification.dismiss(2, TimeUnit.MINUTES) - Future.successful(WorkspaceLoadedStatus.Rejected) + Interruptable.successful(WorkspaceLoadedStatus.Rejected) } } - } yield installResult + } yield installResult).toCancellable } } } diff --git a/metals/src/main/scala/scala/meta/internal/builds/BloopInstallProvider.scala b/metals/src/main/scala/scala/meta/internal/builds/BloopInstallProvider.scala index fa3731e6ca4..d3fa5c0e4c7 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BloopInstallProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BloopInstallProvider.scala @@ -2,8 +2,7 @@ package scala.meta.internal.builds import java.io.IOException import java.nio.file.Files -import scala.concurrent.Future - +import scala.meta.internal.metals.CancelableFuture import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.io.AbsolutePath @@ -18,8 +17,8 @@ trait BloopInstallProvider extends BuildTool { */ def bloopInstall( workspace: AbsolutePath, - systemProcess: List[String] => Future[WorkspaceLoadedStatus], - ): Future[WorkspaceLoadedStatus] = { + systemProcess: List[String] => CancelableFuture[WorkspaceLoadedStatus], + ): CancelableFuture[WorkspaceLoadedStatus] = { cleanupStaleConfig() systemProcess(bloopInstallArgs(workspace)) } diff --git a/metals/src/main/scala/scala/meta/internal/builds/BuildServerProvider.scala b/metals/src/main/scala/scala/meta/internal/builds/BuildServerProvider.scala index c47fcb04aff..aa45eda0760 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BuildServerProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BuildServerProvider.scala @@ -4,6 +4,7 @@ import scala.annotation.nowarn import scala.concurrent.Future import scala.meta.internal.bsp.BspConfigGenerationStatus._ +import scala.meta.internal.metals.CancelableFuture import scala.meta.internal.metals.Messages import scala.meta.internal.metals.StatusBar import scala.meta.io.AbsolutePath @@ -20,12 +21,16 @@ trait BuildServerProvider extends BuildTool { @nowarn("msg=parameter statusBar in method generateBspConfig is never used") def generateBspConfig( workspace: AbsolutePath, - systemProcess: List[String] => Future[BspConfigGenerationStatus], + systemProcess: List[String] => CancelableFuture[ + BspConfigGenerationStatus + ], statusBar: StatusBar, - ): Future[BspConfigGenerationStatus] = + ): CancelableFuture[BspConfigGenerationStatus] = createBspFileArgs(workspace).map(systemProcess).getOrElse { - Future.successful( - Failed(Right(Messages.NoBspSupport.toString())) + CancelableFuture( + Future.successful( + Failed(Right(Messages.NoBspSupport.toString())) + ) ) } diff --git a/metals/src/main/scala/scala/meta/internal/builds/NewProjectProvider.scala b/metals/src/main/scala/scala/meta/internal/builds/NewProjectProvider.scala index ad0dcb6fdbe..cc40a5a6378 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/NewProjectProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/NewProjectProvider.scala @@ -134,6 +134,7 @@ class NewProjectProvider( javaHome, javaOptsMap = javaOpts, ) + .future .flatMap { case ExitCodes.Success => askForWindow(projectPath) diff --git a/metals/src/main/scala/scala/meta/internal/builds/SbtBuildTool.scala b/metals/src/main/scala/scala/meta/internal/builds/SbtBuildTool.scala index d36a4f7b52c..a689db6d2a7 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/SbtBuildTool.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/SbtBuildTool.scala @@ -77,7 +77,7 @@ case class SbtBuildTool( def shutdownBspServer( shellRunner: ShellRunner - ): Future[Int] = { + ): CancelableFuture[Int] = { val shutdownArgs = composeArgs(List("--client", "shutdown"), projectRoot, projectRoot.toNIO) scribe.info(s"running ${shutdownArgs.mkString(" ")}") @@ -242,7 +242,7 @@ case class SbtBuildTool( if (promise.isCompleted) { // executes when user chooses `restart` after the timeout restartSbtBuildServer() - } else shutdownBspServer(shellRunner).ignoreValue + } else shutdownBspServer(shellRunner).future.ignoreValue case _ => promise.trySuccess(()) Future.successful(()) diff --git a/metals/src/main/scala/scala/meta/internal/builds/ScalaCliBuildTool.scala b/metals/src/main/scala/scala/meta/internal/builds/ScalaCliBuildTool.scala index b5ad6d1a546..632f47c0d39 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/ScalaCliBuildTool.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/ScalaCliBuildTool.scala @@ -2,10 +2,9 @@ package scala.meta.internal.builds import java.security.MessageDigest -import scala.concurrent.Future - import scala.meta.internal.bsp.BspConfigGenerationStatus._ import scala.meta.internal.metals.BuildInfo +import scala.meta.internal.metals.CancelableFuture import scala.meta.internal.metals.Directories import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.StatusBar @@ -26,9 +25,11 @@ case class ScalaCliBuildTool( override def generateBspConfig( workspace: AbsolutePath, - systemProcess: List[String] => Future[BspConfigGenerationStatus], + systemProcess: List[String] => CancelableFuture[ + BspConfigGenerationStatus + ], statusBar: StatusBar, - ): Future[BspConfigGenerationStatus] = + ): CancelableFuture[BspConfigGenerationStatus] = createBspFileArgs(workspace).map(systemProcess).getOrElse { // fallback to creating `.bsp/scala-cli.json` that starts JVM launcher val bspConfig = @@ -37,7 +38,7 @@ case class ScalaCliBuildTool( bspConfig.writeText( ScalaCli.scalaCliBspJsonContent(projectRoot = projectRoot.toString()) ) - Future.successful(Generated) + CancelableFuture.successful(Generated) } override def createBspFileArgs( diff --git a/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala b/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala index 2e180f42d08..befa53d2708 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala @@ -4,13 +4,13 @@ import java.io.File import scala.concurrent.Await import scala.concurrent.ExecutionContext -import scala.concurrent.Future import scala.concurrent.Promise import scala.concurrent.duration.DurationInt import scala.language.postfixOps import scala.util.Properties import scala.meta.internal.metals.Cancelable +import scala.meta.internal.metals.CancelableFuture import scala.meta.internal.metals.JavaBinary import scala.meta.internal.metals.JdkSources import scala.meta.internal.metals.MetalsEnrichments._ @@ -45,7 +45,7 @@ class ShellRunner(time: Time, workDoneProvider: WorkDoneProgress)(implicit processErr: String => Unit = scribe.error(_), propagateError: Boolean = false, javaOptsMap: Map[String, String] = Map.empty, - ): Future[Int] = { + ): CancelableFuture[Int] = { val classpathSeparator = if (Properties.isWin) ";" else ":" val classpath = Fetch @@ -92,7 +92,7 @@ class ShellRunner(time: Time, workDoneProvider: WorkDoneProgress)(implicit processErr: String => Unit = scribe.error(_), propagateError: Boolean = false, logInfo: Boolean = true, - ): Future[Int] = { + ): CancelableFuture[Int] = { val elapsed = new Timer(time) val env = additionalEnv ++ JdkSources.envVariables(javaHome) val ps = SystemProcess.run( @@ -125,7 +125,7 @@ class ShellRunner(time: Time, workDoneProvider: WorkDoneProgress)(implicit result.trySuccess(code) } result.future.onComplete(_ => cancelables.remove(newCancelable)) - result.future + CancelableFuture(result.future, newCancelable) } } diff --git a/metals/src/main/scala/scala/meta/internal/metals/CancelableFuture.scala b/metals/src/main/scala/scala/meta/internal/metals/CancelableFuture.scala index 984437b8f77..bd50d31c3b1 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/CancelableFuture.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/CancelableFuture.scala @@ -4,7 +4,7 @@ import scala.concurrent.ExecutionContext import scala.concurrent.Future import scala.util.Try -case class CancelableFuture[T]( +case class CancelableFuture[+T]( future: Future[T], cancelable: Cancelable = Cancelable.empty, ) extends Cancelable { diff --git a/metals/src/main/scala/scala/meta/internal/metals/ConnectionProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/ConnectionProvider.scala index 57b1d4a9cdb..6c5494e0b7a 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ConnectionProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ConnectionProvider.scala @@ -324,14 +324,15 @@ class ConnectionProvider( val curr = iter.next() request.cancelCompare(curr.request) match { case TakeOver => curr.cancel() - case Yield => info.cancel() - case _ => + case Yield => info.cancel() + case _ => } } queue.add(info) // maybe cancel ongoing currentRequest.foreach(ongoing => - if (request.cancelCompare(ongoing.request) == TakeOver) ongoing.cancel() + if (request.cancelCompare(ongoing.request) == TakeOver) + ongoing.cancel() ) info } @@ -345,26 +346,25 @@ class ConnectionProvider( } for (request <- optRequest) { - val cancelPromise = request.cancelPromise + implicit val cancelPromise = CancelSwitch(request.cancelPromise) val result = - if (cancelPromise.isCompleted) + if (request.cancelPromise.isCompleted) Interruptable.successful(BuildChange.Cancelled) else request.request match { case Disconnect(shutdownBuildServer) => - disconnect(shutdownBuildServer, cancelPromise) - case Index(check) => index(check, cancelPromise) + disconnect(shutdownBuildServer) + case Index(check) => index(check) case ImportBuildAndIndex(session) => - importBuildAndIndex(session, cancelPromise) + importBuildAndIndex(session) case ConnectToSession(session) => - connectToSession(session, cancelPromise) + connectToSession(session) case CreateSession(shutdownBuildServer) => - createSession(shutdownBuildServer, cancelPromise) + createSession(shutdownBuildServer) case GenerateBspConfigAndConnect(buildTool, shutdownServer) => generateBspConfigAndConnect( buildTool, shutdownServer, - cancelPromise, ) case BloopInstallAndConnect( buildTool, @@ -377,7 +377,6 @@ class ConnectionProvider( checksum, forceImport, shutdownServer, - cancelPromise, ) } result.future.onComplete { res => @@ -393,22 +392,21 @@ class ConnectionProvider( } private def disconnect( - shutdownBuildServer: Boolean, - cancelPromise: Promise[Unit], - ): Interruptable[BuildChange] = { - def shutdownBsp(optMainBsp: Option[String]): Future[Boolean] = { + shutdownBuildServer: Boolean + )(implicit cancelSwitch: CancelSwitch): Interruptable[BuildChange] = { + def shutdownBsp(optMainBsp: Option[String]): Interruptable[Boolean] = { optMainBsp match { case Some(BloopServers.name) => - Future { bloopServers.shutdownServer() } + Interruptable.successful { bloopServers.shutdownServer() } case Some(SbtBuildTool.name) => for { res <- buildToolProvider.buildTool match { case Some(sbt: SbtBuildTool) => - sbt.shutdownBspServer(shellRunner).map(_ == 0) - case _ => Future.successful(false) + sbt.shutdownBspServer(shellRunner).withInterrupt.map(_ == 0) + case _ => Interruptable.successful(false) } } yield res - case s => Future.successful(s.nonEmpty) + case s => Interruptable.successful(s.nonEmpty) } } @@ -420,33 +418,28 @@ class ConnectionProvider( ) for { - _ <- scalaCli.stop(storeLast = true).withInterrupt(cancelPromise) + _ <- scalaCli.stop(storeLast = true).withInterrupt optMainBsp <- (bspSession match { case None => Future.successful(None) case Some(session) => bspSession = None mainBuildTargetsData.resetConnections(List.empty) session.shutdown().map(_ => Some(session.main.name)) - }).withInterrupt(cancelPromise) + }).withInterrupt _ <- - if (shutdownBuildServer) - shutdownBsp(optMainBsp).withInterrupt(cancelPromise) + if (shutdownBuildServer) shutdownBsp(optMainBsp) else Interruptable.successful(()) } yield BuildChange.None } - private def index( - check: () => Unit, - cancelPromise: Promise[Unit], - ): Interruptable[BuildChange] = + private def index(check: () => Unit): Interruptable[BuildChange] = profiledIndexWorkspace(check) .map(_ => BuildChange.None) - .withInterrupt(cancelPromise) + .withInterrupt private def importBuildAndIndex( - session: BspSession, - cancelPromise: Promise[Unit], - ): Interruptable[BuildChange] = { + session: BspSession + )(implicit cancelSwitch: CancelSwitch): Interruptable[BuildChange] = { val importedBuilds0 = timerProvider.timed("Imported build") { session.importBuilds() } @@ -456,7 +449,7 @@ class ConnectionProvider( Messages.importingBuild, importedBuilds0, ) - .withInterrupt(cancelPromise) + .withInterrupt _ = { val idToConnection = bspBuilds.flatMap { bspBuild => val targets = @@ -467,7 +460,7 @@ class ConnectionProvider( saveProjectReferencesInfo(bspBuilds) } _ = compilers.cancel() - buildChange <- index(check, cancelPromise) + buildChange <- index(check) } yield buildChange } @@ -490,9 +483,8 @@ class ConnectionProvider( } private def connectToSession( - session: BspSession, - cancelPromise: Promise[Unit], - ): Interruptable[BuildChange] = { + session: BspSession + )(implicit cancelSwitch: CancelSwitch): Interruptable[BuildChange] = { scribe.info( s"Connected to Build server: ${session.main.name} v${session.version}" ) @@ -503,7 +495,7 @@ class ConnectionProvider( bspSession = Some(session) isConnecting.set(false) for { - _ <- importBuildAndIndex(session, cancelPromise) + _ <- importBuildAndIndex(session) _ = buildToolProvider.buildTool.foreach( workspaceReload.persistChecksumStatus(Digest.Status.Installed, _) ) @@ -535,9 +527,8 @@ class ConnectionProvider( } def createSession( - shutdownServer: Boolean, - cancelPromise: Promise[Unit], - ): Interruptable[BuildChange] = { + shutdownServer: Boolean + )(implicit cancelSwitch: CancelSwitch): Interruptable[BuildChange] = { def compileAllOpenFiles: BuildChange => Future[BuildChange] = { case change if !change.isFailed => Future @@ -554,7 +545,7 @@ class ConnectionProvider( isConnecting.set(true) (for { - _ <- disconnect(shutdownServer, cancelPromise) + _ <- disconnect(shutdownServer) maybeSession <- timerProvider .timed( "Connected to build server", @@ -567,10 +558,10 @@ class ConnectionProvider( shellRunner, ) } - .withInterrupt(cancelPromise) + .withInterrupt result <- maybeSession match { case Some(session) => - val result = connectToSession(session, cancelPromise) + val result = connectToSession(session) session.mainConnection.onReconnection { newMainConn => val updSession = session.copy(main = newMainConn) connect(ConnectToSession(updSession)) @@ -585,11 +576,11 @@ class ConnectionProvider( .startForAllLastPaths(path => !buildTargets.belongsToBuildTarget(path.toNIO) ) - .withInterrupt(cancelPromise) + .withInterrupt _ = initTreeView() } yield result) .recover { case NonFatal(e) => - disconnect(false, cancelPromise) + disconnect(false) val message = "Failed to connect with build server, no functionality will work." val details = " See logs for more details." @@ -599,7 +590,7 @@ class ConnectionProvider( scribe.error(message, e) BuildChange.Failed } - .flatMap(compileAllOpenFiles(_).withInterrupt(cancelPromise)) + .flatMap(compileAllOpenFiles(_).withInterrupt) .map { res => buildServerPromise.trySuccess(()) res @@ -609,13 +600,12 @@ class ConnectionProvider( private def generateBspConfigAndConnect( buildTool: BuildServerProvider, shutdownServer: Boolean, - cancelPromise: Promise[Unit], - ): Interruptable[BuildChange] = { + )(implicit cancelSwitch: CancelSwitch): Interruptable[BuildChange] = { tables.buildTool.chooseBuildTool(buildTool.executableName) maybeChooseServer(buildTool.buildServerName, alreadySelected = false) for { _ <- - if (shutdownServer) disconnect(shutdownServer, cancelPromise) + if (shutdownServer) disconnect(shutdownServer) else Interruptable.successful(()) status <- buildTool .generateBspConfig( @@ -623,10 +613,10 @@ class ConnectionProvider( args => bspConfigGenerator.runUnconditionally(buildTool, args), statusBar, ) - .withInterrupt(cancelPromise) + .withInterrupt shouldConnect = handleGenerationStatus(buildTool, status) status <- - if (shouldConnect) createSession(false, cancelPromise) + if (shouldConnect) createSession(false) else Interruptable.successful(BuildChange.Failed) } yield status } @@ -667,8 +657,7 @@ class ConnectionProvider( checksum: String, forceImport: Boolean, shutdownServer: Boolean, - cancelPromise: Promise[Unit], - ): Interruptable[BuildChange] = { + )(implicit cancelSwitch: CancelSwitch): Interruptable[BuildChange] = { for { result <- { if (forceImport) @@ -680,9 +669,9 @@ class ConnectionProvider( buildTool, checksum, ) - }.withInterrupt(cancelPromise) + }.withInterrupt change <- { - if (result.isInstalled) createSession(shutdownServer, cancelPromise) + if (result.isInstalled) createSession(shutdownServer) else if (result.isFailed) { for { change <- @@ -698,7 +687,7 @@ class ConnectionProvider( // Connect nevertheless, many build import failures are caused // by resolution errors in one weird module while other modules // exported successfully. - createSession(shutdownServer, cancelPromise) + createSession(shutdownServer) } else { languageClient.showMessage(Messages.ImportProjectFailed) Interruptable.successful(BuildChange.Failed) @@ -721,10 +710,11 @@ case object Queue extends ConflictBehaviour sealed trait ConnectRequest extends ConnectKind { - /** Decides what to do with a new connect request + /** + * Decides what to do with a new connect request * in presence of an another ongoing/queued request. * @param other the ongoing or queued request - * @return behavoiur of the incoming request + * @return behavoiur of the incoming request * Yield -- cancel this * TakeOver -- cancel other * Queue -- queue diff --git a/metals/src/main/scala/scala/meta/internal/metals/FileDecoderProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/FileDecoderProvider.scala index c24501c421d..a2d9d87e321 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/FileDecoderProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/FileDecoderProvider.scala @@ -565,6 +565,7 @@ final class FileDecoderProvider( propagateError = true, logInfo = false, ) + .future .map(_ => { if (sbErr.nonEmpty) DecoderResponse.failed(path.toURI, sbErr.toString) @@ -675,6 +676,7 @@ final class FileDecoderProvider( else DecoderResponse.success(path.toURI, sbOut.toString) }) + .future } catch { case NonFatal(e) => scribe.error(e.toString()) diff --git a/metals/src/main/scala/scala/meta/internal/metals/Interruptable.scala b/metals/src/main/scala/scala/meta/internal/metals/Interruptable.scala index 2b3d7cc3f2c..862af05f359 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Interruptable.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Interruptable.scala @@ -1,7 +1,5 @@ package scala.meta.internal.metals -import java.util.concurrent.CompletableFuture - import scala.concurrent.ExecutionContext import scala.concurrent.Future import scala.concurrent.Promise @@ -10,47 +8,76 @@ import scala.meta.internal.metals.Interruptable.CancelConnectException class Interruptable[+T] private ( futureIn: Future[T], - cancelPromise: Promise[Unit], -) extends CompletableFuture { + val cancelable: Cancelable, +) { - def future(implicit executor: ExecutionContext): Future[T] = futureIn.map( - if (cancelPromise.isCompleted) throw CancelConnectException else _ + def future(implicit + executor: ExecutionContext, + cancelPromise: CancelSwitch, + ): Future[T] = futureIn.map( + if (cancelPromise.promise.isCompleted) throw CancelConnectException else _ ) - override def cancel(mayInterruptIfRunning: Boolean): Boolean = { - cancelPromise.trySuccess(()) - true - } - - override def isCancelled(): Boolean = cancelPromise.isCompleted - def flatMap[S]( f: T => Interruptable[S] - )(implicit executor: ExecutionContext): Interruptable[S] = - new Interruptable(future.flatMap(f(_).future), cancelPromise) + )(implicit + executor: ExecutionContext, + cancelPromise: CancelSwitch, + ): Interruptable[S] = { + val mutCancel = + cancelable match { + case c: MutableCancelable => c + case c => new MutableCancelable().add(c) + } + val newFuture = future.flatMap { res => + val i = f(res) + mutCancel.add(i.cancelable) + i.future + } + new Interruptable(newFuture, mutCancel) + } def map[S]( f: T => S - )(implicit executor: ExecutionContext): Interruptable[S] = - new Interruptable(future.map(f(_)), cancelPromise) + )(implicit + executor: ExecutionContext, + cancelPromise: CancelSwitch, + ): Interruptable[S] = + new Interruptable(future.map(f(_)), cancelable) def recover[U >: T]( pf: PartialFunction[Throwable, U] - )(implicit executor: ExecutionContext): Interruptable[U] = { + )(implicit + executor: ExecutionContext, + cancelPromise: CancelSwitch, + ): Interruptable[U] = { val pf0: PartialFunction[Throwable, U] = { case CancelConnectException => throw CancelConnectException } - new Interruptable(future.recover(pf0.orElse(pf)), cancelPromise) + new Interruptable(future.recover(pf0.orElse(pf)), cancelable) } + + def toCancellable(implicit cancelPromise: CancelSwitch): CancelableFuture[T] = + CancelableFuture( + futureIn, + () => { cancelPromise.promise.trySuccess(()); cancelable.cancel() }, + ) } object Interruptable { def successful[T](result: T) = - new Interruptable(Future.successful(result), Promise()) + new Interruptable(Future.successful(result), Cancelable.empty) object CancelConnectException extends RuntimeException implicit class XtensionFuture[+T](future: Future[T]) { - def withInterrupt(cancelPromise: Promise[Unit]): Interruptable[T] = - new Interruptable(future, cancelPromise) + def withInterrupt: Interruptable[T] = + new Interruptable(future, Cancelable.empty) + } + + implicit class XtensionCancelFuture[+T](future: CancelableFuture[T]) { + def withInterrupt: Interruptable[T] = + new Interruptable(future.future, future.cancelable) } } + +case class CancelSwitch(promise: Promise[Unit]) extends AnyVal diff --git a/tests/slow/src/test/scala/tests/bazel/BazelLspSuite.scala b/tests/slow/src/test/scala/tests/bazel/BazelLspSuite.scala index 2f1fd0cb7f8..7a2fef9e181 100644 --- a/tests/slow/src/test/scala/tests/bazel/BazelLspSuite.scala +++ b/tests/slow/src/test/scala/tests/bazel/BazelLspSuite.scala @@ -295,16 +295,18 @@ class BazelLspSuite def jsonFile = workspace.resolve(Directories.bsp).resolve("bazelbsp.json").readText for { - _ <- shellRunner.runJava( - Dependency.of( - BazelBuildTool.dependency.getModule(), - "3.2.0-20240508-f3a81e7-NIGHTLY", - ), - BazelBuildTool.mainClass, - workspace, - BazelBuildTool.projectViewArgs(workspace), - None, - ) + _ <- shellRunner + .runJava( + Dependency.of( + BazelBuildTool.dependency.getModule(), + "3.2.0-20240508-f3a81e7-NIGHTLY", + ), + BazelBuildTool.mainClass, + workspace, + BazelBuildTool.projectViewArgs(workspace), + None, + ) + .future _ = assertContains(jsonFile, "3.2.0-20240508-f3a81e7-NIGHTLY") _ <- initialize( BazelBuildLayout(workspaceLayout, V.bazelScalaVersion, bazelVersion)