Merge pull request #5900 from dotty-staging/worksheet-improvements

Worksheet: improve hover support

Author: smarter

docs/docs/usage/worksheet-mode-implementation-details.md

@@ -65,12 +65,12 @@ indicate that worksheet execution has produced some output.
         * The worksheet that produced this output.
         */
        textDocument: VersionedTextDocumentIdentifier;
-   
+
        /**
-        * The line number of the expression that produced this output.
+        * The range of the expression that produced this output.
         */
-       line: int;
-   
+       range: Range;
+
        /**
         * The output that has been produced.
         */

language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala

@@ -240,17 +240,20 @@ class DottyLanguageServer extends LanguageServer
     val document = params.getTextDocument
     val uri = new URI(document.getUri)
     val driver = driverFor(uri)
+    implicit def ctx: Context = driver.currentCtx
     val worksheetMode = isWorksheet(uri)
 
-    val (text, positionMapper) =
-      if (worksheetMode) (wrapWorksheet(document.getText), Some(toUnwrappedPosition _))
-      else (document.getText, None)
+    val text =
+      if (worksheetMode)
+        wrapWorksheet(document.getText)
+      else
+        document.getText
 
     val diags = driver.run(uri, text)
 
     client.publishDiagnostics(new PublishDiagnosticsParams(
       document.getUri,
-      diags.flatMap(diagnostic(_, positionMapper)(driver.currentCtx)).asJava))
+      diags.flatMap(diagnostic).asJava))
   }
 
   override def didChange(params: DidChangeTextDocumentParams): Unit = {
@@ -262,19 +265,22 @@ class DottyLanguageServer extends LanguageServer
       checkMemory()
 
       val driver = driverFor(uri)
+      implicit def ctx: Context = driver.currentCtx
 
       val change = params.getContentChanges.get(0)
       assert(change.getRange == null, "TextDocumentSyncKind.Incremental support is not implemented")
 
-      val (text, positionMapper) =
-        if (worksheetMode) (wrapWorksheet(change.getText), Some(toUnwrappedPosition _))
-        else (change.getText, None)
+      val text =
+        if (worksheetMode)
+          wrapWorksheet(change.getText)
+        else
+          change.getText
 
       val diags = driver.run(uri, text)
 
       client.publishDiagnostics(new PublishDiagnosticsParams(
         document.getUri,
-        diags.flatMap(diagnostic(_, positionMapper)(driver.currentCtx)).asJava))
+        diags.flatMap(diagnostic).asJava))
     }
   }
 
