Skip to content

[Completions] Fix duplicated methods from implicit conversions #14036

New issue

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

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

Already on GitHub? Sign in to your account

Closed
Closed
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
57 changes: 34 additions & 23 deletions compiler/src/dotty/tools/dotc/interactive/Completion.scala
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ 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._
import dotty.tools.dotc.core.Signature
import dotty.tools.dotc.core.Symbols.{NoSymbol, Symbol, TermSymbol, defn, newSymbol}
import dotty.tools.dotc.core.Scopes.Scope
import dotty.tools.dotc.core.StdNames.{nme, tpnme}
Expand All @@ -21,6 +22,8 @@ import dotty.tools.dotc.core.TypeComparer
import dotty.tools.dotc.core.TypeError
import dotty.tools.dotc.core.Types.{ExprType, MethodOrPoly, NameFilter, NamedType, NoType, PolyType, TermRef, Type}
import dotty.tools.dotc.printing.Texts._
import dotty.tools.dotc.typer.Implicits
import dotty.tools.dotc.typer.Implicits.SearchSuccess
import dotty.tools.dotc.util.{NameTransformer, NoSourcePosition, SourcePosition}

import scala.collection.mutable
Expand Down Expand Up @@ -115,7 +118,7 @@ object Completion {
// Ignore synthetic select from `This` because in code it was `Ident`
// See example in dotty.tools.languageserver.CompletionTest.syntheticThis
case Select(qual @ This(_), _) :: _ if qual.span.isSynthetic => completer.scopeCompletions
case Select(qual, _) :: _ if qual.tpe.hasSimpleKind => completer.selectionCompletions(qual)
case (sel @ Select(qual, _)) :: _ if qual.tpe.hasSimpleKind => completer.selectionCompletions(sel)
case Select(qual, _) :: _ => Map.empty
case Import(expr, _) :: _ => completer.directMemberCompletions(expr)
case (_: untpd.ImportSelector) :: Import(expr, _) :: _ => completer.directMemberCompletions(expr)
Expand Down Expand Up @@ -230,10 +233,10 @@ object Completion {
* Direct members take priority over members from extensions
* and so do members from extensions over members from implicit conversions
*/
def selectionCompletions(qual: Tree)(using Context): CompletionMap =
implicitConversionMemberCompletions(qual) ++
extensionCompletions(qual) ++
directMemberCompletions(qual)
def selectionCompletions(sel: Select)(using Context): CompletionMap =
implicitConversionMemberCompletions(sel) ++
extensionCompletions(sel.qualifier) ++
directMemberCompletions(sel.qualifier)

/** Completions for members of `qual`'s type.
* These include inherited definitions but not members added by extensions or implicit conversions
Expand Down Expand Up @@ -285,13 +288,36 @@ object Completion {
}

/** Completions from implicit conversions including old style extensions using implicit classes */
private def implicitConversionMemberCompletions(qual: Tree)(using Context): CompletionMap =
private def implicitConversionMemberCompletions(sel: Select)(using Context): CompletionMap =
val qual = sel.qualifier
if qual.tpe.widenDealias.isExactlyNothing || qual.tpe.isNullType then
Map.empty
else
// Take all possible conversions for `qual`
val typer = ctx.typer
val searchContext = ctx.fresh.setExploreTyperState()
val search = new typer.ImplicitSearch(defn.AnyType, qual, pos.span)(using searchContext)
val membersFromConversion =
implicitConversionTargets(qual)(using ctx.fresh.setExploreTyperState()).flatMap(accessibleMembers)
membersFromConversion.toSeq.groupByName
search.allImplicits.flatMap(r => accessibleMembers(r.ref.widen.finalResultType).map(_ -> r))

// There might be more that one conversions that have members with the same name and signature
// In order to avoid duplicate completions a kind of ranking (per Name + ParameterSig) is performed
type Key = (Name, List[Signature.ParamSig])
val init = Map.empty[Key, (SingleDenotation, SearchSuccess)]
membersFromConversion.foldLeft(init) { case (acc, (rawDenot, found)) =>
val denot = rawDenot.asSeenFrom(found.tree.tpe)
val sig = denot.info.signature
val key = (denot.name, sig.paramsSig)
acc.get(key) match {
case None => acc.updated(key, (denot, found))
case Some((_, owner)) if search.compareAlternatives(found, owner) > 0 =>
acc.updated(key, (denot, found))
case Some(_) =>
acc
}
}.map {case (_, (denot, _)) => denot}
.toSeq
.groupByName

/** Completions from extension methods */
private def extensionCompletions(qual: Tree)(using Context): CompletionMap =
Expand Down Expand Up @@ -394,21 +420,6 @@ object Completion {
}
}

/**
* 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)(using Context): Set[Type] = {
val typer = ctx.typer
val conversions = new typer.ImplicitSearch(defn.AnyType, qual, pos.span).allImplicits
val targets = conversions.map(_.widen.finalResultType)
interactiv.println(i"implicit conversion targets considered: ${targets.toList}%, %")
targets
}

/** Filter for names that should appear when looking for completions. */
private object completionsFilter extends NameFilter {
def apply(pre: Type, name: Name)(using Context): Boolean =
Expand Down
17 changes: 9 additions & 8 deletions compiler/src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1168,6 +1168,11 @@ trait Implicits:
typingCtx.typerState.gc()
result

def compareAlternatives(alt1: RefAndLevel, alt2: RefAndLevel): Int =
if alt1.ref eq alt2.ref then 0
else if alt1.level != alt2.level then alt1.level - alt2.level
else explore(compare(alt1.ref, alt2.ref))(using nestedContext())

/** Search a list of eligible implicit references */
private def searchImplicit(eligible: List[Candidate], contextual: Boolean): SearchResult =

Expand All @@ -1177,10 +1182,6 @@ trait Implicits:
* a number < 0 if `alt2` is preferred over `alt1`
* 0 if neither alternative is preferred over the other
*/
def compareAlternatives(alt1: RefAndLevel, alt2: RefAndLevel): Int =
if alt1.ref eq alt2.ref then 0
else if alt1.level != alt2.level then alt1.level - alt2.level
else explore(compare(alt1.ref, alt2.ref))(using nestedContext())

/** If `alt1` is also a search success, try to disambiguate as follows:
* - If alt2 is preferred over alt1, pick alt2, otherwise return an
Expand Down Expand Up @@ -1485,12 +1486,12 @@ trait Implicits:
def implicitScope(tp: Type): OfTypeImplicits = ctx.run.implicitScope(tp)

/** All available implicits, without ranking */
def allImplicits: Set[TermRef] = {
def allImplicits: Set[SearchSuccess] = {
val contextuals = ctx.implicits.eligible(wildProto).map(tryImplicit(_, contextual = true))
val inscope = implicitScope(wildProto).eligible.map(tryImplicit(_, contextual = false))
(contextuals.toSet ++ inscope).collect {
case success: SearchSuccess => success.ref
}
(contextuals ++ inscope).collect {
case success: SearchSuccess => success
}.toSet
}

/** Fields needed for divergence checking */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ object CustomCompletion {
var extra = List.empty[Completion]

val completions = path match {
case Select(qual, _) :: _ => completer.selectionCompletions(qual)
case (sel : Select) :: _ => completer.selectionCompletions(sel)
case Import(Ident(name), _) :: _ if name.decode.toString == "$ivy" && dependencyCompleteOpt.nonEmpty =>
val complete = dependencyCompleteOpt.get
val (pos, completions) = complete(prefix)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -976,4 +976,14 @@ class CompletionTest {
("main", Module, "main")
)
)

@Test def singleConversionInstancePerMethod : Unit =
code"""val a = Array(1,2).fil${m1}"""
.withSource
.completion(m1,
Set(
("filterNot", Method, "(p: Int => Boolean): Array[Int]"),
("filter", Method, "(p: Int => Boolean): Array[Int]")
)
)
}