worksheet/publishOutput returns a range instead of line

This command is sent from the server to the client when a statement in
the current worksheet has finished running, but statements can be
multi-line so returning just the line number loses some information.

We use this extra information in vscode-dotty so that the hover pop-up
containing the output of the statement is displayed when hovering over
any part of the statement, and not just the last line. To avoid breaking
this hover pop-up when using an old version of Dotty, some compatibiltiy
code is necessary.

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/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 {

language-server/test/dotty/tools/languageserver/WorksheetTest.scala

@@ -13,86 +13,98 @@ import java.lang.System.{lineSeparator => nl}
 class WorksheetTest {
 
   @Test def runExpression: Unit = {
-    ws"${m1}2 + 2".withSource
-      .run(m1, "1:val res0: Int = 4")
+    ws"${m1}2 + 2${m2}".withSource
+      .run(m1,
+        ((m1 to m2), "val res0: Int = 4"))
   }
 
   @Test def runSimpleVal: Unit = {
-    ws"${m1}val foo = 123".withSource
-      .run(m1, "1:val foo: Int = 123")
+    ws"${m1}val foo = 123${m2}".withSource
+      .run(m1,
+        ((m1 to m2), "val foo: Int = 123"))
   }
 
   @Test def usePreviousDefinition: Unit = {
-    ws"""${m1}val foo = 123
-         val bar = foo + 1""".withSource
-      .run(m1, "1:val foo: Int = 123",
-                    "2:val bar: Int = 124")
+    ws"""${m1}val foo = 123${m2}
+         ${m3}val bar = foo + 1${m4}""".withSource
+      .run(m1,
+        ((m1 to m2), "val foo: Int = 123"),
+        ((m3 to m4), "val bar: Int = 124"))
   }
 
   @Test def defineObject: Unit = {
-    ws"""${m1}def foo(x: Int) = x + 1
-         foo(1)""".withSource
-      .run(m1, "1:def foo(x: Int): Int",
-                    "2:val res0: Int = 2")
+    ws"""${m1}def foo(x: Int) = x + 1${m2}
+         ${m3}foo(1)${m4}""".withSource
+      .run(m1,
+        ((m1 to m2), "def foo(x: Int): Int"),
+        ((m3 to m4), "val res0: Int = 2"))
   }
 
   @Test def defineCaseClass: Unit = {
-    ws"""${m1} case class Foo(x: Int)
-         Foo(1)""".withSource
-      .run(m1, "1:// defined case class Foo",
-                    "2:val res0: Foo = Foo(1)")
+    ws"""${m1}case class Foo(x: Int)${m2}
+         ${m3}Foo(1)${m4}""".withSource
+      .run(m1,
+        ((m1 to m2), "// defined case class Foo"),
+        ((m3 to m4), "val res0: Foo = Foo(1)"))
   }
 
   @Test def defineClass: Unit = {
     ws"""${m1}class Foo(x: Int) {
            override def toString: String = "Foo"
-         }
-         new Foo(1)""".withSource
-      .run(m1, "3:// defined class Foo",
-                    "4:val res0: Foo = Foo")
+         }${m2}
+         ${m3}new Foo(1)${m4}""".withSource
+      .run(m1,
+        ((m1 to m2), "// defined class Foo"),
+        ((m3 to m4), "val res0: Foo = Foo"))
   }
 
   @Test def defineAnonymousClass0: Unit = {
     ws"""${m1}new {
           override def toString: String = "Foo"
-         }""".withSource
-      .run(m1, "3:val res0: Object = Foo")
+         }${m2}""".withSource
+      .run(m1,
+        ((m1 to m2), "val res0: Object = Foo"))
   }
 
   @Test def defineAnonymousClass1: Unit = {
-    ws"""${m1}class Foo
-         trait Bar
-         new Foo with Bar {
+    ws"""${m1}class Foo${m2}
+         ${m3}trait Bar${m4}
+         ${m5}new Foo with Bar {
            override def toString: String = "Foo"
-         }""".withSource
-      .run(m1, "1:// defined class Foo",
-                    "2:// defined trait Bar",
-                    "5:val res0: Foo & Bar = Foo")
+         }${m6}""".withSource
+      .run(m1,
+        ((m1 to m2), "// defined class Foo"),
+        ((m3 to m4), "// defined trait Bar"),
+        ((m5 to m6), "val res0: Foo & Bar = Foo"))
   }
 
   @Test def produceMultilineOutput: Unit = {
-    ws"""${m1}1 to 3 foreach println""".withSource
-      .run(m1, s"1:1${nl}2${nl}3")
+    ws"""${m1}1 to 3 foreach println${m2}""".withSource
+      .run(m1,
+        ((m1 to m2), s"1${nl}2${nl}3"))
   }
 
   @Test def patternMatching0: Unit = {
     ws"""${m1}1 + 2 match {
           case x if x % 2 == 0 => "even"
           case _ => "odd"
-        }""".withSource
-      .run(m1, "4:val res0: String = odd")
+        }${m2}""".withSource
+      .run(m1,
+        ((m1 to m2), "val res0: String = odd"))
   }
 
   @Test def patternMatching1: Unit = {
-    ws"""${m1}val (foo, bar) = (1, 2)""".withSource
-      .run(m1, s"1:val bar: Int = 2${nl}val foo: Int = 1")
+    ws"""${m1}val (foo, bar) = (1, 2)${m2}""".withSource
+      .run(m1,
+        ((m1 to m2), s"val bar: Int = 2${nl}val foo: Int = 1"))
   }
 
   @Test def evaluationException: Unit = {
-    ws"""${m1}val foo = 1 / 0
-         val bar = 2""".withSource
-      .runNonStrict(m1, "1:java.lang.ArithmeticException: / by zero",
-                             "2:val bar: Int = 2")
+    ws"""${m1}val foo = 1 / 0${m2}
+         ${m3}val bar = 2${m4}""".withSource
+      .runNonStrict(m1,
+        ((m1 to m2), "java.lang.ArithmeticException: / by zero"),
+        ((m3 to m4), "val bar: Int = 2"))
   }
 
   @Test def worksheetCompletion(): Unit = {
@@ -204,15 +216,17 @@ class WorksheetTest {
   }
 
   @Test def systemExit(): Unit = {
-    ws"""${m1}println("Hello, world!")
+    ws"""${m1}println("Hello, world!")${m2}
          System.exit(0)
          println("Goodbye!")""".withSource
-      .run(m1, "1:Hello, world!")
+      .run(m1,
+        ((m1 to m2), "Hello, world!"))
   }
 
   @Test def outputOnStdErr(): Unit = {
-    ws"""${m1}System.err.println("Oh no")""".withSource
-      .run(m1, "1:Oh no")
+    ws"""${m1}System.err.println("Oh no")${m2}""".withSource
+      .run(m1,
+        ((m1 to m2), "Oh no"))
   }
 
 }

