Skip to content

Commit

Permalink
Improve signature help by more stable position calculation + better n…
Browse files Browse the repository at this point in the history
…amed arg support (scala#19214)

Previously, signature help had unstable way of calculating active
parameter inside the signature help. It is now changed to work better
with erroneous trees such as unclosed openings.

It also adds a new feature for signature help, which will help user
navigate inside named arguments. Furthermore, it will now find reordered
named arguments, and display in the same way, by also marking the
remaining parameters that they are now required to be named.

Example:


https://github.com/lampepfl/dotty/assets/48657087/b181d2d5-60f0-46a5-b2df-a58aa5f07454

The following changes have been made:

    - we will stop supporting Signature Helps for unclosed openings. It is too hard to recover from such errors reliably and to track where we are with the cursor. All modern editors will close opening on input. It simplified the code, and is the best way forward.
    - if there is an error within a definition return type parameter, we will now fall back to source, instead of print error,
    - tuples and functions are now supported when they are returned from methods.
    - this rewrite made me change a few things, and I took the chance and made it support clause interleaving. There is now a proper test suite for that. It required changing the way we represent Signatures.
    - argument reordering now works for all parameter lists, not only the currently applied one,
    - type parameters now also correctly display documentation

I've changed the PR to use ShortenedTypePrinter and fixed some other minor issues like correctly marking reordered argumentss or hiding synthetic parameter names.

In the future, Signature help should be changed to return symbols from the compiler instead of already printed values, and only print them later using shared API of ShortenedTypePrinter in the Presentation Compiler. Ideally we'd want a shared implementation for label printing with completions, hovers and other features.
  • Loading branch information
rochala authored Jan 19, 2024
1 parent cacf2b6 commit 01171de
Show file tree
Hide file tree
Showing 21 changed files with 1,935 additions and 461 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ class InteractiveDriver(val settings: List[String]) extends Driver {
if (t.symbol.exists && t.hasType) {
if (!t.symbol.isCompleted) t.symbol.info = UnspecifiedErrorType
t.symbol.annotations.foreach { annot =>
/* In some cases annotations are are used on themself (possibly larger cycles).
/* In some cases annotations are used on themself (possibly larger cycles).
* This is the case with the java.lang.annotation.Target annotation, would end
* in an infinite loop while cleaning. The `seen` is added to ensure that those
* trees are not cleand twice.
Expand Down
410 changes: 255 additions & 155 deletions compiler/src/dotty/tools/dotc/util/Signatures.scala

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -552,13 +552,18 @@ class DottyLanguageServer extends LanguageServer
override def signatureHelp(params: TextDocumentPositionParams) = computeAsync { canceltoken =>
val uri = new URI(params.getTextDocument.getUri)
val driver = driverFor(uri)
implicit def ctx: Context = driver.currentCtx

val pos = sourcePosition(driver, uri, params.getPosition)
val trees = driver.openedTrees(uri)
val path = Interactive.pathTo(trees, pos)
val (paramN, callableN, signatures) = Signatures.signatureHelp(path, pos.span)
new SignatureHelp(signatures.map(signatureToSignatureInformation).asJava, callableN, paramN)
driver.compilationUnits.get(uri) match
case Some(unit) =>
given newCtx: Context = driver.currentCtx.fresh.setCompilationUnit(unit)
val pos = sourcePosition(driver, uri, params.getPosition)
val adjustedSpan = pos.span.withEnd(pos.span.end + 1)
val path = Interactive.pathTo(ctx.compilationUnit.tpdTree, adjustedSpan)
val (paramN, callableN, signatures) = Signatures.signatureHelp(path, adjustedSpan)

new SignatureHelp(signatures.map(signatureToSignatureInformation).asJava, callableN, paramN)

case _ => new SignatureHelp()

}

Expand Down Expand Up @@ -930,23 +935,25 @@ object DottyLanguageServer {

/** Convert `signature` to a `SignatureInformation` */
def signatureToSignatureInformation(signature: Signatures.Signature): lsp4j.SignatureInformation = {
val tparams = signature.tparams.map(Signatures.Param("", _))
val paramInfoss =
(tparams ::: signature.paramss.flatten).map(paramToParameterInformation)
(signature.paramss.flatten).map(paramToParameterInformation)
val paramLists =
if signature.paramss.forall(_.isEmpty) && tparams.nonEmpty then
""
else
signature.paramss
.map { paramList =>
val labels = paramList.map(_.show)
val prefix = if paramList.exists(_.isImplicit) then "using " else ""
labels.mkString(prefix, ", ", "")
}
.mkString("(", ")(", ")")
val tparamsLabel = if (signature.tparams.isEmpty) "" else signature.tparams.mkString("[", ", ", "]")
signature.paramss
.map { paramList =>
val labels = paramList.map(_.show)
val isImplicit = paramList.exists:
case p: Signatures.MethodParam => p.isImplicit
case _ => false
val prefix = if isImplicit then "using " else ""
val isTypeParams = paramList.forall(_.isInstanceOf[Signatures.TypeParam]) && paramList.nonEmpty
val wrap: String => String = label => if isTypeParams then
s"[$label]"
else
s"($label)"
wrap(labels.mkString(prefix, ", ", ""))
}.mkString
val returnTypeLabel = signature.returnType.map(t => s": $t").getOrElse("")
val label = s"${signature.name}$tparamsLabel$paramLists$returnTypeLabel"
val label = s"${signature.name}$paramLists$returnTypeLabel"
val documentation = signature.doc.map(DottyLanguageServer.markupContent)
val sig = new lsp4j.SignatureInformation(label)
sig.setParameters(paramInfoss.asJava)
Expand Down
Loading

0 comments on commit 01171de

Please sign in to comment.