From 43455e4bbe227a381625144a55a2c17f1509d590 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Tue, 20 Nov 2018 09:58:01 +0100 Subject: [PATCH 01/14] Fix #5460: Improve completion of import nodes --- .../tools/dotc/interactive/Interactive.scala | 53 +++++++-- .../languageserver/DottyLanguageServer.scala | 2 +- .../tools/languageserver/CompletionTest.scala | 103 +++++++++++++++++- .../languageserver/util/CodeTester.scala | 16 ++- .../util/actions/CodeCompletion.scala | 12 +- 5 files changed, 168 insertions(+), 18 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala index 77b0b5ec134c..294ca32db4bc 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala @@ -166,31 +166,64 @@ object Interactive { private def computeCompletions(pos: SourcePosition, path: List[Tree])(implicit ctx: Context): (Int, List[Symbol]) = { val completions = Scopes.newScope.openForMutations - val (completionPos, prefix, termOnly, typeOnly) = path match { + /** + * Extract basic info about completion location and the kind of symbols to include. + * + * @param path The path to the position where completion happens + * @param inImport If set, indicates that this is the completion of an import node. When + * completing imports, both types and terms are always included. + * @return The point where to insert completion, whether terms should be included in results, + * whether types should be included, and whether we're completing an import. + */ + def completionInfo(path: List[Tree], inImport: Boolean): (Int, String, Boolean, Boolean, Boolean) = path match { case (ref: RefTree) :: _ => if (ref.name == nme.ERROR) - (ref.pos.point, "", false, false) + (ref.pos.point, "", false, false, inImport) else (ref.pos.point, ref.name.toString.take(pos.pos.point - ref.pos.point), - ref.name.isTermName, - ref.name.isTypeName) + !inImport && ref.name.isTermName, // Types and terms are always accepted in imports + !inImport && ref.name.isTypeName, + inImport) case _ => - (0, "", false, false) + (0, "", false, false, false) + } + + val (completionPos, prefix, termOnly, typeOnly, inImport) = path match { + case (imp: Import) :: _ => + imp.selectors.find(_.pos.contains(pos.pos)) match { + case None => (imp.expr.pos.point, "", false, false, true) + case Some(sel) => completionInfo(sel.asInstanceOf[tpd.Tree] :: Nil, /* inImport = */ true) + } + case other => + completionInfo(other, /* inImport = */ false) } /** 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 - * 3. have same term/type kind as name prefix given so far + * 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. + * + * The reason for (4) is that we want to filter, for instance, non-existnet `Module` + * symbols that accompany class symbols. We can't simply return only the source symbols, + * because this would discard some synthetic symbols such as the copy method of case + * classes. */ def include(sym: Symbol) = sym.name.startsWith(prefix) && !sym.name.toString.drop(prefix.length).contains('$') && + !sym.isPrimaryConstructor && + sym.sourceSymbol.exists && + (!sym.is(Package) || !sym.moduleClass.exists) && + !sym.is(allOf(Mutable, Accessor)) && (!termOnly || sym.isTerm) && (!typeOnly || sym.isType) @@ -270,12 +303,16 @@ object Interactive { def getMemberCompletions(qual: Tree): Unit = { addAccessibleMembers(qual.tpe) - implicitConversionTargets(qual)(ctx.fresh.setExploreTyperState()) - .foreach(addAccessibleMembers(_)) + if (!inImport) { + // Implicit conversions do not kick in when importing + implicitConversionTargets(qual)(ctx.fresh.setExploreTyperState()) + .foreach(addAccessibleMembers(_)) + } } path match { case (sel @ Select(qual, _)) :: _ => getMemberCompletions(qual) + case (imp @ Import(expr, _)) :: _ => getMemberCompletions(expr) case _ => getScopeCompletions(ctx) } diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index d4d38f38a4d5..dc73b72b1cce 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -779,7 +779,7 @@ object DottyLanguageServer { def completionItemKind(sym: Symbol)(implicit ctx: Context): lsp4j.CompletionItemKind = { import lsp4j.{CompletionItemKind => CIK} - if (sym.is(Package)) + if (sym.is(Package) || sym.is(Module)) CIK.Module // No CompletionItemKind.Package (https://github.com/Microsoft/language-server-protocol/issues/155) else if (sym.isConstructor) CIK.Constructor diff --git a/language-server/test/dotty/tools/languageserver/CompletionTest.scala b/language-server/test/dotty/tools/languageserver/CompletionTest.scala index 1bdca4903ebd..b7bbc34222af 100644 --- a/language-server/test/dotty/tools/languageserver/CompletionTest.scala +++ b/language-server/test/dotty/tools/languageserver/CompletionTest.scala @@ -1,7 +1,8 @@ package dotty.tools.languageserver +import org.junit.Assert.{assertTrue, assertFalse} import org.junit.Test -import org.eclipse.lsp4j.CompletionItemKind +import org.eclipse.lsp4j.CompletionItemKind._ import dotty.tools.languageserver.util.Code._ @@ -9,7 +10,7 @@ class CompletionTest { @Test def completion0: Unit = { code"class Foo { val xyz: Int = 0; def y: Int = xy$m1 }".withSource - .completion(m1, Set(("xyz", CompletionItemKind.Field, "Int"))) + .completion(m1, Set(("xyz", Field, "Int"))) } @Test def completionWithImplicitConversion: Unit = { @@ -17,6 +18,102 @@ class CompletionTest { code"object Foo { implicit class WithBaz(bar: Bar) { def baz = 0 } }", code"class Bar", code"object Main { import Foo._; val bar: Bar = new Bar; bar.b${m1} }" - ) .completion(m1, Set(("baz", CompletionItemKind.Method, "=> Int"))) + ) .completion(m1, Set(("baz", Method, "=> Int"))) + } + + @Test def importCompleteClassWithPrefix: Unit = { + withSources( + code"""object Foo { class MyClass }""", + code"""import Foo.My${m1}""" + ).completion(m1, Set(("MyClass", Class, "Object{...}"))) + } + + @Test def ImportCompleteClassNoPrefix: Unit = { + withSources( + code"""object Foo { class MyClass }""", + code"""import Foo.${m1}""" + ).completion(m1, results => { + val myClass = ("MyClass", Class, "Object{...}") + assertTrue(results.contains(("MyClass", Class, "Object{...}"))) + + // Verify that apart from `MyClass`, we only have the methods that exists on `Foo` + assertTrue((results - myClass).forall { case (_, kind, _) => kind == Method }) + + // Verify that we don't have things coming from an implicit conversion, such as ensuring + assertFalse(results.exists { case (name, _, _) => name == "ensuring" }) + }) + } + + @Test def importCompleteFromPackage: Unit = { + withSources( + code"""package a + class MyClass""", + code"""package b + import a.My${m1}""" + ).completion(m1, Set(("MyClass", Class, "Object{...}"))) + } + + @Test def importCompleteFromClass: Unit = { + withSources( + code"""class Foo { val x: Int = 0 }""", + code"""import Foo.${m1}""" + ).completion(m1, Set()) + } + + @Test def importCompleteIncludesSynthetic: Unit = { + code"""case class MyCaseClass(foobar: Int) + object O { + val x = MyCaseClass(0) + import x.c${m1} + }""".withSource + .completion( + m1, + Set(("clone", Method, "(): Object"), + ("copy", Method, "(foobar: Int): MyCaseClass"), + ("canEqual", Method, "(that: Any): Boolean"))) + } + + @Test def importCompleteIncludeModule: Unit = { + withSources( + code"""object O { object MyObject }""", + code"""import O.My${m1}""" + ).completion(m1, Set(("MyObject", Module, "O.MyObject"))) + } + + @Test def importCompleteIncludeClassAndCompanion: Unit = { + withSources( + code"""package pkg0 + class Foo + object Foo""", + code"""package pgk1 + import pkg0.F${m1}""" + ).completion(m1, Set(("Foo", Class, "Object{...}"), ("Foo", Module, "pkg0.Foo"))) + } + + @Test def importCompleteIncludePackage: Unit = { + withSources( + code"""package foo.bar + class Fizz""", + code"""import foo.b${m1}""" + ).completion(m1, Set(("bar", Module, "{...}"))) + } + + @Test def importCompleteIncludeMembers: Unit = { + withSources( + code"""object MyObject { + val myVal = 0 + def myDef = 0 + var myVar = 0 + object myObject + class myClass + trait myTrait + }""", + code"""import MyObject.my${m1}""" + ).completion(m1, Set(("myVal", Field, "Int"), + ("myDef", Method, "=> Int"), + ("myVar", Variable, "Int"), + ("myObject", Module, "MyObject.myObject"), + ("myClass", Class, "Object{...}"), + ("myTrait", Class, "Object{...}"))) } } diff --git a/language-server/test/dotty/tools/languageserver/util/CodeTester.scala b/language-server/test/dotty/tools/languageserver/util/CodeTester.scala index 28036655b51e..af8762952be7 100644 --- a/language-server/test/dotty/tools/languageserver/util/CodeTester.scala +++ b/language-server/test/dotty/tools/languageserver/util/CodeTester.scala @@ -12,6 +12,8 @@ import org.eclipse.lsp4j.{ CompletionItemKind, DocumentHighlightKind, Diagnostic import org.junit.Assert.assertEquals +import org.junit.Assert.assertEquals + /** * Simulates an LSP client for test in a project defined by `sources`. * @@ -114,7 +116,19 @@ class CodeTester(projects: List[Project]) { * @see dotty.tools.languageserver.util.actions.CodeCompletion */ def completion(marker: CodeMarker, expected: Set[(String, CompletionItemKind, String)]): this.type = - doAction(new CodeCompletion(marker, expected)) + completion(marker, assertEquals(expected, _)) + + /** + * Requests completion at the position defined by `marker`, and pass the results to + * `checkResults`. + * + * @param marker The position from which to ask for completions. + * @param checkResults A function that verifies that the results of completion are correct. + * + * @see dotty.tools.languageserver.util.actions.CodeCompletion + */ + def completion(marker: CodeMarker, checkResults: Set[(String, CompletionItemKind, String)] => Unit): this.type = + doAction(new CodeCompletion(marker, checkResults)) /** * Performs a workspace-wide renaming of the symbol under `marker`, verifies that the positions to diff --git a/language-server/test/dotty/tools/languageserver/util/actions/CodeCompletion.scala b/language-server/test/dotty/tools/languageserver/util/actions/CodeCompletion.scala index ff639039d505..ee8c4543b9a4 100644 --- a/language-server/test/dotty/tools/languageserver/util/actions/CodeCompletion.scala +++ b/language-server/test/dotty/tools/languageserver/util/actions/CodeCompletion.scala @@ -13,11 +13,13 @@ import scala.collection.JavaConverters._ * An action requesting for code completion at `marker`, expecting `expected`. * This action corresponds to the `textDocument/completion` method of the Language Server Protocol. * - * @param marker The marker indicating the position where completion should be requested. - * @param expected The expected results from the language server. + * @param marker The marker indicating the position where completion should be requested. + * @param checkResults A function that takes the results and verifies that they match + * expectations. */ class CodeCompletion(override val marker: CodeMarker, - expected: Set[(String, CompletionItemKind, String)]) extends ActionOnMarker { + checkResults: Set[(String, CompletionItemKind, String)] => Unit) + extends ActionOnMarker { override def execute(): Exec[Unit] = { val result = server.completion(marker.toCompletionParams).get() @@ -26,9 +28,9 @@ class CodeCompletion(override val marker: CodeMarker, val completionResults = result.getRight.getItems.asScala.toSet.map { item => (item.getLabel, item.getKind, item.getDetail) } - assertEquals(expected, completionResults) + checkResults(completionResults) } override def show: PositionContext.PosCtx[String] = - s"CodeCompletion(${marker.show}, $expected)" + s"CodeCompletion(${marker.show}, $checkResults)" } From c60930a870b2e764ed8503b48dca3fddf963e3d6 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Tue, 20 Nov 2018 11:27:56 +0100 Subject: [PATCH 02/14] Exclude Java module class in completion in imports When completing imports, we don't want to show duplicates for instance when the user does: import java.io.FileDesc When completing imports, we filter out the Java module classes, to keep only the classes. This avoids showing twice `java.io.FileDescriptor`. --- .../tools/dotc/interactive/Interactive.scala | 6 +++-- .../tools/languageserver/CompletionTest.scala | 22 +++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala index 294ca32db4bc..0b3ac89625be 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala @@ -205,8 +205,9 @@ object Interactive { * 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 + * 6. are not Java module classes when completing imports (to avoid duplicate results). + * 7. are mutable accessors, to exclude setters for `var`, + * 8. 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 @@ -223,6 +224,7 @@ object Interactive { !sym.isPrimaryConstructor && sym.sourceSymbol.exists && (!sym.is(Package) || !sym.moduleClass.exists) && + (!inImport || !sym.is(allOf(JavaDefined, Module), butNot = Package)) && !sym.is(allOf(Mutable, Accessor)) && (!termOnly || sym.isTerm) && (!typeOnly || sym.isType) diff --git a/language-server/test/dotty/tools/languageserver/CompletionTest.scala b/language-server/test/dotty/tools/languageserver/CompletionTest.scala index b7bbc34222af..c9d2898ebe79 100644 --- a/language-server/test/dotty/tools/languageserver/CompletionTest.scala +++ b/language-server/test/dotty/tools/languageserver/CompletionTest.scala @@ -116,4 +116,26 @@ class CompletionTest { ("myClass", Class, "Object{...}"), ("myTrait", Class, "Object{...}"))) } + + @Test def importJavaClass: Unit = { + code"""import java.io.FileDesc${m1}""".withSource + .completion(m1, Set(("FileDescriptor", Class, "Object{...}"))) + } + + @Test def importJavaStaticMethod: Unit = { + code"""import java.lang.System.lineSep${m1}""".withSource + .completion(m1, Set(("lineSeparator", Method, "(): String"))) + } + + @Test def importJavaStaticField: Unit = { + code"""import java.lang.System.ou${m1}""".withSource + .completion(m1, Set(("out", Field, "java.io.PrintStream"))) + } + + @Test def completeJavaModuleClass: Unit = { + code"""object O { + val out = java.io.FileDesc${m1} + }""".withSource + .completion(m1, Set(("FileDescriptor", Module, "java.io.FileDescriptor"))) + } } From 25dc9cba85196cd0a39158bbcf28f09a7372d05d Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Tue, 20 Nov 2018 11:59:58 +0100 Subject: [PATCH 03/14] Fix completion with renaming imports --- .../dotty/tools/dotc/interactive/Interactive.scala | 13 ++++++++++--- .../dotty/tools/languageserver/CompletionTest.scala | 5 +++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala index 0b3ac89625be..48213efad90c 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala @@ -190,11 +190,17 @@ object Interactive { } val (completionPos, prefix, termOnly, typeOnly, inImport) = path match { + case (Thicket(name :: _ :: Nil)) :: (imp: Import) :: _ => + if (name.pos.contains(pos.pos)) + completionInfo(name.asInstanceOf[tpd.Tree] :: Nil, /* inImport = */ true) + else completionInfo(path, /* inImport = */ true) + case (imp: Import) :: _ => imp.selectors.find(_.pos.contains(pos.pos)) match { case None => (imp.expr.pos.point, "", false, false, true) case Some(sel) => completionInfo(sel.asInstanceOf[tpd.Tree] :: Nil, /* inImport = */ true) } + case other => completionInfo(other, /* inImport = */ false) } @@ -313,9 +319,10 @@ object Interactive { } path match { - case (sel @ Select(qual, _)) :: _ => getMemberCompletions(qual) - case (imp @ Import(expr, _)) :: _ => getMemberCompletions(expr) - case _ => getScopeCompletions(ctx) + case Select(qual, _) :: _ => getMemberCompletions(qual) + case Import(expr, _) :: _ => getMemberCompletions(expr) + case (_: Thicket) :: Import(expr, _) :: _ => getMemberCompletions(expr) + case _ => getScopeCompletions(ctx) } val completionList = completions.toList diff --git a/language-server/test/dotty/tools/languageserver/CompletionTest.scala b/language-server/test/dotty/tools/languageserver/CompletionTest.scala index c9d2898ebe79..b652cad52f05 100644 --- a/language-server/test/dotty/tools/languageserver/CompletionTest.scala +++ b/language-server/test/dotty/tools/languageserver/CompletionTest.scala @@ -138,4 +138,9 @@ class CompletionTest { }""".withSource .completion(m1, Set(("FileDescriptor", Module, "java.io.FileDescriptor"))) } + + @Test def importRename: Unit = { + code"""import java.io.{FileDesc${m1} => Foo}""".withSource + .completion(m1, Set(("FileDescriptor", Class, "Object{...}"))) + } } From 18f4c824120e9d2cf290fed10ec02186d371ba1f Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Tue, 20 Nov 2018 14:26:27 +0100 Subject: [PATCH 04/14] Refactor `completionInfo` --- .../tools/dotc/interactive/Interactive.scala | 51 +++++++++++-------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala index 48213efad90c..77de198fcc70 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala @@ -166,30 +166,41 @@ object Interactive { private def computeCompletions(pos: SourcePosition, path: List[Tree])(implicit ctx: Context): (Int, List[Symbol]) = { val completions = Scopes.newScope.openForMutations + /** + * The information about the current completion. + * + * @param position The position where the completion result should be inserted. + * @param prefix A prefix that potential completion results must match. + * @param termOnly If set, only terms should be considered as completion results. + * @param typeOnly If set, only types should be considered as completion results. + * @param inImport If set, indicates that this is the completion of an import node. + */ + case class CompletionInfo(position: Int, prefix: String, termOnly: Boolean, typeOnly: Boolean, inImport: Boolean) + /** * Extract basic info about completion location and the kind of symbols to include. * * @param path The path to the position where completion happens * @param inImport If set, indicates that this is the completion of an import node. When * completing imports, both types and terms are always included. - * @return The point where to insert completion, whether terms should be included in results, - * whether types should be included, and whether we're completing an import. + * @return The completion info */ - def completionInfo(path: List[Tree], inImport: Boolean): (Int, String, Boolean, Boolean, Boolean) = path match { + def completionInfo(path: List[Tree], inImport: Boolean): CompletionInfo = path match { case (ref: RefTree) :: _ => if (ref.name == nme.ERROR) - (ref.pos.point, "", false, false, inImport) + CompletionInfo(ref.pos.point, "", false, false, inImport) else - (ref.pos.point, - ref.name.toString.take(pos.pos.point - ref.pos.point), - !inImport && ref.name.isTermName, // Types and terms are always accepted in imports - !inImport && ref.name.isTypeName, - inImport) + CompletionInfo( + ref.pos.point, + ref.name.toString.take(pos.pos.point - ref.pos.point), + !inImport && ref.name.isTermName, // Types and terms are always accepted in imports + !inImport && ref.name.isTypeName, + inImport) case _ => - (0, "", false, false, false) + CompletionInfo(0, "", false, false, false) } - val (completionPos, prefix, termOnly, typeOnly, inImport) = path match { + val info = path match { case (Thicket(name :: _ :: Nil)) :: (imp: Import) :: _ => if (name.pos.contains(pos.pos)) completionInfo(name.asInstanceOf[tpd.Tree] :: Nil, /* inImport = */ true) @@ -197,7 +208,7 @@ object Interactive { case (imp: Import) :: _ => imp.selectors.find(_.pos.contains(pos.pos)) match { - case None => (imp.expr.pos.point, "", false, false, true) + case None => CompletionInfo(imp.expr.pos.point, "", false, false, true) case Some(sel) => completionInfo(sel.asInstanceOf[tpd.Tree] :: Nil, /* inImport = */ true) } @@ -225,15 +236,15 @@ object Interactive { * classes. */ def include(sym: Symbol) = - sym.name.startsWith(prefix) && - !sym.name.toString.drop(prefix.length).contains('$') && + sym.name.startsWith(info.prefix) && + !sym.name.toString.drop(info.prefix.length).contains('$') && !sym.isPrimaryConstructor && sym.sourceSymbol.exists && (!sym.is(Package) || !sym.moduleClass.exists) && - (!inImport || !sym.is(allOf(JavaDefined, Module), butNot = Package)) && + (!info.inImport || !sym.is(allOf(JavaDefined, Module), butNot = Package)) && !sym.is(allOf(Mutable, Accessor)) && - (!termOnly || sym.isTerm) && - (!typeOnly || sym.isType) + (!info.termOnly || sym.isTerm) && + (!info.typeOnly || sym.isType) def enter(sym: Symbol) = if (include(sym)) completions.enter(sym) @@ -311,7 +322,7 @@ object Interactive { def getMemberCompletions(qual: Tree): Unit = { addAccessibleMembers(qual.tpe) - if (!inImport) { + if (!info.inImport) { // Implicit conversions do not kick in when importing implicitConversionTargets(qual)(ctx.fresh.setExploreTyperState()) .foreach(addAccessibleMembers(_)) @@ -326,8 +337,8 @@ object Interactive { } val completionList = completions.toList - interactiv.println(i"completion with pos = $pos, prefix = $prefix, termOnly = $termOnly, typeOnly = $typeOnly = $completionList%, %") - (completionPos, completionList) + interactiv.println(i"completion with pos = $pos, prefix = $info.prefix, termOnly = $info.termOnly, typeOnly = $info.typeOnly = $completionList%, %") + (info.position, completionList) } /** Possible completions of members of `prefix` which are accessible when called inside `boundary` */ From ffa46910c2b198eb2b8825b3671e52f0e3e9ce07 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Tue, 20 Nov 2018 14:44:35 +0100 Subject: [PATCH 05/14] Prefer types over terms in import completions When completing imports, and several symbols with the same name are possible options, we show only the type symbols. --- .../tools/dotc/interactive/Interactive.scala | 26 +++++++++---------- .../tools/languageserver/CompletionTest.scala | 4 +-- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala index 77de198fcc70..20340d73db21 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala @@ -220,28 +220,19 @@ object Interactive { * 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. have an existing source symbol, - * 5. are the module class in case of packages, - * 6. are not Java module classes when completing imports (to avoid duplicate results). - * 7. are mutable accessors, to exclude setters for `var`, - * 8. have same term/type kind as name prefix given so far + * 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 * * 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. - * - * The reason for (4) is that we want to filter, for instance, non-existnet `Module` - * symbols that accompany class symbols. We can't simply return only the source symbols, - * because this would discard some synthetic symbols such as the copy method of case - * classes. */ def include(sym: Symbol) = sym.name.startsWith(info.prefix) && !sym.name.toString.drop(info.prefix.length).contains('$') && !sym.isPrimaryConstructor && - sym.sourceSymbol.exists && (!sym.is(Package) || !sym.moduleClass.exists) && - (!info.inImport || !sym.is(allOf(JavaDefined, Module), butNot = Package)) && !sym.is(allOf(Mutable, Accessor)) && (!info.termOnly || sym.isTerm) && (!info.typeOnly || sym.isType) @@ -336,7 +327,16 @@ object Interactive { case _ => getScopeCompletions(ctx) } - val completionList = completions.toList + val completionList = + if (!info.inImport) completions.toList + else { + // In imports, 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 + } + interactiv.println(i"completion with pos = $pos, prefix = $info.prefix, termOnly = $info.termOnly, typeOnly = $info.typeOnly = $completionList%, %") (info.position, completionList) } diff --git a/language-server/test/dotty/tools/languageserver/CompletionTest.scala b/language-server/test/dotty/tools/languageserver/CompletionTest.scala index b652cad52f05..f041e23b891f 100644 --- a/language-server/test/dotty/tools/languageserver/CompletionTest.scala +++ b/language-server/test/dotty/tools/languageserver/CompletionTest.scala @@ -80,14 +80,14 @@ class CompletionTest { ).completion(m1, Set(("MyObject", Module, "O.MyObject"))) } - @Test def importCompleteIncludeClassAndCompanion: Unit = { + @Test def importCompleteWithClassAndCompanion: Unit = { withSources( code"""package pkg0 class Foo object Foo""", code"""package pgk1 import pkg0.F${m1}""" - ).completion(m1, Set(("Foo", Class, "Object{...}"), ("Foo", Module, "pkg0.Foo"))) + ).completion(m1, Set(("Foo", Class, "Object{...}"))) } @Test def importCompleteIncludePackage: Unit = { From 341dc28577e123244aec1c98198a7861ff1e383b Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Tue, 20 Nov 2018 15:11:21 +0100 Subject: [PATCH 06/14] Improve display of completion results When available, we display the formatted documentation for the imported symbol. For type symbols, we display the full name of the symbol, and for term symbols we show their type. --- .../languageserver/DottyLanguageServer.scala | 14 ++++++++----- .../tools/languageserver/CompletionTest.scala | 20 +++++++++---------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index dc73b72b1cce..036dc405b3b3 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -795,12 +795,16 @@ object DottyLanguageServer { val label = sym.name.show val item = new lsp4j.CompletionItem(label) - item.setDetail(sym.info.widenTermRefExpr.show) + 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)) + } item.setKind(completionItemKind(sym)) item } - def hoverContent(content: String): lsp4j.MarkupContent = { + def markupContent(content: String): lsp4j.MarkupContent = { if (content.isEmpty) null else { val markup = new lsp4j.MarkupContent @@ -824,7 +828,7 @@ object DottyLanguageServer { buf.append(comment.renderAsMarkdown) } - hoverContent(buf.toString) + markupContent(buf.toString) } /** Create an lsp4j.SymbolInfo from a Symbol and a SourcePosition */ @@ -869,7 +873,7 @@ object DottyLanguageServer { val tparamsLabel = if (signature.tparams.isEmpty) "" else signature.tparams.mkString("[", ", ", "]") val returnTypeLabel = signature.returnType.map(t => s": $t").getOrElse("") val label = s"${signature.name}$tparamsLabel$paramLists$returnTypeLabel" - val documentation = signature.doc.map(DottyLanguageServer.hoverContent) + val documentation = signature.doc.map(DottyLanguageServer.markupContent) val sig = new lsp4j.SignatureInformation(label) sig.setParameters(paramInfoss.flatten.asJava) documentation.foreach(sig.setDocumentation(_)) @@ -878,7 +882,7 @@ object DottyLanguageServer { /** Convert `param` to `ParameterInformation` */ private def paramToParameterInformation(param: Signatures.Param): lsp4j.ParameterInformation = { - val documentation = param.doc.map(DottyLanguageServer.hoverContent) + val documentation = param.doc.map(DottyLanguageServer.markupContent) val info = new lsp4j.ParameterInformation(param.show) documentation.foreach(info.setDocumentation(_)) info diff --git a/language-server/test/dotty/tools/languageserver/CompletionTest.scala b/language-server/test/dotty/tools/languageserver/CompletionTest.scala index f041e23b891f..e1f1a8e47487 100644 --- a/language-server/test/dotty/tools/languageserver/CompletionTest.scala +++ b/language-server/test/dotty/tools/languageserver/CompletionTest.scala @@ -25,7 +25,7 @@ class CompletionTest { withSources( code"""object Foo { class MyClass }""", code"""import Foo.My${m1}""" - ).completion(m1, Set(("MyClass", Class, "Object{...}"))) + ).completion(m1, Set(("MyClass", Class, "Foo.MyClass"))) } @Test def ImportCompleteClassNoPrefix: Unit = { @@ -33,8 +33,8 @@ class CompletionTest { code"""object Foo { class MyClass }""", code"""import Foo.${m1}""" ).completion(m1, results => { - val myClass = ("MyClass", Class, "Object{...}") - assertTrue(results.contains(("MyClass", Class, "Object{...}"))) + val myClass = ("MyClass", Class, "Foo.MyClass") + assertTrue(results.contains(("MyClass", Class, "Foo.MyClass"))) // Verify that apart from `MyClass`, we only have the methods that exists on `Foo` assertTrue((results - myClass).forall { case (_, kind, _) => kind == Method }) @@ -50,7 +50,7 @@ class CompletionTest { class MyClass""", code"""package b import a.My${m1}""" - ).completion(m1, Set(("MyClass", Class, "Object{...}"))) + ).completion(m1, Set(("MyClass", Class, "a.MyClass"))) } @Test def importCompleteFromClass: Unit = { @@ -87,7 +87,7 @@ class CompletionTest { object Foo""", code"""package pgk1 import pkg0.F${m1}""" - ).completion(m1, Set(("Foo", Class, "Object{...}"))) + ).completion(m1, Set(("Foo", Class, "pkg0.Foo"))) } @Test def importCompleteIncludePackage: Unit = { @@ -95,7 +95,7 @@ class CompletionTest { code"""package foo.bar class Fizz""", code"""import foo.b${m1}""" - ).completion(m1, Set(("bar", Module, "{...}"))) + ).completion(m1, Set(("bar", Module, "foo.bar"))) } @Test def importCompleteIncludeMembers: Unit = { @@ -113,13 +113,13 @@ class CompletionTest { ("myDef", Method, "=> Int"), ("myVar", Variable, "Int"), ("myObject", Module, "MyObject.myObject"), - ("myClass", Class, "Object{...}"), - ("myTrait", Class, "Object{...}"))) + ("myClass", Class, "MyObject.myClass"), + ("myTrait", Class, "MyObject.myTrait"))) } @Test def importJavaClass: Unit = { code"""import java.io.FileDesc${m1}""".withSource - .completion(m1, Set(("FileDescriptor", Class, "Object{...}"))) + .completion(m1, Set(("FileDescriptor", Class, "java.io.FileDescriptor"))) } @Test def importJavaStaticMethod: Unit = { @@ -141,6 +141,6 @@ class CompletionTest { @Test def importRename: Unit = { code"""import java.io.{FileDesc${m1} => Foo}""".withSource - .completion(m1, Set(("FileDescriptor", Class, "Object{...}"))) + .completion(m1, Set(("FileDescriptor", Class, "java.io.FileDescriptor"))) } } From 82e84d2a4f785f9edb411cdc85ecf8ffb124f34c Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Tue, 20 Nov 2018 15:56:55 +0100 Subject: [PATCH 07/14] Mark deprecated symbols as such in completion --- .../languageserver/DottyLanguageServer.scala | 2 ++ .../tools/languageserver/CompletionTest.scala | 20 +++++++++++++++++-- .../languageserver/util/CodeTester.scala | 6 +++--- .../util/actions/CodeCompletion.scala | 14 ++++++++----- 4 files changed, 32 insertions(+), 10 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index 036dc405b3b3..4b875b0583c7 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -19,6 +19,7 @@ import scala.io.Codec import dotc._ import ast.{Trees, tpd} import core._, core.Decorators.{sourcePos => _, _} +import Annotations.AnnotInfo import Comments._, Constants._, Contexts._, Flags._, Names._, NameOps._, Symbols._, SymDenotations._, Trees._, Types._ import classpath.ClassPathEntries import reporting._, reporting.diagnostic.{Message, MessageContainer, messages} @@ -800,6 +801,7 @@ object DottyLanguageServer { ParsedComment.docOf(sym).foreach { doc => item.setDocumentation(markupContent(doc.renderAsMarkdown)) } + item.setDeprecated(sym.isDeprecated) item.setKind(completionItemKind(sym)) item } diff --git a/language-server/test/dotty/tools/languageserver/CompletionTest.scala b/language-server/test/dotty/tools/languageserver/CompletionTest.scala index e1f1a8e47487..992ea403ca4b 100644 --- a/language-server/test/dotty/tools/languageserver/CompletionTest.scala +++ b/language-server/test/dotty/tools/languageserver/CompletionTest.scala @@ -1,10 +1,11 @@ package dotty.tools.languageserver -import org.junit.Assert.{assertTrue, assertFalse} +import org.junit.Assert.{assertEquals, assertTrue, assertFalse} import org.junit.Test import org.eclipse.lsp4j.CompletionItemKind._ import dotty.tools.languageserver.util.Code._ +import dotty.tools.languageserver.util.actions.CodeCompletion class CompletionTest { @@ -32,7 +33,8 @@ class CompletionTest { withSources( code"""object Foo { class MyClass }""", code"""import Foo.${m1}""" - ).completion(m1, results => { + ).completion(m1, completionItems => { + val results = CodeCompletion.simplifyResults(completionItems) val myClass = ("MyClass", Class, "Foo.MyClass") assertTrue(results.contains(("MyClass", Class, "Foo.MyClass"))) @@ -143,4 +145,18 @@ class CompletionTest { code"""import java.io.{FileDesc${m1} => Foo}""".withSource .completion(m1, Set(("FileDescriptor", Class, "java.io.FileDescriptor"))) } + + @Test def markDeprecatedSymbols: Unit = { + code"""object Foo { + @deprecated + val bar = 0 + } + import Foo.ba${m1}""".withSource + .completion(m1, results => { + assertEquals(1, results.size) + val result = results.head + assertEquals("bar", result.getLabel) + assertTrue("bar was not deprecated", result.getDeprecated) + }) + } } diff --git a/language-server/test/dotty/tools/languageserver/util/CodeTester.scala b/language-server/test/dotty/tools/languageserver/util/CodeTester.scala index af8762952be7..53167188be9c 100644 --- a/language-server/test/dotty/tools/languageserver/util/CodeTester.scala +++ b/language-server/test/dotty/tools/languageserver/util/CodeTester.scala @@ -8,7 +8,7 @@ import dotty.tools.languageserver.util.server.{TestFile, TestServer} import dotty.tools.dotc.reporting.diagnostic.ErrorMessageID import dotty.tools.dotc.util.Signatures.Signature -import org.eclipse.lsp4j.{ CompletionItemKind, DocumentHighlightKind, Diagnostic, DiagnosticSeverity } +import org.eclipse.lsp4j.{ CompletionItem, CompletionItemKind, DocumentHighlightKind, Diagnostic, DiagnosticSeverity } import org.junit.Assert.assertEquals @@ -116,7 +116,7 @@ class CodeTester(projects: List[Project]) { * @see dotty.tools.languageserver.util.actions.CodeCompletion */ def completion(marker: CodeMarker, expected: Set[(String, CompletionItemKind, String)]): this.type = - completion(marker, assertEquals(expected, _)) + completion(marker, results => assertEquals(expected, CodeCompletion.simplifyResults(results))) /** * Requests completion at the position defined by `marker`, and pass the results to @@ -127,7 +127,7 @@ class CodeTester(projects: List[Project]) { * * @see dotty.tools.languageserver.util.actions.CodeCompletion */ - def completion(marker: CodeMarker, checkResults: Set[(String, CompletionItemKind, String)] => Unit): this.type = + def completion(marker: CodeMarker, checkResults: Set[CompletionItem] => Unit): this.type = doAction(new CodeCompletion(marker, checkResults)) /** diff --git a/language-server/test/dotty/tools/languageserver/util/actions/CodeCompletion.scala b/language-server/test/dotty/tools/languageserver/util/actions/CodeCompletion.scala index ee8c4543b9a4..90f7256c2293 100644 --- a/language-server/test/dotty/tools/languageserver/util/actions/CodeCompletion.scala +++ b/language-server/test/dotty/tools/languageserver/util/actions/CodeCompletion.scala @@ -4,7 +4,7 @@ import dotty.tools.languageserver.util.PositionContext import dotty.tools.languageserver.util.embedded.CodeMarker import dotty.tools.languageserver.util.server.TestFile -import org.eclipse.lsp4j.CompletionItemKind +import org.eclipse.lsp4j.{CompletionItem, CompletionItemKind} import org.junit.Assert.{assertEquals, assertFalse, assertTrue} import scala.collection.JavaConverters._ @@ -18,19 +18,23 @@ import scala.collection.JavaConverters._ * expectations. */ class CodeCompletion(override val marker: CodeMarker, - checkResults: Set[(String, CompletionItemKind, String)] => Unit) + checkResults: Set[CompletionItem] => Unit) extends ActionOnMarker { override def execute(): Exec[Unit] = { val result = server.completion(marker.toCompletionParams).get() assertTrue(s"Completion results were not 'right': $result", result.isRight) assertFalse(s"Completion results were 'incomplete': $result", result.getRight.isIncomplete) - val completionResults = result.getRight.getItems.asScala.toSet.map { item => - (item.getLabel, item.getKind, item.getDetail) - } + val completionResults = result.getRight.getItems.asScala.toSet checkResults(completionResults) } override def show: PositionContext.PosCtx[String] = s"CodeCompletion(${marker.show}, $checkResults)" } + +object CodeCompletion { + /** Extract the (label, kind, details) of each `CompletionItem`. */ + def simplifyResults(items: Set[CompletionItem]): Set[(String, CompletionItemKind, String)] = + items.map(item => (item.getLabel, item.getKind, item.getDetail)) +} From ae59176a43d8958905b2c814d8f5d17dc6b9354f Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 21 Nov 2018 13:09:57 +0100 Subject: [PATCH 08/14] Address review comments --- .../tools/dotc/interactive/Interactive.scala | 65 +++++++++++++------ .../tools/languageserver/CompletionTest.scala | 7 ++ 2 files changed, 51 insertions(+), 21 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala index 20340d73db21..4a46f034dcd4 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala @@ -166,16 +166,32 @@ object Interactive { private def computeCompletions(pos: SourcePosition, path: List[Tree])(implicit ctx: Context): (Int, List[Symbol]) = { val completions = Scopes.newScope.openForMutations + type Mode = Int + object Mode { + /** No symbol should be included */ + val None: Mode = 0 + + /** Term symbols are allowed */ + val Term: Mode = 1 + + /** Type symbols are allowed */ + val Type: Mode = 2 + + /** Both term and type symbols are allowed */ + val Import: Mode = Term | Type + + /** Does `m0` include `m1`? */ + def is(m0: Mode, m1: Mode): Boolean = (m0 & m1) == m1 + } + /** * The information about the current completion. * - * @param position The position where the completion result should be inserted. - * @param prefix A prefix that potential completion results must match. - * @param termOnly If set, only terms should be considered as completion results. - * @param typeOnly If set, only types should be considered as completion results. - * @param inImport If set, indicates that this is the completion of an import node. + * @param offset The offset where the completion result should be inserted. + * @param prefix A prefix that potential completion results must match. + * @param mode The completion mode. */ - case class CompletionInfo(position: Int, prefix: String, termOnly: Boolean, typeOnly: Boolean, inImport: Boolean) + case class CompletionInfo(offset: Int, prefix: String, mode: Mode) /** * Extract basic info about completion location and the kind of symbols to include. @@ -183,21 +199,26 @@ object Interactive { * @param path The path to the position where completion happens * @param inImport If set, indicates that this is the completion of an import node. When * completing imports, both types and terms are always included. - * @return The completion info + * @return The information about completion (offset, kinds of symbol, etc.) */ def completionInfo(path: List[Tree], inImport: Boolean): CompletionInfo = path match { case (ref: RefTree) :: _ => if (ref.name == nme.ERROR) - CompletionInfo(ref.pos.point, "", false, false, inImport) - else + CompletionInfo(ref.pos.point, "", Mode.None) + else { + val mode = + if (inImport) Mode.Import + else if (ref.name.isTermName) Mode.Term + else Mode.Type + CompletionInfo( ref.pos.point, ref.name.toString.take(pos.pos.point - ref.pos.point), - !inImport && ref.name.isTermName, // Types and terms are always accepted in imports - !inImport && ref.name.isTypeName, - inImport) + mode) + } + case _ => - CompletionInfo(0, "", false, false, false) + CompletionInfo(0, "", Mode.None) } val info = path match { @@ -208,7 +229,7 @@ object Interactive { case (imp: Import) :: _ => imp.selectors.find(_.pos.contains(pos.pos)) match { - case None => CompletionInfo(imp.expr.pos.point, "", false, false, true) + case None => CompletionInfo(imp.expr.pos.point, "", Mode.Import) case Some(sel) => completionInfo(sel.asInstanceOf[tpd.Tree] :: Nil, /* inImport = */ true) } @@ -228,18 +249,20 @@ object Interactive { * as completion results. However, if a user explicitly writes all '$' characters in an * identifier, we should complete the rest. */ - def include(sym: Symbol) = + def include(sym: Symbol) = { sym.name.startsWith(info.prefix) && !sym.name.toString.drop(info.prefix.length).contains('$') && !sym.isPrimaryConstructor && (!sym.is(Package) || !sym.moduleClass.exists) && !sym.is(allOf(Mutable, Accessor)) && - (!info.termOnly || sym.isTerm) && - (!info.typeOnly || sym.isType) + ( + (Mode.is(info.mode, Mode.Term) && sym.isTerm) + || (Mode.is(info.mode, Mode.Type) && sym.isType) + ) + } def enter(sym: Symbol) = if (include(sym)) completions.enter(sym) - def add(sym: Symbol) = if (sym.exists && !completions.lookup(sym.name).exists) enter(sym) @@ -313,7 +336,7 @@ object Interactive { def getMemberCompletions(qual: Tree): Unit = { addAccessibleMembers(qual.tpe) - if (!info.inImport) { + if (!Mode.is(info.mode, Mode.Import)) { // Implicit conversions do not kick in when importing implicitConversionTargets(qual)(ctx.fresh.setExploreTyperState()) .foreach(addAccessibleMembers(_)) @@ -328,7 +351,7 @@ object Interactive { } val completionList = - if (!info.inImport) completions.toList + if (!Mode.is(info.mode, Mode.Import)) completions.toList else { // In imports, show only the type symbols when there are multiple options with the same name completions.toList.groupBy(_.name.stripModuleClassSuffix.toSimpleName).mapValues { @@ -338,7 +361,7 @@ object Interactive { } interactiv.println(i"completion with pos = $pos, prefix = $info.prefix, termOnly = $info.termOnly, typeOnly = $info.typeOnly = $completionList%, %") - (info.position, completionList) + (info.offset, completionList) } /** Possible completions of members of `prefix` which are accessible when called inside `boundary` */ diff --git a/language-server/test/dotty/tools/languageserver/CompletionTest.scala b/language-server/test/dotty/tools/languageserver/CompletionTest.scala index 992ea403ca4b..b91f3cfa3a35 100644 --- a/language-server/test/dotty/tools/languageserver/CompletionTest.scala +++ b/language-server/test/dotty/tools/languageserver/CompletionTest.scala @@ -159,4 +159,11 @@ class CompletionTest { assertTrue("bar was not deprecated", result.getDeprecated) }) } + + @Test def i4397: Unit = { + code"""class Foo { + | .${m1} + |}""".withSource + .completion(m1, Set()) + } } From 1e82eb064d65c1777469c038bff8821f3f6c5f31 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 21 Nov 2018 13:34:56 +0100 Subject: [PATCH 09/14] Refactor completions, add interactive.Completion This commit moves all completion-related methods to their own file and cleans it up. --- .../tools/dotc/interactive/Completion.scala | 307 ++++++++++++++++++ .../tools/dotc/interactive/Interactive.scala | 236 +------------- .../src/dotty/tools/repl/ReplDriver.scala | 4 +- .../languageserver/DottyLanguageServer.scala | 2 +- 4 files changed, 313 insertions(+), 236 deletions(-) create mode 100644 compiler/src/dotty/tools/dotc/interactive/Completion.scala diff --git a/compiler/src/dotty/tools/dotc/interactive/Completion.scala b/compiler/src/dotty/tools/dotc/interactive/Completion.scala new file mode 100644 index 000000000000..a4e90cd68af2 --- /dev/null +++ b/compiler/src/dotty/tools/dotc/interactive/Completion.scala @@ -0,0 +1,307 @@ +package dotty.tools.dotc.interactive + +import dotty.tools.dotc.ast.Trees._ +import dotty.tools.dotc.config.Printers.interactiv +import dotty.tools.dotc.core.Contexts.{Context, NoContext} +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.NameKinds.SimpleNameKind +import dotty.tools.dotc.core.NameOps.NameDecorator +import dotty.tools.dotc.core.Symbols.{defn, NoSymbol, Symbol} +import dotty.tools.dotc.core.Scopes +import dotty.tools.dotc.core.StdNames.nme +import dotty.tools.dotc.core.TypeError +import dotty.tools.dotc.core.Types.{NamedType, NameFilter, Type, takeAllFilter} +import dotty.tools.dotc.printing.Texts._ +import dotty.tools.dotc.util.{NoSourcePosition, SourcePosition} + +import scala.collection.mutable + +object Completion { + + import dotty.tools.dotc.ast.tpd._ + + /** Get possible completions from tree at `pos` + * + * @return offset and list of symbols for possible completions + */ + def completions(pos: SourcePosition)(implicit ctx: Context): (Int, List[Symbol]) = { + val path = Interactive.pathTo(ctx.compilationUnit.tpdTree, pos.pos) + computeCompletions(pos, path)(Interactive.contextOfPath(path)) + } + + /** + * Inspect `path` to determine what kinds of symbols should be considered. + * + * If the path starts with: + * - a `RefTree`, then accept symbols of the same kind as its name; + * - a renaming import, and the cursor is on the renamee, accept both terms and types; + * - an import, accept both terms and types; + * + * Otherwise, provide no completion suggestion. + */ + private def completionMode(path: List[Tree], pos: SourcePosition): Mode = { + path match { + case (ref: RefTree) :: _ => + if (ref.name == nme.ERROR) Mode.None + else if (ref.name.isTermName) Mode.Term + else if (ref.name.isTypeName) Mode.Type + else Mode.None + + case Thicket(name :: _ :: Nil) :: (_: Import) :: _ => + if (name.pos.contains(pos.pos)) Mode.Import + else Mode.None // Can't help completing the renaming + + case Import(_, _) :: _ => + Mode.Import + + case _ => + Mode.None + } + } + + /** + * Inspect `path` to determine the completion prefix. Only symbols whose name start with the + * returned prefix should be considered. + */ + private def completionPrefix(path: List[Tree], pos: SourcePosition): String = { + path match { + case Thicket(name :: _ :: Nil) :: (_: Import) :: _ => + completionPrefix(name :: Nil, pos) + + case Import(expr, selectors) :: _ => + selectors.find(_.pos.contains(pos.pos)).map { selector => + completionPrefix(selector.asInstanceOf[Tree] :: Nil, pos) + }.getOrElse("") + + case (ref: RefTree) :: _ => + if (ref.name == nme.ERROR) "" + else ref.name.toString.take(pos.pos.point - ref.pos.point) + + case _ => + "" + } + } + + /** Inspect `path` to determine the offset where the completion result should be inserted. */ + private def completionOffset(path: List[Tree]): Int = { + path match { + case (ref: RefTree) :: _ => ref.pos.point + case _ => 0 + } + } + + /** Create a new `CompletionBuffer` for completing at `pos`. */ + private def completionBuffer(path: List[Tree], pos: SourcePosition): CompletionBuffer = { + val mode = completionMode(path, pos) + val prefix = completionPrefix(path, pos) + new CompletionBuffer(mode, prefix, pos) + } + + private def computeCompletions(pos: SourcePosition, path: List[Tree])(implicit ctx: Context): (Int, List[Symbol]) = { + + val offset = completionOffset(path) + val buffer = completionBuffer(path, pos) + + if (buffer.mode != Mode.None) { + path match { + case Select(qual, _) :: _ => buffer.addMemberCompletions(qual) + case Import(expr, _) :: _ => buffer.addMemberCompletions(expr) + case (_: Thicket) :: Import(expr, _) :: _ => buffer.addMemberCompletions(expr) + case _ => buffer.addScopeCompletions + } + } + + val completionList = buffer.getCompletions + + interactiv.println(i"""completion with pos = $pos, + | prefix = ${buffer.prefix}, + | term = ${buffer.mode.is(Mode.Term)}, + | type = ${buffer.mode.is(Mode.Type)} + | results = $completionList%, %""") + (offset, completionList) + } + + private class CompletionBuffer(val mode: Mode, val prefix: String, pos: SourcePosition) { + + private[this] val completions = Scopes.newScope.openForMutations + + /** + * 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. + */ + def getCompletions(implicit ctx: Context): List[Symbol] = { + if (!mode.is(Mode.Import)) completions.toList + else { + // In imports, 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 + } + } + + /** + * Add symbols that are currently in scope to `info`: the members of the current class and the + * symbols that have been imported. + */ + def addScopeCompletions(implicit ctx: Context): Unit = { + if (ctx.owner.isClass) { + addAccessibleMembers(ctx.owner.thisType) + ctx.owner.asClass.classInfo.selfInfo match { + case selfSym: Symbol => add(selfSym) + case _ => + } + } + else if (ctx.scope != null) ctx.scope.foreach(add) + + addImportCompletions + + var outer = ctx.outer + while ((outer.owner `eq` ctx.owner) && (outer.scope `eq` ctx.scope)) { + addImportCompletions(outer) + outer = outer.outer + } + if (outer `ne` NoContext) addScopeCompletions(outer) + } + + /** + * Find all the members of `qual` and add the ones that pass the include filters to `info`. + * + * If `info.mode` is `Import`, the members added via implicit conversion on `qual` are not + * considered. + */ + def addMemberCompletions(qual: Tree)(implicit ctx: Context): Unit = { + addAccessibleMembers(qual.tpe) + if (!mode.is(Mode.Import)) { + // Implicit conversions do not kick in when importing + implicitConversionTargets(qual)(ctx.fresh.setExploreTyperState()) + .foreach(addAccessibleMembers) + } + } + + /** + * 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) + } + + /** 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) + + /** 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 + * 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 + * + * 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)(implicit ctx: Context): Boolean = + sym.name.startsWith(prefix) && + !sym.name.toString.drop(prefix.length).contains('$') && + !sym.isPrimaryConstructor && + (!sym.is(Package) || !sym.moduleClass.exists) && + !sym.is(allOf(Mutable, Accessor)) && + ( + (mode.is(Mode.Term) && sym.isTerm) + || (mode.is(Mode.Type) && sym.isType) + ) + + /** + * Find all the members of `site` that are accessible and which should be included in `info`. + * + * @param site The type to inspect. + * @return The members of `site` that are accessible and pass the include filter of `info`. + */ + 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. + 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 _ => 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) + + /** + * Add in `info` the symbols that are imported by `ctx.importInfo`. If this is a wildcard import, + * all the accessible members of the import's `site` are included. + */ + 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) + } + // 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) + } + } + + /** + * Given `qual` of type T, finds all the types S such that there exists an implicit conversion + * from T to S. + * + * @param qual The argument to which the implicit conversion should be applied. + * @return The set of types that `qual` can be converted to. + */ + private def implicitConversionTargets(qual: Tree)(implicit ctx: Context): Set[Type] = { + val typer = ctx.typer + val conversions = new typer.ImplicitSearch(defn.AnyType, qual, pos.pos).allImplicits + val targets = conversions.map(_.widen.finalResultType) + interactiv.println(i"implicit conversion targets considered: ${targets.toList}%, %") + targets + } + + } + + /** + * The completion mode: defines what kinds of symbols should be included in the completion + * results. + */ + private class Mode(val bits: Int) extends AnyVal { + def is(other: Mode): Boolean = (bits & other.bits) == other.bits + def |(other: Mode): Mode = new Mode(bits | other.bits) + } + private object Mode { + /** No symbol should be included */ + val None: Mode = new Mode(0) + + /** Term symbols are allowed */ + val Term: Mode = new Mode(1) + + /** Type symbols are allowed */ + val Type: Mode = new Mode(2) + + /** Both term and type symbols are allowed */ + val Import: Mode = Term | Type + } + +} diff --git a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala index 4a46f034dcd4..419e82dfa2fb 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala @@ -151,239 +151,6 @@ object Interactive { ) } - private def safely[T](op: => List[T]): List[T] = - try op catch { case ex: TypeError => Nil } - - /** Get possible completions from tree at `pos` - * - * @return offset and list of symbols for possible completions - */ - def completions(pos: SourcePosition)(implicit ctx: Context): (Int, List[Symbol]) = { - val path = pathTo(ctx.compilationUnit.tpdTree, pos.pos) - computeCompletions(pos, path)(contextOfPath(path)) - } - - private def computeCompletions(pos: SourcePosition, path: List[Tree])(implicit ctx: Context): (Int, List[Symbol]) = { - val completions = Scopes.newScope.openForMutations - - type Mode = Int - object Mode { - /** No symbol should be included */ - val None: Mode = 0 - - /** Term symbols are allowed */ - val Term: Mode = 1 - - /** Type symbols are allowed */ - val Type: Mode = 2 - - /** Both term and type symbols are allowed */ - val Import: Mode = Term | Type - - /** Does `m0` include `m1`? */ - def is(m0: Mode, m1: Mode): Boolean = (m0 & m1) == m1 - } - - /** - * The information about the current completion. - * - * @param offset The offset where the completion result should be inserted. - * @param prefix A prefix that potential completion results must match. - * @param mode The completion mode. - */ - case class CompletionInfo(offset: Int, prefix: String, mode: Mode) - - /** - * Extract basic info about completion location and the kind of symbols to include. - * - * @param path The path to the position where completion happens - * @param inImport If set, indicates that this is the completion of an import node. When - * completing imports, both types and terms are always included. - * @return The information about completion (offset, kinds of symbol, etc.) - */ - def completionInfo(path: List[Tree], inImport: Boolean): CompletionInfo = path match { - case (ref: RefTree) :: _ => - if (ref.name == nme.ERROR) - CompletionInfo(ref.pos.point, "", Mode.None) - else { - val mode = - if (inImport) Mode.Import - else if (ref.name.isTermName) Mode.Term - else Mode.Type - - CompletionInfo( - ref.pos.point, - ref.name.toString.take(pos.pos.point - ref.pos.point), - mode) - } - - case _ => - CompletionInfo(0, "", Mode.None) - } - - val info = path match { - case (Thicket(name :: _ :: Nil)) :: (imp: Import) :: _ => - if (name.pos.contains(pos.pos)) - completionInfo(name.asInstanceOf[tpd.Tree] :: Nil, /* inImport = */ true) - else completionInfo(path, /* inImport = */ true) - - case (imp: Import) :: _ => - imp.selectors.find(_.pos.contains(pos.pos)) match { - case None => CompletionInfo(imp.expr.pos.point, "", Mode.Import) - case Some(sel) => completionInfo(sel.asInstanceOf[tpd.Tree] :: Nil, /* inImport = */ true) - } - - case other => - completionInfo(other, /* inImport = */ false) - } - - /** 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 - * 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 - * - * 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. - */ - def include(sym: Symbol) = { - sym.name.startsWith(info.prefix) && - !sym.name.toString.drop(info.prefix.length).contains('$') && - !sym.isPrimaryConstructor && - (!sym.is(Package) || !sym.moduleClass.exists) && - !sym.is(allOf(Mutable, Accessor)) && - ( - (Mode.is(info.mode, Mode.Term) && sym.isTerm) - || (Mode.is(info.mode, Mode.Type) && sym.isType) - ) - } - - def enter(sym: Symbol) = - if (include(sym)) completions.enter(sym) - def add(sym: Symbol) = - if (sym.exists && !completions.lookup(sym.name).exists) enter(sym) - - def addMember(site: Type, name: Name) = - if (!completions.lookup(name).exists) - for (alt <- site.member(name).alternatives) enter(alt.symbol) - - def accessibleMembers(site: Type, superAccess: Boolean = true): 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. - 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).symbol - case _ => NoSymbol - }.filter(_.exists) - } - - def addAccessibleMembers(site: Type, superAccess: Boolean = true): Unit = - for (mbr <- accessibleMembers(site)) addMember(site, mbr.name) - - def getImportCompletions(ictx: Context): Unit = { - implicit val ctx = ictx - val imp = ctx.importInfo - if (imp != null) { - def addImport(name: TermName) = { - addMember(imp.site, name) - addMember(imp.site, name.toTypeName) - } - // 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) - } - } - - def getScopeCompletions(ictx: Context): Unit = { - implicit val ctx = ictx - - if (ctx.owner.isClass) { - addAccessibleMembers(ctx.owner.thisType) - ctx.owner.asClass.classInfo.selfInfo match { - case selfSym: Symbol => add(selfSym) - case _ => - } - } - else if (ctx.scope != null) ctx.scope.foreach(add) - - getImportCompletions(ctx) - - var outer = ctx.outer - while ((outer.owner `eq` ctx.owner) && (outer.scope `eq` ctx.scope)) { - getImportCompletions(outer) - outer = outer.outer - } - if (outer `ne` NoContext) getScopeCompletions(outer) - } - - def implicitConversionTargets(qual: Tree)(implicit ctx: Context): Set[Type] = { - val typer = ctx.typer - val conversions = new typer.ImplicitSearch(defn.AnyType, qual, pos.pos).allImplicits - val targets = conversions.map(_.widen.finalResultType) - interactiv.println(i"implicit conversion targets considered: ${targets.toList}%, %") - targets - } - - def getMemberCompletions(qual: Tree): Unit = { - addAccessibleMembers(qual.tpe) - if (!Mode.is(info.mode, Mode.Import)) { - // Implicit conversions do not kick in when importing - implicitConversionTargets(qual)(ctx.fresh.setExploreTyperState()) - .foreach(addAccessibleMembers(_)) - } - } - - path match { - case Select(qual, _) :: _ => getMemberCompletions(qual) - case Import(expr, _) :: _ => getMemberCompletions(expr) - case (_: Thicket) :: Import(expr, _) :: _ => getMemberCompletions(expr) - case _ => getScopeCompletions(ctx) - } - - val completionList = - if (!Mode.is(info.mode, Mode.Import)) completions.toList - else { - // In imports, 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 - } - - interactiv.println(i"completion with pos = $pos, prefix = $info.prefix, termOnly = $info.termOnly, typeOnly = $info.typeOnly = $completionList%, %") - (info.offset, completionList) - } - - /** Possible completions of members of `prefix` which are accessible when called inside `boundary` */ - def completions(prefix: Type, boundary: Symbol)(implicit ctx: Context): List[Symbol] = - safely { - if (boundary != NoSymbol) { - val boundaryCtx = ctx.withOwner(boundary) - def exclude(sym: Symbol) = sym.isAbsent || sym.is(Synthetic) || sym.is(Artifact) - def addMember(name: Name, buf: mutable.Buffer[SingleDenotation]): Unit = - buf ++= prefix.member(name).altsWith(sym => - !exclude(sym) && sym.isAccessibleFrom(prefix)(boundaryCtx)) - prefix.memberDenots(completionsFilter, addMember).map(_.symbol).toList - } - else Nil - } - - /** 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 - } - /** Find named trees with a non-empty position whose symbol match `sym` in `trees`. * * Note that nothing will be found for symbols not defined in source code, @@ -652,4 +419,7 @@ object Interactive { n0.stripModuleClassSuffix.toTermName eq n1.stripModuleClassSuffix.toTermName } + private[interactive] def safely[T](op: => List[T]): List[T] = + try op catch { case ex: TypeError => Nil } + } diff --git a/compiler/src/dotty/tools/repl/ReplDriver.scala b/compiler/src/dotty/tools/repl/ReplDriver.scala index 3de64529da88..1c219639201b 100644 --- a/compiler/src/dotty/tools/repl/ReplDriver.scala +++ b/compiler/src/dotty/tools/repl/ReplDriver.scala @@ -13,7 +13,7 @@ import dotty.tools.dotc.core.NameOps._ import dotty.tools.dotc.core.Names.Name import dotty.tools.dotc.core.StdNames._ import dotty.tools.dotc.core.Symbols.{Symbol, defn} -import dotty.tools.dotc.interactive.Interactive +import dotty.tools.dotc.interactive.Completion import dotty.tools.dotc.printing.SyntaxHighlighting import dotty.tools.dotc.reporting.MessageRendering import dotty.tools.dotc.reporting.diagnostic.{Message, MessageContainer} @@ -170,7 +170,7 @@ class ReplDriver(settings: Array[String], unit.tpdTree = tree implicit val ctx = state.context.fresh.setCompilationUnit(unit) val srcPos = SourcePosition(file, Position(cursor)) - val (_, completions) = Interactive.completions(srcPos) + val (_, completions) = Completion.completions(srcPos) completions.map(makeCandidate) } .getOrElse(Nil) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index 4b875b0583c7..c6abf045c0db 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -281,7 +281,7 @@ class DottyLanguageServer extends LanguageServer val pos = sourcePosition(driver, uri, params.getPosition) val items = driver.compilationUnits.get(uri) match { - case Some(unit) => Interactive.completions(pos)(ctx.fresh.setCompilationUnit(unit))._2 + case Some(unit) => Completion.completions(pos)(ctx.fresh.setCompilationUnit(unit))._2 case None => Nil } From ace5092230482bfb99fc2f7e8bb05af2756eae11 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Fri, 23 Nov 2018 07:54:11 +0100 Subject: [PATCH 10/14] Fix completion without prefix When the user writes `qualifier.`, we get the tree corresponding to `qualifier.`. When encountering the error, we were not doing any completion, but we should consider both terms and types in this context. --- .../src/dotty/tools/dotc/interactive/Completion.scala | 4 ++-- .../test/dotty/tools/languageserver/CompletionTest.scala | 9 +++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Completion.scala b/compiler/src/dotty/tools/dotc/interactive/Completion.scala index a4e90cd68af2..b51cf522e417 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Completion.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Completion.scala @@ -45,7 +45,7 @@ object Completion { private def completionMode(path: List[Tree], pos: SourcePosition): Mode = { path match { case (ref: RefTree) :: _ => - if (ref.name == nme.ERROR) Mode.None + if (ref.name == nme.ERROR) Mode.Term | Mode.Type else if (ref.name.isTermName) Mode.Term else if (ref.name.isTypeName) Mode.Type else Mode.None @@ -301,7 +301,7 @@ object Completion { val Type: Mode = new Mode(2) /** Both term and type symbols are allowed */ - val Import: Mode = Term | Type + val Import: Mode = new Mode(4) | Term | Type } } diff --git a/language-server/test/dotty/tools/languageserver/CompletionTest.scala b/language-server/test/dotty/tools/languageserver/CompletionTest.scala index b91f3cfa3a35..d9c11f6002ed 100644 --- a/language-server/test/dotty/tools/languageserver/CompletionTest.scala +++ b/language-server/test/dotty/tools/languageserver/CompletionTest.scala @@ -166,4 +166,13 @@ class CompletionTest { |}""".withSource .completion(m1, Set()) } + + @Test def completeNoPrefix: Unit = { + code"""class Foo { def foo = 0 } + |object Bar { + | val foo = new Foo + | foo.${m1} + |}""".withSource + .completion(m1, results => assertTrue(results.nonEmpty)) + } } From 93212108a856fd012e52b8c7a1c52291ff3cebfa Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Fri, 23 Nov 2018 15:10:04 +0100 Subject: [PATCH 11/14] Extract name kind from `ERROR` The parser tells us whether the error happened in term or type position, so we can use this to provide better completion. --- .../dotty/tools/dotc/interactive/Completion.scala | 5 +++-- .../dotty/tools/languageserver/CompletionTest.scala | 13 +++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Completion.scala b/compiler/src/dotty/tools/dotc/interactive/Completion.scala index b51cf522e417..24d4363a8b79 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Completion.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Completion.scala @@ -11,7 +11,7 @@ import dotty.tools.dotc.core.NameKinds.SimpleNameKind import dotty.tools.dotc.core.NameOps.NameDecorator import dotty.tools.dotc.core.Symbols.{defn, NoSymbol, Symbol} import dotty.tools.dotc.core.Scopes -import dotty.tools.dotc.core.StdNames.nme +import dotty.tools.dotc.core.StdNames.{nme, tpnme} import dotty.tools.dotc.core.TypeError import dotty.tools.dotc.core.Types.{NamedType, NameFilter, Type, takeAllFilter} import dotty.tools.dotc.printing.Texts._ @@ -45,7 +45,8 @@ object Completion { private def completionMode(path: List[Tree], pos: SourcePosition): Mode = { path match { case (ref: RefTree) :: _ => - if (ref.name == nme.ERROR) Mode.Term | Mode.Type + if (ref.name == tpnme.ERROR) Mode.Type + else if (ref.name == nme.ERROR) Mode.Term else if (ref.name.isTermName) Mode.Term else if (ref.name.isTypeName) Mode.Type else Mode.None diff --git a/language-server/test/dotty/tools/languageserver/CompletionTest.scala b/language-server/test/dotty/tools/languageserver/CompletionTest.scala index d9c11f6002ed..34cb9b9e168e 100644 --- a/language-server/test/dotty/tools/languageserver/CompletionTest.scala +++ b/language-server/test/dotty/tools/languageserver/CompletionTest.scala @@ -175,4 +175,17 @@ class CompletionTest { |}""".withSource .completion(m1, results => assertTrue(results.nonEmpty)) } + + @Test def completeErrorKnowsKind: Unit = { + code"""object Bar { + | class Zig + | val Zag: Int = 0 + | val b = 3 + Bar.${m1} + |}""".withSource + .completion(m1, completionItems => { + val results = CodeCompletion.simplifyResults(completionItems) + assertTrue(results.contains(("Zag", Field, "Int"))) + assertFalse(results.exists((label, _, _) => label == "Zig")) + }) + } } From 5677dfc36dfeae7b59edcebdea069cc5d446fbdc Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Fri, 23 Nov 2018 15:47:04 +0100 Subject: [PATCH 12/14] Show stable terms when completing types Stable terms can be part of the path to a type, and should be shown in completion when writing a type. --- .../tools/dotc/interactive/Completion.scala | 20 ++++++++++++++++--- .../tools/languageserver/CompletionTest.scala | 11 ++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Completion.scala b/compiler/src/dotty/tools/dotc/interactive/Completion.scala index 24d4363a8b79..7221d4853303 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Completion.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Completion.scala @@ -3,6 +3,7 @@ package dotty.tools.dotc.interactive import dotty.tools.dotc.ast.Trees._ import dotty.tools.dotc.config.Printers.interactiv import dotty.tools.dotc.core.Contexts.{Context, NoContext} +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._ @@ -13,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, NameFilter, Type, takeAllFilter} +import dotty.tools.dotc.core.Types.{NamedType, Type, takeAllFilter} import dotty.tools.dotc.printing.Texts._ import dotty.tools.dotc.util.{NoSourcePosition, SourcePosition} @@ -219,9 +220,20 @@ object Completion { !sym.is(allOf(Mutable, Accessor)) && ( (mode.is(Mode.Term) && sym.isTerm) + || (mode.is(Mode.StableTerm) && sym.isTerm && validPathSegment(sym)) || (mode.is(Mode.Type) && sym.isType) ) + /** Can this symbol be part of a path? See SLS 3.1 for a definition of a valid path. */ + private def validPathSegment(sym: Symbol)(implicit ctx: Context): Boolean = { + def isRealizable = { + val realizability = CheckRealizable.realizability(sym.info) + realizability == CheckRealizable.Realizable + } + + !sym.is(Method) && isRealizable + } + /** * Find all the members of `site` that are accessible and which should be included in `info`. * @@ -298,11 +310,13 @@ object Completion { /** Term symbols are allowed */ val Term: Mode = new Mode(1) + val StableTerm: Mode = new Mode(2) + /** Type symbols are allowed */ - val Type: Mode = new Mode(2) + val Type: Mode = new Mode(4) | StableTerm /** Both term and type symbols are allowed */ - val Import: Mode = new Mode(4) | Term | Type + val Import: Mode = new Mode(8) | Term | Type } } diff --git a/language-server/test/dotty/tools/languageserver/CompletionTest.scala b/language-server/test/dotty/tools/languageserver/CompletionTest.scala index 34cb9b9e168e..6a4dc94c8d5d 100644 --- a/language-server/test/dotty/tools/languageserver/CompletionTest.scala +++ b/language-server/test/dotty/tools/languageserver/CompletionTest.scala @@ -188,4 +188,15 @@ class CompletionTest { assertFalse(results.exists((label, _, _) => label == "Zig")) }) } + + @Test def typeCompletionShowsTerm: Unit = { + code"""class Bar + |object Foo { + | val bar = new Bar + | def baz = new Bar + | object bat + | val bizz: ba${m1} + |}""".withSource + .completion(m1, Set(("bar", Field, "Bar"), ("bat", Module, "Foo.bat"))) + } } From a74621546d96ede91b85b42584f091e3046b7d92 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Fri, 23 Nov 2018 16:09:41 +0100 Subject: [PATCH 13/14] Address review comments --- .../tools/dotc/interactive/Completion.scala | 25 ++++--------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Completion.scala b/compiler/src/dotty/tools/dotc/interactive/Completion.scala index 7221d4853303..640d05fc9d04 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Completion.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Completion.scala @@ -46,9 +46,7 @@ object Completion { private def completionMode(path: List[Tree], pos: SourcePosition): Mode = { path match { case (ref: RefTree) :: _ => - if (ref.name == tpnme.ERROR) Mode.Type - else if (ref.name == nme.ERROR) Mode.Term - else if (ref.name.isTermName) Mode.Term + if (ref.name.isTermName) Mode.Term else if (ref.name.isTypeName) Mode.Type else Mode.None @@ -220,20 +218,9 @@ object Completion { !sym.is(allOf(Mutable, Accessor)) && ( (mode.is(Mode.Term) && sym.isTerm) - || (mode.is(Mode.StableTerm) && sym.isTerm && validPathSegment(sym)) - || (mode.is(Mode.Type) && sym.isType) + || (mode.is(Mode.Type) && (sym.isType || sym.isStable)) ) - /** Can this symbol be part of a path? See SLS 3.1 for a definition of a valid path. */ - private def validPathSegment(sym: Symbol)(implicit ctx: Context): Boolean = { - def isRealizable = { - val realizability = CheckRealizable.realizability(sym.info) - realizability == CheckRealizable.Realizable - } - - !sym.is(Method) && isRealizable - } - /** * Find all the members of `site` that are accessible and which should be included in `info`. * @@ -310,13 +297,11 @@ object Completion { /** Term symbols are allowed */ val Term: Mode = new Mode(1) - val StableTerm: Mode = new Mode(2) - - /** Type symbols are allowed */ - val Type: Mode = new Mode(4) | StableTerm + /** Type and stable term symbols are allowed */ + val Type: Mode = new Mode(2) /** Both term and type symbols are allowed */ - val Import: Mode = new Mode(8) | Term | Type + val Import: Mode = new Mode(4) | Term | Type } } From 89f850a750f237086cd69f4bdac07af3a6fd9f37 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Fri, 23 Nov 2018 16:29:23 +0100 Subject: [PATCH 14/14] Always remove duplicates in completion --- .../dotty/tools/dotc/interactive/Completion.scala | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Completion.scala b/compiler/src/dotty/tools/dotc/interactive/Completion.scala index 640d05fc9d04..5fa83ef229df 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Completion.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Completion.scala @@ -135,14 +135,11 @@ object Completion { * preferred over term symbols. */ def getCompletions(implicit ctx: Context): List[Symbol] = { - if (!mode.is(Mode.Import)) completions.toList - else { - // In imports, 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 - } + // 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 } /**