Skip to content

Commit e196dec

Browse files
authored
Completion assert diffs will now show completion source (#18890)
Previously failed assertions showed a side by side diff of expected vs obtained completions: <img width="275" alt="Screenshot 2023-11-09 at 18 28 46" src="https://github.com/lampepfl/dotty/assets/48657087/c80fbc71-7e58-4cba-b302-b4dfeff9bcec"> And now: <img width="311" alt="Screenshot 2023-11-09 at 18 29 31" src="https://github.com/lampepfl/dotty/assets/48657087/829933c7-31c6-4c26-a20c-4d426eb5c11b"> This is extremely useful when debugging the completions, as we have multiple sources and finding what specific completion comes from is just a waste of time. There is also a chance that we'd want to not include this info in data, but this is minimal trade-off for a significant boost when working on PC.
1 parent 681f182 commit e196dec

File tree

7 files changed

+113
-49
lines changed

7 files changed

+113
-49
lines changed

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

+2-3
Original file line numberDiff line numberDiff line change
@@ -177,9 +177,8 @@ class CompletionProvider(
177177
item.setAdditionalTextEdits((completion.additionalEdits ++ additionalEdits).asJava)
178178
completion.insertMode.foreach(item.setInsertTextMode)
179179

180-
completion
181-
.completionData(buildTargetIdentifier)
182-
.foreach(data => item.setData(data.toJson))
180+
val data = completion.completionData(buildTargetIdentifier)
181+
item.setData(data.toJson)
183182

184183
item.setTags(completion.lspTags.asJava)
185184

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

+44-13
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,24 @@ import org.eclipse.lsp4j.InsertTextMode
1717
import org.eclipse.lsp4j.Range
1818
import org.eclipse.lsp4j.TextEdit
1919

20+
enum CompletionSource:
21+
case Empty
22+
case OverrideKind
23+
case ImplementAllKind
24+
case CompilerKind
25+
case KeywordKind
26+
case ScopeKind
27+
case WorkspaceKind
28+
case ExtensionKind
29+
case NamedArgKind
30+
case AutoFillKind
31+
case FileSystemMemberKind
32+
case IvyImportKind
33+
case InterpolatorKind
34+
case MatchCompletionKind
35+
case CaseKeywordKind
36+
case DocumentKind
37+
2038
sealed trait CompletionValue:
2139
def label: String
2240
def insertText: Option[String] = None
@@ -25,11 +43,12 @@ sealed trait CompletionValue:
2543
def range: Option[Range] = None
2644
def filterText: Option[String] = None
2745
def completionItemKind(using Context): CompletionItemKind
46+
def completionItemDataKind: Integer = CompletionItemData.None
2847
def description(printer: ShortenedTypePrinter)(using Context): String = ""
2948
def insertMode: Option[InsertTextMode] = None
3049
def completionData(buildTargetIdentifier: String)(
3150
using Context
32-
): Option[CompletionItemData] = None
51+
): CompletionItemData = CompletionItemData("<no-symbol>", buildTargetIdentifier, kind = completionItemDataKind)
3352
def command: Option[String] = None
3453

3554
/**
@@ -45,17 +64,15 @@ object CompletionValue:
4564
sealed trait Symbolic extends CompletionValue:
4665
def symbol: Symbol
4766
def isFromWorkspace: Boolean = false
48-
def completionItemDataKind = CompletionItemData.None
67+
override def completionItemDataKind = CompletionItemData.None
4968

5069
override def completionData(
5170
buildTargetIdentifier: String
52-
)(using Context): Option[CompletionItemData] =
53-
Some(
54-
CompletionItemData(
55-
SemanticdbSymbols.symbolName(symbol),
56-
buildTargetIdentifier,
57-
kind = completionItemDataKind
58-
)
71+
)(using Context): CompletionItemData =
72+
CompletionItemData(
73+
SemanticdbSymbols.symbolName(symbol),
74+
buildTargetIdentifier,
75+
kind = completionItemDataKind
5976
)
6077
def importSymbol: Symbol = symbol
6178

@@ -105,19 +122,24 @@ object CompletionValue:
105122
label: String,
106123
symbol: Symbol,
107124
override val snippetSuffix: CompletionSuffix
108-
) extends Symbolic
125+
) extends Symbolic {
126+
override def completionItemDataKind: Integer = CompletionSource.CompilerKind.ordinal
127+
}
109128
case class Scope(
110129
label: String,
111130
symbol: Symbol,
112131
override val snippetSuffix: CompletionSuffix,
113-
) extends Symbolic
132+
) extends Symbolic {
133+
override def completionItemDataKind: Integer = CompletionSource.ScopeKind.ordinal
134+
}
114135
case class Workspace(
115136
label: String,
116137
symbol: Symbol,
117138
override val snippetSuffix: CompletionSuffix,
118139
override val importSymbol: Symbol
119140
) extends Symbolic:
120141
override def isFromWorkspace: Boolean = true
142+
override def completionItemDataKind: Integer = CompletionSource.WorkspaceKind.ordinal
121143

122144
/**
123145
* CompletionValue for extension methods via SymbolSearch
@@ -129,6 +151,7 @@ object CompletionValue:
129151
) extends Symbolic:
130152
override def completionItemKind(using Context): CompletionItemKind =
131153
CompletionItemKind.Method
154+
override def completionItemDataKind: Integer = CompletionSource.ExtensionKind.ordinal
132155
override def description(printer: ShortenedTypePrinter)(using Context): String =
133156
s"${printer.completionSymbol(symbol)} (extension)"
134157

@@ -149,8 +172,7 @@ object CompletionValue:
149172
override val range: Option[Range]
150173
) extends Symbolic:
151174
override def insertText: Option[String] = Some(value)
152-
override def completionItemDataKind: Integer =
153-
CompletionItemData.OverrideKind
175+
override def completionItemDataKind: Integer = CompletionSource.OverrideKind.ordinal
154176
override def completionItemKind(using Context): CompletionItemKind =
155177
CompletionItemKind.Method
156178
override def labelWithDescription(printer: ShortenedTypePrinter)(using Context): String =
@@ -163,6 +185,7 @@ object CompletionValue:
163185
symbol: Symbol
164186
) extends Symbolic:
165187
override def insertText: Option[String] = Some(label.replace("$", "$$").nn)
188+
override def completionItemDataKind: Integer = CompletionSource.OverrideKind.ordinal
166189
override def completionItemKind(using Context): CompletionItemKind =
167190
CompletionItemKind.Field
168191
override def description(printer: ShortenedTypePrinter)(using Context): String =
@@ -177,11 +200,13 @@ object CompletionValue:
177200
) extends CompletionValue:
178201
override def completionItemKind(using Context): CompletionItemKind =
179202
CompletionItemKind.Enum
203+
override def completionItemDataKind: Integer = CompletionSource.OverrideKind.ordinal
180204
override def insertText: Option[String] = Some(value)
181205
override def label: String = "Autofill with default values"
182206

183207
case class Keyword(label: String, override val insertText: Option[String])
184208
extends CompletionValue:
209+
override def completionItemDataKind: Integer = CompletionSource.KeywordKind.ordinal
185210
override def completionItemKind(using Context): CompletionItemKind =
186211
CompletionItemKind.Keyword
187212

@@ -192,6 +217,7 @@ object CompletionValue:
192217
) extends CompletionValue:
193218
override def label: String = filename
194219
override def insertText: Option[String] = Some(filename.stripSuffix(".sc"))
220+
override def completionItemDataKind: Integer = CompletionSource.FileSystemMemberKind.ordinal
195221
override def completionItemKind(using Context): CompletionItemKind =
196222
CompletionItemKind.File
197223

@@ -201,6 +227,7 @@ object CompletionValue:
201227
override val range: Option[Range]
202228
) extends CompletionValue:
203229
override val filterText: Option[String] = insertText
230+
override def completionItemDataKind: Integer = CompletionSource.IvyImportKind.ordinal
204231
override def completionItemKind(using Context): CompletionItemKind =
205232
CompletionItemKind.Folder
206233

@@ -215,6 +242,7 @@ object CompletionValue:
215242
isWorkspace: Boolean = false,
216243
isExtension: Boolean = false
217244
) extends Symbolic:
245+
override def completionItemDataKind: Integer = CompletionSource.InterpolatorKind.ordinal
218246
override def description(
219247
printer: ShortenedTypePrinter
220248
)(using Context): String =
@@ -228,6 +256,7 @@ object CompletionValue:
228256
override val additionalEdits: List[TextEdit],
229257
desc: String
230258
) extends CompletionValue:
259+
override def completionItemDataKind: Integer = CompletionSource.MatchCompletionKind.ordinal
231260
override def completionItemKind(using Context): CompletionItemKind =
232261
CompletionItemKind.Enum
233262
override def description(printer: ShortenedTypePrinter)(using Context): String =
@@ -241,6 +270,7 @@ object CompletionValue:
241270
override val range: Option[Range] = None,
242271
override val command: Option[String] = None
243272
) extends Symbolic:
273+
override def completionItemDataKind: Integer = CompletionSource.CaseKeywordKind.ordinal
244274
override def completionItemKind(using Context): CompletionItemKind =
245275
CompletionItemKind.Method
246276

@@ -253,6 +283,7 @@ object CompletionValue:
253283
override def filterText: Option[String] = Some(description)
254284

255285
override def insertText: Option[String] = Some(doc)
286+
override def completionItemDataKind: Integer = CompletionSource.DocumentKind.ordinal
256287
override def completionItemKind(using Context): CompletionItemKind =
257288
CompletionItemKind.Snippet
258289

presentation-compiler/test/dotty/tools/pc/base/BaseCompletionSuite.scala

+12-5
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import scala.meta.internal.metals.{CompilerOffsetParams, EmptyCancelToken}
88
import scala.meta.pc.CancelToken
99
import scala.language.unsafeNulls
1010

11+
import dotty.tools.pc.completions.CompletionSource
1112
import dotty.tools.pc.utils.MtagsEnrichments.*
1213
import dotty.tools.pc.utils.{TestCompletions, TextEdits}
1314

@@ -172,7 +173,7 @@ abstract class BaseCompletionSuite extends BasePCSuite:
172173
}
173174
.mkString("\n")
174175

175-
assertCompletions(expected, obtained, Some(original))
176+
assertWithDiff(expected, obtained, includeSources = false, Some(original))
176177

177178
/**
178179
* Check completions that will be shown in original param after `@@` marker
@@ -245,13 +246,19 @@ abstract class BaseCompletionSuite extends BasePCSuite:
245246
.append(commitCharacter)
246247
.append("\n")
247248
}
248-
val expectedResult = sortLines(stableOrder, expected)
249-
val actualResult = sortLines(
249+
val completionSources = filteredItems
250+
.map(_.data.map(data => CompletionSource.fromOrdinal(data.kind))
251+
.getOrElse(CompletionSource.Empty))
252+
.toList
253+
254+
val (expectedResult, _) = sortLines(stableOrder, expected)
255+
val (actualResult, sources) = sortLines(
250256
stableOrder,
251-
postProcessObtained(trimTrailingSpace(out.toString()))
257+
postProcessObtained(trimTrailingSpace(out.toString())),
258+
completionSources,
252259
)
253260

254-
assertCompletions(expectedResult, actualResult, Some(original))
261+
assertWithDiff(expectedResult, actualResult, includeSources = true, Some(original), sources)
255262

256263
if (filterText.nonEmpty) {
257264
filteredItems.foreach { item =>

presentation-compiler/test/dotty/tools/pc/base/BasePCSuite.scala

+7-3
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import scala.meta.pc.{PresentationCompiler, PresentationCompilerConfig}
1414
import scala.language.unsafeNulls
1515

1616
import dotty.tools.pc.*
17+
import dotty.tools.pc.completions.CompletionSource
1718
import dotty.tools.pc.ScalaPresentationCompiler
1819
import dotty.tools.pc.tests.buildinfo.BuildInfo
1920
import dotty.tools.pc.utils._
@@ -113,10 +114,13 @@ abstract class BasePCSuite extends PcAssertions:
113114
" " + e.getRight.getValue
114115
}.trim
115116

116-
def sortLines(stableOrder: Boolean, string: String): String =
117+
def sortLines(stableOrder: Boolean, string: String, completionSources: List[CompletionSource] = Nil): (String, List[CompletionSource]) =
117118
val strippedString = string.linesIterator.toList.filter(_.nonEmpty)
118-
if (stableOrder) strippedString.mkString("\n")
119-
else strippedString.sorted.mkString("\n")
119+
if (stableOrder) strippedString.mkString("\n") -> completionSources
120+
else
121+
val paddedSources = completionSources.padTo(strippedString.size, CompletionSource.Empty)
122+
val (sortedCompletions, sortedSources) = (strippedString zip paddedSources).sortBy(_._1).unzip
123+
sortedCompletions.mkString("\n") -> sortedSources
120124

121125
extension (s: String)
122126
def triplequoted: String = s.replace("'''", "\"\"\"")

presentation-compiler/test/dotty/tools/pc/base/BaseSignatureHelpSuite.scala

+3-3
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,6 @@ abstract class BaseSignatureHelpSuite extends BasePCSuite:
8484
}
8585
}
8686

87-
val obtainedSorted = sortLines(stableOrder, out.toString())
88-
val expectedSorted = sortLines(stableOrder, expected)
89-
assertCompletions(expectedSorted, obtainedSorted, Some(original))
87+
val (obtainedSorted, _) = sortLines(stableOrder, out.toString())
88+
val (expectedSorted, _) = sortLines(stableOrder, expected)
89+
assertWithDiff(expectedSorted, obtainedSorted, includeSources = false, Some(original))

presentation-compiler/test/dotty/tools/pc/utils/PcAssertions.scala

+44-17
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package dotty.tools.pc.utils
22

33
import scala.language.unsafeNulls
44

5+
import dotty.tools.pc.completions.CompletionSource
56
import dotty.tools.dotc.util.DiffUtil
67
import dotty.tools.pc.utils.MtagsEnrichments.*
78

@@ -10,20 +11,22 @@ import org.hamcrest.*
1011

1112
trait PcAssertions:
1213

13-
def assertCompletions(
14+
def assertWithDiff(
1415
expected: String,
1516
actual: String,
16-
snippet: Option[String] = None
17+
includeSources: Boolean,
18+
snippet: Option[String] = None,
19+
completionSources: List[CompletionSource] = Nil,
1720
): Unit =
1821
val longestExpeceted =
19-
expected.linesIterator.maxByOption(_.length).map(_.length).getOrElse(0)
22+
expected.linesIterator.map(_.length).maxOption.getOrElse(0)
2023
val longestActual =
21-
actual.linesIterator.maxByOption(_.length).map(_.length).getOrElse(0)
24+
actual.linesIterator.map(_.length).maxOption.getOrElse(0)
2225

2326
val actualMatcher =
2427
if longestActual >= 40 || longestExpeceted >= 40 then
25-
lineByLineDiffMatcher(expected)
26-
else sideBySideDiffMatcher(expected)
28+
lineByLineDiffMatcher(expected, completionSources, includeSources)
29+
else sideBySideDiffMatcher(expected, completionSources, includeSources)
2730

2831
assertThat(actual, actualMatcher, snippet)
2932

@@ -115,27 +118,51 @@ trait PcAssertions:
115118
error.setStackTrace(Array.empty)
116119
throw error
117120

118-
private def lineByLineDiffMatcher(expected: String): TypeSafeMatcher[String] =
121+
122+
private def lineByLineDiffMatcher(
123+
expected: String,
124+
completionSources: List[CompletionSource] = Nil,
125+
isCompletion: Boolean = false
126+
): TypeSafeMatcher[String] =
127+
def getDetailedMessage(diff: String): String =
128+
val lines = diff.linesIterator.toList
129+
val sources = completionSources.padTo(lines.size, CompletionSource.Empty)
130+
val maxLength = lines.map(_.length).maxOption.getOrElse(0)
131+
var redLineIndex = 0
132+
lines.map: line =>
133+
if line.startsWith(Console.BOLD + Console.RED) then
134+
redLineIndex = redLineIndex + 1
135+
s"$line | [${sources(redLineIndex - 1)}]"
136+
else
137+
line
138+
.mkString("\n")
139+
119140
new TypeSafeMatcher[String]:
120141

121142
override def describeMismatchSafely(
122143
item: String,
123144
mismatchDescription: org.hamcrest.Description
124145
): Unit =
146+
val diff = DiffUtil.mkColoredHorizontalLineDiff(unifyNewlines(expected), unifyNewlines(item))
147+
val maybeEnhancedDiff = if isCompletion then getDetailedMessage(diff) else diff
125148
mismatchDescription.appendText(System.lineSeparator)
126-
mismatchDescription.appendText(
127-
DiffUtil.mkColoredHorizontalLineDiff(
128-
unifyNewlines(expected),
129-
unifyNewlines(item)
130-
)
131-
)
149+
mismatchDescription.appendText(maybeEnhancedDiff)
132150
mismatchDescription.appendText(System.lineSeparator)
133151

134152
override def describeTo(description: org.hamcrest.Description): Unit = ()
135153
override def matchesSafely(item: String): Boolean =
136154
unifyNewlines(expected) == unifyNewlines(item)
137155

138-
private def sideBySideDiffMatcher(expected: String): TypeSafeMatcher[String] =
156+
private def sideBySideDiffMatcher(
157+
expected: String,
158+
completionSources: List[CompletionSource] = Nil,
159+
isCompletion: Boolean = false
160+
): TypeSafeMatcher[String] =
161+
def getDetailedMessage(diff: String): String =
162+
val lines = diff.linesIterator.toList
163+
val sources = completionSources.padTo(lines.size, CompletionSource.Empty)
164+
(lines zip sources).map((line, source) => s"$line | [$source]").mkString("\n")
165+
139166
new TypeSafeMatcher[String]:
140167

141168
override def describeMismatchSafely(
@@ -147,10 +174,10 @@ trait PcAssertions:
147174

148175
val expectedLines = cleanedExpected.linesIterator.toSeq
149176
val actualLines = cleanedActual.linesIterator.toSeq
177+
val diff = DiffUtil.mkColoredLineDiff(expectedLines, actualLines)
178+
val maybeEnhancedDiff = if isCompletion then getDetailedMessage(diff) else diff
150179

151-
mismatchDescription.appendText(
152-
DiffUtil.mkColoredLineDiff(expectedLines, actualLines)
153-
)
180+
mismatchDescription.appendText(maybeEnhancedDiff)
154181
mismatchDescription.appendText(System.lineSeparator)
155182

156183
override def describeTo(description: org.hamcrest.Description): Unit = ()

presentation-compiler/test/dotty/tools/pc/utils/TestingWorkspaceSearch.scala

+1-5
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,7 @@ class TestingWorkspaceSearch(classpath: Seq[String]):
4545

4646
val nioPath = Paths.get(path)
4747
val uri = nioPath.toUri()
48-
val symbols =
49-
DefSymbolCollector(
50-
driver,
51-
CompilerVirtualFileParams(uri, text)
52-
).namedDefSymbols
48+
val symbols = DefSymbolCollector(driver, CompilerVirtualFileParams(uri, text)).namedDefSymbols
5349

5450
// We have to map symbol from this Context, to one in PresentationCompiler
5551
// To do it we are searching it with semanticdb symbol

0 commit comments

Comments
 (0)