Stabilise returned completions by improving deduplication + extra completions 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]

Author: WojciechMazur

compiler/src/dotty/tools/dotc/interactive/Completion.scala

@@ -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
@@ -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
@@ -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)
@@ -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

language-server/test/dotty/tools/languageserver/CompletionTest.scala

@@ -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
           |}
@@ -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}"""
@@ -1489,7 +1477,6 @@ class CompletionTest {
         ("xDef", Method, "=> Int"),
         ("xVal", Field, "Int"),
         ("xObject", Module, "Foo.xObject"),
-        ("xClass", Module, "Foo.xClass"),
         ("xClass", Class, "Foo.xClass")))
   }
 
@@ -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:
@@ -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:

presentation-compiler/src/main/dotty/tools/pc/MetalsInteractive.scala

@@ -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

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

@@ -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

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

@@ -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

presentation-compiler/src/main/dotty/tools/pc/completions/CompletionAffix.scala

@@ -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)