Remove artificial CURSOR added to code in the completions (#20899)

This PR aims to remove the need for artificial `CURSOR` identifier which
was appended after current cursor position:

```scala
Lis@@
```

Would result in code:
```scala
Lis@@cursor
```

This lead to super inefficient IDE experience in terms of number of
compilations done after each change, but let's be a bit more specific.
Let's imagine that we have 2 scenarios in which IDE events arrive to
metals:
- Scenario A: Completion (CURSOR compilation) -> Inlay Hints (No CURSOR
compilation)
- Scenario B: Semantic Highlight (No CURSOR compilation) -> Completion
(CURSOR compilation) -> Inlay Hints (No CURSOR compilation)

On top of that, we've implemented a compilation caching, where code
snippet and compiler configuration is a key. Now you should notice the
issue, that adding a CURSOR into a code has different compilation result
with cache invalidations.

In theory, we could handle CURSOR compilation as normal ones, but in
reality it is a completely different run with different result (for
example in diagnostics, as each one will contain CURSOR in the message).
This is a no-go, especially if we would want to have diagnostics coming
from presentation compiler in the future.

Because of that, each keypress results in at least 2 compilation and in
the worst case scenario in 3. This also make metals way more battery
heavy.

This PR is an attempt to drop CURSOR insertion for most cases.

A bit of history, how we ended up with CURSOR in a first place.
Most of the reasons are caused by parser and its recovery.
For example, finding a proper scope in this snippet:
```scala
def outer: Int =
  def inner: Int =
    val innerVal = 1
  @@ // completion triggered here
```
We have to find the correct scope in which we are (inner vs outer). We
can achieve this in multiple ways, for example, count indents. This
solution may not be so straightforward, as there can be different
indentations etc. Inserting a synthetic `CURSOR` into this place:
```scala
def outer: Int =
  def inner: Int =
    val innerVal = 1
  @@cursor // completion triggered here
```
Will actually parse into an identifier with scope provided to us by
Scala compiler. This is way easier and will always be correct.

Second example are keywords, let's say we have the following snippet:
```scala
var value = 0
val newValue = 1
value = new@@
```
This code will expect a type, as the parser found a new keyword. Adding
a `CURSOR` here resolves the problem, as now we're dealing with
`newCURSOR`, not `new` keyword (identifier vs keyword).

This PR is basically a change, which disables adding a CURSOR in all
cases but 2 mentioned above. Those cases are very, very, very rare and
is something that we can deal with. With this change, each compilation
will now be cached and reused as intended resulting in way longer
battery life, performance, response times and will enable us to access
diagnostics for free without risking recompilation.

TODO: 
- [x] - remove caching for snippets with CURSOR, 
- [x] - add tests to verify it.



I'd also love to have this backported to LTS, as it is a significant
performance tweak and will allow me to add diagnostics on the fly for
the Scastie.

[test_windows_full]

Author: tgodzik

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

@@ -3,6 +3,7 @@ package ast
 
 import core.Contexts.*
 import core.Decorators.*
+import core.StdNames
 import util.Spans.*
 import Trees.{Closure, MemberDef, DefTree, WithLazyFields}
 import dotty.tools.dotc.core.Types.AnnotatedType
@@ -74,21 +75,50 @@ object NavigateAST {
   def pathTo(span: Span, from: List[Positioned], skipZeroExtent: Boolean = false)(using Context): List[Positioned] = {
     def childPath(it: Iterator[Any], path: List[Positioned]): List[Positioned] = {
       var bestFit: List[Positioned] = path
-      while (it.hasNext) {
-        val path1 = it.next() match {
+      while (it.hasNext) do
+        val path1 = it.next() match
+          case sel: untpd.Select if isRecoveryTree(sel) => path
+          case sel: untpd.Ident  if isPatternRecoveryTree(sel) => path
           case p: Positioned if !p.isInstanceOf[Closure[?]] => singlePath(p, path)
           case m: untpd.Modifiers => childPath(m.productIterator, path)
           case xs: List[?] => childPath(xs.iterator, path)
           case _ => path
-        }
-        if ((path1 ne path) &&
-            ((bestFit eq path) ||
-             bestFit.head.span != path1.head.span &&
-             bestFit.head.span.contains(path1.head.span)))
+
+        if (path1 ne path) && ((bestFit eq path) || isBetterFit(bestFit, path1)) then
           bestFit = path1
-      }
+
       bestFit
     }
+
+    /**
+      * When choosing better fit we compare spans. If candidate span has starting or ending point inside (exclusive)
+      * current best fit it is selected as new best fit. This means that same spans are failing the first predicate.
+      *
+      * In case when spans start and end at same offsets we prefer non synthethic one.
+      */
+    def isBetterFit(currentBest: List[Positioned], candidate: List[Positioned]): Boolean =
+      if currentBest.isEmpty && candidate.nonEmpty then true
+      else if currentBest.nonEmpty && candidate.nonEmpty then
+        val bestSpan = currentBest.head.span
+        val candidateSpan = candidate.head.span
+
+        bestSpan != candidateSpan &&
+          envelops(bestSpan, candidateSpan) ||
+          bestSpan.contains(candidateSpan) && bestSpan.isSynthetic && !candidateSpan.isSynthetic
+      else false
+
+    def isRecoveryTree(sel: untpd.Select): Boolean =
+      sel.span.isSynthetic
+        && (sel.name == StdNames.nme.??? && sel.qualifier.symbol.name == StdNames.nme.Predef)
+
+    def isPatternRecoveryTree(ident: untpd.Ident): Boolean =
+      ident.span.isSynthetic && StdNames.nme.WILDCARD == ident.name
+
+    def envelops(a: Span, b: Span): Boolean =
+      !b.exists || a.exists && (
+        (a.start < b.start && a.end >= b.end ) || (a.start <= b.start && a.end > b.end)
+      )
+
     /*
      * Annotations trees are located in the Type
      */

Diff for: compiler/src/dotty/tools/dotc/interactive/Completion.scala

@@ -121,16 +121,17 @@ object Completion:
       case _ =>
         ""
 
+  def naiveCompletionPrefix(text: String, offset: Int): String =
+    var i = offset - 1
+    while i >= 0 && text(i).isUnicodeIdentifierPart do i -= 1
+    i += 1 // move to first character
+    text.slice(i, offset)
+
   /**
    * Inspect `path` to determine the completion prefix. Only symbols whose name start with the
    * returned prefix should be considered.
    */
   def completionPrefix(path: List[untpd.Tree], pos: SourcePosition)(using Context): String =
-    def fallback: Int =
-      var i = pos.point - 1
-      while i >= 0 && Character.isUnicodeIdentifierPart(pos.source.content()(i)) do i -= 1
-      i + 1
-
     path match
       case GenericImportSelector(sel) =>
         if sel.isGiven then completionPrefix(sel.bound :: Nil, pos)
@@ -148,7 +149,7 @@ object Completion:
       case (tree: untpd.RefTree) :: _ if tree.name != nme.ERROR =>
         tree.name.toString.take(pos.span.point - tree.span.point)
 
-      case _ => pos.source.content.slice(fallback, pos.point).mkString
+      case _ => naiveCompletionPrefix(pos.source.content().mkString, pos.point)
 
 
   end completionPrefix

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

@@ -406,7 +406,7 @@ object Parsers {
       false
     }
 
-    def errorTermTree(start: Offset): Tree = atSpan(start, in.offset, in.offset) { unimplementedExpr }
+    def errorTermTree(start: Offset): Tree = atSpan(Span(start, in.offset)) { unimplementedExpr }
 
     private var inFunReturnType = false
     private def fromWithinReturnType[T](body: => T): T = {

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

@@ -227,7 +227,7 @@ class HoverTest {
   @Test def enums: Unit = {
     code"""|package example
            |enum TestEnum3:
-           | case ${m1}A${m2} // no tooltip
+           |  case ${m1}A${m2} // no tooltip
            |
            |"""
       .hover(m1 to m2, hoverContent("example.TestEnum3"))

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

@@ -13,7 +13,6 @@ import dotty.tools.dotc.core.Symbols.*
 import dotty.tools.dotc.interactive.Interactive
 import dotty.tools.dotc.interactive.InteractiveDriver
 import dotty.tools.dotc.util.SourceFile
-import dotty.tools.pc.AutoImports.*
 import dotty.tools.pc.completions.CompletionPos
 import dotty.tools.pc.utils.InteractiveEnrichments.*
 
@@ -67,7 +66,8 @@ final class AutoImportsProvider(
     val results = symbols.result.filter(isExactMatch(_, name))
 
     if results.nonEmpty then
-      val correctedPos = CompletionPos.infer(pos, params, path).toSourcePosition
+      val correctedPos =
+        CompletionPos.infer(pos, params, path, wasCursorApplied = false).toSourcePosition
       val mkEdit =
         path match
           // if we are in import section just specify full name

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

@@ -10,8 +10,8 @@ import dotty.tools.dotc.util.SourceFile
 import scala.compiletime.uninitialized
 
 /**
- * MetalsDriver is a wrapper class that provides a compilation cache for InteractiveDriver.
- * MetalsDriver skips running compilation if
+ * CachingDriver is a wrapper class that provides a compilation cache for InteractiveDriver.
+ * CachingDriver skips running compilation if
  * - the target URI of `run` is the same as the previous target URI
  * - the content didn't change since the last compilation.
  *
@@ -27,9 +27,7 @@ import scala.compiletime.uninitialized
  * To avoid the complexity related to currentCtx,
  * we decided to cache only when the target URI only if the same as the previous run.
  */
-class MetalsDriver(
-    override val settings: List[String]
-) extends InteractiveDriver(settings):
+class CachingDriver(override val settings: List[String]) extends InteractiveDriver(settings):
 
   @volatile private var lastCompiledURI: URI = uninitialized
 
@@ -55,4 +53,4 @@ class MetalsDriver(
     lastCompiledURI = uri
     diags
 
-end MetalsDriver
+end CachingDriver

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

@@ -14,7 +14,6 @@ import scala.meta.internal.pc.LabelPart.*
 import scala.meta.pc.InlayHintsParams
 import scala.meta.pc.SymbolSearch
 
-import dotty.tools.dotc.ast.tpd
 import dotty.tools.dotc.ast.tpd.*
 import dotty.tools.dotc.core.Contexts.Context
 import dotty.tools.dotc.core.Flags

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

@@ -8,13 +8,14 @@ import scala.meta.internal.pc.CompilerAccess
 import scala.meta.pc.PresentationCompilerConfig
 
 import dotty.tools.dotc.reporting.StoreReporter
+import dotty.tools.dotc.interactive.InteractiveDriver
 
 class Scala3CompilerAccess(
     config: PresentationCompilerConfig,
     sh: Option[ScheduledExecutorService],
     newCompiler: () => Scala3CompilerWrapper
 )(using ec: ExecutionContextExecutor, rc: ReportContext)
-    extends CompilerAccess[StoreReporter, MetalsDriver](
+    extends CompilerAccess[StoreReporter, InteractiveDriver](
       config,
       sh,
       newCompiler,

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

@@ -4,11 +4,12 @@ import scala.meta.internal.pc.CompilerWrapper
 import scala.meta.internal.pc.ReporterAccess
 
 import dotty.tools.dotc.reporting.StoreReporter
+import dotty.tools.dotc.interactive.InteractiveDriver
 
-class Scala3CompilerWrapper(driver: MetalsDriver)
-    extends CompilerWrapper[StoreReporter, MetalsDriver]:
+class Scala3CompilerWrapper(driver: InteractiveDriver)
+    extends CompilerWrapper[StoreReporter, InteractiveDriver]:
 
-  override def compiler(): MetalsDriver = driver
+  override def compiler(): InteractiveDriver = driver
 
   override def resetReporter(): Unit =
     val ctx = driver.currentCtx

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

@@ -33,11 +33,13 @@ import dotty.tools.pc.completions.CompletionProvider
 import dotty.tools.pc.InferExpectedType
 import dotty.tools.pc.completions.OverrideCompletions
 import dotty.tools.pc.buildinfo.BuildInfo
+import dotty.tools.pc.SymbolInformationProvider
+import dotty.tools.dotc.interactive.InteractiveDriver
 
 import org.eclipse.lsp4j.DocumentHighlight
 import org.eclipse.lsp4j.TextEdit
 import org.eclipse.lsp4j as l
-import dotty.tools.pc.SymbolInformationProvider
+
 
 case class ScalaPresentationCompiler(
     buildTargetIdentifier: String = "",
@@ -76,14 +78,20 @@ case class ScalaPresentationCompiler(
   override def withReportsLoggerLevel(level: String): PresentationCompiler =
     copy(reportsLevel = ReportLevel.fromString(level))
 
-  val compilerAccess: CompilerAccess[StoreReporter, MetalsDriver] =
+  val compilerAccess: CompilerAccess[StoreReporter, InteractiveDriver] =
     Scala3CompilerAccess(
       config,
       sh,
-      () => new Scala3CompilerWrapper(newDriver)
-    )(using
-      ec
-    )
+      () => new Scala3CompilerWrapper(CachingDriver(driverSettings))
+    )(using ec)
+
+  val driverSettings =
+    val implicitSuggestionTimeout = List("-Ximport-suggestion-timeout", "0")
+    val defaultFlags = List("-color:never")
+    val filteredOptions = removeDoubleOptions(options.filterNot(forbiddenOptions))
+
+    filteredOptions ::: defaultFlags ::: implicitSuggestionTimeout ::: "-classpath" :: classpath
+      .mkString(File.pathSeparator) :: Nil
 
   private def removeDoubleOptions(options: List[String]): List[String] =
     options match
@@ -92,19 +100,6 @@ case class ScalaPresentationCompiler(
       case head :: tail => head :: removeDoubleOptions(tail)
       case Nil => options
 
-  def newDriver: MetalsDriver =
-    val implicitSuggestionTimeout = List("-Ximport-suggestion-timeout", "0")
-    val defaultFlags = List("-color:never")
-    val filteredOptions = removeDoubleOptions(
-      options.filterNot(forbiddenOptions)
-    )
-    val settings =
-      filteredOptions ::: defaultFlags ::: implicitSuggestionTimeout ::: "-classpath" :: classpath
-        .mkString(
-          File.pathSeparator
-        ) :: Nil
-    new MetalsDriver(settings)
-
   override def semanticTokens(
       params: VirtualFileParams
   ): CompletableFuture[ju.List[Node]] =
@@ -146,6 +141,7 @@ case class ScalaPresentationCompiler(
       new CompletionProvider(
         search,
         driver,
+        () => InteractiveDriver(driverSettings),
         params,
         config,
         buildTargetIdentifier,

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

@@ -1,6 +1,5 @@
 package dotty.tools.pc
 
-import dotty.tools.dotc.ast.tpd.*
 import dotty.tools.dotc.core.Comments.Comment
 
 object ScriptFirstImportPosition:

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

@@ -1,6 +1,5 @@
 package dotty.tools.pc
 
-import dotty.tools.dotc.ast.tpd.*
 import dotty.tools.dotc.core.Contexts.*
 import dotty.tools.dotc.core.Flags
 import dotty.tools.dotc.core.Symbols.*

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

@@ -7,8 +7,6 @@ import scala.meta.pc.PcSymbolKind
 import scala.meta.pc.PcSymbolProperty
 
 import dotty.tools.dotc.core.Contexts.Context
-import dotty.tools.dotc.core.Denotations.Denotation
-import dotty.tools.dotc.core.Denotations.MultiDenotation
 import dotty.tools.dotc.core.Flags
 import dotty.tools.dotc.core.Names.*
 import dotty.tools.dotc.core.StdNames.nme
@@ -19,6 +17,7 @@ import dotty.tools.pc.utils.InteractiveEnrichments.allSymbols
 import dotty.tools.pc.utils.InteractiveEnrichments.stripBackticks
 import scala.meta.internal.pc.PcSymbolInformation
 import scala.meta.internal.pc.SymbolInfo
+import dotty.tools.dotc.core.Denotations.{Denotation, MultiDenotation}
 
 class SymbolInformationProvider(using Context):
 

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

@@ -22,7 +22,8 @@ case class CompletionPos(
   identEnd: Int,
   query: String,
   originalCursorPosition: SourcePosition,
-  sourceUri: URI
+  sourceUri: URI,
+  withCURSOR: Boolean
 ):
   def queryEnd: Int = originalCursorPosition.point
   def stripSuffixEditRange: l.Range = new l.Range(originalCursorPosition.offsetToPos(queryStart), originalCursorPosition.offsetToPos(identEnd))
@@ -34,17 +35,19 @@ object CompletionPos:
   def infer(
       sourcePos: SourcePosition,
       offsetParams: OffsetParams,
-      adjustedPath: List[Tree]
+      adjustedPath: List[Tree],
+      wasCursorApplied: Boolean
   )(using Context): CompletionPos =
     val identEnd = adjustedPath match
       case (refTree: RefTree) :: _ if refTree.name.toString.contains(Cursor.value) =>
         refTree.span.end - Cursor.value.length
+      case (refTree: RefTree) :: _  => refTree.span.end
       case _ => sourcePos.end
 
     val query = Completion.completionPrefix(adjustedPath, sourcePos)
     val start = sourcePos.end - query.length()
 
-    CompletionPos(start, identEnd, query.nn, sourcePos, offsetParams.uri.nn)
+    CompletionPos(start, identEnd, query.nn, sourcePos, offsetParams.uri.nn, wasCursorApplied)
 
   /**
    * Infer the indentation by counting the number of spaces in the given line.