improvement: print better bracket suffix in completion item label

kasiaMarek · Kordyjan · commit 5e00703efe97 · 2023-12-07T20:32:06.000+01:00
[Cherry-picked 4d195f7]
diff --git a/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionSuffix.scala b/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionSuffix.scala
@@ -8,12 +8,7 @@ case class CompletionSuffix(
     suffixes: Set[SuffixKind],
     snippet: SuffixKind,
 ):
-  def labelSnippet =
-    suffixes.collectFirst {
-      case SuffixKind.Bracket(1) => "[T]"
-      case SuffixKind.Bracket(n) =>
-        (for (i <- 1 to n) yield s"T$i").mkString("[", ", ", "]")
-    }
+  def addLabelSnippet = suffixes.contains(SuffixKind.Bracket)
   def hasSnippet = snippet != SuffixKind.NoSuffix
   def chain(copyFn: CompletionSuffix => CompletionSuffix) = copyFn(this)
   def withNewSuffix(kind: SuffixKind) =
@@ -25,7 +20,7 @@ case class CompletionSuffix(
       def cursor = if suffixes.head == snippet then "$0" else ""
       suffixes match
         case SuffixKind.Brace :: tail => s"($cursor)" + loop(tail)
-        case SuffixKind.Bracket(_) :: tail => s"[$cursor]" + loop(tail)
+        case SuffixKind.Bracket :: tail => s"[$cursor]" + loop(tail)
         case SuffixKind.Template :: tail => s" {$cursor}" + loop(tail)
         case _ => ""
     loop(suffixes.toList)
@@ -41,7 +36,4 @@ object CompletionSuffix:
   )
 
 enum SuffixKind:
-  case Brace extends SuffixKind
-  case Bracket(typeParamsCount: Int) extends SuffixKind
-  case Template extends SuffixKind
-  case NoSuffix extends SuffixKind
+  case Brace, Bracket, Template, NoSuffix
diff --git a/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionValue.scala b/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionValue.scala
@@ -9,6 +9,7 @@ import dotty.tools.dotc.core.Symbols.Symbol
 import dotty.tools.dotc.core.Types.Type
 import dotty.tools.dotc.transform.SymUtils.*
 import dotty.tools.pc.printer.ShortenedTypePrinter
+import dotty.tools.pc.utils.MtagsEnrichments.decoded
 
 import org.eclipse.lsp4j.CompletionItemKind
 import org.eclipse.lsp4j.CompletionItemTag
@@ -35,9 +36,7 @@ sealed trait CompletionValue:
    * Label with potentially attached description.
    */
   def labelWithDescription(printer: ShortenedTypePrinter)(using Context): String =
-    labelWithSuffix
-  def labelWithSuffix: String =
-    s"$label${snippetSuffix.labelSnippet.getOrElse("")}"
+    label
   def lspTags(using Context): List[CompletionItemTag] = Nil
 end CompletionValue
 
@@ -77,14 +76,24 @@ object CompletionValue:
     override def labelWithDescription(
         printer: ShortenedTypePrinter
     )(using Context): String =
-      if symbol.is(Method) then s"${labelWithSuffix}${description(printer)}"
-      else if symbol.isConstructor then labelWithSuffix
-      else if symbol.is(Mutable) then
-        s"${labelWithSuffix}: ${description(printer)}"
+      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(Package) || symbol.is(Module) || symbol.isClass then
-        if isFromWorkspace then s"${labelWithSuffix} -${description(printer)}"
-        else s"${labelWithSuffix}${description(printer)}"
-      else s"${labelWithSuffix}: ${description(printer)}"
+        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)}"
+
+    private def labelWithSuffix(printer: ShortenedTypePrinter)(using Context): String =
+      if snippetSuffix.addLabelSnippet
+      then
+        val printedParams = symbol.info.typeParams.map(p =>
+          p.paramName.decoded ++ printer.tpe(p.paramInfo)
+        )
+        s"${label}${printedParams.mkString("[", ",", "]")}"
+      else label
 
     override def description(printer: ShortenedTypePrinter)(
       using Context
@@ -97,7 +106,11 @@ object CompletionValue:
       symbol: Symbol,
       override val snippetSuffix: CompletionSuffix
   ) extends Symbolic
-  case class Scope(label: String, symbol: Symbol) extends Symbolic
+  case class Scope(
+      label: String,
+      symbol: Symbol,
+      override val snippetSuffix: CompletionSuffix,
+  ) extends Symbolic
   case class Workspace(
       label: String,
       symbol: Symbol,
@@ -262,6 +275,4 @@ object CompletionValue:
   ): CompletionValue =
     Document(label, insertText, description)
 
-  def scope(label: String, sym: Symbol): CompletionValue =
-    Scope(label, sym)
 end CompletionValue
