Address review comments

Author: Duhemm

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

@@ -7,7 +7,7 @@ import java.util.concurrent.CancellationException
 /**
  * Regularly check whether execution has been cancelled, kill REPL if it is.
  */
-private class CancellationThread(private[this] var cancelChecker: CancelChecker,
+private class CancellationThread(@volatile private[this] var cancelChecker: CancelChecker,
                                  evaluator: Evaluator) extends Thread {
   private final val checkCancelledDelayMs = 50
 

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

@@ -9,7 +9,7 @@ import org.eclipse.lsp4j.jsonrpc.CancelChecker
 private object Evaluator {
 
   private val javaExec: Option[String] = {
-    val bin = new File(sys.props("java.home"), "bin")
+    val bin = new File(scala.util.Properties.javaHome, "bin")
     val java = new File(bin, if (scala.util.Properties.isWin) "java.exe" else "java")
 
     if (java.exists()) Some(java.getAbsolutePath())
@@ -56,8 +56,8 @@ private class Evaluator private (javaExec: String,
   private val process =
     new ProcessBuilder(
       javaExec,
-      "-classpath", sys.props("java.class.path"),
-      dotty.tools.repl.WorksheetMain.getClass.getName.init,
+      "-classpath", scala.util.Properties.javaClassPath,
+      dotty.tools.repl.WorksheetMain.getClass.getName.stripSuffix("$"),
       "-classpath", userClasspath,
       "-color:never")
        .redirectErrorStream(true)

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

@@ -1,9 +1,9 @@
 package dotty.tools.languageserver.worksheet
 
-import java.net.URI
+import org.eclipse.lsp4j.VersionedTextDocumentIdentifier
 
 /** The parameter for the `worksheet/exec` request. */
-case class WorksheetExecParams(uri: URI) {
+case class WorksheetExecParams(textDocument: VersionedTextDocumentIdentifier) {
   // Used for deserialization
   // see https://github.com/lampepfl/dotty/pull/5102#discussion_r222055355
   def this() = this(null)
@@ -20,7 +20,7 @@ case class WorksheetExecResponse(success: Boolean) {
  * A notification that tells the client that a line of a worksheet
  * produced the specified output.
  */
-case class WorksheetExecOutput(uri: URI, line: Int, content: String) {
+case class WorksheetExecOutput(textDocument: VersionedTextDocumentIdentifier, line: Int, content: String) {
   // Used for deserialization
   // see https://github.com/lampepfl/dotty/pull/5102#discussion_r222055355
   def this() = this(null, 0, null)

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

@@ -17,12 +17,12 @@ trait WorksheetService { thisServer: DottyLanguageServer =>
 
   @JsonRequest
   def exec(params: WorksheetExecParams): CompletableFuture[WorksheetExecResponse] = thisServer.synchronized {
-    val uri = params.uri
+    val uri = new URI(params.textDocument.getUri)
     val future =
       computeAsync { cancelChecker =>
         try {
           val driver = driverFor(uri)
-          val sendMessage = (line: Int, msg: String) => client.publishOutput(WorksheetExecOutput(uri, line, msg))
+          val sendMessage = (line: Int, msg: String) => client.publishOutput(WorksheetExecOutput(params.textDocument, line, msg))
           evaluateWorksheet(driver, uri, sendMessage, cancelChecker)(driver.currentCtx)
           WorksheetExecResponse(success = true)
         } catch {

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

@@ -6,21 +6,19 @@ import dotty.tools.languageserver.util.embedded.CodeMarker
 import java.net.URI
 import java.util.concurrent.CompletableFuture
 
-abstract class WorksheetAction extends Action {
+import org.eclipse.lsp4j.VersionedTextDocumentIdentifier
 
-  /** Get the URI of the worksheet that contains `marker`. */
-  def getUri(marker: CodeMarker): Exec[URI] = new URI(marker.toTextDocumentIdentifier.getUri)
+abstract class WorksheetAction extends Action {
 
   /** Triggers the evaluation of the worksheet. */
   def triggerEvaluation(marker: CodeMarker): Exec[CompletableFuture[WorksheetExecResponse]] = {
-    val uri = getUri(marker)
-    server.exec(WorksheetExecParams(uri))
+    server.exec(WorksheetExecParams(marker.toVersionedTextDocumentIdentifier))
   }
 
   /** The output of the worksheet that contains `marker`. */
   def worksheetOutput(marker: CodeMarker): Exec[List[WorksheetExecOutput]] = {
-    val uri = getUri(marker)
-    client.worksheetOutput.get.filter(_.uri == uri)
+    val textDocument = marker.toVersionedTextDocumentIdentifier
+    client.worksheetOutput.get.filter(_.textDocument.getUri == textDocument.getUri)
   }
 
 }

language-server/test/dotty/tools/languageserver/util/embedded/CodeMarker.scala

@@ -40,6 +40,9 @@ class CodeMarker(val name: String) extends Embedded {
   def toTextDocumentIdentifier: PosCtx[TextDocumentIdentifier] =
     new TextDocumentIdentifier(file.uri)
 
+  def toVersionedTextDocumentIdentifier: PosCtx[VersionedTextDocumentIdentifier] =
+    new VersionedTextDocumentIdentifier(file.uri, 0)
+
   def toReferenceParams(withDecl: Boolean): PosCtx[ReferenceParams] = {
     val rp = new ReferenceParams(new ReferenceContext(withDecl))
     rp.setTextDocument(toTextDocumentIdentifier)

vscode-dotty/src/worksheet.ts

@@ -1,7 +1,6 @@
 import * as vscode from 'vscode'
-import { LanguageClient } from 'vscode-languageclient'
-import * as rpc from 'vscode-jsonrpc'
 import { client } from './extension'
+import { VersionedTextDocumentIdentifier } from 'vscode-languageserver-protocol'
 
 /** A worksheet managed by vscode */
 class Worksheet {
@@ -59,23 +58,23 @@ class Worksheet {
 }
 
 /** The parameter for the `worksheet/exec` request. */
-class WorksheetExec {
-  constructor(uri: string) {
-    this.uri = uri
+class WorksheetExecParams {
+  constructor(textDocument: vscode.TextDocument) {
+    this.textDocument = VersionedTextDocumentIdentifier.create(textDocument.uri.toString(), textDocument.version)
   }
 
-  readonly uri: string
+  readonly textDocument: VersionedTextDocumentIdentifier
 }
 
 /** The parameter for the `worksheet/publishOutput` notification. */
 class WorksheetOutput {
-  constructor(uri: vscode.Uri, line: number, content: string) {
-    this.uri = uri
+  constructor(textDocument: VersionedTextDocumentIdentifier, line: number, content: string) {
+    this.textDocument = textDocument
     this.line = line
     this.content = content
   }
 
-  readonly uri: vscode.Uri
+  readonly textDocument: VersionedTextDocumentIdentifier
   readonly line: number
   readonly content: string
 }
@@ -136,7 +135,7 @@ export function evaluateWorksheet(document: vscode.TextDocument): Thenable<{}> {
       title: "Evaluating worksheet",
       cancellable: true
     }, (_, token) => {
-      return client.sendRequest("worksheet/exec", new WorksheetExec(worksheet.document.uri.toString()), token)
+      return client.sendRequest("worksheet/exec", new WorksheetExecParams(worksheet.document), token)
     })
   } else {
     return Promise.reject()
@@ -173,8 +172,8 @@ function _prepareWorksheet(worksheet: Worksheet) {
 export function handleMessage(output: WorksheetOutput) {
 
   const editor = vscode.window.visibleTextEditors.find(e => {
-    let uri = e.document.uri
-    return uri == output.uri
+    let uri = e.document.uri.toString()
+    return uri == output.textDocument.uri
   })
 
   if (editor) {
@@ -196,18 +195,15 @@ export function handleMessage(output: WorksheetOutput) {
  * @return a new `TextEditorDecorationType`.
  */
 function worksheetCreateDecoration(margin: number, text: string) {
-  const decorationType =
-    vscode.window.createTextEditorDecorationType({
-      isWholeLine: true,
-      after: {
-        contentText: text,
-        margin: `0px 0px 0px ${margin}ch`,
-        fontStyle: "italic",
-        color: "light gray",
-      }
-    })
-
-  return decorationType
+  return vscode.window.createTextEditorDecorationType({
+    isWholeLine: true,
+    after: {
+      contentText: text,
+      margin: `0px 0px 0px ${margin}ch`,
+      fontStyle: "italic",
+      color: "light gray",
+    }
+  })
 }
 
 /**