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.

Sign up for GitHub

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

Add proper signatureHelp for unapply methods #15203

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ae580af
add proper unappply signatures for signatureHelp
rochala May 13, 2022
143177c
empty line, small cosmetic errors fix
rochala May 13, 2022
9bddab3
rename val
rochala May 13, 2022
4c05795
add tests from language reference and fix accordingly
rochala May 16, 2022
db8b2a3
fix unapplySeq parameter position
rochala May 17, 2022
cec2001
fix tests, active parameter position and simplify code
rochala May 17, 2022
7627258
remove duplicated method available by extension method
rochala May 18, 2022
baeb5a0
add comments, simplify code and add missing cases
rochala May 19, 2022
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ import ast.tpd
import core.Constants.Constant
import core.Contexts._
import core.Denotations.SingleDenotation
import core.Flags
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 +36,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 +48,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 +84,55 @@ 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 {
case ref: TypeRef if !ref.symbol.isPrimitiveValueClass =>
if (
ref.symbol.asClass.baseClasses.contains(ctx.definitions.ProductClass) &&
!ref.symbol.is(Flags.CaseClass)
) || ref.symbol.isStaticOwner then
val productAccessors = ref.memberDenots(
underscoreMembersFilter,
(name, buf) => buf += ref.member(name).asSingleDenotation
)
List(productAccessors.map(_.info.finalResultType).toList)
else
ref.symbol.primaryConstructor.paramInfo.paramInfoss
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)
case AppliedType(_, args) =>
List(args)
case MethodTpe(_, _, resultType) =>
extractParamTypess(resultType.widenDealias)
case _ =>
Nil
}

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 +149,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 +159,43 @@ object Signatures {
params :: rest
}

/**
* This function is a hack which allows Signatures API to remain unchanged
*
* @return true if denot is "unapply" or "unapplySeq", false otherwise
*/
def isUnapplyDenotation: Boolean =
List(core.Names.termName("unapply"), core.Names.termName("unapplySeq")) contains denot.name

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 isUnapplyDenotation =>
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 @@ -141,6 +223,11 @@ object Signatures {
}
}

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

/**
* The number of parameters that are applied in `tree`.
*
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