Merge pull request #602 from olafurpg/hover-param

Remove signature help fallback in hover.
scalameta · Apr 1, 2019 · 19d33c5 · 19d33c5
2 parents f6986d4 + 7f12a9a
commit 19d33c5
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 84 deletions.
diff --git a/mtags/src/main/scala/scala/meta/internal/pc/HoverMarkup.scala b/mtags/src/main/scala/scala/meta/internal/pc/HoverMarkup.scala
@@ -24,12 +24,17 @@ object HoverMarkup {
         .append(expressionType)
         .append("\n```\n")
     }
-    markdown
-      .append(if (needsExpressionType) "**Symbol signature**:\n" else "")
-      .append("```scala\n")
-      .append(symbolSignature)
-      .append("\n```\n")
-      .append(docstring)
+    if (symbolSignature.nonEmpty) {
+      markdown
+        .append(if (needsExpressionType) "**Symbol signature**:\n" else "")
+        .append("```scala\n")
+        .append(symbolSignature)
+        .append("\n```")
+    }
+    if (docstring.nonEmpty)
+      markdown
+        .append("\n")
+        .append(docstring)
     markdown.toString()
   }
 

diff --git a/mtags/src/main/scala/scala/meta/internal/pc/HoverProvider.scala b/mtags/src/main/scala/scala/meta/internal/pc/HoverProvider.scala
@@ -24,10 +24,7 @@ class HoverProvider(val compiler: MetalsGlobal, params: OffsetParams) {
       )
       val pos = unit.position(params.offset())
       val tree = typedHoverTreeAt(pos)
