Skip to content

Commit 02497d4

Browse files
committed
[Completions] Fix duplicated methods from implicit conversions
Fixes the following case: ```scala Array(1,2,3).fil@@ ``` Before this fix there were several `filter` and `filterNot` methods in completions they were taken from all eligible implicit candidates. Now there is a kind of ranking per denotation.
1 parent 1f48de1 commit 02497d4

File tree

4 files changed

+54
-32
lines changed

4 files changed

+54
-32
lines changed

compiler/src/dotty/tools/dotc/interactive/Completion.scala

Lines changed: 34 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import dotty.tools.dotc.core.Flags._
1313
import dotty.tools.dotc.core.Names.{Name, TermName}
1414
import dotty.tools.dotc.core.NameKinds.SimpleNameKind
1515
import dotty.tools.dotc.core.NameOps._
16+
import dotty.tools.dotc.core.Signature
1617
import dotty.tools.dotc.core.Symbols.{NoSymbol, Symbol, TermSymbol, defn, newSymbol}
1718
import dotty.tools.dotc.core.Scopes.Scope
1819
import dotty.tools.dotc.core.StdNames.{nme, tpnme}
@@ -21,6 +22,8 @@ import dotty.tools.dotc.core.TypeComparer
2122
import dotty.tools.dotc.core.TypeError
2223
import dotty.tools.dotc.core.Types.{ExprType, MethodOrPoly, NameFilter, NamedType, NoType, PolyType, TermRef, Type}
2324
import dotty.tools.dotc.printing.Texts._
25+
import dotty.tools.dotc.typer.Implicits
26+
import dotty.tools.dotc.typer.Implicits.SearchSuccess
2427
import dotty.tools.dotc.util.{NameTransformer, NoSourcePosition, SourcePosition}
2528

