Skip to content

Commit

Permalink
Merge pull request #2637 from tanishiking/prettify-signature
Browse files Browse the repository at this point in the history
Related #2495



## Problem
Previously, metals shows
- fully qualified name for types in completion items
  - e.g. `foo(arg: very.long.prefix.Type): very.long.prefix.Type`
- print evidence params as it is.
  - e.g. `foo[T](arg: Type)(implicit evidence$1: Ordering[T]): Type` for `foo[T: Ordering](arg: Type): Type`
- don't show belonging package for Class and Modules
  - `NoClassDefFoundError: class and object NoClassDefFoundError` as it's completion's label, for `java.lang.NoClassDefFoundError`.


## This PR fixes those problems

### shorten type signature when the prefix can be seen from the context
  - e.g. if `very.long.prefix` is imported in the context (scope)
  - before: `foo(arg: very.long.prefix.Type): very.long.prefix.Type`
    - ![Screen Shot 2021-03-25 at 0 26 41](https://user-images.githubusercontent.com/9353584/112758255-50958580-9028-11eb-9227-b9449062e138.png)
  - after: `foo(arg: prefix.Type): prefix.Type`
    - <img width="522" alt="Screen Shot 2021-03-29 at 0 39 45" src="https://user-images.githubusercontent.com/9353584/112758238-3360b700-9028-11eb-81aa-e7473fc0eafb.png">


### Pretty-print context bounds, and remove evidence params
- before `foo[T](arg: Type)(implicit evidence$1: Ordering[T]): Type`
  - <img width="518" alt="Screen Shot 2021-03-29 at 0 43 36" src="https://user-images.githubusercontent.com/9353584/112758287-7fabf700-9028-11eb-8558-048fb73e0efc.png">
- after `foo[T: Ordering](arg: Type): Type`
  - <img width="516" alt="Screen Shot 2021-03-29 at 0 45 34" src="https://user-images.githubusercontent.com/9353584/112758288-8470ab00-9028-11eb-8cdf-35478122e7b1.png">

### Show parent packages in detail section of completionItem
- e.g. for `java.langInteger`
- before:
  - <img width="505" alt="Screen Shot 2021-03-29 at 0 24 03" src="https://user-images.githubusercontent.com/9353584/112758313-a1a57980-9028-11eb-88f1-305f06ababbd.png">
- after:
  - <img width="503" alt="Screen Shot 2021-03-29 at 0 25 45" src="https://user-images.githubusercontent.com/9353584/112758326-b97cfd80-9028-11eb-9bb6-c38a29653bbd.png">


## out of scope from this scope
-  print default value (not needed for completions)
   - https://github.com/scalameta/metals/blob/45eaa4151ac5f54cabddce73de3d888d5157b1f3/mtags/src/main/scala-2/scala/meta/internal/pc/Signatures.scala#L399
- Compute type parameters based on the qualifier.	
  - https://github.com/scalameta/metals/blob/45eaa4151ac5f54cabddce73de3d888d5157b1f3/mtags/src/main/scala-2/scala/meta/internal/pc/completions/Completions.scala#L258-L262
  - Gonna fix in another PR
- include documentation
  • Loading branch information
tanishiking authored Apr 20, 2021
1 parent 9de2f3e commit 374f159
Show file tree
Hide file tree
Showing 9 changed files with 531 additions and 288 deletions.
27 changes: 27 additions & 0 deletions mtags/src/main/scala-3/scala/meta/internal/pc/Params.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package scala.meta.internal.pc

import dotty.tools.dotc.core.Contexts.Context
import dotty.tools.dotc.core.Symbols.Symbol
import dotty.tools.dotc.core.Flags._

case class Params(
labels: Seq[String],
kind: Params.Kind
)

object Params {
enum Kind {
case TypeParameter, Normal, Implicit, Using
}

def paramsKind(syms: List[Symbol])(using Context): Params.Kind = {
syms match {
case head :: _ =>
if (head.isType) Kind.TypeParameter
else if (head.is(Given)) Kind.Using
else if (head.is(Implicit)) Kind.Implicit
else Kind.Normal
case Nil => Kind.Normal
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -134,14 +134,24 @@ case class ScalaPresentationCompiler(
CompletionProvider(pos, ctx.fresh.setCompilationUnit(unit))
.completions()
val metalsCompletions = completionPosition(pos, path)(using ctx)
completions ++ metalsCompletions

val newctx = ctx.fresh.setCompilationUnit(unit)
val tpdPath =
Interactive.pathTo(newctx.compilationUnit.tpdTree, pos.span)(using
newctx
)
val locatedCtx = Interactive.contextOfPath(tpdPath)(using newctx)
val history = ShortenedNames(locatedCtx)

(completions ++ metalsCompletions).zipWithIndex.flatMap {
case (item, idx) =>
completionItems(item, history, idx)(using newctx)
}
case None => Nil
}

new CompletionList(
/*isIncomplete = */ false,
items.zipWithIndex.flatMap { case (item, idx) =>
completionItems(item, idx)(using ctx)
}.asJava
items.asJava
)
}
}
Expand All @@ -164,7 +174,6 @@ case class ScalaPresentationCompiler(
"",
definitions.flatMap(d => location(d.namePos(using ctx))).asJava
)

}
}

