Skip to content

Commit c90ad6b

Browse files
tgodziktgodzik
authored
improvement: print better bracket suffix in completion item label (#18380)
backport from metals: scalameta/metals#5497 CC: @tgodzik
2 parents 17a9c0c + 4d195f7 commit c90ad6b

File tree

6 files changed

+120
-33
lines changed

6 files changed

+120
-33
lines changed

‎presentation-compiler/src/main/dotty/tools/pc/completions/CompletionSuffix.scala

+3-11
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,7 @@ case class CompletionSuffix(
88
suffixes: Set[SuffixKind],
99
snippet: SuffixKind,
1010
):
11-
def labelSnippet =
12-
suffixes.collectFirst {
13-
case SuffixKind.Bracket(1) => "[T]"
14-
case SuffixKind.Bracket(n) =>
15-
(for (i <- 1 to n) yield s"T$i").mkString("[", ", ", "]")
16-
}
11+
def addLabelSnippet = suffixes.contains(SuffixKind.Bracket)
1712
def hasSnippet = snippet != SuffixKind.NoSuffix
1813
def chain(copyFn: CompletionSuffix => CompletionSuffix) = copyFn(this)
1914
def withNewSuffix(kind: SuffixKind) =
@@ -25,7 +20,7 @@ case class CompletionSuffix(
2520
def cursor = if suffixes.head == snippet then "$0" else ""
2621
suffixes match
2722
case SuffixKind.Brace :: tail => s"($cursor)" + loop(tail)
28-
case SuffixKind.Bracket(_) :: tail => s"[$cursor]" + loop(tail)
23+
case SuffixKind.Bracket :: tail => s"[$cursor]" + loop(tail)
2924
case SuffixKind.Template :: tail => s" {$cursor}" + loop(tail)
3025
case _ => ""
3126
loop(suffixes.toList)
@@ -41,7 +36,4 @@ object CompletionSuffix:
4136
)
4237

4338
enum SuffixKind:
44-
case Brace extends SuffixKind
45-
case Bracket(typeParamsCount: Int) extends SuffixKind
46-
case Template extends SuffixKind
47-
case NoSuffix extends SuffixKind
39+
case Brace, Bracket, Template, NoSuffix

‎presentation-compiler/src/main/dotty/tools/pc/completions/CompletionValue.scala

+24-13
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import dotty.tools.dotc.core.Symbols.Symbol
99
import dotty.tools.dotc.core.Types.Type
1010
import dotty.tools.dotc.transform.SymUtils.*
1111
import dotty.tools.pc.printer.ShortenedTypePrinter
12+
import dotty.tools.pc.utils.MtagsEnrichments.decoded
1213

1314
import org.eclipse.lsp4j.CompletionItemKind
1415
import org.eclipse.lsp4j.CompletionItemTag
@@ -35,9 +36,7 @@ sealed trait CompletionValue:
3536
* Label with potentially attached description.
3637
*/
3738
def labelWithDescription(printer: ShortenedTypePrinter)(using Context): String =
38-
labelWithSuffix
39-
def labelWithSuffix: String =
40-
s"$label${snippetSuffix.labelSnippet.getOrElse("")}"
39+
label
4140
def lspTags(using Context): List[CompletionItemTag] = Nil
4241
end CompletionValue
4342

@@ -77,14 +76,24 @@ object CompletionValue:
7776
override def labelWithDescription(
7877
printer: ShortenedTypePrinter
7978
)(using Context): String =
80-
if symbol.is(Method) then s"${labelWithSuffix}${description(printer)}"
81-
else if symbol.isConstructor then labelWithSuffix
82-
else if symbol.is(Mutable) then
83-
s"${labelWithSuffix}: ${description(printer)}"
79+
if symbol.is(Method) then s"${label}${description(printer)}"
80+
else if symbol.isConstructor then label
81+
else if symbol.is(Mutable) then s"$label: ${description(printer)}"
8482
else if symbol.is(Package) || symbol.is(Module) || symbol.isClass then
85-
if isFromWorkspace then s"${labelWithSuffix} -${description(printer)}"
86-
else s"${labelWithSuffix}${description(printer)}"
87-
else s"${labelWithSuffix}: ${description(printer)}"
83+
if isFromWorkspace then
84+
s"${labelWithSuffix(printer)} -${description(printer)}"
85+
else s"${labelWithSuffix(printer)}${description(printer)}"
86+
else if symbol.isType then labelWithSuffix(printer)
87+
else s"$label: ${description(printer)}"
88+
89+
private def labelWithSuffix(printer: ShortenedTypePrinter)(using Context): String =
90+
if snippetSuffix.addLabelSnippet
91+
then
92+
val printedParams = symbol.info.typeParams.map(p =>
93+
p.paramName.decoded ++ printer.tpe(p.paramInfo)
94+
)
95+
s"${label}${printedParams.mkString("[", ",", "]")}"
96+
else label
8897

8998
override def description(printer: ShortenedTypePrinter)(
9099
using Context
@@ -97,7 +106,11 @@ object CompletionValue:
97106
symbol: Symbol,
98107
override val snippetSuffix: CompletionSuffix
99108
) extends Symbolic
100-
case class Scope(label: String, symbol: Symbol) extends Symbolic
109+
case class Scope(
110+
label: String,
111+
symbol: Symbol,
112+
override val snippetSuffix: CompletionSuffix,
113+
) extends Symbolic
101114
case class Workspace(
102115
label: String,
103116
symbol: Symbol,
@@ -262,6 +275,4 @@ object CompletionValue:
262275
): CompletionValue =
263276
Document(label, insertText, description)
264277

265-
def scope(label: String, sym: Symbol): CompletionValue =
266-
Scope(label, sym)
267278
end CompletionValue

‎presentation-compiler/src/main/dotty/tools/pc/completions/Completions.scala

+6-6
Original file line numberDiff line numberDiff line change
@@ -258,9 +258,7 @@ class Completions(
258258
.chain { suffix => // for [] suffix
259259
if shouldAddSnippet &&
260260
cursorPos.allowBracketSuffix && symbol.info.typeParams.nonEmpty
261-
then
262-
val typeParamsCount = symbol.info.typeParams.length
263-
suffix.withNewSuffixSnippet(SuffixKind.Bracket(typeParamsCount))
261+
then suffix.withNewSuffixSnippet(SuffixKind.Bracket)
264262
else suffix
265263
}
266264
.chain { suffix => // for () suffix
@@ -563,14 +561,14 @@ class Completions(
563561
)
564562

565563
filtered.map { sym =>
566-
visit(CompletionValue.scope(sym.decodedName, sym))
564+
visit(CompletionValue.Scope(sym.decodedName, sym, findSuffix(sym)))
567565
}
568566
Some(SymbolSearch.Result.INCOMPLETE)
569567
case CompletionKind.Scope =>
570568
val visitor = new CompilerSearchVisitor(sym =>
571569
indexedContext.lookupSym(sym) match
572570
case IndexedContext.Result.InScope =>
573-
visit(CompletionValue.scope(sym.decodedName, sym))
571+
visit(CompletionValue.Scope(sym.decodedName, sym, findSuffix(sym)))
574572
case _ =>
575573
completionsWithSuffix(
576574
sym,
@@ -634,7 +632,9 @@ class Completions(
634632
// drop #|. at the end to avoid duplication
635633
name.substring(0, name.length - 1)
636634
else name
637-
val id = nameId + symOnly.snippetSuffix.labelSnippet.getOrElse("")
635+
val suffix =
636+
if symOnly.snippetSuffix.addLabelSnippet then "[]" else ""
637+
val id = nameId + suffix
638638
val include = cursorPos.include(sym)
639639
(id, include)
640640
case kw: CompletionValue.Keyword => (kw.label, true)

‎presentation-compiler/src/main/dotty/tools/pc/printer/ShortenedTypePrinter.scala

+7
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,14 @@ class ShortenedTypePrinter(
244244
else " " + fullNameString(sym.effectiveOwner)
245245
else if sym.is(Flags.Method) then
246246
defaultMethodSignature(sym, info, onlyMethodParams = true)
247+
else if sym.isType
248+
then
249+
info match
250+
case TypeAlias(t) => " = " + tpe(t.resultType)
251+
case t => tpe(t.resultType)
247252
else tpe(info)
253+
end if
254+
end completionSymbol
248255

249256
/**
250257
* Compute method signature for the given (method) symbol.

‎presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala

+79-2
Original file line numberDiff line numberDiff line change
@@ -591,7 +591,7 @@ class CompletionSuite extends BaseCompletionSuite:
591591
| val foo: ListBuffe@@
592592
|}
593593
|""".stripMargin,
594-
"""|ListBuffer[T] - scala.collection.mutable
594+
"""|ListBuffer[A] - scala.collection.mutable
595595
|ListBuffer - scala.collection.mutable
596596
|""".stripMargin
597597
)
@@ -602,7 +602,7 @@ class CompletionSuite extends BaseCompletionSuite:
602602
| val foo: Map[Int, ListBuffe@@]
603603
|}
604604
|""".stripMargin,
605-
"""|ListBuffer[T] - scala.collection.mutable
605+
"""|ListBuffer[A] - scala.collection.mutable
606606
|ListBuffer - scala.collection.mutable
607607
|""".stripMargin
608608
)
@@ -1252,3 +1252,80 @@ class CompletionSuite extends BaseCompletionSuite:
12521252
assertSingleItem = false,
12531253
)
12541254

1255+
@Test def `type-with-params` =
1256+
check(
1257+
s"""|object O {
1258+
| type TTT[A <: Int] = List[A]
1259+
| val t: TT@@
1260+
|}
1261+
|""".stripMargin,
1262+
"TTT[A <: Int]",
1263+
includeDetail = false,
1264+
)
1265+
1266+
@Test def `type-with-params-with-detail` =
1267+
check(
1268+
s"""|object O {
1269+
| type TTT[A <: Int] = List[A]
1270+
| val t: TT@@
1271+
|}
1272+
|""".stripMargin,
1273+
"TTT[A <: Int] = List[A]"
1274+
)
1275+
1276+
@Test def `type-lambda` =
1277+
check(
1278+
s"""|object O {
1279+
| type TTT = [A <: Int] =>> List[A]
1280+
| val t: TT@@
1281+
|}
1282+
|""".stripMargin,
1283+
"TTT[A <: Int]",
1284+
includeDetail = false,
1285+
)
1286+
1287+
@Test def `type-lambda2` =
1288+
check(
1289+
s"""|object O {
1290+
| type TTT[K <: Int] = [V] =>> Map[K, V]
1291+
| val t: TT@@
1292+
|}
1293+
|""".stripMargin,
1294+
"TTT[K <: Int]",
1295+
includeDetail = false,
1296+
)
1297+
1298+
@Test def `type-lambda2-with-detail` =
1299+
check(
1300+
s"""|object O {
1301+
| type TTT[K <: Int] = [V] =>> Map[K, V]
1302+
| val t: TT@@
1303+
|}
1304+
|""".stripMargin,
1305+
"TTT[K <: Int] = [V] =>> Map[K, V]",
1306+
)
1307+
1308+
@Test def `type-bound` =
1309+
check(
1310+
s"""|trait O {
1311+
| type TTT <: Int
1312+
| val t: TT@@
1313+
|}
1314+
|""".stripMargin,
1315+
"TTT <: Int"
1316+
)
1317+
1318+
@Test def `class-with-params` =
1319+
check(
1320+
s"""|object O {
1321+
| class AClass[A <: Int]
1322+
| object AClass
1323+
| val v: ACla@@
1324+
|}
1325+
|""".stripMargin,
1326+
"""|AClass[A <: Int] test.O
1327+
|AClass test.O
1328+
|AbstractTypeClassManifest - scala.reflect.ClassManifestFactory
1329+
|""".stripMargin
1330+
)
1331+

‎presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionWorkspaceSuite.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -684,7 +684,7 @@ class CompletionWorkspaceSuite extends BaseCompletionSuite:
684684
| ): String = ???
685685
|}
686686
|""".stripMargin,
687-
"""|Future[T] java.util.concurrent
687+
"""|Future[V] java.util.concurrent
688688
|Future java.util.concurrent
689689
|Future[T] - scala.concurrent
690690
|""".stripMargin,

0 commit comments

Comments
 (0)