Skip to content

Add proper signatureHelp for unapply methods #15203

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
May 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
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
120 changes: 103 additions & 17 deletions compiler/src/dotty/tools/dotc/util/Signatures.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ import ast.tpd
import core.Constants.Constant
import core.Contexts._
import core.Denotations.SingleDenotation
import core.Flags
import core.NameOps.isUnapplyName
import core.Names._
import core.Types._
import util.Spans.Span
import core.Types.{ErrorType, MethodType, PolyType}
import reporting._

import dotty.tools.dotc.core.Types.Type

object Signatures {

Expand All @@ -35,8 +37,7 @@ object Signatures {
* @param isImplicit Is this parameter implicit?
*/
case class Param(name: String, tpe: String, doc: Option[String] = None, isImplicit: Boolean = false) {
def show: String =
s"$name: $tpe"
def show: String = if name.nonEmpty then s"$name: $tpe" else tpe
}

/**
Expand All @@ -48,21 +49,21 @@ object Signatures {
* being called, the list of overloads of this function).
*/
def callInfo(path: List[tpd.Tree], span: Span)(using Context): (Int, Int, List[SingleDenotation]) =
val enclosingApply = path.find {
case Apply(fun, _) => !fun.span.contains(span)
case UnApply(fun, _, _) => !fun.span.contains(span)
case _ => false
}
val enclosingApply = path.dropWhile {
case apply @ Apply(fun, _) => fun.span.contains(span) || apply.span.end == span.end
case unapply @ UnApply(fun, _, _) => fun.span.contains(span) || unapply.span.end == span.end || isTuple(unapply)
case _ => true
}.headOption

enclosingApply.map {
case UnApply(fun, _, patterns) => callInfo(span, patterns, fun, Signatures.countParams(fun))
case UnApply(fun, _, patterns) => unapplyCallInfo(span, fun, patterns)
case Apply(fun, params) => callInfo(span, params, fun, Signatures.countParams(fun))
}.getOrElse((0, 0, Nil))

def callInfo(
span: Span,
params: List[Tree[Type]],
fun: Tree[Type],
params: List[tpd.Tree],
fun: tpd.Tree,
alreadyAppliedCount : Int
)(using Context): (Int, Int, List[SingleDenotation]) =
val paramIndex = params.indexWhere(_.span.contains(span)) match {
Expand All @@ -84,10 +85,67 @@ object Signatures {

(paramIndex, alternativeIndex, alternatives)

private def unapplyCallInfo(
span: Span,
fun: tpd.Tree,
patterns: List[tpd.Tree]
)(using Context): (Int, Int, List[SingleDenotation]) =
val patternPosition = patterns.indexWhere(_.span.contains(span))
val activeParameter = extractParamTypess(fun.tpe.finalResultType.widen).headOption.map { params =>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possible performance optimization as this is computed twice. We need to find number of parameters we can unapply to but i can't pass it without changing the API

(patternPosition, patterns.length) match
case (-1, 0) => 0 // there are no patterns yet so it must be first one
case (-1, pos) => -1 // there are patterns, we must be outside range so we set no active parameter
case _ => (params.size - 1) min patternPosition max 0 // handle unapplySeq to always highlight Seq[A] on elements
}.getOrElse(-1)

val appliedDenot = fun.symbol.asSingleDenotation.mapInfo(_ => fun.tpe) :: Nil
(activeParameter, 0, appliedDenot)

private def isTuple(tree: tpd.Tree)(using Context): Boolean =
ctx.definitions.isTupleClass(tree.symbol.owner.companionClass)

private def extractParamTypess(resultType: Type)(using Context): List[List[Type]] =
resultType match {
// Reference to a type which is not a type class
case ref: TypeRef if !ref.symbol.isPrimitiveValueClass =>
getExtractorMembers(ref)
// Option or Some applied type. There is special syntax for multiple returned arguments:
// Option[TupleN] and Option[Seq],
// We are not intrested in them, instead we extract proper type parameters from the Option type parameter.
case AppliedType(TypeRef(_, cls), (appliedType @ AppliedType(tycon, args)) :: Nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you able to add examples to each case to say in which situation each is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I managed to simplify this logic and added comments why each case is necessary for proper handling

Copy link
Contributor

Choose a reason for hiding this comment

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

awesome, thanks!

if (cls == ctx.definitions.OptionClass || cls == ctx.definitions.SomeClass) =>
tycon match
case TypeRef(_, cls) if cls == ctx.definitions.SeqClass => List(List(appliedType))
case _ => List(args)
// Applied type extractor. We must extract from applied type to retain type parameters
case appliedType: AppliedType => getExtractorMembers(appliedType)
// This is necessary to extract proper result type as unapply can return other methods eg. apply
case MethodTpe(_, _, resultType) =>
extractParamTypess(resultType.widenDealias)
case _ =>
Nil
}

// Returns extractors from given type. In case if there are no extractor methods it fallbacks to get method
private def getExtractorMembers(resultType: Type)(using Context): List[List[Type]] =
val productAccessors = resultType.memberDenots(
underscoreMembersFilter,
(name, buf) => buf += resultType.member(name).asSingleDenotation
)
val availableExtractors = if productAccessors.isEmpty then
List(resultType.member(core.Names.termName("get")))
else
productAccessors
List(availableExtractors.map(_.info.finalResultType.stripAnnots).toList)

object underscoreMembersFilter extends NameFilter {
def apply(pre: Type, name: Name)(using Context): Boolean = name.startsWith("_")
def isStable = true
}

def toSignature(denot: SingleDenotation)(using Context): Option[Signature] = {
val symbol = denot.symbol
val docComment = ParsedComment.docOf(symbol)
val classTree = symbol.topLevelClass.asClass.rootTree

def toParamss(tp: MethodType)(using Context): List[List[Param]] = {
val rest = tp.resType match {
Expand All @@ -104,7 +162,8 @@ object Signatures {
Nil
}
val params = tp.paramNames.zip(tp.paramInfos).map { case (name, info) =>
Signatures.Param(name.show,
Signatures.Param(
name.show,
info.widenTermRefExpr.show,
docComment.flatMap(_.paramDoc(name)),
isImplicit = tp.isImplicitMethod)
Expand All @@ -113,7 +172,35 @@ object Signatures {
params :: rest
}

def extractParamNamess(resultType: Type): List[List[Name]] =
if resultType.typeSymbol.flags.is(Flags.CaseClass) && symbol.flags.is(Flags.Synthetic) then
resultType.typeSymbol.primaryConstructor.paramInfo.paramNamess
else
Nil

def toUnapplyParamss(method: Type)(using Context): List[Param] = {
val resultType = method.finalResultType.widenDealias match
case methodType: MethodType => methodType.resultType.widen
case other => other

val paramNames = extractParamNamess(resultType).flatten
val paramTypes = extractParamTypess(resultType).flatten

if paramNames.length == paramTypes.length then
(paramNames zip paramTypes).map((name, info) => Param(name.show, info.show))
else
paramTypes.map(info => Param("", info.show))

}

denot.info.stripPoly match {
case tpe if denot.name.isUnapplyName =>
val params = toUnapplyParamss(tpe)
if params.nonEmpty then
Some(Signature("", Nil, List(params), None))
else
None

case tpe: MethodType =>
val paramss = toParamss(tpe)
val typeParams = denot.info match {
Expand Down Expand Up @@ -174,7 +261,7 @@ object Signatures {
err.msg match
case msg: AmbiguousOverload => msg.alternatives
case msg: NoMatchingOverload => msg.alternatives
case _ => Nil
case _ => Nil

// If the user writes `foo(bar, <cursor>)`, the typer will insert a synthetic
// `null` parameter: `foo(bar, null)`. This may influence what's the "best"
Expand All @@ -191,8 +278,7 @@ object Signatures {
alt.info.stripPoly match {
case tpe: MethodType =>
userParamsTypes.zip(tpe.paramInfos).takeWhile{ case (t0, t1) => t0 <:< t1 }.size
case _ =>
0
case _ => 0
}
}
val bestAlternative =
Expand Down
Loading