diff --git a/presentation-compiler/src/main/dotty/tools/pc/completions/Completions.scala b/presentation-compiler/src/main/dotty/tools/pc/completions/Completions.scala
@@ -258,9 +258,7 @@ class Completions(
       .chain { suffix => // for [] suffix
         if shouldAddSnippet &&
           cursorPos.allowBracketSuffix && symbol.info.typeParams.nonEmpty
-        then
-          val typeParamsCount = symbol.info.typeParams.length
-          suffix.withNewSuffixSnippet(SuffixKind.Bracket(typeParamsCount))
+        then suffix.withNewSuffixSnippet(SuffixKind.Bracket)
         else suffix
       }
       .chain { suffix => // for () suffix
@@ -563,14 +561,14 @@ class Completions(
           )
 
         filtered.map { sym =>
-          visit(CompletionValue.scope(sym.decodedName, sym))
+          visit(CompletionValue.Scope(sym.decodedName, sym, findSuffix(sym)))
         }
         Some(SymbolSearch.Result.INCOMPLETE)
       case CompletionKind.Scope =>
         val visitor = new CompilerSearchVisitor(sym =>
           indexedContext.lookupSym(sym) match
             case IndexedContext.Result.InScope =>
-              visit(CompletionValue.scope(sym.decodedName, sym))
+              visit(CompletionValue.Scope(sym.decodedName, sym, findSuffix(sym)))
             case _ =>
               completionsWithSuffix(
                 sym,
@@ -634,7 +632,9 @@ class Completions(
                   // drop #|. at the end to avoid duplication
                   name.substring(0, name.length - 1)
                 else name
-              val id = nameId + symOnly.snippetSuffix.labelSnippet.getOrElse("")
+              val suffix =
+                if symOnly.snippetSuffix.addLabelSnippet then "[]" else ""
+              val id = nameId + suffix
               val include = cursorPos.include(sym)
               (id, include)
             case kw: CompletionValue.Keyword => (kw.label, true)
diff --git a/presentation-compiler/src/main/dotty/tools/pc/printer/ShortenedTypePrinter.scala b/presentation-compiler/src/main/dotty/tools/pc/printer/ShortenedTypePrinter.scala
@@ -244,7 +244,14 @@ class ShortenedTypePrinter(
       else " " + fullNameString(sym.effectiveOwner)
     else if sym.is(Flags.Method) then
       defaultMethodSignature(sym, info, onlyMethodParams = true)
+    else if sym.isType
+    then
+      info match
+        case TypeAlias(t) => " = " + tpe(t.resultType)
+        case t => tpe(t.resultType)
     else tpe(info)
+    end if
+  end completionSymbol
 
   /**
    * Compute method signature for the given (method) symbol.
diff --git a/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala
@@ -591,7 +591,7 @@ class CompletionSuite extends BaseCompletionSuite:
           |  val foo: ListBuffe@@
           |}
           |""".stripMargin,
-      """|ListBuffer[T] - scala.collection.mutable
+      """|ListBuffer[A] - scala.collection.mutable
          |ListBuffer - scala.collection.mutable
          |""".stripMargin
     )
@@ -602,7 +602,7 @@ class CompletionSuite extends BaseCompletionSuite:
           |  val foo: Map[Int, ListBuffe@@]
           |}
           |""".stripMargin,
-      """|ListBuffer[T] - scala.collection.mutable
+      """|ListBuffer[A] - scala.collection.mutable
          |ListBuffer - scala.collection.mutable
          |""".stripMargin
     )
@@ -1252,3 +1252,80 @@ class CompletionSuite extends BaseCompletionSuite:
       assertSingleItem = false,
     )
 
+  @Test def `type-with-params` =
+    check(
+      s"""|object O {
+          | type TTT[A <: Int] = List[A]
+          | val t: TT@@
+          |}
+          |""".stripMargin,
+      "TTT[A <: Int]",
+      includeDetail = false,
+    )
+
+  @Test def `type-with-params-with-detail` =
+    check(
+      s"""|object O {
+          | type TTT[A <: Int] = List[A]
+          | val t: TT@@
+          |}
+          |""".stripMargin,
+    "TTT[A <: Int] = List[A]"
+    )
+
+  @Test def `type-lambda` =
+    check(
+      s"""|object O {
+          | type TTT = [A <: Int] =>> List[A]
+          | val t: TT@@
+          |}
+          |""".stripMargin,
+      "TTT[A <: Int]",
+      includeDetail = false,
+    )
+
+  @Test def `type-lambda2` =
+    check(
+      s"""|object O {
+          | type TTT[K <: Int] = [V] =>> Map[K, V]
+          | val t: TT@@
+          |}
+          |""".stripMargin,
+      "TTT[K <: Int]",
+      includeDetail = false,
+    )
+
+  @Test def `type-lambda2-with-detail` =
+    check(
+      s"""|object O {
+          | type TTT[K <: Int] = [V] =>> Map[K, V]
+          | val t: TT@@
+          |}
+          |""".stripMargin,
+      "TTT[K <: Int] = [V] =>> Map[K, V]",
+    )
+
+  @Test def `type-bound` =
+    check(
+      s"""|trait O {
+          | type TTT <: Int
+          | val t: TT@@
+          |}
+          |""".stripMargin,
+      "TTT <: Int"
+    )
+
+  @Test def `class-with-params` =
+    check(
+      s"""|object O {
+          | class AClass[A <: Int]
+          | object AClass
+          | val v: ACla@@
+          |}
+          |""".stripMargin,
+      """|AClass[A <: Int] test.O
+         |AClass test.O
+         |AbstractTypeClassManifest - scala.reflect.ClassManifestFactory
+         |""".stripMargin
+    )
+
diff --git a/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionWorkspaceSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionWorkspaceSuite.scala
@@ -684,7 +684,7 @@ class CompletionWorkspaceSuite extends BaseCompletionSuite:
          |  ): String = ???
          |}
          |""".stripMargin,
-      """|Future[T] java.util.concurrent
+      """|Future[V] java.util.concurrent
          |Future java.util.concurrent
          |Future[T] - scala.concurrent
          |""".stripMargin,
