Skip to content

Commit 7205f20

Browse files
rochalarochala
committed
Add tests, don't cache when CURSOR is added
1 parent 748d439 commit 7205f20

27 files changed

+256
-94
lines changed

Diff for: ‎compiler/src/dotty/tools/dotc/ast/NavigateAST.scala

+27-10
Original file line numberDiff line numberDiff line change
@@ -75,24 +75,41 @@ object NavigateAST {
7575
def pathTo(span: Span, from: List[Positioned], skipZeroExtent: Boolean = false)(using Context): List[Positioned] = {
7676
def childPath(it: Iterator[Any], path: List[Positioned]): List[Positioned] = {
7777
var bestFit: List[Positioned] = path
78-
while (it.hasNext) {
79-
val path1 = it.next() match {
80-
// FIXME this has to be changed to deterministicaly find recoveed tree
81-
case untpd.Select(qual, name) if name == StdNames.nme.??? => path
78+
while (it.hasNext) do
79+
val path1 = it.next() match
80+
case sel: untpd.Select if isTreeFromRecovery(sel) => path
8281
case p: Positioned if !p.isInstanceOf[Closure[?]] => singlePath(p, path)
8382
case m: untpd.Modifiers => childPath(m.productIterator, path)
8483
case xs: List[?] => childPath(xs.iterator, path)
8584
case _ => path
86-
}
87-
if ((path1 ne path) &&
88-
((bestFit eq path) ||
89-
bestFit.head.span != path1.head.span &&
90-
envelops(bestFit.head.span, path1.head.span)))
85+
86+
if (path1 ne path) && ((bestFit eq path) || isBetterFit(bestFit, path1)) then
9187
bestFit = path1
92-
}
88+
9389
bestFit
9490
}
9591

92+
/**
93+
* When choosing better fit we compare spans. If candidate span has starting or ending point inside (exclusive)
94+
* current best fit it is selected as new best fit. This means that same spans are failing the first predicate.
95+
*
96+
* In case when spans start and end at same offsets we prefer non synthethic one.
97+
*/
98+
def isBetterFit(currentBest: List[Positioned], candidate: List[Positioned]): Boolean =
99+
if currentBest.isEmpty && candidate.nonEmpty then true
100+
else if currentBest.nonEmpty && candidate.nonEmpty then
101+
val bestSpan= currentBest.head.span
102+
val candidateSpan = candidate.head.span
103+
104+
bestSpan != candidateSpan &&
105+
envelops(bestSpan, candidateSpan) ||
106+
bestSpan.contains(candidateSpan) && bestSpan.isSynthetic && !candidateSpan.isSynthetic
107+
else false
108+
109+
110+
def isTreeFromRecovery(p: untpd.Select): Boolean =
111+
p.name == StdNames.nme.??? && p.qualifier.symbol.name == StdNames.nme.Predef && p.span.isSynthetic
112+
96113
def envelops(a: Span, b: Span): Boolean =
97114
!b.exists || a.exists && (
98115
(a.start < b.start && a.end >= b.end ) || (a.start <= b.start && a.end > b.end)

Diff for: ‎compiler/src/dotty/tools/dotc/parsing/Parsers.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,7 @@ object Parsers {
406406
false
407407
}
408408

409-
def errorTermTree(start: Offset): Tree = atSpan(start, in.offset, in.offset) { unimplementedExpr }
409+
def errorTermTree(start: Offset): Tree = atSpan(Span(start, in.offset)) { unimplementedExpr }
410410

411411
private var inFunReturnType = false
412412
private def fromWithinReturnType[T](body: => T): T = {

Diff for: ‎language-server/test/dotty/tools/languageserver/CompletionTest.scala

-9
Original file line numberDiff line numberDiff line change
@@ -1706,15 +1706,6 @@ class CompletionTest {
17061706
("getOrElse", Method, "[V1 >: String](key: Int, default: => V1): V1"),
17071707
))
17081708

1709-
@Test def testtest: Unit =
1710-
code"""|object M {
1711-
| def sel$m1
1712-
|}
1713-
|"""
1714-
.completion(m1, Set(
1715-
("getOrElse", Method, "[V1 >: String](key: Int, default: => V1): V1"),
1716-
))
1717-
17181709
@Test def noEnumCompletionInNewContext: Unit =
17191710
code"""|enum TestEnum:
17201711
| case TestCase

Diff for: ‎language-server/test/dotty/tools/languageserver/HoverTest.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ class HoverTest {
227227
@Test def enums: Unit = {
228228
code"""|package example
229229
|enum TestEnum3:
230-
| case ${m1}A${m2} // no tooltip
230+
| case ${m1}A${m2} // no tooltip
231231
|
232232
|"""
233233
.hover(m1 to m2, hoverContent("example.TestEnum3"))

Diff for: ‎presentation-compiler/src/main/dotty/tools/pc/AutoImportsProvider.scala

-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import dotty.tools.dotc.core.Symbols.*
1313
import dotty.tools.dotc.interactive.Interactive
1414
import dotty.tools.dotc.interactive.InteractiveDriver
1515
import dotty.tools.dotc.util.SourceFile
16-
import dotty.tools.pc.AutoImports.*
1716
import dotty.tools.pc.completions.CompletionPos
1817
import dotty.tools.pc.utils.InteractiveEnrichments.*
1918

Diff for: ‎presentation-compiler/src/main/dotty/tools/pc/MetalsDriver.scala renamed to ‎presentation-compiler/src/main/dotty/tools/pc/CachingDriver.scala

+4-6
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ import dotty.tools.dotc.util.SourceFile
1010
import scala.compiletime.uninitialized
1111

1212
/**
13-
* MetalsDriver is a wrapper class that provides a compilation cache for InteractiveDriver.
14-
* MetalsDriver skips running compilation if
13+
* CachingDriver is a wrapper class that provides a compilation cache for InteractiveDriver.
14+
* CachingDriver skips running compilation if
1515
* - the target URI of `run` is the same as the previous target URI
1616
* - the content didn't change since the last compilation.
1717
*
@@ -27,9 +27,7 @@ import scala.compiletime.uninitialized
2727
* To avoid the complexity related to currentCtx,
2828
* we decided to cache only when the target URI only if the same as the previous run.
2929
*/
30-
class MetalsDriver(
31-
override val settings: List[String]
32-
) extends InteractiveDriver(settings):
30+
class CachingDriver(override val settings: List[String]) extends InteractiveDriver(settings):
3331

3432
@volatile private var lastCompiledURI: URI = uninitialized
3533

@@ -55,4 +53,4 @@ class MetalsDriver(
5553
lastCompiledURI = uri
5654
diags
5755

58-
end MetalsDriver
56+
end CachingDriver

Diff for: ‎presentation-compiler/src/main/dotty/tools/pc/PcInlayHintsProvider.scala

-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import scala.meta.internal.pc.LabelPart.*
1414
import scala.meta.pc.InlayHintsParams
1515
import scala.meta.pc.SymbolSearch
1616

17-
import dotty.tools.dotc.ast.tpd
1817
import dotty.tools.dotc.ast.tpd.*
1918
import dotty.tools.dotc.core.Contexts.Context
2019
import dotty.tools.dotc.core.Flags

Diff for: ‎presentation-compiler/src/main/dotty/tools/pc/Scala3CompilerAccess.scala

+2-1
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,14 @@ import scala.meta.internal.pc.CompilerAccess
88
import scala.meta.pc.PresentationCompilerConfig
99

1010
import dotty.tools.dotc.reporting.StoreReporter
11+
import dotty.tools.dotc.interactive.InteractiveDriver
1112

1213
class Scala3CompilerAccess(
1314
config: PresentationCompilerConfig,
1415
sh: Option[ScheduledExecutorService],
1516
newCompiler: () => Scala3CompilerWrapper
1617
)(using ec: ExecutionContextExecutor, rc: ReportContext)
17-
extends CompilerAccess[StoreReporter, MetalsDriver](
18+
extends CompilerAccess[StoreReporter, InteractiveDriver](
1819
config,
1920
sh,
2021
newCompiler,

Diff for: ‎presentation-compiler/src/main/dotty/tools/pc/Scala3CompilerWrapper.scala

+4-3
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,12 @@ import scala.meta.internal.pc.CompilerWrapper
44
import scala.meta.internal.pc.ReporterAccess
55

66
import dotty.tools.dotc.reporting.StoreReporter
7+
import dotty.tools.dotc.interactive.InteractiveDriver
78

8-
class Scala3CompilerWrapper(driver: MetalsDriver)
9-
extends CompilerWrapper[StoreReporter, MetalsDriver]:
9+
class Scala3CompilerWrapper(driver: InteractiveDriver)
10+
extends CompilerWrapper[StoreReporter, InteractiveDriver]:
1011

11-
override def compiler(): MetalsDriver = driver
12+
override def compiler(): InteractiveDriver = driver
1213

1314
override def resetReporter(): Unit =
1415
val ctx = driver.currentCtx

Diff for: ‎presentation-compiler/src/main/dotty/tools/pc/ScalaPresentationCompiler.scala

+15-19
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,13 @@ import dotty.tools.pc.completions.CompletionProvider
3333
import dotty.tools.pc.InferExpectedType
3434
import dotty.tools.pc.completions.OverrideCompletions
3535
import dotty.tools.pc.buildinfo.BuildInfo
36+
import dotty.tools.pc.SymbolInformationProvider
37+
import dotty.tools.dotc.interactive.InteractiveDriver
3638

3739
import org.eclipse.lsp4j.DocumentHighlight
3840
import org.eclipse.lsp4j.TextEdit
3941
import org.eclipse.lsp4j as l
40-
import dotty.tools.pc.SymbolInformationProvider
42+
4143

4244
case class ScalaPresentationCompiler(
4345
buildTargetIdentifier: String = "",
@@ -76,14 +78,20 @@ case class ScalaPresentationCompiler(
7678
override def withReportsLoggerLevel(level: String): PresentationCompiler =
7779
copy(reportsLevel = ReportLevel.fromString(level))
7880

79-
val compilerAccess: CompilerAccess[StoreReporter, MetalsDriver] =
81+
val compilerAccess: CompilerAccess[StoreReporter, InteractiveDriver] =
8082
Scala3CompilerAccess(
8183
config,
8284
sh,
83-
() => new Scala3CompilerWrapper(newDriver)
84-
)(using
85-
ec
86-
)
85+
() => new Scala3CompilerWrapper(CachingDriver(driverSettings))
86+
)(using ec)
87+
88+
val driverSettings =
89+
val implicitSuggestionTimeout = List("-Ximport-suggestion-timeout", "0")
90+
val defaultFlags = List("-color:never")
91+
val filteredOptions = removeDoubleOptions(options.filterNot(forbiddenOptions))
92+
93+
filteredOptions ::: defaultFlags ::: implicitSuggestionTimeout ::: "-classpath" :: classpath
94+
.mkString(File.pathSeparator) :: Nil
8795

8896
private def removeDoubleOptions(options: List[String]): List[String] =
8997
options match
@@ -92,19 +100,6 @@ case class ScalaPresentationCompiler(
92100
case head :: tail => head :: removeDoubleOptions(tail)
93101
case Nil => options
94102

95-
def newDriver: MetalsDriver =
96-
val implicitSuggestionTimeout = List("-Ximport-suggestion-timeout", "0")
97-
val defaultFlags = List("-color:never")
98-
val filteredOptions = removeDoubleOptions(
99-
options.filterNot(forbiddenOptions)
100-
)
101-
val settings =
102-
filteredOptions ::: defaultFlags ::: implicitSuggestionTimeout ::: "-classpath" :: classpath
103-
.mkString(
104-
File.pathSeparator
105-
) :: Nil
106-
new MetalsDriver(settings)
107-
108103
override def semanticTokens(
109104
params: VirtualFileParams
110105
): CompletableFuture[ju.List[Node]] =
@@ -146,6 +141,7 @@ case class ScalaPresentationCompiler(
146141
new CompletionProvider(
147142
search,
148143
driver,
144+
() => InteractiveDriver(driverSettings),
149145
params,
150146
config,
151147
buildTargetIdentifier,

Diff for: ‎presentation-compiler/src/main/dotty/tools/pc/ScriptFirstImportPosition.scala

-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package dotty.tools.pc
22

3-
import dotty.tools.dotc.ast.tpd.*
43
import dotty.tools.dotc.core.Comments.Comment
54

65
object ScriptFirstImportPosition:

Diff for: ‎presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala

-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package dotty.tools.pc
22

3-
import dotty.tools.dotc.ast.tpd.*
43
import dotty.tools.dotc.core.Contexts.*
54
import dotty.tools.dotc.core.Flags
65
import dotty.tools.dotc.core.Symbols.*

Diff for: ‎presentation-compiler/src/main/dotty/tools/pc/SymbolInformationProvider.scala

+1-2
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ import scala.meta.pc.PcSymbolKind
77
import scala.meta.pc.PcSymbolProperty
88

99
import dotty.tools.dotc.core.Contexts.Context
10-
import dotty.tools.dotc.core.Denotations.Denotation
11-
import dotty.tools.dotc.core.Denotations.MultiDenotation
1210
import dotty.tools.dotc.core.Flags
1311
import dotty.tools.dotc.core.Names.*
1412
import dotty.tools.dotc.core.StdNames.nme
@@ -19,6 +17,7 @@ import dotty.tools.pc.utils.InteractiveEnrichments.allSymbols
1917
import dotty.tools.pc.utils.InteractiveEnrichments.stripBackticks
2018
import scala.meta.internal.pc.PcSymbolInformation
2119
import scala.meta.internal.pc.SymbolInfo
20+
import dotty.tools.dotc.core.Denotations.{Denotation, MultiDenotation}
2221

2322
class SymbolInformationProvider(using Context):
2423

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

+19-6
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import dotty.tools.dotc.core.Contexts.Context
1616
import dotty.tools.dotc.core.Phases
1717
import dotty.tools.dotc.core.StdNames.nme
1818
import dotty.tools.dotc.core.Flags
19+
import dotty.tools.dotc.core.Names.DerivedName
1920
import dotty.tools.dotc.interactive.Interactive
2021
import dotty.tools.dotc.interactive.Completion
2122
import dotty.tools.dotc.interactive.InteractiveDriver
@@ -39,7 +40,8 @@ import scala.meta.pc.CompletionItemPriority
3940

4041
class CompletionProvider(
4142
search: SymbolSearch,
42-
driver: InteractiveDriver,
43+
cachingDriver: InteractiveDriver,
44+
freshDriver: () => InteractiveDriver,
4345
params: OffsetParams,
4446
config: PresentationCompilerConfig,
4547
buildTargetIdentifier: String,
@@ -52,6 +54,16 @@ class CompletionProvider(
5254

5355
val (wasCursorApplied, code) = applyCompletionCursor(params)
5456
val sourceFile = SourceFile.virtual(uri, code)
57+
58+
/** Creating a new fresh driver is way slower than reusing existing one,
59+
* but runnig a compilation has side effects that modifies the state of the driver.
60+
* We don't want to affect cachingDriver state with compilation including "CURSOR" suffix.
61+
*
62+
* We could in theory save this fresh driver for reuse, but it is a choice between extra memory usage and speed.
63+
* The scenario in which "CURSOR" is applied (empty query or query equal to any keyword) has a slim chance of happening.
64+
*/
65+
66+
val driver = if wasCursorApplied then freshDriver() else cachingDriver
5567
driver.run(uri, sourceFile)
5668

5769
given ctx: Context = driver.currentCtx
@@ -63,11 +75,12 @@ class CompletionProvider(
6375
val adjustedPath = Interactive.resolveTypedOrUntypedPath(tpdPath0, pos)(using newctx)
6476

6577
val tpdPath = tpdPath0 match
66-
// $1$ // FIXME add check for a $1$ name to make sure we only do the below in lifting case
67-
case Select(qual, name) :: tail if qual.symbol.is(Flags.Synthetic) =>
68-
qual.symbol.defTree match
69-
case valdef: ValDef => Select(valdef.rhs, name) :: tail
70-
case _ => tpdPath0
78+
case Select(qual, name) :: tail
79+
// If for any reason we end up in param after lifting, we want to inline the synthetic val
80+
if qual.symbol.is(Flags.Synthetic) && qual.symbol.name.isInstanceOf[DerivedName] =>
81+
qual.symbol.defTree match
82+
case valdef: ValDef => Select(valdef.rhs, name) :: tail
83+
case _ => tpdPath0
7184
case _ => tpdPath0
7285

7386

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

+1-6
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import java.nio.file.Path
55
import java.nio.file.Paths
66

77
import scala.collection.mutable
8-
import scala.meta.internal.metals.Fuzzy
98
import scala.meta.internal.metals.ReportContext
109
import scala.meta.internal.mtags.CoursierComplete
1110
import scala.meta.internal.pc.{IdentifierComparator, MemberOrdering, CompletionFuzzy}
@@ -27,15 +26,12 @@ import dotty.tools.dotc.core.Symbols.*
2726
import dotty.tools.dotc.core.Types.*
2827
import dotty.tools.dotc.interactive.Completion
2928
import dotty.tools.dotc.interactive.Completion.Mode
30-
import dotty.tools.dotc.interactive.Interactive
3129
import dotty.tools.dotc.util.SourcePosition
3230
import dotty.tools.dotc.util.SrcPos
3331
import dotty.tools.pc.AutoImports.AutoImportsGenerator
3432
import dotty.tools.pc.buildinfo.BuildInfo
3533
import dotty.tools.pc.completions.OverrideCompletions.OverrideExtractor
3634
import dotty.tools.pc.utils.InteractiveEnrichments.*
37-
import dotty.tools.dotc.core.Denotations.SingleDenotation
38-
import dotty.tools.dotc.interactive.Interactive
3935

4036
class Completions(
4137
text: String,
@@ -279,7 +275,6 @@ class Completions(
279275
val affix = if methodDenot.symbol.isConstructor && existsApply then
280276
adjustedPath match
281277
case (select @ Select(qual, _)) :: _ =>
282-
val start = qual.span.start
283278
val insertRange = select.sourcePos.startPos.withEnd(completionPos.queryEnd).toLsp
284279

285280
suffix
@@ -662,7 +657,7 @@ class Completions(
662657
.collect { case symbolic: CompletionValue.Symbolic => symbolic }
663658
.groupBy(_.symbol.fullName) // we somehow have to ignore proxy type
664659

665-
val filteredSymbolicCompletions = symbolicCompletionsMap.filter: (name, denots) =>
660+
val filteredSymbolicCompletions = symbolicCompletionsMap.filter: (name, _) =>
666661
lazy val existsTypeWithoutSuffix: Boolean = !symbolicCompletionsMap
667662
.get(name.toTypeName)
668663
.forall(_.forall(sym => sym.snippetAffix.suffixes.nonEmpty))

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

-2
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import dotty.tools.dotc.core.Types.TermRef
2727
import dotty.tools.dotc.core.Types.Type
2828
import dotty.tools.dotc.core.Types.TypeBounds
2929
import dotty.tools.dotc.core.Types.WildcardType
30-
import dotty.tools.dotc.util.SourcePosition
3130
import dotty.tools.pc.IndexedContext
3231
import dotty.tools.pc.utils.InteractiveEnrichments.*
3332
import scala.annotation.tailrec
@@ -295,7 +294,6 @@ object NamedArgCompletions:
295294
)
296295
}
297296

298-
// FIXME pass query here
299297
val prefix = ident
300298
.map(_.name.toString)
301299
.getOrElse("")

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

-2
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@ import dotty.tools.dotc.printing.RefinedPrinter
2424
import dotty.tools.dotc.printing.Texts.Text
2525
import dotty.tools.pc.AutoImports.AutoImportsGenerator
2626
import dotty.tools.pc.AutoImports.ImportSel
27-
import dotty.tools.pc.AutoImports.ImportSel.Direct
28-
import dotty.tools.pc.AutoImports.ImportSel.Rename
2927
import dotty.tools.pc.IndexedContext
3028
import dotty.tools.pc.IndexedContext.Result
3129
import dotty.tools.pc.Params

Diff for: ‎presentation-compiler/test/dotty/tools/pc/base/BaseInlayHintsSuite.scala

+1-3
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,7 @@ import scala.meta.internal.metals.CompilerRangeParams
88
import scala.language.unsafeNulls
99

1010
import dotty.tools.pc.utils.TestInlayHints
11-
import dotty.tools.pc.utils.TextEdits
1211

13-
import org.eclipse.lsp4j.TextEdit
1412

1513
class BaseInlayHintsSuite extends BasePCSuite {
1614

@@ -55,4 +53,4 @@ class BaseInlayHintsSuite extends BasePCSuite {
5553
obtained,
5654
)
5755

58-
}
56+
}

0 commit comments

Comments
 (0)