From f343e44a92e7de755794dd2a8fe7f351b09cca62 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Tue, 12 Jun 2018 09:22:04 +0200 Subject: [PATCH 1/5] IDE: Show documentation for symbol on hover --- .../dotty/tools/dotc/interactive/InteractiveDriver.scala | 2 +- .../dotty/tools/languageserver/DottyLanguageServer.scala | 8 ++++++-- .../test/dotty/tools/languageserver/HoverTest.scala | 7 +++++++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala b/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala index fa7ca5437cd2..91104acda262 100644 --- a/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala +++ b/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala @@ -31,7 +31,7 @@ class InteractiveDriver(val settings: List[String]) extends Driver { override def sourcesRequired = false private val myInitCtx: Context = { - val rootCtx = initCtx.fresh.addMode(Mode.ReadPositions).addMode(Mode.Interactive) + val rootCtx = initCtx.fresh.addMode(Mode.ReadPositions).addMode(Mode.Interactive).addMode(Mode.ReadComments) rootCtx.setSetting(rootCtx.settings.YretainTrees, true) val ctx = setup(settings.toArray, rootCtx)._2 ctx.initialize()(ctx) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index e1f99c25b29a..e5edbaa9a4fb 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -340,13 +340,17 @@ class DottyLanguageServer extends LanguageServer implicit val ctx = driver.currentCtx val pos = sourcePosition(driver, uri, params.getPosition) - val tp = Interactive.enclosingType(driver.openedTrees(uri), pos) + val trees = driver.openedTrees(uri) + val tp = Interactive.enclosingType(trees, pos) val tpw = tp.widenTermRefExpr if (tpw == NoType) new Hover else { + import dotty.tools.dotc.core.Comments._ + val symbol = Interactive.enclosingSourceSymbol(trees, pos) + val doc = ctx.docCtx.flatMap(_.docstring(symbol)).map(_.raw + " / ").getOrElse("") val str = tpw.show.toString - new Hover(List(JEither.forLeft(str)).asJava, null) + new Hover(List(JEither.forLeft(doc + str)).asJava, null) } } diff --git a/language-server/test/dotty/tools/languageserver/HoverTest.scala b/language-server/test/dotty/tools/languageserver/HoverTest.scala index 34dac2ce0b91..d084262d2ef4 100644 --- a/language-server/test/dotty/tools/languageserver/HoverTest.scala +++ b/language-server/test/dotty/tools/languageserver/HoverTest.scala @@ -9,6 +9,13 @@ class HoverTest { @Test def hoverOnWhiteSpace0: Unit = code"$m1 $m2".withSource.hover(m1 to m2, "") + @Test def hoverOnClassShowsDoc: Unit = { + code"""$m1 /** foo */ ${m2}class Foo $m3 $m4""".withSource + .hover(m1 to m2, "") + .hover(m2 to m3, "/** foo */ / Foo") + .hover(m3 to m4, "") + } + @Test def hoverOnClass0: Unit = { code"""$m1 ${m2}class Foo $m3 $m4""".withSource .hover(m1 to m2, "") From a8a39423d1d60574d334aba0faf8fef3dbaeb6c0 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Tue, 12 Jun 2018 11:37:21 +0200 Subject: [PATCH 2/5] IDE: Use `MarkedString` in hover --- .../languageserver/DottyLanguageServer.scala | 23 +++++++++++++++---- .../tools/languageserver/HoverTest.scala | 5 +++- .../util/actions/CodeHover.scala | 5 ++-- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index e5edbaa9a4fb..c38a5824c26a 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -19,7 +19,7 @@ import scala.io.Codec import dotc._ import ast.{Trees, tpd} import core._, core.Decorators.{sourcePos => _, _} -import Contexts._, Flags._, Names._, NameOps._, Symbols._, SymDenotations._, Trees._, Types._ +import Comments.Comment, Contexts._, Flags._, Names._, NameOps._, Symbols._, SymDenotations._, Trees._, Types._ import classpath.ClassPathEntries import reporting._, reporting.diagnostic.MessageContainer import util._ @@ -348,9 +348,9 @@ class DottyLanguageServer extends LanguageServer else { import dotty.tools.dotc.core.Comments._ val symbol = Interactive.enclosingSourceSymbol(trees, pos) - val doc = ctx.docCtx.flatMap(_.docstring(symbol)).map(_.raw + " / ").getOrElse("") - val str = tpw.show.toString - new Hover(List(JEither.forLeft(doc + str)).asJava, null) + val docComment = ctx.docCtx.flatMap(_.docstring(symbol)) + val markedString = docMarkedString(docComment, tpw.show.toString) + new Hover(List(JEither.forRight(markedString)).asJava, null) } } @@ -467,6 +467,21 @@ object DottyLanguageServer { item } + private def docMarkedString(comment: Option[Comment], info: String): lsp4j.MarkedString = { + + val formattedComment = comment.map { comment => + s"""```scala + |${comment.raw} + |``` + |""".stripMargin + }.getOrElse("") + + val markedString = new lsp4j.MarkedString() + markedString.setValue(formattedComment + info) + markedString + } + + /** Create an lsp4j.SymbolInfo from a Symbol and a SourcePosition */ def symbolInfo(sym: Symbol, pos: SourcePosition)(implicit ctx: Context): lsp4j.SymbolInformation = { def symbolKind(sym: Symbol)(implicit ctx: Context): lsp4j.SymbolKind = { diff --git a/language-server/test/dotty/tools/languageserver/HoverTest.scala b/language-server/test/dotty/tools/languageserver/HoverTest.scala index d084262d2ef4..985b4f102f41 100644 --- a/language-server/test/dotty/tools/languageserver/HoverTest.scala +++ b/language-server/test/dotty/tools/languageserver/HoverTest.scala @@ -12,7 +12,10 @@ class HoverTest { @Test def hoverOnClassShowsDoc: Unit = { code"""$m1 /** foo */ ${m2}class Foo $m3 $m4""".withSource .hover(m1 to m2, "") - .hover(m2 to m3, "/** foo */ / Foo") + .hover(m2 to m3, """```scala + |/** foo */ + |``` + |Foo""".stripMargin) .hover(m3 to m4, "") } diff --git a/language-server/test/dotty/tools/languageserver/util/actions/CodeHover.scala b/language-server/test/dotty/tools/languageserver/util/actions/CodeHover.scala index 62b1c8564b0f..5a8e80427782 100644 --- a/language-server/test/dotty/tools/languageserver/util/actions/CodeHover.scala +++ b/language-server/test/dotty/tools/languageserver/util/actions/CodeHover.scala @@ -21,8 +21,9 @@ class CodeHover(override val range: CodeRange, expected: String) extends ActionO else { assertEquals(1, result.getContents.size) val content = result.getContents.get(0) - assertTrue(content.isLeft) - assertEquals(expected, content.getLeft) + assertTrue(content.isRight) + val markedString = content.getRight.getValue + assertEquals(expected, markedString) } } From b3542f4c835808dce44fb8ebf8c0620a98c1505e Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Tue, 12 Jun 2018 13:26:05 +0200 Subject: [PATCH 3/5] Put doc and type info in separate hover --- .../languageserver/DottyLanguageServer.scala | 22 ++++----- .../tools/languageserver/HoverTest.scala | 49 +++++++++---------- .../languageserver/util/CodeTester.scala | 4 +- .../util/actions/CodeHover.scala | 14 +++--- 4 files changed, 43 insertions(+), 46 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index c38a5824c26a..00c66f14ede6 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -349,8 +349,8 @@ class DottyLanguageServer extends LanguageServer import dotty.tools.dotc.core.Comments._ val symbol = Interactive.enclosingSourceSymbol(trees, pos) val docComment = ctx.docCtx.flatMap(_.docstring(symbol)) - val markedString = docMarkedString(docComment, tpw.show.toString) - new Hover(List(JEither.forRight(markedString)).asJava, null) + val markedStrings = docMarkedStrings(docComment, tpw.show.toString) + new Hover(markedStrings.map(JEither.forRight(_)).asJava, null) } } @@ -467,18 +467,16 @@ object DottyLanguageServer { item } - private def docMarkedString(comment: Option[Comment], info: String): lsp4j.MarkedString = { + private def docMarkedStrings(comment: Option[Comment], typeInfo: String): List[lsp4j.MarkedString] = { - val formattedComment = comment.map { comment => - s"""```scala - |${comment.raw} - |``` - |""".stripMargin - }.getOrElse("") + val docHover = comment.map { comment => + new lsp4j.MarkedString("scala", comment.raw) + } + + val typeInfoHover = new lsp4j.MarkedString() + typeInfoHover.setValue(typeInfo) - val markedString = new lsp4j.MarkedString() - markedString.setValue(formattedComment + info) - markedString + docHover.toList :+ typeInfoHover } diff --git a/language-server/test/dotty/tools/languageserver/HoverTest.scala b/language-server/test/dotty/tools/languageserver/HoverTest.scala index 985b4f102f41..3532831aa3b1 100644 --- a/language-server/test/dotty/tools/languageserver/HoverTest.scala +++ b/language-server/test/dotty/tools/languageserver/HoverTest.scala @@ -7,63 +7,60 @@ import dotty.tools.languageserver.util.Code._ class HoverTest { @Test def hoverOnWhiteSpace0: Unit = - code"$m1 $m2".withSource.hover(m1 to m2, "") + code"$m1 $m2".withSource.hover(m1 to m2, Nil) @Test def hoverOnClassShowsDoc: Unit = { code"""$m1 /** foo */ ${m2}class Foo $m3 $m4""".withSource - .hover(m1 to m2, "") - .hover(m2 to m3, """```scala - |/** foo */ - |``` - |Foo""".stripMargin) - .hover(m3 to m4, "") + .hover(m1 to m2, Nil) + .hover(m2 to m3, List("/** foo */", "Foo")) + .hover(m3 to m4, Nil) } @Test def hoverOnClass0: Unit = { code"""$m1 ${m2}class Foo $m3 $m4""".withSource - .hover(m1 to m2, "") - .hover(m2 to m3, "Foo") - .hover(m3 to m4, "") + .hover(m1 to m2, Nil) + .hover(m2 to m3, "Foo" :: Nil) + .hover(m3 to m4, Nil) } @Test def hoverOnClass1: Unit = { code"""$m1 ${m2}class Foo { } $m3 $m4""".withSource - .hover(m1 to m2, "") - .hover(m2 to m3, "Foo") - .hover(m3 to m4, "") + .hover(m1 to m2, Nil) + .hover(m2 to m3, "Foo" :: Nil) + .hover(m3 to m4, Nil) } @Test def hoverOnValDef0: Unit = { code"""class Foo { | ${m1}val x = ${m2}8$m3; ${m4}x$m5 |}""".withSource - .hover(m1 to m2, "Int") - .hover(m2 to m3, "Int(8)") - .hover(m4 to m5, "Int") + .hover(m1 to m2, "Int" :: Nil) + .hover(m2 to m3, "Int(8)" :: Nil) + .hover(m4 to m5, "Int" :: Nil) } @Test def hoverOnValDef1: Unit = { code"""class Foo { | ${m1}final val x = 8$m2; ${m3}x$m4 |}""".withSource - .hover(m1 to m2, "Int(8)") - .hover(m3 to m4, "Int(8)") + .hover(m1 to m2, "Int(8)" :: Nil) + .hover(m3 to m4, "Int(8)" :: Nil) } @Test def hoverOnDefDef0: Unit = { code"""class Foo { | ${m1}def x = ${m2}8$m3; ${m4}x$m5 |}""".withSource - .hover(m1 to m2, "Int") - .hover(m2 to m3, "Int(8)") - .hover(m4 to m5, "Int") + .hover(m1 to m2, "Int" :: Nil) + .hover(m2 to m3, "Int(8)" :: Nil) + .hover(m4 to m5, "Int" :: Nil) } @Test def hoverMissingRef0: Unit = { code"""class Foo { | ${m1}x$m2 |}""".withSource - .hover(m1 to m2, "") + .hover(m1 to m2, "" :: Nil) } @Test def hoverFun0: Unit = { @@ -75,10 +72,10 @@ class HoverTest { | ${m5}y($m6)$m7 |} """.withSource - .hover(m1 to m2, "String(\"abc\")") - .hover(m3 to m4, "String") - .hover(m5 to m6, "(): Int") - .hover(m6 to m7, "Int") + .hover(m1 to m2, "String(\"abc\")" :: Nil) + .hover(m3 to m4, "String" :: Nil) + .hover(m5 to m6, "(): Int" :: Nil) + .hover(m6 to m7, "Int" :: Nil) } } diff --git a/language-server/test/dotty/tools/languageserver/util/CodeTester.scala b/language-server/test/dotty/tools/languageserver/util/CodeTester.scala index 5d79ff733a86..d5d4a40cbfeb 100644 --- a/language-server/test/dotty/tools/languageserver/util/CodeTester.scala +++ b/language-server/test/dotty/tools/languageserver/util/CodeTester.scala @@ -25,11 +25,11 @@ class CodeTester(sources: List[SourceWithPositions], actions: List[Action]) { * Perform a hover over `range`, verifies that result matches `expected`. * * @param range The range over which to hover. - * @param expected The expected result. + * @param expected The expected results. * * @see dotty.tools.languageserver.util.actions.CodeHover */ - def hover(range: CodeRange, expected: String): this.type = + def hover(range: CodeRange, expected: List[String]): this.type = doAction(new CodeHover(range, expected)) /** diff --git a/language-server/test/dotty/tools/languageserver/util/actions/CodeHover.scala b/language-server/test/dotty/tools/languageserver/util/actions/CodeHover.scala index 5a8e80427782..d8635588f6af 100644 --- a/language-server/test/dotty/tools/languageserver/util/actions/CodeHover.scala +++ b/language-server/test/dotty/tools/languageserver/util/actions/CodeHover.scala @@ -5,6 +5,8 @@ import dotty.tools.languageserver.util.{CodeRange, PositionContext} import org.junit.Assert.{assertEquals, assertNull, assertTrue} +import scala.collection.JavaConverters._ + /** * An action requesting for the info shown when `range` is hovered. * This action corresponds to the `textDocument/hover` method of the Language Server Protocol. @@ -12,18 +14,18 @@ import org.junit.Assert.{assertEquals, assertNull, assertTrue} * @param range The range of positions that should be hovered. * @param expected The expected result. */ -class CodeHover(override val range: CodeRange, expected: String) extends ActionOnRange { +class CodeHover(override val range: CodeRange, expected: List[String]) extends ActionOnRange { override def onMarker(marker: CodeMarker): Exec[Unit] = { val result = server.hover(marker.toTextDocumentPositionParams).get() assertNull(result.getRange) if (expected.isEmpty) assertNull(result.getContents) else { - assertEquals(1, result.getContents.size) - val content = result.getContents.get(0) - assertTrue(content.isRight) - val markedString = content.getRight.getValue - assertEquals(expected, markedString) + assertEquals(expected.size, result.getContents.size) + expected.zip(result.getContents.asScala).foreach { case (expected, actual) => + assertTrue(actual.isRight) + assertEquals(expected, actual.getRight.getValue) + } } } From 550be2249f1159f9bc4bb9880413ca59a1ee1c0d Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Tue, 12 Jun 2018 13:31:54 +0200 Subject: [PATCH 4/5] IDE: Show type info and then doc in hover Otherwise, we only see the docs at first glance when the docstring is long, and we need to scroll to get to the type info (which is generally a single line). --- .../src/dotty/tools/languageserver/DottyLanguageServer.scala | 2 +- language-server/test/dotty/tools/languageserver/HoverTest.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index 00c66f14ede6..dfbd90de3bc9 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -476,7 +476,7 @@ object DottyLanguageServer { val typeInfoHover = new lsp4j.MarkedString() typeInfoHover.setValue(typeInfo) - docHover.toList :+ typeInfoHover + typeInfoHover :: docHover.toList } diff --git a/language-server/test/dotty/tools/languageserver/HoverTest.scala b/language-server/test/dotty/tools/languageserver/HoverTest.scala index 3532831aa3b1..e8d8761f0b5a 100644 --- a/language-server/test/dotty/tools/languageserver/HoverTest.scala +++ b/language-server/test/dotty/tools/languageserver/HoverTest.scala @@ -12,7 +12,7 @@ class HoverTest { @Test def hoverOnClassShowsDoc: Unit = { code"""$m1 /** foo */ ${m2}class Foo $m3 $m4""".withSource .hover(m1 to m2, Nil) - .hover(m2 to m3, List("/** foo */", "Foo")) + .hover(m2 to m3, List("Foo", "/** foo */")) .hover(m3 to m4, Nil) } From b47cc1f311307ea3d31336dd60501cc4c47540bb Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Thu, 28 Jun 2018 14:34:10 +0200 Subject: [PATCH 5/5] Address review comment --- .../src/dotty/tools/languageserver/DottyLanguageServer.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index dfbd90de3bc9..23b2c3f745d3 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -19,7 +19,7 @@ import scala.io.Codec import dotc._ import ast.{Trees, tpd} import core._, core.Decorators.{sourcePos => _, _} -import Comments.Comment, Contexts._, Flags._, Names._, NameOps._, Symbols._, SymDenotations._, Trees._, Types._ +import Comments._, Contexts._, Flags._, Names._, NameOps._, Symbols._, SymDenotations._, Trees._, Types._ import classpath.ClassPathEntries import reporting._, reporting.diagnostic.MessageContainer import util._ @@ -346,7 +346,6 @@ class DottyLanguageServer extends LanguageServer if (tpw == NoType) new Hover else { - import dotty.tools.dotc.core.Comments._ val symbol = Interactive.enclosingSourceSymbol(trees, pos) val docComment = ctx.docCtx.flatMap(_.docstring(symbol)) val markedStrings = docMarkedStrings(docComment, tpw.show.toString)