Expand Down Expand Up @@ -272,22 +281,20 @@ case class ScalaPresentationCompiler(
case _: ImportType =>
printer.typeString(symbol.paramRef(using ctx))
case _ =>
val shortendType =
driver.compilationUnits.get(uri) match {
case Some(unit) =>
val newctx =
ctx.fresh.setCompilationUnit(unit)
val path = Interactive.pathTo(
newctx.compilationUnit.tpdTree,
pos.span
)(using newctx)
val context =
Interactive.contextOfPath(path)(using newctx)
val history = ShortenedNames(context)
shortType(tpw, history)(using newctx)
case None => tpw
}
printer.typeString(shortendType)
driver.compilationUnits.get(uri) match {
case Some(unit) =>
val newctx =
ctx.fresh.setCompilationUnit(unit)
val tpdPath = Interactive.pathTo(
newctx.compilationUnit.tpdTree,
pos.span
)(using newctx)
val context =
Interactive.contextOfPath(tpdPath)(using newctx)
val history = new ShortenedNames(context)
printer.infoString(symbol, history, tpw)(using context)
case None => printer.typeString(tpw)
}
}
}
val content = hoverContent(
Expand Down Expand Up @@ -424,8 +431,29 @@ case class ScalaPresentationCompiler(

private def completionItems(
completion: Completion,
history: ShortenedNames,
idx: Int
)(using ctx: Context): List[CompletionItem] = {
)(using Context): List[CompletionItem] = {
val printer = SymbolPrinter()(using ctx)

/**
* Calculate the string for "detail" field in CompletionItem.
*
* for class or module, it's package name that it belongs to (e.g. "scala.collection" for "scala.collection.Seq")
* otherwise, it's shortened type/method signature
* e.g. "[A: Ordering](x: List[Int]): A", " java.lang.String"
*
* @param sym The symbol for completion item.
*/
def detailString(sym: Symbol): String = {
if (sym.isClass || sym.is(Module)) {
val printer = SymbolPrinter()
s" ${printer.fullNameString(sym.owner)}"
} else {
printer.infoString(sym, history, sym.info.widenTermRefExpr)(using ctx)
}
}

def completionItemKind(
sym: Symbol
)(using ctx: Context): CompletionItemKind = {
Expand All @@ -442,42 +470,42 @@ case class ScalaPresentationCompiler(
else
CompletionItemKind.Field
}
val printer = SymbolPrinter()(using ctx)

completion.symbols.map { sym =>
// For overloaded signatures we get multiple symbols, so we need
// to recalculate the description
// related issue https://github.com/lampepfl/dotty/issues/11941
val description =
if (completion.symbols.size > 1)
if (sym.isType) printer.fullNameString(sym)
else {
printer.typeString(sym.denot.info.widenTermRefExpr)
}
else
completion.description.stripSuffix("$")
lazy val kind: CompletionItemKind = completionItemKind(sym)

val description = detailString(sym)

val ident = completion.label
val label = kind match {
case CompletionItemKind.Method =>
s"${ident}${description}"
case CompletionItemKind.Variable | CompletionItemKind.Field =>
s"${ident}: ${description}"
case _ => ident
}

val colonNotNeeded = sym.is(Method)
val colon = if (colonNotNeeded) "" else ": "
val label = s"${completion.label}$colon${description}"
val item = new CompletionItem(label)

item.setSortText(f"${idx}%05d")

item.setDetail(description)
item.setFilterText(completion.label)
// TODO we should use edit text
item.setInsertText(completion.label)
val documentation = ParsedComment.docOf(sym)

if (documentation.nonEmpty) {
item.setDocumentation(
hoverContent(None, None, documentation.toList)
)
item.setDocumentation(hoverContent(None, None, documentation.toList))
}
if (sym.isDeprecated) {
item.setTags(List(CompletionItemTag.Deprecated).asJava)
}
item.setKind(completionItemKind(sym))
item

}
}

Expand Down Expand Up @@ -546,134 +574,4 @@ case class ScalaPresentationCompiler(
}

override def isLoaded() = compilerAccess.isLoaded()

private case class ShortName(
name: Name,
symbol: Symbol
)
private object ShortName {
def apply(sym: Symbol)(using ctx: Context): ShortName =
ShortName(sym.name, sym)
}

private class ShortenedNames(context: Context) {
val history = collection.mutable.Map.empty[Name, ShortName]

def lookupSymbol(short: ShortName): Type = {
context.findRef(short.name)
}

def tryShortenName(short: ShortName)(using Context): Boolean = {
history.get(short.name) match {
case Some(ShortName(_, other)) => true
case None =>
val foundTpe = lookupSymbol(short)
val syms = List(foundTpe.termSymbol, foundTpe.typeSymbol)
val isOk = syms.filter(_ != NoSymbol) match {
case Nil => false
case founds => founds.exists(_ == short.symbol)
}
if (isOk) {
history(short.name) = short
true
} else {
false
}
}
}

def tryShortenName(name: Option[ShortName])(using Context): Boolean =
name match {
case Some(short) => tryShortenName(short)
case None => false
}
}

/**
* Shorten the long (fully qualified) type to shorter representation, so printers
* can obtain more readable form of type like `SrcPos` instead of `dotc.util.SrcPos`
* (if the name can be resolved from the context).
*
* For example,
* when the longType is like `TypeRef(TermRef(ThisType(TypeRef(NoPrefix,module class dotc)),module util),SrcPos)`,
* if `dotc.util.SrcPos` found from the scope, then `TypeRef(NoPrefix, SrcPos)`
* if not, and `dotc.util` found from the scope then `TypeRef(TermRef(NoPrefix, module util), SrcPos)`
*
* @see Scala 3/Internals/Type System https://dotty.epfl.ch/docs/internals/type-system.html
*/
private def shortType(longType: Type, history: ShortenedNames)(using
ctx: Context
): Type = {
val isVisited = collection.mutable.Set.empty[(Type, Option[ShortName])]
val cached = new ju.HashMap[(Type, Option[ShortName]), Type]()

def loop(tpe: Type, name: Option[ShortName]): Type = {
val key = tpe -> name
// NOTE: Prevent infinite recursion, see https://github.com/scalameta/metals/issues/749
if (isVisited(key)) return cached.getOrDefault(key, tpe)
isVisited += key
val result = tpe match {
case TypeRef(prefix, designator) =>
// designator is not necessarily an instance of `Symbol` and it's an instance of `Name`
// this can be seen, for example, when we are shortening the signature of 3rd party APIs.
val sym =
if (designator.isInstanceOf[Symbol])
designator.asInstanceOf[Symbol]
else tpe.typeSymbol
val short = ShortName(sym)
TypeRef(loop(prefix, Some(short)), sym)

case TermRef(prefix, designator) =>
val sym =
if (designator.isInstanceOf[Symbol])
designator.asInstanceOf[Symbol]
else tpe.termSymbol
val short = ShortName(sym)
if (history.tryShortenName(name)) NoPrefix
else TermRef(loop(prefix, None), sym)

case ThisType(tyref) =>
if (history.tryShortenName(name)) NoPrefix
else ThisType.raw(loop(tyref, None).asInstanceOf[TypeRef])

case mt @ MethodTpe(pnames, ptypes, restpe) if mt.isImplicitMethod =>
ImplicitMethodType(
pnames,
ptypes.map(loop(_, None)),
loop(restpe, None)
)
case mt @ MethodTpe(pnames, ptypes, restpe) =>
MethodType(pnames, ptypes.map(loop(_, None)), loop(restpe, None))

case pl @ PolyType(_, restpe) =>
PolyType(
pl.paramNames,
pl.paramInfos.map(bound =>
TypeBounds(loop(bound.lo, None), loop(bound.hi, None))
),
loop(restpe, None)
)
case ConstantType(value) => value.tpe
case SuperType(thistpe, supertpe) =>
SuperType(loop(thistpe, None), loop(supertpe, None))
case AppliedType(tycon, args) =>
AppliedType(loop(tycon, None), args.map(a => loop(a, None)))
case TypeBounds(lo, hi) =>
TypeBounds(loop(lo, None), loop(hi, None))
case ExprType(res) =>
ExprType(loop(res, None))
case AnnotatedType(parent, annot) =>
AnnotatedType(loop(parent, None), annot)
case AndType(tp1, tp2) =>
AndType(loop(tp1, None), loop(tp2, None))
case or @ OrType(tp1, tp2) =>
OrType(loop(tp1, None), loop(tp2, None), or.isSoft)
case t => t
}

cached.putIfAbsent(key, result)
result
}
loop(longType, None)
}
}
Loading

0 comments on commit 374f159

Please sign in to comment.