From 915581216669ab13031db16a63eb1b8ebe795c14 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Tue, 27 Nov 2018 14:26:00 +0100 Subject: [PATCH 1/9] Fix #5508: Add `Completion` structure One `Completion` represents one completion results that should be included in the list of completion options. It can represent zero symbols (for instance, a wildcard import), or more than one (a class and its companion object). Each `Completion` also has a `description` which should be displayed in the tool that shows the completion candidates. Fixes #5508 --- .../tools/dotc/interactive/Completion.scala | 43 ++++++++++++++----- .../src/dotty/tools/repl/ReplDriver.scala | 4 +- .../languageserver/DottyLanguageServer.scala | 25 ++++++----- 3 files changed, 50 insertions(+), 22 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Completion.scala b/compiler/src/dotty/tools/dotc/interactive/Completion.scala index 5fa83ef229df..a0b0003842a1 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Completion.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Completion.scala @@ -20,6 +20,17 @@ import dotty.tools.dotc.util.{NoSourcePosition, SourcePosition} import scala.collection.mutable +/** + * One of the results of a completion query. + * + * @param label The label of this completion result, or the text that this completion result + * should insert in the scope where the completion request happened. + * @param description The description of this completion result: the fully qualified name for + * types, or the type for terms. + * @param symbols The symbols that are matched by this completion result. + */ +case class Completion(label: String, description: String, symbols: List[Symbol]) + object Completion { import dotty.tools.dotc.ast.tpd._ @@ -28,7 +39,7 @@ object Completion { * * @return offset and list of symbols for possible completions */ - def completions(pos: SourcePosition)(implicit ctx: Context): (Int, List[Symbol]) = { + def completions(pos: SourcePosition)(implicit ctx: Context): (Int, List[Completion]) = { val path = Interactive.pathTo(ctx.compilationUnit.tpdTree, pos.pos) computeCompletions(pos, path)(Interactive.contextOfPath(path)) } @@ -100,7 +111,7 @@ object Completion { new CompletionBuffer(mode, prefix, pos) } - private def computeCompletions(pos: SourcePosition, path: List[Tree])(implicit ctx: Context): (Int, List[Symbol]) = { + private def computeCompletions(pos: SourcePosition, path: List[Tree])(implicit ctx: Context): (Int, List[Completion]) = { val offset = completionOffset(path) val buffer = completionBuffer(path, pos) @@ -131,15 +142,27 @@ object Completion { /** * Return the list of symbols that shoudl be included in completion results. * - * If the mode is `Import` and several symbols share the same name, the type symbols are - * preferred over term symbols. + * If several symbols share the same name, the type symbols appear before term symbols inside + * the same `Completion`. + */ + def getCompletions(implicit ctx: Context): List[Completion] = { + val groupedSymbols = completions.toList.groupBy(_.name.stripModuleClassSuffix.toSimpleName).toList + groupedSymbols.map { case (name, symbols) => + val typesFirst = symbols.sortWith((s, _) => s.isType) + // Use distinct to remove duplicates with class, module class, etc. + val descriptions = typesFirst.map(description).distinct.mkString(", ") + Completion(name.toString, descriptions, typesFirst) + } + } + + /** + * A description for `sym`. + * + * For types, show the symbol's full name, or its type for term symbols. */ - def getCompletions(implicit ctx: Context): List[Symbol] = { - // Show only the type symbols when there are multiple options with the same name - completions.toList.groupBy(_.name.stripModuleClassSuffix.toSimpleName).mapValues { - case sym :: Nil => sym :: Nil - case syms => syms.filter(_.isType) - }.values.flatten.toList + private def description(sym: Symbol)(implicit ctx: Context): String = { + if (sym.isType) sym.showFullName + else sym.info.widenTermRefExpr.show } /** diff --git a/compiler/src/dotty/tools/repl/ReplDriver.scala b/compiler/src/dotty/tools/repl/ReplDriver.scala index 1c219639201b..dd15c917d604 100644 --- a/compiler/src/dotty/tools/repl/ReplDriver.scala +++ b/compiler/src/dotty/tools/repl/ReplDriver.scala @@ -149,8 +149,8 @@ class ReplDriver(settings: Array[String], /** Extract possible completions at the index of `cursor` in `expr` */ protected[this] final def completions(cursor: Int, expr: String, state0: State): List[Candidate] = { - def makeCandidate(completion: Symbol)(implicit ctx: Context) = { - val displ = completion.name.toString + def makeCandidate(completion: Completion)(implicit ctx: Context) = { + val displ = completion.label new Candidate( /* value = */ displ, /* displ = */ displ, // displayed value diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index dfcd5c7cf2cf..8e4e80bd2e52 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -780,8 +780,8 @@ object DottyLanguageServer { symbol.owner == ctx.definitions.EmptyPackageClass } - /** Create an lsp4j.CompletionItem from a Symbol */ - def completionItem(sym: Symbol)(implicit ctx: Context): lsp4j.CompletionItem = { + /** Create an lsp4j.CompletionItem from a completion result */ + def completionItem(completion: Completion)(implicit ctx: Context): lsp4j.CompletionItem = { def completionItemKind(sym: Symbol)(implicit ctx: Context): lsp4j.CompletionItemKind = { import lsp4j.{CompletionItemKind => CIK} @@ -799,15 +799,20 @@ object DottyLanguageServer { CIK.Field } - val label = sym.name.show - val item = new lsp4j.CompletionItem(label) - val detail = if (sym.isType) sym.showFullName else sym.info.widenTermRefExpr.show - item.setDetail(detail) - ParsedComment.docOf(sym).foreach { doc => - item.setDocumentation(markupContent(doc.renderAsMarkdown)) + val item = new lsp4j.CompletionItem(completion.label) + item.setDetail(completion.description) + + val documentation = for { + sym <- completion.symbols + doc <- ParsedComment.docOf(sym) + } yield doc + + if (documentation.nonEmpty) { + item.setDocumentation(hoverContent(None, documentation)) } - item.setDeprecated(sym.isDeprecated) - item.setKind(completionItemKind(sym)) + + item.setDeprecated(completion.symbols.forall(_.isDeprecated)) + completion.symbols.headOption.foreach(s => item.setKind(completionItemKind(s))) item } From 317f16cf888650c3b687cbf9b04d05a22a68f0a9 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Tue, 27 Nov 2018 16:23:43 +0100 Subject: [PATCH 2/9] Fix #3979: Completion for renamed imports We introduce a new `RenameAwareScope` that tracks the name associated with a symbol in the scope. Calling `toListWithNames` returns the list of symbols in this scope along with the name that should be used to refer to the symbol in this scope. Fixes #3979 --- .../tools/dotc/interactive/Completion.scala | 73 +++++++++++++------ .../dotty/tools/repl/TabcompleteTests.scala | 10 +++ .../tools/languageserver/CompletionTest.scala | 15 ++++ 3 files changed, 74 insertions(+), 24 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Completion.scala b/compiler/src/dotty/tools/dotc/interactive/Completion.scala index a0b0003842a1..22b976e659ac 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Completion.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Completion.scala @@ -137,7 +137,7 @@ object Completion { private class CompletionBuffer(val mode: Mode, val prefix: String, pos: SourcePosition) { - private[this] val completions = Scopes.newScope.openForMutations + private[this] val completions = new RenameAwareScope /** * Return the list of symbols that shoudl be included in completion results. @@ -146,7 +146,11 @@ object Completion { * the same `Completion`. */ def getCompletions(implicit ctx: Context): List[Completion] = { - val groupedSymbols = completions.toList.groupBy(_.name.stripModuleClassSuffix.toSimpleName).toList + val groupedSymbols = { + val symbols = completions.toListWithNames + val nameToSymbols = symbols.groupBy(_._2.stripModuleClassSuffix.toSimpleName) + nameToSymbols.mapValues(_.map(_._1)).toList + } groupedSymbols.map { case (name, symbols) => val typesFirst = symbols.sortWith((s, _) => s.isType) // Use distinct to remove duplicates with class, module class, etc. @@ -173,11 +177,11 @@ object Completion { if (ctx.owner.isClass) { addAccessibleMembers(ctx.owner.thisType) ctx.owner.asClass.classInfo.selfInfo match { - case selfSym: Symbol => add(selfSym) + case selfSym: Symbol => add(selfSym, selfSym.name) case _ => } } - else if (ctx.scope != null) ctx.scope.foreach(add) + else if (ctx.scope != null) ctx.scope.foreach(s => add(s, s.name)) addImportCompletions @@ -208,15 +212,16 @@ object Completion { * If `sym` exists, no symbol with the same name is already included, and it satisfies the * inclusion filter, then add it to the completions. */ - private def add(sym: Symbol)(implicit ctx: Context) = - if (sym.exists && !completions.lookup(sym.name).exists && include(sym)) { - completions.enter(sym) + private def add(sym: Symbol, nameInScope: Name)(implicit ctx: Context) = + if (sym.exists && !completions.lookup(nameInScope).exists && include(sym, nameInScope)) { + completions.enter(sym, nameInScope) } /** Lookup members `name` from `site`, and try to add them to the completion list. */ - private def addMember(site: Type, name: Name)(implicit ctx: Context) = - if (!completions.lookup(name).exists) - for (alt <- site.member(name).alternatives) add(alt.symbol) + private def addMember(site: Type, name: Name, nameInScope: Name)(implicit ctx: Context) = + if (!completions.lookup(nameInScope).exists) { + for (alt <- site.member(name).alternatives) add(alt.symbol, nameInScope) + } /** Include in completion sets only symbols that * 1. start with given name prefix, and @@ -230,9 +235,9 @@ object Completion { * as completion results. However, if a user explicitly writes all '$' characters in an * identifier, we should complete the rest. */ - private def include(sym: Symbol)(implicit ctx: Context): Boolean = - sym.name.startsWith(prefix) && - !sym.name.toString.drop(prefix.length).contains('$') && + private def include(sym: Symbol, nameInScope: Name)(implicit ctx: Context): Boolean = + nameInScope.startsWith(prefix) && + !nameInScope.toString.drop(prefix.length).contains('$') && !sym.isPrimaryConstructor && (!sym.is(Package) || !sym.moduleClass.exists) && !sym.is(allOf(Mutable, Accessor)) && @@ -249,20 +254,20 @@ object Completion { */ private def accessibleMembers(site: Type)(implicit ctx: Context): Seq[Symbol] = site match { case site: NamedType if site.symbol.is(Package) => - site.decls.toList.filter(include) // Don't look inside package members -- it's too expensive. + site.decls.toList.filter(sym => include(sym, sym.name)) // Don't look inside package members -- it's too expensive. case _ => def appendMemberSyms(name: Name, buf: mutable.Buffer[SingleDenotation]): Unit = try buf ++= site.member(name).alternatives catch { case ex: TypeError => } site.memberDenots(takeAllFilter, appendMemberSyms).collect { - case mbr if include(mbr.symbol) => mbr.accessibleFrom(site, superAccess = true).symbol + case mbr if include(mbr.symbol, mbr.symbol.name) => mbr.accessibleFrom(site, superAccess = true).symbol case _ => NoSymbol }.filter(_.exists) } /** Add all the accessible members of `site` in `info`. */ private def addAccessibleMembers(site: Type)(implicit ctx: Context): Unit = - for (mbr <- accessibleMembers(site)) addMember(site, mbr.name) + for (mbr <- accessibleMembers(site)) addMember(site, mbr.name, mbr.name) /** * Add in `info` the symbols that are imported by `ctx.importInfo`. If this is a wildcard import, @@ -271,17 +276,18 @@ object Completion { private def addImportCompletions(implicit ctx: Context): Unit = { val imp = ctx.importInfo if (imp != null) { - def addImport(name: TermName) = { - addMember(imp.site, name) - addMember(imp.site, name.toTypeName) + def addImport(name: TermName, nameInScope: TermName) = { + addMember(imp.site, name, nameInScope) + addMember(imp.site, name.toTypeName, nameInScope.toTypeName) + } + imp.reverseMapping.foreachBinding { (nameInScope, original) => + if (original != nameInScope || !imp.excluded.contains(original)) { + addImport(original, nameInScope) + } } - // FIXME: We need to also take renamed items into account for completions, - // That means we have to return list of a pairs (Name, Symbol) instead of a list - // of symbols from `completions`.!= - for (imported <- imp.originals if !imp.excluded.contains(imported)) addImport(imported) if (imp.isWildcardImport) for (mbr <- accessibleMembers(imp.site) if !imp.excluded.contains(mbr.name.toTermName)) - addMember(imp.site, mbr.name) + addMember(imp.site, mbr.name, mbr.name) } } @@ -324,4 +330,23 @@ object Completion { val Import: Mode = new Mode(4) | Term | Type } + /** A scope that tracks renames of the entered symbols. + * Useful for providing completions for renamed symbols + * in the REPL and the IDE. + */ + private class RenameAwareScope extends Scopes.MutableScope { + private[this] val renames: mutable.Map[Symbol, Name] = mutable.Map.empty + + /** Enter the symbol `sym` in this scope, recording a potential renaming. */ + def enter[T <: Symbol](sym: T, name: Name)(implicit ctx: Context): T = { + if (name != sym.name) renames += sym -> name + newScopeEntry(name, sym) + sym + } + + /** Lists the symbols in this scope along with the name associated with them. */ + def toListWithNames(implicit ctx: Context): List[(Symbol, Name)] = + toList.map(sym => (sym, renames.get(sym).getOrElse(sym.name))) + } + } diff --git a/compiler/test/dotty/tools/repl/TabcompleteTests.scala b/compiler/test/dotty/tools/repl/TabcompleteTests.scala index 75f28c15a213..20b3552cdaef 100644 --- a/compiler/test/dotty/tools/repl/TabcompleteTests.scala +++ b/compiler/test/dotty/tools/repl/TabcompleteTests.scala @@ -80,4 +80,14 @@ class TabcompleteTests extends ReplTest { val expected = List("FileDescriptor") assertEquals(expected, tabComplete("val foo: FileDesc")) } + + @Test def tabCompleteRenamedImport = + fromInitialState { implicit state => + val src = "import java.io.{FileDescriptor => Renamed}" + run(src) + } + .andThen { implicit state => + val expected = List("Renamed") + assertEquals(expected, tabComplete("val foo: Rena")) + } } diff --git a/language-server/test/dotty/tools/languageserver/CompletionTest.scala b/language-server/test/dotty/tools/languageserver/CompletionTest.scala index 6a4dc94c8d5d..4e55810a02f8 100644 --- a/language-server/test/dotty/tools/languageserver/CompletionTest.scala +++ b/language-server/test/dotty/tools/languageserver/CompletionTest.scala @@ -199,4 +199,19 @@ class CompletionTest { |}""".withSource .completion(m1, Set(("bar", Field, "Bar"), ("bat", Module, "Foo.bat"))) } + + @Test def completionOnRenamedImport: Unit = { + code"""import java.io.{FileDescriptor => AwesomeStuff} + trait Foo { val x: Awesom$m1 }""".withSource + .completion(m1, Set(("AwesomeStuff", Class, "java.io.FileDescriptor"))) + } + + @Test def completionOnRenamedImport2: Unit = { + code"""import java.util.{HashMap => MyImportedSymbol} + trait Foo { + import java.io.{FileDescriptor => MyImportedSymbol} + val x: MyImp$m1 + }""".withSource + .completion(m1, Set(("MyImportedSymbol", Class, "java.io.FileDescriptor"))) + } } From 55c55cc48a72d071f393a5fbbc557845c2cc0a48 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 28 Nov 2018 09:02:51 +0100 Subject: [PATCH 3/9] Support multiple renames for the same symbol --- .../tools/dotc/interactive/Completion.scala | 12 ++++++---- .../tools/languageserver/CompletionTest.scala | 22 +++++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Completion.scala b/compiler/src/dotty/tools/dotc/interactive/Completion.scala index 22b976e659ac..798674cbb8f5 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Completion.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Completion.scala @@ -335,18 +335,22 @@ object Completion { * in the REPL and the IDE. */ private class RenameAwareScope extends Scopes.MutableScope { - private[this] val renames: mutable.Map[Symbol, Name] = mutable.Map.empty + private[this] val renames: mutable.Map[Symbol, List[Name]] = mutable.Map.empty /** Enter the symbol `sym` in this scope, recording a potential renaming. */ def enter[T <: Symbol](sym: T, name: Name)(implicit ctx: Context): T = { - if (name != sym.name) renames += sym -> name + renames += sym -> (name :: renames.getOrElse(sym, Nil)) newScopeEntry(name, sym) sym } /** Lists the symbols in this scope along with the name associated with them. */ - def toListWithNames(implicit ctx: Context): List[(Symbol, Name)] = - toList.map(sym => (sym, renames.get(sym).getOrElse(sym.name))) + def toListWithNames(implicit ctx: Context): List[(Symbol, Name)] = { + for { + sym <- toList + name <- renames.getOrElse(sym, List(sym.name)) + } yield (sym, name) + } } } diff --git a/language-server/test/dotty/tools/languageserver/CompletionTest.scala b/language-server/test/dotty/tools/languageserver/CompletionTest.scala index 4e55810a02f8..4ace0b6cbace 100644 --- a/language-server/test/dotty/tools/languageserver/CompletionTest.scala +++ b/language-server/test/dotty/tools/languageserver/CompletionTest.scala @@ -214,4 +214,26 @@ class CompletionTest { }""".withSource .completion(m1, Set(("MyImportedSymbol", Class, "java.io.FileDescriptor"))) } + + @Test def completionRenamedAndOriginalNames: Unit = { + code"""import java.util.HashMap + |trait Foo { + | import java.util.{HashMap => HashMap2} + | val x: Hash$m1 + |}""".withSource + .completion(m1, Set(("HashMap", Class, "java.util.HashMap"), + ("HashMap2", Class, "java.util.HashMap"))) + } + + @Test def completionRenamedThrice: Unit = { + code"""import java.util.{HashMap => MyHashMap} + |import java.util.{HashMap => MyHashMap2} + |trait Foo { + | import java.util.{HashMap => MyHashMap3} + | val x: MyHash$m1 + |}""".withSource + .completion(m1, Set(("MyHashMap", Class, "java.util.HashMap"), + ("MyHashMap2", Class, "java.util.HashMap"), + ("MyHashMap3", Class, "java.util.HashMap"))) + } } From 21b0b9711143b42a8405beb52d0fe6ff6210e975 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 28 Nov 2018 09:27:20 +0100 Subject: [PATCH 4/9] Exclude symbols w/o sourceSymbol in completions For instance, Scala classes have a synthetic accompanying `Module` which doesn't have a corresponding source symbol. We don't want to show the `Module` in the completions. --- .../src/dotty/tools/dotc/interactive/Completion.scala | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Completion.scala b/compiler/src/dotty/tools/dotc/interactive/Completion.scala index 798674cbb8f5..f2a5c10b236b 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Completion.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Completion.scala @@ -227,9 +227,10 @@ object Completion { * 1. start with given name prefix, and * 2. do not contain '$' except in prefix where it is explicitly written by user, and * 3. are not a primary constructor, - * 4. are the module class in case of packages, - * 5. are mutable accessors, to exclude setters for `var`, - * 6. have same term/type kind as name prefix given so far + * 4. have an existing source symbol, + * 5. are the module class in case of packages, + * 6. are mutable accessors, to exclude setters for `var`, + * 7. have same term/type kind as name prefix given so far * * The reason for (2) is that we do not want to present compiler-synthesized identifiers * as completion results. However, if a user explicitly writes all '$' characters in an @@ -239,6 +240,7 @@ object Completion { nameInScope.startsWith(prefix) && !nameInScope.toString.drop(prefix.length).contains('$') && !sym.isPrimaryConstructor && + sym.sourceSymbol.exists && (!sym.is(Package) || !sym.moduleClass.exists) && !sym.is(allOf(Mutable, Accessor)) && ( From fa7206748474b91432a1d4332ee1fb7b024cafae Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 28 Nov 2018 09:34:38 +0100 Subject: [PATCH 5/9] Better completion description for multiple symbols When multiple symbols are concerned by one completion result, we show the different kinds of symbols. --- .../tools/dotc/interactive/Completion.scala | 35 ++++++++++++----- .../tools/languageserver/CompletionTest.scala | 38 ++++++++++++++----- 2 files changed, 53 insertions(+), 20 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Completion.scala b/compiler/src/dotty/tools/dotc/interactive/Completion.scala index f2a5c10b236b..73f869b52d32 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Completion.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Completion.scala @@ -149,24 +149,39 @@ object Completion { val groupedSymbols = { val symbols = completions.toListWithNames val nameToSymbols = symbols.groupBy(_._2.stripModuleClassSuffix.toSimpleName) - nameToSymbols.mapValues(_.map(_._1)).toList + nameToSymbols.mapValues { symbols => + symbols + .map(_._1) + .distinct // Show symbols that have been renamed multiple times only once + }.toList } groupedSymbols.map { case (name, symbols) => - val typesFirst = symbols.sortWith((s, _) => s.isType) - // Use distinct to remove duplicates with class, module class, etc. - val descriptions = typesFirst.map(description).distinct.mkString(", ") - Completion(name.toString, descriptions, typesFirst) + val typesFirst = symbols.sortWith((s1, s2) => s1.isType && !s2.isType) + val desc = description(typesFirst) + Completion(name.toString, desc, typesFirst) } } /** - * A description for `sym`. + * A description for completion result that represents `symbols`. * - * For types, show the symbol's full name, or its type for term symbols. + * If `symbols` contains a single symbol, show its full name in case it's a type, or its type if + * it's a term. + * + * When there are multiple symbols, show their kinds. */ - private def description(sym: Symbol)(implicit ctx: Context): String = { - if (sym.isType) sym.showFullName - else sym.info.widenTermRefExpr.show + private def description(symbols: List[Symbol])(implicit ctx: Context): String = { + symbols match { + case sym :: Nil => + if (sym.isType) sym.showFullName + else sym.info.widenTermRefExpr.show + + case sym :: _ => + symbols.map(ctx.printer.kindString).mkString("", " and ", s" ${sym.name.show}") + + case Nil => + "" + } } /** diff --git a/language-server/test/dotty/tools/languageserver/CompletionTest.scala b/language-server/test/dotty/tools/languageserver/CompletionTest.scala index 4ace0b6cbace..39b37ddfeb19 100644 --- a/language-server/test/dotty/tools/languageserver/CompletionTest.scala +++ b/language-server/test/dotty/tools/languageserver/CompletionTest.scala @@ -89,7 +89,7 @@ class CompletionTest { object Foo""", code"""package pgk1 import pkg0.F${m1}""" - ).completion(m1, Set(("Foo", Class, "pkg0.Foo"))) + ).completion(m1, Set(("Foo", Class, "class and object Foo"))) } @Test def importCompleteIncludePackage: Unit = { @@ -121,7 +121,7 @@ class CompletionTest { @Test def importJavaClass: Unit = { code"""import java.io.FileDesc${m1}""".withSource - .completion(m1, Set(("FileDescriptor", Class, "java.io.FileDescriptor"))) + .completion(m1, Set(("FileDescriptor", Class, "class and object FileDescriptor"))) } @Test def importJavaStaticMethod: Unit = { @@ -143,7 +143,7 @@ class CompletionTest { @Test def importRename: Unit = { code"""import java.io.{FileDesc${m1} => Foo}""".withSource - .completion(m1, Set(("FileDescriptor", Class, "java.io.FileDescriptor"))) + .completion(m1, Set(("FileDescriptor", Class, "class and object FileDescriptor"))) } @Test def markDeprecatedSymbols: Unit = { @@ -203,7 +203,7 @@ class CompletionTest { @Test def completionOnRenamedImport: Unit = { code"""import java.io.{FileDescriptor => AwesomeStuff} trait Foo { val x: Awesom$m1 }""".withSource - .completion(m1, Set(("AwesomeStuff", Class, "java.io.FileDescriptor"))) + .completion(m1, Set(("AwesomeStuff", Class, "class and object FileDescriptor"))) } @Test def completionOnRenamedImport2: Unit = { @@ -212,7 +212,7 @@ class CompletionTest { import java.io.{FileDescriptor => MyImportedSymbol} val x: MyImp$m1 }""".withSource - .completion(m1, Set(("MyImportedSymbol", Class, "java.io.FileDescriptor"))) + .completion(m1, Set(("MyImportedSymbol", Class, "class and object FileDescriptor"))) } @Test def completionRenamedAndOriginalNames: Unit = { @@ -221,8 +221,8 @@ class CompletionTest { | import java.util.{HashMap => HashMap2} | val x: Hash$m1 |}""".withSource - .completion(m1, Set(("HashMap", Class, "java.util.HashMap"), - ("HashMap2", Class, "java.util.HashMap"))) + .completion(m1, Set(("HashMap", Class, "class and object HashMap"), + ("HashMap2", Class, "class and object HashMap"))) } @Test def completionRenamedThrice: Unit = { @@ -232,8 +232,26 @@ class CompletionTest { | import java.util.{HashMap => MyHashMap3} | val x: MyHash$m1 |}""".withSource - .completion(m1, Set(("MyHashMap", Class, "java.util.HashMap"), - ("MyHashMap2", Class, "java.util.HashMap"), - ("MyHashMap3", Class, "java.util.HashMap"))) + .completion(m1, Set(("MyHashMap", Class, "class and object HashMap"), + ("MyHashMap2", Class, "class and object HashMap"), + ("MyHashMap3", Class, "class and object HashMap"))) + } + + @Test def completionClassAndMethod: Unit = { + code"""object Foo { + | class bar + | def bar = 0 + |} + |import Foo.b$m1""".withSource + .completion(m1, Set(("bar", Class, "class and method bar"))) + } + + @Test def completionTypeAndLazyValue: Unit = { + code"""object Foo { + | type bar = Int + | lazy val bar = 3 + |} + |import Foo.b$m1""".withSource + .completion(m1, Set(("bar", Field, "type and lazy value bar"))) } } From 6ed0ccd64a46fbc9b48f1939235912ae1faf4269 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 28 Nov 2018 09:39:14 +0100 Subject: [PATCH 6/9] Handle symbols renamed multiple times better --- .../dotty/tools/dotc/interactive/Completion.scala | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Completion.scala b/compiler/src/dotty/tools/dotc/interactive/Completion.scala index 73f869b52d32..bc12798b7829 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Completion.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Completion.scala @@ -149,11 +149,7 @@ object Completion { val groupedSymbols = { val symbols = completions.toListWithNames val nameToSymbols = symbols.groupBy(_._2.stripModuleClassSuffix.toSimpleName) - nameToSymbols.mapValues { symbols => - symbols - .map(_._1) - .distinct // Show symbols that have been renamed multiple times only once - }.toList + nameToSymbols.mapValues(_.map(_._1)).toList } groupedSymbols.map { case (name, symbols) => val typesFirst = symbols.sortWith((s1, s2) => s1.isType && !s2.isType) @@ -352,11 +348,11 @@ object Completion { * in the REPL and the IDE. */ private class RenameAwareScope extends Scopes.MutableScope { - private[this] val renames: mutable.Map[Symbol, List[Name]] = mutable.Map.empty + private[this] val nameToSymbols: mutable.Map[Name, List[Symbol]] = mutable.Map.empty /** Enter the symbol `sym` in this scope, recording a potential renaming. */ def enter[T <: Symbol](sym: T, name: Name)(implicit ctx: Context): T = { - renames += sym -> (name :: renames.getOrElse(sym, Nil)) + nameToSymbols += name -> (sym :: nameToSymbols.getOrElse(name, Nil)) newScopeEntry(name, sym) sym } @@ -364,8 +360,8 @@ object Completion { /** Lists the symbols in this scope along with the name associated with them. */ def toListWithNames(implicit ctx: Context): List[(Symbol, Name)] = { for { - sym <- toList - name <- renames.getOrElse(sym, List(sym.name)) + (name, syms) <- nameToSymbols.toList + sym <- syms } yield (sym, name) } } From f9443e1b61bf7f86b3659a29b58acc13302199fd Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Fri, 30 Nov 2018 08:48:06 +0100 Subject: [PATCH 7/9] Return syms grouped by name in `RenameAwareScope` --- .../tools/dotc/interactive/Completion.scala | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Completion.scala b/compiler/src/dotty/tools/dotc/interactive/Completion.scala index bc12798b7829..63d6b9b1385b 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Completion.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Completion.scala @@ -7,7 +7,7 @@ import dotty.tools.dotc.core.CheckRealizable import dotty.tools.dotc.core.Decorators.StringInterpolators import dotty.tools.dotc.core.Denotations.SingleDenotation import dotty.tools.dotc.core.Flags._ -import dotty.tools.dotc.core.Names.{Name, TermName} +import dotty.tools.dotc.core.Names.{Name, SimpleName, TermName} import dotty.tools.dotc.core.NameKinds.SimpleNameKind import dotty.tools.dotc.core.NameOps.NameDecorator import dotty.tools.dotc.core.Symbols.{defn, NoSymbol, Symbol} @@ -146,12 +146,8 @@ object Completion { * the same `Completion`. */ def getCompletions(implicit ctx: Context): List[Completion] = { - val groupedSymbols = { - val symbols = completions.toListWithNames - val nameToSymbols = symbols.groupBy(_._2.stripModuleClassSuffix.toSimpleName) - nameToSymbols.mapValues(_.map(_._1)).toList - } - groupedSymbols.map { case (name, symbols) => + val nameToSymbols = completions.mappings.toList + nameToSymbols.map { case (name, symbols) => val typesFirst = symbols.sortWith((s1, s2) => s1.isType && !s2.isType) val desc = description(typesFirst) Completion(name.toString, desc, typesFirst) @@ -357,12 +353,16 @@ object Completion { sym } - /** Lists the symbols in this scope along with the name associated with them. */ - def toListWithNames(implicit ctx: Context): List[(Symbol, Name)] = { - for { - (name, syms) <- nameToSymbols.toList - sym <- syms - } yield (sym, name) + /** Get the names that are known in this scope, along with the list of symbols they refer to. */ + def mappings(implicit ctx: Context): Map[SimpleName, List[Symbol]] = { + val symbols = + for { + (name, syms) <- nameToSymbols.toList + sym <- syms + } yield (sym, name) + symbols + .groupBy(_._2.stripModuleClassSuffix.toSimpleName) + .mapValues(_.map(_._1)) } } From 976eecb6b21d20942954861285bd31628343fb66 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Sat, 1 Dec 2018 14:16:26 +0100 Subject: [PATCH 8/9] Use `NameFilter` in `Completion` --- .../dotty/tools/dotc/core/Definitions.scala | 1 + .../tools/dotc/interactive/Completion.scala | 26 ++++++++++++------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 37d5ac8ee4cc..8078b5da6e24 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -285,6 +285,7 @@ class Definitions { // technique to do that. Here we need to set it before completing // attempt to load Object's classfile, which causes issue #1648. val companion = JavaLangPackageVal.info.decl(nme.Object).symbol + companion.moduleClass.info = NoType // to indicate that it does not really exist companion.info = NoType // to indicate that it does not really exist completeClass(cls) diff --git a/compiler/src/dotty/tools/dotc/interactive/Completion.scala b/compiler/src/dotty/tools/dotc/interactive/Completion.scala index 63d6b9b1385b..d4f5341b81f6 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Completion.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Completion.scala @@ -14,7 +14,7 @@ import dotty.tools.dotc.core.Symbols.{defn, NoSymbol, Symbol} import dotty.tools.dotc.core.Scopes import dotty.tools.dotc.core.StdNames.{nme, tpnme} import dotty.tools.dotc.core.TypeError -import dotty.tools.dotc.core.Types.{NamedType, Type, takeAllFilter} +import dotty.tools.dotc.core.Types.{NameFilter, NamedType, Type, NoType} import dotty.tools.dotc.printing.Texts._ import dotty.tools.dotc.util.{NoSourcePosition, SourcePosition} @@ -220,7 +220,10 @@ object Completion { * inclusion filter, then add it to the completions. */ private def add(sym: Symbol, nameInScope: Name)(implicit ctx: Context) = - if (sym.exists && !completions.lookup(nameInScope).exists && include(sym, nameInScope)) { + if (sym.exists && + completionsFilter(NoType, nameInScope) && + !completions.lookup(nameInScope).exists && + include(sym, nameInScope)) { completions.enter(sym, nameInScope) } @@ -232,20 +235,16 @@ object Completion { /** Include in completion sets only symbols that * 1. start with given name prefix, and - * 2. do not contain '$' except in prefix where it is explicitly written by user, and + * 2. is not absent (info is not NoType) * 3. are not a primary constructor, * 4. have an existing source symbol, * 5. are the module class in case of packages, * 6. are mutable accessors, to exclude setters for `var`, * 7. have same term/type kind as name prefix given so far - * - * The reason for (2) is that we do not want to present compiler-synthesized identifiers - * as completion results. However, if a user explicitly writes all '$' characters in an - * identifier, we should complete the rest. */ private def include(sym: Symbol, nameInScope: Name)(implicit ctx: Context): Boolean = nameInScope.startsWith(prefix) && - !nameInScope.toString.drop(prefix.length).contains('$') && + !sym.isAbsent && !sym.isPrimaryConstructor && sym.sourceSymbol.exists && (!sym.is(Package) || !sym.moduleClass.exists) && @@ -263,12 +262,13 @@ object Completion { */ private def accessibleMembers(site: Type)(implicit ctx: Context): Seq[Symbol] = site match { case site: NamedType if site.symbol.is(Package) => - site.decls.toList.filter(sym => include(sym, sym.name)) // Don't look inside package members -- it's too expensive. + // Don't look inside package members -- it's too expensive. + site.decls.toList.filter(sym => sym.isAccessibleFrom(site, superAccess = false)) case _ => def appendMemberSyms(name: Name, buf: mutable.Buffer[SingleDenotation]): Unit = try buf ++= site.member(name).alternatives catch { case ex: TypeError => } - site.memberDenots(takeAllFilter, appendMemberSyms).collect { + site.memberDenots(completionsFilter, appendMemberSyms).collect { case mbr if include(mbr.symbol, mbr.symbol.name) => mbr.accessibleFrom(site, superAccess = true).symbol case _ => NoSymbol }.filter(_.exists) @@ -315,6 +315,12 @@ object Completion { targets } + /** Filter for names that should appear when looking for completions. */ + private[this] object completionsFilter extends NameFilter { + def apply(pre: Type, name: Name)(implicit ctx: Context): Boolean = + !name.isConstructorName && name.toTermName.info.kind == SimpleNameKind + } + } /** From 29fe495c10d8b6f4472907713d2e602ba3ec569f Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Sat, 1 Dec 2018 15:29:17 +0100 Subject: [PATCH 9/9] Simplify `def mappings` --- .../tools/dotc/interactive/Completion.scala | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Completion.scala b/compiler/src/dotty/tools/dotc/interactive/Completion.scala index d4f5341b81f6..d740d4098e3e 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Completion.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Completion.scala @@ -7,7 +7,7 @@ import dotty.tools.dotc.core.CheckRealizable import dotty.tools.dotc.core.Decorators.StringInterpolators import dotty.tools.dotc.core.Denotations.SingleDenotation import dotty.tools.dotc.core.Flags._ -import dotty.tools.dotc.core.Names.{Name, SimpleName, TermName} +import dotty.tools.dotc.core.Names.{Name, TermName} import dotty.tools.dotc.core.NameKinds.SimpleNameKind import dotty.tools.dotc.core.NameOps.NameDecorator import dotty.tools.dotc.core.Symbols.{defn, NoSymbol, Symbol} @@ -350,26 +350,18 @@ object Completion { * in the REPL and the IDE. */ private class RenameAwareScope extends Scopes.MutableScope { - private[this] val nameToSymbols: mutable.Map[Name, List[Symbol]] = mutable.Map.empty + private[this] val nameToSymbols: mutable.Map[TermName, List[Symbol]] = mutable.Map.empty /** Enter the symbol `sym` in this scope, recording a potential renaming. */ def enter[T <: Symbol](sym: T, name: Name)(implicit ctx: Context): T = { - nameToSymbols += name -> (sym :: nameToSymbols.getOrElse(name, Nil)) + val termName = name.stripModuleClassSuffix.toTermName + nameToSymbols += termName -> (sym :: nameToSymbols.getOrElse(termName, Nil)) newScopeEntry(name, sym) sym } /** Get the names that are known in this scope, along with the list of symbols they refer to. */ - def mappings(implicit ctx: Context): Map[SimpleName, List[Symbol]] = { - val symbols = - for { - (name, syms) <- nameToSymbols.toList - sym <- syms - } yield (sym, name) - symbols - .groupBy(_._2.stripModuleClassSuffix.toSimpleName) - .mapValues(_.map(_._1)) - } + def mappings: Map[TermName, List[Symbol]] = nameToSymbols.toMap } }