From a1ecf2c77c1c58feba13060f5aa1718499572a74 Mon Sep 17 00:00:00 2001
From: Tomasz Godzik <tgodzik@virtuslab.com>
Date: Fri, 4 Aug 2023 20:18:12 +0200
Subject: [PATCH] bugfix: Fix issues with select in debugger completions

Previously, we set the wrong start of the expression since we actually need to calculate that when insertText is used. Now, we calculate it based on the current identifier.

We should use TextEdit in the future, which should work better. I also realized that start actually needs to take into account if the client starts at 1 or 0.
---
 .../meta/internal/metals/Compilers.scala      | 27 ++++++++++++++++---
 .../debug/ClientConfigurationAdapter.scala    |  2 +-
 .../internal/metals/debug/DebugProxy.scala    |  1 +
 .../tests/debug/CompletionDapSuite.scala      |  2 +-
 4 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala b/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala
index 46ae1900bc0..5dece6e521a 100644
--- a/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala
+++ b/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala
@@ -288,11 +288,22 @@ class Compilers(
       Nil
     }
 
+  /**
+   * Calculates completions for a expression evaluator at breakpointPosition
+   *
+   * @param path path to file containing ht ebreakpoint
+   * @param breakpointPosition actual breakpoint position
+   * @param token cancel token for the compiler
+   * @param expression expression that is currently being types
+   * @param isZeroBased whether the client supports starting at 0 or 1 index
+   * @return
+   */
   def debugCompletions(
       path: AbsolutePath,
       breakpointPosition: LspPosition,
       token: CancelToken,
       expression: d.CompletionsArguments,
+      isZeroBased: Boolean,
   ): Future[Seq[d.CompletionItem]] = {
 
     /**
@@ -342,6 +353,16 @@ class Compilers(
 
             val rangeEnd =
               lineStart + expressionOffset(expressionText, indentation) + 1
+
+            /**
+             * Calculate the start if insertText is used for item, which does not declare an exact start.
+             */
+            def insertStart = {
+              var i = rangeEnd - 1
+              while (modified.charAt(i).isLetterOrDigit) i -= 1
+              if (isZeroBased) i else i + 1
+            }
+
             val offsetParams = CompilerOffsetParams(
               path.toURI,
               modified,
@@ -377,8 +398,7 @@ class Compilers(
                       adjustStart,
                       Position.Range(
                         input.copy(value = modified),
-                        // account for the added ;
-                        lineStart + 1,
+                        insertStart,
                         rangeEnd,
                       ),
                     )
@@ -1140,7 +1160,7 @@ class Compilers(
   private def toDebugCompletionItem(
       item: CompletionItem,
       adjustStart: Int,
-      insertTextPosition: Position,
+      insertTextPosition: Position.Range,
   ): d.CompletionItem = {
     val debugItem = new d.CompletionItem()
     debugItem.setLabel(item.getLabel())
@@ -1174,6 +1194,7 @@ class Compilers(
       debugItem.setSelectionStart(selection)
     }
 
+    debugItem.setDetail(item.getDetail())
     debugItem.setText(fullText.replace("$0", ""))
     debugItem.setStart(start)
     debugItem.setType(toDebugCompletionType(item.getKind()))
diff --git a/metals/src/main/scala/scala/meta/internal/metals/debug/ClientConfigurationAdapter.scala b/metals/src/main/scala/scala/meta/internal/metals/debug/ClientConfigurationAdapter.scala
index 5a148c7ecfb..db97f3ac5bf 100644
--- a/metals/src/main/scala/scala/meta/internal/metals/debug/ClientConfigurationAdapter.scala
+++ b/metals/src/main/scala/scala/meta/internal/metals/debug/ClientConfigurationAdapter.scala
@@ -20,7 +20,7 @@ import org.eclipse.lsp4j.debug.SourceBreakpoint
  */
 private[debug] final case class ClientConfigurationAdapter(
     pathFormat: String,
-    linesStartAt1: Boolean,
+    val linesStartAt1: Boolean,
     sourceMapper: SourceMapper,
 ) {
   // The scala-debug-adapter uses the JVM class file format
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 95b3da6b558..a8fae0395cf 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
@@ -135,6 +135,7 @@ private[debug] final class DebugProxy(
           new Position(frame.getLine() - 1, 0),
           EmptyCancelToken,
           args,
+          isZeroBased = !clientAdapter.linesStartAt1,
         )
       }
       completions
diff --git a/tests/unit/src/test/scala/tests/debug/CompletionDapSuite.scala b/tests/unit/src/test/scala/tests/debug/CompletionDapSuite.scala
index 2a31db0fe7e..8a3ed3bbf7e 100644
--- a/tests/unit/src/test/scala/tests/debug/CompletionDapSuite.scala
+++ b/tests/unit/src/test/scala/tests/debug/CompletionDapSuite.scala
@@ -250,7 +250,7 @@ class CompletionDapSuite
            |scale(int arg0, float arg1)
            |startsWith(java.lang.String arg0, int arg1)
            |""".stripMargin,
-    expectedEdit = "serialVersionUID",
+    expectedEdit = "name.serialVersionUID",
     topLines = Some(5),
   )(
     """|/a/src/main/java/a/Main.java