From 52e6265c8851bc62d7673e94b5818a5032b2aa91 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Mon, 5 Nov 2018 08:59:31 +0100 Subject: [PATCH 1/4] Error message for `none of the overloaded...` This error message allows us to keep track of all the possible alternatives. --- .../dotc/reporting/diagnostic/ErrorMessageID.java | 3 ++- .../dotc/reporting/diagnostic/messages.scala | 15 +++++++++++++-- compiler/src/dotty/tools/dotc/typer/Typer.scala | 4 +--- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java b/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java index 0a33a207c94c..1877f68df87d 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java @@ -140,7 +140,8 @@ public enum ErrorMessageID { TraitCompanionWithMutableStaticID, LazyStaticFieldID, StaticOverridingNonStaticMembersID, - OverloadInRefinementID + OverloadInRefinementID, + NoMatchingOverloadID ; public int errorNumber() { diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index a9811d1da065..fd21974a7ccc 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -23,6 +23,7 @@ import dotty.tools.dotc.ast.Trees import dotty.tools.dotc.config.ScalaVersion import dotty.tools.dotc.core.Flags._ import dotty.tools.dotc.core.SymDenotations.SymDenotation +import dotty.tools.dotc.typer.ErrorReporting.Errors import scala.util.control.NonFatal object messages { @@ -1382,7 +1383,7 @@ object messages { } case class AmbiguousOverload(tree: tpd.Tree, alts: List[SingleDenotation], pt: Type)( - err: typer.ErrorReporting.Errors)( + err: Errors)( implicit ctx: Context) extends Message(AmbiguousOverloadID) { @@ -1463,7 +1464,7 @@ object messages { } case class DoesNotConformToBound(tpe: Type, which: String, bound: Type)( - err: typer.ErrorReporting.Errors)(implicit ctx: Context) + err: Errors)(implicit ctx: Context) extends Message(DoesNotConformToBoundID) { val msg: String = hl"Type argument ${tpe} does not conform to $which bound $bound ${err.whyNoMatchStr(tpe, bound)}" val kind: String = "Type Mismatch" @@ -2181,4 +2182,14 @@ object messages { hl"""The refinement `$rsym` introduces an overloaded definition. |Refinements cannot contain overloaded definitions.""".stripMargin } + + case class NoMatchingOverload(alternatives: List[SingleDenotation], pt: Type)( + err: Errors)(implicit val ctx: Context) + extends Message(NoMatchingOverloadID) { + val msg: String = + hl"""None of the ${err.overloadedAltsStr(alternatives)} + |match ${err.expectedTypeStr(pt)}""" + val kind: String = "Type Mismatch" + val explanation: String = "" + } } diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index d4ab901f9198..cea957340d90 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2234,9 +2234,7 @@ class Typer extends Namer readaptSimplified(tree.withType(alt)) case Nil => def noMatches = - errorTree(tree, - em"""none of the ${err.overloadedAltsStr(altDenots)} - |match ${err.expectedTypeStr(pt)}""") + errorTree(tree, NoMatchingOverload(altDenots, pt)(err)) def hasEmptyParams(denot: SingleDenotation) = denot.info.paramInfoss == ListOfNil pt match { case pt: FunProto => From a94de7f2d6250338e2bfdc43fba190531457f81d Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Mon, 5 Nov 2018 20:24:14 +0100 Subject: [PATCH 2/4] IDE: Support `textDocument/signatureHelp` When the user starts writing a function application, the language client requests `textDocument/signatureHelp`. This is used to display help associated with the function application that is being written. --- .../languageserver/DottyLanguageServer.scala | 71 +++- .../tools/languageserver/Signatures.scala | 143 ++++++++ .../languageserver/SignatureHelpTest.scala | 312 ++++++++++++++++++ .../languageserver/util/CodeTester.scala | 18 + .../util/actions/SignatureHelp.scala | 64 ++++ 5 files changed, 605 insertions(+), 3 deletions(-) create mode 100644 language-server/src/dotty/tools/languageserver/Signatures.scala create mode 100644 language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala create mode 100644 language-server/test/dotty/tools/languageserver/util/actions/SignatureHelp.scala diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index b5ad03887d1e..94e7227df665 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -19,7 +19,7 @@ import scala.io.Codec import dotc._ import ast.{Trees, tpd} import core._, core.Decorators.{sourcePos => _, _} -import Comments._, Contexts._, Flags._, Names._, NameOps._, Symbols._, SymDenotations._, Trees._, Types._ +import Comments._, Constants._, Contexts._, Flags._, Names._, NameOps._, Symbols._, SymDenotations._, Trees._, Types._ import classpath.ClassPathEntries import reporting._, reporting.diagnostic.{Message, MessageContainer, messages} import typer.Typer @@ -198,6 +198,8 @@ class DottyLanguageServer extends LanguageServer c.setCompletionProvider(new CompletionOptions( /* resolveProvider = */ false, /* triggerCharacters = */ List(".").asJava)) + c.setSignatureHelpProvider(new SignatureHelpOptions( + /* triggerCharacters = */ List("(").asJava)) // Do most of the initialization asynchronously so that we can return early // from this method and thus let the client know our capabilities. @@ -457,6 +459,71 @@ class DottyLanguageServer extends LanguageServer implementations.flatten.asJava } + override def signatureHelp(params: TextDocumentPositionParams) = computeAsync { canceltoken => + + val uri = new URI(params.getTextDocument.getUri) + val driver = driverFor(uri) + implicit val ctx = driver.currentCtx + + val pos = sourcePosition(driver, uri, params.getPosition) + val trees = driver.openedTrees(uri) + val path = Interactive.pathTo(trees, pos).dropWhile(!_.isInstanceOf[Apply]) + + val (paramN, callableN, alternatives) = Signatures.callInfo(path, pos.pos) + + val signatureInfos = alternatives.flatMap { denot => + val symbol = denot.symbol + val docComment = ParsedComment.docOf(symbol) + val classTree = symbol.topLevelClass.asClass.rootTree + val isImplicit: TermName => Boolean = tpd.defPath(symbol, classTree).lastOption match { + case Some(DefDef(_, _, paramss, _, _)) => + val flatParams = paramss.flatten + name => flatParams.find(_.name == name).map(_.symbol.is(Implicit)).getOrElse(false) + case _ => + _ => false + } + + denot.info.stripPoly match { + case tpe: MethodType => + val infos = { + tpe.paramInfoss.zip(tpe.paramNamess).map { (infos, names) => + infos.zip(names).map { (info, name) => + Signatures.Param(name.show, + info.widenTermRefExpr.show, + docComment.flatMap(_.paramDoc(name)), + isImplicit = isImplicit(name)) + } + } + } + + val typeParams = denot.info match { + case poly: PolyType => + poly.paramNames.zip(poly.paramInfos).map((x, y) => x.show + y.show) + case _ => + Nil + } + + val (name, returnType) = + if (symbol.isConstructor) (symbol.owner.name.show, None) + else (denot.name.show, Some(tpe.finalResultType.widenTermRefExpr.show)) + + val signature = + Signatures.Signature(name, + typeParams, + infos, + returnType, + docComment.map(_.mainDoc)) + + Some(signature) + + case other => + None + } + } + + new SignatureHelp(signatureInfos.map(_.toSignatureInformation).asJava, callableN, paramN) + } + override def getTextDocumentService: TextDocumentService = this override def getWorkspaceService: WorkspaceService = this @@ -469,7 +536,6 @@ class DottyLanguageServer extends LanguageServer override def onTypeFormatting(params: DocumentOnTypeFormattingParams) = null override def resolveCodeLens(params: CodeLens) = null override def resolveCompletionItem(params: CompletionItem) = null - override def signatureHelp(params: TextDocumentPositionParams) = null /** * Find the set of projects that have any of `definitions` on their classpath. @@ -511,7 +577,6 @@ class DottyLanguageServer extends LanguageServer } } - } object DottyLanguageServer { diff --git a/language-server/src/dotty/tools/languageserver/Signatures.scala b/language-server/src/dotty/tools/languageserver/Signatures.scala new file mode 100644 index 000000000000..6bff01943e02 --- /dev/null +++ b/language-server/src/dotty/tools/languageserver/Signatures.scala @@ -0,0 +1,143 @@ +package dotty.tools.languageserver + +import dotty.tools.dotc.ast.tpd._ +import dotty.tools.dotc.core.Constants.Constant +import dotty.tools.dotc.core.Contexts.Context +import dotty.tools.dotc.core.Denotations.SingleDenotation +import dotty.tools.dotc.util.Positions.Position +import dotty.tools.dotc.core.Types.{ErrorType, MethodType} +import dotty.tools.dotc.reporting.diagnostic.messages + +import org.eclipse.lsp4j.{ParameterInformation, SignatureInformation} + +import scala.collection.JavaConverters._ + +object Signatures { + + def callInfo(path: List[Tree], pos: Position)(implicit ctx: Context): (Int, Int, List[SingleDenotation]) = { + path match { + case Apply(fun, params) :: _ => + val alreadyAppliedCount = Signatures.countParams(fun) + val paramIndex = params.indexWhere(_.pos.contains(pos)) match { + case -1 => (params.length - 1 max 0) + alreadyAppliedCount + case n => n + alreadyAppliedCount + } + + val (alternativeIndex, alternatives) = fun.tpe match { + case err: ErrorType => + val (alternativeIndex, alternatives) = alternativesFromError(err, params) + (alternativeIndex, alternatives) + + case _ => + val funSymbol = fun.symbol + val alternatives = funSymbol.owner.info.member(funSymbol.name).alternatives + val alternativeIndex = alternatives.indexOf(funSymbol.denot) max 0 + (alternativeIndex, alternatives) + } + + (paramIndex, alternativeIndex, alternatives) + + case _ => + (0, 0, Nil) + } + } + + /** + * The number of parameters that are applied in `tree`. + * + * This handles currying, so for an application such as `foo(1, 2)(3)`, the result of + * `countParams` should be 3. + * + * @param tree The tree to inspect. + * @return The number of parameters that are passed. + */ + private def countParams(tree: Tree): Int = { + tree match { + case Apply(fun, params) => countParams(fun) + params.length + case _ => 0 + } + } + + /** + * Inspect `err` to determine, if it is an error related to application of an overloaded + * function, what were the possible alternatives. + * + * If several alternatives are found, determines what is the best suited alternatives + * given the parameters `params`: The alternative that has the most formal parameters + * matching the given arguments is chosen. + * + * @param err The error message to inspect. + * @param params The parameters that were given at the call site. + * @return A pair composed of the index of the best alternative (0 if no alternatives + * were found), and the list of alternatives. + */ + private def alternativesFromError(err: ErrorType, params: List[Tree])(implicit ctx: Context): (Int, List[SingleDenotation]) = { + val alternatives = + err.msg match { + case messages.AmbiguousOverload(_, alternatives, _) => + alternatives + case messages.NoMatchingOverload(alternatives, _) => + alternatives + case _ => + Nil + } + + // If the user writes `foo(bar, )`, the typer will insert a synthetic + // `null` parameter: `foo(bar, null)`. This may influence what's the "best" + // alternative, so we discard it. + val userParams = params match { + case xs :+ (nul @ Literal(Constant(null))) if nul.pos.isZeroExtent => xs + case _ => params + } + val userParamsTypes = userParams.map(_.tpe) + + // Assign a score to each alternative (how many parameters are correct so far), and + // use that to determine what is the current active signature. + val alternativesScores = alternatives.map { alt => + alt.info.stripPoly match { + case tpe: MethodType => + userParamsTypes.zip(tpe.paramInfos).takeWhile(_ <:< _).size + case _ => + 0 + } + } + val bestAlternative = + if (alternativesScores.isEmpty) 0 + else alternativesScores.zipWithIndex.maxBy(_._1)._2 + + (bestAlternative, alternatives) + } + + case class Signature(name: String, tparams: List[String], paramss: List[List[Param]], returnType: Option[String], doc: Option[String] = None) { + def toSignatureInformation: SignatureInformation = { + val paramInfoss = paramss.map(_.map(_.toParameterInformation)) + val paramLists = paramss.map { paramList => + val labels = paramList.map(_.show) + val prefix = if (paramList.exists(_.isImplicit)) "implicit " else "" + labels.mkString(prefix, ", ", "") + }.mkString("(", ")(", ")") + val tparamsLabel = if (tparams.isEmpty) "" else tparams.mkString("[", ", ", "]") + val returnTypeLabel = returnType.map(t => s": $t").getOrElse("") + val label = s"$name$tparamsLabel$paramLists$returnTypeLabel" + val documentation = doc.map(DottyLanguageServer.hoverContent) + val signature = new SignatureInformation(label) + signature.setParameters(paramInfoss.flatten.asJava) + documentation.foreach(signature.setDocumentation(_)) + signature + } + } + + case class Param(name: String, tpe: String, doc: Option[String] = None, isImplicit: Boolean = false) { + + def toParameterInformation: ParameterInformation = { + val label = s"$name: $tpe" + val documentation = doc.map(DottyLanguageServer.hoverContent) + val info = new ParameterInformation(label) + documentation.foreach(info.setDocumentation(_)) + info + } + + def show: String = + s"$name: $tpe" + } +} diff --git a/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala b/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala new file mode 100644 index 000000000000..670d82f6cf5e --- /dev/null +++ b/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala @@ -0,0 +1,312 @@ +package dotty.tools.languageserver + +import org.junit.Test + +import dotty.tools.languageserver.util.Code._ +import dotty.tools.languageserver.Signatures.{Param => P, Signature => S} + +class SignatureHelpTest { + + @Test def singleParam: Unit = { + val signature = + S("foo", Nil, List(List(P("param0", "Int"))), Some("Int")) + code"""object O { + def foo(param0: Int): Int = ??? + foo($m1) + foo(0$m2) + }""".withSource + .signatureHelp(m1, List(signature), Some(0), 0) + .signatureHelp(m2, List(signature), Some(0), 0) + } + + @Test def twoParams: Unit = { + val signature = S("foo", Nil, List(List(P("param0", "Int"), P("param1", "String"))), Some("Int")) + code"""object O { + def foo(param0: Int, param1: String): Int = ??? + foo($m1) + foo(0, $m2) + foo(0, "$m3") + }""".withSource + .signatureHelp(m1, List(signature), Some(0), 0) + .signatureHelp(m2, List(signature), Some(0), 1) + .signatureHelp(m3, List(signature), Some(0), 1) + } + + @Test def noMatchingOverload: Unit = { + val sig0 = S("foo", Nil, List(List(P("param0", "Int"))), Some("Nothing")) + val sig1 = S("foo", Nil, List(List(P("param1", "String"))), Some("Nothing")) + + code"""object O { + def foo(param0: Int): Nothing = ??? + def foo(param1: String): Nothing = ??? + foo($m1) + foo(0$m2) + foo(""$m3) + }""".withSource + .signatureHelp(m1, List(sig0, sig1), None, 0) + .signatureHelp(m2, List(sig0, sig1), Some(0), 0) + .signatureHelp(m3, List(sig0, sig1), Some(1), 0) + } + + @Test def singleMatchingOverload: Unit = { + val sig0 = S("foo", Nil, List(List(P("param0", "Int"), P("param1", "String"))), Some("Nothing")) + val sig1 = S("foo", Nil, List(List(P("param0", "String"), P("param1", "Int"))), Some("Nothing")) + code"""object O { + def foo(param0: Int, param1: String): Nothing = ??? + def foo(param0: String, param1: Int): Nothing = ??? + foo($m1) + foo(0$m2) + foo(""$m3) + foo(0, $m4) + foo("", $m5) + }""".withSource + .signatureHelp(m1, List(sig0, sig1), None, 0) + .signatureHelp(m2, List(sig0, sig1), Some(0), 0) + .signatureHelp(m3, List(sig0, sig1), Some(1), 0) + .signatureHelp(m4, List(sig0, sig1), Some(0), 1) + .signatureHelp(m5, List(sig0, sig1), Some(1), 1) + } + + @Test def multipleMatchingOverloads: Unit = { + val sig0 = S("foo", Nil, List(List(P("param0", "Int"), P("param1", "Int"))), Some("Nothing")) + val sig1 = S("foo", Nil, List(List(P("param0", "Int"), P("param1", "Boolean"))), Some("Nothing")) + val sig2 = S("foo", Nil, List(List(P("param0", "String"), P("param1", "Int"))), Some("Nothing")) + val sigs = List(sig0, sig1, sig2) + code"""object O { + def foo(param0: Int, param1: Int): Nothing = ??? + def foo(param0: Int, param1: Boolean): Nothing = ??? + def foo(param0: String, param1: Int): Nothing = ??? + foo($m1) + foo(0$m2) + foo(""$m3) + foo(0, $m4) + foo("", $m5) + foo(0, 0$m6) + foo(0, ""$m7) + foo("", 0$m8) + }""".withSource + .signatureHelp(m1, sigs, None, 0) + .signatureHelp(m2, sigs, None, 0) + .signatureHelp(m3, sigs, Some(2), 0) + .signatureHelp(m4, sigs, None, 1) + .signatureHelp(m5, sigs, Some(2), 1) + .signatureHelp(m6, sigs, Some(0), 1) + .signatureHelp(m7, sigs, Some(1), 1) + .signatureHelp(m8, sigs, Some(2), 1) + } + + @Test def ambiguousOverload: Unit = { + val sig0 = S("foo", Nil, List(List(P("param0", "String")), List(P("param1", "String"))), Some("Nothing")) + val sig1 = S("foo", Nil, List(List(P("param0", "String"))), Some("Nothing")) + code"""object O { + def foo(param0: String)(param1: String): Nothing = ??? + def foo(param0: String): Nothing = ??? + foo($m1) + foo(""$m2) + foo("")($m3) + }""".withSource + .signatureHelp(m1, List(sig0, sig1), None, 0) + .signatureHelp(m2, List(sig0, sig1), None, 0) + .signatureHelp(m3, List(sig0, sig1), Some(1), 1) + } + + @Test def multipleParameterLists: Unit = { + val signature = + S("foo", + Nil, + List( + List(P("param0", "Int"), P("param1", "Int")), + List(P("param2", "Int")), + List(P("param3", "Int"), P("param4", "Int")) + ), + Some("Int")) + code"""object O { + def foo(param0: Int, param1: Int)(param2: Int)(param3: Int, param4: Int): Int = ??? + foo($m1) + foo(1, $m2) + foo(1, 2)($m3) + foo(1, 2)(3)($m4) + foo(1, 2)(3)(4, $m5) + }""".withSource + .signatureHelp(m1, List(signature), Some(0), 0) + .signatureHelp(m2, List(signature), Some(0), 1) + .signatureHelp(m3, List(signature), Some(0), 2) + .signatureHelp(m4, List(signature), Some(0), 3) + .signatureHelp(m5, List(signature), Some(0), 4) + } + + @Test def implicitParams: Unit = { + val signature = + S("foo", + Nil, + List( + List(P("param0", "Int"), P("param1", "Int")), + List(P("param2", "Int", isImplicit = true)) + ), + Some("Int")) + code"""object O { + def foo(param0: Int, param1: Int)(implicit param2: Int): Int = ??? + foo($m1) + foo(1, $m2) + foo(1, 2)($m3) + }""".withSource + .signatureHelp(m1, List(signature), Some(0), 0) + .signatureHelp(m2, List(signature), Some(0), 1) + .signatureHelp(m3, List(signature), Some(0), 2) + } + + @Test def typeParameters: Unit = { + val signature = + S("foo", + List("M <: [X] => Any", "T <: [Z] => M[Z]", "U >: T"), + List( + List(P("p0", "M[Int]"), P("p1", "T[Int]"), P("p2", "U")) + ), + Some("Int")) + code"""object O { + def foo[M[X], T[Z] <: M[Z], U >: T](p0: M[Int], p1: T[Int], p2: U): Int = ??? + foo($m1) + }""".withSource + .signatureHelp(m1, List(signature), Some(0), 0) + } + + @Test def constructorCall: Unit = { + val signature = + S("Foo", + Nil, + List( + List(P("x", "Int"), P("y", "String")), + List(P("z", "String")) + ), + None) + code"""class Foo(x: Int, y: String)(z: String) + object O { + new Foo($m1) + new Foo(0, $m2) + new Foo(0, "hello")($m3) + }""".withSource + .signatureHelp(m1, List(signature), Some(0), 0) + .signatureHelp(m2, List(signature), Some(0), 1) + .signatureHelp(m3, List(signature), Some(0), 2) + } + + @Test def overloadedConstructorCall: Unit = { + val sig0 = + S("Foo", + Nil, + List( + List(P("x", "Int"), P("y", "String")), + List(P("z", "Int")) + ), + None) + val sig1 = + S("Foo", + Nil, + List( + List(P("x", "Int"), P("y", "Int")) + ), + None) + code"""class Foo(x: Int, y: String)(z: Int) { + def this(x: Int, y: Int) = this(x, y.toString)(0) + } + object O { + new Foo($m1) + new Foo(0, $m2) + new Foo(0, "")($m3) + new Foo(0, 0$m4) + }""".withSource + .signatureHelp(m1, List(sig0, sig1), None, 0) + .signatureHelp(m2, List(sig0, sig1), None, 1) + .signatureHelp(m3, List(sig0, sig1), Some(0), 2) + .signatureHelp(m4, List(sig0, sig1), Some(1), 1) + } + + @Test def constructorCallDoc: Unit = { + val signatures = List( + S("Foo", Nil, List(List(P("x", "Int", Some("An int")), P("y", "String", Some("A string")))), None, Some("A Foo")), + S("Foo", Nil, List(List(P("z", "Boolean", Some("A boolean")), P("foo", "Foo", Some("A Foo")))), None, Some("An alternative constructor for Foo")) + ) + + code"""/** + * A Foo + * + * @param x An int + * @param y A string + */ + class Foo(x: Int, y: String) { + /** + * An alternative constructor for Foo + * + * @param z A boolean + * @param foo A Foo + */ + def this(z: Boolean, foo: Foo) = this(0, "") + } + object O { + new Foo($m1) + new Foo(0$m2) + new Foo(true$m3) + new Foo(0, $m4) + new Foo(0, ""$m5) + new Foo(true, $m6) + new Foo(true, ???$m7) + }""".withSource + .signatureHelp(m1, signatures, None, 0) + .signatureHelp(m2, signatures, Some(0), 0) + .signatureHelp(m3, signatures, Some(1), 0) + .signatureHelp(m4, signatures, Some(0), 1) + .signatureHelp(m5, signatures, Some(0), 1) + .signatureHelp(m6, signatures, Some(1), 1) + .signatureHelp(m7, signatures, Some(1), 1) + } + + @Test def classTypeParameters: Unit = { + val signature = + S("Foo", + List("M <: [X] => Any", "T <: [Z] => M[Z]", "U"), + List( + List(P("p0", "M[Int]"), P("p1", "T[Int]"), P("p2", "U")), + List(P("p3", "Int")) + ), + None) + code"""class Foo[M[X], T[Z] <: M[Z], U](p0: M[Int], p1: T[Int], p2: U)(p3: Int) + object O { + new Foo($m1) + new Foo(???, $m2) + new Foo(???, ???, $m3) + new Foo(???, ???, ???)($m4) + }""".withSource + .signatureHelp(m1, List(signature), Some(0), 0) + .signatureHelp(m2, List(signature), Some(0), 1) + .signatureHelp(m3, List(signature), Some(0), 2) + .signatureHelp(m4, List(signature), Some(0), 3) + } + + @Test def showDoc: Unit = { + code"""object O { + /** Hello, world! */ def foo(param0: Int): Int = 0 + foo($m1) + }""".withSource + .signatureHelp(m1, List(S("foo", Nil, List(List(P("param0", "Int"))), Some("Int"), Some("Hello, world!"))), None, 0) + } + + @Test def showParamDoc: Unit = { + code"""object O { + | /** + | * Buzzes a fizz up to bar + | * + | * @param fizz The fizz to buzz + | * @param bar Buzzing limit + | * @return The fizz after being buzzed up to bar + | */ + | def buzz(fizz: Int, bar: Int): Int = ??? + | buzz($m1) + |}""".withSource + .signatureHelp(m1, List( + S("buzz", Nil, List(List( + P("fizz", "Int", Some("The fizz to buzz")), + P("bar", "Int", Some("Buzzing limit")) + )), Some("Int"), Some("Buzzes a fizz up to bar")) + ), None, 0) + } +} diff --git a/language-server/test/dotty/tools/languageserver/util/CodeTester.scala b/language-server/test/dotty/tools/languageserver/util/CodeTester.scala index 6697c5c28a10..5a9aff02f6ca 100644 --- a/language-server/test/dotty/tools/languageserver/util/CodeTester.scala +++ b/language-server/test/dotty/tools/languageserver/util/CodeTester.scala @@ -1,5 +1,6 @@ package dotty.tools.languageserver.util +import dotty.tools.languageserver.Signatures.Signature import dotty.tools.languageserver.util.Code._ import dotty.tools.languageserver.util.actions._ import dotty.tools.languageserver.util.embedded.CodeMarker @@ -167,6 +168,23 @@ class CodeTester(projects: List[Project]) { def implementation(range: CodeRange, expected: List[CodeRange]): this.type = doAction(new Implementation(range, expected)) + /** + * Requests `textDocument/signatureHelp` from the language server at the specified `marker`, + * verifies that the results match the expectations. + * + * @param marker A maker that specifies the cursor position. + * @param expected The expected list of signature returned by the server. + * @param activeSignature The expected active signature. + * @param activeParam The expected active parameter. + * + * @see dotty.tools.languageserver.util.actions.SignatureHelp + */ + def signatureHelp(marker: CodeMarker, + expected: List[Signature], + activeSignature: Option[Int], + activeParam: Int): this.type = + doAction(new SignatureHelp(marker, expected, activeSignature, activeParam)) + private def doAction(action: Action): this.type = { try { action.execute()(testServer, testServer.client, positions) diff --git a/language-server/test/dotty/tools/languageserver/util/actions/SignatureHelp.scala b/language-server/test/dotty/tools/languageserver/util/actions/SignatureHelp.scala new file mode 100644 index 000000000000..a7b32cbb483b --- /dev/null +++ b/language-server/test/dotty/tools/languageserver/util/actions/SignatureHelp.scala @@ -0,0 +1,64 @@ +package dotty.tools.languageserver.util.actions + +import dotty.tools.languageserver.util.PositionContext +import dotty.tools.languageserver.util.embedded.CodeMarker + +import dotty.tools.languageserver.Signatures.Signature + +import org.eclipse.lsp4j.{MarkupContent, ParameterInformation, SignatureInformation} +import org.junit.Assert.{assertEquals, assertFalse, assertTrue} + +import scala.collection.JavaConverters._ + +import SignatureHelp._ + +/** + * An action requesting for signature help at `marker`. + * This action corresponds to the `textDocument/signatureHelp` method of the Language + * Server Protocol. + * + * @param marker The marker indicating the position where the signature help should be + * requested. + * @param expected The results that are expected for this query. + * @param activeSignature The expected index of the active signature. + * @param activeParam The expected index of the active paremeter. + */ +class SignatureHelp(override val marker: CodeMarker, + expected: List[Signature], + activeSignature: Option[Int], + activeParam: Int) extends ActionOnMarker { + + val expectedSignatures = expected.map(_.toSignatureInformation) + + override def execute(): Exec[Unit] = { + val results = server.signatureHelp(marker.toTextDocumentPositionParams).get() + val resultSignatures = results.getSignatures.asScala + + assertEquals("Number of signatures", expected.length, resultSignatures.length) + + // We don't check that we get the same active signature, just that the signature at + // `activeSignature` is the same, if any signature can be considered "active". + activeSignature match { + case Some(active) if expectedSignatures.nonEmpty => + val expectedActive = expectedSignatures(active) + val resultActive = resultSignatures(results.getActiveSignature) + assertEquals("active signature", expectedActive, resultActive) + case _ => + () + } + + assertEquals("activeParam", activeParam, results.getActiveParameter) + + expectedSignatures.sorted.zip(resultSignatures.sorted).foreach { + assertEquals(_, _) + } + } + + override def show: PositionContext.PosCtx[String] = + s"SignatureHelp(${marker.show}, $expected, $activeSignature, $activeParam)" +} + +object SignatureHelp { + + implicit val signatureInformationOrdering: Ordering[SignatureInformation] = Ordering.by(_.toString) +} From 604c0676297c20f3cbf3b1fafdb304694a54cd9e Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Mon, 12 Nov 2018 10:01:06 +0100 Subject: [PATCH 3/4] Improve readability Co-Authored-By: Duhemm --- language-server/src/dotty/tools/languageserver/Signatures.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/language-server/src/dotty/tools/languageserver/Signatures.scala b/language-server/src/dotty/tools/languageserver/Signatures.scala index 6bff01943e02..427a105748f2 100644 --- a/language-server/src/dotty/tools/languageserver/Signatures.scala +++ b/language-server/src/dotty/tools/languageserver/Signatures.scala @@ -19,7 +19,7 @@ object Signatures { case Apply(fun, params) :: _ => val alreadyAppliedCount = Signatures.countParams(fun) val paramIndex = params.indexWhere(_.pos.contains(pos)) match { - case -1 => (params.length - 1 max 0) + alreadyAppliedCount + case -1 => ((params.length - 1) max 0) + alreadyAppliedCount case n => n + alreadyAppliedCount } From 25d436bdd6434df6e436b6713e8f50cf9c29522a Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Mon, 12 Nov 2018 10:41:41 +0100 Subject: [PATCH 4/4] Move `Signatures` to `dotty.tools.dotc.util` And address review comments. --- .../dotty/tools/dotc/util}/Signatures.scala | 137 ++++++++++++------ .../languageserver/DottyLanguageServer.scala | 79 ++++------ .../languageserver/SignatureHelpTest.scala | 3 +- .../languageserver/util/CodeTester.scala | 4 +- .../util/actions/SignatureHelp.scala | 5 +- 5 files changed, 131 insertions(+), 97 deletions(-) rename {language-server/src/dotty/tools/languageserver => compiler/src/dotty/tools/dotc/util}/Signatures.scala (52%) diff --git a/language-server/src/dotty/tools/languageserver/Signatures.scala b/compiler/src/dotty/tools/dotc/util/Signatures.scala similarity index 52% rename from language-server/src/dotty/tools/languageserver/Signatures.scala rename to compiler/src/dotty/tools/dotc/util/Signatures.scala index 427a105748f2..4c44d337cbf3 100644 --- a/language-server/src/dotty/tools/languageserver/Signatures.scala +++ b/compiler/src/dotty/tools/dotc/util/Signatures.scala @@ -1,25 +1,59 @@ -package dotty.tools.languageserver +package dotty.tools.dotc.util -import dotty.tools.dotc.ast.tpd._ +import dotty.tools.dotc.ast.Trees._ +import dotty.tools.dotc.ast.tpd import dotty.tools.dotc.core.Constants.Constant import dotty.tools.dotc.core.Contexts.Context import dotty.tools.dotc.core.Denotations.SingleDenotation +import dotty.tools.dotc.core.Flags.Implicit +import dotty.tools.dotc.core.Names.TermName import dotty.tools.dotc.util.Positions.Position -import dotty.tools.dotc.core.Types.{ErrorType, MethodType} +import dotty.tools.dotc.core.Types.{ErrorType, MethodType, PolyType} import dotty.tools.dotc.reporting.diagnostic.messages -import org.eclipse.lsp4j.{ParameterInformation, SignatureInformation} - import scala.collection.JavaConverters._ object Signatures { - def callInfo(path: List[Tree], pos: Position)(implicit ctx: Context): (Int, Int, List[SingleDenotation]) = { + /** + * Represent a method signature. + * + * @param name The name of the method + * @param tparams The type parameters and their bounds + * @param paramss The parameter lists of this method + * @param returnType The return type of this method, if this is not a constructor. + * @param doc The documentation for this method. + */ + case class Signature(name: String, tparams: List[String], paramss: List[List[Param]], returnType: Option[String], doc: Option[String] = None) { + } + + /** + * Represent a method's parameter. + * + * @param name The name of the parameter + * @param tpe The type of the parameter + * @param doc The documentation of this parameter + * @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" + } + + /** + * Extract (current parameter index, function index, functions) out of a method call. + * + * @param path The path to the function application + * @param pos The position of the cursor + * @return A triple containing the index of the parameter being edited, the index of the function + * being called, the list of overloads of this function). + */ + def callInfo(path: List[tpd.Tree], pos: Position)(implicit ctx: Context): (Int, Int, List[SingleDenotation]) = { path match { case Apply(fun, params) :: _ => val alreadyAppliedCount = Signatures.countParams(fun) val paramIndex = params.indexWhere(_.pos.contains(pos)) match { - case -1 => ((params.length - 1) max 0) + alreadyAppliedCount + case -1 => (params.length - 1 max 0) + alreadyAppliedCount case n => n + alreadyAppliedCount } @@ -42,6 +76,56 @@ object Signatures { } } + def toSignature(denot: SingleDenotation)(implicit ctx: Context): Option[Signature] = { + val symbol = denot.symbol + val docComment = ParsedComment.docOf(symbol) + val classTree = symbol.topLevelClass.asClass.rootTree + val isImplicit: TermName => Boolean = tpd.defPath(symbol, classTree).lastOption match { + case Some(DefDef(_, _, paramss, _, _)) => + val flatParams = paramss.flatten + name => flatParams.find(_.name == name).map(_.symbol.is(Implicit)).getOrElse(false) + case _ => + _ => false + } + + denot.info.stripPoly match { + case tpe: MethodType => + val infos = { + tpe.paramInfoss.zip(tpe.paramNamess).map { case (infos, names) => + infos.zip(names).map { case (info, name) => + Signatures.Param(name.show, + info.widenTermRefExpr.show, + docComment.flatMap(_.paramDoc(name)), + isImplicit = isImplicit(name)) + } + } + } + + val typeParams = denot.info match { + case poly: PolyType => + poly.paramNames.zip(poly.paramInfos).map { case (x, y) => x.show + y.show } + case _ => + Nil + } + + val (name, returnType) = + if (symbol.isConstructor) (symbol.owner.name.show, None) + else (denot.name.show, Some(tpe.finalResultType.widenTermRefExpr.show)) + + val signature = + Signatures.Signature(name, + typeParams, + infos, + returnType, + docComment.map(_.mainDoc)) + + Some(signature) + + case other => + None + } + } + /** * The number of parameters that are applied in `tree`. * @@ -51,7 +135,7 @@ object Signatures { * @param tree The tree to inspect. * @return The number of parameters that are passed. */ - private def countParams(tree: Tree): Int = { + private def countParams(tree: tpd.Tree): Int = { tree match { case Apply(fun, params) => countParams(fun) + params.length case _ => 0 @@ -71,7 +155,7 @@ object Signatures { * @return A pair composed of the index of the best alternative (0 if no alternatives * were found), and the list of alternatives. */ - private def alternativesFromError(err: ErrorType, params: List[Tree])(implicit ctx: Context): (Int, List[SingleDenotation]) = { + private def alternativesFromError(err: ErrorType, params: List[tpd.Tree])(implicit ctx: Context): (Int, List[SingleDenotation]) = { val alternatives = err.msg match { case messages.AmbiguousOverload(_, alternatives, _) => @@ -96,7 +180,7 @@ object Signatures { val alternativesScores = alternatives.map { alt => alt.info.stripPoly match { case tpe: MethodType => - userParamsTypes.zip(tpe.paramInfos).takeWhile(_ <:< _).size + userParamsTypes.zip(tpe.paramInfos).takeWhile{ case (t0, t1) => t0 <:< t1 }.size case _ => 0 } @@ -108,36 +192,5 @@ object Signatures { (bestAlternative, alternatives) } - case class Signature(name: String, tparams: List[String], paramss: List[List[Param]], returnType: Option[String], doc: Option[String] = None) { - def toSignatureInformation: SignatureInformation = { - val paramInfoss = paramss.map(_.map(_.toParameterInformation)) - val paramLists = paramss.map { paramList => - val labels = paramList.map(_.show) - val prefix = if (paramList.exists(_.isImplicit)) "implicit " else "" - labels.mkString(prefix, ", ", "") - }.mkString("(", ")(", ")") - val tparamsLabel = if (tparams.isEmpty) "" else tparams.mkString("[", ", ", "]") - val returnTypeLabel = returnType.map(t => s": $t").getOrElse("") - val label = s"$name$tparamsLabel$paramLists$returnTypeLabel" - val documentation = doc.map(DottyLanguageServer.hoverContent) - val signature = new SignatureInformation(label) - signature.setParameters(paramInfoss.flatten.asJava) - documentation.foreach(signature.setDocumentation(_)) - signature - } - } - - case class Param(name: String, tpe: String, doc: Option[String] = None, isImplicit: Boolean = false) { - - def toParameterInformation: ParameterInformation = { - val label = s"$name: $tpe" - val documentation = doc.map(DottyLanguageServer.hoverContent) - val info = new ParameterInformation(label) - documentation.foreach(info.setDocumentation(_)) - info - } - - def show: String = - s"$name: $tpe" - } } + diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index 94e7227df665..0712e0f3029c 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -470,58 +470,9 @@ class DottyLanguageServer extends LanguageServer val path = Interactive.pathTo(trees, pos).dropWhile(!_.isInstanceOf[Apply]) val (paramN, callableN, alternatives) = Signatures.callInfo(path, pos.pos) + val signatureInfos = alternatives.flatMap(Signatures.toSignature) - val signatureInfos = alternatives.flatMap { denot => - val symbol = denot.symbol - val docComment = ParsedComment.docOf(symbol) - val classTree = symbol.topLevelClass.asClass.rootTree - val isImplicit: TermName => Boolean = tpd.defPath(symbol, classTree).lastOption match { - case Some(DefDef(_, _, paramss, _, _)) => - val flatParams = paramss.flatten - name => flatParams.find(_.name == name).map(_.symbol.is(Implicit)).getOrElse(false) - case _ => - _ => false - } - - denot.info.stripPoly match { - case tpe: MethodType => - val infos = { - tpe.paramInfoss.zip(tpe.paramNamess).map { (infos, names) => - infos.zip(names).map { (info, name) => - Signatures.Param(name.show, - info.widenTermRefExpr.show, - docComment.flatMap(_.paramDoc(name)), - isImplicit = isImplicit(name)) - } - } - } - - val typeParams = denot.info match { - case poly: PolyType => - poly.paramNames.zip(poly.paramInfos).map((x, y) => x.show + y.show) - case _ => - Nil - } - - val (name, returnType) = - if (symbol.isConstructor) (symbol.owner.name.show, None) - else (denot.name.show, Some(tpe.finalResultType.widenTermRefExpr.show)) - - val signature = - Signatures.Signature(name, - typeParams, - infos, - returnType, - docComment.map(_.mainDoc)) - - Some(signature) - - case other => - None - } - } - - new SignatureHelp(signatureInfos.map(_.toSignatureInformation).asJava, callableN, paramN) + new SignatureHelp(signatureInfos.map(signatureToSignatureInformation).asJava, callableN, paramN) } override def getTextDocumentService: TextDocumentService = this @@ -830,4 +781,30 @@ object DottyLanguageServer { location(pos, positionMapper).map(l => new lsp4j.SymbolInformation(name, symbolKind(sym), l, containerName)) } + + /** Convert `signature` to a `SignatureInformation` */ + def signatureToSignatureInformation(signature: Signatures.Signature): lsp4j.SignatureInformation = { + val paramInfoss = signature.paramss.map(_.map(paramToParameterInformation)) + val paramLists = signature.paramss.map { paramList => + val labels = paramList.map(_.show) + val prefix = if (paramList.exists(_.isImplicit)) "implicit " else "" + labels.mkString(prefix, ", ", "") + }.mkString("(", ")(", ")") + val tparamsLabel = if (signature.tparams.isEmpty) "" else signature.tparams.mkString("[", ", ", "]") + val returnTypeLabel = signature.returnType.map(t => s": $t").getOrElse("") + val label = s"${signature.name}$tparamsLabel$paramLists$returnTypeLabel" + val documentation = signature.doc.map(DottyLanguageServer.hoverContent) + val sig = new lsp4j.SignatureInformation(label) + sig.setParameters(paramInfoss.flatten.asJava) + documentation.foreach(sig.setDocumentation(_)) + sig + } + + /** Convert `param` to `ParameterInformation` */ + private def paramToParameterInformation(param: Signatures.Param): lsp4j.ParameterInformation = { + val documentation = param.doc.map(DottyLanguageServer.hoverContent) + val info = new lsp4j.ParameterInformation(param.show) + documentation.foreach(info.setDocumentation(_)) + info + } } diff --git a/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala b/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala index 670d82f6cf5e..aa90ce570d51 100644 --- a/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala +++ b/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala @@ -3,7 +3,8 @@ package dotty.tools.languageserver import org.junit.Test import dotty.tools.languageserver.util.Code._ -import dotty.tools.languageserver.Signatures.{Param => P, Signature => S} + +import dotty.tools.dotc.util.Signatures.{Param => P, Signature => S} class SignatureHelpTest { diff --git a/language-server/test/dotty/tools/languageserver/util/CodeTester.scala b/language-server/test/dotty/tools/languageserver/util/CodeTester.scala index 5a9aff02f6ca..8614e13bf721 100644 --- a/language-server/test/dotty/tools/languageserver/util/CodeTester.scala +++ b/language-server/test/dotty/tools/languageserver/util/CodeTester.scala @@ -1,10 +1,12 @@ package dotty.tools.languageserver.util -import dotty.tools.languageserver.Signatures.Signature import dotty.tools.languageserver.util.Code._ import dotty.tools.languageserver.util.actions._ import dotty.tools.languageserver.util.embedded.CodeMarker import dotty.tools.languageserver.util.server.{TestFile, TestServer} + +import dotty.tools.dotc.util.Signatures.Signature + import org.eclipse.lsp4j.{CompletionItemKind, DocumentHighlightKind} /** diff --git a/language-server/test/dotty/tools/languageserver/util/actions/SignatureHelp.scala b/language-server/test/dotty/tools/languageserver/util/actions/SignatureHelp.scala index a7b32cbb483b..bd82850dab35 100644 --- a/language-server/test/dotty/tools/languageserver/util/actions/SignatureHelp.scala +++ b/language-server/test/dotty/tools/languageserver/util/actions/SignatureHelp.scala @@ -1,9 +1,10 @@ package dotty.tools.languageserver.util.actions +import dotty.tools.languageserver.DottyLanguageServer import dotty.tools.languageserver.util.PositionContext import dotty.tools.languageserver.util.embedded.CodeMarker -import dotty.tools.languageserver.Signatures.Signature +import dotty.tools.dotc.util.Signatures.Signature import org.eclipse.lsp4j.{MarkupContent, ParameterInformation, SignatureInformation} import org.junit.Assert.{assertEquals, assertFalse, assertTrue} @@ -28,7 +29,7 @@ class SignatureHelp(override val marker: CodeMarker, activeSignature: Option[Int], activeParam: Int) extends ActionOnMarker { - val expectedSignatures = expected.map(_.toSignatureInformation) + val expectedSignatures = expected.map(DottyLanguageServer.signatureToSignatureInformation) override def execute(): Exec[Unit] = { val results = server.signatureHelp(marker.toTextDocumentPositionParams).get()