fix: disambiguate workspace completions for vals

scala · Aug 8, 2024 · a28bc0f · a28bc0f
1 parent e0e3695
commit a28bc0f
Show file tree

Hide file tree

Showing 4 changed files with 96 additions and 8 deletions.
diff --git a/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionValue.scala b/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionValue.scala
@@ -101,13 +101,13 @@ object CompletionValue:
     )(using Context): String =
       if symbol.isConstructor then s"${snippetAffix.toPrefix}${label}${description(printer)}"
       else if symbol.is(Method) then s"${label}${description(printer)}"
-      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
         s"${labelWithSuffix(printer)}${description(printer)}"
       else if symbol.isType then labelWithSuffix(printer)
       else if symbol.isTerm && symbol.info.typeSymbol.is(Module) then
         s"${label}${description(printer)}"
-      else s"$label: ${description(printer)}"
+      else s"$label${description(printer)}"
 
     protected def labelWithSuffix(printer: ShortenedTypePrinter)(using Context): String =
       if snippetAffix.addLabelSnippet
@@ -119,7 +119,10 @@ object CompletionValue:
       else label
 
     override def description(printer: ShortenedTypePrinter)(using Context): String =
-      printer.completionSymbol(denotation)
+      def info = denotation.info.widenTermRefExpr
+      val isVal = !(symbol.is(Module) || symbol.is(Method) || symbol.isType || info.typeSymbol.is(Module))
+      val prefix = if isVal then ": " else ""
+      prefix ++ printer.completionSymbol(denotation)
 
   end Symbolic
 
@@ -178,9 +181,10 @@ object CompletionValue:
     override def completionItemDataKind: Integer = CompletionSource.WorkspaceKind.ordinal
 
     override def labelWithDescription(printer: ShortenedTypePrinter)(using Context): String =
+      def isMethodOrValue = !(symbol.isType || symbol.is(Module))
       if symbol.isConstructor || symbol.name == nme.apply then
         s"${snippetAffix.toPrefix}${label}${description(printer)} - ${printer.fullNameString(importSymbol.effectiveOwner)}"
-      else if symbol.is(Method) then
+      else if isMethodOrValue then
         s"${labelWithSuffix(printer)} - ${printer.fullNameString(symbol.effectiveOwner)}"
       else if symbol.is(Package) || symbol.is(Module) || symbol.isClass then
         s"${labelWithSuffix(printer)} -${description(printer)}"
@@ -199,7 +203,7 @@ object CompletionValue:
       CompletionItemKind.Method
     override def completionItemDataKind: Integer = CompletionSource.ImplicitClassKind.ordinal
     override def description(printer: ShortenedTypePrinter)(using Context): String =
-      s"${printer.completionSymbol(denotation)} (implicit)"
+      s"${super.description(printer)} (implicit)"
 
   /**
    * CompletionValue for extension methods via SymbolSearch
@@ -339,6 +343,9 @@ object CompletionValue:
 
     override def labelWithDescription(printer: ShortenedTypePrinter)(using Context): String =
       label
+
+    override def description(printer: ShortenedTypePrinter)(using Context): String =
+       printer.completionSymbol(denotation)
   end CaseKeyword
 
   case class Document(label: String, doc: String, description: String)

diff --git a/presentation-compiler/src/main/dotty/tools/pc/completions/Completions.scala b/presentation-compiler/src/main/dotty/tools/pc/completions/Completions.scala
@@ -914,7 +914,7 @@ class Completions(
         completionItemPriority
           .workspaceMemberPriority(
             SemanticdbSymbols.symbolName(symbol),
-          )
+          ).nn
 
       def compareFrequency(o1: CompletionValue, o2: CompletionValue): Int =
         (o1, o2) match

diff --git a/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala
@@ -2055,3 +2055,84 @@ class CompletionSuite extends BaseCompletionSuite:
         |""".stripMargin,
      ""
    )
+
+  @Test def conflict =
+    check(
+      """|package a
+         |object O {
+         |  val foofoo: Int = 123
+         |  def method = {
+         |    val foofoo: String = "abc"
+         |    foofoo@@
+         |  }
+         |}
+         |""".stripMargin,
+      """|foofoo: String
+         |foofoo - a.O: Int
+         |""".stripMargin
+    )
+
+  @Test def `conflict-2` =
+    check(
+      """|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
+    )
+
+  @Test def `conflict-3` = 
+   check(
+     """|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
+   )
+
+  @Test def `conflict-edit-2` =
+    checkEdit(
+      """|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/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionWorkspaceSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionWorkspaceSuite.scala
@@ -767,7 +767,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
@@ -810,7 +810,7 @@ class CompletionWorkspaceSuite extends BaseCompletionSuite:
          |}
          |""".stripMargin,
       """|fooBar: String
-         |fooBar: List[Int]
+         |fooBar - test.A: List[Int]
          |""".stripMargin,
     )