Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ import dotty.tools.pc.printer.ShortenedTypePrinter.IncludeDefaultParam
import dotty.tools.pc.utils.InteractiveEnrichments.*
import org.eclipse.lsp4j as l

import dotty.tools.dotc.ast.tpd
import dotty.tools.dotc.core.Types.{MethodType, TypeVar}
import dotty.tools.dotc.util.Spans.Span

import scala.jdk.CollectionConverters.*
import scala.meta.pc.reports.ReportContext
import scala.meta.pc.OffsetParams
Expand Down Expand Up @@ -44,8 +48,9 @@ object SignatureHelpProvider:
.setPrinterFn(_ => ShortenedTypePrinter(search, IncludeDefaultParam.Never)(using indexedContext))

val (paramN, callableN, alternatives) = Signatures.signatureHelp(path, pos.span)
val refinedAlternatives = refineSignatures(alternatives, path, pos.span)(using driver.currentCtx)

val infos = alternatives.flatMap: signature =>
val infos = refinedAlternatives.flatMap: signature =>
signature.denot.map(signature -> _)

val signatureInfos = infos.map { case (signature, denot) =>
Expand Down Expand Up @@ -163,4 +168,99 @@ object SignatureHelpProvider:
markup.setValue(content.trim())
markup

private def refineSignatures(
help: List[Signatures.Signature],
path: List[tpd.Tree],
span: Span
)(using Context): List[Signatures.Signature] =
val enclosingApply = path.find {
case tpd.Apply(fun, _) => !fun.span.contains(span)
case _ => false
}

enclosingApply match
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggested using asSeenFrom methods and alike. This approach seems very manual and fragile. We should instead of fixing the created signature, fix the code that creates it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pointers.

I refactored the logic into Signatures.scala
using asSeenFrom and appliedTo as suggested. I also added the regression test for the parametric extension case.

The diff looks large mostly because I had to update the existing test expectations in SignatureHelpSuite to match the corrected signatures (e.g. showing [Int] instead of [T])

case Some(tpd.Apply(fun, _)) =>
help.map { signature =>
val matches = signature.denot.exists(_.symbol == fun.symbol)
if matches then
val (head, actions) = unwind(fun)
var currType = head.tpe.widenTermRefExpr
var actionIndex = 0

val newParamss = signature.paramss.map { paramList =>
paramList match
case (p: Signatures.MethodParam) :: _ =>
// Handle PolyTypes if they were skipped in signature but exist in type
while currType.isInstanceOf[dotty.tools.dotc.core.Types.PolyType] do
if actionIndex < actions.size && actions(actionIndex).isInstanceOf[tpd.TypeApply] then
val targs = actions(actionIndex).asInstanceOf[tpd.TypeApply].args.map(_.tpe)
currType = currType.appliedTo(targs)
actionIndex += 1
else
// Fallback: strip aliases or stop if strict matching fails
currType = currType.resultType

if currType.isInstanceOf[MethodType] then
val mt = currType.asInstanceOf[MethodType]
val paramNames = mt.paramNames.map(_.show)
val res = paramList.map {
case p: Signatures.MethodParam =>
val idx = paramNames.indexOf(p.name)
val shown = if idx >= 0 then mt.paramInfos(idx).widenTermRefExpr.show else p.tpe
if shown.contains("Any") || shown.contains("Nothing") || shown.contains("error") then p
else p.copy(tpe = shown)
case other => other
}
currType = mt.resultType
if actionIndex < actions.size && actions(actionIndex).isInstanceOf[tpd.Apply] then
actionIndex += 1
res
else paramList

case (p: Signatures.TypeParam) :: _ =>
if currType.isInstanceOf[dotty.tools.dotc.core.Types.PolyType] then
if actionIndex < actions.size && actions(actionIndex).isInstanceOf[tpd.TypeApply] then
val targs = actions(actionIndex).asInstanceOf[tpd.TypeApply].args.map(_.tpe)
val newParams = paramList.zip(targs).map {
case (p: Signatures.TypeParam, targ) =>
val shown = targ.show
if shown.contains("Any") || shown.contains("Nothing") || shown.contains("error") then p
else p.copy(tpe = shown)
case (p, _) => p
}
currType = currType.appliedTo(targs)
actionIndex += 1
newParams
else paramList
else paramList

case _ => paramList
}
// Update return type if we have consumed all actions and ended up with a result type
val finalType = currType.widen.show
val returnTypeLabel =
if actionIndex == actions.size && !finalType.contains("error") && !finalType.contains("Any") && !finalType.contains("Nothing") then Some(finalType)
else signature.returnType

signature.copy(paramss = newParamss, returnType = returnTypeLabel)
else signature
}
case _ => help

private def unwind(tree: tpd.Tree): (tpd.Tree, List[tpd.Tree]) =
tree match
case tpd.Apply(fn, _) =>
val (head, actions) = unwind(fn)
(head, actions :+ tree)
case tpd.TypeApply(fn, _) =>
val (head, actions) = unwind(fn)
(head, actions :+ tree)
case _ => (tree, Nil)

private def countParams(tree: tpd.Tree): Int =
tree match
case tpd.Apply(fun, _) => 1 + countParams(fun)
case tpd.TypeApply(fun, _) => 1 + countParams(fun)
case _ => 0

end SignatureHelpProvider
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ class SignatureHelpDocSuite extends BaseSignatureHelpSuite:
|}
""".stripMargin,
s"""Found documentation for scala/Option#fold().
|fold[B](ifEmpty: => B)(f: Int => B): B
| ^^^^^^^^^^^
| @param B Found documentation for type param B
|fold[String](ifEmpty: => String)(f: Int => String): String
| ^^^^^^^^^^^^^^^^
| @param String Found documentation for type param B
| @param ifEmpty Found documentation for param ifEmpty
| @param f Found documentation for param f
""".stripMargin
Expand All @@ -59,9 +59,9 @@ class SignatureHelpDocSuite extends BaseSignatureHelpSuite:
|}
""".stripMargin,
s"""|Found documentation for scala/Option#fold().
|fold[B](ifEmpty: => B)(f: Int => B): B
| ^^^^^^^^^^^^^
| @param B Found documentation for type param B
|fold[String](ifEmpty: => String)(f: Int => String): String
| ^^^^^^^^^^^^^^^^^^
| @param String Found documentation for type param B
| @param ifEmpty Found documentation for param ifEmpty
| @param f Found documentation for param f
|""".stripMargin
Expand All @@ -77,9 +77,9 @@ class SignatureHelpDocSuite extends BaseSignatureHelpSuite:
|}
""".stripMargin,
"""|Found documentation for scala/collection/LinearSeqOps#foldLeft().
|foldLeft[B](z: B)(op: (B, Int) => B): B
| ^^^^^^^^^^^^^^^^^
| @param B Found documentation for type param B
|foldLeft[Int](z: Int)(op: (Int, Int) => Int): Int
| ^^^^^^^^^^^^^^^^^^^^^
| @param Int Found documentation for type param B
| @param z Found documentation for param z
| @param op Found documentation for param op
|""".stripMargin
Expand Down Expand Up @@ -136,9 +136,9 @@ class SignatureHelpDocSuite extends BaseSignatureHelpSuite:
|}
""".stripMargin,
"""|Found documentation for java/util/Collections#singleton().
|singleton[T](o: T): java.util.Set[T]
| ^^^^
| @param T Found documentation for type param T
|singleton[Object](o: Object): java.util.Set[Object]
| ^^^^^^^^^
| @param Object Found documentation for type param T
| @param o Found documentation for param o
|""".stripMargin
)
Expand All @@ -151,7 +151,7 @@ class SignatureHelpDocSuite extends BaseSignatureHelpSuite:
|}
""".stripMargin,
"""|Found documentation for scala/util/control/Exception.Catch#
|Catch[T](pf: Catcher[T], fin: Option[Finally], rethrow: Throwable => Boolean)
|Catch[T](pf: Catcher[T], fin: Option[scala.util.control.Exception.Finally], rethrow: Throwable => Boolean)
| ^^^^^^^^^^^^^^
| @param pf Found documentation for param pf
| @param fin Found documentation for param fin
Expand Down Expand Up @@ -235,7 +235,7 @@ class SignatureHelpDocSuite extends BaseSignatureHelpSuite:
|}
""".stripMargin,
"""|Found documentation for scala/Some#
|Some[A](value: A)
| ^^^^^^^^
|Some[Int](value: Int): Some[Int]
| ^^^^^^^^^^
|""".stripMargin
)
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ class SignatureHelpInterleavingSuite extends BaseSignatureHelpSuite:
| pair[Int](@@1)[String]("1")
""".stripMargin,
"""
|pair[A](a: A)[B](b: B): (A, B)
| ^^^^
|pair[Int](a: Int)[B](b: B): (Int, B)
| ^^^^^^
|""".stripMargin
)

