diff --git a/metals/src/main/scala/scala/meta/internal/metals/Messages.scala b/metals/src/main/scala/scala/meta/internal/metals/Messages.scala index 317091cb03c..779636c0aa5 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Messages.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Messages.scala @@ -585,31 +585,6 @@ object Messages { show = true, ) - object DebugClassNotFound { - - def invalidTargetClass(cls: String, target: String): MessageParams = { - new MessageParams( - MessageType.Error, - s"Class '$cls' not found in build target '$target'.", - ) - } - - def invalidTarget(target: String): MessageParams = { - new MessageParams( - MessageType.Error, - s"Target '$target' not found.", - ) - } - - def invalidClass(cls: String): MessageParams = { - new MessageParams( - MessageType.Error, - s"Class '$cls' not found.", - ) - } - - } - object MissingScalafmtConf { def tryAgain(isAgain: Boolean): String = if (isAgain) ", try formatting again" diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala index 0129b27ed2e..b31969117b4 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala @@ -58,6 +58,8 @@ import com.google.gson.JsonObject import fansi.ErrorMode import io.undertow.server.HttpServerExchange import org.eclipse.lsp4j.TextDocumentIdentifier +import org.eclipse.lsp4j.jsonrpc.ResponseErrorException +import org.eclipse.lsp4j.jsonrpc.messages import org.eclipse.{lsp4j => l} /** @@ -203,6 +205,17 @@ object MetalsEnrichments def asJavaObject: CompletableFuture[Object] = future.asJava.asInstanceOf[CompletableFuture[Object]] + def liftToLspError(implicit ec: ExecutionContext): Future[A] = + future.recoverWith { case NonFatal(e) => + val newException = new ResponseErrorException( + new messages.ResponseError( + messages.ResponseErrorCode.InvalidRequest, + e.getMessage(), + null, + ) + ) + Future.failed(newException) + } def asJavaUnit(implicit ec: ExecutionContext): CompletableFuture[Unit] = future.ignoreValue.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..5d6a1359046 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala @@ -1904,11 +1904,10 @@ class MetalsLspService( def analyzeStackTrace(content: String): Option[ExecuteCommandParams] = stacktraceAnalyzer.analyzeCommand(content) - def debugDiscovery(params: DebugDiscoveryParams): CompletableFuture[Object] = + def debugDiscovery(params: DebugDiscoveryParams): Future[DebugSession] = debugProvider .debugDiscovery(params) .flatMap(debugProvider.asSession) - .asJavaObject def findBuildTargetByDisplayName(target: String): Option[b.BuildTarget] = buildTargets.findByDisplayName(target) diff --git a/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala index def71ee16f9..799e019bf62 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala @@ -23,8 +23,6 @@ import scala.meta.internal.metals.WindowStateDidChangeParams import scala.meta.internal.metals.clients.language.ConfiguredLanguageClient import scala.meta.internal.metals.clients.language.MetalsLanguageClient import scala.meta.internal.metals.config.StatusBarState -import scala.meta.internal.metals.debug.BuildTargetNotFoundException -import scala.meta.internal.metals.debug.BuildTargetUndefinedException import scala.meta.internal.metals.debug.DebugProvider import scala.meta.internal.metals.doctor.DoctorVisibilityDidChangeParams import scala.meta.internal.metals.doctor.HeadDoctor @@ -93,6 +91,7 @@ import org.eclipse.lsp4j.TextDocumentPositionParams import org.eclipse.lsp4j.TextEdit import org.eclipse.lsp4j.WorkspaceEdit import org.eclipse.lsp4j.WorkspaceSymbolParams +import org.eclipse.lsp4j.jsonrpc.ResponseErrorException import org.eclipse.lsp4j.jsonrpc.messages class WorkspaceLspService( @@ -639,10 +638,20 @@ class WorkspaceLspService( folderServices.foreach(_.pause()) } - private def displayNotFound(objectName: String, id: String) = - languageClient.showMessage( - Messages.errorMessageParams(s"$objectName not found: $id") - ) + private def failedRequest( + message: String + ): Future[Object] = { + Future + .failed( + new ResponseErrorException( + new messages.ResponseError( + messages.ResponseErrorCode.InvalidParams, + message, + null, + ) + ) + ) + } private def onFirstSatifying[T, R](mapTo: MetalsLspService => Future[T])( satisfies: T => Boolean, @@ -842,21 +851,16 @@ class WorkspaceLspService( ) match { case Some(service) => service.startDebugProvider(params).asJavaObject case None => - languageClient.showMessage( - Messages.errorMessageParams( - s"Could not find folder for build targets: ${targets.mkString(",")}" - ) - ) - Future.failed(DebugProvider.WorkspaceErrorsException).asJavaObject + failedRequest( + s"Could not find folder for build targets: ${targets.mkString(",")}" + ).asJavaObject } case ServerCommands.StartMainClass(params) if params.mainClass != null => DebugProvider .getResultFromSearches( folderServices.map(_.mainClassSearch(params)) - ) { - displayNotFound("Main class", params.mainClass) - Future.failed(new ju.NoSuchElementException(params.mainClass)) - } + ) + .liftToLspError .asJavaObject case ServerCommands.StartTestSuite(params) @@ -867,20 +871,15 @@ class WorkspaceLspService( _.isDefined, (service, someTarget) => service.startTestSuite(someTarget.get, params), - () => { - displayNotFound("Build target", params.target.toString()) - Future.failed(BuildTargetNotFoundException(params.target.getUri)) - }, + () => failedRequest(s"Could not find '${params.target}' build target"), ).asJavaObject case ServerCommands.ResolveAndStartTestSuite(params) if params.testClass != null => DebugProvider .getResultFromSearches( folderServices.map(_.testClassSearch(params)) - ) { - displayNotFound("Test class", params.testClass) - Future.failed(new ju.NoSuchElementException(params.testClass)) - } + ) + .liftToLspError .asJavaObject case ServerCommands.StartAttach(params) if params.hostName != null => onFirstSatifying(service => @@ -891,13 +890,16 @@ class WorkspaceLspService( _.isDefined, (service, someTarget) => service.createDebugSession(someTarget.get.getId()), - () => { - displayNotFound("Build target", params.buildTarget) - Future.failed(BuildTargetUndefinedException()) - }, + () => + failedRequest( + s"Could not find '${params.buildTarget}' build target" + ), ).asJavaObject case ServerCommands.DiscoverAndRun(params) => - getServiceFor(params.path).debugDiscovery(params) + getServiceFor(params.path) + .debugDiscovery(params) + .liftToLspError + .asJavaObject case ServerCommands.AnalyzeStacktrace(content) => Future { // Getting the service for focused document and first one otherwise diff --git a/metals/src/main/scala/scala/meta/internal/metals/debug/BuildTargetClassesFinder.scala b/metals/src/main/scala/scala/meta/internal/metals/debug/BuildTargetClassesFinder.scala index 1d13cd66872..11931f706f2 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/debug/BuildTargetClassesFinder.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/debug/BuildTargetClassesFinder.scala @@ -39,8 +39,9 @@ class BuildTargetClassesFinder( // We check whether there is a main in dependencies that is not reported via BSP case ClassNotFoundInBuildTargetException(className, target) => revertToDependencies(className, Some(target)) - case _: ClassNotFoundException => + case _: NoClassFoundException => revertToDependencies(className, buildTarget = None) + case _ => Nil } found match { case Nil => Failure(ex) @@ -109,7 +110,7 @@ class BuildTargetClassesFinder( } .reverse if (classes.nonEmpty) Success(classes) - else Failure(new ClassNotFoundException(className)) + else Failure(new NoClassFoundException(className)) } { targetName => buildTargets .findByDisplayName(targetName) @@ -148,15 +149,19 @@ class BuildTargetClassesFinder( case class BuildTargetNotFoundException(buildTargetName: String) extends Exception(s"Build target not found: $buildTargetName") -case class BuildTargetUndefinedException() - extends Exception("Debugger configuration is missing 'buildTarget' param.") - case class ClassNotFoundInBuildTargetException( className: String, buildTarget: b.BuildTarget, ) extends Exception( s"Class '$className' not found in build target '${buildTarget.getDisplayName()}'" ) + +case class NoClassFoundException( + className: String +) extends Exception( + s"Class '$className' not found in any build target" + ) + case class BuildTargetNotFoundForPathException(path: AbsolutePath) extends Exception( s"No build target could be found for the path: ${path.toString()}" diff --git a/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProvider.scala index d97b8199217..2022b56bef6 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProvider.scala @@ -688,40 +688,6 @@ class DebugProvider( languageClient.metalsStatus( Messages.DebugErrorsPresent(clientConfig.icons()) ) - case t: ClassNotFoundException => - languageClient.showMessage( - Messages.DebugClassNotFound.invalidClass(t.getMessage()) - ) - case ClassNotFoundInBuildTargetException(cls, buildTarget) => - languageClient.showMessage( - Messages.DebugClassNotFound - .invalidTargetClass(cls, buildTarget.getDisplayName()) - ) - case BuildTargetNotFoundException(target) => - languageClient.showMessage( - Messages.DebugClassNotFound - .invalidTarget(target) - ) - case e: BuildTargetNotFoundForPathException => - languageClient.showMessage( - Messages.errorMessageParams(e.getMessage()) - ) - case e: BuildTargetContainsNoMainException => - languageClient.showMessage( - Messages.errorMessageParams(e.getMessage()) - ) - case e: BuildTargetUndefinedException => - languageClient.showMessage( - Messages.errorMessageParams(e.getMessage()) - ) - case e: NoTestsFoundException => - languageClient.showMessage( - Messages.errorMessageParams(e.getMessage()) - ) - case e: RunType.UnknownRunTypeException => - languageClient.showMessage( - Messages.errorMessageParams(e.getMessage()) - ) case e @ SemanticDbNotFoundException => languageClient.metalsStatus( MetalsStatusParams( @@ -730,13 +696,7 @@ class DebugProvider( command = ClientCommands.RunDoctor.id, ) ) - case e @ DotEnvFileParser.InvalidEnvFileException(_) => - languageClient.showMessage(Messages.errorMessageParams(e.getMessage())) - - case e @ NoRunOptionException => - languageClient.showMessage( - Messages.errorMessageParams(e.getMessage()) - ) + case _ => } private def parseSessionName( @@ -829,7 +789,7 @@ class DebugProvider( _ <- buildTargetClasses.rebuildIndex(target) result <- Future(f()) } yield result - case Failure(_: ClassNotFoundException) => + case Failure(_: NoClassFoundException) => val allTargetIds = buildTargets.allBuildTargetIds for { _ <- compilations.compileTargets(allTargetIds) @@ -867,6 +827,26 @@ class DebugProvider( object DebugProvider { + private def exceptionOrder(t: Throwable): Int = t match { + /* Validation errors, should shown first as they are relevant for each build target. + */ + case DotEnvFileParser.InvalidEnvFileException(_) => 0 + case NoRunOptionException => 1 + case _: RunType.UnknownRunTypeException => 2 + /* Target found, but class not, means that we managed to find the proper workspace folder + */ + case _: ClassNotFoundInBuildTargetException => 3 + case _: BuildTargetContainsNoMainException => 4 + case _: NoTestsFoundException => 5 + /* These exception will show up and it's hard to pinpoint which folder we should be in + * since we failed to fin the exception anywhere. Probably just showing the error is enough. + */ + case _: BuildTargetNotFoundForPathException => 6 + case _: NoClassFoundException => 7 + case _: BuildTargetNotFoundException => 8 + case _ => 9 + } + /** * Given an occurence and a text document return the symbol of a main method * that could be defined using the Scala 3 @main annotation. @@ -958,16 +938,16 @@ object DebugProvider { protected def dapSessionParams(res: A): Future[DebugSessionParams] def createDapSession(args: A): Future[DebugSession] = dapSessionParams(args).flatMap(debugProvider.asSession(_)) - def searchResult: Future[(Option[A], ClassSearch[A])] = { + def searchResult: Future[(Try[A], ClassSearch[A])] = { Future { val resolved = search() searchPromise.trySuccess(resolved) } - searchPromise.future.map(e => (e.toOption, this)) + searchPromise.future.map(e => (e, this)) } - def retrySearchResult: Future[(Option[A], ClassSearch[A])] = + def retrySearchResult: Future[(Try[A], ClassSearch[A])] = searchPromise.future - .flatMap(debugProvider.retryAfterRebuild(_, search).map(_.toOption)) + .flatMap(debugProvider.retryAfterRebuild(_, search)) .map((_, this)) } @@ -1000,27 +980,32 @@ object DebugProvider { debugProvider.buildTestClassParams(res, params) } - def getResultFromSearches[A](searches: List[ClassSearch[A]])( - default: => Future[DebugSession] + def getResultFromSearches[A]( + searches: List[ClassSearch[A]] )(implicit ec: ExecutionContext): Future[DebugSession] = Future .sequence(searches.map(_.searchResult)) - .getFirstOrElse( - Future - .sequence(searches.map(_.retrySearchResult)) - .getFirstOrElse(default) - ) + .getFirstOrError + .recoverWith { case _ => + Future.sequence(searches.map(_.retrySearchResult)).getFirstOrError + } private implicit class FindFirstDebugSession[A]( - from: Future[List[(Option[A], ClassSearch[A])]] + from: Future[List[(Try[A], ClassSearch[A])]] ) { - def getFirstOrElse( - default: => Future[DebugSession] - )(implicit ec: ExecutionContext): Future[DebugSession] = - from.flatMap { - _.collectFirst { case (Some(dapSession), search) => - search.createDapSession(dapSession) - }.getOrElse(default) + def getFirstOrError(implicit ec: ExecutionContext): Future[DebugSession] = + from.flatMap { all => + def mostRelevantError() = all + .collect { case (Failure(error), _) => + error + } + .sortBy(DebugProvider.exceptionOrder) + .head + all + .collectFirst { case (Success(dapSession), search) => + search.createDapSession(dapSession) + } + .getOrElse(Future.failed(mostRelevantError())) } } } diff --git a/tests/unit/src/test/scala/tests/DebugDiscoverySuite.scala b/tests/unit/src/test/scala/tests/DebugDiscoverySuite.scala index a5d7391649c..22a25d697be 100644 --- a/tests/unit/src/test/scala/tests/DebugDiscoverySuite.scala +++ b/tests/unit/src/test/scala/tests/DebugDiscoverySuite.scala @@ -12,6 +12,8 @@ import scala.meta.internal.metals.debug.DotEnvFileParser.InvalidEnvFileException import scala.meta.internal.metals.debug.NoTestsFoundException import scala.meta.io.AbsolutePath +import org.eclipse.lsp4j.jsonrpc.ResponseErrorException + // note(@tgodzik) all test have `System.exit(0)` added to avoid occasional issue due to: // https://stackoverflow.com/questions/2225737/error-jdwp-unable-to-get-jni-1-2-environment class DebugDiscoverySuite @@ -148,10 +150,10 @@ class DebugDiscoverySuite "runOrTestFile", ).toJson ) - .recover { case e @ DebugProvider.NoRunOptionException => e } + .recover { case e: ResponseErrorException => e.getMessage } } yield assertNoDiff( result.toString(), - DebugProvider.NoRunOptionException.toString(), + DebugProvider.NoRunOptionException.getMessage(), ) } @@ -215,12 +217,10 @@ class DebugDiscoverySuite "run", ).toJson ) - .recover { case e: BuildTargetContainsNoMainException => - e - } + .recover { case e: ResponseErrorException => e.getMessage } } yield assertNoDiff( result.toString, - BuildTargetContainsNoMainException("a").toString(), + BuildTargetContainsNoMainException("a").getMessage(), ) } @@ -247,12 +247,10 @@ class DebugDiscoverySuite "run", ).toJson ) - .recover { case WorkspaceErrorsException => - WorkspaceErrorsException - } + .recover { case e: ResponseErrorException => e.getMessage } } yield assertNoDiff( result.toString, - WorkspaceErrorsException.toString(), + WorkspaceErrorsException.getMessage(), ) } @@ -324,10 +322,10 @@ class DebugDiscoverySuite envFile = fakePath, ).toJson ) - .recover { case e: InvalidEnvFileException => e } + .recover { case e: ResponseErrorException => e.getMessage } } yield assertNoDiff( result.toString, - InvalidEnvFileException(AbsolutePath(fakePath)).toString(), + InvalidEnvFileException(AbsolutePath(fakePath)).getMessage(), ) } @@ -426,10 +424,10 @@ class DebugDiscoverySuite "testTarget", ).toJson ) - .recover { case e: NoTestsFoundException => e } + .recover { case e: ResponseErrorException => e.getMessage } } yield assertNoDiff( result.toString(), - NoTestsFoundException("build target", "a").toString(), + NoTestsFoundException("build target", "a").getMessage(), ) } @@ -460,12 +458,10 @@ class DebugDiscoverySuite "testFile", ).toJson ) - .recover { case SemanticDbNotFoundException => - SemanticDbNotFoundException - } + .recover { case e: ResponseErrorException => e.getMessage } } yield assertNoDiff( result.toString, - SemanticDbNotFoundException.toString(), + SemanticDbNotFoundException.getMessage(), ) } } diff --git a/tests/unit/src/test/scala/tests/DebugProtocolSuite.scala b/tests/unit/src/test/scala/tests/DebugProtocolSuite.scala index 43505140713..5cc9e4abd37 100644 --- a/tests/unit/src/test/scala/tests/DebugProtocolSuite.scala +++ b/tests/unit/src/test/scala/tests/DebugProtocolSuite.scala @@ -16,6 +16,7 @@ import scala.meta.internal.metals.debug.DebugProvider.WorkspaceErrorsException import ch.epfl.scala.bsp4j.DebugSessionParamsDataKind import ch.epfl.scala.bsp4j.ScalaMainClass +import org.eclipse.lsp4j.jsonrpc.ResponseErrorException // note(@tgodzik) all test have `System.exit(0)` added to avoid occasional issue due to: // https://stackoverflow.com/questions/2225737/error-jdwp-unable-to-get-jni-1-2-environment @@ -410,12 +411,12 @@ class DebugProtocolSuite singletonList("Foo"), ).toJson ) - .recover { case WorkspaceErrorsException => - WorkspaceErrorsException + .recover { case e: ResponseErrorException => + e.getMessage() } - } yield assertDiffEqual( + } yield assertNoDiff( result.toString(), - WorkspaceErrorsException.toString(), + WorkspaceErrorsException.getMessage(), ) } @@ -484,12 +485,12 @@ class DebugProtocolSuite "a.Foo" ).toJson ) - .recover { case WorkspaceErrorsException => - WorkspaceErrorsException + .recover { case e: ResponseErrorException => + e.getMessage() } - } yield assertContains( + } yield assertNoDiff( result.toString(), - WorkspaceErrorsException.toString(), + WorkspaceErrorsException.getMessage(), ) } }