Fix #5460: Improve completion of import nodes

Duhemm · Duhemm · commit 43455e4bbe22 · 2018-11-23T08:26:46.000+01:00
diff --git 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
@@ -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
@@ -1,22 +1,119 @@
 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._
 
 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 = {
     withSources(
       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
@@ -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
@@ -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)"
 }
