From cb079b9b052335f1a92f9307aa984e212f26d511 Mon Sep 17 00:00:00 2001 From: kasiaMarek Date: Wed, 7 Aug 2024 10:09:16 +0200 Subject: [PATCH 1/3] improvement: sorting workspace members with same name by frequency --- .../tools/pc/ScalaPresentationCompiler.scala | 11 ++++- .../pc/completions/CompletionProvider.scala | 7 +++- .../tools/pc/completions/Completions.scala | 40 ++++++++++++++----- .../dotty/tools/pc/base/BasePCSuite.scala | 3 ++ .../completion/CompletionContextSuite.scala | 27 +++++++++++++ 5 files changed, 73 insertions(+), 15 deletions(-) create mode 100644 presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionContextSuite.scala diff --git a/presentation-compiler/src/main/dotty/tools/pc/ScalaPresentationCompiler.scala b/presentation-compiler/src/main/dotty/tools/pc/ScalaPresentationCompiler.scala index ad8ac02ec811..a8ab7af0d147 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/ScalaPresentationCompiler.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/ScalaPresentationCompiler.scala @@ -48,7 +48,8 @@ case class ScalaPresentationCompiler( sh: Option[ScheduledExecutorService] = None, config: PresentationCompilerConfig = PresentationCompilerConfigImpl(), folderPath: Option[Path] = None, - reportsLevel: ReportLevel = ReportLevel.Info + reportsLevel: ReportLevel = ReportLevel.Info, + completionItemPriority: CompletionItemPriority = (_: String) => 0, ) extends PresentationCompiler: def this() = this("", None, Nil, Nil) @@ -63,6 +64,11 @@ case class ScalaPresentationCompiler( .map(StdReportContext(_, _ => buildTargetName, reportsLevel)) .getOrElse(EmptyReportContext) + override def withCompletionItemPriority( + priority: CompletionItemPriority + ): PresentationCompiler = + copy(completionItemPriority = priority) + override def withBuildTargetName(buildTargetName: String) = copy(buildTargetName = Some(buildTargetName)) @@ -142,7 +148,8 @@ case class ScalaPresentationCompiler( params, config, buildTargetIdentifier, - folderPath + folderPath, + completionItemPriority ).completions() } diff --git a/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionProvider.scala b/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionProvider.scala index 9cd98de33141..4d45595dac8d 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionProvider.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionProvider.scala @@ -32,6 +32,7 @@ import org.eclipse.lsp4j.InsertTextFormat import org.eclipse.lsp4j.InsertTextMode import org.eclipse.lsp4j.Range as LspRange import org.eclipse.lsp4j.TextEdit +import scala.meta.pc.CompletionItemPriority class CompletionProvider( search: SymbolSearch, @@ -39,7 +40,8 @@ class CompletionProvider( params: OffsetParams, config: PresentationCompilerConfig, buildTargetIdentifier: String, - folderPath: Option[Path] + folderPath: Option[Path], + referenceCounter: CompletionItemPriority )(using reports: ReportContext): def completions(): CompletionList = val uri = params.uri().nn @@ -86,7 +88,8 @@ class CompletionProvider( folderPath, autoImportsGen, unit.comments, - driver.settings + driver.settings, + referenceCounter ).completions() val items = completions.zipWithIndex.map { case (item, idx) => diff --git a/presentation-compiler/src/main/dotty/tools/pc/completions/Completions.scala b/presentation-compiler/src/main/dotty/tools/pc/completions/Completions.scala index d043a2cfddbf..b0441c18df9a 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/completions/Completions.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/completions/Completions.scala @@ -50,7 +50,8 @@ class Completions( workspace: Option[Path], autoImports: AutoImportsGenerator, comments: List[Comment], - options: List[String] + options: List[String], + completionItemPriority: CompletionItemPriority )(using ReportContext): given context: Context = ctx @@ -909,6 +910,20 @@ class Completions( else 0 end compareLocalSymbols + private def workspaceMemberPriority(symbol: Symbol): Int = + completionItemPriority + .workspaceMemberPriority( + SemanticdbSymbols.symbolName(symbol), + ) + + def compareFrequency(o1: CompletionValue, o2: CompletionValue): Int = + (o1, o2) match + case (w1: CompletionValue.Workspace, w2: CompletionValue.Workspace) => + workspaceMemberPriority(w1.symbol) + .compareTo(workspaceMemberPriority(w2.symbol)) + case _ => 0 + end compareFrequency + def compareByRelevance(o1: CompletionValue, o2: CompletionValue): Int = Integer.compare( computeRelevancePenalty(o1, application), @@ -1018,17 +1033,20 @@ class Completions( ) if byIdentifier != 0 then byIdentifier else - val byOwner = - s1.owner.fullName.toString - .compareTo(s2.owner.fullName.toString) - if byOwner != 0 then byOwner + val byFrequency = compareFrequency(o1, o2) + if byFrequency != 0 then byFrequency else - val byParamCount = Integer.compare( - s1.paramSymss.flatten.size, - s2.paramSymss.flatten.size - ) - if byParamCount != 0 then byParamCount - else s1.detailString.compareTo(s2.detailString) + val byOwner = + s1.owner.fullName.toString + .compareTo(s2.owner.fullName.toString) + if byOwner != 0 then byOwner + else + val byParamCount = Integer.compare( + s1.paramSymss.flatten.size, + s2.paramSymss.flatten.size + ) + if byParamCount != 0 then byParamCount + else s1.detailString.compareTo(s2.detailString) end if end if end if diff --git a/presentation-compiler/test/dotty/tools/pc/base/BasePCSuite.scala b/presentation-compiler/test/dotty/tools/pc/base/BasePCSuite.scala index a1fec0af3e8f..1158e433e732 100644 --- a/presentation-compiler/test/dotty/tools/pc/base/BasePCSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/base/BasePCSuite.scala @@ -22,6 +22,7 @@ import dotty.tools.pc.utils._ import org.eclipse.lsp4j.MarkupContent import org.eclipse.lsp4j.jsonrpc.messages.Either as JEither import org.junit.runner.RunWith +import scala.meta.pc.CompletionItemPriority object TestResources: val scalaLibrary = BuildInfo.ideTestsDependencyClasspath.map(_.toPath).toSeq @@ -30,6 +31,7 @@ object TestResources: @RunWith(classOf[ReusableClassRunner]) abstract class BasePCSuite extends PcAssertions: + val completionItemPriority: CompletionItemPriority = (_: String) => 0 private val isDebug = ManagementFactory.getRuntimeMXBean.getInputArguments.toString.contains("-agentlib:jdwp") val tmp = Files.createTempDirectory("stable-pc-tests") @@ -53,6 +55,7 @@ abstract class BasePCSuite extends PcAssertions: .withExecutorService(executorService) .withScheduledExecutorService(executorService) .withSearch(search) + .withCompletionItemPriority(completionItemPriority) .newInstance("", myclasspath.asJava, scalacOpts.asJava) protected def config: PresentationCompilerConfig = diff --git a/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionContextSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionContextSuite.scala new file mode 100644 index 000000000000..5314a61ab599 --- /dev/null +++ b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionContextSuite.scala @@ -0,0 +1,27 @@ +package dotty.tools.pc.tests.completion + +import dotty.tools.pc.base.BaseCompletionSuite +import scala.meta.pc.CompletionItemPriority +import org.junit.Test + +class CompletionContextSuite extends BaseCompletionSuite: + override val completionItemPriority: CompletionItemPriority = { + case "scala/concurrent/Future." => -1 + case _ => 0 + } + // scala.concurrent.Future should be ranked higher than java.util.concurrent.Future + val futureCompletionResult: List[String] = + List("Future - scala.concurrent", "Future - java.util.concurrent") + + @Test + def `context` = + check( + """package fut + |object A { + | Futur@@ + |}""".stripMargin, + """Future - scala.concurrent + |Future - java.util.concurrent + |""".stripMargin, + filter = futureCompletionResult.contains + ) From e0e3695cf1908cb03019bfa8d1656bc781b28930 Mon Sep 17 00:00:00 2001 From: kasiaMarek Date: Wed, 7 Aug 2024 12:06:27 +0200 Subject: [PATCH 2/3] test: don't suggest completions for param names in definition --- .../tools/pc/tests/completion/CompletionSuite.scala | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala index 437fe606932b..2282e3e5346d 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala @@ -2042,3 +2042,16 @@ class CompletionSuite extends BaseCompletionSuite: |""".stripMargin, includeCompletionKind = true ) + + @Test def `def-arg` = + check( + """|package a + |object W { + | val aaaaaa = 1 + |} + |object O { + | def foo(aa@@) + |} + |""".stripMargin, + "" + ) From a28bc0f394fe1f9ca5426f87aba2b007c74a026a Mon Sep 17 00:00:00 2001 From: kasiaMarek Date: Wed, 7 Aug 2024 13:39:18 +0200 Subject: [PATCH 3/3] fix: disambiguate workspace completions for vals --- .../pc/completions/CompletionValue.scala | 17 ++-- .../tools/pc/completions/Completions.scala | 2 +- .../pc/tests/completion/CompletionSuite.scala | 81 +++++++++++++++++++ .../completion/CompletionWorkspaceSuite.scala | 4 +- 4 files changed, 96 insertions(+), 8 deletions(-) diff --git a/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionValue.scala b/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionValue.scala index 98cceae149d3..90b285bffb3a 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionValue.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionValue.scala @@ -101,13 +101,13 @@ object CompletionValue: )(using Context): String = if symbol.isConstructor then s"${snippetAffix.toPrefix}${label}${description(printer)}" else if symbol.is(Method) then s"${label}${description(printer)}" - else if symbol.is(Mutable) then s"$label: ${description(printer)}" + else if symbol.is(Mutable) then s"$label${description(printer)}" else if symbol.is(Package) || symbol.is(Module) || symbol.isClass then s"${labelWithSuffix(printer)}${description(printer)}" else if symbol.isType then labelWithSuffix(printer) else if symbol.isTerm && symbol.info.typeSymbol.is(Module) then s"${label}${description(printer)}" - else s"$label: ${description(printer)}" + else s"$label${description(printer)}" protected def labelWithSuffix(printer: ShortenedTypePrinter)(using Context): String = if snippetAffix.addLabelSnippet @@ -119,7 +119,10 @@ object CompletionValue: else label override def description(printer: ShortenedTypePrinter)(using Context): String = - printer.completionSymbol(denotation) + def info = denotation.info.widenTermRefExpr + val isVal = !(symbol.is(Module) || symbol.is(Method) || symbol.isType || info.typeSymbol.is(Module)) + val prefix = if isVal then ": " else "" + prefix ++ printer.completionSymbol(denotation) end Symbolic @@ -178,9 +181,10 @@ object CompletionValue: override def completionItemDataKind: Integer = CompletionSource.WorkspaceKind.ordinal override def labelWithDescription(printer: ShortenedTypePrinter)(using Context): String = + def isMethodOrValue = !(symbol.isType || symbol.is(Module)) if symbol.isConstructor || symbol.name == nme.apply then s"${snippetAffix.toPrefix}${label}${description(printer)} - ${printer.fullNameString(importSymbol.effectiveOwner)}" - else if symbol.is(Method) then + else if isMethodOrValue then s"${labelWithSuffix(printer)} - ${printer.fullNameString(symbol.effectiveOwner)}" else if symbol.is(Package) || symbol.is(Module) || symbol.isClass then s"${labelWithSuffix(printer)} -${description(printer)}" @@ -199,7 +203,7 @@ object CompletionValue: CompletionItemKind.Method override def completionItemDataKind: Integer = CompletionSource.ImplicitClassKind.ordinal override def description(printer: ShortenedTypePrinter)(using Context): String = - s"${printer.completionSymbol(denotation)} (implicit)" + s"${super.description(printer)} (implicit)" /** * CompletionValue for extension methods via SymbolSearch @@ -339,6 +343,9 @@ object CompletionValue: override def labelWithDescription(printer: ShortenedTypePrinter)(using Context): String = label + + override def description(printer: ShortenedTypePrinter)(using Context): String = + printer.completionSymbol(denotation) end CaseKeyword case class Document(label: String, doc: String, description: String) diff --git a/presentation-compiler/src/main/dotty/tools/pc/completions/Completions.scala b/presentation-compiler/src/main/dotty/tools/pc/completions/Completions.scala index b0441c18df9a..3bebaa76a309 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/completions/Completions.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/completions/Completions.scala @@ -914,7 +914,7 @@ class Completions( completionItemPriority .workspaceMemberPriority( SemanticdbSymbols.symbolName(symbol), - ) + ).nn def compareFrequency(o1: CompletionValue, o2: CompletionValue): Int = (o1, o2) match diff --git a/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala index 2282e3e5346d..47e4cabb76f4 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala @@ -2055,3 +2055,84 @@ class CompletionSuite extends BaseCompletionSuite: |""".stripMargin, "" ) + + @Test def conflict = + check( + """|package a + |object O { + | val foofoo: Int = 123 + | def method = { + | val foofoo: String = "abc" + | foofoo@@ + | } + |} + |""".stripMargin, + """|foofoo: String + |foofoo - a.O: Int + |""".stripMargin + ) + + @Test def `conflict-2` = + check( + """|package a + |object A { + | val foo = 1 + |} + |object B { + | val foo = 1 + |} + |object O { + | val x: Int = foo@@ + |} + |""".stripMargin, + """|foo - a.A: Int + |foo - a.B: Int + |""".stripMargin + ) + + @Test def `conflict-3` = + check( + """|package a + |object A { + | var foo = 1 + |} + |object B { + | var foo = 1 + |} + |object O { + | val x: Int = foo@@ + |} + |""".stripMargin, + """|foo - a.A: Int + |foo - a.B: Int + |""".stripMargin + ) + + @Test def `conflict-edit-2` = + checkEdit( + """|package a + |object A { + | val foo = 1 + |} + |object B { + | val foo = 1 + |} + |object O { + | val x: Int = foo@@ + |} + |""".stripMargin, + """|package a + | + |import a.A.foo + |object A { + | val foo = 1 + |} + |object B { + | val foo = 1 + |} + |object O { + | val x: Int = foo + |} + |""".stripMargin, + assertSingleItem = false + ) diff --git a/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionWorkspaceSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionWorkspaceSuite.scala index e5c81e3c044e..488ae0923ea4 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionWorkspaceSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionWorkspaceSuite.scala @@ -767,7 +767,7 @@ class CompletionWorkspaceSuite extends BaseCompletionSuite: |package b: | def main: Unit = incre@@ |""".stripMargin, - """|increment3: Int + """|increment3 - d: Int |increment - a: Int |increment2 - a.c: Int |""".stripMargin @@ -810,7 +810,7 @@ class CompletionWorkspaceSuite extends BaseCompletionSuite: |} |""".stripMargin, """|fooBar: String - |fooBar: List[Int] + |fooBar - test.A: List[Int] |""".stripMargin, )