Skip to content

Don't merge code completion items having the same name. #12029

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 9, 2021
Merged

Don't merge code completion items having the same name. #12029

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 12 additions & 33 deletions compiler/src/dotty/tools/dotc/interactive/Completion.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,40 +131,18 @@ object Completion {

/**
* 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, the type denotations appear before term denotations inside
* the same `Completion`.
* If several denotations share the same name, each denotation will be transformed into a separate completion item.
*/
def describeCompletions(completions: CompletionMap)(using Context): List[Completion] = {
completions
.toList.groupBy(_._1.toTermName) // don't distinguish between names of terms and types
.toList.map { (name, namedDenots) =>
val denots = namedDenots.flatMap(_._2)
val typesFirst = denots.sortWith((d1, d2) => d1.isType && !d2.isType)
val desc = description(typesFirst)
Completion(name.show, desc, typesFirst.map(_.symbol))
}
}

/**
* A description for completion result that represents `symbols`.
*
* If `denots` contains a single denotation, show its full name in case it's a type, or its type if
* it's a term.
*
* When there are multiple denotations, show their kinds.
*/
def description(denots: List[SingleDenotation])(using Context): String =
denots match {
case denot :: Nil =>
if (denot.isType) denot.symbol.showFullName
else denot.info.widenTermRefExpr.show

case denot :: _ =>
denots.map(den => ctx.printer.kindString(den.symbol)).distinct.mkString("", " and ", s" ${denot.name.show}")
def describeCompletions(completions: CompletionMap)(using Context): List[Completion] =
for
(name, denots) <- completions.toList
denot <- denots
yield
Completion(name.show, description(denot), List(denot.symbol))

case Nil =>
""
}
def description(denot: SingleDenotation)(using Context): String =
if denot.isType then denot.symbol.showFullName
else denot.info.widenTermRefExpr.show

/** Computes code completions depending on the context in which completion is requested
* @param mode Should complete names of terms, types or both
Expand Down Expand Up @@ -195,8 +173,9 @@ object Completion {
var ctx = context

while ctx ne NoContext do
given Context = ctx
if ctx.isImportContext then
importedCompletions(using ctx).foreach { (name, denots) =>
importedCompletions.foreach { (name, denots) =>
addMapping(name, ScopedDenotations(denots, ctx))
}
else if ctx.owner.isClass then
Expand Down
9 changes: 4 additions & 5 deletions compiler/src/dotty/tools/repl/ReplDriver.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,10 @@ 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(completion: Completion) = {
val displ = completion.label
def makeCandidate(label: String) = {
new Candidate(
/* value = */ displ,
/* displ = */ displ, // displayed value
/* value = */ label,
/* displ = */ label, // displayed value
/* group = */ null, // can be used to group completions together
/* descr = */ null, // TODO use for documentation?
/* suffix = */ null,
Expand All @@ -201,7 +200,7 @@ class ReplDriver(settings: Array[String],
given Context = state.context.fresh.setCompilationUnit(unit)
val srcPos = SourcePosition(file, Span(cursor))
val (_, completions) = Completion.completions(srcPos)
completions.map(makeCandidate)
completions.map(_.label).distinct.map(makeCandidate)
}
.getOrElse(Nil)
}
Expand Down
13 changes: 5 additions & 8 deletions compiler/test/dotty/tools/dotc/interactive/CustomCompletion.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,11 @@ object CustomCompletion {
private type CompletionMap = Map[Name, Seq[SingleDenotation]]

private def describeCompletions(completions: CompletionMap)(using Context): List[Completion] = {
completions
.toList.groupBy(_._1.toTermName) // don't distinguish between names of terms and types
.toList.map { (name, namedDenots) =>
val denots = namedDenots.flatMap(_._2)
val typesFirst = denots.sortWith((d1, d2) => d1.isType && !d2.isType)
val desc = Completion.description(typesFirst)
Completion(label(name), desc, typesFirst.map(_.symbol))
}
for
(name, denots) <- completions.toList
denot <- denots
yield
Completion(label(name), Completion.description(denot), List(denot.symbol))
}

class DeepCompleter(mode: Completion.Mode, prefix: String, pos: SourcePosition) extends Completion.Completer(mode, prefix, pos):
Expand Down
79 changes: 61 additions & 18 deletions language-server/test/dotty/tools/languageserver/CompletionTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ class CompletionTest {
.completion(m1, Set(
("print", Method, "(x: Any): Unit"),
("printf", Method, "(text: String, xs: Any*): Unit"),
("println", Method, "method println")
("println", Method, "(x: Any): Unit"),
("println", Method, "(): Unit")
))
}

Expand All @@ -35,17 +36,22 @@ class CompletionTest {

@Test def completionFromScalaPackageObject: Unit = {
code"class Foo { val foo: BigD${m1} }".withSource
.completion(m1, Set(("BigDecimal", Field, "type and getter BigDecimal")))
.completion(m1, Set(("BigDecimal", Field, "scala.BigDecimal"),
("BigDecimal", Method, "=> math.BigDecimal.type")))
}

@Test def completionFromSyntheticPackageObject: Unit = {
code"class Foo { val foo: IArr${m1} }".withSource
.completion(m1, Set(("IArray", Field, "type and object IArray")))
.completion(m1, Set(("IArray", Field, "scala.IArray"),
("IArray", Module, "scala.IArray$package.IArray$")))
}

@Test def completionFromJavaDefaults: Unit = {
code"class Foo { val foo: Runn${m1} }".withSource
.completion(m1, Set(("Runnable", Class, "trait and object Runnable")))
.completion(m1, Set(
("Runnable", Class, "java.lang.Runnable"),
("Runnable", Module, "Runnable$")
))
}

@Test def completionWithImplicitConversion: Unit = {
Expand Down Expand Up @@ -125,7 +131,8 @@ class CompletionTest {
object Foo""",
code"""package pgk1
import pkg0.F${m1}"""
).completion(m1, Set(("Foo", Class, "class and object Foo")))
).completion(m1, Set(("Foo", Class, "pkg0.Foo"),
("Foo", Module, "pkg0.Foo$")))
}

@Test def importCompleteIncludePackage: Unit = {
Expand Down Expand Up @@ -157,7 +164,8 @@ class CompletionTest {

@Test def importJavaClass: Unit = {
code"""import java.io.FileDesc${m1}""".withSource
.completion(m1, Set(("FileDescriptor", Class, "class and object FileDescriptor")))
.completion(m1, Set(("FileDescriptor", Class, "java.io.FileDescriptor"),
("FileDescriptor", Module, "java.io.FileDescriptor$")))
}

@Test def importJavaStaticMethod: Unit = {
Expand Down Expand Up @@ -187,7 +195,8 @@ class CompletionTest {

@Test def importRename: Unit = {
code"""import java.io.{FileDesc${m1} => Foo}""".withSource
.completion(m1, Set(("FileDescriptor", Class, "class and object FileDescriptor")))
.completion(m1, Set(("FileDescriptor", Class, "java.io.FileDescriptor"),
("FileDescriptor", Module, "java.io.FileDescriptor$")))
}

@Test def importGivenByType: Unit = {
Expand Down Expand Up @@ -254,7 +263,8 @@ class CompletionTest {
@Test def completionOnRenamedImport: Unit = {
code"""import java.io.{FileDescriptor => AwesomeStuff}
trait Foo { val x: Awesom$m1 }""".withSource
.completion(m1, Set(("AwesomeStuff", Class, "class and object FileDescriptor")))
.completion(m1, Set(("AwesomeStuff", Class, "java.io.FileDescriptor"),
("AwesomeStuff", Module, "java.io.FileDescriptor$")))
}

@Test def completionOnRenamedImport2: Unit = {
Expand All @@ -263,7 +273,8 @@ class CompletionTest {
import java.io.{FileDescriptor => MyImportedSymbol}
val x: MyImp$m1
}""".withSource
.completion(m1, Set(("MyImportedSymbol", Class, "class and object FileDescriptor")))
.completion(m1, Set(("MyImportedSymbol", Class, "java.io.FileDescriptor"),
("MyImportedSymbol", Module, "java.io.FileDescriptor$")))
}

@Test def completionRenamedAndOriginalNames: Unit = {
Expand All @@ -272,8 +283,10 @@ class CompletionTest {
| import java.util.{HashMap => HashMap2}
| val x: Hash$m1
|}""".withSource
.completion(m1, Set(("HashMap", Class, "class and object HashMap"),
("HashMap2", Class, "class and object HashMap")))
.completion(m1, Set(("HashMap", Class, "java.util.HashMap"),
("HashMap", Module, "java.util.HashMap$"),
("HashMap2", Class, "java.util.HashMap"),
("HashMap2", Module, "java.util.HashMap$")))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be an orthogonal issue: for a module, it is more friendly to show java.util.HashMap instead of java.util.HashMap$.

It can be addressed in a different PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but that's because of how .show works by default. An even more enigmatic example is

code"class Foo { val foo: Runn${m1} }".withSource
      .completion(m1, Set(
        ("Runnable", Class, "java.lang.Runnable"),
        ("Runnable", Module, "Runnable$")
      ))

but that's definitely a separate issue

}

@Test def completionRenamedThrice: Unit = {
Expand All @@ -283,9 +296,12 @@ class CompletionTest {
| import java.util.{HashMap => MyHashMap3}
| val x: MyHash$m1
|}""".withSource
.completion(m1, Set(("MyHashMap", Class, "class and object HashMap"),
("MyHashMap2", Class, "class and object HashMap"),
("MyHashMap3", Class, "class and object HashMap")))
.completion(m1, Set(("MyHashMap", Class, "java.util.HashMap"),
("MyHashMap", Module, "java.util.HashMap$"),
("MyHashMap2", Class, "java.util.HashMap"),
("MyHashMap2", Module, "java.util.HashMap$"),
("MyHashMap3", Class, "java.util.HashMap"),
("MyHashMap3", Module, "java.util.HashMap$")))
}

@Test def completeFromWildcardImports: Unit = {
Expand Down Expand Up @@ -369,7 +385,8 @@ class CompletionTest {
|object Test extends Foo, Bar {
| val x = xx$m1
|}""".withSource
.completion(m1, Set(("xxxx", Method, "method xxxx"))) // 2 different signatures are merged into one generic description
.completion(m1, Set(("xxxx", Method, "(s: String): String"),
("xxxx", Method, "(i: Int): Int")))
}

@Test def dontCompleteFromAmbiguousImportsForEqualNestingLevels: Unit = {
Expand Down Expand Up @@ -482,7 +499,8 @@ class CompletionTest {
| def bar(i: Int) = 0
|}
|import Foo.b$m1""".withSource
.completion(m1, Set(("bar", Class, "class and method bar")))
.completion(m1, Set(("bar", Class, "Foo.bar"),
("bar", Method, "(i: Int): Int")))
}

@Test def completionTypeAndLazyValue: Unit = {
Expand All @@ -491,7 +509,8 @@ class CompletionTest {
| lazy val bar = 3
|}
|import Foo.b$m1""".withSource
.completion(m1, Set(("bar", Field, "type and lazy value bar")))
.completion(m1, Set(("bar", Field, "Foo.bar"),
("bar", Field, "Int")))
}

@Test def keepTrackOfTermsAndTypesSeparately: Unit = {
Expand All @@ -506,7 +525,8 @@ class CompletionTest {
| type ZZZZ = YY$m2
|}""".withSource
.completion(m1, Set(("YYYY", Field, "Int$")))
.completion(m2, Set(("YYYY", Field, "type and value YYYY")))
.completion(m2, Set(("YYYY", Field, "XXXX.YYYY"),
("YYYY", Field, "Int$")))
}

@Test def completeRespectingAccessModifiers: Unit = {
Expand Down Expand Up @@ -535,6 +555,29 @@ class CompletionTest {
.completion(m1, Set(("xxxx", Method, "(a: Int): Int")))
}

@Test def completePrimaryConstructorParameter: Unit = {
code"""class Foo(abc: Int) {
| ab$m1
| def method1: Int = {
| ab$m2
| 42
| }
| def method2: Int = {
| val smth = ab$m3
| 42
| }
|}""".withSource
.completion(m1, Set(("abc", Field, "Int")))
.completion(m2, Set(("abc", Field, "Int")))
.completion(m2, Set(("abc", Field, "Int")))
}

@Test def completeExtensionReceiver: Unit = {
code"""extension (string: String) def xxxx = str$m1"""
.withSource
.completion(m1, Set(("string", Field, "String")))
}

@Test def completeExtensionMethodWithoutParameter: Unit = {
code"""object Foo
|extension (foo: Foo.type) def xxxx = 1
Expand Down