2629
import scala.collection.mutable
@@ -115,7 +118,7 @@ object Completion {
115118
// Ignore synthetic select from `This` because in code it was `Ident`
116119
// See example in dotty.tools.languageserver.CompletionTest.syntheticThis
117120
case Select(qual @ This(_), _) :: _ if qual.span.isSynthetic => completer.scopeCompletions
118-
case Select(qual, _) :: _ if qual.tpe.hasSimpleKind => completer.selectionCompletions(qual)
121+
case (sel @ Select(qual, _)) :: _ if qual.tpe.hasSimpleKind => completer.selectionCompletions(sel)
119122
case Select(qual, _) :: _ => Map.empty
120123
case Import(expr, _) :: _ => completer.directMemberCompletions(expr)
121124
case (_: untpd.ImportSelector) :: Import(expr, _) :: _ => completer.directMemberCompletions(expr)
@@ -230,10 +233,10 @@ object Completion {
230233
* Direct members take priority over members from extensions
231234
* and so do members from extensions over members from implicit conversions
232235
*/
233-
def selectionCompletions(qual: Tree)(using Context): CompletionMap =
234-
implicitConversionMemberCompletions(qual) ++
235-
extensionCompletions(qual) ++
236-
directMemberCompletions(qual)
236+
def selectionCompletions(sel: Select)(using Context): CompletionMap =
237+
implicitConversionMemberCompletions(sel) ++
238+
extensionCompletions(sel.qualifier) ++
239+
directMemberCompletions(sel.qualifier)
237240

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

287290
/** Completions from implicit conversions including old style extensions using implicit classes */
288-
private def implicitConversionMemberCompletions(qual: Tree)(using Context): CompletionMap =
291+
private def implicitConversionMemberCompletions(sel: Select)(using Context): CompletionMap =
292+
val qual = sel.qualifier
289293
if qual.tpe.widenDealias.isExactlyNothing || qual.tpe.isNullType then
290294
Map.empty
291295
else
296+
// Take all possible conversions for `qual`
297+
val typer = ctx.typer
298+
val searchContext = ctx.fresh.setExploreTyperState()
299+
val search = new typer.ImplicitSearch(defn.AnyType, qual, pos.span)(using searchContext)
292300
val membersFromConversion =
293-
implicitConversionTargets(qual)(using ctx.fresh.setExploreTyperState()).flatMap(accessibleMembers)
294-
membersFromConversion.toSeq.groupByName
301+
search.allImplicits.flatMap(r => accessibleMembers(r.ref.widen.finalResultType).map(_ -> r))
302+
303+
// There might be more that one conversions that have members with the same name and signature
304+
// In order to avoid duplicate completions a kind of ranking (per Name + ParameterSig) is performed
305+
type Key = (Name, List[Signature.ParamSig])
306+
val init = Map.empty[Key, (SingleDenotation, SearchSuccess)]
307+
membersFromConversion.foldLeft(init) { case (acc, (rawDenot, found)) =>
308+
val denot = rawDenot.asSeenFrom(found.tree.tpe)
309+
val sig = denot.info.signature
310+
val key = (denot.name, sig.paramsSig)
311+
acc.get(key) match {
312+
case None => acc.updated(key, (denot, found))
313+
case Some((_, owner)) if search.compareAlternatives(found, owner) > 0 =>
314+
acc.updated(key, (denot, found))
315+
case Some(_) =>
316+
acc
317+
}
318+
}.map {case (_, (denot, _)) => denot}
319+
.toSeq
320+
.groupByName
295321

296322
/** Completions from extension methods */
297323
private def extensionCompletions(qual: Tree)(using Context): CompletionMap =
@@ -394,21 +420,6 @@ object Completion {
394420
}
395421
}
396422

397-
/**
398-
* Given `qual` of type T, finds all the types S such that there exists an implicit conversion
399-
* from T to S.
400-
*
401-
* @param qual The argument to which the implicit conversion should be applied.
402-
* @return The set of types that `qual` can be converted to.
403-
*/
404-
private def implicitConversionTargets(qual: Tree)(using Context): Set[Type] = {
405-
val typer = ctx.typer
406-
val conversions = new typer.ImplicitSearch(defn.AnyType, qual, pos.span).allImplicits
407-
val targets = conversions.map(_.widen.finalResultType)
408-
interactiv.println(i"implicit conversion targets considered: ${targets.toList}%, %")
409-
targets
410-
}
411-
412423
/** Filter for names that should appear when looking for completions. */
413424
private object completionsFilter extends NameFilter {
414425
def apply(pre: Type, name: Name)(using Context): Boolean =

compiler/src/dotty/tools/dotc/typer/Implicits.scala

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1168,6 +1168,11 @@ trait Implicits:
11681168
typingCtx.typerState.gc()
11691169
result
11701170

1171+
def compareAlternatives(alt1: RefAndLevel, alt2: RefAndLevel): Int =
1172+
if alt1.ref eq alt2.ref then 0
1173+
else if alt1.level != alt2.level then alt1.level - alt2.level
1174+
else explore(compare(alt1.ref, alt2.ref))(using nestedContext())
1175+
11711176
/** Search a list of eligible implicit references */
11721177
private def searchImplicit(eligible: List[Candidate], contextual: Boolean): SearchResult =
11731178

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

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

14871488
/** All available implicits, without ranking */
1488-
def allImplicits: Set[TermRef] = {
1489+
def allImplicits: Set[SearchSuccess] = {
14891490
val contextuals = ctx.implicits.eligible(wildProto).map(tryImplicit(_, contextual = true))
14901491
val inscope = implicitScope(wildProto).eligible.map(tryImplicit(_, contextual = false))
1491-
(contextuals.toSet ++ inscope).collect {
1492-
case success: SearchSuccess => success.ref
1493-
}
1492+
(contextuals ++ inscope).collect {
1493+
case success: SearchSuccess => success
1494+
}.toSet
14941495
}
14951496

14961497
/** Fields needed for divergence checking */

compiler/test/dotty/tools/dotc/interactive/CustomCompletion.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ object CustomCompletion {
3737
var extra = List.empty[Completion]
3838

3939
val completions = path match {
40-
case Select(qual, _) :: _ => completer.selectionCompletions(qual)
40+
case (sel : Select) :: _ => completer.selectionCompletions(sel)
4141
case Import(Ident(name), _) :: _ if name.decode.toString == "$ivy" && dependencyCompleteOpt.nonEmpty =>
4242
val complete = dependencyCompleteOpt.get
4343
val (pos, completions) = complete(prefix)

language-server/test/dotty/tools/languageserver/CompletionTest.scala

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -976,4 +976,14 @@ class CompletionTest {
976976
("main", Module, "main")
977977
)
978978
)
979+
980+
@Test def singleConversionInstancePerMethod : Unit =
981+
code"""val a = Array(1,2).fil${m1}"""
982+
.withSource
983+
.completion(m1,
984+
Set(
985+
("filterNot", Method, "(p: Int => Boolean): Array[Int]"),
986+
("filter", Method, "(p: Int => Boolean): Array[Int]")
987+
)
988+
)
979989
}

0 commit comments

Comments
 (0)