Apply comments, make new completions work with fully qualified path prefix

Author: rochala

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

@@ -1,56 +1,95 @@
 package dotty.tools.pc.completions
 
+import org.eclipse.lsp4j.TextEdit
+import org.eclipse.lsp4j.Position
+
 /**
  * @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[SuffixKind],
-    prefixes: List[PrefixKind],
-    snippet: SuffixKind,
+    suffixes: Set[Suffix],
+    prefixes: Set[Prefix],
+    snippet: Suffix,
+    currentPrefix: Option[String],
 ):
-  def addLabelSnippet = suffixes.contains(SuffixKind.Bracket)
-  def hasSnippet = snippet != SuffixKind.NoSuffix
+  def addLabelSnippet = suffixes.exists(_.kind == SuffixKind.Bracket)
+  def hasSnippet = snippet.kind != SuffixKind.NoSuffix
   def chain(copyFn: CompletionAffix => CompletionAffix) = copyFn(this)
-  def withNewSuffix(kind: SuffixKind) = this.copy(suffixes = suffixes + kind)
-  def withNewPrefix(kind: PrefixKind) = this.copy(prefixes = prefixes :+ kind)
-  def withNewSuffixSnippet(kind: SuffixKind) =
-    this.copy(suffixes = suffixes + kind, snippet = kind)
+  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 then "$0" else ""
+      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)
+    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[org.eclipse.lsp4j.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 org.eclipse.lsp4j.Range(startPos, endPos)
+
+  private def loopPrefix(prefixes: List[PrefixKind]) =
+    prefixes match
+      case PrefixKind.New :: tail => "new "
+      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 =
-    def loop(prefixes: List[PrefixKind]) =
-      prefixes match
-        case PrefixKind.New :: tail => "new "
-        case _ => ""
-    loop(prefixes)
+    loopPrefix(prefixes.toList.map(_.kind))
 
 end CompletionAffix
 
 object CompletionAffix:
   val empty = CompletionAffix(
     suffixes = Set.empty,
-    prefixes = Nil,
-    snippet = SuffixKind.NoSuffix,
+    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[org.eclipse.lsp4j.Range] = None)

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

@@ -197,7 +197,7 @@ class CompletionProvider(
     end mkItem
 
     val completionTextSuffix = completion.snippetAffix.toSuffix
-    val completionTextPrefix = completion.snippetAffix.toPrefix
+    val completionTextPrefix = completion.snippetAffix.toInsertPrefix
 
     lazy val isInStringInterpolation =
       path match

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

@@ -79,6 +79,9 @@ object CompletionValue:
       )
     def importSymbol: Symbol = symbol
 
+    override def range: Option[Range] =
+      snippetAffix.toInsertRange
+
     def completionItemKind(using Context): CompletionItemKind =
       val symbol = this.symbol
       if symbol.is(Package) || symbol.is(Module) then

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

@@ -33,6 +33,8 @@ import dotty.tools.pc.completions.OverrideCompletions.OverrideExtractor
 import dotty.tools.pc.buildinfo.BuildInfo
 import dotty.tools.pc.utils.MtagsEnrichments.*
 import dotty.tools.dotc.core.Denotations.SingleDenotation
+import org.eclipse.lsp4j.TextEdit
+import org.eclipse.lsp4j.Position
 
 class Completions(
     text: String,
@@ -144,7 +146,7 @@ class Completions(
       denots: Seq[SingleDenotation]
   ): List[CompletionValue] =
     denots.toList.flatMap: denot =>
-      completionsWithSuffix(
+      completionsWithAffix(
         denot,
         completion.show,
         (label, denot, suffix) => CompletionValue.Compiler(label, denot, suffix)
@@ -189,44 +191,43 @@ class Completions(
     CompletionAffix.empty
       .chain { suffix => // for [] suffix
         if shouldAddSnippet && symbol.info.typeParams.nonEmpty then
-          suffix.withNewSuffixSnippet(SuffixKind.Bracket)
+          suffix.withNewSuffixSnippet(Affix(SuffixKind.Bracket))
         else suffix
       }
       .chain { suffix => // for () suffix
         if shouldAddSnippet && symbol.is(Flags.Method) then
           val paramss = getParams(symbol)
           paramss match
             case Nil => suffix
-            case List(Nil) => suffix.withNewSuffix(SuffixKind.Brace)
+            case List(Nil) => suffix.withNewSuffix(Affix(SuffixKind.Brace))
             case _ if config.isCompletionSnippetsEnabled() =>
               val onlyParameterless = paramss.forall(_.isEmpty)
               lazy val onlyImplicitOrTypeParams = paramss.forall(
                 _.exists { sym =>
                   sym.isType || sym.is(Implicit) || sym.is(Given)
                 }
               )
-              if onlyParameterless then suffix.withNewSuffix(SuffixKind.Brace)
+              if onlyParameterless then suffix.withNewSuffix(Affix(SuffixKind.Brace))
               else if onlyImplicitOrTypeParams then suffix
-              else if suffix.hasSnippet then
-                suffix.withNewSuffix(SuffixKind.Brace)
-              else suffix.withNewSuffixSnippet(SuffixKind.Brace)
+              else if suffix.hasSnippet then suffix.withNewSuffix(Affix(SuffixKind.Brace))
+              else suffix.withNewSuffixSnippet(Affix(SuffixKind.Brace))
             case _ => suffix
           end match
         else suffix
       }
       .chain { suffix => // for {} suffix
         if shouldAddSnippet && isNew && isAbstractType(symbol) then
-          if suffix.hasSnippet then suffix.withNewSuffix(SuffixKind.Template)
-          else suffix.withNewSuffixSnippet(SuffixKind.Template)
+          if suffix.hasSnippet then suffix.withNewSuffix(Affix(SuffixKind.Template))
+          else suffix.withNewSuffixSnippet(Affix(SuffixKind.Template))
         else suffix
       }
 
   end findSuffix
 
-  def completionsWithSuffix(
+  def completionsWithAffix(
       denot: SingleDenotation,
       label: String,
-      toCompletionValue: (String, SingleDenotation, CompletionAffix) => CompletionValue
+      toCompletionValue: (String, SingleDenotation, CompletionAffix) => CompletionValue.Symbolic
   ): List[CompletionValue] =
     val sym = denot.symbol
     val hasNonSyntheticConstructor = sym.name.isTypeName && sym.isClass
@@ -258,17 +259,30 @@ class Completions(
 
       else denot :: Nil
 
-    val requiresInitDisambiguiation = methodDenots.exists(_.symbol.isConstructor) && methodDenots.exists(_.symbol.name == nme.apply)
+    val existsApply = methodDenots.exists(_.symbol.name == nme.apply)
 
     methodDenots.map { methodDenot =>
       val suffix = findSuffix(methodDenot.symbol)
-      val affix = if methodDenot.symbol.isConstructor && requiresInitDisambiguiation then
-        suffix.withNewPrefix(PrefixKind.New)
+      val currentPrefix = adjustedPath match
+        case Select(qual, _) :: _ => Some(qual.show + ".", qual.span.start)
+        case _ => None
+
+      val affix = if methodDenot.symbol.isConstructor && existsApply then
+        adjustedPath match
+          case (select @ Select(qual, _)) :: _ =>
+            val start = qual.span.start
+            val insertRange = select.sourcePos.startPos.withEnd(completionPos.queryEnd).toLsp
+
+            suffix
+              .withCurrentPrefix(qual.show + ".")
+              .withNewPrefix(Affix(PrefixKind.New, insertRange = Some(insertRange)))
+          case _ =>
+            suffix.withNewPrefix(Affix(PrefixKind.New))
       else suffix
       val name = undoBacktick(label)
       toCompletionValue(name, methodDenot, affix)
     }
-  end completionsWithSuffix
+  end completionsWithAffix
 
   /**
    * @return Tuple of completionValues and flag. If the latter boolean value is true
@@ -522,7 +536,7 @@ class Completions(
                 )
               )
             case _ =>
-              completionsWithSuffix(
+              completionsWithAffix(
                 sym,
                 sym.decodedName,
                 CompletionValue.Workspace(_, _, _, sym)
@@ -555,13 +569,13 @@ class Completions(
           && !sym.isConstructor && !isDefaultVariableSetter
 
         if isExtensionMethod then
-          completionsWithSuffix(
+          completionsWithAffix(
             sym,
             sym.decodedName,
             CompletionValue.Extension(_, _, _)
           ).map(visit).forall(_ == true)
         else if isImplicitClassMember then
-          completionsWithSuffix(
+          completionsWithAffix(
             sym,
             sym.decodedName,
             CompletionValue.ImplicitClass(_, _, _, sym.maybeOwner),
@@ -602,15 +616,15 @@ class Completions(
       .groupBy(_.symbol.fullName) // we somehow have to ignore proxy type
 
     symbolicCompletionsMap.foreach: (name, denots) =>
-      lazy val existsTypeWithSuffix: Boolean = symbolicCompletionsMap
+      lazy val existsTypeWithoutSuffix: Boolean = !symbolicCompletionsMap
         .get(name.toTypeName)
         .forall(_.forall(sym => sym.snippetAffix.suffixes.nonEmpty))
 
       if completionMode.is(Mode.Term) && !completionMode.is(Mode.ImportOrExport) then
         typeResultMappings += name -> denots
       // show non synthetic symbols
       // companion test should not result TrieMap[K, V]
-      else if name.isTermName && existsTypeWithSuffix then
+      else if name.isTermName && !existsTypeWithoutSuffix then
         typeResultMappings += name -> denots
       else if name.isTypeName then
         typeResultMappings += name -> denots

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

@@ -153,7 +153,7 @@ object InterpolatorCompletions:
               sym.name.toString()
             ) =>
           val label = sym.name.decoded
-          completions.completionsWithSuffix(
+          completions.completionsWithAffix(
             sym,
             label,
             (name, denot, suffix) =>
@@ -284,7 +284,7 @@ object InterpolatorCompletions:
             sym.name.decoded
           ) && !sym.isType =>
         val label = sym.name.decoded
-        completions.completionsWithSuffix(
+        completions.completionsWithAffix(
           sym,
           label,
           (name, denot, suffix) =>

presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionExtraConstructorSuite.scala

@@ -512,3 +512,27 @@ class CompletionExtraConstructorSuite extends BaseCompletionSuite:
       includeCompletionKind = true
     )
 
+  @Test def `prepend-new` =
+    checkSnippet(
+      """|object Wrapper:
+         |  Try@@
+         |
+         |""".stripMargin,
+      """|Try
+         |Try($0)
+         |new Try
+         |""".stripMargin,
+    )
+
+  @Test def `prepend-new-fully-qualified-path` =
+    checkSnippet(
+      """|object Wrapper:
+         |  scala.util.Try@@
+         |
+         |""".stripMargin,
+      """|Try
+         |Try($0)
+         |new scala.util.Try
+         |""".stripMargin,
+    )
+

presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionInterpolatorSuite.scala

@@ -626,7 +626,6 @@ class CompletionInterpolatorSuite extends BaseCompletionSuite:
          |  s"this is an interesting ${java.nio.file.Paths}"
          |}
          |""".stripMargin,
-      itemIndex = 0,
       assertSingleItem = false,
     )
 

presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSnippetSuite.scala

@@ -310,10 +310,21 @@ class CompletionSnippetSuite extends BaseCompletionSuite:
           |""".stripMargin,
       """|Try
          |Try($0)
-         |new Try
+         |new scala.util.Try
          |""".stripMargin
     )
 
+  @Test def `case-class2-edit` =
+    checkEditLine(
+      s"""|object Main {
+          |  ___
+          |}
+          |""".stripMargin,
+      "scala.util.Tr@@",
+      "new scala.util.Try",
+      filter = _.contains("new Try")
+    )
+
   @Test def `case-class3` =
     checkSnippet(
       s"""|object Main {