Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove artificial CURSOR added to code in the completions #20899

Merged
merged 4 commits into from
Sep 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 38 additions & 8 deletions compiler/src/dotty/tools/dotc/ast/NavigateAST.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
*/
Expand Down
13 changes: 7 additions & 6 deletions compiler/src/dotty/tools/dotc/interactive/Completion.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/parsing/Parsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.*

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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

Expand All @@ -55,4 +53,4 @@ class MetalsDriver(
lastCompiledURI = uri
diags

end MetalsDriver
end CachingDriver
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "",
Expand Down Expand Up @@ -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
Expand All @@ -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]] =
Expand Down Expand Up @@ -146,6 +141,7 @@ case class ScalaPresentationCompiler(
new CompletionProvider(
search,
driver,
() => InteractiveDriver(driverSettings),
params,
config,
buildTargetIdentifier,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package dotty.tools.pc

import dotty.tools.dotc.ast.tpd.*
import dotty.tools.dotc.core.Comments.Comment

object ScriptFirstImportPosition:
Expand Down
Original file line number Diff line number Diff line change
@@ -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.*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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.
Expand Down
Loading
Loading