Expand All @@ -56,8 +56,8 @@ class SignatureHelpInterleavingSuite extends BaseSignatureHelpSuite:
| pair[Int](1)[String](@@"1")
""".stripMargin,
"""
|pair[A](a: A)[B](b: B): (A, B)
| ^^^^
|pair[Int](a: Int)[String](b: String): (Int, String)
| ^^^^^^^^^
|""".stripMargin
)

Expand All @@ -83,8 +83,8 @@ class SignatureHelpInterleavingSuite extends BaseSignatureHelpSuite:
| pair[Int](@@1)
""".stripMargin,
"""
|pair[A](a: A)[B](b: B): (A, B)
| ^^^^
|pair[Int](a: Int)[B](b: B): (Int, B)
| ^^^^^^
|""".stripMargin
)

Expand Down Expand Up @@ -121,8 +121,8 @@ class SignatureHelpInterleavingSuite extends BaseSignatureHelpSuite:
| pair(1@@)
""".stripMargin,
"""
|pair[A](a: A)[B](b: B): (A, B)
| ^^^^
|pair[Int](a: Int)[B](b: B): (Int, B)
| ^^^^^^
|""".stripMargin
)

Expand All @@ -147,8 +147,8 @@ class SignatureHelpInterleavingSuite extends BaseSignatureHelpSuite:
| pair(1)[String]("1"@@)
""".stripMargin,
"""
|pair[A](a: A)[B](b: B): (A, B)
| ^^^^
|pair[Int](a: Int)[String](b: String): (Int, String)
| ^^^^^^^^^
|""".stripMargin
)