language-server/test/dotty/tools/languageserver/util/CodeTester.scala

@@ -182,7 +182,7 @@ class CodeTester(projects: List[Project]) {
    *
    * @see dotty.tools.languageserver.util.actions.WorksheetRun
    */
-  def run(marker: CodeMarker, expected: String*): this.type =
+  def run(marker: CodeMarker, expected: (CodeRange, String)*): this.type =
     doAction(new WorksheetRun(marker, expected, strict = true))
 
   /**
@@ -194,7 +194,7 @@ class CodeTester(projects: List[Project]) {
    *
    * @see dotty.tools.languageserver.util.actions.WorksheetRun
    */
-  def runNonStrict(marker: CodeMarker, expected: String*): this.type =
+  def runNonStrict(marker: CodeMarker, expected: (CodeRange, String)*): this.type =
     doAction(new WorksheetRun(marker, expected, strict = false))
 
   /**

language-server/test/dotty/tools/languageserver/util/actions/WorksheetRun.scala

@@ -1,26 +1,27 @@
 package dotty.tools.languageserver.util.actions
 
-import dotty.tools.languageserver.util.PositionContext
+import dotty.tools.languageserver.util.{ CodeRange, PositionContext }
 import dotty.tools.languageserver.util.embedded.CodeMarker
 
 import java.util.concurrent.TimeUnit
 
 import org.junit.Assert.{assertEquals, assertTrue, fail}
 
-class WorksheetRun(marker: CodeMarker, expected: Seq[String], strict: Boolean) extends WorksheetAction {
+class WorksheetRun(marker: CodeMarker, expected: Seq[(CodeRange, String)], strict: Boolean) extends WorksheetAction {
 
   override def execute(): Exec[Unit] = {
     val result = triggerRun(marker).get(30, TimeUnit.SECONDS)
     assertTrue(result.success)
 
-    val logs = worksheetOutput(marker).map(out => s"${out.line}:${out.content}")
+    val logs = worksheetOutput(marker).map(out => (out.range, out.content))
 
-    if (strict) {
-      assertEquals(expected, logs)
-    } else {
-      expected.zip(logs).foreach {
-        case (expected, message) => assertTrue(s"'$message' didn't start with '$expected'", message.startsWith(expected))
-      }
+    expected.zip(logs).foreach {
+      case ((expectedCodeRange, expectedOutput), (actualRange, actualOutput)) =>
+        assertEquals(expectedCodeRange.toRange, actualRange)
+        if (strict)
+          assertEquals(expectedOutput, actualOutput)
+        else
+          assertTrue(s"'$actualOutput' didn't start with '$expectedOutput'", actualOutput.startsWith(expectedOutput))
     }
     client.worksheetOutput.clear()
   }

vscode-dotty/src/protocol.ts

@@ -1,6 +1,6 @@
 import * as vscode from 'vscode'
 import { RequestType, NotificationType } from 'vscode-jsonrpc'
-import { VersionedTextDocumentIdentifier, TextDocumentIdentifier } from 'vscode-languageserver-protocol'
+import { Range, VersionedTextDocumentIdentifier, TextDocumentIdentifier } from 'vscode-languageserver-protocol'
 
 import { client } from './extension'
 
@@ -17,7 +17,13 @@ export interface WorksheetRunResult {
 /** The parameters for the `worksheet/publishOutput` notification. */
 export interface WorksheetPublishOutputParams {
   textDocument: VersionedTextDocumentIdentifier
-  line: number
+  // TODO: remove this property and make `range` non-optional once we
+  // stop supporting Dotty < 0.13.0-RC1
+  /**
+   * @deprecated Use range instead.
+   */
+  line?: number
+  range?: Range
   content: string
 }
 

vscode-dotty/src/worksheet.ts

@@ -99,7 +99,12 @@ class Worksheet implements Disposable {
 
   /** Display the output in the worksheet's editor. */
   handleMessage(output: WorksheetPublishOutputParams, editor: vscode.TextEditor) {
-    this.displayAndSaveResult(output.line - 1, output.content, editor)
+    let range = output.range ?
+      this.client.protocol2CodeConverter.asRange(output.range) :
+      (output.line ?
+        new Range(output.line - 1, 0, output.line - 1, 0) :
+        new Range(0, 0, 0, 0))
+    this.displayAndSaveResult(range, output.content, editor)
   }
 
   /**
@@ -140,22 +145,22 @@ class Worksheet implements Disposable {
    * Parse and display the result of running part of this worksheet. The result is saved so that it
    * can be restored if this buffer is closed.
    *
-   * @param lineNumber The number of the line in the source that produced the result.
+   * @param range      The range in the source that produced the result.
    * @param runResult  The result itself.
    * @param editor     The editor where to display the result.
    */
-  private displayAndSaveResult(lineNumber: number, runResult: string, editor: vscode.TextEditor): void {
+  private displayAndSaveResult(range: Range, runResult: string, editor: vscode.TextEditor): void {
     const resultLines = runResult.split(/\r\n|\r|\n/g)
 
     if (resultLines.length == 0)
       return
 
-    const line = editor.document.lineAt(lineNumber)
+    const lastLine = editor.document.lineAt(range.end.line)
     const decorationOptions = {
-      range: line.range,
+      range: range,
       hoverMessage: new vscode.MarkdownString().appendCodeblock(runResult)
     }
-    const decorationMargin = this.margin - line.text.length
+    const decorationMargin = this.margin - lastLine.text.length
     const decorationText = resultLines[0] + (resultLines.length > 1 ? `<${resultLines.length - 1} lines hidden, hover to see full output>` : "")
     const decorationType = this.createDecoration(decorationMargin, decorationText)
     const decoration = new Decoration(decorationType, decorationOptions)