Fix missing braces in interpolator, apply last review

Author: rochala

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

@@ -53,9 +53,9 @@ case class CompletionAffix(
       endPos   <- ranges.map(_.getEnd).maxOption
     yield Range(startPos, endPos)
 
-  private def loopPrefix(prefixes: List[PrefixKind]) =
+  private def loopPrefix(prefixes: List[PrefixKind]): String =
     prefixes match
-      case PrefixKind.New :: tail => "new "
+      case PrefixKind.New :: tail => "new " + loopPrefix(tail)
       case _ => ""
 
   /**

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

@@ -161,6 +161,25 @@ class CompletionProvider(
     val label = completion.labelWithDescription(printer)
     val ident = completion.insertText.getOrElse(completion.label)
 
+    lazy val isInStringInterpolation =
+      path match
+        // s"My name is $name"
+        case (_: Ident) :: (_: SeqLiteral) :: (_: Typed) :: Apply(
+              Select(Apply(Select(Select(_, name), _), _), _),
+              _
+            ) :: _ =>
+          name == StdNames.nme.StringContext
+        // "My name is $name"
+        case Literal(Constant(_: String)) :: _ =>
+          true
+        case _ =>
+          false
+
+    def wrapInBracketsIfRequired(newText: String): String =
+      if completion.snippetAffix.nonEmpty && isInStringInterpolation then
+        "{" + newText + "}"
+      else newText
+
     def mkItem(
         newText: String,
         additionalEdits: List[TextEdit] = Nil,
@@ -170,7 +189,7 @@ class CompletionProvider(
       val editRange = if newText.startsWith(oldText) then completionPos.stripSuffixEditRange
         else completionPos.toEditRange
 
-      val textEdit = new TextEdit(range.getOrElse(editRange), newText)
+      val textEdit = new TextEdit(range.getOrElse(editRange), wrapInBracketsIfRequired(newText))
 
       val item = new CompletionItem(label)
       item.setSortText(f"${idx}%05d")
@@ -199,20 +218,6 @@ class CompletionProvider(
     val completionTextSuffix = completion.snippetAffix.toSuffix
     val completionTextPrefix = completion.snippetAffix.toInsertPrefix
 
-    lazy val isInStringInterpolation =
-      path match
-        // s"My name is $name"
-        case (_: Ident) :: (_: SeqLiteral) :: (_: Typed) :: Apply(
-              Select(Apply(Select(Select(_, name), _), _), _),
-              _
-            ) :: _ =>
-          name == StdNames.nme.StringContext
-        // "My name is $name"
-        case Literal(Constant(_: String)) :: _ =>
-          true
-        case _ =>
-          false
-
     lazy val backtickSoftKeyword = path match
       case (_: Select) :: _ => false
       case _ => true
@@ -243,13 +248,12 @@ class CompletionProvider(
                 case IndexedContext.Result.InScope =>
                   mkItem(
                     v.insertText.getOrElse(
-                      completionTextPrefix +
-                      ident.backticked(
-                        backtickSoftKeyword
-                      ) + completionTextSuffix
+                      completionTextPrefix + ident.backticked(backtickSoftKeyword) + completionTextSuffix
                     ),
                     range = v.range,
                   )
+                // Special case when symbol is out of scope, and there is no auto import.
+                // It means that it will use fully qualified path
                 case _ if isInStringInterpolation =>
                   mkItem(
                     "{" + completionTextPrefix + sym.fullNameBackticked + completionTextSuffix + "}",
@@ -280,10 +284,7 @@ class CompletionProvider(
       case _ =>
         val nameText = completion.insertText.getOrElse(ident.backticked(backtickSoftKeyword))
         val nameWithAffixes = completionTextPrefix + nameText + completionTextSuffix
-        val insertText = if completion.snippetAffix.nonEmpty && isInStringInterpolation then
-          "{" + nameWithAffixes + "}"
-        else nameWithAffixes
-        mkItem(insertText, range = completion.range)
+        mkItem(nameWithAffixes, range = completion.range)
 
     end match
   end completionItems

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

@@ -262,10 +262,6 @@ class Completions(
 
     methodDenots.map { methodDenot =>
       val suffix = findSuffix(methodDenot.symbol)
-      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, _)) :: _ =>
@@ -606,29 +602,26 @@ class Completions(
   /** If we try to complete TypeName, we should favor types over terms with same name value and without suffix.
    */
   def deduplicateCompletions(completions: List[CompletionValue]): List[CompletionValue] =
-    val typeResultMappings = mutable.Map.empty[Name, Seq[CompletionValue]]
     val (symbolicCompletions, rest) = completions.partition:
       _.isInstanceOf[CompletionValue.Symbolic]
 
     val symbolicCompletionsMap = symbolicCompletions
       .collect { case symbolic: CompletionValue.Symbolic => symbolic }
       .groupBy(_.symbol.fullName) // we somehow have to ignore proxy type
 
-    symbolicCompletionsMap.foreach: (name, denots) =>
+    val filteredSymbolicCompletions = symbolicCompletionsMap.filter: (name, denots) =>
       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
+      (completionMode.is(Mode.Term) && !completionMode.is(Mode.ImportOrExport)) ||
       // show non synthetic symbols
       // companion test should not result TrieMap[K, V]
-      else if name.isTermName && !existsTypeWithoutSuffix then
-        typeResultMappings += name -> denots
-      else if name.isTypeName then
-        typeResultMappings += name -> denots
+      (name.isTermName && !existsTypeWithoutSuffix) ||
+      name.isTypeName
+    .toList.unzip._2.flatten
 
-    typeResultMappings.toList.unzip._2.flatten ++ rest
+    filteredSymbolicCompletions ++ rest
 
   extension (l: List[CompletionValue])
     def filterInteresting(

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

@@ -110,18 +110,17 @@ object InterpolatorCompletions:
       buildTargetIdentifier: String
   )(using Context, ReportContext): List[CompletionValue] =
     def newText(
-        name: String,
-        suffix: Option[String],
+        label: String,
+        affix: CompletionAffix ,
         identOrSelect: Ident | Select
     ): String =
-      val snippetCursor = suffixEnding(suffix, areSnippetsSupported)
+      val snippetCursor = suffixEnding(affix.toSuffixOpt, areSnippetsSupported)
       new StringBuilder()
         .append('{')
-        .append(
-          text.substring(identOrSelect.span.start, identOrSelect.span.end)
-        )
+        .append(affix.toPrefix) // we use toPrefix here, because previous prefix is added in the next step
+        .append(text.substring(identOrSelect.span.start, identOrSelect.span.end))
         .append('.')
-        .append(name.backticked)
+        .append(label.backticked)
         .append(snippetCursor)
         .append('}')
         .toString
@@ -156,11 +155,11 @@ object InterpolatorCompletions:
           completions.completionsWithAffix(
             sym,
             label,
-            (name, denot, suffix) =>
+            (name, denot, affix) =>
               CompletionValue.Interpolator(
                 denot.symbol,
                 label,
-                Some(newText(name, suffix.toSuffixOpt, identOrSelect)),
+                Some(newText(name, affix, identOrSelect)),
                 Nil,
                 Some(completionPos.originalCursorPosition.withStart(identOrSelect.span.start).toLsp),
                 // Needed for VS Code which will not show the completion otherwise
@@ -250,16 +249,18 @@ object InterpolatorCompletions:
       interpolatorEdit ++ dollarEdits
     end additionalEdits
 
-    def newText(symbolName: String, suffix: Option[String]): String =
+    def newText(symbolName: String, affix: CompletionAffix): String =
       val out = new StringBuilder()
       val identifier = symbolName.backticked
       val symbolNeedsBraces =
         interpolator.needsBraces ||
           identifier.startsWith("`") ||
-          suffix.isDefined
+          affix.toSuffixOpt.isDefined ||
+          affix.toPrefix.nonEmpty
       if symbolNeedsBraces && !hasOpeningBrace then out.append('{')
+      out.append(affix.toInsertPrefix)
       out.append(identifier)
-      out.append(suffixEnding(suffix, areSnippetsSupported))
+      out.append(suffixEnding(affix.toSuffixOpt, areSnippetsSupported))
       if symbolNeedsBraces && !hasClosingBrace then out.append('}')
       out.toString
     end newText
@@ -287,11 +288,11 @@ object InterpolatorCompletions:
         completions.completionsWithAffix(
           sym,
           label,
-          (name, denot, suffix) =>
+          (name, denot, affix) =>
             CompletionValue.Interpolator(
               denot.symbol,
               label,
-              Some(newText(name, suffix.toSuffixOpt)),
+              Some(newText(name, affix)),
               additionalEdits(),
               Some(nameRange),
               None,

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

@@ -779,3 +779,27 @@ class CompletionInterpolatorSuite extends BaseCompletionSuite:
          |""".stripMargin,
       "host: String"
     )
+
+  @Test def `prepend-new-missing-interpolator` =
+    checkSnippet(
+      """|object Wrapper:
+         |  "$Try@@"
+         |
+         |""".stripMargin,
+      """|Try$0
+         |{Try($0)}
+         |{new Try$0}
+         |""".stripMargin,
+    )
+
+  @Test def `prepend-new-interpolator` =
+    checkSnippet(
+      """|object Wrapper:
+         |  s"$Try@@"
+         |
+         |""".stripMargin,
+      """|Try
+         |{Try($0)}
+         |{new Try}
+         |""".stripMargin,
+    )