Skip to content

Commit

Permalink
improvement: show both type symbol and object in scala 3 completions
Browse files Browse the repository at this point in the history
  • Loading branch information
kasiaMarek authored and tgodzik committed Jun 28, 2023
1 parent 9ef4859 commit 6ccc512
Show file tree
Hide file tree
Showing 8 changed files with 145 additions and 157 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@ object MemberOrdering {
val IsInherited: Int = 1 << 27
val IsNotLocalByBlock: Int = 1 << 26
val IsNotDefinedInFile: Int = 1 << 25
val IsNotGetter: Int = 1 << 24
val IsPackage: Int = 1 << 23
val IsNotCaseAccessor: Int = 1 << 22
val IsNotPublic: Int = 1 << 21
val IsSynthetic: Int = 1 << 20
val IsDeprecated: Int = 1 << 19
val IsEvilMethod: Int = 1 << 18 // example: clone() and finalize()
val IsNotTypeInTypePos: Int = 1 << 24
val IsNotGetter: Int = 1 << 23
val IsPackage: Int = 1 << 22
val IsNotCaseAccessor: Int = 1 << 21
val IsNotPublic: Int = 1 << 20
val IsSynthetic: Int = 1 << 19
val IsDeprecated: Int = 1 << 18
val IsEvilMethod: Int = 1 << 17 // example: clone() and finalize()

// OverrideDefMember
val IsNotAbstract: Int = 1 << 30
Expand All @@ -26,6 +27,7 @@ object MemberOrdering {
(IsInherited, "inherited"),
(IsNotLocalByBlock, "notLocalByBlock"),
(IsNotDefinedInFile, "notDefinedInFile"),
(IsNotTypeInTypePos, "notTypeInTypePosition"),
(IsNotGetter, "notGetter"),
(IsPackage, "package"),
(IsNotCaseAccessor, "notCaseAccessor"),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,48 +1,47 @@
package scala.meta.internal.pc.completions

/**
* @param brace should we add "()" suffix?
* @param bracket should we add "[]" suffix?
* @param template should we add "{}" suffix?
* @param suffixes which we should insert
* @param snippet which suffix should we insert the snippet $0
*/
case class CompletionSuffix(
brace: Boolean,
bracket: Boolean,
template: Boolean,
suffixes: Set[SuffixKind],
snippet: SuffixKind,
):
def labelSnippet =
suffixes.collectFirst {
case SuffixKind.Bracket(1) => "[T]"
case SuffixKind.Bracket(n) =>
(for (i <- 1 to n) yield s"T$i").mkString("[", ", ", "]")
}
def hasSnippet = snippet != SuffixKind.NoSuffix
def chain(copyFn: CompletionSuffix => CompletionSuffix) = copyFn(this)
def withNewSuffix(kind: SuffixKind) =
CompletionSuffix(suffixes + kind, snippet)
def withNewSuffixSnippet(kind: SuffixKind) =
CompletionSuffix(suffixes + kind, kind)
def toEdit: String =
if !hasSuffix then ""
else
val braceSuffix =
if brace && snippet == SuffixKind.Brace then "($0)"
else if brace then "()"
else ""
val bracketSuffix =
if bracket && snippet == SuffixKind.Bracket then "[$0]"
else if bracket then "[]"
else ""
val templateSuffix =
if template && snippet == SuffixKind.Template then " {$0}"
else if template then " {}"
else ""
s"$bracketSuffix$braceSuffix$templateSuffix"
def loop(suffixes: List[SuffixKind]): String =
def cursor = if suffixes.head == snippet then "$0" else ""
suffixes match
case SuffixKind.Brace :: tail => s"($cursor)" + loop(tail)
case SuffixKind.Bracket(_) :: tail => s"[$cursor]" + loop(tail)
case SuffixKind.Template :: tail => s" {$cursor}" + loop(tail)
case _ => ""
loop(suffixes.toList)
def toEditOpt: Option[String] =
val edit = toEdit
if edit.nonEmpty then Some(edit) else None
private def hasSuffix = brace || bracket || template
end CompletionSuffix

object CompletionSuffix:
val empty = CompletionSuffix(
brace = false,
bracket = false,
template = false,
suffixes = Set.empty,
snippet = SuffixKind.NoSuffix,
)

enum SuffixKind:
case Brace, Bracket, Template, NoSuffix
case Brace extends SuffixKind
case Bracket(typeParamsCount: Int) extends SuffixKind
case Template extends SuffixKind
case NoSuffix extends SuffixKind
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ sealed trait CompletionValue:
* Label with potentially attached description.
*/
def labelWithDescription(printer: MetalsPrinter)(using Context): String =
label
labelWithSuffix
def labelWithSuffix: String =
s"$label${snippetSuffix.labelSnippet.getOrElse("")}"
def lspTags(using Context): List[CompletionItemTag] = Nil
end CompletionValue

Expand Down Expand Up @@ -73,13 +75,14 @@ object CompletionValue:
override def labelWithDescription(
printer: MetalsPrinter
)(using Context): String =
if symbol.is(Method) then s"${label}${description(printer)}"
else if symbol.isConstructor then label
else if symbol.is(Mutable) then s"${label}: ${description(printer)}"
if symbol.is(Method) then s"${labelWithSuffix}${description(printer)}"
else if symbol.isConstructor then labelWithSuffix
else if symbol.is(Mutable) then
s"${labelWithSuffix}: ${description(printer)}"
else if symbol.is(Package) || symbol.is(Module) || symbol.isClass then
if isFromWorkspace then s"${label} -${description(printer)}"
else s"${label}${description(printer)}"
else s"${label}: ${description(printer)}"
if isFromWorkspace then s"${labelWithSuffix} -${description(printer)}"
else s"${labelWithSuffix}${description(printer)}"
else s"${labelWithSuffix}: ${description(printer)}"

override def description(printer: MetalsPrinter)(using Context): String =
printer.completionSymbol(symbol)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,23 +94,7 @@ class Completions(
if generalExclude then false
else
this match
case Type(_, _) if sym.isType => true
case Type(_, _) if sym.isTerm =>
/* Type might be referenced by a path over an object:
* ```
* object sample:
* class X
* val a: samp@@le.X = ???
* ```
* ignore objects that has companion class
*
* Also ignore objects that doesn't have any members.
* By some reason traits might have a fake companion object(example: scala.sys.process.ProcessBuilderImpl)
*/
val allowModule =
sym.is(Module) &&
(sym.companionClass == NoSymbol && sym.info.allMembers.nonEmpty)
allowModule
case Type(_, _) => true
case Term if isWildcardParam(sym) => false
case Term if sym.isTerm || sym.is(Package) => true
case Import => true
Expand All @@ -128,6 +112,11 @@ class Completions(
case Type(_, hasNewKw) => hasNewKw
case _ => false

def allowApplicationSuffix: Boolean =
this match
case Term => true
case _ => false

end CursorPos

private lazy val cursorPos =
Expand Down Expand Up @@ -211,7 +200,7 @@ class Completions(
end match

val application = CompletionApplication.fromPath(path)
val ordering = completionOrdering(application)
val ordering = completionOrdering(application, cursorPos)
val values = application.postProcess(all.sorted(ordering))
(values, result)
end completions
Expand Down Expand Up @@ -263,7 +252,9 @@ class Completions(
.chain { suffix => // for [] suffix
if shouldAddSnippet &&
cursorPos.allowBracketSuffix && symbol.info.typeParams.nonEmpty
then suffix.copy(bracket = true, snippet = SuffixKind.Bracket)
then
val typeParamsCount = symbol.info.typeParams.length
suffix.withNewSuffixSnippet(SuffixKind.Bracket(typeParamsCount))
else suffix
}
.chain { suffix => // for () suffix
Expand All @@ -272,18 +263,19 @@ class Completions(
val paramss = getParams(symbol)
paramss match
case Nil => suffix
case List(Nil) => suffix.copy(brace = true)
case List(Nil) => suffix.withNewSuffix(SuffixKind.Brace)
case _ if config.isCompletionSnippetsEnabled =>
val onlyParameterless = paramss.forall(_.isEmpty)
lazy val onlyImplicitOrTypeParams = paramss.forall(
_.exists { sym =>
sym.isType || sym.is(Implicit) || sym.is(Given)
}
)
if onlyParameterless then suffix.copy(brace = true)
if onlyParameterless then suffix.withNewSuffix(SuffixKind.Brace)
else if onlyImplicitOrTypeParams then suffix
else if suffix.hasSnippet then suffix.copy(brace = true)
else suffix.copy(brace = true, snippet = SuffixKind.Brace)
else if suffix.hasSnippet then
suffix.withNewSuffix(SuffixKind.Brace)
else suffix.withNewSuffixSnippet(SuffixKind.Brace)
case _ => suffix
end match
else suffix
Expand All @@ -292,8 +284,8 @@ class Completions(
if shouldAddSnippet && cursorPos.allowTemplateSuffix
&& isAbstractType(symbol)
then
if suffix.hasSnippet then suffix.copy(template = true)
else suffix.copy(template = true, snippet = SuffixKind.Template)
if suffix.hasSnippet then suffix.withNewSuffix(SuffixKind.Template)
else suffix.withNewSuffixSnippet(SuffixKind.Template)
else suffix
}

Expand All @@ -316,7 +308,7 @@ class Completions(
val methodSymbols =
if shouldAddSnippet &&
(sym.is(Flags.Module) || sym.isClass && !sym.is(Flags.Trait)) &&
!isJavaDefined
!isJavaDefined && cursorPos.allowApplicationSuffix
then
val info =
/* Companion will be added even for normal classes now,
Expand Down Expand Up @@ -636,11 +628,12 @@ class Completions(
case symOnly: CompletionValue.Symbolic =>
val sym = symOnly.symbol
val name = SemanticdbSymbols.symbolName(sym)
val id =
val nameId =
if sym.isClass || sym.is(Module) then
// drop #|. at the end to avoid duplication
name.substring(0, name.length - 1)
else name
val id = nameId + symOnly.snippetSuffix.labelSnippet.getOrElse("")
val include = cursorPos.include(sym)
(id, include)
case kw: CompletionValue.Keyword => (kw.label, true)
Expand Down Expand Up @@ -701,6 +694,7 @@ class Completions(
private def computeRelevancePenalty(
completion: CompletionValue,
application: CompletionApplication,
cursorPos: CursorPos,
): Int =
import scala.meta.internal.pc.MemberOrdering.*

Expand Down Expand Up @@ -746,7 +740,10 @@ class Completions(
relevance |= IsSynthetic
if sym.isDeprecated then relevance |= IsDeprecated
if isEvilMethod(sym.name) then relevance |= IsEvilMethod

cursorPos match
case CursorPos.Type(_, _) if !sym.isType =>
relevance |= IsNotTypeInTypePos
case _ =>
relevance
end symbolRelevance

Expand Down Expand Up @@ -824,7 +821,8 @@ class Completions(
end CompletionApplication

private def completionOrdering(
application: CompletionApplication
application: CompletionApplication,
cursorPos: CursorPos,
): Ordering[CompletionValue] =
new Ordering[CompletionValue]:
val queryLower = completionPos.query.toLowerCase()
Expand All @@ -839,8 +837,8 @@ class Completions(

def compareByRelevance(o1: CompletionValue, o2: CompletionValue): Int =
Integer.compare(
computeRelevancePenalty(o1, application),
computeRelevancePenalty(o2, application),
computeRelevancePenalty(o1, application, cursorPos),
computeRelevancePenalty(o2, application, cursorPos),
)

def fuzzyScore(o: CompletionValue.Symbolic): Int =
Expand Down
3 changes: 2 additions & 1 deletion tests/cross/src/test/scala/tests/pc/CompletionDocSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -703,9 +703,10 @@ class CompletionDocSuite extends BaseCompletionSuite {
|- `fin`: Finally logic which if defined will be invoked after catch logic
|- `rethrow`: Predicate on throwables determining when to rethrow a caught [Throwable](Throwable)
|- `pf`: Partial function used when applying catch logic to determine result value
|Catch - scala.util.control.Exception
|Catch[T] - scala.util.control.Exception
|""".stripMargin,
),
topLines = Some(1), // for Scala3, result contains `Catch[$0]` and `Catch`
)

check(
Expand Down
13 changes: 13 additions & 0 deletions tests/cross/src/test/scala/tests/pc/CompletionSnippetSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ class CompletionSnippetSuite extends BaseCompletionSuite {
compat = Map(
"3" ->
"""|IndexedSeq[$0]
|IndexedSeq
|""".stripMargin
),
)
Expand Down Expand Up @@ -182,6 +183,9 @@ class CompletionSnippetSuite extends BaseCompletionSuite {
"""|ArrayDeque[$0]
|ArrayDeque[$0]
|ArrayDequeOps[$0]
|ArrayDeque
|ArrayDeque
|ArrayDequeOps
|""".stripMargin,
),
)
Expand All @@ -194,6 +198,12 @@ class CompletionSnippetSuite extends BaseCompletionSuite {
|""".stripMargin,
"""|SimpleFileVisitor[$0]
|""".stripMargin,
compat = Map(
"3" ->
"""|SimpleFileVisitor[$0]
|SimpleFileVisitor
|""".stripMargin
),
)

checkSnippet(
Expand All @@ -214,6 +224,7 @@ class CompletionSnippetSuite extends BaseCompletionSuite {
"3" ->
"""|Iterable[$0] {}
|IterableOnce[$0] {}
|Iterable
|""".stripMargin,
),
)
Expand All @@ -236,6 +247,7 @@ class CompletionSnippetSuite extends BaseCompletionSuite {
"3" ->
"""|Iterable[$0]
|IterableOnce[$0]
|Iterable
|""".stripMargin,
),
)
Expand All @@ -258,6 +270,7 @@ class CompletionSnippetSuite extends BaseCompletionSuite {
"3" ->
"""|Iterable[$0]
|IterableOnce[$0]
|Iterable
|""".stripMargin,
),
)
Expand Down
13 changes: 13 additions & 0 deletions tests/cross/src/test/scala/tests/pc/CompletionSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ class CompletionSuite extends BaseCompletionSuite {
|ProcessBuilder - scala.sys.process
|CertPathBuilder - java.security.cert
|CertPathBuilderSpi - java.security.cert
|ProcessBuilderImpl - scala.sys.process
|CertPathBuilderResult - java.security.cert
|PKIXBuilderParameters - java.security.cert
|PooledConnectionBuilder - javax.sql
Expand Down Expand Up @@ -904,6 +905,12 @@ class CompletionSuite extends BaseCompletionSuite {
|""".stripMargin,
"""|ListBuffer - scala.collection.mutable
|""".stripMargin,
compat = Map(
"3" ->
"""|ListBuffer[T] - scala.collection.mutable
|ListBuffer - scala.collection.mutable
|""".stripMargin
),
)

check(
Expand All @@ -914,6 +921,12 @@ class CompletionSuite extends BaseCompletionSuite {
|""".stripMargin,
"""|ListBuffer - scala.collection.mutable
|""".stripMargin,
compat = Map(
"3" ->
"""|ListBuffer[T] - scala.collection.mutable
|ListBuffer - scala.collection.mutable
|""".stripMargin
),
)

check(
Expand Down
Loading

0 comments on commit 6ccc512

Please sign in to comment.