Fix #3979: Completion for renamed imports

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}
@@ -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)
@@ -126,20 +137,43 @@ 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.
      *
-     * 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 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)
+      }
+    }
+
+    /**
+     * A description for completion result that represents `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.
      */
-    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(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 =>
+          ""
+      }
     }
 
     /**
@@ -150,11 +184,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
 
@@ -185,32 +219,35 @@ 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
      *   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
      *  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.sourceSymbol.exists &&
       (!sym.is(Package) || !sym.moduleClass.exists) &&
       !sym.is(allOf(Mutable, Accessor)) &&
       (
@@ -226,20 +263,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,
@@ -248,17 +285,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)
       }
     }
 
@@ -301,4 +339,31 @@ 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 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 = {
+       nameToSymbols += name -> (sym :: nameToSymbols.getOrElse(name, 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))
+     }
+   }
+
 }

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

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"))
+    }
 }

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
   }
 

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 = {
@@ -199,4 +199,59 @@ 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, "class and object 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, "class and object 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, "class and object HashMap"),
+                          ("HashMap2", Class, "class and object 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, "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")))
+  }
 }