Skip to content

Commit

Permalink
Fix missing braces in interpolator, apply last review
Browse files Browse the repository at this point in the history
  • Loading branch information
rochala committed Apr 10, 2024
1 parent b74ca93 commit 64a02c1
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 _ => ""

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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")
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 + "}",
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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, _)) :: _ =>
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)

0 comments on commit 64a02c1

Please sign in to comment.