bugfix: Document highlight on named arguments

For methods with multiple parameter lists and default args, the tree looks like this: `Block(List(val x&1, val x&2, ...), Apply(<<method>>, List(x&1, x&2, ...)))`, so we need to extract this `Apply`. Also, if rhs of named argument is complex, the position is often incorrect, so we need to use `include` for `namedArgCache`
scalameta · Oct 26, 2023 · 5742c56 · 5742c56
1 parent 45ac965
commit 5742c56
Show file tree

Hide file tree

Showing 2 changed files with 134 additions and 51 deletions.
diff --git a/mtags/src/main/scala-2/scala/meta/internal/pc/PcCollector.scala b/mtags/src/main/scala-2/scala/meta/internal/pc/PcCollector.scala
@@ -95,7 +95,7 @@ abstract class PcCollector[T](
   private lazy val namedArgCache = {
     val parsedTree = parseTree(unit.source)
     parsedTree.collect { case arg @ AssignOrNamedArg(_, rhs) =>
-      rhs.pos.start -> arg
+      rhs.pos -> arg
     }.toMap
   }
   def fallbackSymbol(name: Name, pos: Position): Option[Symbol] = {
@@ -122,48 +122,7 @@ abstract class PcCollector[T](
       else {
         Some(symbolAlternatives(id.symbol), id.pos)
       }
-    /* named argument, which is a bit complex:
-     * foo(nam@@e = "123")
-     */
-    case (apply: Apply) =>
-      apply.args
-        .flatMap { arg =>
-          namedArgCache.get(arg.pos.start)
-        }
-        .collectFirst {
-          case AssignOrNamedArg(id: Ident, _) if id.pos.includes(pos) =>
-            apply.symbol.paramss.flatten.find(_.name == id.name).map { s =>
-              // if it's a case class we need to look for parameters also
-              if (caseClassSynthetics(s.owner.name) && s.owner.isSynthetic) {
-                val applyOwner = s.owner.owner
-                val constructorOwner =
-                  if (applyOwner.isCaseClass) applyOwner
-                  else
-                    applyOwner.companion match {
-                      case NoSymbol =>
-                        applyOwner.localCompanion(pos).getOrElse(NoSymbol)
-                      case comp => comp
-                    }
-                val info = constructorOwner.info
-                val constructorParams = info.members
-                  .filter(_.isConstructor)
-                  .flatMap(_.paramss)
-                  .flatten
-                  .filter(_.name == id.name)
-                  .toSet
-                (
-                  (constructorParams ++ Set(
-                    s,
-                    info.member(s.getterName),
-                    info.member(s.setterName),
-                    info.member(s.localName)
-                  )).filter(_ != NoSymbol),
-                  id.pos
-                )
-              } else (Set(s), id.pos)
-            }
-        }
-        .flatten
+
     /* all definitions:
      * def fo@@o = ???
      * class Fo@@o = ???
@@ -198,8 +157,69 @@ abstract class PcCollector[T](
             )
           )
         )
+    /* named argument, which is a bit complex:
+     * foo(nam@@e = "123")
+     */
     case _ =>
-      None
+      val apply = typedTree match {
+        case (apply: Apply) => Some(apply)
+        /**
+         * For methods with multiple parameter lists and default args, the tree looks like this:
+         * Block(List(val x&1, val x&2, ...), Apply(<<method>>, List(x&1, x&2, ...)))
+         */
+        case _ =>
+          typedTree.children.collectFirst {
+            case Block(_, apply: Apply) if apply.pos.includes(pos) =>
+              apply
+          }
+      }
+      apply
+        .collect {
+          case apply if apply.symbol != null =>
+            collectArguments(apply)
+              .flatMap(arg => namedArgCache.find(_._1.includes(arg.pos)))
+              .collectFirst {
+                case (_, AssignOrNamedArg(id: Ident, _))
+                    if id.pos.includes(pos) =>
+                  apply.symbol.paramss.flatten.find(_.name == id.name).map {
+                    s =>
+                      // if it's a case class we need to look for parameters also
+                      if (
+                        caseClassSynthetics(s.owner.name) && s.owner.isSynthetic
+                      ) {
+                        val applyOwner = s.owner.owner
+                        val constructorOwner =
+                          if (applyOwner.isCaseClass) applyOwner
+                          else
+                            applyOwner.companion match {
+                              case NoSymbol =>
+                                applyOwner
+                                  .localCompanion(pos)
+                                  .getOrElse(NoSymbol)
+                              case comp => comp
+                            }
+                        val info = constructorOwner.info
+                        val constructorParams = info.members
+                          .filter(_.isConstructor)
+                          .flatMap(_.paramss)
+                          .flatten
+                          .filter(_.name == id.name)
+                          .toSet
+                        (
+                          (constructorParams ++ Set(
+                            s,
+                            info.member(s.getterName),
+                            info.member(s.setterName),
+                            info.member(s.localName)
+                          )).filter(_ != NoSymbol),
+                          id.pos
+                        )
+                      } else (Set(s), id.pos)
+                  }
+              }
+              .flatten
+        }
+        .getOrElse(None)
   }
 
   def result(): List[T] = {
@@ -244,8 +264,9 @@ abstract class PcCollector[T](
               (soughtOrOverride(df.symbol) ||
               isForComprehensionOwner(df))
             case appl: Apply =>
-              owners(appl.symbol) ||
-              symbolAlternatives(appl.symbol.owner).exists(owners(_))
+              appl.symbol != null &&
+              (owners(appl.symbol) ||
+                symbolAlternatives(appl.symbol.owner).exists(owners(_)))
             case imp: Import =>
               owners(imp.expr.symbol) && imp.selectors
                 .exists(sel => soughtNames(sel.name))
@@ -346,18 +367,20 @@ abstract class PcCollector[T](
          * etc.
          */
         case appl: Apply if filter(appl) =>
-          val named = appl.args
+          val named = collectArguments(appl)
             .flatMap { arg =>
-              namedArgCache.get(arg.pos.start)
+              namedArgCache.find(_._1.includes(arg.pos))
             }
             .collect {
-              case AssignOrNamedArg(i @ Ident(name), _)
+              case (_, AssignOrNamedArg(i @ Ident(name), _))
                   if soughtFilter(_.decodedName == name.decoded) =>
+                val symbol = Option(appl.symbol).flatMap(
+                  _.paramss.flatten.find(_.name == i.name)
+                )
                 collect(
                   i,
                   i.pos,
-                  appl.symbol.paramss.flatten
-                    .find(_.name == i.name)
+                  symbol
                 )
             }
           tree.children.foldLeft(acc ++ named)(traverse(_, _))
@@ -491,6 +514,13 @@ abstract class PcCollector[T](
     }
   }
 
+  private def collectArguments(apply: Apply): List[Tree] = {
+    apply.fun match {
+      case appl: Apply => collectArguments(appl) ++ apply.args
+      case _ => apply.args
+    }
+  }
+
   private val forCompMethods =
     Set(nme.map, nme.flatMap, nme.withFilter, nme.foreach)
 

diff --git a/tests/cross/src/test/scala/tests/highlight/DocumentHighlightSuite.scala b/tests/cross/src/test/scala/tests/highlight/DocumentHighlightSuite.scala
@@ -79,6 +79,59 @@ class DocumentHighlightSuite extends BaseDocumentHighlightSuite {
       |}""".stripMargin,
   )
 
+  check(
+    "named-args2",
+    """
+      |object Main {
+      |  def foo = check("abc")(
+      |    directory = "foo",
+      |    <<file@@name>> = 1,
+      |  )
+      |
+      |  private def check(name: String)(
+      |    directory: String,
+      |    <<filename>>: Int,
+      |    otherDefault: String = "default"
+      |  ): Unit = ()
+      |}""".stripMargin,
+  )
+
+  check(
+    "named-args2",
+    """
+      |object Main {
+      |  def foo = check("abc")(
+      |    directory = "foo",
+      |    <<file@@name>> = 1,
+      |  )
+      |
+      |  private def check(name: String)(
+      |    directory: String,
+      |    <<filename>>: Int,
+      |    otherDefault: String = "default"
+      |  ): Unit = ()
+      |}""".stripMargin,
+  )
+
+  check(
+    "named-args3",
+    """
+      |package example
+      |
+      |object Main {
+      |  check("abc")(
+      |    directory = Some("foo"),
+      |    <<filename>> = 1
+      |  )(loc = true)
+      |
+      |  private def check(name: String)(
+      |      directory: Option[String],
+      |      <<fil@@ename>>: Int,
+      |      otherDefault: String = "default"
+      |  )(loc: Boolean): Unit = ()
+      |}""".stripMargin,
+  )
+
   check(
     "scopes",
     """