From c465c0936a9a024af343588ab819fbd784dc11f0 Mon Sep 17 00:00:00 2001 From: rochala Date: Fri, 1 Dec 2023 09:53:21 +0100 Subject: [PATCH 01/14] Add better signature helps for named args, fix various bugs with positions --- .../dotc/interactive/InteractiveDriver.scala | 2 +- .../dotty/tools/dotc/util/Signatures.scala | 123 +++++----- .../languageserver/DottyLanguageServer.scala | 17 +- .../languageserver/SignatureHelpTest.scala | 17 ++ .../tools/pc/SignatureHelpProvider.scala | 85 ++++--- .../tools/pc/completions/Completions.scala | 3 - .../pc/completions/KeywordsCompletions.scala | 1 - .../pc/completions/NamedArgCompletions.scala | 1 - .../edit/ConvertToNamedArgumentsSuite.scala | 1 - .../SignatureHelpNamedArgsSuite.scala | 185 +++++++++++++++ .../signaturehelp/SignatureHelpSuite.scala | 220 +++++++++++++++++- 11 files changed, 543 insertions(+), 112 deletions(-) create mode 100644 presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala diff --git a/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala b/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala index b00cd1036018..92326b0ffeb0 100644 --- a/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala +++ b/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala @@ -281,7 +281,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. diff --git a/compiler/src/dotty/tools/dotc/util/Signatures.scala b/compiler/src/dotty/tools/dotc/util/Signatures.scala index ce05cfb40294..32befc6ba1c7 100644 --- a/compiler/src/dotty/tools/dotc/util/Signatures.scala +++ b/compiler/src/dotty/tools/dotc/util/Signatures.scala @@ -1,13 +1,15 @@ package dotty.tools.dotc package util +import dotty.tools.dotc.ast.NavigateAST +import dotty.tools.dotc.ast.untpd +import dotty.tools.dotc.core.NameOps.* + import ast.Trees.* import ast.tpd -import core.Constants.Constant import core.Contexts.* import core.Denotations.{SingleDenotation, Denotation} import core.Flags -import core.NameOps.isUnapplyName import core.Names.* import core.NameKinds import core.Types.* @@ -46,8 +48,10 @@ object Signatures { * @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 = if name.nonEmpty then s"$name: $tpe" else tpe + case class Param(name: String, tpe: String, doc: Option[String] = None, isImplicit: Boolean = false, isReordered: Boolean = false) { + def show: String = if name.nonEmpty && !isReordered then s"$name: $tpe" + else if name.nonEmpty then s"[$name: $tpe]" + else tpe } /** @@ -56,7 +60,7 @@ object Signatures { * @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 functeon + * @return A triple containing the index of the parameter being edited, the index of function * being called, the list of overloads of this function). */ def signatureHelp(path: List[tpd.Tree], pos: Span)(using Context): (Int, Int, List[Signature]) = @@ -204,18 +208,43 @@ object Signatures { (alternativeIndex, alternatives) if alternativeIndex < alternatives.length then - val curriedArguments = countParams(fun, alternatives(alternativeIndex)) - // We have to shift all arguments by number of type parameters to properly show activeParameter index - val typeParamsShift = if !isTypeApply then fun.symbol.denot.paramSymss.flatten.filter(_.isType).length else 0 - val paramIndex = params.indexWhere(_.span.contains(span)) match - case -1 => (params.length - 1 max 0) + curriedArguments + typeParamsShift - case n => n + curriedArguments + typeParamsShift + val alternativeSymbol = alternatives(alternativeIndex).symbol + val paramssListIndex = findParamssIndex(fun, alternatives(alternativeIndex)) + + val previousArgs = alternativeSymbol.paramSymss.take(paramssListIndex).foldLeft(0)(_ + _.length) + val previousTypeParams = if !isTypeApply then alternativeSymbol.paramSymss.flatten.filter(_.isType).length else 0 + + val untpdPath: List[untpd.Tree] = NavigateAST + .untypedPath(fun, false).collect { case untpdTree: untpd.Tree => untpdTree } + + val untpdArgs = untpdPath match + case Ident(_) :: New(_) :: Select(_, name) :: untpd.Apply(_, args) :: _ if name.isConstructorName => args + case _ :: untpd.Apply(_, args) :: _ => args + case _ :: untpd.TypeApply(_, args) :: _ => args + case _ => Nil + + val currentParamsIndex = untpdArgs.indexWhere(_.span.contains(span)) match + case -1 => if untpdArgs.forall(_.span.start > span.end) then 0 else (params.length - 1) max 0 + case n => n min (alternativeSymbol.paramSymss(paramssListIndex).length - 1) + + val firstOrderedParams = + val originalParams = params.map(_.span) + val untpdParams = untpdArgs.map(_.span) + originalParams.zip(untpdParams).takeWhile((original, untpd) => original == untpd).length + + val reorderedNamedArgs = untpdArgs.drop(firstOrderedParams).takeWhile(_.span.start < span.start).collect: + case namedArg: untpd.NamedArg => namedArg.name.show + + val namedArgsAfterCursor = untpdArgs.drop(firstOrderedParams).dropWhile(_.span.start < span.start).collect: + case namedArg: untpd.NamedArg => namedArg.name.show val pre = treeQualifier(fun) val alternativesWithTypes = alternatives.map(_.asSeenFrom(pre.tpe.widenTermRefExpr)) - val alternativeSignatures = alternativesWithTypes.flatMap(toApplySignature) + val alternativeSignatures = alternativesWithTypes + .flatMap(toApplySignature(_, reorderedNamedArgs, namedArgsAfterCursor, firstOrderedParams)) - (paramIndex, alternativeIndex, alternativeSignatures) + val finalParamIndex = currentParamsIndex + previousArgs + previousTypeParams + (finalParamIndex, alternativeIndex, alternativeSignatures) else (0, 0, Nil) @@ -235,7 +264,6 @@ object Signatures { patterns: List[tpd.Tree] )(using Context): (Int, Int, List[Signature]) = val resultType = unapplyMethodResult(fun) - val isUnapplySeq = fun.denot.name == core.Names.termName("unapplySeq") val denot = fun.denot.mapInfo(_ => resultType) val paramTypes = extractParamTypess(resultType, denot, patterns.size).flatten.map(stripAllAnnots) @@ -397,7 +425,12 @@ object Signatures { * * @return Signature if denot is a function, None otherwise */ - private def toApplySignature(denot: SingleDenotation)(using Context): Option[Signature] = { + private def toApplySignature( + denot: SingleDenotation, + namedParamsBeforeCursor: List[String], + namedParamsAfterCursor: List[String], + firstOrderedParams: Int + )(using Context): Option[Signature] = { val symbol = denot.symbol val docComment = ParsedComment.docOf(symbol) @@ -416,16 +449,26 @@ object Signatures { case (params :: _, infos :: _) => params zip infos case _ => Nil - val params = currentParams.map { (name, info) => - Signatures.Param( - name.show, - info.widenTermRefExpr.show, - docComment.flatMap(_.paramDoc(name)), - isImplicit = tp.isImplicitMethod - ) - } + val params = currentParams.map: (name, info) => + Signatures.Param( + name.show, + info.widenTermRefExpr.show, + docComment.flatMap(_.paramDoc(name)), + isImplicit = tp.isImplicitMethod, + ) + + val ordered = params.take(firstOrderedParams) + val reorderedParamsBeforeCursor = namedParamsBeforeCursor.flatMap: name => + params.find(_.name == name) + + val (remainingNamed, remaining) = params + .diff(reorderedParamsBeforeCursor).diff(ordered) + .partition(p => namedParamsAfterCursor.contains(p.name)) - (params :: rest) + val reorderedArgs = (reorderedParamsBeforeCursor ++ remaining ++ remainingNamed) + .map(_.copy(isReordered = reorderedParamsBeforeCursor.nonEmpty)) + + (ordered ++ reorderedArgs) :: rest def isSyntheticEvidence(name: String) = if !name.startsWith(NameKinds.ContextBoundParamName.separator) then false else @@ -461,23 +504,6 @@ object Signatures { case other => None } - @deprecated("Deprecated in favour of `signatureHelp` which now returns Signature along SingleDenotation", "3.1.3") - def toSignature(denot: SingleDenotation)(using Context): Option[Signature] = { - if denot.name.isUnapplyName then - val resultType = denot.info.stripPoly.finalResultType match - case methodType: MethodType => methodType.resultType.widen - case other => other - - // We can't get already applied patterns so we won't be able to get proper signature in case when - // it can be both name-based or single match extractor. See test `nameBasedTest` - val paramTypes = extractParamTypess(resultType, denot, 0).flatten.map(stripAllAnnots) - val paramNames = extractParamNamess(resultType, denot).flatten - - toUnapplySignature(denot.asSingleDenotation, paramNames, paramTypes) - else - toApplySignature(denot) - } - /** * Creates signature for unapply method. It is different from apply one as it should not show function name, * return type and type parameters. Instead we show function in the following pattern (_$1: T1, _$2: T2, _$n: Tn), @@ -516,11 +542,10 @@ object Signatures { * * @return The number of parameters that are passed. */ - private def countParams(tree: tpd.Tree, denot: SingleDenotation, alreadyCurried: Int = 0)(using Context): Int = + private def findParamssIndex(tree: tpd.Tree, denot: SingleDenotation, alreadyCurried: Int = 0)(using Context): Int = tree match - case Apply(fun, params) => - countParams(fun, denot, alreadyCurried + 1) + denot.symbol.paramSymss(alreadyCurried).length - case _ => 0 + case Apply(fun, params) => findParamssIndex(fun, denot, alreadyCurried + 1) + case _ => alreadyCurried /** * Inspect `err` to determine, if it is an error related to application of an overloaded @@ -543,13 +568,7 @@ object Signatures { case msg: NoMatchingOverload => msg.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.span.isZeroExtent => xs - case _ => params - val userParamsTypes = userParams.map(_.tpe) + val userParamsTypes = params.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. diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index 903a5ae3b36a..6a7eb9c6a629 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -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 path = Interactive.pathTo(ctx.compilationUnit.tpdTree, pos.span) + val (paramN, callableN, signatures) = Signatures.signatureHelp(path, pos.span) + + new SignatureHelp(signatures.map(signatureToSignatureInformation).asJava, callableN, paramN) + + case _ => new SignatureHelp() } diff --git a/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala b/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala index 78fd99d0c3ed..dd7d522231e7 100644 --- a/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala +++ b/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala @@ -974,4 +974,21 @@ class SignatureHelpTest { .signatureHelp(m2, List(signature), None, 0) } + @Test def instantiatedTypeVarInExtensionMethods: Unit = { + val signature = S(s"test", Nil, List(List(P("x", "Int"))), Some("List[Int]")) + code"""|object O: + | extension [T](xs: List[T]) def test(x: T): List[T] = ??? + | List(1,2,3).test(${m1}""" + .signatureHelp(m1, List(signature), None, 2) + } + + @Test def instantiatedTypeVarInOldExtensionMethods: Unit = { + val signature = S(s"test", Nil, List(List(P("x", "Int"))), Some("List[Int]")) + code"""|object O: + | implicit class TypeVarTest[T](xs: List[T]): + | def test(x: T): List[T] = ??? + | List(1,2,3).test(${m1}""" + .signatureHelp(m1, List(signature), None, 0) + } + } diff --git a/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala b/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala index bfaa56138547..e3597d023a69 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala @@ -5,18 +5,14 @@ import scala.meta.pc.OffsetParams import scala.meta.pc.SymbolDocumentation import scala.meta.pc.SymbolSearch -import dotty.tools.dotc.ast.Trees.AppliedTypeTree -import dotty.tools.dotc.ast.Trees.TypeApply -import dotty.tools.dotc.ast.tpd import dotty.tools.dotc.core.Contexts.* import dotty.tools.dotc.core.Flags import dotty.tools.dotc.core.Symbols.* import dotty.tools.dotc.interactive.Interactive import dotty.tools.dotc.interactive.InteractiveDriver import dotty.tools.dotc.util.Signatures -import dotty.tools.dotc.util.Signatures.Signature +import dotty.tools.dotc.util.Spans import dotty.tools.dotc.util.SourceFile -import dotty.tools.dotc.util.SourcePosition import dotty.tools.pc.utils.MtagsEnrichments.* import org.eclipse.lsp4j as l @@ -28,52 +24,55 @@ object SignatureHelpProvider: params: OffsetParams, search: SymbolSearch ) = - val uri = params.uri() - val sourceFile = SourceFile.virtual(params.uri().nn, params.text().nn) + val uri = params.uri().nn + val sourceFile = SourceFile.virtual(uri, params.text().nn) driver.run(uri.nn, sourceFile) - given ctx: Context = driver.currentCtx - - val pos = driver.sourcePosition(params) - val trees = driver.openedTrees(uri.nn) - - val path = Interactive.pathTo(trees, pos) - - val (paramN, callableN, alternatives) = - Signatures.signatureHelp(path, pos.span) - val infos = alternatives.flatMap { signature => - signature.denot.map { - (signature, _) - } - } - - val signatureInfos = infos.map { case (signature, denot) => - search.symbolDocumentation(denot.symbol) match - case Some(doc) => - withDocumentation( - doc, - signature, - denot.symbol.is(Flags.JavaDefined) - ).getOrElse(signature) - case _ => signature - - } - - /* Versions prior to 3.2.1 did not support type parameters - * so we need to skip them. - */ - new l.SignatureHelp( - signatureInfos.map(signatureToSignatureInformation).asJava, - callableN, - paramN - ) + driver.compilationUnits.get(uri) match + case Some(unit) => + given newCtx: Context = driver.currentCtx.fresh.setCompilationUnit(unit) + + val pos = driver.sourcePosition(params) + val treeSpanEnd = ctx.compilationUnit.tpdTree.span.end + + // If we try to run signature help for named arg which happens to be the last line of the code + // it will return span outside tree, as parser ignores additional whitespaces: + // def foo(aaa: Int, bbb: Int) = ??? + // foo(aaa // fail before writing the = sign + val adjustedSpan = if pos.span.end > treeSpanEnd then Spans.Span(treeSpanEnd) + else pos.span + + val path = Interactive.pathTo(ctx.compilationUnit.tpdTree, adjustedSpan) + val (paramN, callableN, alternatives) = Signatures.signatureHelp(path, pos.span) + + val infos = alternatives.flatMap: signature => + signature.denot.map(signature -> _) + + val signatureInfos = infos.map { case (signature, denot) => + search.symbolDocumentation(denot.symbol) match + case Some(doc) => + withDocumentation( + doc, + signature, + denot.symbol.is(Flags.JavaDefined) + ).getOrElse(signature) + case _ => signature + + } + + new l.SignatureHelp( + signatureInfos.map(signatureToSignatureInformation).asJava, + callableN, + paramN + ) + case _ => new l.SignatureHelp() end signatureHelp private def withDocumentation( info: SymbolDocumentation, signature: Signatures.Signature, isJavaSymbol: Boolean - ): Option[Signature] = + ): Option[Signatures.Signature] = val allParams = info.parameters().nn.asScala def updateParams( params: List[Signatures.Param], diff --git a/presentation-compiler/src/main/dotty/tools/pc/completions/Completions.scala b/presentation-compiler/src/main/dotty/tools/pc/completions/Completions.scala index 1e2dbb5cdaa1..2f2f2bcfdc41 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/completions/Completions.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/completions/Completions.scala @@ -12,7 +12,6 @@ import scala.meta.internal.pc.{IdentifierComparator, MemberOrdering} import scala.meta.pc.* import dotty.tools.dotc.ast.tpd.* -import dotty.tools.dotc.ast.NavigateAST import dotty.tools.dotc.core.Comments.Comment import dotty.tools.dotc.core.Constants.Constant import dotty.tools.dotc.core.Contexts.* @@ -27,8 +26,6 @@ import dotty.tools.dotc.core.Types.* import dotty.tools.dotc.interactive.Completion import dotty.tools.dotc.interactive.Completion.Mode import dotty.tools.dotc.util.SourcePosition -import dotty.tools.dotc.util.Spans -import dotty.tools.dotc.util.Spans.Span import dotty.tools.dotc.util.SrcPos import dotty.tools.pc.AutoImports.AutoImportsGenerator import dotty.tools.pc.completions.OverrideCompletions.OverrideExtractor diff --git a/presentation-compiler/src/main/dotty/tools/pc/completions/KeywordsCompletions.scala b/presentation-compiler/src/main/dotty/tools/pc/completions/KeywordsCompletions.scala index a052a0a2c8eb..e912bc49032f 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/completions/KeywordsCompletions.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/completions/KeywordsCompletions.scala @@ -12,7 +12,6 @@ import dotty.tools.dotc.core.Comments import dotty.tools.dotc.core.Comments.Comment import dotty.tools.dotc.core.Contexts.Context import dotty.tools.dotc.util.SourcePosition -import dotty.tools.dotc.util.Spans.Span object KeywordsCompletions: diff --git a/presentation-compiler/src/main/dotty/tools/pc/completions/NamedArgCompletions.scala b/presentation-compiler/src/main/dotty/tools/pc/completions/NamedArgCompletions.scala index e4ae4070edb9..5e375f2e9821 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/completions/NamedArgCompletions.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/completions/NamedArgCompletions.scala @@ -9,7 +9,6 @@ import dotty.tools.dotc.ast.untpd import dotty.tools.dotc.core.Constants.Constant import dotty.tools.dotc.core.ContextOps.localContext import dotty.tools.dotc.core.Contexts.Context -import dotty.tools.dotc.core.Definitions import dotty.tools.dotc.core.Flags import dotty.tools.dotc.core.Flags.Method import dotty.tools.dotc.core.NameKinds.DefaultGetterName diff --git a/presentation-compiler/test/dotty/tools/pc/tests/edit/ConvertToNamedArgumentsSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/edit/ConvertToNamedArgumentsSuite.scala index 5285be83b537..359a63442408 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/edit/ConvertToNamedArgumentsSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/edit/ConvertToNamedArgumentsSuite.scala @@ -6,7 +6,6 @@ import java.util.concurrent.ExecutionException import scala.meta.internal.jdk.CollectionConverters.* import scala.meta.internal.metals.CompilerOffsetParams import scala.meta.internal.pc.CodeActionErrorMessages -import scala.meta.pc.DisplayableException import scala.language.unsafeNulls import dotty.tools.pc.base.BaseCodeActionSuite diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala new file mode 100644 index 000000000000..3009d660fb6c --- /dev/null +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala @@ -0,0 +1,185 @@ +package dotty.tools.pc.tests.signaturehelp + +import dotty.tools.pc.base.BaseSignatureHelpSuite + +import org.junit.Test + +class SignatureHelpNamedArgsSuite extends BaseSignatureHelpSuite: + + + @Test def `new-named-param-style-1` = + check( + """|object O: + | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? + | method(paramA = 1, @@) + |""".stripMargin, + """|method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit + | ^^^^^^^^^^^ + |""".stripMargin + ) + + @Test def `new-named-param-style-double-space` = + check( + """|object O: + | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? + | method(paramA = 1, @@ + |""".stripMargin, + """|method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit + | ^^^^^^^^^^^ + |""".stripMargin + ) + + @Test def `new-named-param-style-before-equal` = + check( + """|object O: + | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? + | method(paramA = 1, paramB @@ + |""".stripMargin, + """|method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit + | ^^^^^^^^^^^ + |""".stripMargin + ) + + @Test def `new-named-param-style-before-equal-something-after` = + check( + """|object O: + | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? + | method(paramA = 1, paramB @@ + | def test: Unit = () + |""".stripMargin, + """|method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit + | ^^^^^^^^^^^ + |""".stripMargin + ) + + @Test def `new-named-param-style-at-equal` = + check( + """|object O: + | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? + | method(paramA = 1, paramB =@@ + |""".stripMargin, + """|method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit + | ^^^^^^^^^^^ + |""".stripMargin + ) + + @Test def `new-named-param-style-after-equal` = + check( + """|object O: + | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? + | method(paramA = 1, paramB = 1@@ + |""".stripMargin, + """|method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit + | ^^^^^^^^^^^ + |""".stripMargin + ) + + @Test def `new-named-param-style-2` = + check( + """|object O: + | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? + | method(paramB = 1, @@) + |""".stripMargin, + """|method([paramB: Int], [paramA: Int], [paramC: Int], [paramD: Int]): Unit + | ^^^^^^^^^^^^^ + |""".stripMargin + ) + + @Test def `new-named-param-style-3` = + check( + """|object O: + | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? + | method(paramB = 1, paramC = 2, paramD = 3, @@) + |""".stripMargin, + """|method([paramB: Int], [paramC: Int], [paramD: Int], [paramA: Int]): Unit + | ^^^^^^^^^^^^^ + |""".stripMargin + ) + + @Test def `new-named-param-style-4` = + check( + """|object O: + | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? + | method(1, 2, @@) + |""".stripMargin, + """|method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit + | ^^^^^^^^^^^ + |""".stripMargin + ) + + @Test def `new-named-param-style-5` = + check( + """|object O: + | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? + | method(1, paramB = 2, @@) + |""".stripMargin, + """|method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit + | ^^^^^^^^^^^ + |""".stripMargin + ) + + @Test def `new-named-param-style-6` = + check( + """|object O: + | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? + | method(paramA = 1, 2, @@) + |""".stripMargin, + """|method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit + | ^^^^^^^^^^^ + |""".stripMargin + ) + + @Test def `new-named-param-style-7` = + check( + """|object O: + | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? + | method(paramA = 1, paramB = 2, @@) + |""".stripMargin, + """|method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit + | ^^^^^^^^^^^ + |""".stripMargin + ) + + @Test def `new-named-param-style-8` = + check( + """|object O: + | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? + | method(paramB = 2, paramA = 1, @@) + |""".stripMargin, + """|method([paramB: Int], [paramA: Int], [paramC: Int], [paramD: Int]): Unit + | ^^^^^^^^^^^^^ + |""".stripMargin + ) + + @Test def `new-named-param-style-9` = + check( + """|object O: + | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? + | method(paramC = 3, paramA = 1, @@) + |""".stripMargin, + """|method([paramC: Int], [paramA: Int], [paramB: Int], [paramD: Int]): Unit + | ^^^^^^^^^^^^^ + |""".stripMargin + ) + + @Test def `new-named-param-style-10` = + check( + """|object O: + | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? + | method(paramC = 3, @@ ,paramA = 1) + |""".stripMargin, + """|method([paramC: Int], [paramB: Int], [paramD: Int], [paramA: Int]): Unit + | ^^^^^^^^^^^^^ + |""".stripMargin + ) + + @Test def `new-named-param-style-11` = + check( + """|object O: + | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? + | method(paramC = 3, paramB = 1, @@ ,paramA = 1) + |""".stripMargin, + """|method([paramC: Int], [paramB: Int], [paramD: Int], [paramA: Int]): Unit + | ^^^^^^^^^^^^^ + |""".stripMargin + ) diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala index 906a0bd9b72e..008a02076219 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala @@ -437,8 +437,8 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | User(age = 1, @@) |} """.stripMargin, - """|apply(name: String, age: Int): User - | ^^^^^^^^ + """|apply([age: Int], [name: String]): User + | ^^^^^^^^^^^^^^ |""".stripMargin ) @@ -477,8 +477,8 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | def x = user(str@@eet = 42, name = "", age = 2) |} """.stripMargin, - """|user(name: String, age: Int, street: Int): Int - | ^^^^^^^^^^^ + """|user([street: Int], [name: String], [age: Int]): Int + | ^^^^^^^^^^^^^ |user(name: String, age: Int): Int |""".stripMargin ) @@ -758,6 +758,15 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: |""".stripMargin ) + @Test def `instantiated-type-var-ext-1` = + check( + """|object O: + | extension [T](xs: List[T]) def test(x: T): List[T] = ??? + | List(1,2,3).test(@@""".stripMargin, + """ + """.stripMargin + ) + @Test def `instantiated-type-var-old-ext-1` = check( """|object O: @@ -829,3 +838,206 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | ^^^^^^ |""".stripMargin ) + + @Test def `multiline-before` = + check( + """|object Main { + | def deployment( + | fst: String, + | snd: Int = 1, + | ): Option[Int] = ??? + | val abc = deployment(@@ + | fst = "abc", + | snd = 1 + | ) + |} + |""".stripMargin, + """|deployment(fst: String, snd: Int): Option[Int] + | ^^^^^^^^^^^ + |""".stripMargin + ) + + @Test def `multiline-after-first` = + check( + """|object Main { + | def deployment( + | fst: String, + | snd: Int = 1, + | ): Option[Int] = ??? + | val abc = deployment( + | fst = "abc", @@ + | snd = 1 + | ) + |} + |""".stripMargin, + """|deployment(fst: String, snd: Int): Option[Int] + | ^^^^^^^^ + |""".stripMargin + ) + + @Test def `multiline-between-first-and-second-a` = + check( + """|object Main { + | def deployment( + | fst: String, + | snd: Int = 1, + | ): Option[Int] = ??? + | val abc = deployment( + | fst = "abc" + | @@ + | + | snd = 1 + | ) + |} + |""".stripMargin, + """|deployment(fst: String, snd: Int): Option[Int] + | ^^^^^^^^^^^ + |""".stripMargin + ) + + @Test def `multiline-between-first-and-second-b` = + check( + """|object Main { + | def deployment( + | fst: String, + | snd: Int = 1, + | ): Option[Int] = ??? + | val abc = deployment( + | fst = "abc", + | @@ + | + | snd = 1 + | ) + |} + |""".stripMargin, + """|deployment(fst: String, snd: Int): Option[Int] + | ^^^^^^^^ + |""".stripMargin + ) + + @Test def `multiline-end` = + check( + """|object Main { + | def deployment( + | fst: String, + | snd: Int = 1, + | ): Option[Int] = ??? + | val abc = deployment( + | fst = "abc", + | snd = 1 + | @@) + |} + |""".stripMargin, + """|deployment(fst: String, snd: Int): Option[Int] + | ^^^^^^^^ + |""".stripMargin + ) + + @Test def `type-var-multiline-before` = + check( + """|object Main { + | def deployment[A, B]( + | fst: A, + | snd: B, + | ): Option[Int] = ??? + | val abc = deployment[@@ + Int, + | String, + | ]( + | fst = "abc", + | snd = 1 + | ) + |} + |""".stripMargin, + """|deployment[A, B](fst: String, snd: Int): Option[Int] + | ^ + |""".stripMargin + ) + + @Test def `type-var-multiline-after` = + check( + """|object Main { + | def deployment[A, B]( + | fst: A, + | snd: B, + | ): Option[Int] = ??? + | val abc = deployment[ + | Int, @@ + | String, + | ]( + | fst = "abc", + | snd = 1 + | ) + |} + |""".stripMargin, + """|deployment[A, B](fst: String, snd: Int): Option[Int] + | ^ + |""".stripMargin + ) + + @Test def `type-var-multiline-between-first-and-second-a` = + check( + """|object Main { + | def deployment[A, B]( + | fst: A, + | snd: B, + | ): Option[Int] = ??? + | val abc = deployment[ + | Int, + | @@ + | + | String + | ]( + | fst = "abc", + | snd = 1 + | ) + |} + |""".stripMargin, + """|deployment[A, B](fst: String, snd: Int): Option[Int] + | ^ + |""".stripMargin + ) + + @Test def `type-var-multiline-between-first-and-second-b` = + check( + """|object Main { + | def deployment[A, B]( + | fst: A, + | snd: B, + | ): Option[Int] = ??? + | val abc = deployment[ + | Int, + | @@ + | + | String, + | ]( + | fst = "abc", + | snd = 1 + | ) + |} + |""".stripMargin, + """|deployment[A, B](fst: String, snd: Int): Option[Int] + | ^ + |""".stripMargin + ) + + @Test def `type-var-multiline-end` = + check( + """|object Main { + | def deployment[A, B]( + | fst: A, + | snd: B, + | ): Option[Int] = ??? + | val abc = deployment[ + | String, + | Int, + | @@]( + | fst = "abc", + | snd = 1 + | ) + |} + |""".stripMargin, + """|deployment[A, B](fst: String, snd: Int): Option[Int] + | ^ + |""".stripMargin + ) From 1a386397b9d43403c41994e9819db4fe16375622 Mon Sep 17 00:00:00 2001 From: rochala Date: Fri, 1 Dec 2023 12:16:24 +0100 Subject: [PATCH 02/14] Properly compute index when between the args --- .../dotty/tools/dotc/util/Signatures.scala | 20 +++++++++++++- .../languageserver/SignatureHelpTest.scala | 8 ------ .../signaturehelp/SignatureHelpSuite.scala | 27 +++++++------------ 3 files changed, 28 insertions(+), 27 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/util/Signatures.scala b/compiler/src/dotty/tools/dotc/util/Signatures.scala index 32befc6ba1c7..badda90b32d5 100644 --- a/compiler/src/dotty/tools/dotc/util/Signatures.scala +++ b/compiler/src/dotty/tools/dotc/util/Signatures.scala @@ -224,7 +224,15 @@ object Signatures { case _ => Nil val currentParamsIndex = untpdArgs.indexWhere(_.span.contains(span)) match - case -1 => if untpdArgs.forall(_.span.start > span.end) then 0 else (params.length - 1) max 0 + case -1 if untpdArgs.isEmpty => 0 + case -1 => + commaIndex(untpdArgs, span) match + case Some(index) if index <= span.start => untpdArgs.takeWhile(_.span.start < span.start).length + case Some(index) => untpdArgs.takeWhile(_.span.start < span.start).length - 1 + case None => + if untpdArgs.head.span.start > span.start then 0 + else untpdArgs.length - 1 max 0 + case n => n min (alternativeSymbol.paramSymss(paramssListIndex).length - 1) val firstOrderedParams = @@ -248,6 +256,16 @@ object Signatures { else (0, 0, Nil) + /** Parser ignores white spaces on next lines, we have to manually find the index of comma */ + private def commaIndex(untpdArgs: List[untpd.Tree], span: Span)(using Context): Option[Int] = + val previousArgIndex = untpdArgs.lastIndexWhere(_.span.end < span.start) + for + previousArg <- untpdArgs.lift(previousArgIndex) + nextArg <- untpdArgs.lift(previousArgIndex + 1) + yield + val text = ctx.source.content.slice(previousArg.span.end, nextArg.span.start) + text.indexOf(',') + previousArg.span.end + /** * Extracts call informatioin for function in unapply context. * diff --git a/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala b/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala index dd7d522231e7..7487dfaba97b 100644 --- a/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala +++ b/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala @@ -974,14 +974,6 @@ class SignatureHelpTest { .signatureHelp(m2, List(signature), None, 0) } - @Test def instantiatedTypeVarInExtensionMethods: Unit = { - val signature = S(s"test", Nil, List(List(P("x", "Int"))), Some("List[Int]")) - code"""|object O: - | extension [T](xs: List[T]) def test(x: T): List[T] = ??? - | List(1,2,3).test(${m1}""" - .signatureHelp(m1, List(signature), None, 2) - } - @Test def instantiatedTypeVarInOldExtensionMethods: Unit = { val signature = S(s"test", Nil, List(List(P("x", "Int"))), Some("List[Int]")) code"""|object O: diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala index 008a02076219..0ed6680e6dd3 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala @@ -758,15 +758,6 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: |""".stripMargin ) - @Test def `instantiated-type-var-ext-1` = - check( - """|object O: - | extension [T](xs: List[T]) def test(x: T): List[T] = ??? - | List(1,2,3).test(@@""".stripMargin, - """ - """.stripMargin - ) - @Test def `instantiated-type-var-old-ext-1` = check( """|object O: @@ -886,7 +877,7 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | fst = "abc" | @@ | - | snd = 1 + | ,snd = 1 | ) |} |""".stripMargin, @@ -949,7 +940,7 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | ) |} |""".stripMargin, - """|deployment[A, B](fst: String, snd: Int): Option[Int] + """|deployment[A, B](fst: A, snd: B): Option[Int] | ^ |""".stripMargin ) @@ -970,8 +961,8 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | ) |} |""".stripMargin, - """|deployment[A, B](fst: String, snd: Int): Option[Int] - | ^ + """|deployment[A, B](fst: A, snd: B): Option[Int] + | ^ |""".stripMargin ) @@ -983,17 +974,17 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | snd: B, | ): Option[Int] = ??? | val abc = deployment[ - | Int, + | Int | @@ | - | String + | ,String | ]( | fst = "abc", | snd = 1 | ) |} |""".stripMargin, - """|deployment[A, B](fst: String, snd: Int): Option[Int] + """|deployment[A, B](fst: A, snd: B): Option[Int] | ^ |""".stripMargin ) @@ -1016,7 +1007,7 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | ) |} |""".stripMargin, - """|deployment[A, B](fst: String, snd: Int): Option[Int] + """|deployment[A, B](fst: A, snd: B): Option[Int] | ^ |""".stripMargin ) @@ -1037,7 +1028,7 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | ) |} |""".stripMargin, - """|deployment[A, B](fst: String, snd: Int): Option[Int] + """|deployment[A, B](fst: A, snd: B): Option[Int] | ^ |""".stripMargin ) From 811d130d51ab2c76247153d5178e1958a1d40b7e Mon Sep 17 00:00:00 2001 From: rochala Date: Fri, 1 Dec 2023 13:58:39 +0100 Subject: [PATCH 03/14] Named args after cursor should properly detect param span --- .../dotty/tools/dotc/util/Signatures.scala | 17 +++++-- .../SignatureHelpNamedArgsSuite.scala | 45 +++++++++++++++++++ 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/util/Signatures.scala b/compiler/src/dotty/tools/dotc/util/Signatures.scala index badda90b32d5..3cf7fed5b949 100644 --- a/compiler/src/dotty/tools/dotc/util/Signatures.scala +++ b/compiler/src/dotty/tools/dotc/util/Signatures.scala @@ -227,12 +227,16 @@ object Signatures { case -1 if untpdArgs.isEmpty => 0 case -1 => commaIndex(untpdArgs, span) match + // comma is after CURSOR, so we are in parameter a case Some(index) if index <= span.start => untpdArgs.takeWhile(_.span.start < span.start).length + // comma is before CURSOR, so we are in parameter b case Some(index) => untpdArgs.takeWhile(_.span.start < span.start).length - 1 + // we are either in first or last parameter case None => - if untpdArgs.head.span.start > span.start then 0 + if untpdArgs.head.span.start >= span.start then 0 else untpdArgs.length - 1 max 0 + // special case if we pass more arguments than function has parameters case n => n min (alternativeSymbol.paramSymss(paramssListIndex).length - 1) val firstOrderedParams = @@ -240,10 +244,10 @@ object Signatures { val untpdParams = untpdArgs.map(_.span) originalParams.zip(untpdParams).takeWhile((original, untpd) => original == untpd).length - val reorderedNamedArgs = untpdArgs.drop(firstOrderedParams).takeWhile(_.span.start < span.start).collect: + val reorderedNamedArgs = untpdArgs.drop(firstOrderedParams).takeWhile(_.span.start <= span.start).collect: case namedArg: untpd.NamedArg => namedArg.name.show - val namedArgsAfterCursor = untpdArgs.drop(firstOrderedParams).dropWhile(_.span.start < span.start).collect: + val namedArgsAfterCursor = untpdArgs.drop(currentParamsIndex + 1).dropWhile(_.span.start <= span.start).collect: case namedArg: untpd.NamedArg => namedArg.name.show val pre = treeQualifier(fun) @@ -256,7 +260,12 @@ object Signatures { else (0, 0, Nil) - /** Parser ignores white spaces on next lines, we have to manually find the index of comma */ + /** Parser ignores chars between arguments, we have to manually find the index of comma + * @param untpdArgs List of applied untyped arguments + * @param span The position of the cursor + * + * @return None if we are in first or last parameter, comma index otherwise + */ private def commaIndex(untpdArgs: List[untpd.Tree], span: Span)(using Context): Option[Int] = val previousArgIndex = untpdArgs.lastIndexWhere(_.span.end < span.start) for diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala index 3009d660fb6c..db3ea12aca0a 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala @@ -183,3 +183,48 @@ class SignatureHelpNamedArgsSuite extends BaseSignatureHelpSuite: | ^^^^^^^^^^^^^ |""".stripMargin ) + + @Test def `named-param-before-first` = + check( + """|object O: + | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? + | method(@@paramC = 3) + |""".stripMargin, + """|method([paramC: Int], [paramA: Int], [paramB: Int], [paramD: Int]): Unit + | ^^^^^^^^^^^^^ + |""".stripMargin + ) + + @Test def `named-param-inside-reordered-last-arg` = + check( + """|object O: + | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? + | method(paramB = 3, paramA = 1, @@paramD = 3, paramC = 1) + |""".stripMargin, + """|method([paramB: Int], [paramA: Int], [paramD: Int], [paramC: Int]): Unit + | ^^^^^^^^^^^^^ + |""".stripMargin + ) + + @Test def `named-param-inside-reorderded-last-arg-1` = + check( + """|object O: + | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? + | method(paramB = 3, paramA = 1, p@@aramD = 3, paramC = 1) + |""".stripMargin, + """|method([paramB: Int], [paramA: Int], [paramD: Int], [paramC: Int]): Unit + | ^^^^^^^^^^^^^ + |""".stripMargin + ) + + @Test def `named-param-before-reorderded-last-arg-1` = + check( + """|object O: + | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? + | method(paramB = 3, paramA = 1,@@ paramD = 3, paramC = 1) + |""".stripMargin, + """|method([paramB: Int], [paramA: Int], [paramD: Int], [paramC: Int]): Unit + | ^^^^^^^^^^^^^ + |""".stripMargin + ) + From f447ba2491f759e8aab38d3321b3614ab65d2bf5 Mon Sep 17 00:00:00 2001 From: rochala Date: Wed, 6 Dec 2023 15:58:40 +0100 Subject: [PATCH 04/14] Improve detection of param, with unclosed params --- .../dotty/tools/dotc/reporting/messages.scala | 4 +- .../dotty/tools/dotc/util/Signatures.scala | 3 +- .../tools/pc/SignatureHelpProvider.scala | 58 +++++-- .../tools/pc/utils/MtagsEnrichments.scala | 2 +- .../SignatureHelpNamedArgsSuite.scala | 22 +++ .../SignatureHelpPatternSuite.scala | 33 ++-- .../signaturehelp/SignatureHelpSuite.scala | 161 ++++++++++++++++++ 7 files changed, 253 insertions(+), 30 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index b6622d67f36a..267859d62f11 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -581,7 +581,7 @@ extends SyntaxMsg(UnboundPlaceholderParameterID) { |""" } -class IllegalStartSimpleExpr(illegalToken: String)(using Context) +class IllegalStartSimpleExpr(val illegalToken: String)(using Context) extends SyntaxMsg(IllegalStartSimpleExprID) { def msg(using Context) = i"expression expected but ${Red(illegalToken)} found" def explain(using Context) = { @@ -1171,7 +1171,7 @@ extends ReferenceMsg(ForwardReferenceExtendsOverDefinitionID) { |""" } -class ExpectedTokenButFound(expected: Token, found: Token, prefix: String = "")(using Context) +class ExpectedTokenButFound(val expected: Token, found: Token, prefix: String = "")(using Context) extends SyntaxMsg(ExpectedTokenButFoundID) { private def foundText = Tokens.showToken(found) diff --git a/compiler/src/dotty/tools/dotc/util/Signatures.scala b/compiler/src/dotty/tools/dotc/util/Signatures.scala index 3cf7fed5b949..96d76573b93e 100644 --- a/compiler/src/dotty/tools/dotc/util/Signatures.scala +++ b/compiler/src/dotty/tools/dotc/util/Signatures.scala @@ -488,10 +488,11 @@ object Signatures { val reorderedParamsBeforeCursor = namedParamsBeforeCursor.flatMap: name => params.find(_.name == name) - val (remainingNamed, remaining) = params + val (remainingNamedUnordered, remaining) = params .diff(reorderedParamsBeforeCursor).diff(ordered) .partition(p => namedParamsAfterCursor.contains(p.name)) + val remainingNamed = remainingNamedUnordered.sortBy(p => namedParamsAfterCursor.indexOf(p.name)) val reorderedArgs = (reorderedParamsBeforeCursor ++ remaining ++ remainingNamed) .map(_.copy(isReordered = reorderedParamsBeforeCursor.nonEmpty)) diff --git a/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala b/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala index e3597d023a69..93dfa993eaa8 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala @@ -1,22 +1,25 @@ package dotty.tools.pc -import scala.jdk.CollectionConverters._ -import scala.meta.pc.OffsetParams -import scala.meta.pc.SymbolDocumentation -import scala.meta.pc.SymbolSearch - import dotty.tools.dotc.core.Contexts.* import dotty.tools.dotc.core.Flags import dotty.tools.dotc.core.Symbols.* import dotty.tools.dotc.interactive.Interactive import dotty.tools.dotc.interactive.InteractiveDriver +import dotty.tools.dotc.parsing.Tokens.closingRegionTokens +import dotty.tools.dotc.reporting.ErrorMessageID +import dotty.tools.dotc.reporting.ExpectedTokenButFound import dotty.tools.dotc.util.Signatures -import dotty.tools.dotc.util.Spans import dotty.tools.dotc.util.SourceFile +import dotty.tools.dotc.util.Spans import dotty.tools.pc.utils.MtagsEnrichments.* - import org.eclipse.lsp4j as l +import scala.jdk.CollectionConverters._ +import scala.jdk.OptionConverters.* +import scala.meta.pc.OffsetParams +import scala.meta.pc.SymbolDocumentation +import scala.meta.pc.SymbolSearch + object SignatureHelpProvider: def signatureHelp( @@ -25,7 +28,8 @@ object SignatureHelpProvider: search: SymbolSearch ) = val uri = params.uri().nn - val sourceFile = SourceFile.virtual(uri, params.text().nn) + val text = params.text().nn + val sourceFile = SourceFile.virtual(uri, text) driver.run(uri.nn, sourceFile) driver.compilationUnits.get(uri) match @@ -35,15 +39,49 @@ object SignatureHelpProvider: val pos = driver.sourcePosition(params) val treeSpanEnd = ctx.compilationUnit.tpdTree.span.end + lazy val firstUnclosedErrorAfterPos = + val unclosedErrors = ctx.reporter.allErrors.filter { _.msg match + case err: ExpectedTokenButFound => closingRegionTokens.contains(err.expected) + case _ => false + } + unclosedErrors.find(_.pos.span.start >= pos.span.end).flatMap(_.position.toScala) + // If we try to run signature help for named arg which happens to be the last line of the code // it will return span outside tree, as parser ignores additional whitespaces: // def foo(aaa: Int, bbb: Int) = ??? // foo(aaa // fail before writing the = sign - val adjustedSpan = if pos.span.end > treeSpanEnd then Spans.Span(treeSpanEnd) + val adjustedSpan = if pos.span.end > treeSpanEnd && firstUnclosedErrorAfterPos.nonEmpty then Spans.Span(treeSpanEnd) else pos.span + val path = Interactive.pathTo(ctx.compilationUnit.tpdTree, adjustedSpan) - val (paramN, callableN, alternatives) = Signatures.signatureHelp(path, pos.span) + + import dotty.tools.dotc.ast.tpd._ + + val maybeNewSpan = firstUnclosedErrorAfterPos match + case Some(errorPosition) => + path + .headOption + // if in the head of the path, there is a tree defined between cursor and error position we are in correct tree + .filter(!_.existsSubTree(tree => tree.span.exists && tree.span.start >= pos.span.end)) + .toList + // find possible trees which can be part of unclosed signature help + .flatMap(_.filterSubTrees: + case tree: (Apply | UnApply | AppliedTypeTree | TypeApply) => tree.span.end <= pos.span.end + case _ => false + ).filter(tree => tree.span.end <= errorPosition.end ) + .maxByOption(_.span.end) + .map(tree => Spans.Span(tree.span.end) + ) + + case None => None + + + val adjustedPath = maybeNewSpan match + case Some(pos) => Interactive.pathTo(ctx.compilationUnit.tpdTree, pos) + case _ => path + + val (paramN, callableN, alternatives) = Signatures.signatureHelp(adjustedPath, pos.span) val infos = alternatives.flatMap: signature => signature.denot.map(signature -> _) diff --git a/presentation-compiler/src/main/dotty/tools/pc/utils/MtagsEnrichments.scala b/presentation-compiler/src/main/dotty/tools/pc/utils/MtagsEnrichments.scala index c006dda2c652..ad6265c00773 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/utils/MtagsEnrichments.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/utils/MtagsEnrichments.scala @@ -273,7 +273,7 @@ object MtagsEnrichments extends CommonMtagsEnrichments: val denot = sym.denot.asSeenFrom(pre.tpe.widenTermRefExpr) (denot.info, sym.withUpdatedTpe(denot.info)) catch case NonFatal(e) => (sym.info, sym) - + def isInfix(using ctx: Context) = tree match case Select(New(_), _) => false diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala index db3ea12aca0a..8c9ea79d4444 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala @@ -228,3 +228,25 @@ class SignatureHelpNamedArgsSuite extends BaseSignatureHelpSuite: |""".stripMargin ) + @Test def `named-param-properly-order-1` = + check( + """|object O: + | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? + | method(par@@amB = 3, paramA = 1, paramD = 3, paramC = 1) + |""".stripMargin, + """|method([paramB: Int], [paramA: Int], [paramD: Int], [paramC: Int]): Unit + | ^^^^^^^^^^^^^ + |""".stripMargin + ) + + @Test def `named-param-properly-order-2` = + check( + """|object O: + | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? + | method(paramB = 3, par@@amA = 1, paramD = 3, paramC = 1) + |""".stripMargin, + """|method([paramB: Int], [paramA: Int], [paramD: Int], [paramC: Int]): Unit + | ^^^^^^^^^^^^^ + |""".stripMargin + ) + diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpPatternSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpPatternSuite.scala index f8a368a5ea3a..76f1c5c42b76 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpPatternSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpPatternSuite.scala @@ -202,22 +202,23 @@ class SignatureHelpPatternSuite extends BaseSignatureHelpSuite: |""".stripMargin ) - @Test def `pat4` = - check( - """ - |object & { - | def unapply[A](a: A): Some[(A, A)] = Some((a, a)) - |} - |object a { - | "" match { - | case "" & s@@ - | } - |} - """.stripMargin, - """|(String, String) - | ^^^^^^ - |""".stripMargin - ) +// This test is not important, it won't automatically trigger, and finding whether we're in infix expression is hard +// @Test def `pat4` = +// check( +// """ +// |object & { +// | def unapply[A](a: A): Some[(A, A)] = Some((a, a)) +// |} +// |object a { +// | "" match { +// | case "" & s@@ +// | } +// |} +// """.stripMargin, +// """|(String, String) +// | ^^^^^^ +// |""".stripMargin +// ) @Test def `pat5` = check( diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala index 0ed6680e6dd3..1ef6bb99fa65 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala @@ -1032,3 +1032,164 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | ^ |""".stripMargin ) + + // @Test def `dont-show-directly-after-parenthesis` = + // check( + // """|object Main { + // | def test(a: Int, b: Int): Int = ??? + // | test(1, 2)@@ + // |} + // |""".stripMargin, + // "" + // ) + + // @Test def `dont-show-directly-after-parenthesis-2` = + // check( + // """|object Main { + // | def test(a: Int, b: Int): Int = ??? + // | test(1, 2)@@ + // |""".stripMargin, + // "" + // ) + + @Test def `show-directly-when-unclosed` = + check( + """|object Main { + | def test(a: Int, b: Int): Int = ??? + | test(1, (2 + 1)@@ + |} + |""".stripMargin, + """|test(a: Int, b: Int): Int + | ^^^^^^ + |""".stripMargin + ) + + @Test def `dont-show-after-parenthesis-1` = + check( + """|object Main { + | def test(a: Int, b: Int): Int = ??? + | test(1, 2) @@ + |} + |""".stripMargin, + "" + ) + + @Test def `dont-show-after-parenthesis-2` = + check( + """|object Main { + | def test(a: Int, b: Int): Int = ??? + | test(1, 2) @@ + |} + |""".stripMargin, + "" + ) + + @Test def `dont-show-after-parenthesis-newline` = + check( + """|object Main { + | def test(a: Int, b: Int): Int = ??? + | test(1, 2) + |@@ + |} + |""".stripMargin, + "" + ) + + @Test def `dont-show-after-parenthesis-newline-last-statement` = + check( + """|object Main: + | def test(a: Int, b: Int): Int = ??? + | test(1, 2) + | + |@@ + | + |""".stripMargin, + "" + ) + + @Test def `show-after-parenthesis-newline-last-statement-unclosed-1` = + check( + """|object Main: + | def test(a: Int, b: Int): Int = ??? + | test(1, 2 + | + |@@ + | + |""".stripMargin, + """|test(a: Int, b: Int): Int + | ^^^^^^ + |""".stripMargin + ) + + @Test def `show-after-parenthesis-newline-last-statement-unclosed-2` = + check( + """|object Main: + | def test(a: Int, b: Int): Int = ??? + | test(1, (1 + 2) + | + |@@ + | + |""".stripMargin, + """|test(a: Int, b: Int): Int + | ^^^^^^ + |""".stripMargin + ) + + @Test def `show-after-parenthesis-unclosed-1` = + check( + """|object Main { + | def test(a: Int, b: Int): Int = ??? + | test(1, 2 + | + | @@ + | println("") + |} + |""".stripMargin, + """|test(a: Int, b: Int): Int + | ^^^^^^ + |""".stripMargin + ) + + @Test def `show-after-parenthesis-unclosed-2` = + check( + """|object Main { + | def test(a: Int, b: Int): Int = ??? + | test(1, 2 + | + | @@ + | println("") + |} + |""".stripMargin, + """|test(a: Int, b: Int): Int + | ^^^^^^ + |""".stripMargin + ) + + @Test def `show-after-parenthesis-unclosed-3` = + check( + """|object Main { + | def test(a: Int, b: Int): Int = ??? + | test(1, 2 + | + | @@ + |} + |""".stripMargin, + """|test(a: Int, b: Int): Int + | ^^^^^^ + |""".stripMargin + ) + + @Test def `show-after-parenthesis-unclosed-4` = + check( + """|object Main { + | def test(a: Int, b: Int): Int = ??? + | test(1, 2 + | + | @@ + |} + |""".stripMargin, + """|test(a: Int, b: Int): Int + | ^^^^^^ + |""".stripMargin + ) + From 04261652e320d9ecda2339b1f4e3ad9440997eed Mon Sep 17 00:00:00 2001 From: rochala Date: Wed, 6 Dec 2023 16:18:22 +0100 Subject: [PATCH 05/14] Refactor SignatureHelpProvider --- .../tools/pc/SignatureHelpProvider.scala | 110 ++++++++++-------- 1 file changed, 64 insertions(+), 46 deletions(-) diff --git a/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala b/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala index 93dfa993eaa8..504d88a6b13c 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala @@ -1,5 +1,6 @@ package dotty.tools.pc +import dotty.tools.dotc.ast.tpd.* import dotty.tools.dotc.core.Contexts.* import dotty.tools.dotc.core.Flags import dotty.tools.dotc.core.Symbols.* @@ -11,10 +12,11 @@ import dotty.tools.dotc.reporting.ExpectedTokenButFound import dotty.tools.dotc.util.Signatures import dotty.tools.dotc.util.SourceFile import dotty.tools.dotc.util.Spans +import dotty.tools.dotc.util.Spans.Span import dotty.tools.pc.utils.MtagsEnrichments.* import org.eclipse.lsp4j as l -import scala.jdk.CollectionConverters._ +import scala.jdk.CollectionConverters.* import scala.jdk.OptionConverters.* import scala.meta.pc.OffsetParams import scala.meta.pc.SymbolDocumentation @@ -37,51 +39,8 @@ object SignatureHelpProvider: given newCtx: Context = driver.currentCtx.fresh.setCompilationUnit(unit) val pos = driver.sourcePosition(params) - val treeSpanEnd = ctx.compilationUnit.tpdTree.span.end - - lazy val firstUnclosedErrorAfterPos = - val unclosedErrors = ctx.reporter.allErrors.filter { _.msg match - case err: ExpectedTokenButFound => closingRegionTokens.contains(err.expected) - case _ => false - } - unclosedErrors.find(_.pos.span.start >= pos.span.end).flatMap(_.position.toScala) - - // If we try to run signature help for named arg which happens to be the last line of the code - // it will return span outside tree, as parser ignores additional whitespaces: - // def foo(aaa: Int, bbb: Int) = ??? - // foo(aaa // fail before writing the = sign - val adjustedSpan = if pos.span.end > treeSpanEnd && firstUnclosedErrorAfterPos.nonEmpty then Spans.Span(treeSpanEnd) - else pos.span - - - val path = Interactive.pathTo(ctx.compilationUnit.tpdTree, adjustedSpan) - - import dotty.tools.dotc.ast.tpd._ - - val maybeNewSpan = firstUnclosedErrorAfterPos match - case Some(errorPosition) => - path - .headOption - // if in the head of the path, there is a tree defined between cursor and error position we are in correct tree - .filter(!_.existsSubTree(tree => tree.span.exists && tree.span.start >= pos.span.end)) - .toList - // find possible trees which can be part of unclosed signature help - .flatMap(_.filterSubTrees: - case tree: (Apply | UnApply | AppliedTypeTree | TypeApply) => tree.span.end <= pos.span.end - case _ => false - ).filter(tree => tree.span.end <= errorPosition.end ) - .maxByOption(_.span.end) - .map(tree => Spans.Span(tree.span.end) - ) - - case None => None - - - val adjustedPath = maybeNewSpan match - case Some(pos) => Interactive.pathTo(ctx.compilationUnit.tpdTree, pos) - case _ => path - - val (paramN, callableN, alternatives) = Signatures.signatureHelp(adjustedPath, pos.span) + val path = getPathWithUnclosedOpenings(pos.span) + val (paramN, callableN, alternatives) = Signatures.signatureHelp(path, pos.span) val infos = alternatives.flatMap: signature => signature.denot.map(signature -> _) @@ -106,6 +65,65 @@ object SignatureHelpProvider: case _ => new l.SignatureHelp() end signatureHelp + /** Interactive.pathTo returns a path to the tree, by relying on Span.contains(otherSpan). + * When there is an unclosed opening, parser will try to repair it, but the span of that tree + * will end on the new line or next illegal token. + * + * To support getting the tree withh unclosed opening, when our cursor is on the newline such as: + * ```scala + * def foo(aaa: Int, bbb: Int) = ??? + * foo(1, + * + * @@ + * // eof or illegal token + * ``` + * we need to rely on dianostics to locate last tree before the cursor + * and `opnning closed` expected syntax error. + * + * This is very hard to work around, as changing spans in the parser + * will affect other parts of the compiler such as reporting + */ + private def getPathWithUnclosedOpenings(span: Span)(using Context): List[Tree] = + lazy val firstUnclosedErrorAfterPos = + val unclosedErrors = ctx.reporter.allErrors.filter { _.msg match + case err: ExpectedTokenButFound => closingRegionTokens.contains(err.expected) + case _ => false + } + unclosedErrors.find(_.pos.span.start >= span.end).flatMap(_.position.toScala) + + val treeSpanEnd = ctx.compilationUnit.tpdTree.span.end + + // If we try to run signature help for named arg which happens to be the last line of the code + // it will return span outside tree, as parser ignores additional whitespaces: + // def foo(aaa: Int, bbb: Int) = ??? + // foo(aaa // fail before writing the = sign + val adjustedSpan = if span.end > treeSpanEnd && firstUnclosedErrorAfterPos.nonEmpty then + Spans.Span(treeSpanEnd) + else span + + lazy val path = Interactive.pathTo(ctx.compilationUnit.tpdTree, adjustedSpan) + + val maybeNewSpan = firstUnclosedErrorAfterPos match + case Some(errorPosition) => + path + .headOption + // if in the head of the path, there is a tree defined between cursor and error position we are in correct tree + .filter(!_.existsSubTree(tree => tree.span.exists && tree.span.start >= span.end)) + .toList + // find possible trees which can be part of unclosed signature help + .flatMap(_.filterSubTrees: + case tree: (Apply | UnApply | AppliedTypeTree | TypeApply) => tree.span.end <= span.end + case _ => false + ).filter(tree => tree.span.end <= errorPosition.end) + .maxByOption(_.span.end) + .map(tree => Spans.Span(tree.span.end) + ) + case None => None + + maybeNewSpan + .map(Interactive.pathTo(ctx.compilationUnit.tpdTree, _)) + .getOrElse(path) + private def withDocumentation( info: SymbolDocumentation, signature: Signatures.Signature, From a859a1aecd020864dc51e100ce7867e201cc000f Mon Sep 17 00:00:00 2001 From: rochala Date: Wed, 6 Dec 2023 16:19:53 +0100 Subject: [PATCH 06/14] Remove deprecated code --- .../src/dotty/tools/dotc/util/Signatures.scala | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/util/Signatures.scala b/compiler/src/dotty/tools/dotc/util/Signatures.scala index 96d76573b93e..9e5161d47d91 100644 --- a/compiler/src/dotty/tools/dotc/util/Signatures.scala +++ b/compiler/src/dotty/tools/dotc/util/Signatures.scala @@ -66,24 +66,6 @@ object Signatures { def signatureHelp(path: List[tpd.Tree], pos: Span)(using Context): (Int, Int, List[Signature]) = computeSignatureHelp(path, pos) - /** - * Extract (current parameter index, function index, functions) out of a method call. - * - * @param path The path to the function application - * @param span 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). - */ - @deprecated( - """This method is deprecated in favor of `signatureHelp`. - Returned denotation cannot be distinguished if its unapply or apply context""", - "3.1.3" - ) - def callInfo(path: List[tpd.Tree], span: Span)(using Context): (Int, Int, List[SingleDenotation]) = - val (paramN, funN, signatures) = computeSignatureHelp(path, span) - (paramN, funN, signatures.flatMap(_.denot)) - /** * Computes call info (current parameter index, function index, functions) for a method call. * From 83e337e704b451570ce229c200a192c9eae49818 Mon Sep 17 00:00:00 2001 From: rochala Date: Fri, 22 Dec 2023 20:23:07 +0100 Subject: [PATCH 07/14] Drop support for signature help with unclosed openings. Alternative search for curried methods --- .../dotty/tools/dotc/util/Signatures.scala | 81 +++++------ .../languageserver/DottyLanguageServer.scala | 6 +- .../languageserver/SignatureHelpTest.scala | 39 ++--- .../tools/pc/SignatureHelpProvider.scala | 63 +------- .../tools/pc/utils/MtagsEnrichments.scala | 4 +- .../SignatureHelpNamedArgsSuite.scala | 56 ++++++- .../SignatureHelpPatternSuite.scala | 18 --- .../signaturehelp/SignatureHelpSuite.scala | 137 ++++++------------ 8 files changed, 152 insertions(+), 252 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/util/Signatures.scala b/compiler/src/dotty/tools/dotc/util/Signatures.scala index 9e5161d47d91..b5e5703ce195 100644 --- a/compiler/src/dotty/tools/dotc/util/Signatures.scala +++ b/compiler/src/dotty/tools/dotc/util/Signatures.scala @@ -4,6 +4,7 @@ package util import dotty.tools.dotc.ast.NavigateAST import dotty.tools.dotc.ast.untpd import dotty.tools.dotc.core.NameOps.* +import dotty.tools.dotc.core.Symbols.defn import ast.Trees.* import ast.tpd @@ -122,7 +123,6 @@ object Signatures { val enclosingTree = enclosingFunction.getOrElse(expr) findEnclosingApply(Interactive.pathTo(enclosingTree, span), span) - case direct :: enclosing :: _ if isClosingSymbol(direct.source(span.end -1)) => enclosing case direct :: _ => direct @@ -169,9 +169,10 @@ object Signatures { case Select(qual, _) => qual case _ => tree + val paramssListIndex = findParamssIndex(fun) val (alternativeIndex, alternatives) = fun.tpe match case err: ErrorType => - val (alternativeIndex, alternatives) = alternativesFromError(err, params) match + val (alternativeIndex, alternatives) = alternativesFromError(err, params, paramssListIndex) match // if we have no alternatives from error, we have to fallback to function denotation // Check `partialyFailedCurriedFunctions` test for example case (_, Nil) => @@ -191,45 +192,42 @@ object Signatures { if alternativeIndex < alternatives.length then val alternativeSymbol = alternatives(alternativeIndex).symbol - val paramssListIndex = findParamssIndex(fun, alternatives(alternativeIndex)) - val previousArgs = alternativeSymbol.paramSymss.take(paramssListIndex).foldLeft(0)(_ + _.length) - val previousTypeParams = if !isTypeApply then alternativeSymbol.paramSymss.flatten.filter(_.isType).length else 0 val untpdPath: List[untpd.Tree] = NavigateAST .untypedPath(fun, false).collect { case untpdTree: untpd.Tree => untpdTree } val untpdArgs = untpdPath match - case Ident(_) :: New(_) :: Select(_, name) :: untpd.Apply(_, args) :: _ if name.isConstructorName => args + case (Ident(_) | Select(_, _)) :: New(_) :: Select(_, name) :: untpd.Apply(_, args) :: _ if name.isConstructorName => args case _ :: untpd.Apply(_, args) :: _ => args case _ :: untpd.TypeApply(_, args) :: _ => args case _ => Nil - val currentParamsIndex = untpdArgs.indexWhere(_.span.contains(span)) match + val currentParamsIndex = (untpdArgs.indexWhere(_.span.contains(span)) match case -1 if untpdArgs.isEmpty => 0 case -1 => commaIndex(untpdArgs, span) match - // comma is after CURSOR, so we are in parameter a - case Some(index) if index <= span.start => untpdArgs.takeWhile(_.span.start < span.start).length - // comma is before CURSOR, so we are in parameter b - case Some(index) => untpdArgs.takeWhile(_.span.start < span.start).length - 1 + // comma is before CURSOR, so we are in parameter b example: test("a", CURSOR) + case Some(index) if index <= span.end => untpdArgs.takeWhile(_.span.end < span.start).length + // comma is after CURSOR, so we are in parameter a example: test("a" CURSOR,) + case Some(index) => untpdArgs.takeWhile(_.span.start < span.end).length - 1 // we are either in first or last parameter case None => - if untpdArgs.head.span.start >= span.start then 0 + if untpdArgs.head.span.start >= span.end then 0 else untpdArgs.length - 1 max 0 - // special case if we pass more arguments than function has parameters - case n => n min (alternativeSymbol.paramSymss(paramssListIndex).length - 1) + case n => n + ) min (alternativeSymbol.paramSymss(paramssListIndex).length - 1) val firstOrderedParams = val originalParams = params.map(_.span) val untpdParams = untpdArgs.map(_.span) originalParams.zip(untpdParams).takeWhile((original, untpd) => original == untpd).length - val reorderedNamedArgs = untpdArgs.drop(firstOrderedParams).takeWhile(_.span.start <= span.start).collect: + val reorderedNamedArgs = untpdArgs.drop(firstOrderedParams).takeWhile(_.span.start <= span.end).collect: case namedArg: untpd.NamedArg => namedArg.name.show - val namedArgsAfterCursor = untpdArgs.drop(currentParamsIndex + 1).dropWhile(_.span.start <= span.start).collect: + val namedArgsAfterCursor = untpdArgs.drop(currentParamsIndex).dropWhile(_.span.start <= span.end).collect: case namedArg: untpd.NamedArg => namedArg.name.show val pre = treeQualifier(fun) @@ -237,7 +235,7 @@ object Signatures { val alternativeSignatures = alternativesWithTypes .flatMap(toApplySignature(_, reorderedNamedArgs, namedArgsAfterCursor, firstOrderedParams)) - val finalParamIndex = currentParamsIndex + previousArgs + previousTypeParams + val finalParamIndex = currentParamsIndex + previousArgs (finalParamIndex, alternativeIndex, alternativeSignatures) else (0, 0, Nil) @@ -249,13 +247,15 @@ object Signatures { * @return None if we are in first or last parameter, comma index otherwise */ private def commaIndex(untpdArgs: List[untpd.Tree], span: Span)(using Context): Option[Int] = - val previousArgIndex = untpdArgs.lastIndexWhere(_.span.end < span.start) + val previousArgIndex = untpdArgs.lastIndexWhere(_.span.end < span.end) for previousArg <- untpdArgs.lift(previousArgIndex) - nextArg <- untpdArgs.lift(previousArgIndex + 1) + nextArg = untpdArgs.lift(previousArgIndex + 1) + text = ctx.source.content.slice(previousArg.span.end - 1, nextArg.map(_.span.start).getOrElse(span.end)) + commaIndex = text.indexOf(',') + if commaIndex != -1 yield - val text = ctx.source.content.slice(previousArg.span.end, nextArg.span.start) - text.indexOf(',') + previousArg.span.end + commaIndex + previousArg.span.end /** * Extracts call informatioin for function in unapply context. @@ -278,7 +278,7 @@ object Signatures { val paramTypes = extractParamTypess(resultType, denot, patterns.size).flatten.map(stripAllAnnots) val paramNames = extractParamNamess(resultType, denot).flatten - val activeParameter = unapplyParameterIndex(patterns, span, paramTypes.length) + val activeParameter = patterns.takeWhile(_.span.end < span.start).length min (paramTypes.length - 1) val unapplySignature = toUnapplySignature(denot.asSingleDenotation, paramNames, paramTypes).toList (activeParameter, 0, unapplySignature) @@ -339,21 +339,6 @@ object Signatures { case AppliedType(t, args) => AppliedType(stripAllAnnots(t), args.map(stripAllAnnots)) case other => other.stripAnnots - /** - * Get index of currently edited parameter in unapply context. - * - * @param patterns Currently applied patterns for unapply method - * @param span The position of the cursor - * @param maximumParams Number of parameters taken by unapply method - * @return Index of currently edited parameter - */ - private def unapplyParameterIndex(patterns: List[tpd.Tree], span: Span, maximumParams: Int)(using Context): Int = - val patternPosition = patterns.indexWhere(_.span.contains(span)) - (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 _ => (maximumParams - 1) min patternPosition max 0 // handle unapplySeq to always highlight Seq[A] on elements - /** * Checks if tree is valid for signatureHelp. Skipped trees are either tuple type or function type * @@ -476,7 +461,7 @@ object Signatures { val remainingNamed = remainingNamedUnordered.sortBy(p => namedParamsAfterCursor.indexOf(p.name)) val reorderedArgs = (reorderedParamsBeforeCursor ++ remaining ++ remainingNamed) - .map(_.copy(isReordered = reorderedParamsBeforeCursor.nonEmpty)) + .map(_.copy(isReordered = reorderedParamsBeforeCursor.nonEmpty || remainingNamed != remainingNamedUnordered)) (ordered ++ reorderedArgs) :: rest @@ -543,18 +528,17 @@ object Signatures { * parameter number for erroneous applications before current one. * * This handles currying, so for an application such as `foo(1, 2)(3)`, the result of - * `countParams` should be 3. It also takes into considerations unapplied arguments so for `foo(1)(3)` - * we will still get 3, as first application `foo(1)` takes 2 parameters with currently only 1 applied. + * `countParams` should be 1. * * @param tree The tree to inspect. * @param denot Denotation of function we are trying to apply * @param alreadyCurried Number of subsequent Apply trees before current tree * - * @return The number of parameters that are passed. + * @return The index of paramss we are currently in. */ - private def findParamssIndex(tree: tpd.Tree, denot: SingleDenotation, alreadyCurried: Int = 0)(using Context): Int = + private def findParamssIndex(tree: tpd.Tree, alreadyCurried: Int = 0)(using Context): Int = tree match - case Apply(fun, params) => findParamssIndex(fun, denot, alreadyCurried + 1) + case GenericApply(fun, params) => findParamssIndex(fun, alreadyCurried + 1) case _ => alreadyCurried /** @@ -565,13 +549,14 @@ object Signatures { * 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. + * @param err The error message to inspect. + * @param params The parameters that were given at the call site. + * @param alreadyCurried Index of paramss we are currently in. * * @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[tpd.Tree])(using Context): (Int, List[SingleDenotation]) = { + private def alternativesFromError(err: ErrorType, params: List[tpd.Tree], paramssIndex: Int)(using Context): (Int, List[SingleDenotation]) = { val alternatives = err.msg match case msg: AmbiguousOverload => msg.alternatives @@ -583,11 +568,13 @@ object Signatures { // 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 => + val alreadyCurriedBonus = if (alt.symbol.paramSymss.length > paramssIndex) 1 else 0 alt.info.stripPoly match - case tpe: MethodType => + case tpe: MethodType => alreadyCurriedBonus + userParamsTypes.zip(tpe.paramInfos).takeWhile{ case (t0, t1) => t0 <:< t1 }.size case _ => 0 } + val bestAlternative = if (alternativesScores.isEmpty) 0 else alternativesScores.zipWithIndex.maxBy(_._1)._2 diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index 6a7eb9c6a629..534fa4a9b995 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -556,10 +556,10 @@ class DottyLanguageServer extends LanguageServer driver.compilationUnits.get(uri) match case Some(unit) => given newCtx: Context = driver.currentCtx.fresh.setCompilationUnit(unit) - val pos = sourcePosition(driver, uri, params.getPosition) - val path = Interactive.pathTo(ctx.compilationUnit.tpdTree, pos.span) - val (paramN, callableN, signatures) = Signatures.signatureHelp(path, pos.span) + 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) diff --git a/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala b/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala index 7487dfaba97b..5421e48ac5de 100644 --- a/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala +++ b/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala @@ -17,23 +17,6 @@ class SignatureHelpTest { .signatureHelp(m1, List(signature), Some(0), 0) } - @Test def properFunctionReturnWithoutParenthesis: Unit = { - val listSignature = S("apply", List("A"), List(List(P("elems", "A*"))), Some("List[A]")) - val optionSignature = S("apply", List("A"), List(List(P("x", "A"))), Some("Option[A]")) - code"""object O { - | List(1, 2$m1 - |} - |object T { - | List(Option(1$m2 - |} - |object Z { - | List(Option(1)$m3 - |}""" - .signatureHelp(m1, List(listSignature), Some(0), 1) - .signatureHelp(m2, List(optionSignature), Some(0), 1) - .signatureHelp(m3, List(listSignature), Some(0), 1) - } - @Test def errorTypeParameter: Unit = { val emptySignature = S("empty", List("K", "V"), Nil, Some("Map[K, V]")) code"""object O: @@ -102,7 +85,7 @@ class SignatureHelpTest { | curry(1$m1)$m2(3$m3) |}""" .signatureHelp(m1, List(listSignature), Some(0), 0) - .signatureHelp(m2, List(listSignature), Some(0), 0) + .signatureHelp(m2, List(listSignature), Some(0), 2) .signatureHelp(m3, List(listSignature), Some(0), 2) } @@ -141,7 +124,7 @@ class SignatureHelpTest { |object O { | Foo(5).method($m1) |}""" - .signatureHelp(m1, List(testSig), Some(0), 0) + .signatureHelp(m1, List(testSig), Some(0), -1) } @Test def unapplyBooleanReturn: Unit = { @@ -153,7 +136,7 @@ class SignatureHelpTest { | case s @ Even(${m1}) => println(s"s has an even number of characters") | case s => println(s"s has an odd number of characters") """ - .signatureHelp(m1, Nil, Some(0), 0) + .signatureHelp(m1, Nil, Some(0), -1) } @Test def unapplyCustomClass: Unit = { @@ -226,9 +209,9 @@ class SignatureHelpTest { | } |}""" .signatureHelp(m1, List(signature), Some(0), 0) - .signatureHelp(m2, List(signature), Some(0), -1) - .signatureHelp(m3, List(signature), Some(0), -1) - .signatureHelp(m4, List(signature), Some(0), -1) + .signatureHelp(m2, List(signature), Some(0), 1) + .signatureHelp(m3, List(signature), Some(0), 1) + .signatureHelp(m4, List(), Some(0), 0) } @Test def unapplyClass: Unit = { @@ -325,13 +308,11 @@ class SignatureHelpTest { |object Main { | Seq(1,2,3) match { | case Seq($m1) => - | case h$m2 :: t$m3 => + | case h :: t => | } |} """ .signatureHelp(m1, List(signatureSeq), Some(0), 0) - .signatureHelp(m2, List(signatureVariadicExtractor), Some(0), 0) - .signatureHelp(m3, List(signatureVariadicExtractor), Some(0), 1) } @Test def productTypeClassMatch: Unit = { @@ -392,7 +373,7 @@ class SignatureHelpTest { | case _ => () """ .signatureHelp(m1, List(nameBasedMatch), Some(0), 0) - .signatureHelp(m2, List(nameBasedMatch), Some(0), -1) + .signatureHelp(m2, List(nameBasedMatch), Some(0), 0) .signatureHelp(m3, List(singleMatch), Some(0), 0) } @@ -713,7 +694,7 @@ class SignatureHelpTest { }""" .signatureHelp(m1, List(sig0, sig1), None, 0) .signatureHelp(m2, List(sig0, sig1), None, 0) - .signatureHelp(m3, List(sig0, sig1), Some(1), 1) + .signatureHelp(m3, List(sig0, sig1), Some(0), 1) } @Test def multipleParameterLists: Unit = { @@ -979,7 +960,7 @@ class SignatureHelpTest { code"""|object O: | implicit class TypeVarTest[T](xs: List[T]): | def test(x: T): List[T] = ??? - | List(1,2,3).test(${m1}""" + | List(1,2,3).test(${m1})""" .signatureHelp(m1, List(signature), None, 0) } diff --git a/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala b/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala index 504d88a6b13c..9044aefa2b3d 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala @@ -38,8 +38,8 @@ object SignatureHelpProvider: case Some(unit) => given newCtx: Context = driver.currentCtx.fresh.setCompilationUnit(unit) - val pos = driver.sourcePosition(params) - val path = getPathWithUnclosedOpenings(pos.span) + val pos = driver.sourcePosition(params, isZeroExtent = false) + val path = Interactive.pathTo(unit.tpdTree, pos.span) val (paramN, callableN, alternatives) = Signatures.signatureHelp(path, pos.span) val infos = alternatives.flatMap: signature => @@ -65,65 +65,6 @@ object SignatureHelpProvider: case _ => new l.SignatureHelp() end signatureHelp - /** Interactive.pathTo returns a path to the tree, by relying on Span.contains(otherSpan). - * When there is an unclosed opening, parser will try to repair it, but the span of that tree - * will end on the new line or next illegal token. - * - * To support getting the tree withh unclosed opening, when our cursor is on the newline such as: - * ```scala - * def foo(aaa: Int, bbb: Int) = ??? - * foo(1, - * - * @@ - * // eof or illegal token - * ``` - * we need to rely on dianostics to locate last tree before the cursor - * and `opnning closed` expected syntax error. - * - * This is very hard to work around, as changing spans in the parser - * will affect other parts of the compiler such as reporting - */ - private def getPathWithUnclosedOpenings(span: Span)(using Context): List[Tree] = - lazy val firstUnclosedErrorAfterPos = - val unclosedErrors = ctx.reporter.allErrors.filter { _.msg match - case err: ExpectedTokenButFound => closingRegionTokens.contains(err.expected) - case _ => false - } - unclosedErrors.find(_.pos.span.start >= span.end).flatMap(_.position.toScala) - - val treeSpanEnd = ctx.compilationUnit.tpdTree.span.end - - // If we try to run signature help for named arg which happens to be the last line of the code - // it will return span outside tree, as parser ignores additional whitespaces: - // def foo(aaa: Int, bbb: Int) = ??? - // foo(aaa // fail before writing the = sign - val adjustedSpan = if span.end > treeSpanEnd && firstUnclosedErrorAfterPos.nonEmpty then - Spans.Span(treeSpanEnd) - else span - - lazy val path = Interactive.pathTo(ctx.compilationUnit.tpdTree, adjustedSpan) - - val maybeNewSpan = firstUnclosedErrorAfterPos match - case Some(errorPosition) => - path - .headOption - // if in the head of the path, there is a tree defined between cursor and error position we are in correct tree - .filter(!_.existsSubTree(tree => tree.span.exists && tree.span.start >= span.end)) - .toList - // find possible trees which can be part of unclosed signature help - .flatMap(_.filterSubTrees: - case tree: (Apply | UnApply | AppliedTypeTree | TypeApply) => tree.span.end <= span.end - case _ => false - ).filter(tree => tree.span.end <= errorPosition.end) - .maxByOption(_.span.end) - .map(tree => Spans.Span(tree.span.end) - ) - case None => None - - maybeNewSpan - .map(Interactive.pathTo(ctx.compilationUnit.tpdTree, _)) - .getOrElse(path) - private def withDocumentation( info: SymbolDocumentation, signature: Signatures.Signature, diff --git a/presentation-compiler/src/main/dotty/tools/pc/utils/MtagsEnrichments.scala b/presentation-compiler/src/main/dotty/tools/pc/utils/MtagsEnrichments.scala index ad6265c00773..d59a7b24eee6 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/utils/MtagsEnrichments.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/utils/MtagsEnrichments.scala @@ -36,7 +36,8 @@ object MtagsEnrichments extends CommonMtagsEnrichments: extension (driver: InteractiveDriver) def sourcePosition( - params: OffsetParams + params: OffsetParams, + isZeroExtent: Boolean = true ): SourcePosition = val uri = params.uri() val source = driver.openedFiles(uri.nn) @@ -50,6 +51,7 @@ object MtagsEnrichments extends CommonMtagsEnrichments: case offset => Spans.Span(p.offset(), p.offset()) } + case _ if !isZeroExtent => Spans.Span(params.offset(), params.offset() + 1) case _ => Spans.Span(params.offset()) new SourcePosition(source, span) diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala index 8c9ea79d4444..165b0f325dff 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala @@ -33,7 +33,7 @@ class SignatureHelpNamedArgsSuite extends BaseSignatureHelpSuite: check( """|object O: | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? - | method(paramA = 1, paramB @@ + | method(paramA = 1, paramB @@) |""".stripMargin, """|method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit | ^^^^^^^^^^^ @@ -44,7 +44,7 @@ class SignatureHelpNamedArgsSuite extends BaseSignatureHelpSuite: check( """|object O: | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? - | method(paramA = 1, paramB @@ + | method(paramA = 1, paramB @@) | def test: Unit = () |""".stripMargin, """|method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit @@ -56,7 +56,7 @@ class SignatureHelpNamedArgsSuite extends BaseSignatureHelpSuite: check( """|object O: | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? - | method(paramA = 1, paramB =@@ + | method(paramA = 1, paramB =@@) |""".stripMargin, """|method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit | ^^^^^^^^^^^ @@ -67,7 +67,7 @@ class SignatureHelpNamedArgsSuite extends BaseSignatureHelpSuite: check( """|object O: | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? - | method(paramA = 1, paramB = 1@@ + | method(paramA = 1, paramB = 1@@) |""".stripMargin, """|method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit | ^^^^^^^^^^^ @@ -250,3 +250,51 @@ class SignatureHelpNamedArgsSuite extends BaseSignatureHelpSuite: |""".stripMargin ) + @Test def `named-with-type-args` = + check( + """|object Main: + | def test[T, F](aaa: Int, bbb: T, ccc: F): T = ??? + | val x = test(1, ccc = 2, b@@) + |""".stripMargin, + """|test[T, F](aaa: Int, [ccc: F], [bbb: T]): T + | ^^^^^^^^ + |""".stripMargin + ) + + @Test def `named-newline` = + check( + """|object Main: + | def test2(aaa: Int, bbb: Int, ccc: Int): Int = ??? + | val x = test2( + | 1, + | @@ + | ) + |""".stripMargin, + """|test2(aaa: Int, bbb: Int, ccc: Int): Int + | ^^^^^^^^ + |""".stripMargin + ) + + @Test def `named-before-comma` = + check( + """|object Main: + | def test2(aaa: Int, bbb: Int, ccc: Int): Int = ??? + | val x = test2(aaa = 2@@, ccc = 1) + |""".stripMargin, + """|test2(aaa: Int, bbb: Int, ccc: Int): Int + | ^^^^^^^^ + |""".stripMargin + ) + + @Test def `named-on-whitespaces-between-args` = + check( + """|object Main: + | def test2(aaa: Int, bbb: Int, ccc: Int): Int = ??? + | val x = test2(aaa = 2, @@ ccc = 1, bbb = 3) + | + |""".stripMargin, + """|test2(aaa: Int, [ccc: Int], [bbb: Int]): Int + | ^^^^^^^^^^ + |""".stripMargin + ) + diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpPatternSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpPatternSuite.scala index 76f1c5c42b76..5aae9da1664f 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpPatternSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpPatternSuite.scala @@ -202,24 +202,6 @@ class SignatureHelpPatternSuite extends BaseSignatureHelpSuite: |""".stripMargin ) -// This test is not important, it won't automatically trigger, and finding whether we're in infix expression is hard -// @Test def `pat4` = -// check( -// """ -// |object & { -// | def unapply[A](a: A): Some[(A, A)] = Some((a, a)) -// |} -// |object a { -// | "" match { -// | case "" & s@@ -// | } -// |} -// """.stripMargin, -// """|(String, String) -// | ^^^^^^ -// |""".stripMargin -// ) - @Test def `pat5` = check( """ diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala index 1ef6bb99fa65..8c4e647d91f2 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala @@ -196,7 +196,7 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: check( """ |object a { - | List(1, 2@@ + | List(1, 2@@) |} """.stripMargin, """|apply[A](elems: A*): List[A] @@ -650,9 +650,7 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | identity(42)@@ |} |""".stripMargin, - """|identity[A](x: A): A - | ^^^^ - |""".stripMargin + "" ) @Test def `off-by-one2` = @@ -675,7 +673,7 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: |} |""".stripMargin, """|fold[B](ifEmpty: => B)(f: Int => B): B - | ^^^^^^^^^^^^^ + | ^^^^^^^^^^^ |""".stripMargin ) @@ -763,24 +761,13 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: """|object O: | implicit class Test[T](xs: List[T]): | def test(x: T): List[T] = ??? - | List(1,2,3).test(@@""".stripMargin, + | List(1,2,3).test(s@@)""".stripMargin, """|test(x: Int): List[Int] | ^^^^^^ |""".stripMargin ) @Test def `instantiated-type-var-old-ext-2` = - check( - """|object O: - | implicit class Test[T](xs: List[T]): - | def test(x: T): List[T] = ??? - | List(1,2,3).test(s@@""".stripMargin, - """|test(x: Int): List[Int] - | ^^^^^^ - |""".stripMargin - ) - - @Test def `instantiated-type-var-old-ext-3` = check( """|object O: | implicit class Test[T](xs: List[T]): @@ -791,7 +778,7 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: |""".stripMargin ) - @Test def `instantiated-type-var-old-ext-4` = + @Test def `instantiated-type-var-old-ext-3` = check( """|object O: | implicit class Test[T](xs: List[T]): @@ -804,7 +791,7 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: |""".stripMargin ) - @Test def `instantiated-type-var-old-ext-5` = + @Test def `instantiated-type-var-old-ext-4` = check( """|object O: | implicit class Test[T](xs: List[T]): @@ -818,7 +805,7 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: |""".stripMargin ) - @Test def `instantiated-type-var-old-ext-6` = + @Test def `instantiated-type-var-old-ext-5` = check( """|object O: | implicit class Test[T](xs: List[T]): @@ -1033,24 +1020,24 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: |""".stripMargin ) - // @Test def `dont-show-directly-after-parenthesis` = - // check( - // """|object Main { - // | def test(a: Int, b: Int): Int = ??? - // | test(1, 2)@@ - // |} - // |""".stripMargin, - // "" - // ) - - // @Test def `dont-show-directly-after-parenthesis-2` = - // check( - // """|object Main { - // | def test(a: Int, b: Int): Int = ??? - // | test(1, 2)@@ - // |""".stripMargin, - // "" - // ) + @Test def `dont-show-directly-after-parenthesis` = + check( + """|object Main { + | def test(a: Int, b: Int): Int = ??? + | test(1, 2)@@ + |} + |""".stripMargin, + "" + ) + + @Test def `dont-show-directly-after-parenthesis-2` = + check( + """|object Main { + | def test(a: Int, b: Int): Int = ??? + | test(1, 2)@@ + |""".stripMargin, + "" + ) @Test def `show-directly-when-unclosed` = check( @@ -1059,9 +1046,7 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | test(1, (2 + 1)@@ |} |""".stripMargin, - """|test(a: Int, b: Int): Int - | ^^^^^^ - |""".stripMargin + "" ) @Test def `dont-show-after-parenthesis-1` = @@ -1107,7 +1092,7 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: "" ) - @Test def `show-after-parenthesis-newline-last-statement-unclosed-1` = + @Test def `dont-show-after-parenthesis-newline-last-statement-unclosed-1` = check( """|object Main: | def test(a: Int, b: Int): Int = ??? @@ -1116,12 +1101,10 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: |@@ | |""".stripMargin, - """|test(a: Int, b: Int): Int - | ^^^^^^ - |""".stripMargin + "" ) - @Test def `show-after-parenthesis-newline-last-statement-unclosed-2` = + @Test def `dont-show-after-parenthesis-newline-last-statement-unclosed-2` = check( """|object Main: | def test(a: Int, b: Int): Int = ??? @@ -1130,66 +1113,42 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: |@@ | |""".stripMargin, - """|test(a: Int, b: Int): Int - | ^^^^^^ - |""".stripMargin - ) - - @Test def `show-after-parenthesis-unclosed-1` = - check( - """|object Main { - | def test(a: Int, b: Int): Int = ??? - | test(1, 2 - | - | @@ - | println("") - |} - |""".stripMargin, - """|test(a: Int, b: Int): Int - | ^^^^^^ - |""".stripMargin + "" ) - @Test def `show-after-parenthesis-unclosed-2` = + @Test def `dont-show-after-parenthesis-unclosed-2` = check( """|object Main { | def test(a: Int, b: Int): Int = ??? | test(1, 2 | | @@ - | println("") |} |""".stripMargin, - """|test(a: Int, b: Int): Int - | ^^^^^^ - |""".stripMargin + "" ) - @Test def `show-after-parenthesis-unclosed-3` = + @Test def `select-arg-detection` = check( - """|object Main { - | def test(a: Int, b: Int): Int = ??? - | test(1, 2 - | - | @@ - |} + """|object Main: + | object Foo: + | case class Test(x: Int) + | def test(a: Foo.Test, b: Foo.Test): Int = ??? + | test(Foo.Test(1), @@) |""".stripMargin, - """|test(a: Int, b: Int): Int - | ^^^^^^ + """|test(a: Main.Foo.Test, b: Main.Foo.Test): Int + | ^^^^^^^^^^^^^^^^ |""".stripMargin - ) + ) - @Test def `show-after-parenthesis-unclosed-4` = + @Test def `singature-help-works-in-select` = check( - """|object Main { - | def test(a: Int, b: Int): Int = ??? - | test(1, 2 - | - | @@ - |} + """|object Main: + | object Foo: + | class Test(x: Int, y: Int) + | new Foo.Test(1, @@) |""".stripMargin, - """|test(a: Int, b: Int): Int + """|Test(x: Int, y: Int) | ^^^^^^ |""".stripMargin - ) - + ) From 13a0dcd0346ef4b9017b1de55ad809d883672c13 Mon Sep 17 00:00:00 2001 From: rochala Date: Thu, 28 Dec 2023 18:11:18 +0100 Subject: [PATCH 08/14] Add support for clause interleaving, simplify parameter ordering, make error recovery better, drop support for unclosed openings --- .../dotty/tools/dotc/util/Signatures.scala | 268 ++++++++---- .../languageserver/DottyLanguageServer.scala | 30 +- .../languageserver/SignatureHelpTest.scala | 154 ++++--- .../tools/pc/SignatureHelpProvider.scala | 75 ++-- .../pc/base/BaseSignatureHelpSuite.scala | 5 +- .../tests/completion/CompletionDocSuite.scala | 8 +- .../tools/pc/tests/hover/HoverDocSuite.scala | 6 +- .../pc/tests/hover/HoverNamedArgSuite.scala | 2 +- .../signaturehelp/SignatureHelpDocSuite.scala | 17 +- .../SignatureHelpInterleavingSuite.scala | 401 ++++++++++++++++++ .../SignatureHelpNamedArgsSuite.scala | 14 +- .../signaturehelp/SignatureHelpSuite.scala | 110 ++++- .../dotty/tools/pc/utils/MockEntries.scala | 12 +- 13 files changed, 847 insertions(+), 255 deletions(-) create mode 100644 presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpInterleavingSuite.scala diff --git a/compiler/src/dotty/tools/dotc/util/Signatures.scala b/compiler/src/dotty/tools/dotc/util/Signatures.scala index b5e5703ce195..72ed4e93af1e 100644 --- a/compiler/src/dotty/tools/dotc/util/Signatures.scala +++ b/compiler/src/dotty/tools/dotc/util/Signatures.scala @@ -4,6 +4,7 @@ package util import dotty.tools.dotc.ast.NavigateAST import dotty.tools.dotc.ast.untpd import dotty.tools.dotc.core.NameOps.* +import dotty.tools.dotc.core.StdNames.nme import dotty.tools.dotc.core.Symbols.defn import ast.Trees.* @@ -34,26 +35,44 @@ object Signatures { */ case class Signature( name: String, - tparams: List[String], paramss: List[List[Param]], returnType: Option[String], doc: Option[String] = None, denot: Option[SingleDenotation] = None ) + sealed trait Param: + def show: String + val 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? + * @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? + * @param isReordered Is the parameter reordered in its parameter list? */ - case class Param(name: String, tpe: String, doc: Option[String] = None, isImplicit: Boolean = false, isReordered: Boolean = false) { + case class MethodParam( + name: String, + tpe: String, + override val doc: Option[String] = None, + isImplicit: Boolean = false, + isReordered: Boolean = false + ) extends Param: def show: String = if name.nonEmpty && !isReordered then s"$name: $tpe" else if name.nonEmpty then s"[$name: $tpe]" else tpe - } + + /** + * Represent a type parameter. + * + * @param tpe The type of the parameter + * @param doc The documentation of this parameter + */ + case class TypeParam(tpe: String, override val doc: Option[String] = None) extends Param: + def show = tpe /** * Extract (current parameter index, function index, functions) method call for given position. @@ -78,10 +97,10 @@ object Signatures { */ def computeSignatureHelp(path: List[tpd.Tree], span: Span)(using Context): (Int, Int, List[Signature]) = findEnclosingApply(path, span) match - case Apply(fun, params) => applyCallInfo(span, params, fun) + case Apply(fun, params) => applyCallInfo(span, params, fun, false) case UnApply(fun, _, patterns) => unapplyCallInfo(span, fun, patterns) case appliedTypeTree @ AppliedTypeTree(_, types) => appliedTypeTreeCallInfo(appliedTypeTree, types) - case tp @ TypeApply(fun, types) => applyCallInfo(span, types, fun, true) + case tp @ TypeApply(fun, types) => applyCallInfo(span, types, fun, isTypeApply = true) case _ => (0, 0, Nil) @@ -139,13 +158,26 @@ object Signatures { types: List[tpd.Tree] )(using Context): (Int, Int, List[Signature]) = val typeName = fun.symbol.name.show - val typeParams = fun.symbol.typeRef.typeParams.map(_.paramName.show) + val typeParams = fun.symbol.typeRef.typeParams.map(_.paramName.show).map(TypeParam.apply(_)) val denot = fun.denot.asSingleDenotation val activeParameter = (types.length - 1) max 0 - val signature = Signature(typeName, typeParams, Nil, Some(typeName) , None, Some(denot)) + val signature = Signature(typeName, List(typeParams), Some(typeName) , None, Some(denot)) (activeParameter, 0, List(signature)) + private def findOutermostCurriedApply(untpdPath: List[untpd.Tree]): Option[untpd.GenericApply] = + val trimmedPath = untpdPath.dropWhile(!_.isInstanceOf[untpd.GenericApply]) + val maybeCurriedTrees = (trimmedPath zip trimmedPath.drop(1)).takeWhile: + case (currentTree: untpd.GenericApply, nextTree: untpd.GenericApply) => + nextTree.fun == currentTree + case _ => false + .map(_._2) + + maybeCurriedTrees.lastOption.orElse: + trimmedPath.headOption + .collect: + case genericApply: untpd.GenericApply => genericApply + /** * Extracts call information for a function application and type application. * @@ -192,13 +224,15 @@ object Signatures { if alternativeIndex < alternatives.length then val alternativeSymbol = alternatives(alternativeIndex).symbol - val previousArgs = alternativeSymbol.paramSymss.take(paramssListIndex).foldLeft(0)(_ + _.length) + val safeParamssListIndex = paramssListIndex min (alternativeSymbol.paramSymss.length - 1) + val previousArgs = alternativeSymbol.paramSymss.take(safeParamssListIndex).foldLeft(0)(_ + _.length) val untpdPath: List[untpd.Tree] = NavigateAST .untypedPath(fun, false).collect { case untpdTree: untpd.Tree => untpdTree } val untpdArgs = untpdPath match - case (Ident(_) | Select(_, _)) :: New(_) :: Select(_, name) :: untpd.Apply(_, args) :: _ if name.isConstructorName => args + case (Ident(_) | Select(_, _)) :: New(_) :: Select(_, name) :: untpd.Apply(untpdFun, args) :: _ + if name.isConstructorName => args case _ :: untpd.Apply(_, args) :: _ => args case _ :: untpd.TypeApply(_, args) :: _ => args case _ => Nil @@ -217,23 +251,12 @@ object Signatures { else untpdArgs.length - 1 max 0 case n => n - ) min (alternativeSymbol.paramSymss(paramssListIndex).length - 1) - - val firstOrderedParams = - val originalParams = params.map(_.span) - val untpdParams = untpdArgs.map(_.span) - originalParams.zip(untpdParams).takeWhile((original, untpd) => original == untpd).length - - val reorderedNamedArgs = untpdArgs.drop(firstOrderedParams).takeWhile(_.span.start <= span.end).collect: - case namedArg: untpd.NamedArg => namedArg.name.show - - val namedArgsAfterCursor = untpdArgs.drop(currentParamsIndex).dropWhile(_.span.start <= span.end).collect: - case namedArg: untpd.NamedArg => namedArg.name.show + ) min (alternativeSymbol.paramSymss(safeParamssListIndex).length - 1) val pre = treeQualifier(fun) val alternativesWithTypes = alternatives.map(_.asSeenFrom(pre.tpe.widenTermRefExpr)) val alternativeSignatures = alternativesWithTypes - .flatMap(toApplySignature(_, reorderedNamedArgs, namedArgsAfterCursor, firstOrderedParams)) + .flatMap(toApplySignature(_, findOutermostCurriedApply(untpdPath), safeParamssListIndex)) val finalParamIndex = currentParamsIndex + previousArgs (finalParamIndex, alternativeIndex, alternativeSignatures) @@ -340,12 +363,22 @@ object Signatures { case other => other.stripAnnots /** - * Checks if tree is valid for signatureHelp. Skipped trees are either tuple type or function type + * Checks if tree is valid for signatureHelp. Skipped trees are either tuple or function applies * * @param tree tree to validate */ private def isValid(tree: tpd.Tree)(using Context): Boolean = - !ctx.definitions.isTupleNType(tree.tpe) && !ctx.definitions.isFunctionNType(tree.tpe) + val isTupleApply = + tree.symbol.name == nme.apply + && tree.symbol.exists + && ctx.definitions.isTupleClass(tree.symbol.owner.companionClass) + + val isFunctionNApply = + tree.symbol.name == nme.apply + && tree.symbol.exists + && ctx.definitions.isFunctionSymbol(tree.symbol.owner) + + !isTupleApply && !isFunctionNApply /** * Get unapply method result type omiting unknown types and another method calls. @@ -421,9 +454,8 @@ object Signatures { */ private def toApplySignature( denot: SingleDenotation, - namedParamsBeforeCursor: List[String], - namedParamsAfterCursor: List[String], - firstOrderedParams: Int + untpdFun: Option[untpd.GenericApply], + paramssIndex: Int )(using Context): Option[Signature] = { val symbol = denot.symbol val docComment = ParsedComment.docOf(symbol) @@ -431,72 +463,121 @@ object Signatures { def isDummyImplicit(res: MethodType): Boolean = res.resultType.isParameterless && res.isImplicitMethod && - res.paramInfos.forall(info => - info.classSymbol.derivesFrom(ctx.definitions.DummyImplicitClass)) - - def toParamss(tp: Type)(using Context): List[List[Param]] = - val rest = tp.resultType match - case res: MethodType => if isDummyImplicit(res) then Nil else toParamss(res) - case _ => Nil - - val currentParams = (tp.paramNamess, tp.paramInfoss) match - case (params :: _, infos :: _) => params zip infos - case _ => Nil - - val params = currentParams.map: (name, info) => - Signatures.Param( - name.show, - info.widenTermRefExpr.show, - docComment.flatMap(_.paramDoc(name)), - isImplicit = tp.isImplicitMethod, + ( + res.paramNames.forall(name => + name.startsWith(NameKinds.ContextBoundParamName.separator) || + name.startsWith(NameKinds.ContextFunctionParamName.separator)) || + res.paramInfos.forall(info => + info.classSymbol.derivesFrom(ctx.definitions.DummyImplicitClass)) ) - val ordered = params.take(firstOrderedParams) - val reorderedParamsBeforeCursor = namedParamsBeforeCursor.flatMap: name => - params.find(_.name == name) - - val (remainingNamedUnordered, remaining) = params - .diff(reorderedParamsBeforeCursor).diff(ordered) - .partition(p => namedParamsAfterCursor.contains(p.name)) + def toApplyList(tree: untpd.GenericApply): List[untpd.GenericApply] = + tree match + case untpd.GenericApply(fun: untpd.GenericApply, args) => toApplyList(fun) :+ tree + case _ => List(tree) - val remainingNamed = remainingNamedUnordered.sortBy(p => namedParamsAfterCursor.indexOf(p.name)) - val reorderedArgs = (reorderedParamsBeforeCursor ++ remaining ++ remainingNamed) - .map(_.copy(isReordered = reorderedParamsBeforeCursor.nonEmpty || remainingNamed != remainingNamedUnordered)) - - (ordered ++ reorderedArgs) :: rest + def toMethodTypeList(tpe: Type): List[Type] = + tpe.resultType match + case res: MethodOrPoly => toMethodTypeList(res) :+ tpe + case res => List(tpe) def isSyntheticEvidence(name: String) = if !name.startsWith(NameKinds.ContextBoundParamName.separator) then false else symbol.paramSymss.flatten.find(_.name.show == name).exists(_.flags.is(Flags.Implicit)) - denot.info.stripPoly match - case tpe: (MethodType | AppliedType | TypeRef | TypeParamRef) => - val paramss = toParamss(tpe).map(_.filterNot(param => isSyntheticEvidence(param.name))) - val evidenceParams = (tpe.paramNamess.flatten zip tpe.paramInfoss.flatten).flatMap { - case (name, AppliedType(tpe, (ref: TypeParamRef) :: _)) if isSyntheticEvidence(name.show) => - Some(ref.paramName -> tpe) - case _ => None - } - - val typeParams = denot.info match - case poly: PolyType => - val tparams = poly.paramNames.zip(poly.paramInfos) - tparams.map { (name, info) => - evidenceParams.find((evidenceName: TypeName, _: Type) => name == evidenceName).flatMap { - case (_, tparam) => tparam.show.split('.').lastOption - } match { - case Some(evidenceTypeName) => s"${name.show}: ${evidenceTypeName}" - case None => name.show + info.show - } - } + def toTypeParam(tpe: PolyType): List[Param] = + val evidenceParams = (tpe.paramNamess.flatten zip tpe.paramInfoss.flatten).flatMap: + case (name, AppliedType(tpe, (ref: TypeParamRef) :: _)) if isSyntheticEvidence(name.show) => + Some(ref.paramName -> tpe) + case _ => None + + val tparams = tpe.paramNames.zip(tpe.paramInfos) + tparams.map: (name, info) => + evidenceParams.find((evidenceName: TypeName, _: Type) => name == evidenceName).flatMap: + case (_, tparam) => tparam.show.split('.').lastOption + match + case Some(evidenceTypeName) => TypeParam(s"${name.show}: ${evidenceTypeName}") + case None => TypeParam(name.show + info.show) + + def toParamss(tp: Type, fun: Option[untpd.GenericApply])(using Context): List[List[Param]] = + def reduceToParamss(applies: List[untpd.Tree], types: List[Type]): List[List[Param]] = + applies -> types match + case (Nil, Nil) => Nil + case ((_: untpd.TypeApply) :: restTrees, (poly: PolyType) :: restTypes) => + toTypeParam(poly) :: reduceToParamss(restTrees, restTypes) + case (restTrees, (poly: PolyType) :: restTypes) => + toTypeParam(poly) :: reduceToParamss(restTrees, restTypes) + case ((apply: untpd.GenericApply) :: other, tpe :: otherType) => + toParams(tpe, Some(apply)) :: reduceToParamss(other, otherType) + case (other, (tpe @ MethodTpe(names, _, _)) :: otherType) if !isDummyImplicit(tpe) => + toParams(tpe, None) :: reduceToParamss(other, otherType) + case _ => Nil + + def toParams(tp: Type, apply: Option[untpd.GenericApply])(using Context): List[Param] = + val currentParams = (tp.paramNamess, tp.paramInfoss) match + case (params :: _, infos :: _) => params zip infos case _ => Nil - val (name, returnType) = - if (symbol.isConstructor) then - (symbol.owner.name.show, None) - else - (denot.name.show, Some(tpe.finalResultType.widenTermRefExpr.show)) - Some(Signatures.Signature(name, typeParams, paramss, returnType, docComment.map(_.mainDoc), Some(denot))) - case other => None + + val params = currentParams.map: (name, info) => + Signatures.MethodParam( + name.show, + info.widenTermRefExpr.show, + docComment.flatMap(_.paramDoc(name)), + isImplicit = tp.isImplicitMethod, + ) + + val finalParams = apply.map: apply => + apply.args match + case Nil => params + case n if n.length > params.length => params + case _ => + // map argument order with their corresponding order so + // def foo(a: Int, b: Int, c: Int, d: Int) + // foo(b = 0, a = 0, d = 0, c = 0) order is List(1, 0, 3, 2) + // and if there are missing arguments, they are set to -1 so for the same method: + // foo(b= 0, d = 0, ) order is List(1, 3, -1, -1) + val argsWithOriginalIndices = apply.args.map: + case untpd.NamedArg(name, _) => + params.indexWhere(_.name == name.toString) + case param => -1 + .padTo(params.length, -1) + + var remainingParams = params.zipWithIndex.filter: (param, index) => + !argsWithOriginalIndices.contains(index) + .map(_._1) + + val result = argsWithOriginalIndices.map: + case -1 => + val h = remainingParams.head + remainingParams = remainingParams.tail + h + case n => params(n) + + val isReordered = params != result + + if isReordered then result.map(_.copy(isReordered = true)) + else params + + finalParams.getOrElse(params) + end toParams + + + + val applies = untpdFun.map(toApplyList).getOrElse(Nil) + val types = toMethodTypeList(tp).reverse + + reduceToParamss(applies, types) + + val paramss = toParamss(denot.info, untpdFun) + val (name, returnType) = + if (symbol.isConstructor) then + (symbol.owner.name.show, None) + else + denot.symbol.defTree match + // if there is an error in denotation type, we will fallback to source tree + case defn: tpd.DefDef if denot.info.isErroneous => (denot.name.show, Some(defn.tpt.show)) + case _ => (denot.name.show, Some(denot.info.finalResultType.widenTermRefExpr.show)) + Some(Signatures.Signature(name, paramss, returnType, docComment.map(_.mainDoc), Some(denot))) } /** @@ -514,14 +595,13 @@ object Signatures { */ private def toUnapplySignature(denot: SingleDenotation, paramNames: List[Name], paramTypes: List[Type])(using Context): Option[Signature] = val params = if paramNames.length == paramTypes.length then - (paramNames zip paramTypes).map((name, info) => Param(name.show, info.show)) + (paramNames zip paramTypes).map((name, info) => MethodParam(name.show, info.show)) else - paramTypes.map(info => Param("", info.show)) + // even if we only show types of arguments, they are still method params + paramTypes.map(info => MethodParam("", info.show)) - if params.nonEmpty then - Some(Signature("", Nil, List(params), None, None, Some(denot))) - else - None + if params.nonEmpty then Some(Signature("", List(params), None, None, Some(denot))) + else None /** * The number of parameters before `tree` application. It is necessary to properly show diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index 534fa4a9b995..3604e38375e7 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -935,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) diff --git a/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala b/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala index 5421e48ac5de..433b2665c4c1 100644 --- a/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala +++ b/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala @@ -4,13 +4,13 @@ import org.junit.Test import dotty.tools.languageserver.util.Code._ -import dotty.tools.dotc.util.Signatures.{Param => P, Signature => S} +import dotty.tools.dotc.util.Signatures.{TypeParam => TP, MethodParam => P, Signature => S} class SignatureHelpTest { @Test def fromJava: Unit = { val signature = - S("codePointAt", Nil, List(List(P("x$0", "Int"))), Some("Int")) + S("codePointAt", List(List(P("x$0", "Int"))), Some("Int")) code"""object O { "hello".codePointAt($m1) }""" @@ -18,7 +18,7 @@ class SignatureHelpTest { } @Test def errorTypeParameter: Unit = { - val emptySignature = S("empty", List("K", "V"), Nil, Some("Map[K, V]")) + val emptySignature = S("empty", List(List(TP("K"), TP("V"))), Some("Map[K, V]")) code"""object O: | Map.empty[WrongType, $m1] """ @@ -26,8 +26,8 @@ class SignatureHelpTest { } @Test def methodTypeParameter: Unit = { - val applySignature = S("apply", List("K", "V"), List(List(P("elems", "(K, V)*"))), Some("Map[K, V]")) - val emptySignature = S("empty", List("K", "V"), Nil, Some("Map[K, V]")) + val applySignature = S("apply", List(List(TP("K"), TP("V")), List(P("elems", "(K, V)*"))), Some("Map[K, V]")) + val emptySignature = S("empty", List(List(TP("K"), TP("V"))), Some("Map[K, V]")) code"""object O: | Map[$m1] | Map.empty[$m2] @@ -39,7 +39,7 @@ class SignatureHelpTest { } @Test def classTypeParameter: Unit = { - val signature = S("Test", List("K", "V"), Nil, Some("Test")) + val signature = S("Test", List(List(TP("K"), TP("V"))), Some("Test")) code"""object O: | class Test[K, V] {} | new Test[$m1] @@ -50,7 +50,7 @@ class SignatureHelpTest { } @Test def traitTypeParameter: Unit = { - val signature = S("Test", List("K", "V"), Nil, Some("Test")) + val signature = S("Test", List(List(TP("K"), TP("V"))), Some("Test")) code"""object O: | trait Test[K, V] {} | new Test[$m1] {} @@ -61,7 +61,7 @@ class SignatureHelpTest { } @Test def typeAliasTypeParameter: Unit = { - val signature = S("Test", List("K"), Nil, Some("Test")) + val signature = S("Test", List(List(TP("K"))), Some("Test")) code"""object O: | type Test[K] = List[K] | def test(x: Test[$m1]) @@ -70,7 +70,7 @@ class SignatureHelpTest { } @Test def typeParameterIndex: Unit = { - val mapSignature = S("map", List("B"), List(List(P("f", "Int => B"))), Some("List[B]")) + val mapSignature = S("map", List(List(TP("B")), List(P("f", "Int => B"))), Some("List[B]")) code"""object O { List(1, 2, 3).map[$m1]($m2) }""" @@ -79,7 +79,7 @@ class SignatureHelpTest { } @Test def partialyFailedCurriedFunctions: Unit = { - val listSignature = S("curry", Nil, List(List(P("a", "Int"), P("b", "Int")), List(P("c", "Int"))), Some("Int")) + val listSignature = S("curry", List(List(P("a", "Int"), P("b", "Int")), List(P("c", "Int"))), Some("Int")) code"""object O { |def curry(a: Int, b: Int)(c: Int) = a | curry(1$m1)$m2(3$m3) @@ -90,7 +90,7 @@ class SignatureHelpTest { } @Test def optionProperSignature: Unit = { - val signature = S("apply", List("A"), List(List(P("x", "A"))), Some("Option[A]")) + val signature = S("apply", List(List(TP("A")), List(P("x", "A"))), Some("Option[A]")) code"""object O { | Option(1, 2, 3, $m1) |}""" @@ -106,8 +106,8 @@ class SignatureHelpTest { } @Test def fromScala2: Unit = { - val applySig = S("apply", List("A"), List(List(P("elems", "A*"))), Some("List[A]")) - val mapSig = S("map", List("B"), List(List(P("f", "Int => B"))), Some("List[B]")) + val applySig = S("apply", List(List(TP("A")), List(P("elems", "A*"))), Some("List[A]")) + val mapSig = S("map", List(List(TP("B")), List(P("f", "Int => B"))), Some("List[B]")) code"""object O { List($m1) List(1, 2, 3).map($m2) @@ -117,7 +117,7 @@ class SignatureHelpTest { } @Test def typeParameterMethodApply: Unit = { - val testSig = S("method", Nil, List(List()), Some("Int")) + val testSig = S("method", List(List()), Some("Int")) code"""case class Foo[A](test: A) { | def method(): A = ??? |} @@ -140,7 +140,7 @@ class SignatureHelpTest { } @Test def unapplyCustomClass: Unit = { - val signature = S("", Nil, List(List(P("", "Int"))), None) + val signature = S("", List(List(P("", "Int"))), None) code"""class Nat(val x: Int): | def get: Int = x @@ -157,7 +157,7 @@ class SignatureHelpTest { } @Test def unapplyTypeClass: Unit = { - val signature = S("", Nil, List(List(P("", "Int"), P("", "String"))), None) + val signature = S("", List(List(P("", "Int"), P("", "String"))), None) code"""class Two[A, B](a: A, b: B) |object Two { @@ -175,9 +175,9 @@ class SignatureHelpTest { } @Test def nestedUnapplySignature: Unit = { - val signatureOneTwo = S("", Nil, List(List(P("a", "One"), P("b", "Two"))), None) - val signatureOne = S("", Nil, List(List(P("c", "Int"))), None) - val signatureTwo = S("", Nil, List(List(P("d", "Int"))), None) + val signatureOneTwo = S("", List(List(P("a", "One"), P("b", "Two"))), None) + val signatureOne = S("", List(List(P("c", "Int"))), None) + val signatureTwo = S("", List(List(P("d", "Int"))), None) code"""case class One(c: Int) |case class Two(d: Int) @@ -199,7 +199,7 @@ class SignatureHelpTest { } @Test def properParameterIndexTest: Unit = { - val signature = S("", Nil, List(List(P("a", "Int"), P("b", "String"))), None) + val signature = S("", List(List(P("a", "Int"), P("b", "String"))), None) code"""case class Two(a: Int, b: String) | |object Main { @@ -215,7 +215,7 @@ class SignatureHelpTest { } @Test def unapplyClass: Unit = { - val signature = S("", Nil, List(List(P("", "Int"), P("", "String"))), None) + val signature = S("", List(List(P("", "Int"), P("", "String"))), None) code"""class Two(a: Int, b: String) |object Two { @@ -233,7 +233,7 @@ class SignatureHelpTest { } @Test def productMatch: Unit = { - val signature = S("", Nil, List(List(P("", "Char"), P("", "Char"))), None) + val signature = S("", List(List(P("", "Char"), P("", "Char"))), None) code"""class FirstChars(s: String) extends Product: | def _1 = s.charAt(0) @@ -251,7 +251,7 @@ class SignatureHelpTest { } @Test def noUnapplySignatureWhenApplyingUnapply: Unit = { - val signature = S("unapply", List("A"), List(List(P("a", "A"))), Some("Some[(A, A)]")) + val signature = S("unapply", List(List(TP("A")), List(P("a", "A"))), Some("Some[(A, A)]")) code""" |object And { @@ -265,7 +265,7 @@ class SignatureHelpTest { } @Test def nestedOptionReturnedInUnapply: Unit = { - val signature = S("", Nil, List(List(P("", "Option[Int]"))), None) + val signature = S("", List(List(P("", "Option[Int]"))), None) code"""object OpenBrowserCommand { | def unapply(command: String): Option[Option[Int]] = { @@ -281,8 +281,8 @@ class SignatureHelpTest { } @Test def unknownTypeUnapply: Unit = { - val signature = S("", Nil, List(List(P("a", "A"), P("b", "B"))), None) - val signature2 = S("", Nil, List(List(P("a", "Int"), P("b", "Any"))), None) + val signature = S("", List(List(P("a", "A"), P("b", "B"))), None) + val signature2 = S("", List(List(P("a", "Int"), P("b", "Any"))), None) code"""case class Two[A, B](a: A, b: B) |object Main { @@ -301,8 +301,8 @@ class SignatureHelpTest { } @Test def sequenceMatchUnapply: Unit = { - val signatureSeq = S("", Nil, List(List(P("", "Seq[Int]"))), None) - val signatureVariadicExtractor = S("", Nil, List(List(P("", "Int"), P("","List[Int]"))), None) + val signatureSeq = S("", List(List(P("", "Seq[Int]"))), None) + val signatureVariadicExtractor = S("", List(List(P("", "Int"), P("","List[Int]"))), None) code"""case class Two[A, B](a: A, b: B) |object Main { @@ -316,7 +316,7 @@ class SignatureHelpTest { } @Test def productTypeClassMatch: Unit = { - val signature = S("", Nil, List(List(P("", "String"), P("", "String"))), None) + val signature = S("", List(List(P("", "String"), P("", "String"))), None) code"""class FirstChars[A](s: A) extends Product: | def _1 = s @@ -334,8 +334,8 @@ class SignatureHelpTest { } @Test def nameBasedMatch: Unit = { - val nameBasedMatch = S("", Nil, List(List(P("", "Int"), P("", "String"))), None) - val singleMatch = S("", Nil, List(List(P("", "ProdEmpty.type"))), None) + val nameBasedMatch = S("", List(List(P("", "Int"), P("", "String"))), None) + val singleMatch = S("", List(List(P("", "ProdEmpty.type"))), None) code"""object ProdEmpty: | def _1: Int = ??? @@ -356,8 +356,8 @@ class SignatureHelpTest { } @Test def nameBasedMatchWithWrongGet: Unit = { - val nameBasedMatch = S("", Nil, List(List(P("", "Int"))), None) - val singleMatch = S("", Nil, List(List(P("", "Int"))), None) + val nameBasedMatch = S("", List(List(P("", "Int"))), None) + val singleMatch = S("", List(List(P("", "Int"))), None) code"""object ProdEmpty: | def _1: Int = ??? @@ -378,7 +378,7 @@ class SignatureHelpTest { } @Test def nameBasedSingleMatchOrder: Unit = { - val signature = S("", Nil, List(List(P("", "String"))), None) + val signature = S("", List(List(P("", "String"))), None) code"""object ProdEmpty: | def _1: Int = 1 @@ -395,7 +395,7 @@ class SignatureHelpTest { } @Test def getObjectMatch: Unit = { - val signature = S("", Nil, List(List(P("", "String"))), None) + val signature = S("", List(List(P("", "String"))), None) code"""object ProdEmpty: | def isEmpty = true @@ -411,7 +411,7 @@ class SignatureHelpTest { } @Test def customSequenceMatch: Unit = { - val signature = S("", Nil, List(List(P("", "Seq[Char]"))), None) + val signature = S("", List(List(P("", "Seq[Char]"))), None) code"""object CharList: | def unapplySeq(s: String): Option[Seq[Char]] = Some(s.toList) @@ -429,7 +429,7 @@ class SignatureHelpTest { } @Test def productSequenceMatch: Unit = { - val signature = S("", Nil, List(List(P("", "String"), P("", "Seq[Int]"))), None) + val signature = S("", List(List(P("", "String"), P("", "Seq[Int]"))), None) code"""class Foo(val name: String, val children: Int*) |object Foo: @@ -449,7 +449,7 @@ class SignatureHelpTest { } @Test def productSequenceMatchForCaseClass: Unit = { - val signature = S("", Nil, List(List(P("name", "String"), P("children", "Seq[Int]"))), None) + val signature = S("", List(List(P("name", "String"), P("children", "Seq[Int]"))), None) code"""case class Foo(val name: String, val children: Int*) | @@ -466,7 +466,7 @@ class SignatureHelpTest { } @Test def unapplyManyType: Unit = { - val signature = S("", Nil, List(List(P("", "Int"), P("", "String"))), None) + val signature = S("", List(List(P("", "Int"), P("", "String"))), None) code""" |object Opt { @@ -483,7 +483,7 @@ class SignatureHelpTest { } @Test def unapplyTypeCaseClass: Unit = { - val signature = S("", Nil, List(List(P("a", "Int"), P("b", "String"))), None) + val signature = S("", List(List(P("a", "Int"), P("b", "String"))), None) code"""case class Two[A, B](a: A, b: B) | @@ -498,7 +498,7 @@ class SignatureHelpTest { } @Test def unapplyForTuple: Unit = { - val signature = S("", Nil, List(List(P("", "Int"), P("", "Int"))), None) + val signature = S("", List(List(P("", "Int"), P("", "Int"))), None) code"""object Main { | (1, 2) match | case (x${m1}, ${m2}) => @@ -508,7 +508,7 @@ class SignatureHelpTest { } @Test def unapplyCaseClass: Unit = { - val signature = S("", Nil, List(List(P("a", "Int"), P("b", "String"))), None) + val signature = S("", List(List(P("a", "Int"), P("b", "String"))), None) code"""case class Two(a: Int, b: String) | @@ -523,7 +523,7 @@ class SignatureHelpTest { } @Test def unapplyOption: Unit = { - val signature = S("", Nil, List(List(P("", "Int"))), None) + val signature = S("", List(List(P("", "Int"))), None) code"""|object Main { | Option(1) match { @@ -534,7 +534,7 @@ class SignatureHelpTest { } @Test def unapplyWithImplicits: Unit = { - val signature = S("", Nil, List(List(P("", "Int"))), None) + val signature = S("", List(List(P("", "Int"))), None) code"""| |object Opt: | def unapply[A](using String)(a: Option[A])(using Int) = a @@ -551,7 +551,7 @@ class SignatureHelpTest { } @Test def unapplyWithMultipleImplicits: Unit = { - val signature = S("", Nil, List(List(P("", "Int"))), None) + val signature = S("", List(List(P("", "Int"))), None) code"""| |object Opt: | def unapply[A](using String)(using Int)(a: Option[A]) = a @@ -568,15 +568,10 @@ class SignatureHelpTest { /** Implicit parameter lists consisting solely of DummyImplicits are hidden. */ @Test def hiddenDummyParams: Unit = { - val foo1Sig = - S("foo1", Nil, List(List(P("param0", "Int"))), Some("Int")) - val foo2Sig = - S("foo2", Nil, List(List(P("param0", "Int"))), Some("Int")) - val foo3Sig = - S("foo3", Nil, List(List(P("param0", "Int")), - List(P("dummy", "DummyImplicit"))), Some("Int")) - val foo4Sig = - S("foo4", Nil, List(List(P("param0", "Int")), + val foo1Sig = S("foo1", List(List(P("param0", "Int"))), Some("Int")) + val foo2Sig = S("foo2", List(List(P("param0", "Int"))), Some("Int")) + val foo3Sig = S("foo3", List(List(P("param0", "Int")), List(P("dummy", "DummyImplicit"))), Some("Int")) + val foo4Sig = S("foo4", List(List(P("param0", "Int")), List(P("x", "Int", isImplicit = true), P("dummy", "DummyImplicit", isImplicit = true))), Some("Int")) code"""object O { def foo1(param0: Int)(implicit dummy: DummyImplicit): Int = ??? @@ -596,7 +591,7 @@ class SignatureHelpTest { @Test def singleParam: Unit = { val signature = - S("foo", Nil, List(List(P("param0", "Int"))), Some("Int")) + S("foo", List(List(P("param0", "Int"))), Some("Int")) code"""object O { def foo(param0: Int): Int = ??? foo($m1) @@ -607,7 +602,7 @@ class SignatureHelpTest { } @Test def twoParams: Unit = { - val signature = S("foo", Nil, List(List(P("param0", "Int"), P("param1", "String"))), Some("Int")) + val signature = S("foo", List(List(P("param0", "Int"), P("param1", "String"))), Some("Int")) code"""object O { def foo(param0: Int, param1: String): Int = ??? foo($m1) @@ -620,8 +615,8 @@ class SignatureHelpTest { } @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")) + val sig0 = S("foo", List(List(P("param0", "Int"))), Some("Nothing")) + val sig1 = S("foo", List(List(P("param1", "String"))), Some("Nothing")) code"""object O { def foo(param0: Int): Nothing = ??? @@ -636,8 +631,8 @@ class SignatureHelpTest { } @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")) + val sig0 = S("foo", List(List(P("param0", "Int"), P("param1", "String"))), Some("Nothing")) + val sig1 = S("foo", 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 = ??? @@ -655,9 +650,9 @@ class SignatureHelpTest { } @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 sig0 = S("foo", List(List(P("param0", "Int"), P("param1", "Int"))), Some("Nothing")) + val sig1 = S("foo", List(List(P("param0", "Int"), P("param1", "Boolean"))), Some("Nothing")) + val sig2 = S("foo", 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 = ??? @@ -683,8 +678,8 @@ class SignatureHelpTest { } @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")) + val sig0 = S("foo", List(List(P("param0", "String")), List(P("param1", "String"))), Some("Nothing")) + val sig1 = S("foo", List(List(P("param0", "String"))), Some("Nothing")) code"""object O { def foo(param0: String)(param1: String): Nothing = ??? def foo(param0: String): Nothing = ??? @@ -700,7 +695,6 @@ class SignatureHelpTest { @Test def multipleParameterLists: Unit = { val signature = S("foo", - Nil, List( List(P("param0", "Int"), P("param1", "Int")), List(P("param2", "Int")), @@ -725,7 +719,6 @@ class SignatureHelpTest { @Test def implicitParams: Unit = { val signature = S("foo", - Nil, List( List(P("param0", "Int"), P("param1", "Int")), List(P("param2", "Int", isImplicit = true)) @@ -745,8 +738,8 @@ class SignatureHelpTest { @Test def typeParameters: Unit = { val signature = S("foo", - List("M[X]", "T[Z] <: M[Z]", "U >: T"), List( + List(TP("M[X]"), TP("T[Z] <: M[Z]"), TP("U >: T")), List(P("p0", "M[Int]"), P("p1", "T[Int]"), P("p2", "U")) ), Some("Int")) @@ -760,7 +753,6 @@ class SignatureHelpTest { @Test def constructorCall: Unit = { val signature = S("Foo", - Nil, List( List(P("x", "Int"), P("y", "String")), List(P("z", "String")) @@ -780,7 +772,6 @@ class SignatureHelpTest { @Test def overloadedConstructorCall: Unit = { val sig0 = S("Foo", - Nil, List( List(P("x", "Int"), P("y", "String")), List(P("z", "Int")) @@ -788,7 +779,6 @@ class SignatureHelpTest { None) val sig1 = S("Foo", - Nil, List( List(P("x", "Int"), P("y", "Int")) ), @@ -810,8 +800,8 @@ class SignatureHelpTest { @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")) + S("Foo", List(List(P("x", "Int", Some("An int")), P("y", "String", Some("A string")))), None, Some("A Foo")), + S("Foo", List(List(P("z", "Boolean", Some("A boolean")), P("foo", "Foo", Some("A Foo")))), None, Some("An alternative constructor for Foo")) ) code"""/** @@ -850,8 +840,8 @@ class SignatureHelpTest { @Test def classTypeParameters: Unit = { val signature = S("Foo", - List("M[X]", "T[Z] <: M[Z]", "U"), List( + List(TP("M[X]"), TP("T[Z] <: M[Z]"), TP("U")), List(P("p0", "M[Int]"), P("p1", "T[Int]"), P("p2", "U")), List(P("p3", "Int")) ), @@ -874,7 +864,7 @@ class SignatureHelpTest { /** Hello, world! */ def foo(param0: Int): Int = 0 foo($m1) }""" - .signatureHelp(m1, List(S("foo", Nil, List(List(P("param0", "Int"))), Some("Int"), Some("Hello, world!"))), None, 0) + .signatureHelp(m1, List(S("foo", List(List(P("param0", "Int"))), Some("Int"), Some("Hello, world!"))), None, 0) } @Test def showParamDoc: Unit = { @@ -890,7 +880,7 @@ class SignatureHelpTest { | buzz($m1) |}""" .signatureHelp(m1, List( - S("buzz", Nil, List(List( + S("buzz", 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")) @@ -899,9 +889,9 @@ class SignatureHelpTest { @Test def nestedApplySignatures: Unit = { val signatures = (1 to 5).map { i => - S(s"foo$i", Nil, List(List(P("x", "Int"))), Some("Int")) + S(s"foo$i", List(List(P("x", "Int"))), Some("Int")) } - val booSignature = S(s"boo", Nil, List(List(P("x", "Int"), P("y", "Int"))), Some("Int")) + val booSignature = S(s"boo", List(List(P("x", "Int"), P("y", "Int"))), Some("Int")) code"""|object O: | def foo1(x: Int): Int = ??? | def foo2(x: Int): Int = ??? @@ -920,8 +910,8 @@ class SignatureHelpTest { } @Test def multipleNestedApplySignatures: Unit = { - val simpleSignature = S(s"simpleFoo", Nil, List(List(P("x", "Int"))), Some("Int")) - val complicatedSignature = S(s"complicatedFoo", Nil, List(List(P("x", "Int"), P("y", "Int"), P("z", "Int"))), Some("Int")) + val simpleSignature = S(s"simpleFoo", List(List(P("x", "Int"))), Some("Int")) + val complicatedSignature = S(s"complicatedFoo", List(List(P("x", "Int"), P("y", "Int"), P("z", "Int"))), Some("Int")) code"""|object O: | def simpleFoo(x: Int): Int = ??? | def complicatedFoo(x: Int, y: Int, z: Int): Int = ??? @@ -947,7 +937,7 @@ class SignatureHelpTest { } @Test def noHelpSignatureWithPositionedOnName: Unit = { - val signature = S(s"foo", Nil, List(List(P("x", "Int"))), Some("Int")) + val signature = S(s"foo", List(List(P("x", "Int"))), Some("Int")) code"""|object O: | def foo(x: Int): Int = ??? | f${m1}oo(${m2})""" @@ -956,7 +946,7 @@ class SignatureHelpTest { } @Test def instantiatedTypeVarInOldExtensionMethods: Unit = { - val signature = S(s"test", Nil, List(List(P("x", "Int"))), Some("List[Int]")) + val signature = S(s"test", List(List(P("x", "Int"))), Some("List[Int]")) code"""|object O: | implicit class TypeVarTest[T](xs: List[T]): | def test(x: T): List[T] = ??? diff --git a/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala b/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala index 9044aefa2b3d..dadaa694800a 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala @@ -70,61 +70,70 @@ object SignatureHelpProvider: signature: Signatures.Signature, isJavaSymbol: Boolean ): Option[Signatures.Signature] = - val allParams = info.parameters().nn.asScala - def updateParams( - params: List[Signatures.Param], - index: Int - ): List[Signatures.Param] = + val methodParams = info.parameters().nn.asScala + val typeParams = info.typeParameters().nn.asScala + + def updateParams(params: List[Signatures.Param], typeParamIndex: Int, methodParamIndex: Int): List[Signatures.Param] = params match - case Nil => Nil - case head :: tail => - val rest = updateParams(tail, index + 1) - allParams.lift(index) match + case (head: Signatures.MethodParam) :: tail => + val rest = updateParams(tail, typeParamIndex, methodParamIndex + 1) + methodParams.lift(methodParamIndex) match case Some(paramDoc) => val newName = if isJavaSymbol && head.name.startsWith("x$") then paramDoc.nn.displayName() else head.name - head.copy( - doc = Some(paramDoc.docstring.nn), - name = newName.nn - ) :: rest + head.copy(name = newName.nn, doc = Some(paramDoc.docstring.nn)) :: rest case _ => head :: rest + case (head: Signatures.TypeParam) :: tail => + val rest = updateParams(tail, typeParamIndex + 1, methodParamIndex) + typeParams.lift(typeParamIndex) match + case Some(paramDoc) => + head.copy(doc = Some(paramDoc.docstring.nn)) :: rest + case _ => head :: rest + case _ => Nil def updateParamss( params: List[List[Signatures.Param]], - index: Int + typeParamIndex: Int, + methodParamIndex: Int ): List[List[Signatures.Param]] = params match case Nil => Nil case head :: tail => - val updated = updateParams(head, index) - updated :: updateParamss(tail, index + head.size) - val updatedParams = updateParamss(signature.paramss, 0) + val updated = updateParams(head, typeParamIndex, methodParamIndex) + val (nextTypeParamIndex, nextMethodParamIndex) = head match + case (_: Signatures.MethodParam) :: _ => (typeParamIndex, methodParamIndex + head.size) + case (_: Signatures.TypeParam) :: _ => (typeParamIndex + head.size, methodParamIndex) + case _ => (typeParamIndex, methodParamIndex) + updated :: updateParamss(tail, nextTypeParamIndex, nextMethodParamIndex) + val updatedParams = updateParamss(signature.paramss, 0, 0) Some(signature.copy(doc = Some(info.docstring().nn), paramss = updatedParams)) end withDocumentation private def signatureToSignatureInformation( signature: Signatures.Signature ): l.SignatureInformation = - val tparams = signature.tparams.map(Signatures.Param("", _)) - val paramInfoss = - (tparams ::: signature.paramss.flatten).map(paramToParameterInformation) + val paramInfoss = (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 then "" - 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(markupContent) val sig = new l.SignatureInformation(label) sig.setParameters(paramInfoss.asJava) diff --git a/presentation-compiler/test/dotty/tools/pc/base/BaseSignatureHelpSuite.scala b/presentation-compiler/test/dotty/tools/pc/base/BaseSignatureHelpSuite.scala index 0022be40a630..0cf4fa0dd4e7 100644 --- a/presentation-compiler/test/dotty/tools/pc/base/BaseSignatureHelpSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/base/BaseSignatureHelpSuite.scala @@ -38,10 +38,7 @@ abstract class BaseSignatureHelpSuite extends BasePCSuite: out .append(signature.getLabel) .append("\n") - if ( - result.getActiveSignature == i && result.getActiveParameter != null && signature.getParameters - .size() > 0 - ) { + if (result.getActiveSignature == i && result.getActiveParameter != null && signature.getParameters.size() > 0) { val param = signature.getParameters.get(result.getActiveParameter) val label = param.getLabel.getLeft() /* We need to find the label of the active parameter and show ^ at that spot diff --git a/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionDocSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionDocSuite.scala index c419ce3946d9..bcfd399c7c4a 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionDocSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionDocSuite.scala @@ -16,14 +16,14 @@ class CompletionDocSuite extends BaseCompletionSuite: MockDocumentation("java/lang/String#join().", "join", Seq(), Seq("delimiter", "elements")), MockDocumentation("java/lang/String#substring().", "substring", Seq(), Seq("beginIndex")), MockDocumentation("java/lang/String#substring(+1).", "substring", Seq(), Seq("beginIndex", "endIndex")), - ScalaMockDocumentation("scala/collection/Iterator#sliding().", "sliding", List(MockParam("size"), MockParam("step", "1"))), - ScalaMockDocumentation("scala/collection/immutable/TreeMap#insert().", "insert", List(MockParam("key"), MockParam("value"))), + ScalaMockDocumentation("scala/collection/Iterator#sliding().", "sliding", List(), List(MockParam("size"), MockParam("step", "1"))), + ScalaMockDocumentation("scala/collection/immutable/TreeMap#insert().", "insert", List(), List(MockParam("key"), MockParam("value"))), ScalaMockDocumentation("scala/Option#isDefined().", "isDefined"), ScalaMockDocumentation("scala/util/DynamicVariable#", "DynamicVariable"), ScalaMockDocumentation("scala/util/DynamicVariable.", "DynamicVariable"), - ScalaMockDocumentation("scala/io/Source#reportWarning().", "reportWarning", List(MockParam("pos"), MockParam("msg"), MockParam("out", "Console.out"))), + ScalaMockDocumentation("scala/io/Source#reportWarning().", "reportWarning", List(), List(MockParam("pos"), MockParam("msg"), MockParam("out", "Console.out"))), ScalaMockDocumentation("scala/Predef.println().", "println"), - ScalaMockDocumentation("scala/Predef.println(+1).", "println", List(MockParam("x"))), + ScalaMockDocumentation("scala/Predef.println(+1).", "println", List(), List(MockParam("x"))), ScalaMockDocumentation("scala/Predef.", "Predef"), ScalaMockDocumentation("scala/runtime/stdLibPatches/Predef.", "Predef"), ScalaMockDocumentation("scala/util/control/Exception.Catch#", "Catch"), diff --git a/presentation-compiler/test/dotty/tools/pc/tests/hover/HoverDocSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/hover/HoverDocSuite.scala index 06c629467166..fc9b6835e319 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/hover/HoverDocSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/hover/HoverDocSuite.scala @@ -10,10 +10,10 @@ class HoverDocSuite extends BaseHoverSuite: override protected def mockEntries: MockEntries = new MockEntries: override def documentations: Set[SymbolDocumentation] = Set( - ScalaMockDocumentation("java/lang/String#substring().", "substring", List(MockParam("beginIndex"))), + ScalaMockDocumentation("java/lang/String#substring().", "substring", List(), List(MockParam("beginIndex"))), ScalaMockDocumentation("java/util/Collections#emptyList().", "emptyList"), - ScalaMockDocumentation("_empty_/Alpha.apply().", "apply", List(MockParam("x"))), - ScalaMockDocumentation("_empty_/Alpha#", "init", List(MockParam("x"))), + ScalaMockDocumentation("_empty_/Alpha.apply().", "apply", List(), List(MockParam("x"))), + ScalaMockDocumentation("_empty_/Alpha#", "init", List(), List(MockParam("x"))), ScalaMockDocumentation("scala/collection/LinearSeqOps#headOption().", "headOption"), ) diff --git a/presentation-compiler/test/dotty/tools/pc/tests/hover/HoverNamedArgSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/hover/HoverNamedArgSuite.scala index f92ba9933be4..182f8e1e0644 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/hover/HoverNamedArgSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/hover/HoverNamedArgSuite.scala @@ -11,7 +11,7 @@ class HoverNamedArgSuite extends BaseHoverSuite: override protected def mockEntries: MockEntries = new MockEntries: override def documentations: Set[SymbolDocumentation] = Set( - ScalaMockDocumentation("a/b.foo().(named)", "foo", List(MockParam("named"))) + ScalaMockDocumentation("a/b.foo().(named)", "foo", List(), List(MockParam("named"))) ) @Test def `named` = diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpDocSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpDocSuite.scala index 7d36dc6ce23d..3636afa8d1a9 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpDocSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpDocSuite.scala @@ -26,13 +26,13 @@ class SignatureHelpDocSuite extends BaseSignatureHelpSuite: MockDocumentation("java/io/File#``(+1).", "", Seq(), Seq("parent", "child")), MockDocumentation("java/io/File#``(+2).", "", Seq(), Seq("parent", "child")), MockDocumentation("java/io/File#``(+3).", "", Seq(), Seq("uri")), - ScalaMockDocumentation("java/util/Collections#singleton().", "singleton", List(MockParam("o"))), + ScalaMockDocumentation("java/util/Collections#singleton().", "singleton", List(MockParam("T")), List(MockParam("o"))), ScalaMockDocumentation("scala/Some#", "Some"), - ScalaMockDocumentation("scala/Option#fold().", "fold", List(MockParam("ifEmpty"), MockParam("f"))), - ScalaMockDocumentation("scala/Option.apply().", "apply", List(MockParam("x"))), - ScalaMockDocumentation("scala/collection/immutable/List#map().", "map", List(MockParam("f"))), - ScalaMockDocumentation("scala/collection/LinearSeqOps#foldLeft().", "foldLeft", List(MockParam("z"), MockParam("op"))), - ScalaMockDocumentation("scala/util/control/Exception.Catch#", "Catch", List(MockParam("pf"), MockParam("fin"), MockParam("rethrow"))) + ScalaMockDocumentation("scala/Option#fold().", "fold", List(MockParam("B")), List(MockParam("ifEmpty"), MockParam("f"))), + ScalaMockDocumentation("scala/Option.apply().", "apply", List(), List(MockParam("x"))), + ScalaMockDocumentation("scala/collection/immutable/List#map().", "map", List(MockParam("B")), List(MockParam("f"))), + ScalaMockDocumentation("scala/collection/LinearSeqOps#foldLeft().", "foldLeft", List(MockParam("B")), List(MockParam("z"), MockParam("op"))), + ScalaMockDocumentation("scala/util/control/Exception.Catch#", "Catch", List(), List(MockParam("pf"), MockParam("fin"), MockParam("rethrow"))) ) @Test def `curry` = @@ -45,6 +45,7 @@ class SignatureHelpDocSuite extends BaseSignatureHelpSuite: s"""Found documentation for scala/Option#fold(). |fold[B](ifEmpty: => B)(f: Int => B): B | ^^^^^^^^^^^ + | @param B Found documentation for type param B | @param ifEmpty Found documentation for param ifEmpty | @param f Found documentation for param f """.stripMargin @@ -60,6 +61,7 @@ class SignatureHelpDocSuite extends BaseSignatureHelpSuite: s"""|Found documentation for scala/Option#fold(). |fold[B](ifEmpty: => B)(f: Int => B): B | ^^^^^^^^^^^^^ + | @param B Found documentation for type param B | @param ifEmpty Found documentation for param ifEmpty | @param f Found documentation for param f |""".stripMargin @@ -77,6 +79,7 @@ class SignatureHelpDocSuite extends BaseSignatureHelpSuite: """|Found documentation for scala/collection/LinearSeqOps#foldLeft(). |foldLeft[B](z: B)(op: (B, Int) => B): B | ^^^^^^^^^^^^^^^^^ + | @param B Found documentation for type param B | @param z Found documentation for param z | @param op Found documentation for param op |""".stripMargin @@ -106,6 +109,7 @@ class SignatureHelpDocSuite extends BaseSignatureHelpSuite: """|Found documentation for scala/collection/immutable/List#map(). |map[B](f: Int => B): List[B] | ^^^^^^^^^^^ + | @param B Found documentation for type param B | @param f Found documentation for param f |""".stripMargin ) @@ -134,6 +138,7 @@ class SignatureHelpDocSuite extends BaseSignatureHelpSuite: """|Found documentation for java/util/Collections#singleton(). |singleton[T](o: T): java.util.Set[T] | ^^^^ + | @param T Found documentation for type param T | @param o Found documentation for param o |""".stripMargin ) diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpInterleavingSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpInterleavingSuite.scala new file mode 100644 index 000000000000..098a2549c720 --- /dev/null +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpInterleavingSuite.scala @@ -0,0 +1,401 @@ +package dotty.tools.pc.tests.signaturehelp + +import dotty.tools.pc.base.BaseSignatureHelpSuite + +import org.junit.Test +import java.nio.file.Path + +class SignatureHelpInterleavingSuite extends BaseSignatureHelpSuite: + + override protected def scalacOptions(classpath: Seq[Path]): Seq[String] = + List("-language:experimental.clauseInterleaving") + + @Test def `proper-position-1` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair[@@Int](1)[String]("1") + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^ + |""".stripMargin + ) + + + @Test def `proper-position-2` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair[Int](@@1)[String]("1") + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^^^^ + |""".stripMargin + ) + + @Test def `proper-position-3` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair[Int](1)[@@String]("1") + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^ + |""".stripMargin + ) + + @Test def `proper-position-4` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair[Int](1)[String](@@"1") + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^^^^ + |""".stripMargin + ) + + @Test def `not-fully-applied-1` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair[@@Int] + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^ + |""".stripMargin + ) + + + @Test def `not-fully-applied-2` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair[Int](@@1) + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^^^^ + |""".stripMargin + ) + + @Test def `not-fully-applied-3` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair[Int](1)[@@String] + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^ + |""".stripMargin + ) + + @Test def `error` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair[Int][@@String] + """.stripMargin, + "" + ) + + @Test def `inferred-type-param-1` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair(1@@) + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^^^^ + |""".stripMargin + ) + + @Test def `inferred-type-param-2` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair(1)[String@@] + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^ + |""".stripMargin + ) + + @Test def `inferred-type-param-3` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair(1)[String]("1"@@) + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^^^^ + |""".stripMargin + ) + + @Test def `inferred-type-param-4` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair(1)("1"@@) + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^^^^ + |""".stripMargin + ) + + // Bug in compiler: TypeApply fun has incorrect span for clause interleaving + // @Test def `empty-current-param-1` = + // check( + // """ + // |object Test: + // | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + // | pair[@@] + // """.stripMargin, + // """ + // |pair[A](a: A)[B](b: B): (A, B) + // | ^ + // |""".stripMargin + // ) + + @Test def `empty-current-param-check` = + check( + """ + |object Test: + | def pair[A](a: A): A = ??? + | pair[@@] + """.stripMargin, + """ + |pair[A](a: A): A + | ^ + |""".stripMargin + ) + + @Test def `empty-current-param-2` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair[Int](@@) + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^^^^ + |""".stripMargin + ) + + @Test def `empty-current-param-3` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair[Int](1)[@@] + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^ + |""".stripMargin + ) + + @Test def `empty-current-param-4` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair[Int](1)[String](@@) + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^^^^ + |""".stripMargin + ) + + @Test def `empty-current-inferred-1` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair(@@) + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^^^^ + |""".stripMargin + ) + + @Test def `empty-current-inferred-2` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair(1)(@@) + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^^^^ + |""".stripMargin + ) + + @Test def `empty-previous-1` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair[](@@) + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^^^^ + |""".stripMargin + ) + + @Test def `empty-previous-2` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair[]()[@@] + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^ + |""".stripMargin + ) + + @Test def `empty-previous-3` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair[]()[](@@) + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^^^^ + |""".stripMargin + ) + + @Test def `empty-previous-4` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair[]()[](11@@) + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^^^^ + |""".stripMargin + ) + + @Test def `empty-previous-5` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair[](5@@1) + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^^^^ + |""".stripMargin + ) + + @Test def `empty-previous-6` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair[String]()[@@] + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^ + |""".stripMargin + ) + + @Test def `empty-previous-7` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair[String]()[Int](@@) + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^^^^ + |""".stripMargin + ) + + @Test def `error-previous-1` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair[String](52)[@@] + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^ + |""".stripMargin + ) + + @Test def `error-previous-2` = + check( + """ + |object Test: + | def pair[A](a: A)[B](b: B): (A, B) = (a, b) + | pair[String](52)[Int](""@@) + """.stripMargin, + """ + |pair[A](a: A)[B](b: B): (A, B) + | ^^^^ + |""".stripMargin + ) + + @Test def `complex-1` = + check( + """ + |object Test: + | def 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 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] + | ^^^^^^^^^^ + |""".stripMargin + ) + + @Test def `complex-2` = + check( + """ + |object Test: + | def 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 1)(List(1, 2, 3))((1, 2))[@@] + """.stripMargin, + """ + |foo[A](using a: A)(b: List[A])[C <: a.type, D](cd: (C, D))[E]: Foo[A, B, C, D, E] + | ^ + |""".stripMargin + ) diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala index 165b0f325dff..5a68ecdba3e7 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala @@ -168,7 +168,7 @@ class SignatureHelpNamedArgsSuite extends BaseSignatureHelpSuite: | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? | method(paramC = 3, @@ ,paramA = 1) |""".stripMargin, - """|method([paramC: Int], [paramB: Int], [paramD: Int], [paramA: Int]): Unit + """|method([paramC: Int], [paramB: Int], [paramA: Int], [paramD: Int]): Unit | ^^^^^^^^^^^^^ |""".stripMargin ) @@ -256,8 +256,8 @@ class SignatureHelpNamedArgsSuite extends BaseSignatureHelpSuite: | def test[T, F](aaa: Int, bbb: T, ccc: F): T = ??? | val x = test(1, ccc = 2, b@@) |""".stripMargin, - """|test[T, F](aaa: Int, [ccc: F], [bbb: T]): T - | ^^^^^^^^ + """|test[T, F]([aaa: Int], [ccc: F], [bbb: T]): T + | ^^^^^^^^ |""".stripMargin ) @@ -281,8 +281,8 @@ class SignatureHelpNamedArgsSuite extends BaseSignatureHelpSuite: | def test2(aaa: Int, bbb: Int, ccc: Int): Int = ??? | val x = test2(aaa = 2@@, ccc = 1) |""".stripMargin, - """|test2(aaa: Int, bbb: Int, ccc: Int): Int - | ^^^^^^^^ + """|test2([aaa: Int], [ccc: Int], [bbb: Int]): Int + | ^^^^^^^^^^ |""".stripMargin ) @@ -293,8 +293,8 @@ class SignatureHelpNamedArgsSuite extends BaseSignatureHelpSuite: | val x = test2(aaa = 2, @@ ccc = 1, bbb = 3) | |""".stripMargin, - """|test2(aaa: Int, [ccc: Int], [bbb: Int]): Int - | ^^^^^^^^^^ + """|test2([aaa: Int], [ccc: Int], [bbb: Int]): Int + | ^^^^^^^^^^ |""".stripMargin ) diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala index 8c4e647d91f2..daa86885ef1a 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala @@ -388,8 +388,8 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | | """.stripMargin, - """|apply(viewId: String, nodeUri: String, label: String, command: String, icon: String, tooltip: String, collapseState: String): TreeViewNode - | ^^^^^^^^^^^^^^ + """|apply([viewId: String], [nodeUri: String], [label: String], [collapseState: String], [command: String], [icon: String], [tooltip: String]): TreeViewNode + | ^^^^^^^^^^^^^^^^ |""".stripMargin ) @@ -424,8 +424,8 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | | """.stripMargin, - """|apply(viewId: String, nodeUri: String, label: String, command: String, collapseState: String): TreeViewNode - | ^^^^^^^^^^^^^^ + """|apply([viewId: String], [nodeUri: String], [label: String], [collapseState: String], [command: String]): TreeViewNode + | ^^^^^^^^^^^^^^^^ |""".stripMargin ) @@ -1152,3 +1152,105 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | ^^^^^^ |""".stripMargin ) + + @Test def `curried-help-works-in-select` = + check( + """|object Main: + | def test(xxx: Int, yyy: Int)(zzz: Int): Int = ??? + | test(yyy = 5, xxx = 7)(@@) + |""".stripMargin, + """|test([yyy: Int], [xxx: Int])(zzz: Int): Int + | ^^^^^^^^ + |""".stripMargin + ) + + @Test def `no-signature-help-for-parameterless-method` = + check( + """|object Main: + | def test: Int = ??? + | test(@@) + |""".stripMargin, + "" + ) + + @Test def `show-methods-returning-tuples` = + check( + """|object Main: + | def test(): (Int, Int) = ??? + | test(@@) + |""".stripMargin, + "test(): (Int, Int)" + ) + + @Test def `show-methods-returning-tuples-2` = + check( + """|object Main: + | def test(x: Int): (Int, Int) = ??? + | test(@@) + |""".stripMargin, + """|test(x: Int): (Int, Int) + | ^^^^^^ + |""".stripMargin + ) + + @Test def `dont-show-tuples-application` = + check( + """|object Main: + | (1, @@) + |""".stripMargin, + "" + ) + + // Improvement would be to create synthetic signature help showing + // add(x: Int)(y: Int): Int + @Test def `dont-show-functionN` = + check( + """|object Main: + | val add = (x: Int) => (y: Int) => x + y + | add(@@) + |""".stripMargin, + "" + ) + + @Test def `dont-show-functionN-2` = + check( + """|object Main: + | val add = (x: Int) => (y: Int) => x + y + | add(1, @@) + |""".stripMargin, + "" + ) + + @Test def `type-param-start` = + check( + """|object Main: + | def test[A](x: A): A = ??? + | test[@@] + |""".stripMargin, + """|test[A](x: A): A + | ^ + |""".stripMargin + ) + + @Test def `error-recovery-1` = + check( + """|object Main: + | def test[A](x: A): Foo[A] = ??? + | test[@@] + |""".stripMargin, + """|test[A](x: A): Foo[A] + | ^ + |""".stripMargin + ) + + @Test def `error-recovery-2` = + check( + """|object Main: + | def test[A](x: A): Foo[A] = ??? + | test[Int](@@) + |""".stripMargin, + """|test[A](x: A): Foo[A] + | ^^^^ + |""".stripMargin + ) + diff --git a/presentation-compiler/test/dotty/tools/pc/utils/MockEntries.scala b/presentation-compiler/test/dotty/tools/pc/utils/MockEntries.scala index 05cd2cb8c124..1c0c35c0c71e 100644 --- a/presentation-compiler/test/dotty/tools/pc/utils/MockEntries.scala +++ b/presentation-compiler/test/dotty/tools/pc/utils/MockEntries.scala @@ -28,6 +28,7 @@ abstract class MockEntries: def apply( symbol: String, displayName: String, + typeParams: List[MockParam] = Nil, params: List[MockParam] = Nil ): SymbolDocumentation = ScalaSymbolDocumentation( @@ -35,7 +36,14 @@ abstract class MockEntries: displayName, s"Found documentation for $symbol", "", - Nil.asJava, + typeParams + .map: param => + ScalaSymbolDocumentation( + param.name, + param.name, + s"Found documentation for type param ${param.name}\n", + ) + .asJava, params .map(param => ScalaSymbolDocumentation( @@ -43,8 +51,6 @@ abstract class MockEntries: param.name, s"Found documentation for param ${param.name}\n", param.defaultValue, - Nil.asJava, - Nil.asJava ) ) .asJava From 0a3181d5674cd5db4e5d3808673982a6f31d6b5f Mon Sep 17 00:00:00 2001 From: rochala Date: Tue, 9 Jan 2024 10:46:59 +0100 Subject: [PATCH 09/14] Don't mark non reordered params as reordered --- compiler/src/dotty/tools/dotc/util/Signatures.scala | 6 ++++-- .../signaturehelp/SignatureHelpNamedArgsSuite.scala | 12 ++++++------ .../pc/tests/signaturehelp/SignatureHelpSuite.scala | 8 ++++---- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/util/Signatures.scala b/compiler/src/dotty/tools/dotc/util/Signatures.scala index 72ed4e93af1e..7765288dcd10 100644 --- a/compiler/src/dotty/tools/dotc/util/Signatures.scala +++ b/compiler/src/dotty/tools/dotc/util/Signatures.scala @@ -554,9 +554,11 @@ object Signatures { case n => params(n) val isReordered = params != result + val (ordered, reordered) = params.zip(result).span: (definitionMember, reorderedMember) => + definitionMember == reorderedMember + + ordered.map(_._2) ++ reordered.map(_._2.copy(isReordered = isReordered)) - if isReordered then result.map(_.copy(isReordered = true)) - else params finalParams.getOrElse(params) end toParams diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala index 5a68ecdba3e7..9fd564d9d967 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala @@ -256,8 +256,8 @@ class SignatureHelpNamedArgsSuite extends BaseSignatureHelpSuite: | def test[T, F](aaa: Int, bbb: T, ccc: F): T = ??? | val x = test(1, ccc = 2, b@@) |""".stripMargin, - """|test[T, F]([aaa: Int], [ccc: F], [bbb: T]): T - | ^^^^^^^^ + """|test[T, F](aaa: Int, [ccc: F], [bbb: T]): T + | ^^^^^^^^ |""".stripMargin ) @@ -281,8 +281,8 @@ class SignatureHelpNamedArgsSuite extends BaseSignatureHelpSuite: | def test2(aaa: Int, bbb: Int, ccc: Int): Int = ??? | val x = test2(aaa = 2@@, ccc = 1) |""".stripMargin, - """|test2([aaa: Int], [ccc: Int], [bbb: Int]): Int - | ^^^^^^^^^^ + """|test2(aaa: Int, [ccc: Int], [bbb: Int]): Int + | ^^^^^^^^ |""".stripMargin ) @@ -293,8 +293,8 @@ class SignatureHelpNamedArgsSuite extends BaseSignatureHelpSuite: | val x = test2(aaa = 2, @@ ccc = 1, bbb = 3) | |""".stripMargin, - """|test2([aaa: Int], [ccc: Int], [bbb: Int]): Int - | ^^^^^^^^^^ + """|test2(aaa: Int, [ccc: Int], [bbb: Int]): Int + | ^^^^^^^^^^ |""".stripMargin ) diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala index daa86885ef1a..9fb750c46101 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala @@ -388,8 +388,8 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | | """.stripMargin, - """|apply([viewId: String], [nodeUri: String], [label: String], [collapseState: String], [command: String], [icon: String], [tooltip: String]): TreeViewNode - | ^^^^^^^^^^^^^^^^ + """|apply(viewId: String, nodeUri: String, label: String, [collapseState: String], [command: String], [icon: String], [tooltip: String]): TreeViewNode + | ^^^^^^^^^^^^^^ |""".stripMargin ) @@ -424,8 +424,8 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | | """.stripMargin, - """|apply([viewId: String], [nodeUri: String], [label: String], [collapseState: String], [command: String]): TreeViewNode - | ^^^^^^^^^^^^^^^^ + """|apply(viewId: String, nodeUri: String, label: String, [collapseState: String], [command: String]): TreeViewNode + | ^^^^^^^^^^^^^^ |""".stripMargin ) From 018244fe54e260019b573748e34351a28e6539b3 Mon Sep 17 00:00:00 2001 From: rochala Date: Tue, 9 Jan 2024 14:56:44 +0100 Subject: [PATCH 10/14] Add support for shortened type printer in Signature Help --- .../tools/pc/SignatureHelpProvider.scala | 16 ++++++-- .../signaturehelp/SignatureHelpDocSuite.scala | 10 ++--- .../SignatureHelpPatternSuite.scala | 18 +++++++++ .../signaturehelp/SignatureHelpSuite.scala | 39 ++++++++++++------- 4 files changed, 60 insertions(+), 23 deletions(-) diff --git a/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala b/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala index dadaa694800a..91243559e4c8 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala @@ -13,11 +13,14 @@ import dotty.tools.dotc.util.Signatures import dotty.tools.dotc.util.SourceFile import dotty.tools.dotc.util.Spans import dotty.tools.dotc.util.Spans.Span +import dotty.tools.pc.printer.ShortenedTypePrinter +import dotty.tools.pc.printer.ShortenedTypePrinter.IncludeDefaultParam import dotty.tools.pc.utils.MtagsEnrichments.* import org.eclipse.lsp4j as l import scala.jdk.CollectionConverters.* import scala.jdk.OptionConverters.* +import scala.meta.internal.metals.ReportContext import scala.meta.pc.OffsetParams import scala.meta.pc.SymbolDocumentation import scala.meta.pc.SymbolSearch @@ -28,7 +31,7 @@ object SignatureHelpProvider: driver: InteractiveDriver, params: OffsetParams, search: SymbolSearch - ) = + )(using ReportContext): l.SignatureHelp = val uri = params.uri().nn val text = params.text().nn val sourceFile = SourceFile.virtual(uri, text) @@ -36,10 +39,17 @@ object SignatureHelpProvider: driver.compilationUnits.get(uri) match case Some(unit) => - given newCtx: Context = driver.currentCtx.fresh.setCompilationUnit(unit) val pos = driver.sourcePosition(params, isZeroExtent = false) - val path = Interactive.pathTo(unit.tpdTree, pos.span) + val path = Interactive.pathTo(unit.tpdTree, pos.span)(using driver.currentCtx) + + val localizedContext = Interactive.contextOfPath(path)(using driver.currentCtx) + val indexedContext = IndexedContext(driver.currentCtx) + + given Context = localizedContext.fresh + .setCompilationUnit(unit) + .setPrinterFn(_ => ShortenedTypePrinter(search, IncludeDefaultParam.Include)(using indexedContext)) + val (paramN, callableN, alternatives) = Signatures.signatureHelp(path, pos.span) val infos = alternatives.flatMap: signature => diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpDocSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpDocSuite.scala index 3636afa8d1a9..09b22559d35c 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpDocSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpDocSuite.scala @@ -151,8 +151,8 @@ class SignatureHelpDocSuite extends BaseSignatureHelpSuite: |} """.stripMargin, """|Found documentation for scala/util/control/Exception.Catch# - |Catch[T](pf: scala.util.control.Exception.Catcher[T], fin: Option[scala.util.control.Exception.Finally], rethrow: Throwable => Boolean) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + |Catch[T](pf: Catcher[T], fin: Option[Finally], rethrow: Throwable => Boolean) + | ^^^^^^^^^^^^^^ | @param pf Found documentation for param pf | @param fin Found documentation for param fin | @param rethrow Found documentation for param rethrow @@ -166,9 +166,9 @@ class SignatureHelpDocSuite extends BaseSignatureHelpSuite: | new java.io.File(@@) |} """.stripMargin, - """|File(uri: java.net.URI) - | ^^^^^^^^^^^^^^^^^ - |File(parent: java.io.File, child: String) + """|File(uri: URI) + | ^^^^^^^^ + |File(parent: File, child: String) |File(parent: String, child: String) |File(pathname: String) |""".stripMargin diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpPatternSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpPatternSuite.scala index 5aae9da1664f..d01145510367 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpPatternSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpPatternSuite.scala @@ -238,6 +238,24 @@ class SignatureHelpPatternSuite extends BaseSignatureHelpSuite: |""".stripMargin ) + @Test def `shortened` = + check( + """ + |object Test { + | def unapply(command: java.io.File): Option[java.io.File] = { + | Some(Some(1)) + | } + | + | "" match { + | case Test(@@) => + | } + |} + """.stripMargin, + """|(File) + | ^^^^ + |""".stripMargin + ) + @Test def `pat-negative` = check( """ diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala index 9fb750c46101..11b61e9368c2 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala @@ -66,7 +66,7 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: """|Random() |Random(seed: Int) |Random(seed: Long) - |Random(self: java.util.Random) + |Random(self: Random) |""".stripMargin ) @@ -103,9 +103,9 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | new File(@@) |} """.stripMargin, - """|File(x$0: java.net.URI) - | ^^^^^^^^^^^^^^^^^ - |File(x$0: java.io.File, x$1: String) + """|File(x$0: URI) + | ^^^^^^^^ + |File(x$0: File, x$1: String) |File(x$0: String, x$1: String) |File(x$0: String) |""".stripMargin @@ -118,9 +118,9 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | new java.io.File(@@) |} """.stripMargin, - """|File(x$0: java.net.URI) - | ^^^^^^^^^^^^^^^^^ - |File(x$0: java.io.File, x$1: String) + """|File(x$0: URI) + | ^^^^^^^^ + |File(x$0: File, x$1: String) |File(x$0: String, x$1: String) |File(x$0: String) |""".stripMargin @@ -323,9 +323,9 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | } yield k |} """.stripMargin, - """|to(end: Int): scala.collection.immutable.Range.Inclusive + """|to(end: Int): Inclusive | ^^^^^^^^ - |to(end: Int, step: Int): scala.collection.immutable.Range.Inclusive + |to(end: Int, step: Int): Inclusive |""".stripMargin, stableOrder = false ) @@ -502,9 +502,8 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | new scala.util.control.Exception.Catch(@@) |} """.stripMargin, - // TODO short names are not supported yet - """|Catch[T](pf: scala.util.control.Exception.Catcher[T], fin: Option[scala.util.control.Exception.Finally], rethrow: Throwable => Boolean) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + """|Catch[T](pf: Catcher[T], fin: Option[Finally], rethrow: Throwable => Boolean) + | ^^^^^^^^^^^^^^ |""".stripMargin ) @@ -515,7 +514,7 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | new java.util.HashMap[String, Int]().computeIfAbsent(@@) |} """.stripMargin, - // TODO short names are not supported yet + // This is the correct result, as there is a conflict at Function: scala.Function and java.util.function.Function """|computeIfAbsent(x$0: String, x$1: java.util.function.Function[? >: String, ? <: Int]): Int | ^^^^^^^^^^^ |""".stripMargin @@ -1136,8 +1135,8 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | def test(a: Foo.Test, b: Foo.Test): Int = ??? | test(Foo.Test(1), @@) |""".stripMargin, - """|test(a: Main.Foo.Test, b: Main.Foo.Test): Int - | ^^^^^^^^^^^^^^^^ + """|test(a: Test, b: Test): Int + | ^^^^^^^ |""".stripMargin ) @@ -1254,3 +1253,13 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: |""".stripMargin ) + @Test def `type-param-shortening` = + check( + """|object M: + | def test[T <: java.io.File](x: Int): Int = ??? + | test(@@) + |""".stripMargin, + """|test[T <: File](x: Int): Int + | ^^^^^^ + |""".stripMargin + ) From fa584adc4dd32c228a33d2660284025e5e54e581 Mon Sep 17 00:00:00 2001 From: rochala Date: Tue, 9 Jan 2024 17:07:34 +0100 Subject: [PATCH 11/14] Don't print synthetic param names --- .../dotty/tools/dotc/util/Signatures.scala | 26 +++++++------- .../pc/printer/ShortenedTypePrinter.scala | 2 +- .../signaturehelp/SignatureHelpSuite.scala | 35 +++++++++++++++++++ 3 files changed, 50 insertions(+), 13 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/util/Signatures.scala b/compiler/src/dotty/tools/dotc/util/Signatures.scala index 7765288dcd10..9af8463ee2f3 100644 --- a/compiler/src/dotty/tools/dotc/util/Signatures.scala +++ b/compiler/src/dotty/tools/dotc/util/Signatures.scala @@ -500,25 +500,29 @@ object Signatures { case None => TypeParam(name.show + info.show) def toParamss(tp: Type, fun: Option[untpd.GenericApply])(using Context): List[List[Param]] = - def reduceToParamss(applies: List[untpd.Tree], types: List[Type]): List[List[Param]] = + val paramSymss = symbol.paramSymss + + def reduceToParamss(applies: List[untpd.Tree], types: List[Type], paramList: Int = 0): List[List[Param]] = applies -> types match - case (Nil, Nil) => Nil case ((_: untpd.TypeApply) :: restTrees, (poly: PolyType) :: restTypes) => - toTypeParam(poly) :: reduceToParamss(restTrees, restTypes) + toTypeParam(poly) :: reduceToParamss(restTrees, restTypes, paramList + 1) case (restTrees, (poly: PolyType) :: restTypes) => - toTypeParam(poly) :: reduceToParamss(restTrees, restTypes) + toTypeParam(poly) :: reduceToParamss(restTrees, restTypes, paramList + 1) case ((apply: untpd.GenericApply) :: other, tpe :: otherType) => - toParams(tpe, Some(apply)) :: reduceToParamss(other, otherType) + toParams(tpe, Some(apply), paramList) :: reduceToParamss(other, otherType, paramList + 1) case (other, (tpe @ MethodTpe(names, _, _)) :: otherType) if !isDummyImplicit(tpe) => - toParams(tpe, None) :: reduceToParamss(other, otherType) + toParams(tpe, None, paramList) :: reduceToParamss(other, otherType, paramList + 1) case _ => Nil - def toParams(tp: Type, apply: Option[untpd.GenericApply])(using Context): List[Param] = - val currentParams = (tp.paramNamess, tp.paramInfoss) match - case (params :: _, infos :: _) => params zip infos + def toParams(tp: Type, apply: Option[untpd.GenericApply], paramList: Int)(using Context): List[Param] = + val currentParams = (paramSymss.lift(paramList), tp.paramInfoss.headOption) match + case (Some(params), Some(infos)) => params zip infos case _ => Nil - val params = currentParams.map: (name, info) => + val params = currentParams.map: (symbol, info) => + // TODO after we migrate ShortenedTypePrinter into the compiler, it should rely on its api + val name = if symbol.isAllOf(Flags.SyntheticParam | Flags.Given) then nme.EMPTY else symbol.name.asTermName + Signatures.MethodParam( name.show, info.widenTermRefExpr.show, @@ -563,8 +567,6 @@ object Signatures { finalParams.getOrElse(params) end toParams - - val applies = untpdFun.map(toApplyList).getOrElse(Nil) val types = toMethodTypeList(tp).reverse diff --git a/presentation-compiler/src/main/dotty/tools/pc/printer/ShortenedTypePrinter.scala b/presentation-compiler/src/main/dotty/tools/pc/printer/ShortenedTypePrinter.scala index 9c255d20d212..bfaf4e2e6e79 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/printer/ShortenedTypePrinter.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/printer/ShortenedTypePrinter.scala @@ -492,7 +492,7 @@ class ShortenedTypePrinter( case head :: Nil => s": $head" case many => many.mkString(": ", ": ", "") s"$keywordName$paramTypeString$bounds" - else if param.is(Flags.Given) && param.name.toString.contains('$') then + else if param.isAllOf(Flags.SyntheticParam | Flags.Given) then // For Anonymous Context Parameters // print only type string // e.g. "using Ord[T]" instead of "using x$0: Ord[T]" diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala index 11b61e9368c2..00860b922516 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala @@ -1263,3 +1263,38 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | ^^^^^^ |""".stripMargin ) + + @Test def `implicit-param-without-ident` = + check( + """|object M: + | trait Context + | def test(x: Int)(using Context): Int = ??? + | test(@@) + |""".stripMargin, + """|test(x: Int)(using Context): Int + | ^^^^^^ + |""".stripMargin + ) + + @Test def `implicit-param` = + check( + """|object M: + | trait Context + | def test(x: Int)(using ctx: Context): Int = ??? + | test(@@) + |""".stripMargin, + """|test(x: Int)(using ctx: Context): Int + | ^^^^^^ + |""".stripMargin + ) + + @Test def `context-param` = + check( + """|object M: + | def test(x: Int, y: Int = 7)(z: Int ?=> Int): Int = ??? + | test(@@) + |""".stripMargin, + """|test(x: Int, y: Int)(z: (Int) ?=> Int): Int + | ^^^^^^ + |""".stripMargin + ) From 676a594de10ae0642647156b57aeb50b247fe94f Mon Sep 17 00:00:00 2001 From: rochala Date: Thu, 11 Jan 2024 10:55:02 +0100 Subject: [PATCH 12/14] Select correct parameter when using is missing --- .../dotty/tools/dotc/util/Signatures.scala | 5 +- .../pc/printer/ShortenedTypePrinter.scala | 2 +- .../SignatureHelpInterleavingSuite.scala | 131 ++++++++++++++++++ .../signaturehelp/SignatureHelpSuite.scala | 80 +++++++++++ 4 files changed, 216 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/util/Signatures.scala b/compiler/src/dotty/tools/dotc/util/Signatures.scala index 9af8463ee2f3..4fe877faf373 100644 --- a/compiler/src/dotty/tools/dotc/util/Signatures.scala +++ b/compiler/src/dotty/tools/dotc/util/Signatures.scala @@ -521,7 +521,7 @@ object Signatures { val params = currentParams.map: (symbol, info) => // TODO after we migrate ShortenedTypePrinter into the compiler, it should rely on its api - val name = if symbol.isAllOf(Flags.SyntheticParam | Flags.Given) then nme.EMPTY else symbol.name.asTermName + val name = if symbol.isAllOf(Flags.Given | Flags.Param) && symbol.name.startsWith("x$") then nme.EMPTY else symbol.name.asTermName Signatures.MethodParam( name.show, @@ -622,6 +622,9 @@ object Signatures { */ private def findParamssIndex(tree: tpd.Tree, alreadyCurried: Int = 0)(using Context): Int = tree match + case GenericApply(fun, params) + if params.nonEmpty && params.forall(_.isInstanceOf[untpd.SearchFailureIdent]) => + findParamssIndex(fun, alreadyCurried) case GenericApply(fun, params) => findParamssIndex(fun, alreadyCurried + 1) case _ => alreadyCurried diff --git a/presentation-compiler/src/main/dotty/tools/pc/printer/ShortenedTypePrinter.scala b/presentation-compiler/src/main/dotty/tools/pc/printer/ShortenedTypePrinter.scala index bfaf4e2e6e79..1ab58f74272e 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/printer/ShortenedTypePrinter.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/printer/ShortenedTypePrinter.scala @@ -492,7 +492,7 @@ class ShortenedTypePrinter( case head :: Nil => s": $head" case many => many.mkString(": ", ": ", "") s"$keywordName$paramTypeString$bounds" - else if param.isAllOf(Flags.SyntheticParam | Flags.Given) then + else if param.isAllOf(Given | Param) && param.name.startsWith("x$") then // For Anonymous Context Parameters // print only type string // e.g. "using Ord[T]" instead of "using x$0: Ord[T]" diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpInterleavingSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpInterleavingSuite.scala index 098a2549c720..9d4ec1d995d0 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpInterleavingSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpInterleavingSuite.scala @@ -3,6 +3,7 @@ package dotty.tools.pc.tests.signaturehelp import dotty.tools.pc.base.BaseSignatureHelpSuite import org.junit.Test +import org.junit.Ignore import java.nio.file.Path class SignatureHelpInterleavingSuite extends BaseSignatureHelpSuite: @@ -399,3 +400,133 @@ class SignatureHelpInterleavingSuite extends BaseSignatureHelpSuite: | ^ |""".stripMargin ) + + @Ignore("""Clause interleaving is still experimental. It lifts this tree into series of anonymous functions, which all have the same span. + It requires further investigation to determine whether this is a bug in the compiler.""") + @Test def `clause-interleaving-empty` = + check( + """|object M: + | def test[X](x: X)[Y](y: Y): (X, Y)= ??? + | test[@@] + |""".stripMargin, + """|test[X](x: X)[Y](y: Y): (X, Y) + | ^ + |""".stripMargin + ) + + @Ignore("""Clause interleaving is still experimental. It lifts this tree into series of anonymous functions, which all have the same span. + It requires further investigation to determine whether this is a bug in the compiler.""") + @Test def `more-interleaved-params-1` = + check( + """|object M: + | def test[A](a: A)[B](b: B)[C](c: C)[D](d: D): Int = ??? + | test[@@] + |""".stripMargin, + """|test[A](a: A)[B](b: B)[C](c: C)[D](d: D): Int + | ^ + |""".stripMargin + ) + + @Test def `more-interleaved-params-2` = + check( + """|object M: + | 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 + | ^^^^ + |""".stripMargin + ) + + @Ignore("""Clause interleaving is still experimental. It lifts this tree into series of anonymous functions, which all have the same span. + It requires further investigation to determine whether this is a bug in the compiler.""") + @Test def `more-interleaved-params-3` = + check( + """|object M: + | def test[A](a: A)[B](b: B)[C](c: C)[D](d: D): Int = ??? + | test[Int](1)[@@] + |""".stripMargin, + """|test[A](a: A)[B](b: B)[C](c: C)[D](d: D): Int + | ^ + |""".stripMargin + ) + + @Test def `more-interleaved-params-4` = + check( + """|object M: + | 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 + | ^^^^ + |""".stripMargin + ) + + @Ignore("""Clause interleaving is still experimental. It lifts this tree into series of anonymous functions, which all have the same span. + It requires further investigation to determine whether this is a bug in the compiler.""") + @Test def `more-interleaved-params-5` = + check( + """|object M: + | def test[A](a: A)[B](b: B)[C](c: C)[D](d: D): Int = ??? + | test[Int](1)[String]("1")[@@] + |""".stripMargin, + """|test[A](a: A)[B](b: B)[C](c: C)[D](d: D): Int + | ^ + |""".stripMargin + ) + + @Test def `more-interleaved-params-6` = + check( + """|object M: + | 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 + | ^^^^ + |""".stripMargin + ) + + @Test def `more-interleaved-params-7` = + check( + """|object M: + | def test[A](a: A)[B](b: B)[C](c: C)[D](d: D): Int = ??? + | test[Int](1)[String]("1")[Int](2)[@@] + |""".stripMargin, + """|test[A](a: A)[B](b: B)[C](c: C)[D](d: D): Int + | ^ + |""".stripMargin + ) + + @Test def `more-interleaved-params-8` = + check( + """|object M: + | 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 + | ^^^^ + |""".stripMargin + ) + + @Test def `interleaving-with-implicit` = + check( + """|object M: + | 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 + | ^^^^ + |""".stripMargin + ) + + @Test def `interleaving-with-implicit-recovery` = + check( + """|object M: + | 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 + | ^^^^ + |""".stripMargin + ) + diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala index 00860b922516..87b429f36b30 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala @@ -1298,3 +1298,83 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | ^^^^^^ |""".stripMargin ) + + @Test def `empty-implicit-params` = + check( + """|object M: + | def test(x: Int)(using String): Int = ??? + | test(1)(@@) + |""".stripMargin, + """|test(x: Int)(using String): Int + | ^^^^^^ + |""".stripMargin + ) + + @Test def `multiple-implicits-1` = + check( + """|object M: + | def a(using Int)(using String): Int = ??? + | a(@@) + |""".stripMargin, + """|a(using Int)(using String): Int + | ^^^ + |""".stripMargin + ) + + + @Test def `multiple-implicits-2` = + check( + """|object M: + | def a(using Int)(using String): Int = ??? + | a(using 5)(@@) + |""".stripMargin, + """|a(using Int)(using String): Int + | ^^^^^^ + |""".stripMargin + ) + + @Test def `multiple-implicits-3` = + check( + """|object M: + | def a(using Int)(using String)(x: Int): Int = ??? + | a(@@) + |""".stripMargin, + """|a(using Int)(using String)(x: Int): Int + | ^^^ + |""".stripMargin + ) + + @Test def `multiple-implicits-4` = + check( + """|object M: + | def a(using Int)(using String)(x: Int): Int = ??? + | a(using 5)(@@) + |""".stripMargin, + """|a(using Int)(using String)(x: Int): Int + | ^^^^^^ + |""".stripMargin + ) + + @Test def `multiple-implicits-error-1` = + check( + """|object M: + | def a(using Int)(using String)(x: Int): Int = ??? + | a(5)(@@) + |""".stripMargin, + """| + |a(using Int)(using String)(x: Int): Int + | ^^^^^^ + |""".stripMargin + ) + + @Test def `multiple-implicits-error-2` = + check( + """|object M: + | def a(using Int)(using String)(x: Int): Int = ??? + | a(5)(@@) + |""".stripMargin, + """| + |a(using Int)(using String)(x: Int): Int + | ^^^^^^ + |""".stripMargin + ) From 6647c71d58526f84a4469032d73fd6337f9ca56e Mon Sep 17 00:00:00 2001 From: rochala Date: Thu, 11 Jan 2024 15:52:35 +0100 Subject: [PATCH 13/14] Remove unnecessary added vals --- compiler/src/dotty/tools/dotc/reporting/messages.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 267859d62f11..b6622d67f36a 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -581,7 +581,7 @@ extends SyntaxMsg(UnboundPlaceholderParameterID) { |""" } -class IllegalStartSimpleExpr(val illegalToken: String)(using Context) +class IllegalStartSimpleExpr(illegalToken: String)(using Context) extends SyntaxMsg(IllegalStartSimpleExprID) { def msg(using Context) = i"expression expected but ${Red(illegalToken)} found" def explain(using Context) = { @@ -1171,7 +1171,7 @@ extends ReferenceMsg(ForwardReferenceExtendsOverDefinitionID) { |""" } -class ExpectedTokenButFound(val expected: Token, found: Token, prefix: String = "")(using Context) +class ExpectedTokenButFound(expected: Token, found: Token, prefix: String = "")(using Context) extends SyntaxMsg(ExpectedTokenButFoundID) { private def foundText = Tokens.showToken(found) From 329a2ebbb5483987e54b300acdffaf05436812ff Mon Sep 17 00:00:00 2001 From: rochala Date: Fri, 19 Jan 2024 14:11:07 +0100 Subject: [PATCH 14/14] Apply review changes --- .../dotty/tools/dotc/util/Signatures.scala | 9 +++---- .../tools/pc/SignatureHelpProvider.scala | 2 +- .../pc/base/BaseSignatureHelpSuite.scala | 9 +++++-- .../SignatureHelpInterleavingSuite.scala | 14 ----------- .../SignatureHelpNamedArgsSuite.scala | 2 +- .../signaturehelp/SignatureHelpSuite.scala | 25 ++++++++----------- 6 files changed, 22 insertions(+), 39 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/util/Signatures.scala b/compiler/src/dotty/tools/dotc/util/Signatures.scala index 4fe877faf373..36d1723b9c06 100644 --- a/compiler/src/dotty/tools/dotc/util/Signatures.scala +++ b/compiler/src/dotty/tools/dotc/util/Signatures.scala @@ -608,14 +608,11 @@ object Signatures { else None /** - * The number of parameters before `tree` application. It is necessary to properly show - * parameter number for erroneous applications before current one. - * - * This handles currying, so for an application such as `foo(1, 2)(3)`, the result of - * `countParams` should be 1. + * The number of params lists before `tree` application. + * It handles currying, so for an application such as `foo(1, 2)(3)`, the result of + * `findParamssIndex` should be 1. * * @param tree The tree to inspect. - * @param denot Denotation of function we are trying to apply * @param alreadyCurried Number of subsequent Apply trees before current tree * * @return The index of paramss we are currently in. diff --git a/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala b/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala index 91243559e4c8..f7797efbfb27 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/SignatureHelpProvider.scala @@ -48,7 +48,7 @@ object SignatureHelpProvider: given Context = localizedContext.fresh .setCompilationUnit(unit) - .setPrinterFn(_ => ShortenedTypePrinter(search, IncludeDefaultParam.Include)(using indexedContext)) + .setPrinterFn(_ => ShortenedTypePrinter(search, IncludeDefaultParam.Never)(using indexedContext)) val (paramN, callableN, alternatives) = Signatures.signatureHelp(path, pos.span) diff --git a/presentation-compiler/test/dotty/tools/pc/base/BaseSignatureHelpSuite.scala b/presentation-compiler/test/dotty/tools/pc/base/BaseSignatureHelpSuite.scala index 0cf4fa0dd4e7..ca647502fabf 100644 --- a/presentation-compiler/test/dotty/tools/pc/base/BaseSignatureHelpSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/base/BaseSignatureHelpSuite.scala @@ -1,10 +1,11 @@ package dotty.tools.pc.base -import java.nio.file.Paths +import org.eclipse.lsp4j.SignatureHelp +import java.nio.file.Paths import scala.jdk.CollectionConverters.* -import scala.meta.internal.metals.CompilerOffsetParams import scala.language.unsafeNulls +import scala.meta.internal.metals.CompilerOffsetParams abstract class BaseSignatureHelpSuite extends BasePCSuite: def checkDoc( @@ -27,6 +28,10 @@ abstract class BaseSignatureHelpSuite extends BasePCSuite: ) .get() val out = new StringBuilder() + + // this is default SignatureHelp value which should only be returned on crash + assert(result != new SignatureHelp()) + if (result != null) { result.getSignatures.asScala.zipWithIndex.foreach { case (signature, i) => if (includeDocs) { diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpInterleavingSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpInterleavingSuite.scala index 9d4ec1d995d0..15546d086033 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpInterleavingSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpInterleavingSuite.scala @@ -166,20 +166,6 @@ class SignatureHelpInterleavingSuite extends BaseSignatureHelpSuite: |""".stripMargin ) - // Bug in compiler: TypeApply fun has incorrect span for clause interleaving - // @Test def `empty-current-param-1` = - // check( - // """ - // |object Test: - // | def pair[A](a: A)[B](b: B): (A, B) = (a, b) - // | pair[@@] - // """.stripMargin, - // """ - // |pair[A](a: A)[B](b: B): (A, B) - // | ^ - // |""".stripMargin - // ) - @Test def `empty-current-param-check` = check( """ diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala index 9fd564d9d967..1906e12a0254 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpNamedArgsSuite.scala @@ -22,7 +22,7 @@ class SignatureHelpNamedArgsSuite extends BaseSignatureHelpSuite: check( """|object O: | def method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit = ??? - | method(paramA = 1, @@ + | method(paramA = 1, @@) |""".stripMargin, """|method(paramA: Int, paramB: Int, paramC: Int, paramD: Int): Unit | ^^^^^^^^^^^ diff --git a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala index 87b429f36b30..896cbdb3cad2 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/signaturehelp/SignatureHelpSuite.scala @@ -1038,7 +1038,7 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: "" ) - @Test def `show-directly-when-unclosed` = + @Test def `dont-show-directly-when-unclosed` = check( """|object Main { | def test(a: Int, b: Int): Int = ??? @@ -1264,18 +1264,6 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: |""".stripMargin ) - @Test def `implicit-param-without-ident` = - check( - """|object M: - | trait Context - | def test(x: Int)(using Context): Int = ??? - | test(@@) - |""".stripMargin, - """|test(x: Int)(using Context): Int - | ^^^^^^ - |""".stripMargin - ) - @Test def `implicit-param` = check( """|object M: @@ -1373,8 +1361,15 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite: | def a(using Int)(using String)(x: Int): Int = ??? | a(5)(@@) |""".stripMargin, - """| - |a(using Int)(using String)(x: Int): Int + """|a(using Int)(using String)(x: Int): Int | ^^^^^^ |""".stripMargin ) + + @Test def `dont-crash-at-last-position` = + check( + """|object M: + | def test(x: Int): Int = ??? + | test(@@""".stripMargin, + "" + )