Skip to content

Commit 92de34c

Browse files
rochalarochala
committed
Fix ordering and deduplication of methods coming from traits
1 parent 64a02c1 commit 92de34c

File tree

10 files changed

+330
-202
lines changed

10 files changed

+330
-202
lines changed

‎presentation-compiler/src/main/dotty/tools/pc/completions/CompletionProvider.scala

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import org.eclipse.lsp4j.InsertTextFormat
3232
import org.eclipse.lsp4j.InsertTextMode
3333
import org.eclipse.lsp4j.Range as LspRange
3434
import org.eclipse.lsp4j.TextEdit
35+
import dotty.tools.dotc.cc.CaptureSet.empty
3536

3637
class CompletionProvider(
3738
search: SymbolSearch,
@@ -153,13 +154,17 @@ class CompletionProvider(
153154
val printer =
154155
ShortenedTypePrinter(search, IncludeDefaultParam.ResolveLater)(using indexedContext)
155156

157+
val underlyingCompletion = completion match
158+
case CompletionValue.ExtraMethod(_, underlying) => underlying
159+
case other => other
160+
156161
// For overloaded signatures we get multiple symbols, so we need
157162
// to recalculate the description
158163
// related issue https://github.com/lampepfl/dotty/issues/11941
159-
lazy val kind: CompletionItemKind = completion.completionItemKind
160-
val description = completion.description(printer)
161-
val label = completion.labelWithDescription(printer)
162-
val ident = completion.insertText.getOrElse(completion.label)
164+
lazy val kind: CompletionItemKind = underlyingCompletion.completionItemKind
165+
val description = underlyingCompletion.description(printer)
166+
val label = underlyingCompletion.labelWithDescription(printer)
167+
val ident = underlyingCompletion.insertText.getOrElse(underlyingCompletion.label)
163168

164169
lazy val isInStringInterpolation =
165170
path match
@@ -176,7 +181,7 @@ class CompletionProvider(
176181
false
177182

178183
def wrapInBracketsIfRequired(newText: String): String =
179-
if completion.snippetAffix.nonEmpty && isInStringInterpolation then
184+
if underlyingCompletion.snippetAffix.nonEmpty && isInStringInterpolation then
180185
"{" + newText + "}"
181186
else newText
182187

@@ -194,29 +199,29 @@ class CompletionProvider(
194199
val item = new CompletionItem(label)
195200
item.setSortText(f"${idx}%05d")
196201
item.setDetail(description)
197-
item.setFilterText(completion.filterText.getOrElse(completion.label))
202+
item.setFilterText(underlyingCompletion.filterText.getOrElse(underlyingCompletion.label))
198203
item.setTextEdit(textEdit)
199-
item.setAdditionalTextEdits((completion.additionalEdits ++ additionalEdits).asJava)
200-
completion.insertMode.foreach(item.setInsertTextMode)
204+
item.setAdditionalTextEdits((underlyingCompletion.additionalEdits ++ additionalEdits).asJava)
205+
underlyingCompletion.insertMode.foreach(item.setInsertTextMode)
201206

202-
val data = completion.completionData(buildTargetIdentifier)
207+
val data = underlyingCompletion.completionData(buildTargetIdentifier)
203208
item.setData(data.toJson)
204209

205-
item.setTags(completion.lspTags.asJava)
210+
item.setTags(underlyingCompletion.lspTags.asJava)
206211

207212
if config.isCompletionSnippetsEnabled() then
208213
item.setInsertTextFormat(InsertTextFormat.Snippet)
209214

210-
completion.command.foreach { command =>
215+
underlyingCompletion.command.foreach { command =>
211216
item.setCommand(new Command("", command))
212217
}
213218

214219
item.setKind(kind)
215220
item
216221
end mkItem
217222

218-
val completionTextSuffix = completion.snippetAffix.toSuffix
219-
val completionTextPrefix = completion.snippetAffix.toInsertPrefix
223+
val completionTextSuffix = underlyingCompletion.snippetAffix.toSuffix
224+
val completionTextPrefix = underlyingCompletion.snippetAffix.toInsertPrefix
220225

221226
lazy val backtickSoftKeyword = path match
222227
case (_: Select) :: _ => false
@@ -276,15 +281,15 @@ class CompletionProvider(
276281
end match
277282
end mkItemWithImports
278283

279-
completion match
284+
underlyingCompletion match
280285
case v: (CompletionValue.Workspace | CompletionValue.Extension | CompletionValue.ImplicitClass) =>
281286
mkItemWithImports(v)
282287
case v: CompletionValue.Interpolator if v.isWorkspace || v.isExtension =>
283288
mkItemWithImports(v)
284289
case _ =>
285-
val nameText = completion.insertText.getOrElse(ident.backticked(backtickSoftKeyword))
290+
val nameText = underlyingCompletion.insertText.getOrElse(ident.backticked(backtickSoftKeyword))
286291
val nameWithAffixes = completionTextPrefix + nameText + completionTextSuffix
287-
mkItem(nameWithAffixes, range = completion.range)
292+
mkItem(nameWithAffixes, range = underlyingCompletion.range)
288293

289294
end match
290295
end completionItems

‎presentation-compiler/src/main/dotty/tools/pc/completions/CompletionValue.scala

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,38 @@ object CompletionValue:
130130
) extends Symbolic:
131131
override def completionItemDataKind: Integer = CompletionSource.CompilerKind.ordinal
132132

133+
/**
134+
* We need to access original completion in sorting phase.
135+
* This class is only a wrapper to hold both new completion and original completion.
136+
*
137+
* All methods are proxied to @param extraMethod
138+
*
139+
* FIXME Refactor this file to different architercture. At least to somethhing that is easier to modifiy and scale.
140+
* One solution may be a migration to flag based solution.
141+
*/
142+
case class ExtraMethod(
143+
owner: Denotation,
144+
extraMethod: Symbolic,
145+
) extends Symbolic:
146+
override def additionalEdits: List[TextEdit] = extraMethod.additionalEdits
147+
override def command: Option[String] = extraMethod.command
148+
override def completionData(buildTargetIdentifier: String)(using Context): CompletionItemData = extraMethod.completionData((buildTargetIdentifier))
149+
override def completionItemKind(using Context): CompletionItemKind = extraMethod.completionItemKind
150+
override def description(printer: ShortenedTypePrinter)(using Context): String = extraMethod.description(printer)
151+
override def labelWithDescription(printer: ShortenedTypePrinter)(using Context): String = extraMethod.labelWithDescription(printer)
152+
override def range: Option[Range] = extraMethod.range
153+
override def denotation: Denotation = extraMethod.denotation
154+
override def label: String = extraMethod.label
155+
override def filterText: Option[String] = extraMethod.filterText
156+
override def importSymbol: Symbol = extraMethod.importSymbol
157+
override def lspTags(using Context): List[CompletionItemTag] = extraMethod.lspTags
158+
override def insertText: Option[String] = extraMethod.insertText
159+
override def isExtensionMethod: Boolean = extraMethod.isExtensionMethod
160+
override def snippetAffix: CompletionAffix = extraMethod.snippetAffix
161+
override def insertMode: Option[InsertTextMode] = extraMethod.insertMode
162+
override val symbol: Symbol = extraMethod.symbol
163+
override def completionItemDataKind: Integer = extraMethod.completionItemDataKind
164+
133165
case class Scope(
134166
label: String,
135167
denotation: Denotation,

‎presentation-compiler/src/main/dotty/tools/pc/completions/Completions.scala

Lines changed: 76 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import scala.collection.mutable
88
import scala.meta.internal.metals.Fuzzy
99
import scala.meta.internal.metals.ReportContext
1010
import scala.meta.internal.mtags.CoursierComplete
11-
import scala.meta.internal.pc.{IdentifierComparator, MemberOrdering}
11+
import scala.meta.internal.pc.{IdentifierComparator, MemberOrdering, CompletionFuzzy}
1212
import scala.meta.pc.*
1313

1414
import dotty.tools.dotc.ast.tpd.*
@@ -232,16 +232,18 @@ class Completions(
232232
val hasNonSyntheticConstructor = sym.name.isTypeName && sym.isClass
233233
&& !sym.is(ModuleClass) && !sym.is(Trait) && !sym.is(Abstract) && !sym.is(Flags.JavaDefined)
234234

235-
val methodDenots: List[SingleDenotation] =
235+
val (extraMethodDenots, skipOriginalDenot): (List[SingleDenotation], Boolean) =
236236
if shouldAddSnippet && isNew && hasNonSyntheticConstructor then
237-
sym.info.member(nme.CONSTRUCTOR).allSymbols.map(_.asSingleDenotation)
237+
val constructors = sym.info.member(nme.CONSTRUCTOR).allSymbols.map(_.asSingleDenotation)
238238
.filter(_.symbol.isAccessibleFrom(denot.info))
239+
constructors -> true
240+
239241
else if shouldAddSnippet && completionMode.is(Mode.Term) && sym.name.isTermName && !sym.is(Flags.Method) && !sym.is(Flags.JavaDefined) then
240242
val constructors = if sym.isAllOf(ConstructorProxyModule) then
241243
sym.companionClass.info.member(nme.CONSTRUCTOR).allSymbols
242244
else
243245
val companionApplies = denot.info.member(nme.apply).allSymbols
244-
val classConstructors = if sym.companionClass.exists then
246+
val classConstructors = if sym.companionClass.exists && !sym.companionClass.isOneOf(AbstractOrTrait) then
245247
sym.companionClass.info.member(nme.CONSTRUCTOR).allSymbols
246248
else Nil
247249

@@ -250,33 +252,44 @@ class Completions(
250252
else
251253
companionApplies ++ classConstructors
252254

253-
val extraDenots = constructors.map(_.asSeenFrom(denot.info).asSingleDenotation)
255+
val result = constructors.map(_.asSeenFrom(denot.info).asSingleDenotation)
254256
.filter(_.symbol.isAccessibleFrom(denot.info))
255257

256-
if sym.isAllOf(ConstructorProxyModule) || sym.is(Trait) then extraDenots
257-
else denot :: extraDenots
258+
result -> (sym.isAllOf(ConstructorProxyModule) || sym.is(Trait))
259+
else Nil -> false
258260

259-
else denot :: Nil
261+
val extraCompletionValues =
262+
val existsApply = extraMethodDenots.exists(_.symbol.name == nme.apply)
260263

261-
val existsApply = methodDenots.exists(_.symbol.name == nme.apply)
264+
extraMethodDenots.map { methodDenot =>
265+
val suffix = findSuffix(methodDenot.symbol)
266+
val affix = if methodDenot.symbol.isConstructor && existsApply then
267+
adjustedPath match
268+
case (select @ Select(qual, _)) :: _ =>
269+
val start = qual.span.start
270+
val insertRange = select.sourcePos.startPos.withEnd(completionPos.queryEnd).toLsp
262271

263-
methodDenots.map { methodDenot =>
264-
val suffix = findSuffix(methodDenot.symbol)
265-
val affix = if methodDenot.symbol.isConstructor && existsApply then
266-
adjustedPath match
267-
case (select @ Select(qual, _)) :: _ =>
268-
val start = qual.span.start
269-
val insertRange = select.sourcePos.startPos.withEnd(completionPos.queryEnd).toLsp
272+
suffix
273+
.withCurrentPrefix(qual.show + ".")
274+
.withNewPrefix(Affix(PrefixKind.New, insertRange = Some(insertRange)))
275+
case _ =>
276+
suffix.withNewPrefix(Affix(PrefixKind.New))
277+
else suffix
278+
val name = undoBacktick(label)
270279

271-
suffix
272-
.withCurrentPrefix(qual.show + ".")
273-
.withNewPrefix(Affix(PrefixKind.New, insertRange = Some(insertRange)))
274-
case _ =>
275-
suffix.withNewPrefix(Affix(PrefixKind.New))
276-
else suffix
280+
CompletionValue.ExtraMethod(
281+
owner = denot,
282+
extraMethod = toCompletionValue(name, methodDenot, affix)
283+
)
284+
}
285+
286+
if skipOriginalDenot then extraCompletionValues
287+
else
288+
val suffix = findSuffix(denot.symbol)
277289
val name = undoBacktick(label)
278-
toCompletionValue(name, methodDenot, affix)
279-
}
290+
val denotCompletionValue = toCompletionValue(name, denot, suffix)
291+
denotCompletionValue :: extraCompletionValues
292+
280293
end completionsWithAffix
281294

282295
/**
@@ -638,7 +651,10 @@ class Completions(
638651
case ck: CompletionValue.CaseKeyword => (ck.label, true)
639652
case symOnly: CompletionValue.Symbolic =>
640653
val sym = symOnly.symbol
641-
val name = SemanticdbSymbols.symbolName(sym)
654+
val name = symOnly match
655+
case CompletionValue.ExtraMethod(owner, extraMethod) =>
656+
SemanticdbSymbols.symbolName(owner.symbol) + SemanticdbSymbols.symbolName(extraMethod.symbol)
657+
case _ => SemanticdbSymbols.symbolName(sym)
642658
val suffix =
643659
if symOnly.snippetAffix.addLabelSnippet then "[]" else ""
644660
val id = name + suffix
@@ -749,18 +765,24 @@ class Completions(
749765
relevance
750766
end symbolRelevance
751767

768+
def computeRelevance(sym: Symbol, completionValue: CompletionValue.Symbolic) =
769+
completionValue match
770+
case _: CompletionValue.Override =>
771+
var penalty = symbolRelevance(sym)
772+
// show the abstract members first
773+
if !sym.is(Deferred) then penalty |= MemberOrdering.IsNotAbstract
774+
penalty
775+
case _: CompletionValue.Workspace =>
776+
symbolRelevance(sym) | (IsWorkspaceSymbol + sym.name.show.length())
777+
case _ => symbolRelevance(sym)
778+
752779
completion match
753-
case ov: CompletionValue.Override =>
754-
var penalty = symbolRelevance(ov.symbol)
755-
// show the abstract members first
756-
if !ov.symbol.is(Deferred) then penalty |= MemberOrdering.IsNotAbstract
757-
penalty
758-
case CompletionValue.Workspace(_, denot, _, _) =>
759-
symbolRelevance(denot.symbol) | (IsWorkspaceSymbol + denot.name.show.length())
780+
case CompletionValue.ExtraMethod(owner, extraMethod) =>
781+
computeRelevance(owner.symbol, extraMethod)
760782
case sym: CompletionValue.Symbolic =>
761-
symbolRelevance(sym.symbol)
762-
case _ =>
763-
Int.MaxValue
783+
computeRelevance(sym.symbol, sym)
784+
case _ => Int.MaxValue
785+
764786
end computeRelevancePenalty
765787

766788
private lazy val isEvilMethod: Set[Name] = Set[Name](
@@ -868,6 +890,7 @@ class Completions(
868890
def priority(v: CompletionValue): Int =
869891
v match
870892
case _: CompletionValue.Compiler => 0
893+
case CompletionValue.ExtraMethod(_, _: CompletionValue.Compiler) => 0
871894
case _ => 1
872895

873896
priority(o1) - priority(o2)
@@ -909,12 +932,19 @@ class Completions(
909932

910933
def methodScore(v: CompletionValue.Symbolic)(using Context): Int =
911934
val sym = v.symbol
912-
val workspacePenalty = if v.isInstanceOf[CompletionValue.Workspace] then 3 else 0
935+
val workspacePenalty = v match
936+
case CompletionValue.ExtraMethod(_, _: CompletionValue.Workspace) => 5
937+
case _: CompletionValue.Workspace => 5
938+
case _ => 0
939+
940+
val isExtraMethod = v.isInstanceOf[CompletionValue.ExtraMethod]
913941
val methodPenalty =
914942
if isNew && sym.isConstructor then -1
915-
else if !completionMode.is(Mode.Member) && sym.name == nme.apply then 1
916-
else if sym.isConstructor then 2
917-
else 0
943+
else if isExtraMethod && !sym.isConstructor then 1
944+
else if isExtraMethod then 2
945+
else if !sym.isAllOf(SyntheticModule) then 3
946+
else 4
947+
918948
workspacePenalty + methodPenalty
919949

920950
override def compare(o1: CompletionValue, o2: CompletionValue): Int =
@@ -942,14 +972,14 @@ class Completions(
942972
)
943973
if byFuzzy != 0 then byFuzzy
944974
else
945-
val byMethodScore = Integer.compare(
946-
methodScore(sym1),
947-
methodScore(sym2)
948-
)
949-
if byMethodScore != 0 then byMethodScore
975+
val byRelevance = compareByRelevance(o1, o2)
976+
if byRelevance != 0 then byRelevance
950977
else
951-
val byRelevance = compareByRelevance(o1, o2)
952-
if byRelevance != 0 then byRelevance
978+
val byMethodScore = Integer.compare(
979+
methodScore(sym1),
980+
methodScore(sym2)
981+
)
982+
if byMethodScore != 0 then byMethodScore
953983
else
954984
val byIdentifier = IdentifierComparator.compare(
955985
s1.name.show,

‎presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionArgSuite.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ class CompletionArgSuite extends BaseCompletionSuite:
9696
"""|age = : Int
9797
|followers = : Int
9898
|Main test
99-
|User test
99+
|User(name: String = ..., age: Int = ..., address: String = ..., followers: Int = ...): User
100100
|""".stripMargin,
101101
topLines = Option(4)
102102
)
@@ -130,7 +130,7 @@ class CompletionArgSuite extends BaseCompletionSuite:
130130
"""|age = : Int
131131
|followers = : Int
132132
|Main test
133-
|User test
133+
|User(name: String = ..., age: Int = ..., address: String = ..., followers: Int = ...): User
134134
|""".stripMargin,
135135
topLines = Option(4)
136136
)
@@ -1119,4 +1119,4 @@ class CompletionArgSuite extends BaseCompletionSuite:
11191119
|""".stripMargin,
11201120
"""x: Int
11211121
|x = : Any""".stripMargin,
1122-
)
1122+
)

0 commit comments

Comments
 (0)