Expand All @@ -160,8 +160,8 @@ class SignatureHelpInterleavingSuite extends BaseSignatureHelpSuite:
| pair(1)("1"@@)
""".stripMargin,
"""
|pair[A](a: A)[B](b: B): (A, B)
| ^^^^
|pair[Int](a: Int)[String](b: String): (Int, String)
| ^^^^^^^^^
|""".stripMargin
)

Expand All @@ -186,8 +186,8 @@ class SignatureHelpInterleavingSuite extends BaseSignatureHelpSuite:
| pair[Int](@@)
""".stripMargin,
"""
|pair[A](a: A)[B](b: B): (A, B)
| ^^^^
|pair[Int](a: Int)[B](b: B): (Int, B)
| ^^^^^^
|""".stripMargin
)

Expand All @@ -212,8 +212,8 @@ class SignatureHelpInterleavingSuite extends BaseSignatureHelpSuite:
| pair[Int](1)[String](@@)
""".stripMargin,
"""
|pair[A](a: A)[B](b: B): (A, B)
| ^^^^
|pair[Int](a: Int)[String](b: String): (Int, String)
| ^^^^^^^^^
|""".stripMargin
)

Expand All @@ -238,8 +238,8 @@ class SignatureHelpInterleavingSuite extends BaseSignatureHelpSuite:
| pair(1)(@@)
""".stripMargin,
"""
|pair[A](a: A)[B](b: B): (A, B)
| ^^^^
|pair[Int](a: Int)[B](b: B): (A, B)
| ^^^^
|""".stripMargin
)

Expand Down Expand Up @@ -329,8 +329,8 @@ class SignatureHelpInterleavingSuite extends BaseSignatureHelpSuite:
| pair[String]()[Int](@@)
""".stripMargin,
"""
|pair[A](a: A)[B](b: B): (A, B)
| ^^^^
|pair[String](a: String)[Int](b: Int): (String, Int)
| ^^^^^^
|""".stripMargin
)

Expand All @@ -355,8 +355,8 @@ class SignatureHelpInterleavingSuite extends BaseSignatureHelpSuite:
| pair[String](52)[Int](""@@)
""".stripMargin,
"""
|pair[A](a: A)[B](b: B): (A, B)
| ^^^^
|pair[String](a: String)[Int](b: Int): (String, Int)
| ^^^^^^
|""".stripMargin
)

