Rank suggestions by self-type specificity (enso-org#1629)

MULXCODE · Mar 29, 2021 · 8e942b5 · 8e942b5
1 parent a877e39
commit 8e942b5
Show file tree

Hide file tree

Showing 7 changed files with 41 additions and 24 deletions.
diff --git a/docs/language-server/protocol-language-server.md b/docs/language-server/protocol-language-server.md
@@ -3232,6 +3232,9 @@ Sent from client to the server to receive the autocomplete suggestion.
 
 #### Result
 
+The identifiers in `results` are guaranteed to be ordered by the specificity of
+the type match.
+
 ```typescript
 {
   results: [SuggestionId];

diff --git a/...ne/language-server/src/main/scala/org/enso/languageserver/search/SuggestionsHandler.scala b/...ne/language-server/src/main/scala/org/enso/languageserver/search/SuggestionsHandler.scala
@@ -237,8 +237,7 @@ final class SuggestionsHandler(
         .pipeTo(sender())
 
     case Completion(path, pos, selfType, returnType, tags) =>
-      val selfTypes =
-        selfType.toList.flatMap(ty => (graph.getParents(ty) + ty).toSeq)
+      val selfTypes = selfType.toList.flatMap(ty => ty :: graph.getParents(ty))
       getModuleName(projectName, path)
         .fold(
           Future.successful,

diff --git a/...anguage-server/src/test/scala/org/enso/languageserver/search/SuggestionsHandlerSpec.scala b/...anguage-server/src/test/scala/org/enso/languageserver/search/SuggestionsHandlerSpec.scala
@@ -528,7 +528,7 @@ class SuggestionsHandlerSpec
         expectMsg(
           SearchProtocol.CompletionResult(
             7L,
-            Seq(methodOnAnyId, methodId).flatten
+            Seq(methodId, methodOnAnyId).flatten
           )
         )
     }
@@ -549,7 +549,7 @@ class SuggestionsHandlerSpec
         expectMsg(
           SearchProtocol.CompletionResult(
             7L,
-            Seq(anyMethodId, integerMethodId, numberMethodId).flatten
+            Seq(integerMethodId, numberMethodId, anyMethodId).flatten
           )
         )
     }

diff --git a/engine/polyglot-api/src/main/scala/org/enso/polyglot/data/TypeGraph.scala b/engine/polyglot-api/src/main/scala/org/enso/polyglot/data/TypeGraph.scala
@@ -63,21 +63,25 @@ case class TypeGraph(
   }
 
   /** Get all of the parents (transitively) of the provided typename.
+    *
+    * The resultant types are ordered by specificity, with the more specific
+    * types coming before the less specific types.
     *
     * @param typeName the fully-qualified type name for which to get the parents
-    * @return all parents of `typeName`
+    * @return all parents of `typeName`, ordered by specificity
     */
   @JsonIgnore
-  def getParents(typeName: String): Set[String] = {
+  def getParents(typeName: String): List[String] = {
     var seenNodes: Set[String] = Set()
 
-    val parents = getDirectParents(typeName)
-    parents ++ parents.flatMap(parent => {
+    val parents = List.from(getDirectParents(typeName))
+    val recur = parents ++ parents.flatMap(parent => {
       if (!seenNodes.contains(parent)) {
         seenNodes += parent
         getParents(parent)
       } else { Set() }
     })
+    recur.distinct
   }
 }
 object TypeGraph {

diff --git a/engine/polyglot-api/src/test/scala/org/enso/polyglot/data/TypeGraphTest.scala b/engine/polyglot-api/src/test/scala/org/enso/polyglot/data/TypeGraphTest.scala
@@ -37,15 +37,15 @@ class TypeGraphTest extends AnyWordSpec with Matchers {
       graph.insert("Builtins.Main.Decimal", "Builtins.Main.Number")
       graph.insert("Builtins.Main.Integer", "Builtins.Main.Number")
 
-      graph.getParents("Builtins.Main.Any") shouldEqual Set()
-      graph.getParents("Builtins.Main.Number") shouldEqual Set(
+      graph.getParents("Builtins.Main.Any") shouldEqual List()
+      graph.getParents("Builtins.Main.Number") shouldEqual List(
         "Builtins.Main.Any"
       )
-      graph.getParents("Builtins.Main.Integer") shouldEqual Set(
+      graph.getParents("Builtins.Main.Integer") shouldEqual List(
         "Builtins.Main.Number",
         "Builtins.Main.Any"
       )
-      graph.getParents("Builtins.Main.Decimal") shouldEqual Set(
+      graph.getParents("Builtins.Main.Decimal") shouldEqual List(
         "Builtins.Main.Number",
         "Builtins.Main.Any"
       )
@@ -57,8 +57,8 @@ class TypeGraphTest extends AnyWordSpec with Matchers {
       graph.insert("Builtins.Main.Decimal", "Builtins.Main.Number")
       graph.insert("Builtins.Main.Integer", "Builtins.Main.Number")
 
-      graph.getParents("My_User_Type") shouldEqual Set("Builtins.Main.Any")
-      graph.getParents("Standard.Base.Vector") shouldEqual Set(
+      graph.getParents("My_User_Type") shouldEqual List("Builtins.Main.Any")
+      graph.getParents("Standard.Base.Vector") shouldEqual List(
         "Builtins.Main.Any"
       )
     }

diff --git a/lib/scala/searcher/src/main/scala/org/enso/searcher/SuggestionsRepo.scala b/lib/scala/searcher/src/main/scala/org/enso/searcher/SuggestionsRepo.scala
@@ -34,11 +34,13 @@ trait SuggestionsRepo[F[_]] {
   /** Search suggestion by various parameters.
     *
     * @param module the module name search parameter
-    * @param selfType the self types to search for
+    * @param selfType the self types to search for, ordered by specificity with
+    *                 the most specific type first
     * @param returnType the returnType search parameter
     * @param kinds the list suggestion kinds to search
     * @param position the absolute position in the text
-    * @return the current database version and the list of found suggestion ids
+    * @return the current database version and the list of found suggestion ids,
+    *         ranked by specificity
     */
   def search(
     module: Option[String],

diff --git a/lib/scala/searcher/src/main/scala/org/enso/searcher/sql/SqlSuggestionsRepo.scala b/lib/scala/searcher/src/main/scala/org/enso/searcher/sql/SqlSuggestionsRepo.scala
@@ -1,8 +1,5 @@
 package org.enso.searcher.sql
 
-import java.io.File
-import java.util.UUID
-
 import org.enso.polyglot.Suggestion
 import org.enso.polyglot.data.Tree
 import org.enso.polyglot.runtime.Runtime.Api.{
@@ -16,6 +13,9 @@ import org.enso.searcher.{SuggestionEntry, SuggestionsRepo}
 import slick.jdbc.SQLiteProfile
 import slick.jdbc.SQLiteProfile.api._
 
+import java.io.File
+import java.util.UUID
+import scala.collection.immutable.HashMap
 import scala.concurrent.{ExecutionContext, Future}
 import scala.util.{Failure, Success}
 
@@ -211,11 +211,13 @@ final class SqlSuggestionsRepo(db: SqlDatabase)(implicit ec: ExecutionContext)
   /** The query to search suggestion by various parameters.
     *
     * @param module the module name search parameter
-    * @param selfType the selfType search parameter
+    * @param selfType the selfType search parameter, ordered by specificity
+    *                 with the most specific type first
     * @param returnType the returnType search parameter
     * @param kinds the list suggestion kinds to search
     * @param position the absolute position in the text
-    * @return the list of suggestion ids
+    * @return the list of suggestion ids, ranked by specificity (as for
+    *         `selfType`)
     */
   private def searchQuery(
     module: Option[String],
@@ -224,6 +226,7 @@ final class SqlSuggestionsRepo(db: SqlDatabase)(implicit ec: ExecutionContext)
     kinds: Option[Seq[Suggestion.Kind]],
     position: Option[Suggestion.Position]
   ): DBIO[(Long, Seq[Long])] = {
+    val typeSorterMap: HashMap[String, Int] = HashMap(selfType.zipWithIndex: _*)
     val searchAction =
       if (
         module.isEmpty &&
@@ -236,11 +239,17 @@ final class SqlSuggestionsRepo(db: SqlDatabase)(implicit ec: ExecutionContext)
       } else {
         val query =
           searchQueryBuilder(module, selfType, returnType, kinds, position)
-            .map(_.id)
+            .map(r => (r.id, r.selfType))
         query.result
       }
     val query = for {
-      results <- searchAction
+      resultsWithTypes <- searchAction
+      // This implementation should be revisited if it ever becomes a
+      // performance bottleneck. It may be possible to encode the same logic in
+      // the database query itself.
+      results = resultsWithTypes
+        .sortBy { case (_, ty) => typeSorterMap.getOrElse(ty, -1) }
+        .map(_._1)
       version <- currentVersionQuery
     } yield (version, results)
     query
@@ -673,7 +682,7 @@ final class SqlSuggestionsRepo(db: SqlDatabase)(implicit ec: ExecutionContext)
       .filterOpt(module) { case (row, value) =>
         row.scopeStartLine === ScopeColumn.EMPTY || row.module === value
       }
-      .filterIf(selfTypes.nonEmpty) {row => row.selfType.inSet(selfTypes) }
+      .filterIf(selfTypes.nonEmpty) { row => row.selfType.inSet(selfTypes) }
       .filterOpt(returnType) { case (row, value) =>
         row.returnType === value
       }