Skip to content

Commit

Permalink
Stabilise returned completions by improving deduplication + extra com…
Browse files Browse the repository at this point in the history
…pletions for constructors (#19976)

This PR doesn't address all issues.
In the future we need to get rid of
https://github.com/scala/scala3/compare/main...rochala:improved-deduplication-and-constructors-search?expand=1#diff-035851592480495dfdb20da6b615ec7dd77b3db70cda46aba56230d8cd690773R157-R167
as it is not working as intended. The future PR of shortened type
printer refactor includes a proper way to split extension params.
`CompletionValue.Interpolator` should also be removed, and instead we
should return workspace / completions as it is now hard to sort those
completions.

Next refactor is reusing completion affix for other kinds of completions
such as case completions, so prefix / suffix is handled in single place.

This PR will unblock fuzzy search in the compiler because of stabilizing
returned completions.
[Cherry-picked 97313ed][modified]
  • Loading branch information
WojciechMazur committed Jul 5, 2024
1 parent cb761cb commit 7f28de1
Show file tree
Hide file tree
Showing 23 changed files with 1,252 additions and 524 deletions.
90 changes: 45 additions & 45 deletions compiler/src/dotty/tools/dotc/interactive/Completion.scala
Original file line number Diff line number Diff line change
Expand Up @@ -86,26 +86,21 @@ object Completion:
*
* Otherwise, provide no completion suggestion.
*/
def completionMode(path: List[untpd.Tree], pos: SourcePosition): Mode =

val completionSymbolKind: Mode =
path match
case GenericImportSelector(sel) =>
if sel.imported.span.contains(pos.span) then Mode.ImportOrExport // import scala.@@
else if sel.isGiven && sel.bound.span.contains(pos.span) then Mode.ImportOrExport
else Mode.None // import scala.{util => u@@}
case GenericImportOrExport(_) => Mode.ImportOrExport | Mode.Scope // import TrieMa@@
case untpd.Literal(Constants.Constant(_: String)) :: _ => Mode.Term | Mode.Scope // literal completions
case (ref: untpd.RefTree) :: _ =>
val maybeSelectMembers = if ref.isInstanceOf[untpd.Select] then Mode.Member else Mode.Scope

if (ref.name.isTermName) Mode.Term | maybeSelectMembers
else if (ref.name.isTypeName) Mode.Type | maybeSelectMembers
else Mode.None

case _ => Mode.None

completionSymbolKind
def completionMode(path: List[untpd.Tree], pos: SourcePosition): Mode = path match
case GenericImportSelector(sel) =>
if sel.imported.span.contains(pos.span) then Mode.ImportOrExport // import scala.@@
else if sel.isGiven && sel.bound.span.contains(pos.span) then Mode.ImportOrExport
else Mode.None // import scala.{util => u@@}
case GenericImportOrExport(_) => Mode.ImportOrExport | Mode.Scope // import TrieMa@@
case untpd.Literal(Constants.Constant(_: String)) :: _ => Mode.Term | Mode.Scope // literal completions
case (ref: untpd.RefTree) :: _ =>
val maybeSelectMembers = if ref.isInstanceOf[untpd.Select] then Mode.Member else Mode.Scope

if (ref.name.isTermName) Mode.Term | maybeSelectMembers
else if (ref.name.isTypeName) Mode.Type | maybeSelectMembers
else Mode.None

case _ => Mode.None

/** When dealing with <errors> in varios palces we check to see if they are
* due to incomplete backticks. If so, we ensure we get the full prefix
Expand All @@ -130,7 +125,7 @@ object Completion:
def completionPrefix(path: List[untpd.Tree], pos: SourcePosition)(using Context): String =
def fallback: Int =
var i = pos.point - 1
while i >= 0 && Chars.isIdentifierPart(pos.source.content()(i)) do i -= 1
while i >= 0 && Character.isUnicodeIdentifierPart(pos.source.content()(i)) do i -= 1
i + 1

path match
Expand Down Expand Up @@ -278,6 +273,32 @@ object Completion:
if denot.isType then denot.symbol.showFullName
else denot.info.widenTermRefExpr.show

/** Include in completion sets only symbols that
* 1. is not absent (info is not NoType)
* 2. are not a primary constructor,
* 3. have an existing source symbol,
* 4. are the module class in case of packages,
* 5. are mutable accessors, to exclude setters for `var`,
* 6. symbol is not a package object
* 7. symbol is not an artifact of the compiler
* 8. symbol is not a constructor proxy module when in type completion mode
* 9. have same term/type kind as name prefix given so far
*/
def isValidCompletionSymbol(sym: Symbol, completionMode: Mode)(using Context): Boolean =
sym.exists &&
!sym.isAbsent() &&
!sym.isPrimaryConstructor &&
sym.sourceSymbol.exists &&
(!sym.is(Package) || sym.is(ModuleClass)) &&
!sym.isAllOf(Mutable | Accessor) &&
!sym.isPackageObject &&
!sym.is(Artifact) &&
!(completionMode.is(Mode.Type) && sym.isAllOf(ConstructorProxyModule)) &&
(
(completionMode.is(Mode.Term) && (sym.isTerm || sym.is(ModuleClass))
|| (completionMode.is(Mode.Type) && (sym.isType || sym.isStableMember)))
)

given ScopeOrdering(using Context): Ordering[Seq[SingleDenotation]] with
val order =
List(defn.ScalaPredefModuleClass, defn.ScalaPackageClass, defn.JavaLangPackageClass)
Expand Down Expand Up @@ -531,34 +552,13 @@ object Completion:
extMethodsWithAppliedReceiver.groupByName

/** Include in completion sets only symbols that
* 1. start with given name prefix, and
* 2. is not absent (info is not NoType)
* 3. are not a primary constructor,
* 4. have an existing source symbol,
* 5. are the module class in case of packages,
* 6. are mutable accessors, to exclude setters for `var`,
* 7. symbol is not a package object
* 8. symbol is not an artifact of the compiler
* 9. have same term/type kind as name prefix given so far
* 1. match the filter method,
* 2. satisfy [[Completion.isValidCompletionSymbol]]
*/
private def include(denot: SingleDenotation, nameInScope: Name)(using Context): Boolean =
val sym = denot.symbol


nameInScope.startsWith(prefix) &&
sym.exists &&
completionsFilter(NoType, nameInScope) &&
!sym.isAbsent() &&
!sym.isPrimaryConstructor &&
sym.sourceSymbol.exists &&
(!sym.is(Package) || sym.is(ModuleClass)) &&
!sym.isAllOf(Mutable | Accessor) &&
!sym.isPackageObject &&
!sym.is(Artifact) &&
(
(mode.is(Mode.Term) && (sym.isTerm || sym.is(ModuleClass))
|| (mode.is(Mode.Type) && (sym.isType || sym.isStableMember)))
)
isValidCompletionSymbol(denot.symbol, mode)

private def extractRefinements(site: Type)(using Context): Seq[SingleDenotation] =
site match
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -954,14 +954,8 @@ class CompletionTest {
.noCompletions()

@Test def i13624_annotType: Unit =
val expected1 = Set(
("MyAnnotation", Class, "MyAnnotation"),
("MyAnnotation", Module, "MyAnnotation"),
)
val expected2 = Set(
("MyAnnotation", Class, "Foo.MyAnnotation"),
("MyAnnotation", Module, "Foo.MyAnnotation"),
)
val expected1 = Set(("MyAnnotation", Class, "MyAnnotation"))
val expected2 = Set(("MyAnnotation", Class, "Foo.MyAnnotation"))
code"""object Foo{
| class MyAnnotation extends annotation.StaticAnnotation
|}
Expand All @@ -984,14 +978,8 @@ class CompletionTest {
@Test def i13624_annotation : Unit =
code"""@annotation.implicitNot${m1}
|@annotation.implicitNotFound @mai${m2}"""
.completion(m1,
("implicitNotFound", Class, "scala.annotation.implicitNotFound"),
("implicitNotFound", Module, "scala.annotation.implicitNotFound"),
)
.completion(m2,
("main", Class, "main"),
("main", Module, "main"),
)
.completion(m1, ("implicitNotFound", Class, "scala.annotation.implicitNotFound"))
.completion(m2, ("main", Class, "main"))

@Test def i13623_annotation : Unit =
code"""import annot${m1}"""
Expand Down Expand Up @@ -1489,7 +1477,6 @@ class CompletionTest {
("xDef", Method, "=> Int"),
("xVal", Field, "Int"),
("xObject", Module, "Foo.xObject"),
("xClass", Module, "Foo.xClass"),
("xClass", Class, "Foo.xClass")))
}

Expand Down Expand Up @@ -1557,9 +1544,7 @@ class CompletionTest {
|object T:
| extension (x: Test.TestSel$m1)
|"""
.completion(m1, Set(
("TestSelect", Module, "Test.TestSelect"), ("TestSelect", Class, "Test.TestSelect")
))
.completion(m1, Set(("TestSelect", Class, "Test.TestSelect")))

@Test def extensionDefinitionCompletionsSelectNested: Unit =
code"""|object Test:
Expand All @@ -1568,9 +1553,7 @@ class CompletionTest {
|object T:
| extension (x: Test.Test2.TestSel$m1)
|"""
.completion(m1, Set(
("TestSelect", Module, "Test.Test2.TestSelect"), ("TestSelect", Class, "Test.Test2.TestSelect")
))
.completion(m1, Set(("TestSelect", Class, "Test.Test2.TestSelect")))

@Test def extensionDefinitionCompletionsSelectInside: Unit =
code"""|object Test:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import scala.annotation.tailrec

import dotc.*
import ast.*, tpd.*
import core.*, Contexts.*, Decorators.*, Flags.*, Names.*, Symbols.*, Types.*
import core.*, Contexts.*, Flags.*, Names.*, Symbols.*, Types.*
import interactive.*
import util.*
import util.SourcePosition
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import dotty.tools.dotc.interactive.InteractiveDriver
import dotty.tools.dotc.util.SourceFile
import dotty.tools.dotc.util.SourcePosition
import dotty.tools.dotc.util.Spans.Span
import dotty.tools.pc.IndexedContext

import org.eclipse.lsp4j.InlayHint
import org.eclipse.lsp4j.InlayHintKind
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,14 @@ import dotty.tools.dotc.core.Flags
import dotty.tools.dotc.core.Symbols.*
import dotty.tools.dotc.interactive.Interactive
import dotty.tools.dotc.interactive.InteractiveDriver
import dotty.tools.dotc.parsing.Tokens.closingRegionTokens
import dotty.tools.dotc.reporting.ErrorMessageID
import dotty.tools.dotc.reporting.ExpectedTokenButFound
import dotty.tools.dotc.util.Signatures
import dotty.tools.dotc.util.SourceFile
import dotty.tools.dotc.util.Spans
import dotty.tools.dotc.util.Spans.Span
import dotty.tools.pc.printer.ShortenedTypePrinter
import dotty.tools.pc.printer.ShortenedTypePrinter.IncludeDefaultParam
import dotty.tools.pc.utils.MtagsEnrichments.*
import org.eclipse.lsp4j as l

import scala.jdk.CollectionConverters.*
import scala.jdk.OptionConverters.*
import scala.meta.internal.metals.ReportContext
import scala.meta.pc.OffsetParams
import scala.meta.pc.SymbolDocumentation
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
package dotty.tools.pc.completions

import org.eclipse.lsp4j.Position
import org.eclipse.lsp4j.Range

/**
* @param suffixes which we should insert
* @param prefixes which we should insert
* @param snippet which suffix should we insert the snippet $0
*/
case class CompletionAffix(
suffixes: Set[Suffix],
prefixes: Set[Prefix],
snippet: Suffix,
currentPrefix: Option[String],
):
def addLabelSnippet = suffixes.exists(_.kind == SuffixKind.Bracket)
def hasSnippet = snippet.kind != SuffixKind.NoSuffix
def chain(copyFn: CompletionAffix => CompletionAffix) = copyFn(this)
def withNewSuffix(kind: Suffix) = this.copy(suffixes = suffixes + kind)
def withNewPrefix(kind: Prefix) = this.copy(prefixes = prefixes + kind)
def withCurrentPrefix(currentPrefix: String) = this.copy(currentPrefix = Some(currentPrefix))
def withNewSuffixSnippet(suffix: Suffix) =
this.copy(suffixes = suffixes + suffix, snippet = suffix)

def nonEmpty: Boolean = suffixes.nonEmpty || prefixes.nonEmpty

def toSuffix: String =
def loop(suffixes: List[SuffixKind]): String =
def cursor = if suffixes.head == snippet.kind then "$0" else ""
suffixes match
case SuffixKind.Brace :: tail => s"($cursor)" + loop(tail)
case SuffixKind.Bracket :: tail => s"[$cursor]" + loop(tail)
case SuffixKind.Template :: tail => s" {$cursor}" + loop(tail)
case _ => ""
loop(suffixes.toList.map(_.kind))

def toSuffixOpt: Option[String] =
val edit = toSuffix
if edit.nonEmpty then Some(edit) else None


given Ordering[Position] = Ordering.by(elem => (elem.getLine, elem.getCharacter))

def toInsertRange: Option[Range] =
import scala.language.unsafeNulls

val ranges = prefixes.collect:
case Affix(_, Some(range)) => range
.toList
for
startPos <- ranges.map(_.getStart).minOption
endPos <- ranges.map(_.getEnd).maxOption
yield Range(startPos, endPos)

private def loopPrefix(prefixes: List[PrefixKind]): String =
prefixes match
case PrefixKind.New :: tail => "new " + loopPrefix(tail)
case _ => ""

/**
* We need to insert previous prefix, but we don't want to display it in the label i.e.
* ```scala
* scala.util.Tr@@
* ````
* should return `new Try[T]: Try[T]`
* but insert `new scala.util.Try`
*
*/
def toInsertPrefix: String =
loopPrefix(prefixes.toList.map(_.kind)) + currentPrefix.getOrElse("")

def toPrefix: String =
loopPrefix(prefixes.toList.map(_.kind))

end CompletionAffix

object CompletionAffix:
val empty = CompletionAffix(
suffixes = Set.empty,
prefixes = Set.empty,
snippet = Affix(SuffixKind.NoSuffix),
currentPrefix = None,
)

enum SuffixKind:
case Brace, Bracket, Template, NoSuffix

enum PrefixKind:
case New

type Suffix = Affix[SuffixKind]
type Prefix = Affix[PrefixKind]

private case class Affix[+T](kind: T, insertRange: Option[Range] = None)
Loading

0 comments on commit 7f28de1

Please sign in to comment.