Expand All @@ -368,8 +368,8 @@ class SignatureHelpInterleavingSuite extends BaseSignatureHelpSuite:
| foo[Int](using 1)(List(1, 2, 3))(@@)
""".stripMargin,
"""
|foo[A](using a: A)(b: List[A])[C <: a.type, D](cd: (C, D))[E]: Foo[A, B, C, D, E]
| ^^^^^^^^^^
|foo[Int](using a: Int)(b: List[Int])[(1 : Int), D](cd: (C, D))[E]: Foo[A, B, C, D, E]
| ^^^^^^^^^^
|""".stripMargin
)

Expand Down Expand Up @@ -418,8 +418,8 @@ class SignatureHelpInterleavingSuite extends BaseSignatureHelpSuite:
| def test[A](a: A)[B](b: B)[C](c: C)[D](d: D): Int = ???
| test[Int](@@)
|""".stripMargin,
"""|test[A](a: A)[B](b: B)[C](c: C)[D](d: D): Int
| ^^^^
"""|test[Int](a: Int)[B](b: B)[C](c: C)[D](d: D): Int
| ^^^^^^
|""".stripMargin
)

Expand All @@ -442,8 +442,8 @@ class SignatureHelpInterleavingSuite extends BaseSignatureHelpSuite:
| def test[A](a: A)[B](b: B)[C](c: C)[D](d: D): Int = ???
| test[Int](1)[String](@@)
|""".stripMargin,
"""|test[A](a: A)[B](b: B)[C](c: C)[D](d: D): Int
| ^^^^
"""|test[Int](a: Int)[String](b: String)[C](c: C)[D](d: D): Int
| ^^^^^^^^^
|""".stripMargin
)

Expand All @@ -466,8 +466,8 @@ class SignatureHelpInterleavingSuite extends BaseSignatureHelpSuite:
| def test[A](a: A)[B](b: B)[C](c: C)[D](d: D): Int = ???
| test[Int](1)[String]("1")[Int](@@)
|""".stripMargin,
"""|test[A](a: A)[B](b: B)[C](c: C)[D](d: D): Int
| ^^^^
"""|test[Int](a: Int)[String](b: String)[Int](c: Int)[D](d: D): Int
| ^^^^^^
|""".stripMargin
)

Expand All @@ -488,8 +488,8 @@ class SignatureHelpInterleavingSuite extends BaseSignatureHelpSuite:
| def test[A](a: A)[B](b: B)[C](c: C)[D](d: D): Int = ???
| test[Int](1)[String]("1")[Int](2)[String](@@)
|""".stripMargin,
"""|test[A](a: A)[B](b: B)[C](c: C)[D](d: D): Int
| ^^^^
"""|test[Int](a: Int)[String](b: String)[Int](c: Int)[String](d: String): Int
| ^^^^^^^^^
|""".stripMargin
)

Expand All @@ -499,8 +499,8 @@ class SignatureHelpInterleavingSuite extends BaseSignatureHelpSuite:
| def test[A](a: A)(using Int)[B](b: B)[C](c: C)[D](d: D): Int = ???
| test[Int](1)(using 5)[String]("1")[Int](@@)
|""".stripMargin,
"""|test[A](a: A)(using Int)[B](b: B)[C](c: C)[D](d: D): Int
| ^^^^
"""|test[Int](a: Int)(using Int)[String](b: String)[Int](c: Int)[D](d: D): Int
| ^^^^^^
|""".stripMargin
)

Expand All @@ -510,8 +510,7 @@ class SignatureHelpInterleavingSuite extends BaseSignatureHelpSuite:
| def test[A](a: A)(using Int)[B](b: B)[C](c: C)[D](d: D): Int = ???
| test[Int](1)(5)[String]("1")[Int](@@)
|""".stripMargin,
"""|test[A](a: A)(using Int)[B](b: B)[C](c: C)[D](d: D): Int
| ^^^^
"""|test[Int](a: Int)(using Int)[B](b: B)[String](c: String)[Int](d: Int): Int
| ^^^^^^^^^
|""".stripMargin
)

Loading
Loading