Skip to content

Commit e9b3bf2

Browse files
rochalaWojciechMazur
authored andcommitted
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. [Cherry-picked e196dec]
1 parent 42e8c0c commit e9b3bf2

File tree

7 files changed

+113
-49
lines changed

7 files changed

+113
-49
lines changed

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

+2-3
Original file line numberDiff line numberDiff line change
@@ -181,9 +181,8 @@ class CompletionProvider(
181181
item.setAdditionalTextEdits((completion.additionalEdits ++ additionalEdits).asJava)
182182
completion.insertMode.foreach(item.setInsertTextMode)
183183

184-
completion
185-
.completionData(buildTargetIdentifier)
186-
.foreach(data => item.setData(data.toJson))
184+
val data = completion.completionData(buildTargetIdentifier)
185+
item.setData(data.toJson)
187186

188187
item.setTags(completion.lspTags.asJava)
189188

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

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

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

3453
/**
@@ -44,17 +63,15 @@ object CompletionValue:
4463
sealed trait Symbolic extends CompletionValue:
4564
def symbol: Symbol
4665
def isFromWorkspace: Boolean = false
47-
def completionItemDataKind = CompletionItemData.None
66+
override def completionItemDataKind = CompletionItemData.None
4867

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

@@ -106,19 +123,24 @@ object CompletionValue:
106123
label: String,
107124
symbol: Symbol,
108125
override val snippetSuffix: CompletionSuffix
109-
) extends Symbolic
126+
) extends Symbolic {
127+
override def completionItemDataKind: Integer = CompletionSource.CompilerKind.ordinal
128+
}
110129
case class Scope(
111130
label: String,
112131
symbol: Symbol,
113132
override val snippetSuffix: CompletionSuffix,
114-
) extends Symbolic
133+
) extends Symbolic {
134+
override def completionItemDataKind: Integer = CompletionSource.ScopeKind.ordinal
135+
}
115136
case class Workspace(
116137
label: String,
117138
symbol: Symbol,
118139
override val snippetSuffix: CompletionSuffix,
119140
override val importSymbol: Symbol
120141
) extends Symbolic:
121142
override def isFromWorkspace: Boolean = true
143+
override def completionItemDataKind: Integer = CompletionSource.WorkspaceKind.ordinal
122144

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

@@ -150,8 +173,7 @@ object CompletionValue:
150173
override val range: Option[Range]
151174
) extends Symbolic:
152175
override def insertText: Option[String] = Some(value)
153-
override def completionItemDataKind: Integer =
154-
CompletionItemData.OverrideKind
176+
override def completionItemDataKind: Integer = CompletionSource.OverrideKind.ordinal
155177
override def completionItemKind(using Context): CompletionItemKind =
156178
CompletionItemKind.Method
157179
override def labelWithDescription(printer: ShortenedTypePrinter)(using Context): String =
@@ -164,6 +186,7 @@ object CompletionValue:
164186
symbol: Symbol
165187
) extends Symbolic:
166188
override def insertText: Option[String] = Some(label.replace("$", "$$").nn)
189+
override def completionItemDataKind: Integer = CompletionSource.OverrideKind.ordinal
167190
override def completionItemKind(using Context): CompletionItemKind =
168191
CompletionItemKind.Field
169192
override def description(printer: ShortenedTypePrinter)(using Context): String =
@@ -178,11 +201,13 @@ object CompletionValue:
178201
) extends CompletionValue:
179202
override def completionItemKind(using Context): CompletionItemKind =
180203
CompletionItemKind.Enum
204+
override def completionItemDataKind: Integer = CompletionSource.OverrideKind.ordinal
181205
override def insertText: Option[String] = Some(value)
182206
override def label: String = "Autofill with default values"
183207

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

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

@@ -202,6 +228,7 @@ object CompletionValue:
202228
override val range: Option[Range]
203229
) extends CompletionValue:
204230
override val filterText: Option[String] = insertText
231+
override def completionItemDataKind: Integer = CompletionSource.IvyImportKind.ordinal
205232
override def completionItemKind(using Context): CompletionItemKind =
206233
CompletionItemKind.Folder
207234

@@ -216,6 +243,7 @@ object CompletionValue:
216243
isWorkspace: Boolean = false,
217244
isExtension: Boolean = false
218245
) extends Symbolic:
246+
override def completionItemDataKind: Integer = CompletionSource.InterpolatorKind.ordinal
219247
override def description(
220248
printer: ShortenedTypePrinter
221249
)(using Context): String =
@@ -229,6 +257,7 @@ object CompletionValue:
229257
override val additionalEdits: List[TextEdit],
230258
desc: String
231259
) extends CompletionValue:
260+
override def completionItemDataKind: Integer = CompletionSource.MatchCompletionKind.ordinal
232261
override def completionItemKind(using Context): CompletionItemKind =
233262
CompletionItemKind.Enum
234263
override def description(printer: ShortenedTypePrinter)(using Context): String =
@@ -242,6 +271,7 @@ object CompletionValue:
242271
override val range: Option[Range] = None,
243272
override val command: Option[String] = None
244273
) extends Symbolic:
274+
override def completionItemDataKind: Integer = CompletionSource.CaseKeywordKind.ordinal
245275
override def completionItemKind(using Context): CompletionItemKind =
246276
CompletionItemKind.Method
247277

@@ -254,6 +284,7 @@ object CompletionValue:
254284
override def filterText: Option[String] = Some(description)
255285

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

Diff for: 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 =>

Diff for: 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("'''", "\"\"\"")

Diff for: 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))

Diff for: 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 = ()

Diff for: 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)