From f61c6c88e36475b2ce17ec44c7dcb795f8ab1049 Mon Sep 17 00:00:00 2001 From: Chris Kipp Date: Tue, 1 Mar 2022 15:52:21 +0100 Subject: [PATCH 1/3] fix(completions): add backticks when needed in completions This PR adds in the functionality to add in backticks when needed when giving back completions and also correctly returning completions when the user has already started typing a backtick. Fixes: #4406, #14006 --- .../tools/dotc/interactive/Completion.scala | 59 +++++++++++++++++-- .../src/dotty/tools/repl/JLineTerminal.scala | 11 +++- .../src/dotty/tools/repl/ReplDriver.scala | 3 + .../dotty/tools/repl/TabcompleteTests.scala | 58 ++++++++++++++++++ .../tools/languageserver/CompletionTest.scala | 55 +++++++++++++++++ 5 files changed, 179 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Completion.scala b/compiler/src/dotty/tools/dotc/interactive/Completion.scala index 1192bead245c..971dcd0c40dd 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Completion.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Completion.scala @@ -15,6 +15,9 @@ 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.Scanners +import dotty.tools.dotc.parsing.Tokens +import dotty.tools.dotc.util.Chars import dotty.tools.dotc.util.SourcePosition import scala.collection.mutable @@ -78,8 +81,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) @@ -88,13 +91,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 = @@ -105,7 +117,10 @@ 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 { @@ -120,16 +135,48 @@ object Completion { } val describedCompletions = describeCompletions(completions) + val backtickedCompletions = describedCompletions.map(backtickCompletions) + 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) = + if 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 apprach, 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. diff --git a/compiler/src/dotty/tools/repl/JLineTerminal.scala b/compiler/src/dotty/tools/repl/JLineTerminal.scala index 807ae2bf5eec..6e9a8497f094 100644 --- a/compiler/src/dotty/tools/repl/JLineTerminal.scala +++ b/compiler/src/dotty/tools/repl/JLineTerminal.scala @@ -118,6 +118,8 @@ final class JLineTerminal extends java.io.Closeable { def currentToken: TokenData /* | Null */ = { val source = SourceFile.virtual("", 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 @@ -126,7 +128,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 } diff --git a/compiler/src/dotty/tools/repl/ReplDriver.scala b/compiler/src/dotty/tools/repl/ReplDriver.scala index 72983ff18260..c7442686c840 100644 --- a/compiler/src/dotty/tools/repl/ReplDriver.scala +++ b/compiler/src/dotty/tools/repl/ReplDriver.scala @@ -35,6 +35,8 @@ import org.jline.reader._ import scala.annotation.tailrec import scala.collection.JavaConverters._ import scala.util.Using +import dotty.tools.dotc.util.Chars +import dotty.tools.dotc.parsing.Tokens /** The state of the REPL contains necessary bindings instead of having to have * mutation @@ -199,6 +201,7 @@ class ReplDriver(settings: Array[String], /** 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 diff --git a/compiler/test/dotty/tools/repl/TabcompleteTests.scala b/compiler/test/dotty/tools/repl/TabcompleteTests.scala index ab581d664629..8a9e481ae80c 100644 --- a/compiler/test/dotty/tools/repl/TabcompleteTests.scala +++ b/compiler/test/dotty/tools/repl/TabcompleteTests.scala @@ -131,4 +131,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)) + } + } diff --git a/language-server/test/dotty/tools/languageserver/CompletionTest.scala b/language-server/test/dotty/tools/languageserver/CompletionTest.scala index b9b0ca1f037f..5e7fdf906e6f 100644 --- a/language-server/test/dotty/tools/languageserver/CompletionTest.scala +++ b/language-server/test/dotty/tools/languageserver/CompletionTest.scala @@ -1023,4 +1023,59 @@ 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) + } } From a50e7ca4c08ef9938a66a7c5c1c7b57517266413 Mon Sep 17 00:00:00 2001 From: Chris Kipp Date: Wed, 2 Mar 2022 14:35:28 +0100 Subject: [PATCH 2/3] fix: ensure you still get completion when using backticks This commit addresses the situation where you are trying to use a backtick when it's not needed. For example if you have: ```scala object Foo: case `bar` case foo Foo.`b ``` You still expect to get a completion bar being backticked. This also ensure that if you try ``Foo.`fo`` you also still get a completion. --- .../tools/dotc/interactive/Completion.scala | 12 +++--- .../src/dotty/tools/repl/ReplDriver.scala | 2 - .../tools/languageserver/CompletionTest.scala | 43 +++++++++++++++++++ 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Completion.scala b/compiler/src/dotty/tools/dotc/interactive/Completion.scala index 971dcd0c40dd..d9ba9b3b1a05 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Completion.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Completion.scala @@ -15,7 +15,6 @@ 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.Scanners import dotty.tools.dotc.parsing.Tokens import dotty.tools.dotc.util.Chars import dotty.tools.dotc.util.SourcePosition @@ -118,6 +117,7 @@ object Completion { private def computeCompletions(pos: SourcePosition, path: List[Tree])(using Context): (Int, List[Completion]) = { val mode = completionMode(path, pos) val rawPrefix = completionPrefix(path, pos) + val hasBackTick = rawPrefix.headOption.contains('`') val prefix = if hasBackTick then rawPrefix.drop(1) else rawPrefix @@ -135,7 +135,8 @@ object Completion { } val describedCompletions = describeCompletions(completions) - val backtickedCompletions = describedCompletions.map(backtickCompletions) + val backtickedCompletions = + describedCompletions.map(completion => backtickCompletions(completion, hasBackTick)) val offset = completionOffset(path) @@ -147,14 +148,14 @@ object Completion { (offset, backtickedCompletions) } - def backtickCompletions(completion: Completion) = - if needsBacktick(completion.label) then + 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 apprach, but some of the utils that already exist in Dotty. + // 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) = @@ -429,6 +430,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) && diff --git a/compiler/src/dotty/tools/repl/ReplDriver.scala b/compiler/src/dotty/tools/repl/ReplDriver.scala index c7442686c840..b1c82d569a80 100644 --- a/compiler/src/dotty/tools/repl/ReplDriver.scala +++ b/compiler/src/dotty/tools/repl/ReplDriver.scala @@ -35,8 +35,6 @@ import org.jline.reader._ import scala.annotation.tailrec import scala.collection.JavaConverters._ import scala.util.Using -import dotty.tools.dotc.util.Chars -import dotty.tools.dotc.parsing.Tokens /** The state of the REPL contains necessary bindings instead of having to have * mutation diff --git a/language-server/test/dotty/tools/languageserver/CompletionTest.scala b/language-server/test/dotty/tools/languageserver/CompletionTest.scala index 5e7fdf906e6f..683432ae2a1c 100644 --- a/language-server/test/dotty/tools/languageserver/CompletionTest.scala +++ b/language-server/test/dotty/tools/languageserver/CompletionTest.scala @@ -1078,4 +1078,47 @@ class CompletionTest { | 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) + } } From 5e5866b3d910c83526f902c0c434787ccb4d9cf8 Mon Sep 17 00:00:00 2001 From: Chris Kipp Date: Wed, 2 Mar 2022 14:54:14 +0100 Subject: [PATCH 3/3] fix: don't show backticks in display on repl. This removes the backticks when you are seeing all the available options, but still includes them in the actual value of the completion. --- compiler/src/dotty/tools/repl/ReplDriver.scala | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/repl/ReplDriver.scala b/compiler/src/dotty/tools/repl/ReplDriver.scala index b1c82d569a80..ec361f2dad25 100644 --- a/compiler/src/dotty/tools/repl/ReplDriver.scala +++ b/compiler/src/dotty/tools/repl/ReplDriver.scala @@ -196,13 +196,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,