Skip to content

Commit

Permalink
Merge pull request #14594 from ckipp01/backTickedOff
Browse files Browse the repository at this point in the history
fix(completions): add backticks when needed in completions
  • Loading branch information
prolativ authored Mar 7, 2022
2 parents 1080f1b + 5e5866b commit 3c5dbc3
Show file tree
Hide file tree
Showing 5 changed files with 229 additions and 8 deletions.
61 changes: 55 additions & 6 deletions compiler/src/dotty/tools/dotc/interactive/Completion.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import dotty.tools.dotc.core.StdNames.nme
import dotty.tools.dotc.core.SymDenotations.SymDenotation
import dotty.tools.dotc.core.TypeError
import dotty.tools.dotc.core.Types.{ExprType, MethodOrPoly, NameFilter, NoType, TermRef, Type}
import dotty.tools.dotc.parsing.Tokens
import dotty.tools.dotc.util.Chars
import dotty.tools.dotc.util.SourcePosition

import scala.collection.mutable
Expand Down Expand Up @@ -80,8 +82,8 @@ object Completion {
* Inspect `path` to determine the completion prefix. Only symbols whose name start with the
* returned prefix should be considered.
*/
def completionPrefix(path: List[untpd.Tree], pos: SourcePosition): String =
path match {
def completionPrefix(path: List[untpd.Tree], pos: SourcePosition)(using Context): String =
path match
case (sel: untpd.ImportSelector) :: _ =>
completionPrefix(sel.imported :: Nil, pos)

Expand All @@ -90,13 +92,22 @@ object Completion {
completionPrefix(selector :: Nil, pos)
}.getOrElse("")

// We special case Select here because we want to determine if the name
// is an error due to an unclosed backtick.
case (select: untpd.Select) :: _ if (select.name == nme.ERROR) =>
val content = select.source.content()
content.lift(select.nameSpan.start) match
case Some(char) if char == '`' =>
content.slice(select.nameSpan.start, select.span.end).mkString
case _ =>
""
case (ref: untpd.RefTree) :: _ =>
if (ref.name == nme.ERROR) ""
else ref.name.toString.take(pos.span.point - ref.span.point)

case _ =>
""
}
end completionPrefix

/** Inspect `path` to determine the offset where the completion result should be inserted. */
def completionOffset(path: List[Tree]): Int =
Expand All @@ -107,7 +118,11 @@ object Completion {

private def computeCompletions(pos: SourcePosition, path: List[Tree])(using Context): (Int, List[Completion]) = {
val mode = completionMode(path, pos)
val prefix = completionPrefix(path, pos)
val rawPrefix = completionPrefix(path, pos)

val hasBackTick = rawPrefix.headOption.contains('`')
val prefix = if hasBackTick then rawPrefix.drop(1) else rawPrefix

val completer = new Completer(mode, prefix, pos)

val completions = path match {
Expand All @@ -122,16 +137,49 @@ object Completion {
}

val describedCompletions = describeCompletions(completions)
val backtickedCompletions =
describedCompletions.map(completion => backtickCompletions(completion, hasBackTick))

val offset = completionOffset(path)

interactiv.println(i"""completion with pos = $pos,
| prefix = ${completer.prefix},
| term = ${completer.mode.is(Mode.Term)},
| type = ${completer.mode.is(Mode.Type)}
| results = $describedCompletions%, %""")
(offset, describedCompletions)
| results = $backtickCompletions%, %""")
(offset, backtickedCompletions)
}

def backtickCompletions(completion: Completion, hasBackTick: Boolean) =
if hasBackTick || needsBacktick(completion.label) then
completion.copy(label = s"`${completion.label}`")
else
completion

// This borrows from Metals, which itself borrows from Ammonite. This uses
// the same approach, but some of the utils that already exist in Dotty.
// https://github.com/scalameta/metals/blob/main/mtags/src/main/scala/scala/meta/internal/mtags/KeywordWrapper.scala
// https://github.com/com-lihaoyi/Ammonite/blob/73a874173cd337f953a3edc9fb8cb96556638fdd/amm/util/src/main/scala/ammonite/util/Model.scala
private def needsBacktick(s: String) =
val chunks = s.split("_", -1)

val validChunks = chunks.zipWithIndex.forall { case (chunk, index) =>
chunk.forall(Chars.isIdentifierPart) ||
(chunk.forall(Chars.isOperatorPart) &&
index == chunks.length - 1 &&
!(chunks.lift(index - 1).contains("") && index - 1 == 0))
}

val validStart =
Chars.isIdentifierStart(s(0)) || chunks(0).forall(Chars.isOperatorPart)

val valid = validChunks && validStart && !keywords.contains(s)

!valid
end needsBacktick

private lazy val keywords = Tokens.keywords.map(Tokens.tokenString)

/**
* Return the list of code completions with descriptions based on a mapping from names to the denotations they refer to.
* If several denotations share the same name, each denotation will be transformed into a separate completion item.
Expand Down Expand Up @@ -384,6 +432,7 @@ object Completion {
private def include(denot: SingleDenotation, nameInScope: Name)(using Context): Boolean =
val sym = denot.symbol


nameInScope.startsWith(prefix) &&
sym.exists &&
completionsFilter(NoType, nameInScope) &&
Expand Down
11 changes: 10 additions & 1 deletion compiler/src/dotty/tools/repl/JLineTerminal.scala
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,8 @@ final class JLineTerminal extends java.io.Closeable {
def currentToken: TokenData /* | Null */ = {
val source = SourceFile.virtual("<completions>", input)
val scanner = new Scanner(source)(using ctx.fresh.setReporter(Reporter.NoReporter))
var lastBacktickErrorStart: Option[Int] = None

while (scanner.token != EOF) {
val start = scanner.offset
val token = scanner.token
Expand All @@ -128,7 +130,14 @@ final class JLineTerminal extends java.io.Closeable {

val isCurrentToken = cursor >= start && cursor <= end
if (isCurrentToken)
return TokenData(token, start, end)
return TokenData(token, lastBacktickErrorStart.getOrElse(start), end)


// we need to enclose the last backtick, which unclosed produces ERROR token
if (token == ERROR && input(start) == '`') then
lastBacktickErrorStart = Some(start)
else
lastBacktickErrorStart = None
}
null
}
Expand Down
9 changes: 8 additions & 1 deletion compiler/src/dotty/tools/repl/ReplDriver.scala
Original file line number Diff line number Diff line change
Expand Up @@ -198,12 +198,19 @@ class ReplDriver(settings: Array[String],
state.copy(context = run.runContext)
}

private def stripBackTicks(label: String) =
if label.startsWith("`") && label.endsWith("`") then
label.drop(1).dropRight(1)
else
label

/** Extract possible completions at the index of `cursor` in `expr` */
protected final def completions(cursor: Int, expr: String, state0: State): List[Candidate] = {
def makeCandidate(label: String) = {

new Candidate(
/* value = */ label,
/* displ = */ label, // displayed value
/* displ = */ stripBackTicks(label), // displayed value
/* group = */ null, // can be used to group completions together
/* descr = */ null, // TODO use for documentation?
/* suffix = */ null,
Expand Down
58 changes: 58 additions & 0 deletions compiler/test/dotty/tools/repl/TabcompleteTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -133,4 +133,62 @@ class TabcompleteTests extends ReplTest {
tabComplete("import quoted.* ; def fooImpl(using Quotes): Expr[Int] = { import quotes.reflect.* ; TypeRepr.of[Int].s"))
}

@Test def backticked = initially {
assertEquals(
List(
"!=",
"##",
"->",
"==",
"__system",
"`back-tick`",
"`match`",
"asInstanceOf",
"dot_product_*",
"ensuring",
"eq",
"equals",
"foo",
"formatted",
"fromOrdinal",
"getClass",
"hashCode",
"isInstanceOf",
"ne",
"nn",
"notify",
"notifyAll",
"synchronized",
"toString",
"valueOf",
"values",
"wait",
""
),
tabComplete("""|enum Foo:
| case `back-tick`
| case `match`
| case foo
| case dot_product_*
| case __system
|
|Foo."""stripMargin))
}


@Test def backtickedAlready = initially {
assertEquals(
List(
"`back-tick`"
),
tabComplete("""|enum Foo:
| case `back-tick`
| case `match`
| case foo
| case dot_product_*
| case __system
|
|Foo.`bac"""stripMargin))
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -1023,4 +1023,102 @@ class CompletionTest {
|class Foo[A]{ self: Futu${m1} => }""".withSource
.completion(m1, expected)
}

@Test def backticks: Unit = {
val expected = Set(
("getClass", Method, "[X0 >: Foo.Bar.type](): Class[? <: X0]"),
("ensuring", Method, "(cond: Boolean): A"),
("##", Method, "=> Int"),
("nn", Method, "=> Foo.Bar.type"),
("==", Method, "(x$0: Any): Boolean"),
("ensuring", Method, "(cond: Boolean, msg: => Any): A"),
("ne", Method, "(x$0: Object): Boolean"),
("valueOf", Method, "($name: String): Foo.Bar"),
("equals", Method, "(x$0: Any): Boolean"),
("wait", Method, "(x$0: Long): Unit"),
("hashCode", Method, "(): Int"),
("notifyAll", Method, "(): Unit"),
("values", Method, "=> Array[Foo.Bar]"),
("", Method, "[B](y: B): (A, B)"),
("!=", Method, "(x$0: Any): Boolean"),
("fromOrdinal", Method, "(ordinal: Int): Foo.Bar"),
("asInstanceOf", Method, "[X0] => X0"),
("->", Method, "[B](y: B): (A, B)"),
("wait", Method, "(x$0: Long, x$1: Int): Unit"),
("`back-tick`", Field, "Foo.Bar"),
("notify", Method, "(): Unit"),
("formatted", Method, "(fmtstr: String): String"),
("ensuring", Method, "(cond: A => Boolean, msg: => Any): A"),
("wait", Method, "(): Unit"),
("isInstanceOf", Method, "[X0] => Boolean"),
("`match`", Field, "Foo.Bar"),
("toString", Method, "(): String"),
("ensuring", Method, "(cond: A => Boolean): A"),
("eq", Method, "(x$0: Object): Boolean"),
("synchronized", Method, "[X0](x$0: X0): X0")
)
code"""object Foo:
| enum Bar:
| case `back-tick`
| case `match`
|
| val x = Bar.${m1}"""
.withSource.completion(m1, expected)
}

@Test def backticksPrefix: Unit = {
val expected = Set(
("`back-tick`", Field, "Foo.Bar"),
)
code"""object Foo:
| enum Bar:
| case `back-tick`
| case `match`
|
| val x = Bar.`back${m1}"""
.withSource.completion(m1, expected)
}

@Test def backticksSpace: Unit = {
val expected = Set(
("`has space`", Field, "Foo.Bar"),
)
code"""object Foo:
| enum Bar:
| case `has space`
|
| val x = Bar.`has s${m1}"""
.withSource.completion(m1, expected)
}

@Test def backticksCompleteBoth: Unit = {
val expected = Set(
("formatted", Method, "(fmtstr: String): String"),
("`foo-bar`", Field, "Int"),
("foo", Field, "Int")
)
code"""object Foo:
| object Bar:
| val foo = 1
| val `foo-bar` = 2
| val `bar` = 3
|
| val x = Bar.fo${m1}"""
.withSource.completion(m1, expected)
}

@Test def backticksWhenNotNeeded: Unit = {
val expected = Set(
("`formatted`", Method, "(fmtstr: String): String"),
("`foo-bar`", Field, "Int"),
("`foo`", Field, "Int")
)
code"""object Foo:
| object Bar:
| val foo = 1
| val `foo-bar` = 2
|
| val x = Bar.`fo${m1}"""
.withSource.completion(m1, expected)
}
}

0 comments on commit 3c5dbc3

Please sign in to comment.