From a7f5758e07889bacac55375446ed22a44a695a05 Mon Sep 17 00:00:00 2001
From: Katarzyna Marek <kasia@marek.net>
Date: Thu, 25 Jul 2024 17:38:41 +0200
Subject: [PATCH] fix: disambiguate workspace completions for vals in Scala 3

---
 .../pc/completions/CompletionValue.scala      | 15 ++--
 .../test/scala/tests/pc/CompletionSuite.scala | 70 ++++++++++++++++++-
 .../tests/pc/CompletionWorkspaceSuite.scala   | 16 +----
 3 files changed, 83 insertions(+), 18 deletions(-)

diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/completions/CompletionValue.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/completions/CompletionValue.scala
index 5a6a30494ac..ec5ed7f6f97 100644
--- a/mtags/src/main/scala-3/scala/meta/internal/pc/completions/CompletionValue.scala
+++ b/mtags/src/main/scala-3/scala/meta/internal/pc/completions/CompletionValue.scala
@@ -78,13 +78,13 @@ object CompletionValue:
     )(using Context): String =
       if symbol.is(Method) then s"${label}${description(printer)}"
       else if symbol.isConstructor then label
-      else if symbol.is(Mutable) then s"$label: ${description(printer)}"
+      else if symbol.is(Mutable) then s"$label${description(printer)}"
       else if symbol.is(Package) || symbol.is(Module) || symbol.isClass then
         if isFromWorkspace then
           s"${labelWithSuffix(printer)} -${description(printer)}"
         else s"${labelWithSuffix(printer)}${description(printer)}"
       else if symbol.isType then labelWithSuffix(printer)
-      else s"$label: ${description(printer)}"
+      else s"$label${description(printer)}"
 
     protected def labelWithSuffix(
         printer: MetalsPrinter
@@ -98,7 +98,9 @@ object CompletionValue:
       else label
 
     override def description(printer: MetalsPrinter)(using Context): String =
-      printer.completionSymbol(symbol)
+      val isVal = !(symbol.is(Module) || symbol.is(Method) || symbol.isType)
+      val prefix = if isVal then ": " else ""
+      prefix ++ printer.completionSymbol(symbol)
   end Symbolic
 
   case class Compiler(
@@ -120,7 +122,8 @@ object CompletionValue:
     override def labelWithDescription(
         printer: MetalsPrinter
     )(using Context): String =
-      if symbol.is(Method) && symbol.name != nme.apply then
+      val isMethodOrValue = !(symbol.isType || symbol.is(Module))
+      if isMethodOrValue && symbol.name != nme.apply then
         s"${labelWithSuffix(printer)} - ${printer.fullName(symbol.effectiveOwner)}"
       else super.labelWithDescription(printer)
     override def isFromWorkspace: Boolean = true
@@ -269,6 +272,10 @@ object CompletionValue:
     override def labelWithDescription(printer: MetalsPrinter)(using
         Context
     ): String = label
+
+    override def description(printer: MetalsPrinter)(using Context): String =
+      printer.completionSymbol(symbol)
+
   end CaseKeyword
 
   case class Document(label: String, doc: String, description: String)
diff --git a/tests/cross/src/test/scala/tests/pc/CompletionSuite.scala b/tests/cross/src/test/scala/tests/pc/CompletionSuite.scala
index 6cc7aa9e2bb..90e534eb345 100644
--- a/tests/cross/src/test/scala/tests/pc/CompletionSuite.scala
+++ b/tests/cross/src/test/scala/tests/pc/CompletionSuite.scala
@@ -2341,11 +2341,50 @@ class CompletionSuite extends BaseCompletionSuite {
        |""".stripMargin,
     compat = Map(
       "3" -> """|x: String
-                |x: Int
+                |x - a.O: Int
                 |""".stripMargin
     )
   )
 
+  check(
+    "conflict-2",
+    """|package a
+       |object A {
+       |  val foo = 1
+       |}
+       |object B {
+       |  val foo = 1
+       |}
+       |object O {
+       |  val x: Int = foo@@
+       |}
+       |""".stripMargin,
+    """|foo - a.A: Int
+       |foo - a.B: Int
+       |""".stripMargin
+  )
+
+  check(
+    "conflict-3",
+    """|package a
+       |object A {
+       |  var foo = 1
+       |}
+       |object B {
+       |  var foo = 1
+       |}
+       |object O {
+       |  val x: Int = foo@@
+       |}
+       |""".stripMargin,
+    """|foo - a.A: Int
+       |foo - a.B: Int
+       |""".stripMargin
+  )
+
+  // tests.pc.CompletionCaseSuite
+  // tests.pc.CompletionScalaCliSuite
+
   checkEdit(
     "conflict-edit",
     """|object O {
@@ -2368,4 +2407,33 @@ class CompletionSuite extends BaseCompletionSuite {
     itemIndex = 1
   )
 
+  checkEdit(
+    "conflict-edit-2",
+    """|package a
+       |object A {
+       |  val foo = 1
+       |}
+       |object B {
+       |  val foo = 1
+       |}
+       |object O {
+       |  val x: Int = foo@@
+       |}
+       |""".stripMargin,
+    """|package a
+       |
+       |import a.A.foo
+       |object A {
+       |  val foo = 1
+       |}
+       |object B {
+       |  val foo = 1
+       |}
+       |object O {
+       |  val x: Int = foo
+       |}
+       |""".stripMargin,
+    assertSingleItem = false
+  )
+
 }
diff --git a/tests/cross/src/test/scala/tests/pc/CompletionWorkspaceSuite.scala b/tests/cross/src/test/scala/tests/pc/CompletionWorkspaceSuite.scala
index 79187235a63..efdc4b922e5 100644
--- a/tests/cross/src/test/scala/tests/pc/CompletionWorkspaceSuite.scala
+++ b/tests/cross/src/test/scala/tests/pc/CompletionWorkspaceSuite.scala
@@ -896,7 +896,7 @@ class CompletionWorkspaceSuite extends BaseCompletionSuite {
        |package b:
        |  def main: Unit = incre@@
        |""".stripMargin,
-    """|increment3: Int
+    """|increment3 - d: Int
        |increment - a: Int
        |increment2 - a.c: Int
        |""".stripMargin
@@ -939,18 +939,8 @@ class CompletionWorkspaceSuite extends BaseCompletionSuite {
        |}
        |""".stripMargin,
     """|fooBar: String
-       |fooBar: List[Int]
-       |""".stripMargin,
-    compat = Map(
-      "2" -> """|fooBar: String
-                |fooBar - case_class_param.A: List[Int]
-                |""".stripMargin,
-      ">=3.4.1-RC1-bin-20240208-hash-NIGHTLY" ->
-        """|fooBar: String
-           |fooBar: List[Int]
-           |fooBar(n: Int): Int
-           |""".stripMargin
-    )
+       |fooBar - case_class_param.A: List[Int]
+       |""".stripMargin
   )
 
   check(