Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Rank Suggestions by Self-Type Specificity #1629

Merged
merged 4 commits into from
Mar 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/language-server/protocol-language-server.md
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ class SuggestionsHandlerSpec
expectMsg(
SearchProtocol.CompletionResult(
7L,
Seq(methodOnAnyId, methodId).flatten
Seq(methodId, methodOnAnyId).flatten
)
)
}
Expand All @@ -549,7 +549,7 @@ class SuggestionsHandlerSpec
expectMsg(
SearchProtocol.CompletionResult(
7L,
Seq(anyMethodId, integerMethodId, numberMethodId).flatten
Seq(integerMethodId, numberMethodId, anyMethodId).flatten
)
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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"
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down
Original file line number Diff line number Diff line change
@@ -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.{
Expand All @@ -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}

Expand Down Expand Up @@ -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],
Expand All @@ -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 &&
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand Down