Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove signature help fallback in hover. #602

Merged
merged 3 commits into from Apr 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 11 additions & 6 deletions mtags/src/main/scala/scala/meta/internal/pc/HoverMarkup.scala
Expand Up @@ -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()
}

Expand Down
104 changes: 36 additions & 68 deletions mtags/src/main/scala/scala/meta/internal/pc/HoverProvider.scala
Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand All @@ -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
}
}
}
2 changes: 1 addition & 1 deletion 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")
26 changes: 19 additions & 7 deletions tests/cross/src/test/scala/tests/hover/HoverNamedArgSuite.scala
Expand Up @@ -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)
|}
Expand All @@ -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
)

}
4 changes: 2 additions & 2 deletions tests/unit/src/test/scala/tests/CompletionSlowSuite.scala
Expand Up @@ -9,7 +9,7 @@ object CompletionSlowSuite extends BaseCompletionSlowSuite("completion") {
basicTest(V.scala212)
}

testAsync("workspace") {
flakyTest("workspace") {
cleanWorkspace()
for {
_ <- server.initialize(
Expand Down Expand Up @@ -129,7 +129,7 @@ object CompletionSlowSuite extends BaseCompletionSlowSuite("completion") {
} yield ()
)

testAsync("symbol-prefixes") {
flakyTest("symbol-prefixes") {
cleanWorkspace()
for {
_ <- server.initialize(
Expand Down