Improve signature help by more stable position calculation + better named arg support

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.

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)

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,30 +60,12 @@ 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]) =
     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.
    *
@@ -204,21 +190,73 @@ 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.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
+              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 =
+        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(currentParamsIndex + 1).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)
 
+  /** 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
+      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.
    *
@@ -235,7 +273,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 +434,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 +458,27 @@ 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 (remainingNamedUnordered, remaining) = params
+        .diff(reorderedParamsBeforeCursor).diff(ordered)
+        .partition(p => namedParamsAfterCursor.contains(p.name))
 
-      (params :: rest)
+      val remainingNamed = remainingNamedUnordered.sortBy(p => namedParamsAfterCursor.indexOf(p.name))
+      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 +514,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 +552,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 +578,7 @@ object Signatures {
         case msg: NoMatchingOverload => msg.alternatives
         case _                       => Nil
 
-    // If the user writes `foo(bar, <cursor>)`, the typer will insert a synthetic
-    // `null` parameter: `foo(bar, null)`. This may influence what's the "best"
-    // 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.

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()
 
   }
 

language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala

@@ -974,4 +974,13 @@ class SignatureHelpTest {
      .signatureHelp(m2, List(signature), None, 0)
   }
 
+  @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)
+  }
+
 }