From 1a664e80631908550157f323bf206b4bf97063ab Mon Sep 17 00:00:00 2001 From: Adrien Piquerez Date: Mon, 8 Jan 2024 13:33:20 +0100 Subject: [PATCH] Handle missing jar file in SetBreakpointRequest --- .../internal/metals/debug/DebugProxy.scala | 59 ++++++++++++------ .../metals/debug/FileBreakpoints.scala | 31 ++++++++-- .../internal/metals/debug/TestDebugger.scala | 6 +- .../src/main/scala/tests/BaseDapSuite.scala | 6 +- .../main/scala/tests/DapTestEnrichments.scala | 20 ------- .../tests/debug/BaseBreakpointDapSuite.scala | 60 ++++++++++++++++++- 6 files changed, 130 insertions(+), 52 deletions(-) delete mode 100644 tests/unit/src/main/scala/tests/DapTestEnrichments.scala diff --git a/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProxy.scala b/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProxy.scala index 9bbb77f2238..234ae57d7cb 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProxy.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProxy.scala @@ -7,6 +7,9 @@ import java.util.concurrent.atomic.AtomicBoolean import scala.concurrent.ExecutionContext import scala.concurrent.Future import scala.concurrent.Promise +import scala.util.Failure +import scala.util.Success +import scala.util.Try import scala.util.control.NonFatal import scala.meta.internal.metals.Cancelable @@ -29,6 +32,7 @@ import scala.meta.internal.metals.debug.DebugProxy._ import scala.meta.io.AbsolutePath import org.eclipse.lsp4j.Position +import org.eclipse.lsp4j.debug.Breakpoint import org.eclipse.lsp4j.debug.CompletionsResponse import org.eclipse.lsp4j.debug.SetBreakpointsResponse import org.eclipse.lsp4j.debug.Source @@ -104,26 +108,43 @@ private[debug] final class DebugProxy( client.consume(DebugProtocol.syntheticResponse(request, response)) case request @ SetBreakpointRequest(args) => val originalSource = DebugProtocol.copy(args.getSource) - val metalsSourcePath = clientAdapter.toMetalsPath(originalSource.getPath) - args.getBreakpoints.foreach { breakpoint => - val line = clientAdapter.normalizeLineForServer( - metalsSourcePath, - breakpoint.getLine, - ) - breakpoint.setLine(line) + Try(clientAdapter.toMetalsPath(originalSource.getPath)) match { + case Success(metalsSourcePath) => + args.getBreakpoints.foreach { breakpoint => + val line = clientAdapter.normalizeLineForServer( + metalsSourcePath, + breakpoint.getLine, + ) + breakpoint.setLine(line) + } + val requests = + debugAdapter.adaptSetBreakpointsRequest(metalsSourcePath, args) + server + .sendPartitioned(requests.map(DebugProtocol.syntheticRequest)) + .map(_.map(DebugProtocol.parseResponse[SetBreakpointsResponse])) + .map(_.flatMap(_.toList)) + .map(assembleResponse(_, originalSource, metalsSourcePath)) + .map(DebugProtocol.syntheticResponse(request, _)) + .foreach(client.consume) + case Failure(_) => + scribe.warn( + s"Cannot adapt SetBreakpointRequest because of invalid path: ${originalSource.getPath}" + ) + val breakpoints = args.getBreakpoints.map { sourceBreakpoint => + val breakpoint = new Breakpoint + breakpoint.setLine(sourceBreakpoint.getLine) + breakpoint.setColumn(sourceBreakpoint.getColumn) + breakpoint.setSource(originalSource) + breakpoint.setVerified(false) + breakpoint.setMessage(s"Invalid path: ${originalSource.getPath}") + breakpoint + } + val response = new SetBreakpointsResponse + response.setBreakpoints(breakpoints) + client.consume(DebugProtocol.syntheticResponse(request, response)) } - val requests = - debugAdapter.adaptSetBreakpointsRequest(metalsSourcePath, args) - server - .sendPartitioned(requests.map(DebugProtocol.syntheticRequest)) - .map(_.map(DebugProtocol.parseResponse[SetBreakpointsResponse])) - .map(_.flatMap(_.toList)) - .map(assembleResponse(_, originalSource)) - .map(DebugProtocol.syntheticResponse(request, _)) - .foreach(client.consume) - case request @ CompletionRequest(args) => val completions = for { frame <- lastFrames.find(_.getId() == args.getFrameId()) @@ -161,14 +182,14 @@ private[debug] final class DebugProxy( private def assembleResponse( responses: Iterable[SetBreakpointsResponse], originalSource: Source, + metalsSourcePath: AbsolutePath, ): SetBreakpointsResponse = { val breakpoints = for { response <- responses breakpoint <- response.getBreakpoints } yield { - val sourcePath = clientAdapter.toMetalsPath(originalSource.getPath) val line = - clientAdapter.adaptLineForClient(sourcePath, breakpoint.getLine) + clientAdapter.adaptLineForClient(metalsSourcePath, breakpoint.getLine) breakpoint.setSource(originalSource) breakpoint.setLine(line) breakpoint diff --git a/tests/unit/src/main/scala/scala/meta/internal/metals/debug/FileBreakpoints.scala b/tests/unit/src/main/scala/scala/meta/internal/metals/debug/FileBreakpoints.scala index fdecb46cd2e..c34eca60276 100644 --- a/tests/unit/src/main/scala/scala/meta/internal/metals/debug/FileBreakpoints.scala +++ b/tests/unit/src/main/scala/scala/meta/internal/metals/debug/FileBreakpoints.scala @@ -1,10 +1,21 @@ package scala.meta.internal.metals.debug +import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.io.AbsolutePath +import org.eclipse.lsp4j.debug.Source + sealed trait FileBreakpoints { def breakpoints: List[Int] - def path: AbsolutePath + def sourceName: String + def sourcePath: String + + def source: Source = { + val source = new Source + source.setName(sourceName) + source.setPath(sourcePath) + source + } } final case class LocalFileBreakpoints( @@ -14,15 +25,20 @@ final case class LocalFileBreakpoints( breakpoints: List[Int], ) extends FileBreakpoints { - override def path: AbsolutePath = root.resolve(relativePath) + private def path: AbsolutePath = root.resolve(relativePath) + override def sourceName: String = path.filename + override def sourcePath: String = path.toString override def toString: String = s"""|/$relativePath |$content |""".stripMargin } -final case class LibraryBreakpoints(path: AbsolutePath, breakpoints: List[Int]) - extends FileBreakpoints +final case class LibraryBreakpoints( + sourceName: String, + sourcePath: String, + breakpoints: List[Int], +) extends FileBreakpoints object FileBreakpoints { def apply( @@ -39,4 +55,11 @@ object FileBreakpoints { LocalFileBreakpoints(root, name, text, breakpoints) } + + def apply(path: AbsolutePath, breakpoints: List[Int]): LibraryBreakpoints = { + val sourcePath = + if (path.isJarFileSystem) path.toURI.toString + else path.toString + LibraryBreakpoints(path.filename, sourcePath, breakpoints) + } } diff --git a/tests/unit/src/main/scala/scala/meta/internal/metals/debug/TestDebugger.scala b/tests/unit/src/main/scala/scala/meta/internal/metals/debug/TestDebugger.scala index 13f2a9bdeee..1802ed20e2c 100644 --- a/tests/unit/src/main/scala/scala/meta/internal/metals/debug/TestDebugger.scala +++ b/tests/unit/src/main/scala/scala/meta/internal/metals/debug/TestDebugger.scala @@ -14,14 +14,13 @@ import scala.util.Success import scala.meta.internal.metals.Debug import scala.meta.internal.metals.MetalsEnrichments._ -import scala.meta.io.AbsolutePath import org.eclipse.lsp4j.debug.Capabilities import org.eclipse.lsp4j.debug.OutputEventArguments import org.eclipse.lsp4j.debug.SetBreakpointsResponse +import org.eclipse.lsp4j.debug.Source import org.eclipse.lsp4j.debug.SourceBreakpoint import org.eclipse.lsp4j.debug.StoppedEventArguments -import tests.DapTestEnrichments._ final class TestDebugger( connect: RemoteServer.Listener => Debugger, @@ -56,10 +55,9 @@ final class TestDebugger( } def setBreakpoints( - path: AbsolutePath, + source: Source, positions: List[Int], ): Future[SetBreakpointsResponse] = { - val source = path.toDAP val breakpoints = positions.map { line => val breakpoint = new SourceBreakpoint breakpoint.setLine(line + 1) // breakpoints are 1-based diff --git a/tests/unit/src/main/scala/tests/BaseDapSuite.scala b/tests/unit/src/main/scala/tests/BaseDapSuite.scala index f0215b35f64..87fa6a4a08e 100644 --- a/tests/unit/src/main/scala/tests/BaseDapSuite.scala +++ b/tests/unit/src/main/scala/tests/BaseDapSuite.scala @@ -81,7 +81,7 @@ abstract class BaseDapSuite( workspace.filesBreakpoints .filter(_.breakpoints.nonEmpty) .map { file => - debugger.setBreakpoints(file.path, file.breakpoints) + debugger.setBreakpoints(file.source, file.breakpoints) } } } @@ -94,7 +94,7 @@ abstract class BaseDapSuite( workspace.filesBreakpoints .filter(_.breakpoints.nonEmpty) .map { file => - debugger.setBreakpoints(file.path, Nil) + debugger.setBreakpoints(file.source, Nil) } } } @@ -148,7 +148,7 @@ abstract class BaseDapSuite( ): StepNavigator = { val expectedBreakpoints = workspaceLayout.filesBreakpoints.flatMap { file => - file.breakpoints.map(line => Breakpoint(file.path.toString(), line)) + file.breakpoints.map(line => Breakpoint(file.sourcePath, line)) } expectedBreakpoints.foldLeft(StepNavigator(workspace)) { diff --git a/tests/unit/src/main/scala/tests/DapTestEnrichments.scala b/tests/unit/src/main/scala/tests/DapTestEnrichments.scala deleted file mode 100644 index a13029aa2c7..00000000000 --- a/tests/unit/src/main/scala/tests/DapTestEnrichments.scala +++ /dev/null @@ -1,20 +0,0 @@ -package tests -import scala.meta.internal.metals.MetalsEnrichments._ -import scala.meta.io.AbsolutePath - -import org.eclipse.lsp4j.debug.Source - -object DapTestEnrichments { - implicit class DapXtensionAbsolutePath(path: AbsolutePath) { - def toDAP: Source = { - val adaptedPath = - if (path.isJarFileSystem) path.toURI.toString - else path.toString() - val source = new Source - source.setName(path.filename) - source.setPath(adaptedPath) - source - } - } - -} diff --git a/tests/unit/src/main/scala/tests/debug/BaseBreakpointDapSuite.scala b/tests/unit/src/main/scala/tests/debug/BaseBreakpointDapSuite.scala index e71d1707c8c..6f5c835c174 100644 --- a/tests/unit/src/main/scala/tests/debug/BaseBreakpointDapSuite.scala +++ b/tests/unit/src/main/scala/tests/debug/BaseBreakpointDapSuite.scala @@ -1,8 +1,16 @@ package tests.debug +import java.io.FileOutputStream +import java.net.URI +import java.nio.file.FileSystems +import java.nio.file.Files +import java.util.zip.ZipOutputStream +import java.{util => ju} + import scala.meta.internal.metals.debug.DebugWorkspaceLayout -import scala.meta.internal.metals.debug.LibraryBreakpoints +import scala.meta.internal.metals.debug.FileBreakpoints import scala.meta.internal.metals.debug.Stoppage +import scala.meta.io.AbsolutePath import tests.BaseDapSuite import tests.BuildServerInitializer @@ -701,7 +709,7 @@ abstract class BaseBreakpointDapSuite( test("library-breakpoints", withoutVirtualDocs = true) { def debugLayout = new DebugWorkspaceLayout( List( - LibraryBreakpoints( + FileBreakpoints( server .toPathFromSymbol("scala.Predef", "scala/Predef.scala"), List(404), @@ -809,4 +817,52 @@ abstract class BaseBreakpointDapSuite( output <- debugger.allOutput } yield assertNoDiff(output, "1\n2\n3\n") } + + test("breakpoint in a deleted jar file") { + // an empty jar file + val jarFile = Files.createTempFile("metals-test", ".jar") + val output = new ZipOutputStream(new FileOutputStream(jarFile.toFile)) + output.close() + + // a breakpoint in the empty jar filesystem + val uri = URI.create(s"jar:${jarFile.toUri}") + val fileSystem = FileSystems.newFileSystem(uri, new ju.HashMap[String, Any]) + val unknownPath = AbsolutePath(fileSystem.getPath("/unknown.scala")) + val wrongDebugLayout = new DebugWorkspaceLayout( + List(FileBreakpoints(unknownPath, List(404))) + ) + + // delete the jar file + fileSystem.close() + Files.delete(jarFile) + + val workspaceLayout = buildToolLayout( + """|/a/src/main/scala/a/Main.scala + |package a + |object Main { + | def main(args: Array[String]): Unit = { + | println(0) + | System.exit(0) + | } + |} + |""".stripMargin, + scalaVersion, + ) + + try { + for { + _ <- initialize(workspaceLayout) + debugger <- debugMain("a", "a.Main", Stoppage.Handler.Fail) + _ <- debugger.initialize + _ <- debugger.launch + _ <- setBreakpoints(debugger, wrongDebugLayout) + _ <- debugger.configurationDone + _ <- debugger.shutdown + output <- debugger.allOutput + } yield assertNoDiff(output, "0\n") + } catch { + case e: Exception => + e.printStackTrace() + } + } }