@@ -299,7 +305,7 @@ class DottyLanguageServer extends LanguageServer
   override def completion(params: CompletionParams) = computeAsync { cancelToken =>
     val uri = new URI(params.getTextDocument.getUri)
     val driver = driverFor(uri)
-    implicit val ctx = driver.currentCtx
+    implicit def ctx: Context = driver.currentCtx
 
     val pos = sourcePosition(driver, uri, params.getPosition)
     val items = driver.compilationUnits.get(uri) match {
@@ -318,13 +324,13 @@ class DottyLanguageServer extends LanguageServer
   override def definition(params: TextDocumentPositionParams) = computeAsync { cancelToken =>
     val uri = new URI(params.getTextDocument.getUri)
     val driver = driverFor(uri)
-    implicit val ctx = driver.currentCtx
+    implicit def ctx: Context = driver.currentCtx
 
     val pos = sourcePosition(driver, uri, params.getPosition)
     val path = Interactive.pathTo(driver.openedTrees(uri), pos)
 
     val definitions = Interactive.findDefinitions(path, pos, driver).toList
-    definitions.flatMap(d => location(d.namePos, positionMapperFor(d.source))).asJava
+    definitions.flatMap(d => location(d.namePos)).asJava
   }
 
   override def references(params: ReferenceParams) = computeAsync { cancelToken =>
@@ -341,7 +347,7 @@ class DottyLanguageServer extends LanguageServer
     val pos = sourcePosition(driver, uri, params.getPosition)
 
     val (definitions, originalSymbols) = {
-      implicit val ctx: Context = driver.currentCtx
+      implicit def ctx: Context = driver.currentCtx
       val path = Interactive.pathTo(driver.openedTrees(uri), pos)
       val definitions = Interactive.findDefinitions(path, pos, driver)
       val originalSymbols = Interactive.enclosingSourceSymbols(path, pos)
@@ -359,7 +365,7 @@ class DottyLanguageServer extends LanguageServer
           val name = definition.name(ctx).sourceModuleName.toString
           val trees = remoteDriver.sourceTreesContaining(name)(ctx)
           val matches = Interactive.findTreesMatching(trees, includes, definition)(ctx)
-          matches.map(tree => location(tree.namePos(ctx), positionMapperFor(tree.source)))
+          matches.map(tree => location(tree.namePos(ctx)))
         }
       }
     }.toList
@@ -370,7 +376,7 @@ class DottyLanguageServer extends LanguageServer
   override def rename(params: RenameParams) = computeAsync { cancelToken =>
     val uri = new URI(params.getTextDocument.getUri)
     val driver = driverFor(uri)
-    implicit val ctx = driver.currentCtx
+    implicit def ctx: Context = driver.currentCtx
 
     val uriTrees = driver.openedTrees(uri)
     val pos = sourcePosition(driver, uri, params.getPosition)
@@ -429,15 +435,15 @@ class DottyLanguageServer extends LanguageServer
         .flatMap((uriOpt, ref) => uriOpt.map(uri => (uri.toString, ref)))
         .mapValues(refs =>
           refs.flatMap(ref =>
-            range(ref.namePos, positionMapperFor(ref.source)).map(nameRange => new TextEdit(nameRange, newName))).distinct.asJava)
+            range(ref.namePos).map(nameRange => new TextEdit(nameRange, newName))).distinct.asJava)
 
     new WorkspaceEdit(changes.asJava)
   }
 
   override def documentHighlight(params: TextDocumentPositionParams) = computeAsync { cancelToken =>
     val uri = new URI(params.getTextDocument.getUri)
     val driver = driverFor(uri)
-    implicit val ctx = driver.currentCtx
+    implicit def ctx: Context = driver.currentCtx
 
     val pos = sourcePosition(driver, uri, params.getPosition)
     val uriTrees = driver.openedTrees(uri)
@@ -449,15 +455,15 @@ class DottyLanguageServer extends LanguageServer
       val refs = Interactive.findTreesMatching(uriTrees, includes, sym)
       (for {
         ref <- refs
-        nameRange <- range(ref.namePos, positionMapperFor(ref.source))
+        nameRange <- range(ref.namePos)
       } yield new DocumentHighlight(nameRange, DocumentHighlightKind.Read))
     }.distinct.asJava
   }
 
   override def hover(params: TextDocumentPositionParams) = computeAsync { cancelToken =>
     val uri = new URI(params.getTextDocument.getUri)
     val driver = driverFor(uri)
-    implicit val ctx = driver.currentCtx
+    implicit def ctx: Context = driver.currentCtx
 
     val pos = sourcePosition(driver, uri, params.getPosition)
     val trees = driver.openedTrees(uri)
@@ -481,26 +487,26 @@ class DottyLanguageServer extends LanguageServer
   override def documentSymbol(params: DocumentSymbolParams) = computeAsync { cancelToken =>
     val uri = new URI(params.getTextDocument.getUri)
     val driver = driverFor(uri)
-    implicit val ctx = driver.currentCtx
+    implicit def ctx: Context = driver.currentCtx
 
     val uriTrees = driver.openedTrees(uri)
 
     val defs = Interactive.namedTrees(uriTrees, Include.empty)
     (for {
       d <- defs if !isWorksheetWrapper(d)
-      info <- symbolInfo(d.tree.symbol, d.namePos, positionMapperFor(d.source))
+      info <- symbolInfo(d.tree.symbol, d.namePos)
     } yield JEither.forLeft(info)).asJava
   }
 
   override def symbol(params: WorkspaceSymbolParams) = computeAsync { cancelToken =>
     val query = params.getQuery
 
     drivers.values.toList.flatMap { driver =>
-      implicit val ctx = driver.currentCtx
+      implicit def ctx: Context = driver.currentCtx
 
       val trees = driver.sourceTreesContaining(query)
       val defs = Interactive.namedTrees(trees, Include.empty, _.name.toString.contains(query))
-      defs.flatMap(d => symbolInfo(d.tree.symbol, d.namePos, positionMapperFor(d.source)))
+      defs.flatMap(d => symbolInfo(d.tree.symbol, d.namePos))
     }.asJava
   }
 
@@ -511,7 +517,7 @@ class DottyLanguageServer extends LanguageServer
     val pos = sourcePosition(driver, uri, params.getPosition)
 
     val (definitions, originalSymbols) = {
-      implicit val ctx: Context = driver.currentCtx
+      implicit def ctx: Context = driver.currentCtx
       val path = Interactive.pathTo(driver.openedTrees(uri), pos)
       val originalSymbols = Interactive.enclosingSourceSymbols(path, pos)
       val definitions = Interactive.findDefinitions(path, pos, driver)
@@ -528,7 +534,7 @@ class DottyLanguageServer extends LanguageServer
           tree => predicates.exists(_(tree))
         }
         val matches = Interactive.namedTrees(trees, Include.local, predicate)(ctx)
-        matches.map(tree => location(tree.namePos(ctx), positionMapperFor(tree.source)))
+        matches.map(tree => location(tree.namePos(ctx)))
       }
     }.toList
 
