From 5742c56bf7cd77317e4f26523c01ceb8de328eb2 Mon Sep 17 00:00:00 2001 From: Jakub Ciesluk <323892@uwr.edu.pl> Date: Tue, 24 Oct 2023 12:33:41 +0200 Subject: [PATCH] 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(<>, 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` --- .../scala/meta/internal/pc/PcCollector.scala | 132 +++++++++++------- .../highlight/DocumentHighlightSuite.scala | 53 +++++++ 2 files changed, 134 insertions(+), 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 index 829a30c4399..46fc888864b 100644 --- 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(<>, 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 index 3c6bc30c025..f5aed112ae6 100644 --- 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", + | <> = 1, + | ) + | + | private def check(name: String)( + | directory: String, + | <>: Int, + | otherDefault: String = "default" + | ): Unit = () + |}""".stripMargin, + ) + + check( + "named-args2", + """ + |object Main { + | def foo = check("abc")( + | directory = "foo", + | <> = 1, + | ) + | + | private def check(name: String)( + | directory: String, + | <>: Int, + | otherDefault: String = "default" + | ): Unit = () + |}""".stripMargin, + ) + + check( + "named-args3", + """ + |package example + | + |object Main { + | check("abc")( + | directory = Some("foo"), + | <> = 1 + | )(loc = true) + | + | private def check(name: String)( + | directory: Option[String], + | <>: Int, + | otherDefault: String = "default" + | )(loc: Boolean): Unit = () + |}""".stripMargin, + ) + check( "scopes", """