-      val NamedArgument = new NamedArgument(pos)
       tree match {
-        case NamedArgument(hover) =>
-          Some(hover)
         case Import(qual, selectors) =>
           for {
             sel <- selectors.reverseIterator.find(_.namePos <= pos.start)
@@ -236,69 +233,6 @@ class HoverProvider(val compiler: MetalsGlobal, params: OffsetParams) {
     }
   }
 
-  /**
-   * Extractor for named arguments, example `arg` in `foo(arg = 42)`.
-   *
-   * When the cursor is over a named argument then we fallback to signature help
-   * (parameter hints) because the de-sugaring logic is quite involved.
-   */
-  class NamedArgument(pos: Position) {
-    def unapply(a: Apply): Option[Hover] = a match {
-      case Apply(qual, _) if !qual.pos.includes(pos) && !isForSynthetic(a) =>
-        val signatureHelp =
-          new SignatureHelpProvider(compiler).signatureHelp(params)
-        if (!signatureHelp.getSignatures.isEmpty &&
-          signatureHelp.getActiveParameter >= 0 &&
-          signatureHelp.getActiveSignature >= 0) {
-          val activeParameter = signatureHelp.getSignatures
-            .get(signatureHelp.getActiveSignature)
-            .getParameters
-            .get(signatureHelp.getActiveParameter)
-
-          val contents =
-            new java.util.ArrayList[JEither[String, MarkedString]]()
-          contents.add(
-            JEither.forRight(
-              new MarkedString("scala", activeParameter.getLabel)
-            )
-          )
-          Option(activeParameter.getDocumentation).foreach { documentation =>
-            documentation.asScala match {
-              case Left(value) =>
-                contents.add(JEither.forLeft(value))
-              case Right(value) =>
-                contents.add(
-                  JEither
-                    .forRight(new MarkedString(value.getKind, value.getValue))
-                )
-            }
-          }
-          Some(new Hover(contents))
-        } else {
-          None
-        }
-      case _ =>
-        None
-    }
-    val isForName = Set[Name](
-      nme.map,
-      nme.withFilter,
-      nme.flatMap,
-      nme.foreach
-    )
-    private def isForSynthetic(gtree: Tree): Boolean = {
-      def isForComprehensionSyntheticName(select: Select): Boolean = {
-        select.pos == select.qualifier.pos && isForName(select.name)
-      }
-      gtree match {
-        case Apply(fun, List(_: Function)) => isForSynthetic(fun)
-        case TypeApply(fun, _) => isForSynthetic(fun)
-        case gtree: Select if isForComprehensionSyntheticName(gtree) => true
-        case _ => false
-      }
-    }
-  }
-
   def symbolFlagString(sym: Symbol): String = {
     var mask = sym.flagMask
     // Strip case modifier off non-class symbols like synthetic apply/copy.
@@ -311,9 +245,43 @@ class HoverProvider(val compiler: MetalsGlobal, params: OffsetParams) {
       case Select(qual, _) if qual.pos.includes(pos) => loop(qual)
       case t => t
     }
-    typedTreeAt(pos) match {
-      case Import(qual, _) if qual.pos.includes(pos) => loop(qual)
+    val typedTree = typedTreeAt(pos)
+    typedTree match {
+      case Import(qual, _) if qual.pos.includes(pos) =>
+        loop(qual)
+      case Apply(fun, args)
+          if !fun.pos.includes(pos) &&
+            !isForSynthetic(typedTree) =>
+        // Looks like a named argument, try the arguments.
+        val arg = args.collectFirst {
+          case arg if treePos(arg).includes(pos) =>
+            arg match {
+              case Block(_, expr) if treePos(expr).includes(pos) =>
+                // looks like a desugaring of named arguments in different order from definition-site.
+                expr
+              case a => a
+            }
+        }
+        arg.getOrElse(typedTree)
       case t => t
     }
   }
+
+  lazy val isForName = Set[Name](
+    nme.map,
+    nme.withFilter,
+    nme.flatMap,
+    nme.foreach
+  )
+  def isForSynthetic(gtree: Tree): Boolean = {
+    def isForComprehensionSyntheticName(select: Select): Boolean = {
+      select.pos == select.qualifier.pos && isForName(select.name)
+    }
+    gtree match {
+      case Apply(fun, List(_: Function)) => isForSynthetic(fun)
+      case TypeApply(fun, _) => isForSynthetic(fun)
+      case gtree: Select if isForComprehensionSyntheticName(gtree) => true
+      case _ => false
+    }
+  }
 }
diff --git a/project/project/plugins.sbt b/project/project/plugins.sbt
@@ -1 +1 @@
-addSbtPlugin("io.get-coursier" % "sbt-coursier" % "1.1.0-M9")
+addSbtPlugin("io.get-coursier" % "sbt-coursier" % "1.1.0-M13-2")
diff --git a/tests/cross/src/test/scala/tests/hover/HoverNamedArgSuite.scala b/tests/cross/src/test/scala/tests/hover/HoverNamedArgSuite.scala
@@ -13,22 +13,23 @@ object HoverNamedArgSuite extends BaseHoverSuite {
       |   * @param named the argument
       |   */
       |  def foo(named: Int): Unit = ()
-      |  foo(nam@@ed = 2)
+      |  <<foo(nam@@ed = 2)>>
       |}
       |""".stripMargin,
     """|```scala
-       |named: Int
-       |```
-       |```markdown
-       |the argument
+       |def foo(named: Int): Unit
        |```
+       |Runs foo
+       |
+       |**Parameters**
+       |- `named`: the argument
        |""".stripMargin
   )
 
   check(
     "error",
     """package a
-      |object b {
+      |object c {
       |  def foo(a: String, named: Int): Unit = ()
       |  foo(nam@@ed = 2)
       |}
@@ -39,12 +40,23 @@ object HoverNamedArgSuite extends BaseHoverSuite {
   check(
     "error2",
     """package a
-      |object b {
+      |object d {
       |  def foo(a: Int, named: Int): Unit = ()
       |  foo("error", nam@@ed = 2)
       |}
       |""".stripMargin,
     ""
   )
 
+  check(
+    "nested",
+    """package a
+      |object e {
+      |  class User(name: String, age: Int)
+      |  println(<<new User(age = 42, n@@ame = "")>>)
+      |}
+      |""".stripMargin,
+    "def this(name: String, age: Int): e.User".hover
+  )
+
 }
diff --git a/tests/unit/src/test/scala/tests/CompletionSlowSuite.scala b/tests/unit/src/test/scala/tests/CompletionSlowSuite.scala
@@ -9,7 +9,7 @@ object CompletionSlowSuite extends BaseCompletionSlowSuite("completion") {
     basicTest(V.scala212)
   }
 
-  testAsync("workspace") {
+  flakyTest("workspace") {
     cleanWorkspace()
     for {
       _ <- server.initialize(
@@ -129,7 +129,7 @@ object CompletionSlowSuite extends BaseCompletionSlowSuite("completion") {
     } yield ()
   )
 
-  testAsync("symbol-prefixes") {
+  flakyTest("symbol-prefixes") {
     cleanWorkspace()
     for {
       _ <- server.initialize(