@@ -539,7 +545,7 @@ class DottyLanguageServer extends LanguageServer
 
     val uri = new URI(params.getTextDocument.getUri)
     val driver = driverFor(uri)
-    implicit val ctx = driver.currentCtx
+    implicit def ctx: Context = driver.currentCtx
 
     val pos = sourcePosition(driver, uri, params.getPosition)
     val trees = driver.openedTrees(uri)
@@ -653,9 +659,9 @@ object DottyLanguageServer {
   }
 
   /** Convert a SourcePosition to an lsp4j.Range */
-  def range(p: SourcePosition, positionMapper: Option[SourcePosition => SourcePosition] = None): Option[lsp4j.Range] =
+  def range(p: SourcePosition): Option[lsp4j.Range] =
     if (p.exists) {
-      val mappedPosition = positionMapper.map(_(p)).getOrElse(p)
+      val mappedPosition = positionMapperFor(p.source).map(_(p)).getOrElse(p)
       Some(new lsp4j.Range(
         new lsp4j.Position(mappedPosition.startLine, mappedPosition.startColumn),
         new lsp4j.Position(mappedPosition.endLine, mappedPosition.endColumn)
@@ -664,19 +670,16 @@ object DottyLanguageServer {
       None
 
   /** Convert a SourcePosition to an lsp4.Location */
-  def location(p: SourcePosition, positionMapper: Option[SourcePosition => SourcePosition] = None): Option[lsp4j.Location] =
+  def location(p: SourcePosition): Option[lsp4j.Location] =
     for {
       uri <- toUriOption(p.source)
-      r <- range(p, positionMapper)
+      r <- range(p)
     } yield new lsp4j.Location(uri.toString, r)
 
   /**
-   * Convert a MessageContainer to an lsp4j.Diagnostic. The positions are transformed vy
-   * `positionMapper`.
+   * Convert a MessageContainer to an lsp4j.Diagnostic.
    */
-  def diagnostic(mc: MessageContainer,
-                 positionMapper: Option[SourcePosition => SourcePosition] = None
-                )(implicit ctx: Context): Option[lsp4j.Diagnostic] =
+  def diagnostic(mc: MessageContainer)(implicit ctx: Context): Option[lsp4j.Diagnostic] =
     if (!mc.pos.exists)
       None // diagnostics without positions are not supported: https://github.com/Microsoft/language-server-protocol/issues/249
     else {
@@ -697,7 +700,7 @@ object DottyLanguageServer {
       val message = mc.contained()
       if (displayMessage(message, mc.pos.source)) {
         val code = message.errorId.errorNumber.toString
-        range(mc.pos, positionMapper).map(r =>
+        range(mc.pos).map(r =>
             new lsp4j.Diagnostic(
               r, mc.message, severity(mc.level), /*source =*/ "", code))
       } else {
@@ -866,7 +869,7 @@ object DottyLanguageServer {
   }
 
   /** Create an lsp4j.SymbolInfo from a Symbol and a SourcePosition */
-  def symbolInfo(sym: Symbol, pos: SourcePosition, positionMapper: Option[SourcePosition => SourcePosition])(implicit ctx: Context): Option[lsp4j.SymbolInformation] = {
+  def symbolInfo(sym: Symbol, pos: SourcePosition)(implicit ctx: Context): Option[lsp4j.SymbolInformation] = {
     def symbolKind(sym: Symbol)(implicit ctx: Context): lsp4j.SymbolKind = {
       import lsp4j.{SymbolKind => SK}
 
@@ -893,7 +896,7 @@ object DottyLanguageServer {
       else
         null
 
-    location(pos, positionMapper).map(l => new lsp4j.SymbolInformation(name, symbolKind(sym), l, containerName))
+    location(pos).map(l => new lsp4j.SymbolInformation(name, symbolKind(sym), l, containerName))
   }
 
   /** Convert `signature` to a `SignatureInformation` */

language-server/src/dotty/tools/languageserver/worksheet/Worksheet.scala

@@ -4,7 +4,7 @@ import dotty.tools.dotc.ast.tpd.{DefTree, Template, Tree, TypeDef}
 import dotty.tools.dotc.core.Contexts.Context
 import dotty.tools.dotc.interactive.SourceTree
 import dotty.tools.dotc.util.Spans.Span
-import dotty.tools.dotc.util.SourceFile
+import dotty.tools.dotc.util.{ SourceFile, SourcePosition, NoSourcePosition }
 
 import dotty.tools.dotc.core.Flags.Synthetic
 
@@ -22,15 +22,15 @@ object Worksheet {
    */
   def run(tree: SourceTree,
           treeLock: Object,
-          sendMessage: (Int, String) => Unit,
+          sendMessage: (SourcePosition, String) => Unit,
           cancelChecker: CancelChecker)(
     implicit ctx: Context): Unit = {
     // For now, don't try to run multiple evaluators in parallel, this would require
     // changes to the logic of Evaluator.get among other things.
     Evaluator.synchronized {
       Evaluator.get(cancelChecker) match {
         case None =>
-          sendMessage(1, "Couldn't start the JVM.")
+          sendMessage(NoSourcePosition, "Couldn't start the JVM.")
         case Some(evaluator) =>
           val queries = treeLock.synchronized {
             tree.tree match {
@@ -59,16 +59,15 @@ object Worksheet {
   }
 
   /**
-   * Extract the line number and source code corresponding to this tree
+   * Extract the position and source code corresponding to this tree
    *
    * @param evaluator  The JVM that runs the REPL.
    * @param tree       The compiled tree to evaluate.
    * @param sourcefile The sourcefile of the worksheet.
    */
-  private def query(tree: Tree, sourcefile: SourceFile): (Int, String) = {
-    val line = sourcefile.offsetToLine(tree.span.end)
+  private def query(tree: Tree, sourcefile: SourceFile)(implicit ctx: Context): (SourcePosition, String) = {
     val source = sourcefile.content.slice(tree.span.start, tree.span.end).mkString
-    (line, source)
+    (tree.sourcePos, source)
   }
 
   private def bounds(span: Span): (Int, Int) = (span.start, span.end)

language-server/src/dotty/tools/languageserver/worksheet/WorksheetMessages.scala

@@ -1,6 +1,6 @@
 package dotty.tools.languageserver.worksheet
 
-import org.eclipse.lsp4j.VersionedTextDocumentIdentifier
+import org.eclipse.lsp4j.{ Range, VersionedTextDocumentIdentifier }
 
 // All case classes in this file should have zero-parameters secondary
 // constructors to allow Gson to reflectively create instances on
@@ -17,6 +17,6 @@ case class WorksheetRunResult(success: Boolean) {
 }
 
 /** The parameters to the `worksheet/publishOutput` notification. */
-case class WorksheetRunOutput(textDocument: VersionedTextDocumentIdentifier, line: Int, content: String) {
-  def this() = this(null, 0, null)
+case class WorksheetRunOutput(textDocument: VersionedTextDocumentIdentifier, range: Range, content: String) {
+  def this() = this(null, null, null)
 }

language-server/src/dotty/tools/languageserver/worksheet/WorksheetService.scala

@@ -1,8 +1,10 @@
 package dotty.tools.languageserver.worksheet
 
 import dotty.tools.dotc.core.Contexts.Context
+import dotty.tools.dotc.util.SourcePosition
 import dotty.tools.dotc.interactive.InteractiveDriver
 import dotty.tools.languageserver.DottyLanguageServer
+import dotty.tools.languageserver.DottyLanguageServer.range
 
 import org.eclipse.lsp4j.jsonrpc._//{CancelChecker, CompletableFutures}
 import org.eclipse.lsp4j.jsonrpc.services._//{JsonSegment, JsonRequest}
@@ -19,8 +21,8 @@ trait WorksheetService { thisServer: DottyLanguageServer =>
       val uri = new URI(params.textDocument.getUri)
       try {
         val driver = driverFor(uri)
-        val sendMessage =
-          (line: Int, msg: String) => client.publishOutput(WorksheetRunOutput(params.textDocument, line, msg))
+        val sendMessage = (pos: SourcePosition, msg: String) =>
+          client.publishOutput(WorksheetRunOutput(params.textDocument, range(pos).get, msg))
 
         runWorksheet(driver, uri, sendMessage, cancelChecker)(driver.currentCtx)
         cancelChecker.checkCanceled()
@@ -41,7 +43,7 @@ trait WorksheetService { thisServer: DottyLanguageServer =>
    */
   private def runWorksheet(driver: InteractiveDriver,
                            uri: URI,
-                           sendMessage: (Int, String) => Unit,
+                           sendMessage: (SourcePosition, String) => Unit,
                            cancelChecker: CancelChecker)(
       implicit ctx: Context): Unit = {
     val treeOpt = thisServer.synchronized {