From 3a7d2b7ed533221c863ec1135a4a9e58639d32bc Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 12 Sep 2018 09:35:07 +0200 Subject: [PATCH 01/38] Implement worksheets in Dotty IDE --- .../languageserver/DottyLanguageServer.scala | 94 ++++++-- .../tools/languageserver/Worksheet.scala | 57 +++++ vscode-dotty/package.json | 9 +- vscode-dotty/src/extension.ts | 18 +- vscode-dotty/src/worksheet.ts | 205 ++++++++++++++++++ 5 files changed, 362 insertions(+), 21 deletions(-) create mode 100644 language-server/src/dotty/tools/languageserver/Worksheet.scala create mode 100644 vscode-dotty/src/worksheet.ts diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index 29524d5e58bf..ac52d69d5eda 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -179,13 +179,17 @@ class DottyLanguageServer extends LanguageServer val document = params.getTextDocument val uri = new URI(document.getUri) val driver = driverFor(uri) + val worksheetMode = isWorksheet(uri) + + val (text, positionMapper) = + if (worksheetMode) (wrapWorksheet(document.getText), Some(worksheetPositionMapper _)) + else (document.getText, None) - val text = document.getText val diags = driver.run(uri, text) client.publishDiagnostics(new PublishDiagnosticsParams( document.getUri, - diags.flatMap(diagnostic).asJava)) + diags.flatMap(diagnostic(_, positionMapper)).asJava)) } override def didChange(params: DidChangeTextDocumentParams): Unit = thisServer.synchronized { @@ -193,16 +197,20 @@ class DottyLanguageServer extends LanguageServer val document = params.getTextDocument val uri = new URI(document.getUri) val driver = driverFor(uri) + val worksheetMode = isWorksheet(uri) val change = params.getContentChanges.get(0) assert(change.getRange == null, "TextDocumentSyncKind.Incremental support is not implemented") - val text = change.getText + val (text, positionMapper) = + if (worksheetMode) (wrapWorksheet(change.getText), Some(worksheetPositionMapper _)) + else (change.getText, None) + val diags = driver.run(uri, text) client.publishDiagnostics(new PublishDiagnosticsParams( document.getUri, - diags.flatMap(diagnostic).asJava)) + diags.flatMap(diagnostic(_, positionMapper)).asJava)) } override def didClose(params: DidCloseTextDocumentParams): Unit = thisServer.synchronized { @@ -218,9 +226,16 @@ class DottyLanguageServer extends LanguageServer override def didChangeWatchedFiles(params: DidChangeWatchedFilesParams): Unit = /*thisServer.synchronized*/ {} - override def didSave(params: DidSaveTextDocumentParams): Unit = - /*thisServer.synchronized*/ {} - + override def didSave(params: DidSaveTextDocumentParams): Unit = { + thisServer.synchronized { + val uri = new URI(params.getTextDocument.getUri) + if (isWorksheet(uri)) { + val driver = driverFor(uri) + val sendMessage = (msg: String) => client.logMessage(new MessageParams(MessageType.Info, msg)) + evaluateWorksheet(driver, uri, sendMessage)(driver.currentCtx) + } + } + } // FIXME: share code with messages.NotAMember override def completion(params: CompletionParams) = computeAsync { cancelToken => @@ -417,6 +432,20 @@ class DottyLanguageServer extends LanguageServer override def resolveCodeLens(params: CodeLens) = null override def resolveCompletionItem(params: CompletionItem) = null override def signatureHelp(params: TextDocumentPositionParams) = null + + /** + * Evaluate the worksheet at `uri`. + * + * @param driver The driver for the project that contains the worksheet. + * @param uri The URI of the worksheet. + * @param sendMessage A mean of communicating the results of evaluation back. + */ + private def evaluateWorksheet(driver: InteractiveDriver, uri: URI, sendMessage: String => Unit)(implicit ctx: Context): Unit = { + val trees = driver.openedTrees(uri) + trees.headOption.foreach { tree => + Worksheet.evaluate(tree, sendMessage) + } + } } object DottyLanguageServer { @@ -434,21 +463,25 @@ object DottyLanguageServer { } /** Convert a SourcePosition to an lsp4j.Range */ - def range(p: SourcePosition): Option[lsp4j.Range] = - if (p.exists) + def range(p: SourcePosition, positionMapper: Option[SourcePosition => SourcePosition] = None): Option[lsp4j.Range] = + if (p.exists) { + val mappedPosition = positionMapper.map(_(p)).getOrElse(p) Some(new lsp4j.Range( - new lsp4j.Position(p.startLine, p.startColumn), - new lsp4j.Position(p.endLine, p.endColumn) + new lsp4j.Position(mappedPosition.startLine, mappedPosition.startColumn), + new lsp4j.Position(mappedPosition.endLine, mappedPosition.endColumn) )) - else + } else None /** Convert a SourcePosition to an lsp4.Location */ - def location(p: SourcePosition): Option[lsp4j.Location] = - range(p).map(r => new lsp4j.Location(toUri(p.source).toString, r)) + def location(p: SourcePosition, positionMapper: Option[SourcePosition => SourcePosition] = None): Option[lsp4j.Location] = + range(p, positionMapper).map(r => new lsp4j.Location(toUri(p.source).toString, r)) - /** Convert a MessageContainer to an lsp4j.Diagnostic */ - def diagnostic(mc: MessageContainer): Option[lsp4j.Diagnostic] = + /** + * Convert a MessageContainer to an lsp4j.Diagnostic. The positions are transformed vy + * `positionMapper`. + */ + def diagnostic(mc: MessageContainer, positionMapper: Option[SourcePosition => SourcePosition] = None): Option[lsp4j.Diagnostic] = if (!mc.pos.exists) None // diagnostics without positions are not supported: https://github.com/Microsoft/language-server-protocol/issues/249 else { @@ -467,11 +500,38 @@ object DottyLanguageServer { } val code = mc.contained().errorId.errorNumber.toString - range(mc.pos).map(r => + range(mc.pos, positionMapper).map(r => new lsp4j.Diagnostic( r, mc.message, severity(mc.level), /*source =*/ "", code)) } + /** Does this URI represent a worksheet? */ + private def isWorksheet(uri: URI): Boolean = + uri.toString.endsWith(".sc") + + /** Wrap the source of a worksheet inside an `object`. */ + private def wrapWorksheet(source: String): String = + s"""object Worksheet { + |$source + |}""".stripMargin + + /** + * Map `position` in a wrapped worksheet to the same position in the unwrapped source. + * + * Because worksheet are wrapped in an `object`, the positions in the source are one line + * above from what the compiler sees. + * + * @see wrapWorksheet + * @param position The position as seen by the compiler (after wrapping) + * @return The position in the actual source file (before wrapping). + */ + private def worksheetPositionMapper(position: SourcePosition): SourcePosition = { + new SourcePosition(position.source, position.pos, position.outer) { + override def startLine: Int = position.startLine - 1 + override def endLine: Int = position.endLine - 1 + } + } + /** Create an lsp4j.CompletionItem from a Symbol */ def completionItem(sym: Symbol)(implicit ctx: Context): lsp4j.CompletionItem = { def completionItemKind(sym: Symbol)(implicit ctx: Context): lsp4j.CompletionItemKind = { diff --git a/language-server/src/dotty/tools/languageserver/Worksheet.scala b/language-server/src/dotty/tools/languageserver/Worksheet.scala new file mode 100644 index 000000000000..f6565df43b0c --- /dev/null +++ b/language-server/src/dotty/tools/languageserver/Worksheet.scala @@ -0,0 +1,57 @@ +package dotty.tools.languageserver + +import dotty.tools.dotc.ast.tpd.{Template, Tree, TypeDef} +import dotty.tools.dotc.core.Contexts.Context +import dotty.tools.dotc.interactive.SourceTree +import dotty.tools.dotc.util.SourceFile + +import dotty.tools.repl.{ReplDriver, State} + +import java.io.{ByteArrayOutputStream, PrintStream} + +object Worksheet { + + /** + * Evaluate `tree` as a worksheet using the REPL. + * + * @param tree The top level object wrapping the worksheet. + * @param sendMessage A mean of communicating the results of evaluation back. + */ + def evaluate(tree: SourceTree, sendMessage: String => Unit)(implicit ctx: Context): Unit = { + tree.tree match { + case td @ TypeDef(_, template: Template) => + val replOut = new ByteArrayOutputStream + val repl = new ReplDriver(replOptions, out = new PrintStream(replOut)) + + template.body.foldLeft(repl.initialState) { + case (state, statement) => + val (line, newState) = execute(repl, state, statement, tree.source) + val result = new String(replOut.toByteArray()) + sendMessage(encode(result, line)) + replOut.reset() + newState + } + } + } + + /** + * Extract `tree` from the source and evaluate it in the REPL. + * + * @param repl The REPL that will evaluate the worksheet. + * @param state Current state of the REPL. + * @param tree The compiled tree to evaluate. + * @param sourcefile The sourcefile of the worksheet. + * @return The line in the sourcefile that corresponds to `tree` and the new state of the REPL. + */ + private def execute(repl: ReplDriver, state: State, tree: Tree, sourcefile: SourceFile)(implicit ctx: Context): (Int, State) = { + val source = sourcefile.content.slice(tree.pos.start, tree.pos.end).mkString + val line = sourcefile.offsetToLine(tree.pos.end) + (line, repl.run(source)(state)) + } + + private def replOptions(implicit ctx: Context): Array[String] = + Array("-color:never", "-classpath", ctx.settings.classpath.value) + + private def encode(message: String, line: Int): String = + line + ":" + message +} diff --git a/vscode-dotty/package.json b/vscode-dotty/package.json index 0be74047cae7..8ebc3e52a10e 100644 --- a/vscode-dotty/package.json +++ b/vscode-dotty/package.json @@ -25,13 +25,15 @@ "main": "./out/src/extension", "activationEvents": [ "onLanguage:scala", - "workspaceContains:.dotty-ide.json" + "workspaceContains:.dotty-ide.json", + "workspaceContains:**.sc" ], "languages": [ { "id": "scala", "extensions": [ - ".scala" + ".scala", + ".sc" ], "aliases": [ "Scala" @@ -44,6 +46,9 @@ "editor.tabSize": 2, "editor.insertSpaces": true } + }, + "files.associations": { + "*.sc": "scala" } }, "scripts": { diff --git a/vscode-dotty/src/extension.ts b/vscode-dotty/src/extension.ts index 75e2accbbf64..d77939592b08 100644 --- a/vscode-dotty/src/extension.ts +++ b/vscode-dotty/src/extension.ts @@ -12,6 +12,8 @@ import { LanguageClient, LanguageClientOptions, RevealOutputChannelOn, ServerOptions } from 'vscode-languageclient'; import { enableOldServerWorkaround } from './compat' +import * as worksheet from './worksheet' + let extensionContext: ExtensionContext let outputChannel: vscode.OutputChannel @@ -27,6 +29,8 @@ export function activate(context: ExtensionContext) { const languageServerDefaultConfigFile = path.join(extensionContext.extensionPath, './out/default-dotty-ide-config') const coursierPath = path.join(extensionContext.extensionPath, './out/coursier'); + vscode.workspace.onWillSaveTextDocument(worksheet.prepareWorksheet) + if (process.env['DLS_DEV_MODE']) { const portFile = `${vscode.workspace.rootPath}/.dotty-ide-dev-port` fs.readFile(portFile, (err, port) => { @@ -156,8 +160,10 @@ function configureIDE(sbtClasspath: string, languageServerScalaVersion: string, function run(serverOptions: ServerOptions, isOldServer: boolean) { const clientOptions: LanguageClientOptions = { documentSelector: [ - { language: 'scala', scheme: 'file', pattern: '**/*.scala' }, - { language: 'scala', scheme: 'untitled', pattern: '**/*.scala' } + { scheme: 'file', pattern: '**/*.sc' }, + { scheme: 'untitled', pattern: '**/*.sc' }, + { scheme: 'file', pattern: '**/*.scala' }, + { scheme: 'untitled', pattern: '**/*.scala' } ], synchronize: { configurationSection: 'dotty' @@ -170,6 +176,14 @@ function run(serverOptions: ServerOptions, isOldServer: boolean) { if (isOldServer) enableOldServerWorkaround(client) + // We use the `window/logMessage` command to communicate back the result of evaluating + // a worksheet. + client.onReady().then(() => { + client.onNotification("window/logMessage", (params) => { + worksheet.worksheetHandleMessage(params.message) + }) + }) + // Push the disposable to the context's subscriptions so that the // client can be deactivated on extension deactivation extensionContext.subscriptions.push(client.start()); diff --git a/vscode-dotty/src/worksheet.ts b/vscode-dotty/src/worksheet.ts new file mode 100644 index 000000000000..1f0fad523b2a --- /dev/null +++ b/vscode-dotty/src/worksheet.ts @@ -0,0 +1,205 @@ +'use strict' + +import * as vscode from 'vscode' + +/** All decorations that have been added so far */ +let worksheetDecorationTypes: vscode.TextEditorDecorationType[] = [] + +/** The number of blank lines that have been inserted to fit the output so far. */ +let worksheetInsertedLines: number = 0 + +/** The minimum margin to add so that the decoration is shown after all text. */ +let worksheetMargin: number = 0 + +/** + * If the document that will be saved is a worksheet, resets the "worksheet state" + * (margin and number of inserted lines), and removes redundant blank lines that + * have been inserted by a previous evaluation. + * + * The file save operation is blocked until the worksheet is ready to be evaluated. + * + * @param event `TextDocumentWillSaveEvent`. + */ +export function prepareWorksheet(event: vscode.TextDocumentWillSaveEvent) { + const document = event.document + if (document.fileName.endsWith(".sc")) { + const setup = + removeRedundantBlankLines(document) + .then(_ => { + removeDecorations() + worksheetMargin = longestLine(document) + 5 + worksheetInsertedLines = 0 + }) + event.waitUntil(setup) + } +} + +/** + * Handle the result of evaluating part of a worksheet. + * This is called when we receive a `window/logMessage`. + * + * @param message The result of evaluating part of a worksheet. + */ +export function worksheetHandleMessage(message: string) { + + const ed = vscode.window.activeTextEditor + + if (!ed) { + return; + } + + worksheetDisplayResult(message, ed) +} + +/** + * Create a new `TextEditorDecorationType` showing `text`. The decoration + * will appear `margin` characters after the end of the line. + * + * @param margin The margin in characters between the end of the line + * and the decoration. + * @param text The text of the decoration. + * @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", + } + }) + + worksheetDecorationTypes.push(decorationType) + + return decorationType +} + +/** + * Finds the length in characters of the longest line of `document`. + * + * @param document The document to inspect. + * @return The length in characters of the longest line. + */ +function longestLine(document: vscode.TextDocument) { + let maxLength = 0 + const lineCount = document.lineCount + for (let i = 0; i < lineCount; ++i) { + let length = document.lineAt(i).text.length + maxLength = Math.max(maxLength, length) + } + + return maxLength +} + +/** + * Remove all decorations added by worksheet evaluation. + */ +function removeDecorations() { + worksheetDecorationTypes.forEach(decoration => + decoration.dispose() + ) +} + +/** + * Remove the repeated blank lines in the source. + * + * Evaluating a worksheet can insert new lines in the worksheet so that the + * output of a line fits below the line. Before evaluation, we remove blank + * lines in the worksheet to keep its length under control. This could potentially + * remove manually added blank lines. + * + * @param document The document where blank lines must be removed. + * @return A `Thenable` removing the blank lines upon completion. + */ +function removeRedundantBlankLines(document: vscode.TextDocument) { + + const lineCount = document.lineCount + let rangesToRemove: vscode.Range[] = [] + let rangeStart = 0 + let rangeEnd = 0 + let inRange = true + + function addRange() { + inRange = false + if (rangeStart < rangeEnd) { + // Keep one line between separate chunks of code + rangesToRemove.push(new vscode.Range(rangeStart, 0, rangeEnd - 1, 0)) + } + return + } + + for (let i = 0; i < lineCount; ++i) { + const isEmpty = document.lineAt(i).isEmptyOrWhitespace + if (inRange) { + if (isEmpty) rangeEnd += 1 + else addRange() + } else { + if (isEmpty) { + rangeStart = i + rangeEnd = i + 1 + inRange = true + } + } + } + + if (inRange) { + rangeEnd = lineCount + addRange() + } + + return rangesToRemove.reverse().reduce((chain: Thenable, range) => { + return chain.then(_ => { + const edit = new vscode.WorkspaceEdit() + edit.delete(document.uri, range) + return vscode.workspace.applyEdit(edit) + }) + }, Promise.resolve(true)) +} + +/** + * Parse and display the result of evaluating part of a worksheet. + * + * @see worksheetCreateDecoration + * + * @param message The message to parse. + * @param ed The editor where to display the result. + * @return A `Thenable` that will insert necessary lines to fit the output + * and display the decorations upon completion. + */ +function worksheetDisplayResult(message: string, ed: vscode.TextEditor) { + + const colonIndex = message.indexOf(":") + const lineNumber = parseInt(message.slice(0, colonIndex)) - 1 // lines are 0-indexed + const evalResult = message.slice(colonIndex + 1) + const resultLines = evalResult.trim().split(/\r\n|\r|\n/g) + + // The line where the next decoration should be put. + // It's the number of the line that produced the output, plus the number + // of lines that we've inserted so far. + let actualLine = lineNumber + worksheetInsertedLines + + // If the output has more than one line, we need to insert blank lines + // below the line that produced the output to fit the output. + const addNewLinesEdit = new vscode.WorkspaceEdit() + if (resultLines.length > 1) { + const linesToInsert = resultLines.length - 1 + const editPos = new vscode.Position(actualLine + 1, 0) // add after the line + addNewLinesEdit.insert(ed.document.uri, editPos, "\n".repeat(linesToInsert)) + worksheetInsertedLines += linesToInsert + } + + return vscode.workspace.applyEdit(addNewLinesEdit).then(_ => { + for (let line of resultLines) { + const decorationPosition = new vscode.Position(actualLine, 0) + const decorationMargin = worksheetMargin - ed.document.lineAt(actualLine).text.length + const decorationType = worksheetCreateDecoration(decorationMargin, line) + const decoration = { range: new vscode.Range(decorationPosition, decorationPosition), hoverMessage: line } + ed.setDecorations(decorationType, [decoration]) + actualLine += 1 + } + }) +} + From f93399486b1717762dd3a62d30824f53156d2328 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 12 Sep 2018 15:33:31 +0200 Subject: [PATCH 02/38] Support multiple worksheets --- .../languageserver/DottyLanguageServer.scala | 2 +- vscode-dotty/src/worksheet.ts | 55 ++++++++++++------- 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index ac52d69d5eda..f9d203536229 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -231,7 +231,7 @@ class DottyLanguageServer extends LanguageServer val uri = new URI(params.getTextDocument.getUri) if (isWorksheet(uri)) { val driver = driverFor(uri) - val sendMessage = (msg: String) => client.logMessage(new MessageParams(MessageType.Info, msg)) + val sendMessage = (msg: String) => client.logMessage(new MessageParams(MessageType.Info, uri + msg)) evaluateWorksheet(driver, uri, sendMessage)(driver.currentCtx) } } diff --git a/vscode-dotty/src/worksheet.ts b/vscode-dotty/src/worksheet.ts index 1f0fad523b2a..c2ea4312ec24 100644 --- a/vscode-dotty/src/worksheet.ts +++ b/vscode-dotty/src/worksheet.ts @@ -3,13 +3,13 @@ import * as vscode from 'vscode' /** All decorations that have been added so far */ -let worksheetDecorationTypes: vscode.TextEditorDecorationType[] = [] +let worksheetDecorationTypes: Map = new Map() /** The number of blank lines that have been inserted to fit the output so far. */ -let worksheetInsertedLines: number = 0 +let worksheetInsertedLines: Map = new Map() /** The minimum margin to add so that the decoration is shown after all text. */ -let worksheetMargin: number = 0 +let worksheetMargin: Map = new Map() /** * If the document that will be saved is a worksheet, resets the "worksheet state" @@ -26,9 +26,9 @@ export function prepareWorksheet(event: vscode.TextDocumentWillSaveEvent) { const setup = removeRedundantBlankLines(document) .then(_ => { - removeDecorations() - worksheetMargin = longestLine(document) + 5 - worksheetInsertedLines = 0 + removeDecorations(document) + worksheetMargin.set(document, longestLine(document) + 5) + worksheetInsertedLines.set(document, 0) }) event.waitUntil(setup) } @@ -42,13 +42,15 @@ export function prepareWorksheet(event: vscode.TextDocumentWillSaveEvent) { */ export function worksheetHandleMessage(message: string) { - const ed = vscode.window.activeTextEditor + const editor = vscode.window.visibleTextEditors.find(e => { + let uri = e.document.uri.toString() + return uri == message.slice(0, uri.length) + }) - if (!ed) { - return; + if (editor) { + let payload = message.slice(editor.document.uri.toString().length) + worksheetDisplayResult(payload, editor) } - - worksheetDisplayResult(message, ed) } /** @@ -72,8 +74,6 @@ function worksheetCreateDecoration(margin: number, text: string) { } }) - worksheetDecorationTypes.push(decorationType) - return decorationType } @@ -97,8 +97,9 @@ function longestLine(document: vscode.TextDocument) { /** * Remove all decorations added by worksheet evaluation. */ -function removeDecorations() { - worksheetDecorationTypes.forEach(decoration => +function removeDecorations(document: vscode.TextDocument) { + const decorationTypes = worksheetDecorationTypes.get(document) || [] + decorationTypes.forEach(decoration => decoration.dispose() ) } @@ -169,17 +170,26 @@ function removeRedundantBlankLines(document: vscode.TextDocument) { * @return A `Thenable` that will insert necessary lines to fit the output * and display the decorations upon completion. */ -function worksheetDisplayResult(message: string, ed: vscode.TextEditor) { +function worksheetDisplayResult(message: string, editor: vscode.TextEditor) { const colonIndex = message.indexOf(":") const lineNumber = parseInt(message.slice(0, colonIndex)) - 1 // lines are 0-indexed const evalResult = message.slice(colonIndex + 1) const resultLines = evalResult.trim().split(/\r\n|\r|\n/g) + const margin = worksheetMargin.get(editor.document) || 0 + + let insertedLines = worksheetInsertedLines.get(editor.document) || 0 + + let decorationTypes = worksheetDecorationTypes.get(editor.document) + if (!decorationTypes) { + decorationTypes = [] + worksheetDecorationTypes.set(editor.document, decorationTypes) + } // The line where the next decoration should be put. // It's the number of the line that produced the output, plus the number // of lines that we've inserted so far. - let actualLine = lineNumber + worksheetInsertedLines + let actualLine = lineNumber + insertedLines // If the output has more than one line, we need to insert blank lines // below the line that produced the output to fit the output. @@ -187,17 +197,20 @@ function worksheetDisplayResult(message: string, ed: vscode.TextEditor) { if (resultLines.length > 1) { const linesToInsert = resultLines.length - 1 const editPos = new vscode.Position(actualLine + 1, 0) // add after the line - addNewLinesEdit.insert(ed.document.uri, editPos, "\n".repeat(linesToInsert)) - worksheetInsertedLines += linesToInsert + addNewLinesEdit.insert(editor.document.uri, editPos, "\n".repeat(linesToInsert)) + insertedLines += linesToInsert + worksheetInsertedLines.set(editor.document, insertedLines) } return vscode.workspace.applyEdit(addNewLinesEdit).then(_ => { for (let line of resultLines) { const decorationPosition = new vscode.Position(actualLine, 0) - const decorationMargin = worksheetMargin - ed.document.lineAt(actualLine).text.length + const decorationMargin = margin - editor.document.lineAt(actualLine).text.length const decorationType = worksheetCreateDecoration(decorationMargin, line) + if (decorationTypes) decorationTypes.push(decorationType) + const decoration = { range: new vscode.Range(decorationPosition, decorationPosition), hoverMessage: line } - ed.setDecorations(decorationType, [decoration]) + editor.setDecorations(decorationType, [decoration]) actualLine += 1 } }) From 5fae035bbe9da20a26ca81654981ff9106205981 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Thu, 13 Sep 2018 15:43:06 +0200 Subject: [PATCH 03/38] Show worksheet evaluation progress Also, add a command to evaluate worksheet: "Run worksheet" --- .../languageserver/DottyLanguageServer.scala | 1 + vscode-dotty/package.json | 6 + vscode-dotty/src/extension.ts | 16 ++- vscode-dotty/src/worksheet.ts | 111 +++++++++++++++++- 4 files changed, 128 insertions(+), 6 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index f9d203536229..5500969b4718 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -233,6 +233,7 @@ class DottyLanguageServer extends LanguageServer val driver = driverFor(uri) val sendMessage = (msg: String) => client.logMessage(new MessageParams(MessageType.Info, uri + msg)) evaluateWorksheet(driver, uri, sendMessage)(driver.currentCtx) + sendMessage("FINISHED") } } } diff --git a/vscode-dotty/package.json b/vscode-dotty/package.json index 8ebc3e52a10e..ef0710fafd43 100644 --- a/vscode-dotty/package.json +++ b/vscode-dotty/package.json @@ -41,6 +41,12 @@ } ], "contributes": { + "commands": [ + { + "command": "worksheet.evaluate", + "title": "Run worksheet" + } + ], "configurationDefaults": { "[scala]": { "editor.tabSize": 2, diff --git a/vscode-dotty/src/extension.ts b/vscode-dotty/src/extension.ts index d77939592b08..252ec92817ca 100644 --- a/vscode-dotty/src/extension.ts +++ b/vscode-dotty/src/extension.ts @@ -16,6 +16,7 @@ import * as worksheet from './worksheet' let extensionContext: ExtensionContext let outputChannel: vscode.OutputChannel +let client: LanguageClient export function activate(context: ExtensionContext) { extensionContext = context @@ -30,6 +31,15 @@ export function activate(context: ExtensionContext) { const coursierPath = path.join(extensionContext.extensionPath, './out/coursier'); vscode.workspace.onWillSaveTextDocument(worksheet.prepareWorksheet) + vscode.workspace.onDidSaveTextDocument(document => { + if (worksheet.isWorksheet(document)) { + vscode.commands.executeCommand(worksheet.worksheetEvaluateAfterSaveKey) + } + }) + + vscode.commands.registerCommand(worksheet.worksheetEvaluateAfterSaveKey, () => { + worksheet.evaluateCommand() + }) if (process.env['DLS_DEV_MODE']) { const portFile = `${vscode.workspace.rootPath}/.dotty-ide-dev-port` @@ -172,7 +182,7 @@ function run(serverOptions: ServerOptions, isOldServer: boolean) { revealOutputChannelOn: RevealOutputChannelOn.Never } - const client = new LanguageClient("dotty", "Dotty", serverOptions, clientOptions) + client = new LanguageClient("dotty", "Dotty", serverOptions, clientOptions) if (isOldServer) enableOldServerWorkaround(client) @@ -184,6 +194,10 @@ function run(serverOptions: ServerOptions, isOldServer: boolean) { }) }) + vscode.commands.registerCommand(worksheet.worksheetEvaluateKey, () => { + worksheet.worksheetSave(client) + }) + // Push the disposable to the context's subscriptions so that the // client can be deactivated on extension deactivation extensionContext.subscriptions.push(client.start()); diff --git a/vscode-dotty/src/worksheet.ts b/vscode-dotty/src/worksheet.ts index c2ea4312ec24..d8c99778c39e 100644 --- a/vscode-dotty/src/worksheet.ts +++ b/vscode-dotty/src/worksheet.ts @@ -1,6 +1,7 @@ 'use strict' import * as vscode from 'vscode' +import { LanguageClient } from 'vscode-languageclient' /** All decorations that have been added so far */ let worksheetDecorationTypes: Map = new Map() @@ -11,6 +12,75 @@ let worksheetInsertedLines: Map = new Map = new Map() +/** Whether the given worksheet has finished evaluating. */ +let worksheetFinished: Map = new Map() + +/** + * The command key for evaluating a worksheet. Exposed to users as + * `Run worksheet`. + */ +export const worksheetEvaluateKey = "worksheet.evaluate" + +/** + * The command that is called to evaluate a worksheet after it has been evaluated. + * + * This is not exposed as a standalone callable command; but this command is triggered + * when a worksheet is saved. + */ +export const worksheetEvaluateAfterSaveKey = "worksheet.evaluateAfterSave" + +/** Is this document a worksheet? */ +export function isWorksheet(document: vscode.TextDocument): boolean { + return document.fileName.endsWith(".sc") +} + +/** + * This command is bound to `worksheetEvaluateAfterSaveKey`. This is implemented + * as a command, because we want to display a progress bar that may stay for a while, and + * VSCode will kill promises triggered by file save after some time. + */ +export function evaluateCommand() { + const editor = vscode.window.activeTextEditor + if (editor) { + const document = editor.document + if (isWorksheet(document)) { + showWorksheetProgress(document) + } + } +} + +/** + * The VSCode command executed when the user select `Run worksheet`. + * + * We check whether the buffer is dirty, and if it is, we save it. Evaluation will then be + * triggered by file save. + * If the buffer is clean, we do the necessary preparation for worksheet (compute margin, + * remove blank lines, etc.) and check if the buffer has been changed by that. If it is, we save + * and the evaluation will be triggered by file save. + * If the buffer is still clean, we send a `textDocument/didSave` notification to the language + * server in order to start the execution of the worksheet. + */ +export function worksheetSave(client: LanguageClient) { + const editor = vscode.window.activeTextEditor + if (editor) { + const document = editor.document + if (isWorksheet(document)) { + if (document.isDirty) document.save() + else { + _prepareWorksheet(document).then(_ => { + if (document.isDirty) document.save() + else { + client.sendNotification("textDocument/didSave", { + textDocument: { uri: document.uri.toString() } + }) + showWorksheetProgress(document) + } + }) + } + } + } +} + /** * If the document that will be saved is a worksheet, resets the "worksheet state" * (margin and number of inserted lines), and removes redundant blank lines that @@ -22,18 +92,45 @@ let worksheetMargin: Map = new Map { removeDecorations(document) worksheetMargin.set(document, longestLine(document) + 5) worksheetInsertedLines.set(document, 0) + worksheetFinished.set(document, false) }) - event.waitUntil(setup) + } else { + return Promise.resolve() } } +function showWorksheetProgress(document: vscode.TextDocument) { + return vscode.window.withProgress({ + location: vscode.ProgressLocation.Window, + title: "Evaluating worksheet" + }, _ => { + function isFinished() { + return worksheetFinished.get(document) || false + } + return wait(isFinished, 500) + }) +} + +/** Wait until `cond` evaluates to true; test every `delay` ms. */ +function wait(cond: () => boolean, delayMs: number): Promise { + const isFinished = cond() + if (isFinished) { + return Promise.resolve(true) + } + else return new Promise(fn => setTimeout(fn, delayMs)).then(_ => wait(cond, delayMs)) +} + /** * Handle the result of evaluating part of a worksheet. * This is called when we receive a `window/logMessage`. @@ -49,7 +146,11 @@ export function worksheetHandleMessage(message: string) { if (editor) { let payload = message.slice(editor.document.uri.toString().length) - worksheetDisplayResult(payload, editor) + if (payload == "FINISHED") { + worksheetFinished.set(editor.document, true) + } else { + worksheetDisplayResult(payload, editor) + } } } From 04e40f4e530693880d9b3cc0ec3c926aaf4e17f5 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Thu, 13 Sep 2018 16:32:11 +0200 Subject: [PATCH 04/38] Fix position in constructor call --- compiler/src/dotty/tools/dotc/parsing/Parsers.scala | 3 ++- compiler/test-resources/repl/errmsgs | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index 2a39323c5ee5..2588b9c44691 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -1404,7 +1404,8 @@ object Parsers { val (impl, missingBody) = template(emptyConstructor) impl.parents match { case parent :: Nil if missingBody => - if (parent.isType) ensureApplied(wrapNew(parent)) else parent + if (parent.isType) ensureApplied(wrapNew(parent)) + else parent.withPos(Position(start, in.lastOffset)) case _ => New(impl.withPos(Position(start, in.lastOffset))) } diff --git a/compiler/test-resources/repl/errmsgs b/compiler/test-resources/repl/errmsgs index 157e5518b190..aef05a07c103 100644 --- a/compiler/test-resources/repl/errmsgs +++ b/compiler/test-resources/repl/errmsgs @@ -25,9 +25,9 @@ scala> val z: (List[String], List[Int]) = (List(1), List("a")) | scala> val a: Inv[String] = new Inv(new Inv(1)) 1 | val a: Inv[String] = new Inv(new Inv(1)) - | ^^^^^^ - | found: Inv[Int] - | required: String + | ^^^^^^^^^^ + | found: Inv[Int] + | required: String | scala> val b: Inv[String] = new Inv(1) 1 | val b: Inv[String] = new Inv(1) From 844a105bc0577ce258b84225310de97385cf34c6 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Thu, 13 Sep 2018 17:08:01 +0200 Subject: [PATCH 05/38] Add tests for worksheets --- .../tools/languageserver/Worksheet.scala | 18 +++- .../tools/languageserver/WorksheetTest.scala | 85 +++++++++++++++++++ .../tools/languageserver/util/Code.scala | 47 ++++++++-- .../languageserver/util/CodeTester.scala | 12 ++- .../languageserver/util/actions/Action.scala | 7 +- .../util/actions/WorksheetEvaluate.scala | 32 +++++++ .../util/server/TestClient.scala | 27 ++++-- .../util/server/TestServer.scala | 4 +- 8 files changed, 207 insertions(+), 25 deletions(-) create mode 100644 language-server/test/dotty/tools/languageserver/WorksheetTest.scala create mode 100644 language-server/test/dotty/tools/languageserver/util/actions/WorksheetEvaluate.scala diff --git a/language-server/src/dotty/tools/languageserver/Worksheet.scala b/language-server/src/dotty/tools/languageserver/Worksheet.scala index f6565df43b0c..a9e1186c5957 100644 --- a/language-server/src/dotty/tools/languageserver/Worksheet.scala +++ b/language-server/src/dotty/tools/languageserver/Worksheet.scala @@ -1,10 +1,13 @@ package dotty.tools.languageserver -import dotty.tools.dotc.ast.tpd.{Template, Tree, TypeDef} +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.Positions.Position import dotty.tools.dotc.util.SourceFile +import dotty.tools.dotc.core.Flags.Synthetic + import dotty.tools.repl.{ReplDriver, State} import java.io.{ByteArrayOutputStream, PrintStream} @@ -22,14 +25,21 @@ object Worksheet { case td @ TypeDef(_, template: Template) => val replOut = new ByteArrayOutputStream val repl = new ReplDriver(replOptions, out = new PrintStream(replOut)) + val executed = collection.mutable.Set.empty[(Int, Int)] template.body.foldLeft(repl.initialState) { - case (state, statement) => + case (state, statement: DefTree) if statement.symbol.is(Synthetic) => + state + + case (state, statement) if executed.add(bounds(statement.pos)) => val (line, newState) = execute(repl, state, statement, tree.source) val result = new String(replOut.toByteArray()) - sendMessage(encode(result, line)) + if (result.trim.nonEmpty) sendMessage(encode(result, line)) replOut.reset() newState + + case (state, statement) => + state } } } @@ -54,4 +64,6 @@ object Worksheet { private def encode(message: String, line: Int): String = line + ":" + message + + private def bounds(pos: Position): (Int, Int) = (pos.start, pos.end) } diff --git a/language-server/test/dotty/tools/languageserver/WorksheetTest.scala b/language-server/test/dotty/tools/languageserver/WorksheetTest.scala new file mode 100644 index 000000000000..c47016a7e2f1 --- /dev/null +++ b/language-server/test/dotty/tools/languageserver/WorksheetTest.scala @@ -0,0 +1,85 @@ +package dotty.tools.languageserver + +import org.junit.Test + +import dotty.tools.languageserver.util.Code._ + +class WorksheetTest { + + @Test def evaluateExpression: Unit = { + ws"${m1}2 + 2".withSource + .evaluate(m1, "1:val res0: Int = 4") + } + + @Test def evaluateSimpleVal: Unit = { + ws"${m1}val foo = 123".withSource + .evaluate(m1, "1:val foo: Int = 123") + } + + @Test def usePreviousDefinition: Unit = { + ws"""${m1}val foo = 123 + val bar = foo + 1""".withSource + .evaluate(m1, "1:val foo: Int = 123", + "2:val bar: Int = 124") + } + + @Test def defineObject: Unit = { + ws"""${m1}def foo(x: Int) = x + 1 + foo(1)""".withSource + .evaluate(m1, "1:def foo(x: Int): Int", + "2:val res0: Int = 2") + } + + @Test def defineCaseClass: Unit = { + ws"""${m1} case class Foo(x: Int) + Foo(1)""".withSource + .evaluate(m1, "1:// defined case class Foo", + "2:val res0: Foo = Foo(1)") + } + + @Test def defineClass: Unit = { + ws"""${m1}class Foo(x: Int) { + override def toString: String = "Foo" + } + new Foo(1)""".withSource + .evaluate(m1, "3:// defined class Foo", + "4:val res0: Foo = Foo") + } + + @Test def defineAnonymousClass0: Unit = { + ws"""${m1}new { + override def toString: String = "Foo" + }""".withSource + .evaluate(m1, "3:val res0: Object = Foo") + } + + @Test def defineAnonymousClass1: Unit = { + ws"""${m1}class Foo + trait Bar + new Foo with Bar { + override def toString: String = "Foo" + }""".withSource + .evaluate(m1, "1:// defined class Foo", + "2:// defined trait Bar", + "5:val res0: Foo & Bar = Foo") + } + + @Test def produceMultilineOutput: Unit = { + ws"""${m1}1 to 3 foreach println""".withSource + .evaluate(m1, "1:1\n2\n3") + } + + @Test def patternMatching0: Unit = { + ws"""${m1}1 + 2 match { + case x if x % 2 == 0 => "even" + case _ => "odd" + }""".withSource + .evaluate(m1, "4:val res0: String = odd") + } + + @Test def patternMatching1: Unit = { + ws"""${m1}val (foo, bar) = (1, 2)""".withSource + .evaluate(m1, "1:val foo: Int = 1\nval bar: Int = 2") + } + +} diff --git a/language-server/test/dotty/tools/languageserver/util/Code.scala b/language-server/test/dotty/tools/languageserver/util/Code.scala index dc43e43c57d8..3d3899dae7fa 100644 --- a/language-server/test/dotty/tools/languageserver/util/Code.scala +++ b/language-server/test/dotty/tools/languageserver/util/Code.scala @@ -35,7 +35,22 @@ object Code { * and `m3` and `m4` enclose the identifier `Hello`. These positions can then be used to ask to * perform actions such as finding all references, etc. */ - def code(args: Embedded*): SourceWithPositions = { + def code(args: Embedded*): ScalaSourceWithPositions = { + val (text, positions) = textAndPositions(args: _*) + ScalaSourceWithPositions(text, positions) + } + + /** + * An interpolator similar to `code`, but used for defining a worksheet. + * + * @see code + */ + def ws(args: Embedded*): WorksheetWithPositions = { + val (text, positions) = textAndPositions(args: _*) + WorksheetWithPositions(text, positions) + } + + private def textAndPositions(args: Embedded*): (String, List[(CodeMarker, Int, Int)]) = { val pi = sc.parts.iterator val ai = args.iterator @@ -70,22 +85,40 @@ object Code { if (pi.hasNext) stringBuilder.append(pi.next()) - SourceWithPositions(stringBuilder.result(), positions.result()) + (stringBuilder.result(), positions.result()) } } /** A new `CodeTester` working with `sources` in the workspace. */ def withSources(sources: SourceWithPositions*): CodeTester = new CodeTester(sources.toList, Nil) + sealed trait SourceWithPositions { + + /** The code contained within the virtual source file. */ + def text: String + + /** The positions of the markers that have been set. */ + def positions: List[(CodeMarker, Int, Int)] + + /** A new `CodeTester` with only this source in the workspace. */ + def withSource: CodeTester = new CodeTester(this :: Nil, Nil) + + } + /** - * A virtual source file where several markers have been set. + * A virtual Scala source file where several markers have been set. * * @param text The code contained within the virtual source file. * @param positions The positions of the markers that have been set. */ - case class SourceWithPositions(text: String, positions: List[(CodeMarker, Int, Int)]) { - /** A new `CodeTester` with only this source in the workspace. */ - def withSource: CodeTester = new CodeTester(this :: Nil, Nil) - } + case class ScalaSourceWithPositions(text: String, positions: List[(CodeMarker, Int, Int)]) extends SourceWithPositions + + /** + * A virtual worksheet where several markers have been set. + * + * @param text The code contained within the virtual source file. + * @param positions The positions of the markers that have been set. + */ + case class WorksheetWithPositions(text: String, positions: List[(CodeMarker, Int, Int)]) extends SourceWithPositions } diff --git a/language-server/test/dotty/tools/languageserver/util/CodeTester.scala b/language-server/test/dotty/tools/languageserver/util/CodeTester.scala index 6e6d7cdeefa9..feeea8a47f9e 100644 --- a/language-server/test/dotty/tools/languageserver/util/CodeTester.scala +++ b/language-server/test/dotty/tools/languageserver/util/CodeTester.scala @@ -1,6 +1,6 @@ package dotty.tools.languageserver.util -import dotty.tools.languageserver.util.Code.SourceWithPositions +import dotty.tools.languageserver.util.Code._ import dotty.tools.languageserver.util.actions._ import dotty.tools.languageserver.util.embedded.CodeMarker import dotty.tools.languageserver.util.server.{TestFile, TestServer} @@ -16,8 +16,9 @@ class CodeTester(sources: List[SourceWithPositions], actions: List[Action]) { private val testServer = new TestServer(TestFile.testDir) - private val files = sources.zipWithIndex.map { case (code, i) => - testServer.openCode(code.text, s"Source$i.scala") + private val files = sources.zipWithIndex.map { + case (ScalaSourceWithPositions(text, _), i) => testServer.openCode(text, s"Source$i.scala") + case (WorksheetWithPositions(text, _), i) => testServer.openCode(text, s"Worksheet$i.sc") } private val positions: PositionContext = getPositions(files) @@ -116,9 +117,12 @@ class CodeTester(sources: List[SourceWithPositions], actions: List[Action]) { def symbol(query: String, symbols: SymInfo*): this.type = doAction(new CodeSymbol(query, symbols)) + def evaluate(marker: CodeMarker, expected: String*): this.type = + doAction(new WorksheetEvaluate(marker, expected)) + private def doAction(action: Action): this.type = { try { - action.execute()(testServer, positions) + action.execute()(testServer, testServer.client, positions) } catch { case ex: AssertionError => val sourcesStr = sources.zip(files).map{ case (source, file) => "// " + file.file + "\n" + source.text}.mkString("\n") diff --git a/language-server/test/dotty/tools/languageserver/util/actions/Action.scala b/language-server/test/dotty/tools/languageserver/util/actions/Action.scala index 06d0bed35a53..f4654c763ca7 100644 --- a/language-server/test/dotty/tools/languageserver/util/actions/Action.scala +++ b/language-server/test/dotty/tools/languageserver/util/actions/Action.scala @@ -2,7 +2,7 @@ package dotty.tools.languageserver.util.actions import dotty.tools.languageserver.DottyLanguageServer import dotty.tools.languageserver.util.PositionContext -import dotty.tools.languageserver.util.server.TestServer +import dotty.tools.languageserver.util.server.{TestClient, TestServer} import PositionContext._ @@ -11,7 +11,7 @@ import PositionContext._ * definition, etc.) */ trait Action { - type Exec[T] = implicit (TestServer, PositionContext) => T + type Exec[T] = implicit (TestServer, TestClient, PositionContext) => T /** Execute the action. */ def execute(): Exec[Unit] @@ -22,4 +22,7 @@ trait Action { /** The server that this action targets. */ def server: Exec[DottyLanguageServer] = implicitly[TestServer].server + /** The client that executes this action. */ + def client: Exec[TestClient] = implicitly[TestClient] + } diff --git a/language-server/test/dotty/tools/languageserver/util/actions/WorksheetEvaluate.scala b/language-server/test/dotty/tools/languageserver/util/actions/WorksheetEvaluate.scala new file mode 100644 index 000000000000..6c65ea23ceb4 --- /dev/null +++ b/language-server/test/dotty/tools/languageserver/util/actions/WorksheetEvaluate.scala @@ -0,0 +1,32 @@ +package dotty.tools.languageserver.util.actions + +import dotty.tools.languageserver.util.{PositionContext} +import dotty.tools.languageserver.util.embedded.CodeMarker + +import org.eclipse.lsp4j.{DidSaveTextDocumentParams, MessageParams, MessageType} + +import org.junit.Assert.assertEquals + +class WorksheetEvaluate(marker: CodeMarker, expected: Seq[String]) extends Action { + override def execute(): Exec[Unit] = { + val file = marker.toTextDocumentIdentifier + server.didSave(new DidSaveTextDocumentParams(file)) + + while (!getLogs(marker).contains("FINISHED")) Thread.sleep(100) + + assertEquals(expected, getLogs(marker).init) + client.log.clear() + } + + override def show: PositionContext.PosCtx[String] = + s"WorksheetEvaluate(${marker.file}, ${expected})" + + private def getLogs(marker: CodeMarker): Exec[List[String]] = { + def matches(params: MessageParams): Boolean = + params.getType == MessageType.Info && params.getMessage.startsWith(marker.file.uri) + client.log.get.collect { + case params: MessageParams if matches(params) => + params.getMessage.substring(marker.file.uri.length).trim + } + } +} diff --git a/language-server/test/dotty/tools/languageserver/util/server/TestClient.scala b/language-server/test/dotty/tools/languageserver/util/server/TestClient.scala index c900aadf05f0..b76307851329 100644 --- a/language-server/test/dotty/tools/languageserver/util/server/TestClient.scala +++ b/language-server/test/dotty/tools/languageserver/util/server/TestClient.scala @@ -5,31 +5,42 @@ import java.util.concurrent.CompletableFuture import org.eclipse.lsp4j._ import org.eclipse.lsp4j.services._ +import scala.collection.mutable.Buffer + class TestClient extends LanguageClient { - private val log = new StringBuilder + class Log[T] { + private[this] val log = Buffer.empty[T] + + def +=(elem: T): this.type = { log += elem; this } + def get: List[T] = log.toList + def clear(): Unit = log.clear() + } + + val log = new Log[MessageParams] + val diagnostics = new Log[PublishDiagnosticsParams] + val telemetry = new Log[Any] - def getLog: String = log.result() override def logMessage(message: MessageParams) = { - log.append(message.toString) + log += message } override def showMessage(messageParams: MessageParams) = { - log.append(messageParams.toString) + log += messageParams } override def telemetryEvent(obj: scala.Any) = { - log.append(obj.toString) + telemetry += obj } override def showMessageRequest(requestParams: ShowMessageRequestParams) = { - log.append(requestParams.toString) + log += requestParams new CompletableFuture[MessageActionItem] } - override def publishDiagnostics(diagnostics: PublishDiagnosticsParams) = { - log.append(diagnostics.toString) + override def publishDiagnostics(diagnosticsParams: PublishDiagnosticsParams) = { + diagnostics += diagnosticsParams } } diff --git a/language-server/test/dotty/tools/languageserver/util/server/TestServer.scala b/language-server/test/dotty/tools/languageserver/util/server/TestServer.scala index 3c439de6044a..6859b9e2d777 100644 --- a/language-server/test/dotty/tools/languageserver/util/server/TestServer.scala +++ b/language-server/test/dotty/tools/languageserver/util/server/TestServer.scala @@ -11,6 +11,8 @@ import org.eclipse.lsp4j.{ DidOpenTextDocumentParams, InitializeParams, Initiali class TestServer(testFolder: Path) { val server = new DottyLanguageServer + var client: TestClient = _ + init() private[this] def init(): InitializeResult = { @@ -39,7 +41,7 @@ class TestServer(testFolder: Path) { close() } - val client = new TestClient + client = new TestClient server.connect(client) val initParams = new InitializeParams() From cb6b40f08caeb120e5f715795695bc59c44f8c31 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Fri, 14 Sep 2018 18:51:09 +0200 Subject: [PATCH 06/38] Consider `.sc` files as belonging to Scala The `languages` section has to be in the `contributes` section. --- vscode-dotty/package.json | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/vscode-dotty/package.json b/vscode-dotty/package.json index ef0710fafd43..ef54a1a1875c 100644 --- a/vscode-dotty/package.json +++ b/vscode-dotty/package.json @@ -25,22 +25,21 @@ "main": "./out/src/extension", "activationEvents": [ "onLanguage:scala", - "workspaceContains:.dotty-ide.json", - "workspaceContains:**.sc" - ], - "languages": [ - { - "id": "scala", - "extensions": [ - ".scala", - ".sc" - ], - "aliases": [ - "Scala" - ] - } + "workspaceContains:.dotty-ide.json" ], "contributes": { + "languages": [ + { + "id": "scala", + "extensions": [ + ".scala", + ".sc" + ], + "aliases": [ + "Scala" + ] + } + ], "commands": [ { "command": "worksheet.evaluate", From 27d47337c824b88107f2ef86576fb4605f4c19b7 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Sat, 15 Sep 2018 15:55:45 +0200 Subject: [PATCH 07/38] Log sbt output, and close stdin so sbt doesn't block on error --- vscode-dotty/src/extension.ts | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/vscode-dotty/src/extension.ts b/vscode-dotty/src/extension.ts index 252ec92817ca..20925a0dcdec 100644 --- a/vscode-dotty/src/extension.ts +++ b/vscode-dotty/src/extension.ts @@ -45,7 +45,7 @@ export function activate(context: ExtensionContext) { const portFile = `${vscode.workspace.rootPath}/.dotty-ide-dev-port` fs.readFile(portFile, (err, port) => { if (err) { - outputChannel.append(`Unable to parse ${portFile}`) + outputChannel.appendLine(`Unable to parse ${portFile}`) throw err } @@ -125,11 +125,15 @@ function fetchWithCoursier(coursierPath: string, artifact: string, extra: string coursierProc.stdout.on('data', (data: Buffer) => { classPath += data.toString().trim() }) + coursierProc.stderr.on('data', (data: Buffer) => { + let msg = data.toString().trim() + outputChannel.appendLine(msg) + }) coursierProc.on('close', (code: number) => { if (code != 0) { let msg = `Couldn't fetch '${ artifact }' (exit code ${ code }).` - outputChannel.append(msg) + outputChannel.appendLine(msg) throw new Error(msg) } }) @@ -147,6 +151,7 @@ function configureIDE(sbtClasspath: string, languageServerScalaVersion: string, // eventually run `configureIDE`. const sbtPromise = cpp.spawn("java", [ + "-Dsbt.log.noformat=true", "-classpath", sbtClasspath, "xsbt.boot.Boot", `--addPluginSbtFile=${dottyPluginSbtFile}`, @@ -155,10 +160,23 @@ function configureIDE(sbtClasspath: string, languageServerScalaVersion: string, ]) const sbtProc = sbtPromise.childProcess + // Close stdin, otherwise in case of error sbt will block waiting for the + // user input to reload or exit the build. + sbtProc.stdin.end() + + sbtProc.stdout.on('data', (data: Buffer) => { + let msg = data.toString().trim() + outputChannel.appendLine(msg) + }) + sbtProc.stderr.on('data', (data: Buffer) => { + let msg = data.toString().trim() + outputChannel.appendLine(msg) + }) + sbtProc.on('close', (code: number) => { if (code != 0) { const msg = "Configuring the IDE failed." - outputChannel.append(msg) + outputChannel.appendLine(msg) throw new Error(msg) } }) From ace1d5b6b8dbddd7e470de6b4c94b3a0d97f5b1f Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Sat, 15 Sep 2018 17:24:09 +0200 Subject: [PATCH 08/38] configureIDE: write projects to .dotty-ide.json even if they're empty Otherwise the Dotty Language Server cannot be properly started on a project without any source file. Also turn off the warning when we choose an arbitrary compiler instance in the DLS to avoid spamming the logs of our users. --- .../tools/languageserver/DottyLanguageServer.scala | 2 +- .../src/dotty/tools/sbtplugin/DottyIDEPlugin.scala | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index 5500969b4718..7367135368b8 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -113,7 +113,7 @@ class DottyLanguageServer extends LanguageServer drivers(config) case None => val config = drivers.keys.head - println(s"No configuration contains $uri as a source file, arbitrarily choosing ${config.id}") + // println(s"No configuration contains $uri as a source file, arbitrarily choosing ${config.id}") drivers(config) } } diff --git a/sbt-dotty/src/dotty/tools/sbtplugin/DottyIDEPlugin.scala b/sbt-dotty/src/dotty/tools/sbtplugin/DottyIDEPlugin.scala index d423b07af9a4..4d5dd8348fd8 100644 --- a/sbt-dotty/src/dotty/tools/sbtplugin/DottyIDEPlugin.scala +++ b/sbt-dotty/src/dotty/tools/sbtplugin/DottyIDEPlugin.scala @@ -270,7 +270,15 @@ object DottyIDEPlugin extends AutoPlugin { } private def projectConfigTask(config: Configuration): Initialize[Task[Option[ProjectConfig]]] = Def.taskDyn { - if ((sources in config).value.isEmpty) Def.task { None } + val depClasspath = Attributed.data((dependencyClasspath in config).value) + + // Try to detect if this is a real Scala project or not. This is pretty + // fragile because sbt simply does not keep track of this information. We + // could check if at least one source file ends with ".scala" but that + // doesn't work for empty projects. + val isScalaProject = depClasspath.exists(_.getAbsolutePath.contains("dotty-library")) && depClasspath.exists(_.getAbsolutePath.contains("scala-library")) + + if (!isScalaProject) Def.task { None } else Def.task { // Not needed to generate the config, but this guarantees that the // generated config is usable by an IDE without any extra compilation @@ -281,7 +289,6 @@ object DottyIDEPlugin extends AutoPlugin { val compilerVersion = (scalaVersion in config).value val compilerArguments = (scalacOptions in config).value val sourceDirectories = (unmanagedSourceDirectories in config).value ++ (managedSourceDirectories in config).value - val depClasspath = Attributed.data((dependencyClasspath in config).value) val classDir = (classDirectory in config).value Some(new ProjectConfig( From a91712906b6162fb72565ae399ce7ad12bd65d2a Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Tue, 18 Sep 2018 11:55:25 +0200 Subject: [PATCH 09/38] Support cancelling worksheet evaluation --- .../languageserver/DottyLanguageServer.scala | 40 ++++++++++++----- .../tools/languageserver/Worksheet.scala | 43 +++++++++++++++---- vscode-dotty/package.json | 4 ++ vscode-dotty/src/extension.ts | 4 ++ vscode-dotty/src/worksheet.ts | 33 ++++++++++++-- 5 files changed, 102 insertions(+), 22 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index 7367135368b8..92ea5d92b6ec 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -54,6 +54,7 @@ class DottyLanguageServer extends LanguageServer private[this] var rootUri: String = _ private[this] var client: LanguageClient = _ + private[this] val worksheets: mutable.Map[URI, CompletableFuture[_]] = mutable.Map.empty private[this] var myDrivers: mutable.Map[ProjectConfig, InteractiveDriver] = _ @@ -227,13 +228,25 @@ class DottyLanguageServer extends LanguageServer /*thisServer.synchronized*/ {} override def didSave(params: DidSaveTextDocumentParams): Unit = { - thisServer.synchronized { - val uri = new URI(params.getTextDocument.getUri) - if (isWorksheet(uri)) { - val driver = driverFor(uri) + val uri = new URI(params.getTextDocument.getUri) + if (isWorksheet(uri)) { + if (uri.getScheme == "cancel") { + val fileURI = new URI("file", uri.getUserInfo, uri.getHost, uri.getPort, uri.getPath, uri.getQuery, uri.getFragment) + worksheets.get(fileURI).foreach(_.cancel(true)) + } else { val sendMessage = (msg: String) => client.logMessage(new MessageParams(MessageType.Info, uri + msg)) - evaluateWorksheet(driver, uri, sendMessage)(driver.currentCtx) - sendMessage("FINISHED") + worksheets.put( + uri, + computeAsync { cancelChecker => + try { + val driver = driverFor(uri) + evaluateWorksheet(driver, uri, sendMessage, cancelChecker)(driver.currentCtx) + } finally { + worksheets.remove(uri) + sendMessage("FINISHED") + } + } + ) } } } @@ -437,14 +450,19 @@ class DottyLanguageServer extends LanguageServer /** * Evaluate the worksheet at `uri`. * - * @param driver The driver for the project that contains the worksheet. - * @param uri The URI of the worksheet. - * @param sendMessage A mean of communicating the results of evaluation back. + * @param driver The driver for the project that contains the worksheet. + * @param uri The URI of the worksheet. + * @param sendMessage A mean of communicating the results of evaluation back. + * @param cancelChecker Token to check whether evaluation was cancelled */ - private def evaluateWorksheet(driver: InteractiveDriver, uri: URI, sendMessage: String => Unit)(implicit ctx: Context): Unit = { + private def evaluateWorksheet(driver: InteractiveDriver, + uri: URI, + sendMessage: String => Unit, + cancelChecker: CancelChecker)( + implicit ctx: Context): Unit = { val trees = driver.openedTrees(uri) trees.headOption.foreach { tree => - Worksheet.evaluate(tree, sendMessage) + Worksheet.evaluate(tree, sendMessage, cancelChecker) } } } diff --git a/language-server/src/dotty/tools/languageserver/Worksheet.scala b/language-server/src/dotty/tools/languageserver/Worksheet.scala index a9e1186c5957..5764b8df7847 100644 --- a/language-server/src/dotty/tools/languageserver/Worksheet.scala +++ b/language-server/src/dotty/tools/languageserver/Worksheet.scala @@ -10,17 +10,27 @@ import dotty.tools.dotc.core.Flags.Synthetic import dotty.tools.repl.{ReplDriver, State} +import org.eclipse.lsp4j.jsonrpc.CancelChecker + import java.io.{ByteArrayOutputStream, PrintStream} +import java.util.concurrent.{Callable, CancellationException, Executors, TimeUnit} object Worksheet { + private val executor = Executors.newFixedThreadPool(1) + private final val checkCancelledDelayMs = 50 + /** * Evaluate `tree` as a worksheet using the REPL. * - * @param tree The top level object wrapping the worksheet. - * @param sendMessage A mean of communicating the results of evaluation back. + * @param tree The top level object wrapping the worksheet. + * @param sendMessage A mean of communicating the results of evaluation back. + * @param cancelChecker A token to check whether execution should be cancelled. */ - def evaluate(tree: SourceTree, sendMessage: String => Unit)(implicit ctx: Context): Unit = { + def evaluate(tree: SourceTree, + sendMessage: String => Unit, + cancelChecker: CancelChecker)( + implicit ctx: Context): Unit = { tree.tree match { case td @ TypeDef(_, template: Template) => val replOut = new ByteArrayOutputStream @@ -32,11 +42,28 @@ object Worksheet { state case (state, statement) if executed.add(bounds(statement.pos)) => - val (line, newState) = execute(repl, state, statement, tree.source) - val result = new String(replOut.toByteArray()) - if (result.trim.nonEmpty) sendMessage(encode(result, line)) - replOut.reset() - newState + val task = executor.submit(new Callable[State] { + override def call(): State = { + val (line, newState) = execute(repl, state, statement, tree.source) + val result = new String(replOut.toByteArray()) + if (result.trim.nonEmpty) sendMessage(encode(result, line)) + replOut.reset() + newState + } + }) + + while (!task.isDone()) { + try { + cancelChecker.checkCanceled() + Thread.sleep(checkCancelledDelayMs) + } catch { + case _: CancellationException => + task.cancel(true) + } + } + + try task.get(0, TimeUnit.SECONDS) + catch { case _: Throwable => state } case (state, statement) => state diff --git a/vscode-dotty/package.json b/vscode-dotty/package.json index ef54a1a1875c..682335ff5bbe 100644 --- a/vscode-dotty/package.json +++ b/vscode-dotty/package.json @@ -44,6 +44,10 @@ { "command": "worksheet.evaluate", "title": "Run worksheet" + }, + { + "command": "worksheet.cancel", + "title": "Cancel worksheet evaluation" } ], "configurationDefaults": { diff --git a/vscode-dotty/src/extension.ts b/vscode-dotty/src/extension.ts index 20925a0dcdec..52d4e454321c 100644 --- a/vscode-dotty/src/extension.ts +++ b/vscode-dotty/src/extension.ts @@ -216,6 +216,10 @@ function run(serverOptions: ServerOptions, isOldServer: boolean) { worksheet.worksheetSave(client) }) + vscode.commands.registerCommand(worksheet.worksheetCancelEvaluationKey, () => { + worksheet.cancelExecution(client) + }) + // Push the disposable to the context's subscriptions so that the // client can be deactivated on extension deactivation extensionContext.subscriptions.push(client.start()); diff --git a/vscode-dotty/src/worksheet.ts b/vscode-dotty/src/worksheet.ts index d8c99778c39e..ba475e4646df 100644 --- a/vscode-dotty/src/worksheet.ts +++ b/vscode-dotty/src/worksheet.ts @@ -29,6 +29,11 @@ export const worksheetEvaluateKey = "worksheet.evaluate" */ export const worksheetEvaluateAfterSaveKey = "worksheet.evaluateAfterSave" +/** + * The command key for cancelling worksheet evaluation. + */ +export const worksheetCancelEvaluationKey = "worksheet.cancel" + /** Is this document a worksheet? */ export function isWorksheet(document: vscode.TextDocument): boolean { return document.fileName.endsWith(".sc") @@ -112,9 +117,14 @@ function _prepareWorksheet(document: vscode.TextDocument) { function showWorksheetProgress(document: vscode.TextDocument) { return vscode.window.withProgress({ - location: vscode.ProgressLocation.Window, - title: "Evaluating worksheet" - }, _ => { + location: vscode.ProgressLocation.Notification, + title: "Evaluating worksheet", + cancellable: true + }, (_, token) => { + token.onCancellationRequested(_ => { + vscode.commands.executeCommand(worksheetCancelEvaluationKey) + }) + function isFinished() { return worksheetFinished.get(document) || false } @@ -122,6 +132,23 @@ function showWorksheetProgress(document: vscode.TextDocument) { }) } +/** + * Cancel the execution of the worksheet. + * + * This sends a `textDocument/didSave` message to the language server + * with the scheme of the URI set to `cancel`. The language server + * interprets that as a request to cancel the execution of the specified worksheet. + */ +export function cancelExecution(client: LanguageClient) { + const editor = vscode.window.activeTextEditor + if (editor) { + const document = editor.document + client.sendNotification("textDocument/didSave", { + textDocument: { uri: document.uri.with({ scheme: "cancel" }).toString() } + }) + } +} + /** Wait until `cond` evaluates to true; test every `delay` ms. */ function wait(cond: () => boolean, delayMs: number): Promise { const isFinished = cond() From f11f18497880d1ccecd8ae0abfa5d416712c76ca Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 26 Sep 2018 16:03:22 +0200 Subject: [PATCH 10/38] Add `needsTerminal` param to `JLineTerminal` This parameter configures the JLine terminal so that it throws an exception if `needsTerminal` is set but no terminal is connected. The goal is to reuse that interface from the worksheet, where no terminal is attached. --- compiler/src/dotty/tools/repl/JLineTerminal.scala | 7 ++++--- compiler/src/dotty/tools/repl/Main.scala | 7 ++++++- compiler/src/dotty/tools/repl/ReplDriver.scala | 4 ++-- sbt-bridge/src/xsbt/ConsoleInterface.scala | 2 +- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/repl/JLineTerminal.scala b/compiler/src/dotty/tools/repl/JLineTerminal.scala index 681f105e4788..5948581e1438 100644 --- a/compiler/src/dotty/tools/repl/JLineTerminal.scala +++ b/compiler/src/dotty/tools/repl/JLineTerminal.scala @@ -13,12 +13,13 @@ import org.jline.reader.impl.history.DefaultHistory import org.jline.terminal.TerminalBuilder import org.jline.utils.AttributedString -final class JLineTerminal extends java.io.Closeable { +final class JLineTerminal(needsTerminal: Boolean) extends java.io.Closeable { // import java.util.logging.{Logger, Level} // Logger.getLogger("org.jline").setLevel(Level.FINEST) - private val terminal = TerminalBuilder.builder() - .dumb(false) // fail early if not able to create a terminal + private val terminal = + TerminalBuilder.builder() + .dumb(!needsTerminal) // fail early if we need a terminal and can't create one .build() private val history = new DefaultHistory diff --git a/compiler/src/dotty/tools/repl/Main.scala b/compiler/src/dotty/tools/repl/Main.scala index 725395dcdb3c..adf8d9a6de91 100644 --- a/compiler/src/dotty/tools/repl/Main.scala +++ b/compiler/src/dotty/tools/repl/Main.scala @@ -3,5 +3,10 @@ package dotty.tools.repl /** Main entry point to the REPL */ object Main { def main(args: Array[String]): Unit = - new ReplDriver(args).runUntilQuit() + new ReplDriver(args).runUntilQuit(needsTerminal = true) +} + +object WorksheetMain { + def main(args: Array[String]): Unit = + new ReplDriver(args).runUntilQuit(needsTerminal = false) } diff --git a/compiler/src/dotty/tools/repl/ReplDriver.scala b/compiler/src/dotty/tools/repl/ReplDriver.scala index 63bd7e9dd4a6..eded74cd7c0a 100644 --- a/compiler/src/dotty/tools/repl/ReplDriver.scala +++ b/compiler/src/dotty/tools/repl/ReplDriver.scala @@ -102,8 +102,8 @@ class ReplDriver(settings: Array[String], * observable outside of the CLI, for this reason, most helper methods are * `protected final` to facilitate testing. */ - final def runUntilQuit(initialState: State = initialState): State = { - val terminal = new JLineTerminal() + final def runUntilQuit(needsTerminal: Boolean, initialState: State = initialState): State = { + val terminal = new JLineTerminal(needsTerminal) /** Blockingly read a line, getting back a parse result */ def readLine(state: State): ParseResult = { diff --git a/sbt-bridge/src/xsbt/ConsoleInterface.scala b/sbt-bridge/src/xsbt/ConsoleInterface.scala index 34004528fea4..ba860e84d0ab 100644 --- a/sbt-bridge/src/xsbt/ConsoleInterface.scala +++ b/sbt-bridge/src/xsbt/ConsoleInterface.scala @@ -41,7 +41,7 @@ class ConsoleInterface { val s1 = driver.run(initialCommands)(s0) // TODO handle failure during initialisation - val s2 = driver.runUntilQuit(s1) + val s2 = driver.runUntilQuit(needsTerminal = true, s1) driver.run(cleanupCommands)(s2) } } From 2bd2ffc339463178e938b1dd84d48f2aec48b430 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 26 Sep 2018 16:13:28 +0200 Subject: [PATCH 11/38] Fork a new JVM to execute worksheet This fixes cancellation, which wasn't properly working. --- .../tools/languageserver/Worksheet.scala | 153 +++++++++++++----- 1 file changed, 112 insertions(+), 41 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/Worksheet.scala b/language-server/src/dotty/tools/languageserver/Worksheet.scala index 5764b8df7847..feb86a5020e6 100644 --- a/language-server/src/dotty/tools/languageserver/Worksheet.scala +++ b/language-server/src/dotty/tools/languageserver/Worksheet.scala @@ -12,13 +12,19 @@ import dotty.tools.repl.{ReplDriver, State} import org.eclipse.lsp4j.jsonrpc.CancelChecker -import java.io.{ByteArrayOutputStream, PrintStream} -import java.util.concurrent.{Callable, CancellationException, Executors, TimeUnit} +import java.io.{File, InputStream, InputStreamReader, PrintStream} +import java.util.concurrent.CancellationException object Worksheet { - private val executor = Executors.newFixedThreadPool(1) - private final val checkCancelledDelayMs = 50 + private val javaExec: Option[String] = { + val isWindows = sys.props("os.name").toLowerCase().indexOf("win") >= 0 + val bin = new File(sys.props("java.home"), "bin") + val java = new File(bin, if (isWindows) "java.exe" else "java") + + if (java.exists()) Some(java.getAbsolutePath()) + else None + } /** * Evaluate `tree` as a worksheet using the REPL. @@ -31,42 +37,41 @@ object Worksheet { sendMessage: String => Unit, cancelChecker: CancelChecker)( implicit ctx: Context): Unit = { + + val replMain = dotty.tools.repl.WorksheetMain.getClass.getName.init + val classpath = sys.props("java.class.path") + val options = Array(javaExec.get, "-classpath", classpath, replMain) ++ replOptions + val replProcess = new ProcessBuilder(options: _*).redirectErrorStream(true).start() + + // The stream that we use to send commands to the REPL + val replIn = new PrintStream(replProcess.getOutputStream()) + + // Messages coming out of the REPL + val replOut = new ReplReader(replProcess.getInputStream()) + replOut.start() + + // The thread that monitors cancellation + val cancellationThread = new CancellationThread(replOut, replProcess, cancelChecker) + cancellationThread.start() + + // Wait for the REPL to be ready + replOut.next() + tree.tree match { case td @ TypeDef(_, template: Template) => - val replOut = new ByteArrayOutputStream - val repl = new ReplDriver(replOptions, out = new PrintStream(replOut)) val executed = collection.mutable.Set.empty[(Int, Int)] - template.body.foldLeft(repl.initialState) { - case (state, statement: DefTree) if statement.symbol.is(Synthetic) => - state - - case (state, statement) if executed.add(bounds(statement.pos)) => - val task = executor.submit(new Callable[State] { - override def call(): State = { - val (line, newState) = execute(repl, state, statement, tree.source) - val result = new String(replOut.toByteArray()) - if (result.trim.nonEmpty) sendMessage(encode(result, line)) - replOut.reset() - newState - } - }) - - while (!task.isDone()) { - try { - cancelChecker.checkCanceled() - Thread.sleep(checkCancelledDelayMs) - } catch { - case _: CancellationException => - task.cancel(true) - } - } - - try task.get(0, TimeUnit.SECONDS) - catch { case _: Throwable => state } - - case (state, statement) => - state + template.body.foreach { + case statement: DefTree if statement.symbol.is(Synthetic) => + () + + case statement if executed.add(bounds(statement.pos)) => + val line = execute(replIn, statement, tree.source) + val result = replOut.next().trim + if (result.nonEmpty) sendMessage(encode(result, line)) + + case _ => + () } } } @@ -74,16 +79,17 @@ object Worksheet { /** * Extract `tree` from the source and evaluate it in the REPL. * - * @param repl The REPL that will evaluate the worksheet. - * @param state Current state of the REPL. + * @param replIn A stream to send commands to the REPL. * @param tree The compiled tree to evaluate. * @param sourcefile The sourcefile of the worksheet. - * @return The line in the sourcefile that corresponds to `tree` and the new state of the REPL. + * @return The line in the sourcefile that corresponds to `tree`. */ - private def execute(repl: ReplDriver, state: State, tree: Tree, sourcefile: SourceFile)(implicit ctx: Context): (Int, State) = { + private def execute(replIn: PrintStream, tree: Tree, sourcefile: SourceFile): Int = { val source = sourcefile.content.slice(tree.pos.start, tree.pos.end).mkString val line = sourcefile.offsetToLine(tree.pos.end) - (line, repl.run(source)(state)) + replIn.println(source) + replIn.flush() + line } private def replOptions(implicit ctx: Context): Array[String] = @@ -93,4 +99,69 @@ object Worksheet { line + ":" + message private def bounds(pos: Position): (Int, Int) = (pos.start, pos.end) + + /** + * Regularly check whether execution has been cancelled, kill REPL if it is. + * + * @param replReader The ReplReader that reads the output of the REPL + * @param process The forked JVM that runs the REPL. + * @param cancelChecker The token that reports cancellation. + */ + private class CancellationThread(replReader: ReplReader, process: Process, cancelChecker: CancelChecker) extends Thread { + private final val checkCancelledDelayMs = 50 + + override def run(): Unit = { + try { + while (!Thread.interrupted()) { + cancelChecker.checkCanceled() + Thread.sleep(checkCancelledDelayMs) + } + } catch { + case _: CancellationException => + replReader.interrupt() + process.destroyForcibly() + } + } + } + + /** + * Reads the output from the REPL and makes it available via `next()`. + * + * @param stream The stream of messages coming out of the REPL. + */ + private class ReplReader(stream: InputStream) extends Thread { + private val in = new InputStreamReader(stream) + + private[this] var output: Option[String] = None + + override def run(): Unit = synchronized { + val prompt = "scala> " + val buffer = new StringBuilder + val chars = new Array[Char](256) + var read = 0 + + while (!Thread.interrupted() && { read = in.read(chars); read >= 0 }) { + buffer.appendAll(chars, 0, read) + if (buffer.endsWith(prompt)) { + output = Some(buffer.toString.stripSuffix(prompt)) + buffer.clear() + notify() + wait() + } + } + } + + /** Block until the next message is ready. */ + def next(): String = synchronized { + while (output.isEmpty) { + wait() + } + + val result = output.get + notify() + output = None + result + } + } + } From c1ee31013149b3e4c7766690c5671bca3aff17df Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Thu, 27 Sep 2018 09:09:27 +0200 Subject: [PATCH 12/38] Keep evaluator alive between worksheet runs This allows to shave a few seconds of every worksheet run, because starting up a new JVM is slow. --- .../tools/languageserver/Worksheet.scala | 283 +++++++++++------- 1 file changed, 180 insertions(+), 103 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/Worksheet.scala b/language-server/src/dotty/tools/languageserver/Worksheet.scala index feb86a5020e6..ae06c3c30d62 100644 --- a/language-server/src/dotty/tools/languageserver/Worksheet.scala +++ b/language-server/src/dotty/tools/languageserver/Worksheet.scala @@ -8,8 +8,6 @@ import dotty.tools.dotc.util.SourceFile import dotty.tools.dotc.core.Flags.Synthetic -import dotty.tools.repl.{ReplDriver, State} - import org.eclipse.lsp4j.jsonrpc.CancelChecker import java.io.{File, InputStream, InputStreamReader, PrintStream} @@ -17,15 +15,6 @@ import java.util.concurrent.CancellationException object Worksheet { - private val javaExec: Option[String] = { - val isWindows = sys.props("os.name").toLowerCase().indexOf("win") >= 0 - val bin = new File(sys.props("java.home"), "bin") - val java = new File(bin, if (isWindows) "java.exe" else "java") - - if (java.exists()) Some(java.getAbsolutePath()) - else None - } - /** * Evaluate `tree` as a worksheet using the REPL. * @@ -36,42 +25,30 @@ object Worksheet { def evaluate(tree: SourceTree, sendMessage: String => Unit, cancelChecker: CancelChecker)( - implicit ctx: Context): Unit = { - - val replMain = dotty.tools.repl.WorksheetMain.getClass.getName.init - val classpath = sys.props("java.class.path") - val options = Array(javaExec.get, "-classpath", classpath, replMain) ++ replOptions - val replProcess = new ProcessBuilder(options: _*).redirectErrorStream(true).start() - - // The stream that we use to send commands to the REPL - val replIn = new PrintStream(replProcess.getOutputStream()) - - // Messages coming out of the REPL - val replOut = new ReplReader(replProcess.getInputStream()) - replOut.start() - - // The thread that monitors cancellation - val cancellationThread = new CancellationThread(replOut, replProcess, cancelChecker) - cancellationThread.start() - - // Wait for the REPL to be ready - replOut.next() - - tree.tree match { - case td @ TypeDef(_, template: Template) => - val executed = collection.mutable.Set.empty[(Int, Int)] - - template.body.foreach { - case statement: DefTree if statement.symbol.is(Synthetic) => - () - - case statement if executed.add(bounds(statement.pos)) => - val line = execute(replIn, statement, tree.source) - val result = replOut.next().trim - if (result.nonEmpty) sendMessage(encode(result, line)) - - case _ => - () + implicit ctx: Context): Unit = synchronized { + + Evaluator.get(cancelChecker) match { + case None => + sendMessage(encode("Couldn't start JVM.", 1)) + case Some(evaluator) => + tree.tree match { + case td @ TypeDef(_, template: Template) => + val executed = collection.mutable.Set.empty[(Int, Int)] + + template.body.foreach { + case statement: DefTree if statement.symbol.is(Synthetic) => + () + + case statement if executed.add(bounds(statement.pos)) => + try { + cancelChecker.checkCanceled() + val (line, result) = execute(evaluator, statement, tree.source) + if (result.nonEmpty) sendMessage(encode(result, line)) + } catch { case _: CancellationException => () } + + case _ => + () + } } } } @@ -79,89 +56,189 @@ object Worksheet { /** * Extract `tree` from the source and evaluate it in the REPL. * - * @param replIn A stream to send commands to the REPL. + * @param evaluator The JVM that runs the REPL. * @param tree The compiled tree to evaluate. * @param sourcefile The sourcefile of the worksheet. - * @return The line in the sourcefile that corresponds to `tree`. + * @return The line in the sourcefile that corresponds to `tree`, and the result. */ - private def execute(replIn: PrintStream, tree: Tree, sourcefile: SourceFile): Int = { + private def execute(evaluator: Evaluator, tree: Tree, sourcefile: SourceFile): (Int, String) = { val source = sourcefile.content.slice(tree.pos.start, tree.pos.end).mkString val line = sourcefile.offsetToLine(tree.pos.end) - replIn.println(source) - replIn.flush() - line + (line, evaluator.eval(source)) } - private def replOptions(implicit ctx: Context): Array[String] = - Array("-color:never", "-classpath", ctx.settings.classpath.value) - private def encode(message: String, line: Int): String = line + ":" + message private def bounds(pos: Position): (Int, Int) = (pos.start, pos.end) +} + +private object Evaluator { + + private val javaExec: Option[String] = { + val isWindows = sys.props("os.name").toLowerCase().indexOf("win") >= 0 + val bin = new File(sys.props("java.home"), "bin") + val java = new File(bin, if (isWindows) "java.exe" else "java") + + if (java.exists()) Some(java.getAbsolutePath()) + else None + } + + /** + * The most recent Evaluator that was used. It can be reused if the user classpath hasn't changed + * between two calls. + */ + private[this] var previousEvaluator: Option[(String, Evaluator)] = None + /** - * Regularly check whether execution has been cancelled, kill REPL if it is. + * Get a (possibly reused) Evaluator and set cancel checker. * - * @param replReader The ReplReader that reads the output of the REPL - * @param process The forked JVM that runs the REPL. - * @param cancelChecker The token that reports cancellation. + * @param cancelChecker The token that indicates whether evaluation has been cancelled. + * @return A JVM running the REPL. */ - private class CancellationThread(replReader: ReplReader, process: Process, cancelChecker: CancelChecker) extends Thread { - private final val checkCancelledDelayMs = 50 - - override def run(): Unit = { - try { - while (!Thread.interrupted()) { - cancelChecker.checkCanceled() - Thread.sleep(checkCancelledDelayMs) - } - } catch { - case _: CancellationException => - replReader.interrupt() - process.destroyForcibly() - } + def get(cancelChecker: CancelChecker)(implicit ctx: Context): Option[Evaluator] = { + val classpath = ctx.settings.classpath.value + previousEvaluator match { + case Some(cp, evaluator) if cp == classpath => + evaluator.reset(cancelChecker) + Some(evaluator) + case _ => + previousEvaluator.foreach(_._2.exit()) + val newEvaluator = javaExec.map(new Evaluator(_, ctx.settings.classpath.value, cancelChecker)) + previousEvaluator = newEvaluator.map(jvm => (classpath, jvm)) + newEvaluator } } +} + +/** + * Represents a JVM running the REPL, ready for evaluation. + * + * @param javaExec The path to the `java` executable. + * @param userClasspath The REPL classpath + * @param cancelChecker The token that indicates whether evaluation has been cancelled. + */ +private class Evaluator private (javaExec: String, + userClasspath: String, + cancelChecker: CancelChecker) { + private val process = + new ProcessBuilder( + javaExec, + "-classpath", sys.props("java.class.path"), + dotty.tools.repl.WorksheetMain.getClass.getName.init, + "-classpath", userClasspath, + "-color:never") + .redirectErrorStream(true) + .start() + + // The stream that we use to send commands to the REPL + private val processInput = new PrintStream(process.getOutputStream()) + + // Messages coming out of the REPL + private val processOutput = new ReplReader(process.getInputStream()) + processOutput.start() + + // The thread that monitors cancellation + private val cancellationThread = new CancellationThread(cancelChecker, this) + cancellationThread.start() + + // Wait for the REPL to be ready + processOutput.next() /** - * Reads the output from the REPL and makes it available via `next()`. + * Submit `command` to the REPL, wait for the result. * - * @param stream The stream of messages coming out of the REPL. + * @param command The command to evaluate. + * @return The result from the REPL. */ - private class ReplReader(stream: InputStream) extends Thread { - private val in = new InputStreamReader(stream) - - private[this] var output: Option[String] = None - - override def run(): Unit = synchronized { - val prompt = "scala> " - val buffer = new StringBuilder - val chars = new Array[Char](256) - var read = 0 - - while (!Thread.interrupted() && { read = in.read(chars); read >= 0 }) { - buffer.appendAll(chars, 0, read) - if (buffer.endsWith(prompt)) { - output = Some(buffer.toString.stripSuffix(prompt)) - buffer.clear() - notify() - wait() - } + def eval(command: String): String = { + processInput.println(command) + processInput.flush() + processOutput.next().trim + } + + /** + * Reset the REPL to its initial state, update the cancel checker. + */ + def reset(cancelChecker: CancelChecker): Unit = { + cancellationThread.setCancelChecker(cancelChecker) + eval(":reset") + } + + /** Terminate this JVM. */ + def exit(): Unit = { + processOutput.interrupt() + process.destroyForcibly() + Evaluator.previousEvaluator = None + cancellationThread.interrupt() + } + +} + +/** + * Regularly check whether execution has been cancelled, kill REPL if it is. + */ +private class CancellationThread(private[this] var cancelChecker: CancelChecker, + evaluator: Evaluator) extends Thread { + private final val checkCancelledDelayMs = 50 + + override def run(): Unit = { + try { + while (!Thread.interrupted()) { + cancelChecker.checkCanceled() + Thread.sleep(checkCancelledDelayMs) } + } catch { + case _: CancellationException => evaluator.exit() + case _: InterruptedException => evaluator.exit() } + } + + def setCancelChecker(cancelChecker: CancelChecker): Unit = { + this.cancelChecker = cancelChecker + } +} - /** Block until the next message is ready. */ - def next(): String = synchronized { - while (output.isEmpty) { +/** + * Reads the output from the REPL and makes it available via `next()`. + * + * @param stream The stream of messages coming out of the REPL. + */ +private class ReplReader(stream: InputStream) extends Thread { + private val in = new InputStreamReader(stream) + + private[this] var output: Option[String] = None + + override def run(): Unit = synchronized { + val prompt = "scala> " + val buffer = new StringBuilder + val chars = new Array[Char](256) + var read = 0 + + while (!Thread.interrupted() && { read = in.read(chars); read >= 0 }) { + buffer.appendAll(chars, 0, read) + if (buffer.endsWith(prompt)) { + output = Some(buffer.toString.stripSuffix(prompt)) + buffer.clear() + notify() wait() } - - val result = output.get - notify() - output = None - result } + output = Some("") + notify() } + /** Block until the next message is ready. */ + def next(): String = synchronized { + + while (output.isEmpty) { + wait() + } + + val result = output.get + notify() + output = None + result + } } From d41a47d050351bf772cf6c7504104b342b091557 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Thu, 27 Sep 2018 10:07:57 +0200 Subject: [PATCH 13/38] Dotty IDE: Do not highlight constructors They're always synthetic, they shouldn't be highlighted. --- .../dotty/tools/languageserver/DottyLanguageServer.scala | 2 +- .../test/dotty/tools/languageserver/HighlightTest.scala | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index 92ea5d92b6ec..cb948c8ee01c 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -382,7 +382,7 @@ class DottyLanguageServer extends LanguageServer else { val refs = Interactive.namedTrees(uriTrees, Include.references | Include.overriding, sym) (for { - ref <- refs + ref <- refs if !ref.tree.symbol.isConstructor nameRange <- range(ref.namePos) } yield new DocumentHighlight(nameRange, DocumentHighlightKind.Read)).asJava } diff --git a/language-server/test/dotty/tools/languageserver/HighlightTest.scala b/language-server/test/dotty/tools/languageserver/HighlightTest.scala index a863197fdf2e..3bf0957be1b8 100644 --- a/language-server/test/dotty/tools/languageserver/HighlightTest.scala +++ b/language-server/test/dotty/tools/languageserver/HighlightTest.scala @@ -19,4 +19,10 @@ class HighlightTest { .highlight(xRef.range, (xDef.range, DocumentHighlightKind.Read), (xRef.range, DocumentHighlightKind.Read)) } + @Test def highlightClass(): Unit = { + code"""class ${m1}Foo${m2} { new ${m3}Foo${m4} }""".withSource + .highlight(m1 to m2, (m1 to m2, DocumentHighlightKind.Read), (m3 to m4, DocumentHighlightKind.Read)) + .highlight(m3 to m4, (m1 to m2, DocumentHighlightKind.Read), (m3 to m4, DocumentHighlightKind.Read)) + } + } From de48e957561768efdd9ab9a4bfaefc64209be457 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Thu, 27 Sep 2018 14:00:46 +0200 Subject: [PATCH 14/38] Fix `namedTrees` with substring search This functin used to produce a string representation of the whole tree and then look whether this contains the substring we're looking for. When we're not including references in the search, we can limit the search to the trees names rather than their whole representation. --- .../src/dotty/tools/dotc/interactive/Interactive.scala | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala index 16335f3e5f09..5853cbcd6b2d 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala @@ -269,8 +269,12 @@ object Interactive { * @param includeReferences If true, include references and not just definitions */ def namedTrees(trees: List[SourceTree], includeReferences: Boolean, nameSubstring: String) - (implicit ctx: Context): List[SourceTree] = - namedTrees(trees, includeReferences, _.show.toString.contains(nameSubstring)) + (implicit ctx: Context): List[SourceTree] = { + val predicate: NameTree => Boolean = + if (includeReferences) _.show.contains(nameSubstring) + else _.name.toString.contains(nameSubstring) + namedTrees(trees, includeReferences, predicate) + } /** Find named trees with a non-empty position satisfying `treePredicate` in `trees`. * From 185e7655b210aca23ad9d772495b958891f1bbc7 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Thu, 27 Sep 2018 14:56:34 +0200 Subject: [PATCH 15/38] Support go to def, rename, etc. in worksheets --- .../languageserver/DottyLanguageServer.scala | 68 +++++++++--- .../tools/languageserver/DefinitionTest.scala | 12 --- .../tools/languageserver/WorksheetTest.scala | 102 ++++++++++++++++++ .../tools/languageserver/util/Code.scala | 6 ++ 4 files changed, 163 insertions(+), 25 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index cb948c8ee01c..c3ddc5156815 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -183,7 +183,7 @@ class DottyLanguageServer extends LanguageServer val worksheetMode = isWorksheet(uri) val (text, positionMapper) = - if (worksheetMode) (wrapWorksheet(document.getText), Some(worksheetPositionMapper _)) + if (worksheetMode) (wrapWorksheet(document.getText), Some(toUnwrappedPosition _)) else (document.getText, None) val diags = driver.run(uri, text) @@ -204,7 +204,7 @@ class DottyLanguageServer extends LanguageServer assert(change.getRange == null, "TextDocumentSyncKind.Incremental support is not implemented") val (text, positionMapper) = - if (worksheetMode) (wrapWorksheet(change.getText), Some(worksheetPositionMapper _)) + if (worksheetMode) (wrapWorksheet(change.getText), Some(toUnwrappedPosition _)) else (change.getText, None) val diags = driver.run(uri, text) @@ -317,7 +317,7 @@ class DottyLanguageServer extends LanguageServer (Nil, Include.overriding) } val defs = Interactive.namedTrees(trees, include, sym) - defs.flatMap(d => location(d.namePos)).asJava + defs.flatMap(d => location(d.namePos, positionMapperFor(d.source))).asJava } } @@ -340,7 +340,7 @@ class DottyLanguageServer extends LanguageServer Include.references | Include.overriding | (if (includeDeclaration) Include.definitions else 0) val refs = Interactive.findTreesMatching(trees, includes, sym) - refs.flatMap(ref => location(ref.namePos)).asJava + refs.flatMap(ref => location(ref.namePos, positionMapperFor(ref.source))).asJava } } @@ -363,7 +363,7 @@ class DottyLanguageServer extends LanguageServer val changes = refs.groupBy(ref => toUri(ref.source).toString) .mapValues(refs => refs.flatMap(ref => - range(ref.namePos).map(nameRange => new TextEdit(nameRange, newName))).asJava) + range(ref.namePos, positionMapperFor(ref.source)).map(nameRange => new TextEdit(nameRange, newName))).asJava) new WorkspaceEdit(changes.asJava) } @@ -383,7 +383,7 @@ class DottyLanguageServer extends LanguageServer val refs = Interactive.namedTrees(uriTrees, Include.references | Include.overriding, sym) (for { ref <- refs if !ref.tree.symbol.isConstructor - nameRange <- range(ref.namePos) + nameRange <- range(ref.namePos, positionMapperFor(ref.source)) } yield new DocumentHighlight(nameRange, DocumentHighlightKind.Read)).asJava } } @@ -416,8 +416,8 @@ class DottyLanguageServer extends LanguageServer val defs = Interactive.namedTrees(uriTrees, includeReferences = false, _ => true) (for { - d <- defs - info <- symbolInfo(d.tree.symbol, d.namePos) + d <- defs if !isWorksheetWrapper(d) + info <- symbolInfo(d.tree.symbol, d.namePos, positionMapperFor(d.source)) } yield JEither.forLeft(info)).asJava } @@ -429,7 +429,7 @@ class DottyLanguageServer extends LanguageServer val trees = driver.allTrees val defs = Interactive.namedTrees(trees, includeReferences = false, nameSubstring = query) - defs.flatMap(d => symbolInfo(d.tree.symbol, d.namePos)) + defs.flatMap(d => symbolInfo(d.tree.symbol, d.namePos, positionMapperFor(d.source))) }.asJava } @@ -473,9 +473,12 @@ object DottyLanguageServer { /** Convert an lsp4j.Position to a SourcePosition */ def sourcePosition(driver: InteractiveDriver, uri: URI, pos: lsp4j.Position): SourcePosition = { + val actualPosition = + if (isWorksheet(uri)) toWrappedPosition(pos) + else pos val source = driver.openedFiles(uri) if (source.exists) { - val p = Positions.Position(source.lineToOffset(pos.getLine) + pos.getCharacter) + val p = Positions.Position(source.lineToOffset(actualPosition.getLine) + actualPosition.getCharacter) new SourcePosition(source, p) } else NoSourcePosition @@ -528,6 +531,10 @@ object DottyLanguageServer { private def isWorksheet(uri: URI): Boolean = uri.toString.endsWith(".sc") + /** Does this sourcefile represent a worksheet? */ + private def isWorksheet(sourcefile: SourceFile): Boolean = + sourcefile.file.extension == "sc" + /** Wrap the source of a worksheet inside an `object`. */ private def wrapWorksheet(source: String): String = s"""object Worksheet { @@ -544,13 +551,48 @@ object DottyLanguageServer { * @param position The position as seen by the compiler (after wrapping) * @return The position in the actual source file (before wrapping). */ - private def worksheetPositionMapper(position: SourcePosition): SourcePosition = { + private def toUnwrappedPosition(position: SourcePosition): SourcePosition = { new SourcePosition(position.source, position.pos, position.outer) { override def startLine: Int = position.startLine - 1 override def endLine: Int = position.endLine - 1 } } + /** + * Map `position` in an unwrapped worksheet to the same position in the wrapped source. + * + * Because worksheet are wrapped in an `object`, the positions in the source are one line + * above from what the compiler sees. + * + * @see wrapWorksheet + * @param position The position as seen by VSCode (before wrapping) + * @return The position as seen by the compiler (after wrapping) + */ + private def toWrappedPosition(position: lsp4j.Position): lsp4j.Position = { + new lsp4j.Position(position.getLine + 1, position.getCharacter) + } + + /** + * Returns the position mapper necessary to unwrap positions for `sourcefile`. If `sourcefile` is + * not a worksheet, no mapper is necessary. Otherwise, return `toUnwrappedPosition`. + */ + private def positionMapperFor(sourcefile: SourceFile): Option[SourcePosition => SourcePosition] = { + if (isWorksheet(sourcefile)) Some(toUnwrappedPosition _) + else None + } + + /** + * Is `sourceTree` the wrapper object that we put around worksheet sources? + * + * @see wrapWorksheet + */ + def isWorksheetWrapper(sourceTree: SourceTree)(implicit ctx: Context): Boolean = { + val symbol = sourceTree.tree.symbol + isWorksheet(sourceTree.source) && + symbol.name.toString == "Worksheet$" && + symbol.owner == ctx.definitions.EmptyPackageClass + } + /** Create an lsp4j.CompletionItem from a Symbol */ def completionItem(sym: Symbol)(implicit ctx: Context): lsp4j.CompletionItem = { def completionItemKind(sym: Symbol)(implicit ctx: Context): lsp4j.CompletionItemKind = { @@ -596,7 +638,7 @@ object DottyLanguageServer { } /** Create an lsp4j.SymbolInfo from a Symbol and a SourcePosition */ - def symbolInfo(sym: Symbol, pos: SourcePosition)(implicit ctx: Context): Option[lsp4j.SymbolInformation] = { + def symbolInfo(sym: Symbol, pos: SourcePosition, positionMapper: Option[SourcePosition => SourcePosition])(implicit ctx: Context): Option[lsp4j.SymbolInformation] = { def symbolKind(sym: Symbol)(implicit ctx: Context): lsp4j.SymbolKind = { import lsp4j.{SymbolKind => SK} @@ -621,6 +663,6 @@ object DottyLanguageServer { else null - location(pos).map(l => new lsp4j.SymbolInformation(name, symbolKind(sym), l, containerName)) + location(pos, positionMapper).map(l => new lsp4j.SymbolInformation(name, symbolKind(sym), l, containerName)) } } diff --git a/language-server/test/dotty/tools/languageserver/DefinitionTest.scala b/language-server/test/dotty/tools/languageserver/DefinitionTest.scala index 363f27c7d951..4520e9bbb9cc 100644 --- a/language-server/test/dotty/tools/languageserver/DefinitionTest.scala +++ b/language-server/test/dotty/tools/languageserver/DefinitionTest.scala @@ -55,12 +55,6 @@ class DefinitionTest { } @Test def goToDefNamedArgOverload: Unit = { - val m9 = new CodeMarker("m9") - val m10 = new CodeMarker("m10") - val m11 = new CodeMarker("m11") - val m12 = new CodeMarker("m12") - val m13 = new CodeMarker("m13") - val m14 = new CodeMarker("m14") code"""object Foo { def foo(${m1}x${m2}: String): String = ${m3}x${m4} @@ -88,10 +82,6 @@ class DefinitionTest { } @Test def goToConstructorNamedArgOverload: Unit = { - val m9 = new CodeMarker("m9") - val m10 = new CodeMarker("m10") - val m11 = new CodeMarker("m11") - val m12 = new CodeMarker("m12") withSources( code"""class Foo(${m1}x${m2}: String) { @@ -110,8 +100,6 @@ class DefinitionTest { } @Test def goToParamCopyMethod: Unit = { - val m9 = new CodeMarker("m9") - val m10 = new CodeMarker("m10") withSources( code"""case class Foo(${m1}x${m2}: Int, ${m3}y${m4}: String)""", diff --git a/language-server/test/dotty/tools/languageserver/WorksheetTest.scala b/language-server/test/dotty/tools/languageserver/WorksheetTest.scala index c47016a7e2f1..0c0361a0bb05 100644 --- a/language-server/test/dotty/tools/languageserver/WorksheetTest.scala +++ b/language-server/test/dotty/tools/languageserver/WorksheetTest.scala @@ -1,8 +1,10 @@ package dotty.tools.languageserver import org.junit.Test +import org.eclipse.lsp4j.{CompletionItemKind, DocumentHighlightKind, SymbolKind} import dotty.tools.languageserver.util.Code._ +import dotty.tools.languageserver.util.embedded.CodeMarker class WorksheetTest { @@ -82,4 +84,104 @@ class WorksheetTest { .evaluate(m1, "1:val foo: Int = 1\nval bar: Int = 2") } + @Test def worksheetCompletion(): Unit = { + ws"""class Foo { def bar = 123 } + val x = new Foo + x.b${m1}""".withSource + .completion(m1, Set(("bar", CompletionItemKind.Method, "=> Int"))) + } + + @Test def worksheetGoToDefinition(): Unit = { + + withSources( + code"""class ${m11}Baz${m12}""", + ws"""class ${m1}Foo${m2} { def ${m3}bar${m4} = new ${m5}Baz${m6} } + val x = new ${m7}Foo${m8} + x.${m9}bar${m10}""" + ).definition(m1 to m2, List(m1 to m2)) + .definition(m3 to m4, List(m3 to m4)) + .definition(m5 to m6, List(m11 to m12)) + .definition(m7 to m8, List(m1 to m2)) + .definition(m9 to m10, List(m3 to m4)) + .definition(m11 to m12, List(m11 to m12)) + } + + @Test def worksheetReferences(): Unit = { + + withSources( + code"""class ${m11}Baz${m12}""", + ws"""class ${m1}Foo${m2} { def ${m3}bar${m4} = new ${m9}Baz${m10} } + val x = new ${m5}Foo${m6} + x.${m7}bar${m8}""" + ).references(m1 to m2, List(m5 to m6)) + .references(m3 to m4, List(m7 to m8)) + .references(m11 to m12, List(m9 to m10)) + } + + @Test def worksheetRename(): Unit = { + + def sources = + withSources( + code"""class ${m9}Baz${m10}""", + ws"""class ${m1}Foo${m2}(baz: ${m3}Baz${m4}) + val x = new ${m5}Foo${m6}(new ${m7}Baz${m8})""" + ) + + def testRenameFooFrom(m: CodeMarker) = + sources.rename(m, "Bar", Set(m1 to m2, m5 to m6)) + + def testRenameBazFrom(m: CodeMarker) = + sources.rename(m, "Bar", Set(m3 to m4, m7 to m8, m9 to m10)) + + testRenameFooFrom(m1) + testRenameBazFrom(m3) + testRenameFooFrom(m5) + testRenameBazFrom(m7) + testRenameBazFrom(m9) + } + + @Test def worksheetHighlight(): Unit = { + ws"""class ${m1}Foo${m2} { def ${m3}bar${m4} = 123 } + val x = new ${m5}Foo${m6} + x.${m7}bar${m8}""".withSource + .highlight(m1 to m2, (m1 to m2, DocumentHighlightKind.Read), (m5 to m6, DocumentHighlightKind.Read)) + .highlight(m3 to m4, (m3 to m4, DocumentHighlightKind.Read), (m7 to m8, DocumentHighlightKind.Read)) + .highlight(m5 to m6, (m1 to m2, DocumentHighlightKind.Read), (m5 to m6, DocumentHighlightKind.Read)) + .highlight(m7 to m8, (m3 to m4, DocumentHighlightKind.Read), (m7 to m8, DocumentHighlightKind.Read)) + } + + def hoverContent(typeInfo: String, comment: String): Option[String] = + Some(s"""```scala + |$typeInfo + |$comment + |```""".stripMargin) + @Test def worksheetHover(): Unit = { + ws"""/** A class */ class ${m1}Foo${m2} { /** A method */ def ${m3}bar${m4} = 123 } + val x = new ${m5}Foo${m6} + x.${m7}bar${m8}""".withSource + .hover(m1 to m2, hoverContent("Worksheet.Foo", "/** A class */")) + .hover(m3 to m4, hoverContent("Int", "/** A method */")) + .hover(m5 to m6, hoverContent("Worksheet.Foo", "/** A class */")) + .hover(m7 to m8, hoverContent("Int", "/** A method */")) + } + + @Test def worksheetDocumentSymbol(): Unit = { + ws"""class ${m1}Foo${m2} { + def ${m3}bar${m4} = 123 + }""".withSource + .documentSymbol(m1, (m1 to m2).symInfo("Foo", SymbolKind.Class, "Worksheet$"), + (m3 to m4).symInfo("bar", SymbolKind.Method, "Foo")) + } + + @Test def worksheetSymbol(): Unit = { + withSources( + ws"""class ${m1}Foo${m2} { + def ${m3}bar${m4} = 123 + }""", + code"""class ${m5}Baz${m6}""" + ).symbol("Foo", (m1 to m2).symInfo("Foo", SymbolKind.Class, "Worksheet$")) + .symbol("bar", (m3 to m4).symInfo("bar", SymbolKind.Method, "Foo")) + .symbol("Baz", (m5 to m6).symInfo("Baz", SymbolKind.Class)) + } + } diff --git a/language-server/test/dotty/tools/languageserver/util/Code.scala b/language-server/test/dotty/tools/languageserver/util/Code.scala index 3d3899dae7fa..aca2279beb8c 100644 --- a/language-server/test/dotty/tools/languageserver/util/Code.scala +++ b/language-server/test/dotty/tools/languageserver/util/Code.scala @@ -20,6 +20,12 @@ object Code { val m6 = new CodeMarker("m6") val m7 = new CodeMarker("m7") val m8 = new CodeMarker("m8") + val m9 = new CodeMarker("m9") + val m10 = new CodeMarker("m10") + val m11 = new CodeMarker("m11") + val m12 = new CodeMarker("m12") + val m13 = new CodeMarker("m13") + val m14 = new CodeMarker("m14") implicit class CodeHelper(val sc: StringContext) extends AnyVal { From 79860e052752b831d8f1e0345bdcc9098738ab07 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Thu, 27 Sep 2018 15:37:20 +0200 Subject: [PATCH 16/38] Add worksheet cancellation test --- .../tools/languageserver/WorksheetTest.scala | 8 +++++ .../languageserver/util/CodeTester.scala | 21 +++++++++++++ .../util/actions/WorksheetAction.scala | 31 +++++++++++++++++++ .../util/actions/WorksheetCancel.scala | 30 ++++++++++++++++++ .../util/actions/WorksheetEvaluate.scala | 31 +++++++++---------- 5 files changed, 104 insertions(+), 17 deletions(-) create mode 100644 language-server/test/dotty/tools/languageserver/util/actions/WorksheetAction.scala create mode 100644 language-server/test/dotty/tools/languageserver/util/actions/WorksheetCancel.scala diff --git a/language-server/test/dotty/tools/languageserver/WorksheetTest.scala b/language-server/test/dotty/tools/languageserver/WorksheetTest.scala index 0c0361a0bb05..01fc06bd4a66 100644 --- a/language-server/test/dotty/tools/languageserver/WorksheetTest.scala +++ b/language-server/test/dotty/tools/languageserver/WorksheetTest.scala @@ -184,4 +184,12 @@ class WorksheetTest { .symbol("Baz", (m5 to m6).symInfo("Baz", SymbolKind.Class)) } + @Test def worksheetCancel(): Unit = { + ws"""${m1}val foo = 1 + val bar = 2 + while (true) {} + val baz = 3""".withSource + .cancelEvaluation(m1, afterMs = 5000) + } + } diff --git a/language-server/test/dotty/tools/languageserver/util/CodeTester.scala b/language-server/test/dotty/tools/languageserver/util/CodeTester.scala index feeea8a47f9e..c91b7e42236e 100644 --- a/language-server/test/dotty/tools/languageserver/util/CodeTester.scala +++ b/language-server/test/dotty/tools/languageserver/util/CodeTester.scala @@ -117,9 +117,30 @@ class CodeTester(sources: List[SourceWithPositions], actions: List[Action]) { def symbol(query: String, symbols: SymInfo*): this.type = doAction(new CodeSymbol(query, symbols)) + /** + * Triggers evaluation of the worksheet specified by `marker`, verifies that the results of + * evaluation match `expected. + * + * @param marker A marker a identifies the worksheet to evaluate. + * @param expected The expected output. + * + * @see dotty.tools.languageserver.util.actions.WorksheetEvaluate + */ def evaluate(marker: CodeMarker, expected: String*): this.type = doAction(new WorksheetEvaluate(marker, expected)) + /** + * Triggers evaluation of the worksheet specified by `marker`, then verifies that execution can be + * cancelled after `afterMs` milliseconds. + * + * @param marker A marker that identifier the worksheet to evaluate. + * @param afterMs The delay in milliseconds before cancelling execution. + * + * @see dotty.tools.languageserver.util.actions.WorksheetCancel + */ + def cancelEvaluation(marker: CodeMarker, afterMs: Long): this.type = + doAction(new WorksheetCancel(marker, afterMs)) + private def doAction(action: Action): this.type = { try { action.execute()(testServer, testServer.client, positions) diff --git a/language-server/test/dotty/tools/languageserver/util/actions/WorksheetAction.scala b/language-server/test/dotty/tools/languageserver/util/actions/WorksheetAction.scala new file mode 100644 index 000000000000..1bb35dde0097 --- /dev/null +++ b/language-server/test/dotty/tools/languageserver/util/actions/WorksheetAction.scala @@ -0,0 +1,31 @@ +package dotty.tools.languageserver.util.actions + +import dotty.tools.languageserver.util.embedded.CodeMarker + +import org.eclipse.lsp4j.{DidSaveTextDocumentParams, MessageParams, MessageType} + +abstract class WorksheetAction extends Action { + + def triggerEvaluation(marker: CodeMarker): Exec[Unit] = { + val file = marker.toTextDocumentIdentifier + server.didSave(new DidSaveTextDocumentParams(file)) + } + + def triggerCancellation(marker: CodeMarker): Exec[Unit] = { + val file = { + val file = marker.toTextDocumentIdentifier + file.setUri(file.getUri.replaceFirst("file:", "cancel:")) + file + } + server.didSave(new DidSaveTextDocumentParams(file)) + } + + def getLogs(marker: CodeMarker): Exec[List[String]] = { + def matches(params: MessageParams): Boolean = + params.getType == MessageType.Info && params.getMessage.startsWith(marker.file.uri) + client.log.get.collect { + case params: MessageParams if matches(params) => + params.getMessage.substring(marker.file.uri.length).trim + } + } +} diff --git a/language-server/test/dotty/tools/languageserver/util/actions/WorksheetCancel.scala b/language-server/test/dotty/tools/languageserver/util/actions/WorksheetCancel.scala new file mode 100644 index 000000000000..0c7be1413ccd --- /dev/null +++ b/language-server/test/dotty/tools/languageserver/util/actions/WorksheetCancel.scala @@ -0,0 +1,30 @@ +package dotty.tools.languageserver.util.actions + +import dotty.tools.languageserver.util.PositionContext +import dotty.tools.languageserver.util.embedded.CodeMarker + +import org.junit.Assert.{assertEquals, fail} + +class WorksheetCancel(marker: CodeMarker, afterMs: Long) extends WorksheetAction { + + private final val cancellationTimeoutMs = 10 * 1000 + + override def execute(): Exec[Unit] = { + triggerEvaluation(marker) + Thread.sleep(afterMs) + triggerCancellation(marker) + + val timeAtCancellation = System.currentTimeMillis() + while (!getLogs(marker).contains("FINISHED")) { + if (System.currentTimeMillis() - timeAtCancellation > cancellationTimeoutMs) { + fail(s"Couldn't cancel worksheet evaluation after ${cancellationTimeoutMs} ms.") + } + Thread.sleep(100) + } + + client.log.clear() + } + + override def show: PositionContext.PosCtx[String] = + s"WorksheetCancel(${marker.file}, ${afterMs})" +} diff --git a/language-server/test/dotty/tools/languageserver/util/actions/WorksheetEvaluate.scala b/language-server/test/dotty/tools/languageserver/util/actions/WorksheetEvaluate.scala index 6c65ea23ceb4..e7f8490795d4 100644 --- a/language-server/test/dotty/tools/languageserver/util/actions/WorksheetEvaluate.scala +++ b/language-server/test/dotty/tools/languageserver/util/actions/WorksheetEvaluate.scala @@ -1,18 +1,24 @@ package dotty.tools.languageserver.util.actions -import dotty.tools.languageserver.util.{PositionContext} +import dotty.tools.languageserver.util.PositionContext import dotty.tools.languageserver.util.embedded.CodeMarker -import org.eclipse.lsp4j.{DidSaveTextDocumentParams, MessageParams, MessageType} +import org.junit.Assert.{assertEquals, fail} -import org.junit.Assert.assertEquals +class WorksheetEvaluate(marker: CodeMarker, expected: Seq[String]) extends WorksheetAction { -class WorksheetEvaluate(marker: CodeMarker, expected: Seq[String]) extends Action { - override def execute(): Exec[Unit] = { - val file = marker.toTextDocumentIdentifier - server.didSave(new DidSaveTextDocumentParams(file)) + private final val evaluationTimeoutMs = 30 * 1000 - while (!getLogs(marker).contains("FINISHED")) Thread.sleep(100) + override def execute(): Exec[Unit] = { + triggerEvaluation(marker) + + val timeAtEvaluation = System.currentTimeMillis() + while (!getLogs(marker).contains("FINISHED")) { + if (System.currentTimeMillis() - timeAtEvaluation > evaluationTimeoutMs) { + fail(s"Evaluation didn't finish after ${evaluationTimeoutMs} ms.") + } + Thread.sleep(100) + } assertEquals(expected, getLogs(marker).init) client.log.clear() @@ -20,13 +26,4 @@ class WorksheetEvaluate(marker: CodeMarker, expected: Seq[String]) extends Actio override def show: PositionContext.PosCtx[String] = s"WorksheetEvaluate(${marker.file}, ${expected})" - - private def getLogs(marker: CodeMarker): Exec[List[String]] = { - def matches(params: MessageParams): Boolean = - params.getType == MessageType.Info && params.getMessage.startsWith(marker.file.uri) - client.log.get.collect { - case params: MessageParams if matches(params) => - params.getMessage.substring(marker.file.uri.length).trim - } - } } From c3d13671c75d67a6fd1ca4786a695997ad73ecda Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Thu, 27 Sep 2018 15:58:15 +0200 Subject: [PATCH 17/38] Test exception in worksheet evaluation --- .../dotty/tools/languageserver/WorksheetTest.scala | 7 +++++++ .../tools/languageserver/util/CodeTester.scala | 14 +++++++++++++- .../util/actions/WorksheetEvaluate.scala | 12 +++++++++--- 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/language-server/test/dotty/tools/languageserver/WorksheetTest.scala b/language-server/test/dotty/tools/languageserver/WorksheetTest.scala index 01fc06bd4a66..32e14ab2c018 100644 --- a/language-server/test/dotty/tools/languageserver/WorksheetTest.scala +++ b/language-server/test/dotty/tools/languageserver/WorksheetTest.scala @@ -84,6 +84,13 @@ class WorksheetTest { .evaluate(m1, "1:val foo: Int = 1\nval bar: Int = 2") } + @Test def evaluationException: Unit = { + ws"""${m1}val foo = 1 / 0 + val bar = 2""".withSource + .evaluateNonStrict(m1, "1:java.lang.ArithmeticException: / by zero", + "2:val bar: Int = 2") + } + @Test def worksheetCompletion(): Unit = { ws"""class Foo { def bar = 123 } val x = new Foo diff --git a/language-server/test/dotty/tools/languageserver/util/CodeTester.scala b/language-server/test/dotty/tools/languageserver/util/CodeTester.scala index c91b7e42236e..42f5ed4fcc12 100644 --- a/language-server/test/dotty/tools/languageserver/util/CodeTester.scala +++ b/language-server/test/dotty/tools/languageserver/util/CodeTester.scala @@ -127,7 +127,19 @@ class CodeTester(sources: List[SourceWithPositions], actions: List[Action]) { * @see dotty.tools.languageserver.util.actions.WorksheetEvaluate */ def evaluate(marker: CodeMarker, expected: String*): this.type = - doAction(new WorksheetEvaluate(marker, expected)) + doAction(new WorksheetEvaluate(marker, expected, strict = true)) + + /** + * Triggers evaluation of the worksheet specified by `marker`, verifies that each line of output + * starts with `expected`. + * + * @param marker A marker a identifies the worksheet to evaluate. + * @param expected The expected starts of output. + * + * @see dotty.tools.languageserver.util.actions.WorksheetEvaluate + */ + def evaluateNonStrict(marker: CodeMarker, expected: String*): this.type = + doAction(new WorksheetEvaluate(marker, expected, strict = false)) /** * Triggers evaluation of the worksheet specified by `marker`, then verifies that execution can be diff --git a/language-server/test/dotty/tools/languageserver/util/actions/WorksheetEvaluate.scala b/language-server/test/dotty/tools/languageserver/util/actions/WorksheetEvaluate.scala index e7f8490795d4..647ace90ff64 100644 --- a/language-server/test/dotty/tools/languageserver/util/actions/WorksheetEvaluate.scala +++ b/language-server/test/dotty/tools/languageserver/util/actions/WorksheetEvaluate.scala @@ -3,9 +3,9 @@ package dotty.tools.languageserver.util.actions import dotty.tools.languageserver.util.PositionContext import dotty.tools.languageserver.util.embedded.CodeMarker -import org.junit.Assert.{assertEquals, fail} +import org.junit.Assert.{assertEquals, assertTrue, fail} -class WorksheetEvaluate(marker: CodeMarker, expected: Seq[String]) extends WorksheetAction { +class WorksheetEvaluate(marker: CodeMarker, expected: Seq[String], strict: Boolean) extends WorksheetAction { private final val evaluationTimeoutMs = 30 * 1000 @@ -20,7 +20,13 @@ class WorksheetEvaluate(marker: CodeMarker, expected: Seq[String]) extends Works Thread.sleep(100) } - assertEquals(expected, getLogs(marker).init) + if (strict) { + assertEquals(expected, getLogs(marker).init) + } else { + expected.zip(getLogs(marker).init).foreach { + case (expected, message) => assertTrue(s"'$message' didn't start with '$expected'", message.startsWith(expected)) + } + } client.log.clear() } From 4223ecb75b972ede70a6e40f4a8bc724af9ee832 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Fri, 28 Sep 2018 09:57:05 +0200 Subject: [PATCH 18/38] Exclude only primary constructors --- compiler/src/dotty/tools/dotc/interactive/Interactive.scala | 2 +- .../src/dotty/tools/languageserver/DottyLanguageServer.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala index 5853cbcd6b2d..c66f0554dddc 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala @@ -326,7 +326,7 @@ object Interactive { val includeLinkedClass = (includes & Include.linkedClass) != 0 val predicate: NameTree => Boolean = tree => ( tree.pos.isSourceDerived - && !tree.symbol.isConstructor + && !tree.symbol.isPrimaryConstructor && (includeDeclaration || !Interactive.isDefinition(tree)) && ( Interactive.matchSymbol(tree, symbol, includes) || ( includeDeclaration diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index c3ddc5156815..2a06c6975575 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -382,7 +382,7 @@ class DottyLanguageServer extends LanguageServer else { val refs = Interactive.namedTrees(uriTrees, Include.references | Include.overriding, sym) (for { - ref <- refs if !ref.tree.symbol.isConstructor + ref <- refs if !ref.tree.symbol.isPrimaryConstructor nameRange <- range(ref.namePos, positionMapperFor(ref.source)) } yield new DocumentHighlight(nameRange, DocumentHighlightKind.Read)).asJava } From ae1099451da9e72b4cef630886f6ac2a156641f4 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Fri, 28 Sep 2018 09:58:00 +0200 Subject: [PATCH 19/38] Respect `-color:never` with typedef in REPL --- compiler/src/dotty/tools/repl/ReplDriver.scala | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/repl/ReplDriver.scala b/compiler/src/dotty/tools/repl/ReplDriver.scala index eded74cd7c0a..38d6dfdf7287 100644 --- a/compiler/src/dotty/tools/repl/ReplDriver.scala +++ b/compiler/src/dotty/tools/repl/ReplDriver.scala @@ -285,7 +285,12 @@ class ReplDriver(settings: Array[String], .foreach { sym => // FIXME syntax highlighting on comment is currently not working // out.println(SyntaxHighlighting.highlight("// defined " + sym.showUser)) - out.println(SyntaxHighlighting.CommentColor + "// defined " + sym.showUser + SyntaxHighlighting.NoColor) + val message = "// defined " + sym.showUser + if (ctx.settings.color.value != "never") { + println(SyntaxHighlighting.CommentColor + message + SyntaxHighlighting.NoColor) + } else { + println(message) + } } From 719acd8e6e4af2a67626f041e4b1744d9552d3c2 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Fri, 28 Sep 2018 10:43:53 +0200 Subject: [PATCH 20/38] IDE: Remove unused substring reference search `Interactive.scala` offered an API to find all references to names given substrings of the names. This feature was unused. This worked by pretty printing the tree and looking whether it contained the substring. This is broken, because the substring may match parts that are not references (what if we look for `if`?) It's gone now. --- .../src/dotty/tools/dotc/interactive/Interactive.scala | 10 +++------- .../tools/languageserver/DottyLanguageServer.scala | 2 +- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala index c66f0554dddc..74bd3d86c3bc 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala @@ -265,15 +265,11 @@ object Interactive { namedTrees(trees, (include & Include.references) != 0, matchSymbol(_, sym, include)) /** Find named trees with a non-empty position whose name contains `nameSubstring` in `trees`. - * - * @param includeReferences If true, include references and not just definitions */ - def namedTrees(trees: List[SourceTree], includeReferences: Boolean, nameSubstring: String) + def namedTrees(trees: List[SourceTree], nameSubstring: String) (implicit ctx: Context): List[SourceTree] = { - val predicate: NameTree => Boolean = - if (includeReferences) _.show.contains(nameSubstring) - else _.name.toString.contains(nameSubstring) - namedTrees(trees, includeReferences, predicate) + val predicate: NameTree => Boolean = _.name.toString.contains(nameSubstring) + namedTrees(trees, includeReferences = false, predicate) } /** Find named trees with a non-empty position satisfying `treePredicate` in `trees`. diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index 2a06c6975575..7a319d096584 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -428,7 +428,7 @@ class DottyLanguageServer extends LanguageServer implicit val ctx = driver.currentCtx val trees = driver.allTrees - val defs = Interactive.namedTrees(trees, includeReferences = false, nameSubstring = query) + val defs = Interactive.namedTrees(trees, nameSubstring = query) defs.flatMap(d => symbolInfo(d.tree.symbol, d.namePos, positionMapperFor(d.source))) }.asJava } From ced2868d9a937c858e41bb1c4826dd11bb2dddff Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Fri, 28 Sep 2018 11:32:22 +0200 Subject: [PATCH 21/38] Support `System.exit` in worksheet --- .../src/dotty/tools/languageserver/Worksheet.scala | 9 ++++++--- .../test/dotty/tools/languageserver/WorksheetTest.scala | 7 +++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/Worksheet.scala b/language-server/src/dotty/tools/languageserver/Worksheet.scala index ae06c3c30d62..4f8bb6fabc85 100644 --- a/language-server/src/dotty/tools/languageserver/Worksheet.scala +++ b/language-server/src/dotty/tools/languageserver/Worksheet.scala @@ -39,7 +39,7 @@ object Worksheet { case statement: DefTree if statement.symbol.is(Synthetic) => () - case statement if executed.add(bounds(statement.pos)) => + case statement if evaluator.isAlive() && executed.add(bounds(statement.pos)) => try { cancelChecker.checkCanceled() val (line, result) = execute(evaluator, statement, tree.source) @@ -100,7 +100,7 @@ private object Evaluator { def get(cancelChecker: CancelChecker)(implicit ctx: Context): Option[Evaluator] = { val classpath = ctx.settings.classpath.value previousEvaluator match { - case Some(cp, evaluator) if cp == classpath => + case Some(cp, evaluator) if evaluator.isAlive() && cp == classpath => evaluator.reset(cancelChecker) Some(evaluator) case _ => @@ -146,6 +146,9 @@ private class Evaluator private (javaExec: String, // Wait for the REPL to be ready processOutput.next() + /** Is the process that runs the REPL still alive? */ + def isAlive(): Boolean = process.isAlive() + /** * Submit `command` to the REPL, wait for the result. * @@ -185,7 +188,7 @@ private class CancellationThread(private[this] var cancelChecker: CancelChecker, override def run(): Unit = { try { - while (!Thread.interrupted()) { + while (evaluator.isAlive() && !Thread.interrupted()) { cancelChecker.checkCanceled() Thread.sleep(checkCancelledDelayMs) } diff --git a/language-server/test/dotty/tools/languageserver/WorksheetTest.scala b/language-server/test/dotty/tools/languageserver/WorksheetTest.scala index 32e14ab2c018..c389e31ae5f5 100644 --- a/language-server/test/dotty/tools/languageserver/WorksheetTest.scala +++ b/language-server/test/dotty/tools/languageserver/WorksheetTest.scala @@ -199,4 +199,11 @@ class WorksheetTest { .cancelEvaluation(m1, afterMs = 5000) } + @Test def systemExit(): Unit = { + ws"""${m1}println("Hello, world!") + System.exit(0) + println("Goodbye!")""".withSource + .evaluate(m1, "1:Hello, world!") + } + } From f4cbbcacd16af7696349771cf4feb5cb334afd9f Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Fri, 28 Sep 2018 11:33:10 +0200 Subject: [PATCH 22/38] Check that worksheets capture stderr --- .../test/dotty/tools/languageserver/WorksheetTest.scala | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/language-server/test/dotty/tools/languageserver/WorksheetTest.scala b/language-server/test/dotty/tools/languageserver/WorksheetTest.scala index c389e31ae5f5..c26e8bbf68ee 100644 --- a/language-server/test/dotty/tools/languageserver/WorksheetTest.scala +++ b/language-server/test/dotty/tools/languageserver/WorksheetTest.scala @@ -206,4 +206,9 @@ class WorksheetTest { .evaluate(m1, "1:Hello, world!") } + @Test def outputOnStdErr(): Unit = { + ws"""${m1}System.err.println("Oh no")""".withSource + .evaluate(m1, "1:Oh no") + } + } From 0eb7fc9afee605d05edee95820be2c6905916a41 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Mon, 1 Oct 2018 10:12:59 +0200 Subject: [PATCH 23/38] Don't wait for messages after stream is closed --- .../dotty/tools/languageserver/Worksheet.scala | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/Worksheet.scala b/language-server/src/dotty/tools/languageserver/Worksheet.scala index 4f8bb6fabc85..3326e8990749 100644 --- a/language-server/src/dotty/tools/languageserver/Worksheet.scala +++ b/language-server/src/dotty/tools/languageserver/Worksheet.scala @@ -64,7 +64,7 @@ object Worksheet { private def execute(evaluator: Evaluator, tree: Tree, sourcefile: SourceFile): (Int, String) = { val source = sourcefile.content.slice(tree.pos.start, tree.pos.end).mkString val line = sourcefile.offsetToLine(tree.pos.end) - (line, evaluator.eval(source)) + (line, evaluator.eval(source).getOrElse("")) } private def encode(message: String, line: Int): String = @@ -155,10 +155,10 @@ private class Evaluator private (javaExec: String, * @param command The command to evaluate. * @return The result from the REPL. */ - def eval(command: String): String = { + def eval(command: String): Option[String] = { processInput.println(command) processInput.flush() - processOutput.next().trim + processOutput.next().map(_.trim) } /** @@ -212,6 +212,7 @@ private class ReplReader(stream: InputStream) extends Thread { private val in = new InputStreamReader(stream) private[this] var output: Option[String] = None + private[this] var closed: Boolean = false override def run(): Unit = synchronized { val prompt = "scala> " @@ -228,18 +229,18 @@ private class ReplReader(stream: InputStream) extends Thread { wait() } } - output = Some("") + closed = true notify() } /** Block until the next message is ready. */ - def next(): String = synchronized { + def next(): Option[String] = synchronized { - while (output.isEmpty) { + while (!closed && output.isEmpty) { wait() } - val result = output.get + val result = output notify() output = None result From c58b31410506a5508ed320023d8a0a1c34d9cad2 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Mon, 1 Oct 2018 11:02:30 +0200 Subject: [PATCH 24/38] Fix synchronization issue with worksheets Because the process running the worksheets is shared between worksheets (when possible), we need to synchronize worksheet evaluation. --- .../dotty/tools/languageserver/DottyLanguageServer.scala | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index 7a319d096584..d532cfb61ba7 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -4,7 +4,7 @@ package languageserver import java.net.URI import java.io._ import java.nio.file._ -import java.util.concurrent.CompletableFuture +import java.util.concurrent.{CompletableFuture, ConcurrentHashMap} import java.util.function.Function import com.fasterxml.jackson.databind.ObjectMapper @@ -54,7 +54,7 @@ class DottyLanguageServer extends LanguageServer private[this] var rootUri: String = _ private[this] var client: LanguageClient = _ - private[this] val worksheets: mutable.Map[URI, CompletableFuture[_]] = mutable.Map.empty + private[this] val worksheets: ConcurrentHashMap[URI, CompletableFuture[_]] = new ConcurrentHashMap() private[this] var myDrivers: mutable.Map[ProjectConfig, InteractiveDriver] = _ @@ -232,8 +232,8 @@ class DottyLanguageServer extends LanguageServer if (isWorksheet(uri)) { if (uri.getScheme == "cancel") { val fileURI = new URI("file", uri.getUserInfo, uri.getHost, uri.getPort, uri.getPath, uri.getQuery, uri.getFragment) - worksheets.get(fileURI).foreach(_.cancel(true)) - } else { + Option(worksheets.get(fileURI)).foreach(_.cancel(true)) + } else thisServer.synchronized { val sendMessage = (msg: String) => client.logMessage(new MessageParams(MessageType.Info, uri + msg)) worksheets.put( uri, From ce04cc8e639d4a880f5f1a4c01ebc669034b95e5 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Mon, 1 Oct 2018 11:36:54 +0200 Subject: [PATCH 25/38] Split worksheet implementation --- .../languageserver/DottyLanguageServer.scala | 1 + .../tools/languageserver/Worksheet.scala | 248 ------------------ .../worksheet/CancellationThread.scala | 29 ++ .../languageserver/worksheet/Evaluator.scala | 113 ++++++++ .../languageserver/worksheet/ReplReader.scala | 47 ++++ .../languageserver/worksheet/Worksheet.scala | 75 ++++++ 6 files changed, 265 insertions(+), 248 deletions(-) delete mode 100644 language-server/src/dotty/tools/languageserver/Worksheet.scala create mode 100644 language-server/src/dotty/tools/languageserver/worksheet/CancellationThread.scala create mode 100644 language-server/src/dotty/tools/languageserver/worksheet/Evaluator.scala create mode 100644 language-server/src/dotty/tools/languageserver/worksheet/ReplReader.scala create mode 100644 language-server/src/dotty/tools/languageserver/worksheet/Worksheet.scala diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index d532cfb61ba7..f85168c7cec4 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -29,6 +29,7 @@ import Interactive.Include import config.Printers.interactiv import languageserver.config.ProjectConfig +import languageserver.worksheet.Worksheet import lsp4j.services._ diff --git a/language-server/src/dotty/tools/languageserver/Worksheet.scala b/language-server/src/dotty/tools/languageserver/Worksheet.scala deleted file mode 100644 index 3326e8990749..000000000000 --- a/language-server/src/dotty/tools/languageserver/Worksheet.scala +++ /dev/null @@ -1,248 +0,0 @@ -package dotty.tools.languageserver - -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.Positions.Position -import dotty.tools.dotc.util.SourceFile - -import dotty.tools.dotc.core.Flags.Synthetic - -import org.eclipse.lsp4j.jsonrpc.CancelChecker - -import java.io.{File, InputStream, InputStreamReader, PrintStream} -import java.util.concurrent.CancellationException - -object Worksheet { - - /** - * Evaluate `tree` as a worksheet using the REPL. - * - * @param tree The top level object wrapping the worksheet. - * @param sendMessage A mean of communicating the results of evaluation back. - * @param cancelChecker A token to check whether execution should be cancelled. - */ - def evaluate(tree: SourceTree, - sendMessage: String => Unit, - cancelChecker: CancelChecker)( - implicit ctx: Context): Unit = synchronized { - - Evaluator.get(cancelChecker) match { - case None => - sendMessage(encode("Couldn't start JVM.", 1)) - case Some(evaluator) => - tree.tree match { - case td @ TypeDef(_, template: Template) => - val executed = collection.mutable.Set.empty[(Int, Int)] - - template.body.foreach { - case statement: DefTree if statement.symbol.is(Synthetic) => - () - - case statement if evaluator.isAlive() && executed.add(bounds(statement.pos)) => - try { - cancelChecker.checkCanceled() - val (line, result) = execute(evaluator, statement, tree.source) - if (result.nonEmpty) sendMessage(encode(result, line)) - } catch { case _: CancellationException => () } - - case _ => - () - } - } - } - } - - /** - * Extract `tree` from the source and evaluate it in the REPL. - * - * @param evaluator The JVM that runs the REPL. - * @param tree The compiled tree to evaluate. - * @param sourcefile The sourcefile of the worksheet. - * @return The line in the sourcefile that corresponds to `tree`, and the result. - */ - private def execute(evaluator: Evaluator, tree: Tree, sourcefile: SourceFile): (Int, String) = { - val source = sourcefile.content.slice(tree.pos.start, tree.pos.end).mkString - val line = sourcefile.offsetToLine(tree.pos.end) - (line, evaluator.eval(source).getOrElse("")) - } - - private def encode(message: String, line: Int): String = - line + ":" + message - - private def bounds(pos: Position): (Int, Int) = (pos.start, pos.end) - -} - -private object Evaluator { - - private val javaExec: Option[String] = { - val isWindows = sys.props("os.name").toLowerCase().indexOf("win") >= 0 - val bin = new File(sys.props("java.home"), "bin") - val java = new File(bin, if (isWindows) "java.exe" else "java") - - if (java.exists()) Some(java.getAbsolutePath()) - else None - } - - /** - * The most recent Evaluator that was used. It can be reused if the user classpath hasn't changed - * between two calls. - */ - private[this] var previousEvaluator: Option[(String, Evaluator)] = None - - /** - * Get a (possibly reused) Evaluator and set cancel checker. - * - * @param cancelChecker The token that indicates whether evaluation has been cancelled. - * @return A JVM running the REPL. - */ - def get(cancelChecker: CancelChecker)(implicit ctx: Context): Option[Evaluator] = { - val classpath = ctx.settings.classpath.value - previousEvaluator match { - case Some(cp, evaluator) if evaluator.isAlive() && cp == classpath => - evaluator.reset(cancelChecker) - Some(evaluator) - case _ => - previousEvaluator.foreach(_._2.exit()) - val newEvaluator = javaExec.map(new Evaluator(_, ctx.settings.classpath.value, cancelChecker)) - previousEvaluator = newEvaluator.map(jvm => (classpath, jvm)) - newEvaluator - } - } -} - -/** - * Represents a JVM running the REPL, ready for evaluation. - * - * @param javaExec The path to the `java` executable. - * @param userClasspath The REPL classpath - * @param cancelChecker The token that indicates whether evaluation has been cancelled. - */ -private class Evaluator private (javaExec: String, - userClasspath: String, - cancelChecker: CancelChecker) { - private val process = - new ProcessBuilder( - javaExec, - "-classpath", sys.props("java.class.path"), - dotty.tools.repl.WorksheetMain.getClass.getName.init, - "-classpath", userClasspath, - "-color:never") - .redirectErrorStream(true) - .start() - - // The stream that we use to send commands to the REPL - private val processInput = new PrintStream(process.getOutputStream()) - - // Messages coming out of the REPL - private val processOutput = new ReplReader(process.getInputStream()) - processOutput.start() - - // The thread that monitors cancellation - private val cancellationThread = new CancellationThread(cancelChecker, this) - cancellationThread.start() - - // Wait for the REPL to be ready - processOutput.next() - - /** Is the process that runs the REPL still alive? */ - def isAlive(): Boolean = process.isAlive() - - /** - * Submit `command` to the REPL, wait for the result. - * - * @param command The command to evaluate. - * @return The result from the REPL. - */ - def eval(command: String): Option[String] = { - processInput.println(command) - processInput.flush() - processOutput.next().map(_.trim) - } - - /** - * Reset the REPL to its initial state, update the cancel checker. - */ - def reset(cancelChecker: CancelChecker): Unit = { - cancellationThread.setCancelChecker(cancelChecker) - eval(":reset") - } - - /** Terminate this JVM. */ - def exit(): Unit = { - processOutput.interrupt() - process.destroyForcibly() - Evaluator.previousEvaluator = None - cancellationThread.interrupt() - } - -} - -/** - * Regularly check whether execution has been cancelled, kill REPL if it is. - */ -private class CancellationThread(private[this] var cancelChecker: CancelChecker, - evaluator: Evaluator) extends Thread { - private final val checkCancelledDelayMs = 50 - - override def run(): Unit = { - try { - while (evaluator.isAlive() && !Thread.interrupted()) { - cancelChecker.checkCanceled() - Thread.sleep(checkCancelledDelayMs) - } - } catch { - case _: CancellationException => evaluator.exit() - case _: InterruptedException => evaluator.exit() - } - } - - def setCancelChecker(cancelChecker: CancelChecker): Unit = { - this.cancelChecker = cancelChecker - } -} - -/** - * Reads the output from the REPL and makes it available via `next()`. - * - * @param stream The stream of messages coming out of the REPL. - */ -private class ReplReader(stream: InputStream) extends Thread { - private val in = new InputStreamReader(stream) - - private[this] var output: Option[String] = None - private[this] var closed: Boolean = false - - override def run(): Unit = synchronized { - val prompt = "scala> " - val buffer = new StringBuilder - val chars = new Array[Char](256) - var read = 0 - - while (!Thread.interrupted() && { read = in.read(chars); read >= 0 }) { - buffer.appendAll(chars, 0, read) - if (buffer.endsWith(prompt)) { - output = Some(buffer.toString.stripSuffix(prompt)) - buffer.clear() - notify() - wait() - } - } - closed = true - notify() - } - - /** Block until the next message is ready. */ - def next(): Option[String] = synchronized { - - while (!closed && output.isEmpty) { - wait() - } - - val result = output - notify() - output = None - result - } -} diff --git a/language-server/src/dotty/tools/languageserver/worksheet/CancellationThread.scala b/language-server/src/dotty/tools/languageserver/worksheet/CancellationThread.scala new file mode 100644 index 000000000000..e4d6b3c306a5 --- /dev/null +++ b/language-server/src/dotty/tools/languageserver/worksheet/CancellationThread.scala @@ -0,0 +1,29 @@ +package dotty.tools.languageserver.worksheet + +import org.eclipse.lsp4j.jsonrpc.CancelChecker + +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, + evaluator: Evaluator) extends Thread { + private final val checkCancelledDelayMs = 50 + + override def run(): Unit = { + try { + while (evaluator.isAlive() && !Thread.interrupted()) { + cancelChecker.checkCanceled() + Thread.sleep(checkCancelledDelayMs) + } + } catch { + case _: CancellationException => evaluator.exit() + case _: InterruptedException => evaluator.exit() + } + } + + def setCancelChecker(cancelChecker: CancelChecker): Unit = { + this.cancelChecker = cancelChecker + } +} diff --git a/language-server/src/dotty/tools/languageserver/worksheet/Evaluator.scala b/language-server/src/dotty/tools/languageserver/worksheet/Evaluator.scala new file mode 100644 index 000000000000..28430fb08a4f --- /dev/null +++ b/language-server/src/dotty/tools/languageserver/worksheet/Evaluator.scala @@ -0,0 +1,113 @@ +package dotty.tools.languageserver.worksheet + +import dotty.tools.dotc.core.Contexts.Context + +import java.io.{File, PrintStream} + +import org.eclipse.lsp4j.jsonrpc.CancelChecker + +private object Evaluator { + + private val javaExec: Option[String] = { + val isWindows = sys.props("os.name").toLowerCase().indexOf("win") >= 0 + val bin = new File(sys.props("java.home"), "bin") + val java = new File(bin, if (isWindows) "java.exe" else "java") + + if (java.exists()) Some(java.getAbsolutePath()) + else None + } + + /** + * The most recent Evaluator that was used. It can be reused if the user classpath hasn't changed + * between two calls. + */ + private[this] var previousEvaluator: Option[(String, Evaluator)] = None + + /** + * Get a (possibly reused) Evaluator and set cancel checker. + * + * @param cancelChecker The token that indicates whether evaluation has been cancelled. + * @return A JVM running the REPL. + */ + def get(cancelChecker: CancelChecker)(implicit ctx: Context): Option[Evaluator] = { + val classpath = ctx.settings.classpath.value + previousEvaluator match { + case Some(cp, evaluator) if evaluator.isAlive() && cp == classpath => + evaluator.reset(cancelChecker) + Some(evaluator) + case _ => + previousEvaluator.foreach(_._2.exit()) + val newEvaluator = javaExec.map(new Evaluator(_, ctx.settings.classpath.value, cancelChecker)) + previousEvaluator = newEvaluator.map(jvm => (classpath, jvm)) + newEvaluator + } + } +} + +/** + * Represents a JVM running the REPL, ready for evaluation. + * + * @param javaExec The path to the `java` executable. + * @param userClasspath The REPL classpath + * @param cancelChecker The token that indicates whether evaluation has been cancelled. + */ +private class Evaluator private (javaExec: String, + userClasspath: String, + cancelChecker: CancelChecker) { + private val process = + new ProcessBuilder( + javaExec, + "-classpath", sys.props("java.class.path"), + dotty.tools.repl.WorksheetMain.getClass.getName.init, + "-classpath", userClasspath, + "-color:never") + .redirectErrorStream(true) + .start() + + // The stream that we use to send commands to the REPL + private val processInput = new PrintStream(process.getOutputStream()) + + // Messages coming out of the REPL + private val processOutput = new ReplReader(process.getInputStream()) + processOutput.start() + + // The thread that monitors cancellation + private val cancellationThread = new CancellationThread(cancelChecker, this) + cancellationThread.start() + + // Wait for the REPL to be ready + processOutput.next() + + /** Is the process that runs the REPL still alive? */ + def isAlive(): Boolean = process.isAlive() + + /** + * Submit `command` to the REPL, wait for the result. + * + * @param command The command to evaluate. + * @return The result from the REPL. + */ + def eval(command: String): Option[String] = { + processInput.println(command) + processInput.flush() + processOutput.next().map(_.trim) + } + + /** + * Reset the REPL to its initial state, update the cancel checker. + */ + def reset(cancelChecker: CancelChecker): Unit = { + cancellationThread.setCancelChecker(cancelChecker) + eval(":reset") + } + + /** Terminate this JVM. */ + def exit(): Unit = { + processOutput.interrupt() + process.destroyForcibly() + Evaluator.previousEvaluator = None + cancellationThread.interrupt() + } + +} + diff --git a/language-server/src/dotty/tools/languageserver/worksheet/ReplReader.scala b/language-server/src/dotty/tools/languageserver/worksheet/ReplReader.scala new file mode 100644 index 000000000000..bd50f9c4faf8 --- /dev/null +++ b/language-server/src/dotty/tools/languageserver/worksheet/ReplReader.scala @@ -0,0 +1,47 @@ +package dotty.tools.languageserver.worksheet + +import java.io.{InputStream, InputStreamReader} + +/** + * Reads the output from the REPL and makes it available via `next()`. + * + * @param stream The stream of messages coming out of the REPL. + */ +private class ReplReader(stream: InputStream) extends Thread { + private val in = new InputStreamReader(stream) + + private[this] var output: Option[String] = None + private[this] var closed: Boolean = false + + override def run(): Unit = synchronized { + val prompt = "scala> " + val buffer = new StringBuilder + val chars = new Array[Char](256) + var read = 0 + + while (!Thread.interrupted() && { read = in.read(chars); read >= 0 }) { + buffer.appendAll(chars, 0, read) + if (buffer.endsWith(prompt)) { + output = Some(buffer.toString.stripSuffix(prompt)) + buffer.clear() + notify() + wait() + } + } + closed = true + notify() + } + + /** Block until the next message is ready. */ + def next(): Option[String] = synchronized { + + while (!closed && output.isEmpty) { + wait() + } + + val result = output + notify() + output = None + result + } +} diff --git a/language-server/src/dotty/tools/languageserver/worksheet/Worksheet.scala b/language-server/src/dotty/tools/languageserver/worksheet/Worksheet.scala new file mode 100644 index 000000000000..1e1442053b19 --- /dev/null +++ b/language-server/src/dotty/tools/languageserver/worksheet/Worksheet.scala @@ -0,0 +1,75 @@ +package dotty.tools.languageserver.worksheet + +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.Positions.Position +import dotty.tools.dotc.util.SourceFile + +import dotty.tools.dotc.core.Flags.Synthetic + +import org.eclipse.lsp4j.jsonrpc.CancelChecker + +import java.util.concurrent.CancellationException + +object Worksheet { + + /** + * Evaluate `tree` as a worksheet using the REPL. + * + * @param tree The top level object wrapping the worksheet. + * @param sendMessage A mean of communicating the results of evaluation back. + * @param cancelChecker A token to check whether execution should be cancelled. + */ + def evaluate(tree: SourceTree, + sendMessage: String => Unit, + cancelChecker: CancelChecker)( + implicit ctx: Context): Unit = synchronized { + + Evaluator.get(cancelChecker) match { + case None => + sendMessage(encode("Couldn't start JVM.", 1)) + case Some(evaluator) => + tree.tree match { + case td @ TypeDef(_, template: Template) => + val executed = collection.mutable.Set.empty[(Int, Int)] + + template.body.foreach { + case statement: DefTree if statement.symbol.is(Synthetic) => + () + + case statement if evaluator.isAlive() && executed.add(bounds(statement.pos)) => + try { + cancelChecker.checkCanceled() + val (line, result) = execute(evaluator, statement, tree.source) + if (result.nonEmpty) sendMessage(encode(result, line)) + } catch { case _: CancellationException => () } + + case _ => + () + } + } + } + } + + /** + * Extract `tree` from the source and evaluate it in the REPL. + * + * @param evaluator The JVM that runs the REPL. + * @param tree The compiled tree to evaluate. + * @param sourcefile The sourcefile of the worksheet. + * @return The line in the sourcefile that corresponds to `tree`, and the result. + */ + private def execute(evaluator: Evaluator, tree: Tree, sourcefile: SourceFile): (Int, String) = { + val source = sourcefile.content.slice(tree.pos.start, tree.pos.end).mkString + val line = sourcefile.offsetToLine(tree.pos.end) + (line, evaluator.eval(source).getOrElse("")) + } + + private def encode(message: String, line: Int): String = + line + ":" + message + + private def bounds(pos: Position): (Int, Int) = (pos.start, pos.end) + +} + From e6b3abdd2ceb8131a25aa51763decd50459240f0 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Mon, 1 Oct 2018 13:10:58 +0200 Subject: [PATCH 26/38] Check if evaluator runs on Windows --- .../src/dotty/tools/languageserver/worksheet/Evaluator.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/worksheet/Evaluator.scala b/language-server/src/dotty/tools/languageserver/worksheet/Evaluator.scala index 28430fb08a4f..27c9ac774e68 100644 --- a/language-server/src/dotty/tools/languageserver/worksheet/Evaluator.scala +++ b/language-server/src/dotty/tools/languageserver/worksheet/Evaluator.scala @@ -9,9 +9,8 @@ import org.eclipse.lsp4j.jsonrpc.CancelChecker private object Evaluator { private val javaExec: Option[String] = { - val isWindows = sys.props("os.name").toLowerCase().indexOf("win") >= 0 val bin = new File(sys.props("java.home"), "bin") - val java = new File(bin, if (isWindows) "java.exe" else "java") + val java = new File(bin, if (scala.util.Properties.isWin) "java.exe" else "java") if (java.exists()) Some(java.getAbsolutePath()) else None From 471a0591ba9b926c27462d3988b58233bb2922ca Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Mon, 1 Oct 2018 13:11:16 +0200 Subject: [PATCH 27/38] Fix worksheet tests for Windows --- .../test/dotty/tools/languageserver/WorksheetTest.scala | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/language-server/test/dotty/tools/languageserver/WorksheetTest.scala b/language-server/test/dotty/tools/languageserver/WorksheetTest.scala index c26e8bbf68ee..c7469f811a5e 100644 --- a/language-server/test/dotty/tools/languageserver/WorksheetTest.scala +++ b/language-server/test/dotty/tools/languageserver/WorksheetTest.scala @@ -6,6 +6,8 @@ import org.eclipse.lsp4j.{CompletionItemKind, DocumentHighlightKind, SymbolKind} import dotty.tools.languageserver.util.Code._ import dotty.tools.languageserver.util.embedded.CodeMarker +import java.lang.System.{lineSeparator => nl} + class WorksheetTest { @Test def evaluateExpression: Unit = { @@ -68,7 +70,7 @@ class WorksheetTest { @Test def produceMultilineOutput: Unit = { ws"""${m1}1 to 3 foreach println""".withSource - .evaluate(m1, "1:1\n2\n3") + .evaluate(m1, s"1:1${nl}2${nl}3") } @Test def patternMatching0: Unit = { @@ -81,7 +83,7 @@ class WorksheetTest { @Test def patternMatching1: Unit = { ws"""${m1}val (foo, bar) = (1, 2)""".withSource - .evaluate(m1, "1:val foo: Int = 1\nval bar: Int = 2") + .evaluate(m1, s"1:val foo: Int = 1${nl}val bar: Int = 2") } @Test def evaluationException: Unit = { From 701e8ea83ed9cb20440b2599f196c90df749487e Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Mon, 1 Oct 2018 14:35:44 +0200 Subject: [PATCH 28/38] vscode: Add `Worksheet` class It is used to represent the worksheets managed by vscode, instead of having several global variables. --- vscode-dotty/src/worksheet.ts | 151 +++++++++++++++++----------------- 1 file changed, 74 insertions(+), 77 deletions(-) diff --git a/vscode-dotty/src/worksheet.ts b/vscode-dotty/src/worksheet.ts index ba475e4646df..2247b13b7af0 100644 --- a/vscode-dotty/src/worksheet.ts +++ b/vscode-dotty/src/worksheet.ts @@ -1,19 +1,40 @@ -'use strict' - import * as vscode from 'vscode' import { LanguageClient } from 'vscode-languageclient' -/** All decorations that have been added so far */ -let worksheetDecorationTypes: Map = new Map() +/** A worksheet managed by vscode */ +class Worksheet { + + constructor(document: vscode.TextDocument) { + this.document = document + } + + /** The text document that this worksheet represents. */ + readonly document: vscode.TextDocument -/** The number of blank lines that have been inserted to fit the output so far. */ -let worksheetInsertedLines: Map = new Map() + /** All decorations that have been added so far */ + decorationTypes: vscode.TextEditorDecorationType[] = [] -/** The minimum margin to add so that the decoration is shown after all text. */ -let worksheetMargin: Map = new Map() + /** The number of blank lines that have been inserted to fit the output so far. */ + insertedLines: number = 0 -/** Whether the given worksheet has finished evaluating. */ -let worksheetFinished: Map = new Map() + /** The minimum margin to add so that the decoration is shown after all text. */ + margin: number = 0 + + /** Whether this worksheet has finished evaluating. */ + finished: boolean = false + + /** Remove all decorations and resets this worksheet. */ + reset() { + this.decorationTypes.forEach(decoration => decoration.dispose()) + this.insertedLines = 0 + this.margin = longestLine(this.document) + 5 + this.finished = false + } + +} + +/** All the worksheets */ +let worksheets: Map = new Map() /** * The command key for evaluating a worksheet. Exposed to users as @@ -47,9 +68,9 @@ export function isWorksheet(document: vscode.TextDocument): boolean { export function evaluateCommand() { const editor = vscode.window.activeTextEditor if (editor) { - const document = editor.document - if (isWorksheet(document)) { - showWorksheetProgress(document) + const worksheet = worksheets.get(editor.document) + if (worksheet) { + showWorksheetProgress(worksheet) } } } @@ -69,19 +90,20 @@ export function worksheetSave(client: LanguageClient) { const editor = vscode.window.activeTextEditor if (editor) { const document = editor.document - if (isWorksheet(document)) { - if (document.isDirty) document.save() - else { - _prepareWorksheet(document).then(_ => { - if (document.isDirty) document.save() - else { - client.sendNotification("textDocument/didSave", { - textDocument: { uri: document.uri.toString() } - }) - showWorksheetProgress(document) - } - }) - } + const worksheet = worksheets.get(document) || new Worksheet(document) + worksheets.set(document, worksheet) + + if (document.isDirty) document.save() + else { + _prepareWorksheet(worksheet).then(_ => { + if (document.isDirty) document.save() + else { + client.sendNotification("textDocument/didSave", { + textDocument: { uri: document.uri.toString() } + }) + showWorksheetProgress(worksheet) + } + }) } } } @@ -96,26 +118,18 @@ export function worksheetSave(client: LanguageClient) { * @param event `TextDocumentWillSaveEvent`. */ export function prepareWorksheet(event: vscode.TextDocumentWillSaveEvent) { - const document = event.document - const setup = _prepareWorksheet(document) - event.waitUntil(setup) + const worksheet = worksheets.get(event.document) + if (worksheet) { + const setup = _prepareWorksheet(worksheet) + event.waitUntil(setup) + } } -function _prepareWorksheet(document: vscode.TextDocument) { - if (isWorksheet(document)) { - return removeRedundantBlankLines(document) - .then(_ => { - removeDecorations(document) - worksheetMargin.set(document, longestLine(document) + 5) - worksheetInsertedLines.set(document, 0) - worksheetFinished.set(document, false) - }) - } else { - return Promise.resolve() - } +function _prepareWorksheet(worksheet: Worksheet) { + return removeRedundantBlankLines(worksheet.document).then(_ => worksheet.reset()) } -function showWorksheetProgress(document: vscode.TextDocument) { +function showWorksheetProgress(worksheet: Worksheet) { return vscode.window.withProgress({ location: vscode.ProgressLocation.Notification, title: "Evaluating worksheet", @@ -125,10 +139,7 @@ function showWorksheetProgress(document: vscode.TextDocument) { vscode.commands.executeCommand(worksheetCancelEvaluationKey) }) - function isFinished() { - return worksheetFinished.get(document) || false - } - return wait(isFinished, 500) + return wait(() => worksheet.finished, 500) }) } @@ -172,11 +183,15 @@ export function worksheetHandleMessage(message: string) { }) if (editor) { - let payload = message.slice(editor.document.uri.toString().length) - if (payload == "FINISHED") { - worksheetFinished.set(editor.document, true) - } else { - worksheetDisplayResult(payload, editor) + const worksheet = worksheets.get(editor.document) + + if (worksheet) { + let payload = message.slice(editor.document.uri.toString().length) + if (payload == "FINISHED") { + worksheet.finished = true + } else { + worksheetDisplayResult(payload, worksheet, editor) + } } } } @@ -222,16 +237,6 @@ function longestLine(document: vscode.TextDocument) { return maxLength } -/** - * Remove all decorations added by worksheet evaluation. - */ -function removeDecorations(document: vscode.TextDocument) { - const decorationTypes = worksheetDecorationTypes.get(document) || [] - decorationTypes.forEach(decoration => - decoration.dispose() - ) -} - /** * Remove the repeated blank lines in the source. * @@ -293,31 +298,24 @@ function removeRedundantBlankLines(document: vscode.TextDocument) { * * @see worksheetCreateDecoration * - * @param message The message to parse. - * @param ed The editor where to display the result. + * @param message The message to parse. + * @param worksheet The worksheet that receives the result. + * @param editor The editor where to display the result. * @return A `Thenable` that will insert necessary lines to fit the output * and display the decorations upon completion. */ -function worksheetDisplayResult(message: string, editor: vscode.TextEditor) { +function worksheetDisplayResult(message: string, worksheet: Worksheet, editor: vscode.TextEditor) { const colonIndex = message.indexOf(":") const lineNumber = parseInt(message.slice(0, colonIndex)) - 1 // lines are 0-indexed const evalResult = message.slice(colonIndex + 1) const resultLines = evalResult.trim().split(/\r\n|\r|\n/g) - const margin = worksheetMargin.get(editor.document) || 0 - - let insertedLines = worksheetInsertedLines.get(editor.document) || 0 - - let decorationTypes = worksheetDecorationTypes.get(editor.document) - if (!decorationTypes) { - decorationTypes = [] - worksheetDecorationTypes.set(editor.document, decorationTypes) - } + const margin = worksheet.margin // The line where the next decoration should be put. // It's the number of the line that produced the output, plus the number // of lines that we've inserted so far. - let actualLine = lineNumber + insertedLines + let actualLine = lineNumber + worksheet.insertedLines // If the output has more than one line, we need to insert blank lines // below the line that produced the output to fit the output. @@ -326,8 +324,7 @@ function worksheetDisplayResult(message: string, editor: vscode.TextEditor) { const linesToInsert = resultLines.length - 1 const editPos = new vscode.Position(actualLine + 1, 0) // add after the line addNewLinesEdit.insert(editor.document.uri, editPos, "\n".repeat(linesToInsert)) - insertedLines += linesToInsert - worksheetInsertedLines.set(editor.document, insertedLines) + worksheet.insertedLines += linesToInsert } return vscode.workspace.applyEdit(addNewLinesEdit).then(_ => { @@ -335,7 +332,7 @@ function worksheetDisplayResult(message: string, editor: vscode.TextEditor) { const decorationPosition = new vscode.Position(actualLine, 0) const decorationMargin = margin - editor.document.lineAt(actualLine).text.length const decorationType = worksheetCreateDecoration(decorationMargin, line) - if (decorationTypes) decorationTypes.push(decorationType) + worksheet.decorationTypes.push(decorationType) const decoration = { range: new vscode.Range(decorationPosition, decorationPosition), hoverMessage: line } editor.setDecorations(decorationType, [decoration]) From a3bdde12c850ea503893109aa4cb5ea0276dc504 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Mon, 1 Oct 2018 14:40:02 +0200 Subject: [PATCH 29/38] Remove worksheets when closed --- vscode-dotty/src/extension.ts | 5 +++++ vscode-dotty/src/worksheet.ts | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/vscode-dotty/src/extension.ts b/vscode-dotty/src/extension.ts index 52d4e454321c..c3d6a923d963 100644 --- a/vscode-dotty/src/extension.ts +++ b/vscode-dotty/src/extension.ts @@ -36,6 +36,11 @@ export function activate(context: ExtensionContext) { vscode.commands.executeCommand(worksheet.worksheetEvaluateAfterSaveKey) } }) + vscode.workspace.onDidCloseTextDocument(document => { + if (worksheet.isWorksheet(document)) { + worksheet.removeWorksheet(document) + } + }) vscode.commands.registerCommand(worksheet.worksheetEvaluateAfterSaveKey, () => { worksheet.evaluateCommand() diff --git a/vscode-dotty/src/worksheet.ts b/vscode-dotty/src/worksheet.ts index 2247b13b7af0..b1e6cab0ec9e 100644 --- a/vscode-dotty/src/worksheet.ts +++ b/vscode-dotty/src/worksheet.ts @@ -55,6 +55,11 @@ export const worksheetEvaluateAfterSaveKey = "worksheet.evaluateAfterSave" */ export const worksheetCancelEvaluationKey = "worksheet.cancel" +/** Remove the worksheet corresponding to the given document. */ +export function removeWorksheet(document: vscode.TextDocument) { + worksheets.delete(document) +} + /** Is this document a worksheet? */ export function isWorksheet(document: vscode.TextDocument): boolean { return document.fileName.endsWith(".sc") From de5c28e313bafcd58a921b913a1bf12c256c29e6 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Tue, 2 Oct 2018 16:18:04 +0200 Subject: [PATCH 30/38] Add `worksheet/exec` and `worksheet/publishOutput` Instead of relying on `textDocument/didSave` and `window/logMessage` like we did in the past. --- .../languageserver/DottyLanguageServer.scala | 55 +---- .../src/dotty/tools/languageserver/Main.scala | 14 +- .../languageserver/worksheet/Worksheet.scala | 9 +- .../worksheet/WorksheetClient.scala | 15 ++ .../worksheet/WorksheetMessages.scala | 15 ++ .../worksheet/WorksheetService.scala | 57 ++++++ .../util/actions/WorksheetAction.scala | 33 ++- .../util/actions/WorksheetCancel.scala | 20 +- .../util/actions/WorksheetEvaluate.scala | 21 +- .../util/server/TestClient.scala | 10 +- vscode-dotty/src/extension.ts | 20 +- vscode-dotty/src/worksheet.ts | 193 ++++++++---------- 12 files changed, 241 insertions(+), 221 deletions(-) create mode 100644 language-server/src/dotty/tools/languageserver/worksheet/WorksheetClient.scala create mode 100644 language-server/src/dotty/tools/languageserver/worksheet/WorksheetMessages.scala create mode 100644 language-server/src/dotty/tools/languageserver/worksheet/WorksheetService.scala diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index f85168c7cec4..e8996a7ad633 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -29,7 +29,7 @@ import Interactive.Include import config.Printers.interactiv import languageserver.config.ProjectConfig -import languageserver.worksheet.Worksheet +import languageserver.worksheet.{Worksheet, WorksheetClient, WorksheetService} import lsp4j.services._ @@ -42,7 +42,8 @@ import lsp4j.services._ * - This implementation is based on the LSP4J library: https://github.com/eclipse/lsp4j */ class DottyLanguageServer extends LanguageServer - with LanguageClientAware with TextDocumentService with WorkspaceService { thisServer => + with LanguageClientAware with TextDocumentService with WorkspaceService + with WorksheetService { thisServer => import ast.tpd._ import DottyLanguageServer._ @@ -54,7 +55,10 @@ class DottyLanguageServer extends LanguageServer private[this] var rootUri: String = _ - private[this] var client: LanguageClient = _ + + private[this] var myClient: WorksheetClient = _ + def client: WorksheetClient = myClient + private[this] val worksheets: ConcurrentHashMap[URI, CompletableFuture[_]] = new ConcurrentHashMap() private[this] var myDrivers: mutable.Map[ProjectConfig, InteractiveDriver] = _ @@ -121,7 +125,7 @@ class DottyLanguageServer extends LanguageServer } override def connect(client: LanguageClient): Unit = { - this.client = client + myClient = client.asInstanceOf[WorksheetClient] } override def exit(): Unit = { @@ -132,7 +136,7 @@ class DottyLanguageServer extends LanguageServer CompletableFuture.completedFuture(new Object) } - private[this] def computeAsync[R](fun: CancelChecker => R): CompletableFuture[R] = + def computeAsync[R](fun: CancelChecker => R): CompletableFuture[R] = CompletableFutures.computeAsync { cancelToken => // We do not support any concurrent use of the compiler currently. thisServer.synchronized { @@ -229,27 +233,7 @@ class DottyLanguageServer extends LanguageServer /*thisServer.synchronized*/ {} override def didSave(params: DidSaveTextDocumentParams): Unit = { - val uri = new URI(params.getTextDocument.getUri) - if (isWorksheet(uri)) { - if (uri.getScheme == "cancel") { - val fileURI = new URI("file", uri.getUserInfo, uri.getHost, uri.getPort, uri.getPath, uri.getQuery, uri.getFragment) - Option(worksheets.get(fileURI)).foreach(_.cancel(true)) - } else thisServer.synchronized { - val sendMessage = (msg: String) => client.logMessage(new MessageParams(MessageType.Info, uri + msg)) - worksheets.put( - uri, - computeAsync { cancelChecker => - try { - val driver = driverFor(uri) - evaluateWorksheet(driver, uri, sendMessage, cancelChecker)(driver.currentCtx) - } finally { - worksheets.remove(uri) - sendMessage("FINISHED") - } - } - ) - } - } + /*thisServer.synchronized*/ {} } // FIXME: share code with messages.NotAMember @@ -447,25 +431,6 @@ class DottyLanguageServer extends LanguageServer override def resolveCodeLens(params: CodeLens) = null override def resolveCompletionItem(params: CompletionItem) = null override def signatureHelp(params: TextDocumentPositionParams) = null - - /** - * Evaluate the worksheet at `uri`. - * - * @param driver The driver for the project that contains the worksheet. - * @param uri The URI of the worksheet. - * @param sendMessage A mean of communicating the results of evaluation back. - * @param cancelChecker Token to check whether evaluation was cancelled - */ - private def evaluateWorksheet(driver: InteractiveDriver, - uri: URI, - sendMessage: String => Unit, - cancelChecker: CancelChecker)( - implicit ctx: Context): Unit = { - val trees = driver.openedTrees(uri) - trees.headOption.foreach { tree => - Worksheet.evaluate(tree, sendMessage, cancelChecker) - } - } } object DottyLanguageServer { diff --git a/language-server/src/dotty/tools/languageserver/Main.scala b/language-server/src/dotty/tools/languageserver/Main.scala index fbdf9e4efdbb..5223f0d2c63b 100644 --- a/language-server/src/dotty/tools/languageserver/Main.scala +++ b/language-server/src/dotty/tools/languageserver/Main.scala @@ -10,6 +10,7 @@ import java.nio.channels._ import org.eclipse.lsp4j._ import org.eclipse.lsp4j.services._ import org.eclipse.lsp4j.launch._ +import org.eclipse.lsp4j.jsonrpc.Launcher /** Run the Dotty Language Server. * @@ -65,9 +66,16 @@ object Main { val server = new DottyLanguageServer println("Starting server") - // For debugging JSON messages: - // val launcher = LSPLauncher.createServerLauncher(server, in, out, false, new java.io.PrintWriter(System.err, true)) - val launcher = LSPLauncher.createServerLauncher(server, in, out) + val launcher = + new Launcher.Builder[worksheet.WorksheetClient]() + .setLocalService(server) + .setRemoteInterface(classOf[worksheet.WorksheetClient]) + .setInput(in) + .setOutput(out) + // For debugging JSON messages: + // .traceMessages(new java.io.PrintWriter(System.err, true)) + .create(); + val client = launcher.getRemoteProxy() server.connect(client) launcher.startListening() diff --git a/language-server/src/dotty/tools/languageserver/worksheet/Worksheet.scala b/language-server/src/dotty/tools/languageserver/worksheet/Worksheet.scala index 1e1442053b19..2f6e7aa7cd3f 100644 --- a/language-server/src/dotty/tools/languageserver/worksheet/Worksheet.scala +++ b/language-server/src/dotty/tools/languageserver/worksheet/Worksheet.scala @@ -22,13 +22,13 @@ object Worksheet { * @param cancelChecker A token to check whether execution should be cancelled. */ def evaluate(tree: SourceTree, - sendMessage: String => Unit, + sendMessage: (Int, String) => Unit, cancelChecker: CancelChecker)( implicit ctx: Context): Unit = synchronized { Evaluator.get(cancelChecker) match { case None => - sendMessage(encode("Couldn't start JVM.", 1)) + sendMessage(1, "Couldn't start JVM.") case Some(evaluator) => tree.tree match { case td @ TypeDef(_, template: Template) => @@ -42,7 +42,7 @@ object Worksheet { try { cancelChecker.checkCanceled() val (line, result) = execute(evaluator, statement, tree.source) - if (result.nonEmpty) sendMessage(encode(result, line)) + if (result.nonEmpty) sendMessage(line, result) } catch { case _: CancellationException => () } case _ => @@ -66,9 +66,6 @@ object Worksheet { (line, evaluator.eval(source).getOrElse("")) } - private def encode(message: String, line: Int): String = - line + ":" + message - private def bounds(pos: Position): (Int, Int) = (pos.start, pos.end) } diff --git a/language-server/src/dotty/tools/languageserver/worksheet/WorksheetClient.scala b/language-server/src/dotty/tools/languageserver/worksheet/WorksheetClient.scala new file mode 100644 index 000000000000..d0ee71670162 --- /dev/null +++ b/language-server/src/dotty/tools/languageserver/worksheet/WorksheetClient.scala @@ -0,0 +1,15 @@ +package dotty.tools.languageserver.worksheet + +import org.eclipse.lsp4j.services.LanguageClient +import org.eclipse.lsp4j.jsonrpc.services.JsonNotification + +/** + * A `LanguageClient` that supports the `worksheet/publishOutput` notification. + * + * @see dotty.tools.languageserver.worksheet.WorksheetExecOutput + */ +trait WorksheetClient extends LanguageClient { + @JsonNotification("worksheet/publishOutput") + def publishOutput(output: WorksheetExecOutput): Unit +} + diff --git a/language-server/src/dotty/tools/languageserver/worksheet/WorksheetMessages.scala b/language-server/src/dotty/tools/languageserver/worksheet/WorksheetMessages.scala new file mode 100644 index 000000000000..7985d4c0c434 --- /dev/null +++ b/language-server/src/dotty/tools/languageserver/worksheet/WorksheetMessages.scala @@ -0,0 +1,15 @@ +package dotty.tools.languageserver.worksheet + +import java.net.URI + +/** The parameter for the `worksheet/exec` request. */ +case class WorksheetExecParams(uri: URI) + +/** The response to a `worksheet/exec` request. */ +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) diff --git a/language-server/src/dotty/tools/languageserver/worksheet/WorksheetService.scala b/language-server/src/dotty/tools/languageserver/worksheet/WorksheetService.scala new file mode 100644 index 000000000000..a460d431334c --- /dev/null +++ b/language-server/src/dotty/tools/languageserver/worksheet/WorksheetService.scala @@ -0,0 +1,57 @@ +package dotty.tools.languageserver.worksheet + +import dotty.tools.dotc.core.Contexts.Context +import dotty.tools.dotc.interactive.InteractiveDriver +import dotty.tools.languageserver.DottyLanguageServer + +import org.eclipse.lsp4j.jsonrpc._//{CancelChecker, CompletableFutures} +import org.eclipse.lsp4j.jsonrpc.services._//{JsonSegment, JsonRequest} + +import java.net.URI +import java.util.concurrent.{CompletableFuture, ConcurrentHashMap} + +@JsonSegment("worksheet") +trait WorksheetService { thisServer: DottyLanguageServer => + + val worksheets: ConcurrentHashMap[URI, CompletableFuture[_]] = new ConcurrentHashMap() + + @JsonRequest + def exec(params: WorksheetExecParams): CompletableFuture[WorksheetExecResponse] = thisServer.synchronized { + val uri = params.uri + val future = + computeAsync { cancelChecker => + try { + val driver = driverFor(uri) + val sendMessage = (line: Int, msg: String) => client.publishOutput(WorksheetExecOutput(uri, line, msg)) + evaluateWorksheet(driver, uri, sendMessage, cancelChecker)(driver.currentCtx) + WorksheetExecResponse(success = true) + } catch { + case _: Throwable => + WorksheetExecResponse(success = false) + } finally { + worksheets.remove(uri) + } + } + worksheets.put(uri, future) + future + } + + /** + * Evaluate the worksheet at `uri`. + * + * @param driver The driver for the project that contains the worksheet. + * @param uri The URI of the worksheet. + * @param sendMessage A mean of communicating the results of evaluation back. + * @param cancelChecker Token to check whether evaluation was cancelled + */ + private def evaluateWorksheet(driver: InteractiveDriver, + uri: URI, + sendMessage: (Int, String) => Unit, + cancelChecker: CancelChecker)( + implicit ctx: Context): Unit = { + val trees = driver.openedTrees(uri) + trees.headOption.foreach { tree => + Worksheet.evaluate(tree, sendMessage, cancelChecker) + } + } +} diff --git a/language-server/test/dotty/tools/languageserver/util/actions/WorksheetAction.scala b/language-server/test/dotty/tools/languageserver/util/actions/WorksheetAction.scala index 1bb35dde0097..aaef6ea48ab0 100644 --- a/language-server/test/dotty/tools/languageserver/util/actions/WorksheetAction.scala +++ b/language-server/test/dotty/tools/languageserver/util/actions/WorksheetAction.scala @@ -1,31 +1,26 @@ package dotty.tools.languageserver.util.actions +import dotty.tools.languageserver.worksheet.{WorksheetExecOutput, WorksheetExecParams, WorksheetExecResponse} import dotty.tools.languageserver.util.embedded.CodeMarker -import org.eclipse.lsp4j.{DidSaveTextDocumentParams, MessageParams, MessageType} +import java.net.URI +import java.util.concurrent.CompletableFuture abstract class WorksheetAction extends Action { - def triggerEvaluation(marker: CodeMarker): Exec[Unit] = { - val file = marker.toTextDocumentIdentifier - server.didSave(new DidSaveTextDocumentParams(file)) - } + /** Get the URI of the worksheet that contains `marker`. */ + def getUri(marker: CodeMarker): Exec[URI] = new URI(marker.toTextDocumentIdentifier.getUri) - def triggerCancellation(marker: CodeMarker): Exec[Unit] = { - val file = { - val file = marker.toTextDocumentIdentifier - file.setUri(file.getUri.replaceFirst("file:", "cancel:")) - file - } - server.didSave(new DidSaveTextDocumentParams(file)) + /** Triggers the evaluation of the worksheet. */ + def triggerEvaluation(marker: CodeMarker): Exec[CompletableFuture[WorksheetExecResponse]] = { + val uri = getUri(marker) + server.exec(WorksheetExecParams(uri)) } - def getLogs(marker: CodeMarker): Exec[List[String]] = { - def matches(params: MessageParams): Boolean = - params.getType == MessageType.Info && params.getMessage.startsWith(marker.file.uri) - client.log.get.collect { - case params: MessageParams if matches(params) => - params.getMessage.substring(marker.file.uri.length).trim - } + /** 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) } + } diff --git a/language-server/test/dotty/tools/languageserver/util/actions/WorksheetCancel.scala b/language-server/test/dotty/tools/languageserver/util/actions/WorksheetCancel.scala index 0c7be1413ccd..f6374e72ac99 100644 --- a/language-server/test/dotty/tools/languageserver/util/actions/WorksheetCancel.scala +++ b/language-server/test/dotty/tools/languageserver/util/actions/WorksheetCancel.scala @@ -3,26 +3,20 @@ package dotty.tools.languageserver.util.actions import dotty.tools.languageserver.util.PositionContext import dotty.tools.languageserver.util.embedded.CodeMarker -import org.junit.Assert.{assertEquals, fail} +import org.junit.Assert.assertTrue -class WorksheetCancel(marker: CodeMarker, afterMs: Long) extends WorksheetAction { +import java.util.concurrent.TimeUnit - private final val cancellationTimeoutMs = 10 * 1000 +class WorksheetCancel(marker: CodeMarker, afterMs: Long) extends WorksheetAction { override def execute(): Exec[Unit] = { - triggerEvaluation(marker) + val futureResult = triggerEvaluation(marker) Thread.sleep(afterMs) - triggerCancellation(marker) + val cancelled = futureResult.cancel(true) - val timeAtCancellation = System.currentTimeMillis() - while (!getLogs(marker).contains("FINISHED")) { - if (System.currentTimeMillis() - timeAtCancellation > cancellationTimeoutMs) { - fail(s"Couldn't cancel worksheet evaluation after ${cancellationTimeoutMs} ms.") - } - Thread.sleep(100) - } + assertTrue(cancelled) - client.log.clear() + client.worksheetOutput.clear() } override def show: PositionContext.PosCtx[String] = diff --git a/language-server/test/dotty/tools/languageserver/util/actions/WorksheetEvaluate.scala b/language-server/test/dotty/tools/languageserver/util/actions/WorksheetEvaluate.scala index 647ace90ff64..0c3af2403292 100644 --- a/language-server/test/dotty/tools/languageserver/util/actions/WorksheetEvaluate.scala +++ b/language-server/test/dotty/tools/languageserver/util/actions/WorksheetEvaluate.scala @@ -3,31 +3,26 @@ package dotty.tools.languageserver.util.actions import dotty.tools.languageserver.util.PositionContext import dotty.tools.languageserver.util.embedded.CodeMarker +import java.util.concurrent.TimeUnit + import org.junit.Assert.{assertEquals, assertTrue, fail} class WorksheetEvaluate(marker: CodeMarker, expected: Seq[String], strict: Boolean) extends WorksheetAction { - private final val evaluationTimeoutMs = 30 * 1000 - override def execute(): Exec[Unit] = { - triggerEvaluation(marker) + val result = triggerEvaluation(marker).get(30, TimeUnit.SECONDS) + assertTrue(result.success) - val timeAtEvaluation = System.currentTimeMillis() - while (!getLogs(marker).contains("FINISHED")) { - if (System.currentTimeMillis() - timeAtEvaluation > evaluationTimeoutMs) { - fail(s"Evaluation didn't finish after ${evaluationTimeoutMs} ms.") - } - Thread.sleep(100) - } + val logs = worksheetOutput(marker).map(out => s"${out.line}:${out.content}") if (strict) { - assertEquals(expected, getLogs(marker).init) + assertEquals(expected, logs) } else { - expected.zip(getLogs(marker).init).foreach { + expected.zip(logs).foreach { case (expected, message) => assertTrue(s"'$message' didn't start with '$expected'", message.startsWith(expected)) } } - client.log.clear() + client.worksheetOutput.clear() } override def show: PositionContext.PosCtx[String] = diff --git a/language-server/test/dotty/tools/languageserver/util/server/TestClient.scala b/language-server/test/dotty/tools/languageserver/util/server/TestClient.scala index b76307851329..b4aa5b815870 100644 --- a/language-server/test/dotty/tools/languageserver/util/server/TestClient.scala +++ b/language-server/test/dotty/tools/languageserver/util/server/TestClient.scala @@ -1,5 +1,7 @@ package dotty.tools.languageserver.util.server +import dotty.tools.languageserver.worksheet.{WorksheetExecOutput, WorksheetClient} + import java.util.concurrent.CompletableFuture import org.eclipse.lsp4j._ @@ -7,7 +9,7 @@ import org.eclipse.lsp4j.services._ import scala.collection.mutable.Buffer -class TestClient extends LanguageClient { +class TestClient extends WorksheetClient { class Log[T] { private[this] val log = Buffer.empty[T] @@ -20,7 +22,7 @@ class TestClient extends LanguageClient { val log = new Log[MessageParams] val diagnostics = new Log[PublishDiagnosticsParams] val telemetry = new Log[Any] - + val worksheetOutput = new Log[WorksheetExecOutput] override def logMessage(message: MessageParams) = { log += message @@ -43,4 +45,8 @@ class TestClient extends LanguageClient { diagnostics += diagnosticsParams } + override def publishOutput(output: WorksheetExecOutput) = { + worksheetOutput += output + } + } diff --git a/vscode-dotty/src/extension.ts b/vscode-dotty/src/extension.ts index c3d6a923d963..e9c25ad5ec2c 100644 --- a/vscode-dotty/src/extension.ts +++ b/vscode-dotty/src/extension.ts @@ -16,7 +16,7 @@ import * as worksheet from './worksheet' let extensionContext: ExtensionContext let outputChannel: vscode.OutputChannel -let client: LanguageClient +export let client: LanguageClient export function activate(context: ExtensionContext) { extensionContext = context @@ -33,7 +33,7 @@ export function activate(context: ExtensionContext) { vscode.workspace.onWillSaveTextDocument(worksheet.prepareWorksheet) vscode.workspace.onDidSaveTextDocument(document => { if (worksheet.isWorksheet(document)) { - vscode.commands.executeCommand(worksheet.worksheetEvaluateAfterSaveKey) + worksheet.evaluateWorksheet(document) } }) vscode.workspace.onDidCloseTextDocument(document => { @@ -42,10 +42,6 @@ export function activate(context: ExtensionContext) { } }) - vscode.commands.registerCommand(worksheet.worksheetEvaluateAfterSaveKey, () => { - worksheet.evaluateCommand() - }) - if (process.env['DLS_DEV_MODE']) { const portFile = `${vscode.workspace.rootPath}/.dotty-ide-dev-port` fs.readFile(portFile, (err, port) => { @@ -209,20 +205,14 @@ function run(serverOptions: ServerOptions, isOldServer: boolean) { if (isOldServer) enableOldServerWorkaround(client) - // We use the `window/logMessage` command to communicate back the result of evaluating - // a worksheet. client.onReady().then(() => { - client.onNotification("window/logMessage", (params) => { - worksheet.worksheetHandleMessage(params.message) + client.onNotification("worksheet/publishOutput", (params) => { + worksheet.handleMessage(params) }) }) vscode.commands.registerCommand(worksheet.worksheetEvaluateKey, () => { - worksheet.worksheetSave(client) - }) - - vscode.commands.registerCommand(worksheet.worksheetCancelEvaluationKey, () => { - worksheet.cancelExecution(client) + worksheet.evaluateWorksheetCommand() }) // Push the disposable to the context's subscriptions so that the diff --git a/vscode-dotty/src/worksheet.ts b/vscode-dotty/src/worksheet.ts index b1e6cab0ec9e..e220ae8fa2ba 100644 --- a/vscode-dotty/src/worksheet.ts +++ b/vscode-dotty/src/worksheet.ts @@ -1,10 +1,12 @@ import * as vscode from 'vscode' import { LanguageClient } from 'vscode-languageclient' +import * as rpc from 'vscode-jsonrpc' +import { client } from './extension' /** A worksheet managed by vscode */ class Worksheet { - constructor(document: vscode.TextDocument) { + private constructor(document: vscode.TextDocument) { this.document = document } @@ -31,10 +33,52 @@ class Worksheet { this.finished = false } + /** All the worksheets */ + private static worksheets: Map = new Map() + + /** + * If `document` is a worksheet, create a new worksheet for it, or return the existing one. */ + static getOrNewWorksheet(document: vscode.TextDocument): Worksheet | undefined { + if (!isWorksheet(document)) return + else { + const existing = Worksheet.worksheets.get(document) + if (existing) { + return existing + } else { + const newWorksheet = new Worksheet(document) + Worksheet.worksheets.set(document, newWorksheet) + return newWorksheet + } + } + } + + /** If it exists, remove the worksheet representing `document`. */ + static delete(document: vscode.TextDocument) { + Worksheet.worksheets.delete(document) + } } -/** All the worksheets */ -let worksheets: Map = new Map() +/** The parameter for the `worksheet/exec` request. */ +class WorksheetExec { + constructor(uri: string) { + this.uri = uri + } + + readonly uri: string +} + +/** The parameter for the `worksheet/publishOutput` notification. */ +class WorksheetOutput { + constructor(uri: vscode.Uri, line: number, content: string) { + this.uri = uri + this.line = line + this.content = content + } + + readonly uri: vscode.Uri + readonly line: number + readonly content: string +} /** * The command key for evaluating a worksheet. Exposed to users as @@ -42,22 +86,9 @@ let worksheets: Map = new Map { - if (document.isDirty) document.save() - else { - client.sendNotification("textDocument/didSave", { - textDocument: { uri: document.uri.toString() } - }) - showWorksheetProgress(worksheet) - } - }) + const worksheet = Worksheet.getOrNewWorksheet(document) + if (worksheet) { + _prepareWorksheet(worksheet).then(_ => { + if (document.isDirty) document.save() // This will trigger evaluation + else evaluateWorksheet(document) + }) + } } } } +/** + * Evaluate the worksheet in `document`, display a progress bar during evaluation. + */ +export function evaluateWorksheet(document: vscode.TextDocument): Thenable<{}> { + + const worksheet = Worksheet.getOrNewWorksheet(document) + if (worksheet) { + return vscode.window.withProgress({ + location: vscode.ProgressLocation.Notification, + title: "Evaluating worksheet", + cancellable: true + }, (_, token) => { + return client.sendRequest("worksheet/exec", new WorksheetExec(worksheet.document.uri.toString()), token) + }) + } else { + return Promise.reject() + } +} + /** * If the document that will be saved is a worksheet, resets the "worksheet state" * (margin and number of inserted lines), and removes redundant blank lines that @@ -123,7 +153,7 @@ export function worksheetSave(client: LanguageClient) { * @param event `TextDocumentWillSaveEvent`. */ export function prepareWorksheet(event: vscode.TextDocumentWillSaveEvent) { - const worksheet = worksheets.get(event.document) + const worksheet = Worksheet.getOrNewWorksheet(event.document) if (worksheet) { const setup = _prepareWorksheet(worksheet) event.waitUntil(setup) @@ -134,69 +164,24 @@ function _prepareWorksheet(worksheet: Worksheet) { return removeRedundantBlankLines(worksheet.document).then(_ => worksheet.reset()) } -function showWorksheetProgress(worksheet: Worksheet) { - return vscode.window.withProgress({ - location: vscode.ProgressLocation.Notification, - title: "Evaluating worksheet", - cancellable: true - }, (_, token) => { - token.onCancellationRequested(_ => { - vscode.commands.executeCommand(worksheetCancelEvaluationKey) - }) - - return wait(() => worksheet.finished, 500) - }) -} - -/** - * Cancel the execution of the worksheet. - * - * This sends a `textDocument/didSave` message to the language server - * with the scheme of the URI set to `cancel`. The language server - * interprets that as a request to cancel the execution of the specified worksheet. - */ -export function cancelExecution(client: LanguageClient) { - const editor = vscode.window.activeTextEditor - if (editor) { - const document = editor.document - client.sendNotification("textDocument/didSave", { - textDocument: { uri: document.uri.with({ scheme: "cancel" }).toString() } - }) - } -} - -/** Wait until `cond` evaluates to true; test every `delay` ms. */ -function wait(cond: () => boolean, delayMs: number): Promise { - const isFinished = cond() - if (isFinished) { - return Promise.resolve(true) - } - else return new Promise(fn => setTimeout(fn, delayMs)).then(_ => wait(cond, delayMs)) -} - /** * Handle the result of evaluating part of a worksheet. * This is called when we receive a `window/logMessage`. * * @param message The result of evaluating part of a worksheet. */ -export function worksheetHandleMessage(message: string) { +export function handleMessage(output: WorksheetOutput) { const editor = vscode.window.visibleTextEditors.find(e => { - let uri = e.document.uri.toString() - return uri == message.slice(0, uri.length) + let uri = e.document.uri + return uri == output.uri }) if (editor) { - const worksheet = worksheets.get(editor.document) + const worksheet = Worksheet.getOrNewWorksheet(editor.document) if (worksheet) { - let payload = message.slice(editor.document.uri.toString().length) - if (payload == "FINISHED") { - worksheet.finished = true - } else { - worksheetDisplayResult(payload, worksheet, editor) - } + worksheetDisplayResult(output.line - 1, output.content, worksheet, editor) } } } @@ -303,17 +288,15 @@ function removeRedundantBlankLines(document: vscode.TextDocument) { * * @see worksheetCreateDecoration * - * @param message The message to parse. - * @param worksheet The worksheet that receives the result. - * @param editor The editor where to display the result. + * @param lineNumber The number of the line in the source that produced the result. + * @param evalResult The evaluation result. + * @param worksheet The worksheet that receives the result. + * @param editor The editor where to display the result. * @return A `Thenable` that will insert necessary lines to fit the output * and display the decorations upon completion. */ -function worksheetDisplayResult(message: string, worksheet: Worksheet, editor: vscode.TextEditor) { +function worksheetDisplayResult(lineNumber: number, evalResult: string, worksheet: Worksheet, editor: vscode.TextEditor) { - const colonIndex = message.indexOf(":") - const lineNumber = parseInt(message.slice(0, colonIndex)) - 1 // lines are 0-indexed - const evalResult = message.slice(colonIndex + 1) const resultLines = evalResult.trim().split(/\r\n|\r|\n/g) const margin = worksheet.margin From 0689ef31702e613fae809203561644f19069574c Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 3 Oct 2018 09:13:48 +0200 Subject: [PATCH 31/38] Add empty secondary ctors to worksheet messages They are used when deserializing from JSON. --- .../worksheet/WorksheetMessages.scala | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/worksheet/WorksheetMessages.scala b/language-server/src/dotty/tools/languageserver/worksheet/WorksheetMessages.scala index 7985d4c0c434..e6cdee2a4f89 100644 --- a/language-server/src/dotty/tools/languageserver/worksheet/WorksheetMessages.scala +++ b/language-server/src/dotty/tools/languageserver/worksheet/WorksheetMessages.scala @@ -3,13 +3,25 @@ package dotty.tools.languageserver.worksheet import java.net.URI /** The parameter for the `worksheet/exec` request. */ -case class WorksheetExecParams(uri: URI) +case class WorksheetExecParams(uri: URI) { + // Used for deserialization + // see https://github.com/lampepfl/dotty/pull/5102#discussion_r222055355 + def this() = this(null) +} /** The response to a `worksheet/exec` request. */ -case class WorksheetExecResponse(success: Boolean) +case class WorksheetExecResponse(success: Boolean) { + // Used for deserialization + // see https://github.com/lampepfl/dotty/pull/5102#discussion_r222055355 + def this() = this(false) +} /** * 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(uri: URI, line: Int, content: String) { + // Used for deserialization + // see https://github.com/lampepfl/dotty/pull/5102#discussion_r222055355 + def this() = this(null, 0, null) +} From 9ed69710974d8c51a21774b47439397dfc9dbb23 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 3 Oct 2018 13:49:39 +0200 Subject: [PATCH 32/38] Address review comments --- .../worksheet/CancellationThread.scala | 2 +- .../languageserver/worksheet/Evaluator.scala | 6 +-- .../worksheet/WorksheetMessages.scala | 6 +-- .../worksheet/WorksheetService.scala | 4 +- .../util/actions/WorksheetAction.scala | 12 +++-- .../util/embedded/CodeMarker.scala | 3 ++ vscode-dotty/src/worksheet.ts | 44 +++++++++---------- 7 files changed, 37 insertions(+), 40 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/worksheet/CancellationThread.scala b/language-server/src/dotty/tools/languageserver/worksheet/CancellationThread.scala index e4d6b3c306a5..f4dbf500e810 100644 --- a/language-server/src/dotty/tools/languageserver/worksheet/CancellationThread.scala +++ b/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 diff --git a/language-server/src/dotty/tools/languageserver/worksheet/Evaluator.scala b/language-server/src/dotty/tools/languageserver/worksheet/Evaluator.scala index 27c9ac774e68..c8c346b7c695 100644 --- a/language-server/src/dotty/tools/languageserver/worksheet/Evaluator.scala +++ b/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) diff --git a/language-server/src/dotty/tools/languageserver/worksheet/WorksheetMessages.scala b/language-server/src/dotty/tools/languageserver/worksheet/WorksheetMessages.scala index e6cdee2a4f89..6ddb72903573 100644 --- a/language-server/src/dotty/tools/languageserver/worksheet/WorksheetMessages.scala +++ b/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) diff --git a/language-server/src/dotty/tools/languageserver/worksheet/WorksheetService.scala b/language-server/src/dotty/tools/languageserver/worksheet/WorksheetService.scala index a460d431334c..2436d834784c 100644 --- a/language-server/src/dotty/tools/languageserver/worksheet/WorksheetService.scala +++ b/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 { diff --git a/language-server/test/dotty/tools/languageserver/util/actions/WorksheetAction.scala b/language-server/test/dotty/tools/languageserver/util/actions/WorksheetAction.scala index aaef6ea48ab0..a60be99b7e0d 100644 --- a/language-server/test/dotty/tools/languageserver/util/actions/WorksheetAction.scala +++ b/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) } } diff --git a/language-server/test/dotty/tools/languageserver/util/embedded/CodeMarker.scala b/language-server/test/dotty/tools/languageserver/util/embedded/CodeMarker.scala index 25d8984567b5..8c198ec12900 100644 --- a/language-server/test/dotty/tools/languageserver/util/embedded/CodeMarker.scala +++ b/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) diff --git a/vscode-dotty/src/worksheet.ts b/vscode-dotty/src/worksheet.ts index e220ae8fa2ba..803b723badd2 100644 --- a/vscode-dotty/src/worksheet.ts +++ b/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", + } + }) } /** From 9666d23664bc3d8d88cc3006a8b6e7e758a0e36a Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 3 Oct 2018 14:14:49 +0200 Subject: [PATCH 33/38] Remove only lines added to fit output --- vscode-dotty/src/worksheet.ts | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/vscode-dotty/src/worksheet.ts b/vscode-dotty/src/worksheet.ts index 803b723badd2..d6b972cebbdc 100644 --- a/vscode-dotty/src/worksheet.ts +++ b/vscode-dotty/src/worksheet.ts @@ -18,6 +18,9 @@ class Worksheet { /** The number of blank lines that have been inserted to fit the output so far. */ insertedLines: number = 0 + /** The lines that contain decorations */ + decoratedLines: Set = new Set() + /** The minimum margin to add so that the decoration is shown after all text. */ margin: number = 0 @@ -28,6 +31,7 @@ class Worksheet { reset() { this.decorationTypes.forEach(decoration => decoration.dispose()) this.insertedLines = 0 + this.decoratedLines.clear() this.margin = longestLine(this.document) + 5 this.finished = false } @@ -160,7 +164,7 @@ export function prepareWorksheet(event: vscode.TextDocumentWillSaveEvent) { } function _prepareWorksheet(worksheet: Worksheet) { - return removeRedundantBlankLines(worksheet.document).then(_ => worksheet.reset()) + return removeRedundantBlankLines(worksheet).then(_ => worksheet.reset()) } /** @@ -228,14 +232,18 @@ function longestLine(document: vscode.TextDocument) { * * Evaluating a worksheet can insert new lines in the worksheet so that the * output of a line fits below the line. Before evaluation, we remove blank - * lines in the worksheet to keep its length under control. This could potentially - * remove manually added blank lines. + * lines in the worksheet to keep its length under control. * - * @param document The document where blank lines must be removed. + * @param worksheet The worksheet where blank lines must be removed. * @return A `Thenable` removing the blank lines upon completion. */ -function removeRedundantBlankLines(document: vscode.TextDocument) { +function removeRedundantBlankLines(worksheet: Worksheet) { + + function hasDecoration(line: number): boolean { + return worksheet.decoratedLines.has(line) + } + const document = worksheet.document const lineCount = document.lineCount let rangesToRemove: vscode.Range[] = [] let rangeStart = 0 @@ -245,14 +253,13 @@ function removeRedundantBlankLines(document: vscode.TextDocument) { function addRange() { inRange = false if (rangeStart < rangeEnd) { - // Keep one line between separate chunks of code - rangesToRemove.push(new vscode.Range(rangeStart, 0, rangeEnd - 1, 0)) + rangesToRemove.push(new vscode.Range(rangeStart, 0, rangeEnd, 0)) } return } for (let i = 0; i < lineCount; ++i) { - const isEmpty = document.lineAt(i).isEmptyOrWhitespace + const isEmpty = document.lineAt(i).isEmptyOrWhitespace && hasDecoration(i) if (inRange) { if (isEmpty) rangeEnd += 1 else addRange() @@ -317,6 +324,7 @@ function worksheetDisplayResult(lineNumber: number, evalResult: string, workshee const decorationMargin = margin - editor.document.lineAt(actualLine).text.length const decorationType = worksheetCreateDecoration(decorationMargin, line) worksheet.decorationTypes.push(decorationType) + worksheet.decoratedLines.add(actualLine) const decoration = { range: new vscode.Range(decorationPosition, decorationPosition), hoverMessage: line } editor.setDecorations(decorationType, [decoration]) From a3efd480d154b2b4956ebb1904150f71906036cc Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Thu, 4 Oct 2018 10:06:00 +0200 Subject: [PATCH 34/38] Cancel worksheet execution on `didChange` --- .../languageserver/DottyLanguageServer.scala | 34 +++++++++++-------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index e8996a7ad633..3ec566d38f4f 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -59,8 +59,6 @@ class DottyLanguageServer extends LanguageServer private[this] var myClient: WorksheetClient = _ def client: WorksheetClient = myClient - private[this] val worksheets: ConcurrentHashMap[URI, CompletableFuture[_]] = new ConcurrentHashMap() - private[this] var myDrivers: mutable.Map[ProjectConfig, InteractiveDriver] = _ def drivers: Map[ProjectConfig, InteractiveDriver] = thisServer.synchronized { @@ -198,25 +196,33 @@ class DottyLanguageServer extends LanguageServer diags.flatMap(diagnostic(_, positionMapper)).asJava)) } - override def didChange(params: DidChangeTextDocumentParams): Unit = thisServer.synchronized { - checkMemory() + override def didChange(params: DidChangeTextDocumentParams): Unit = { val document = params.getTextDocument val uri = new URI(document.getUri) - val driver = driverFor(uri) val worksheetMode = isWorksheet(uri) - val change = params.getContentChanges.get(0) - assert(change.getRange == null, "TextDocumentSyncKind.Incremental support is not implemented") + if (worksheetMode) { + Option(worksheets.get(uri)).foreach(_.cancel(true)) + } - val (text, positionMapper) = - if (worksheetMode) (wrapWorksheet(change.getText), Some(toUnwrappedPosition _)) - else (change.getText, None) + thisServer.synchronized { + checkMemory() - val diags = driver.run(uri, text) + val driver = driverFor(uri) - client.publishDiagnostics(new PublishDiagnosticsParams( - document.getUri, - diags.flatMap(diagnostic(_, positionMapper)).asJava)) + 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 diags = driver.run(uri, text) + + client.publishDiagnostics(new PublishDiagnosticsParams( + document.getUri, + diags.flatMap(diagnostic(_, positionMapper)).asJava)) + } } override def didClose(params: DidCloseTextDocumentParams): Unit = thisServer.synchronized { From 2b7e8cf3dd13366e54f44edd0329b87e37a38f60 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Thu, 4 Oct 2018 12:26:47 +0200 Subject: [PATCH 35/38] Error message for pure expr in stat position --- .../dotc/reporting/diagnostic/ErrorMessageID.java | 3 ++- .../tools/dotc/reporting/diagnostic/messages.scala | 10 ++++++++++ compiler/src/dotty/tools/dotc/typer/Typer.scala | 2 +- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java b/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java index 2e2182abd131..14c8c1ecf7ec 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java @@ -135,7 +135,8 @@ public enum ErrorMessageID { CaseClassCannotExtendEnumID, ValueClassParameterMayNotBeCallByNameID, NotAnExtractorID, - MemberWithSameNameAsStaticID + MemberWithSameNameAsStaticID, + PureExpressionInStatementPositionID ; public int errorNumber() { diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 0b324812f517..bd19f7954f43 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -2139,4 +2139,14 @@ object messages { override def kind: String = "Syntax" override def explanation: String = "" } + + case class PureExpressionInStatementPosition(stat: untpd.Tree, exprOwner: Symbol)(implicit ctx: Context) + extends Message(PureExpressionInStatementPositionID) { + + val kind = "Potential Issue" + val msg = "a pure expression does nothing in statement position; you may be omitting necessary parentheses" + val explanation = + hl"""The pure expression `$stat` doesn't have any side effect and its result is not assigned elsewhere. + |It can be removed without changing the semantics of the program. This may indicate an error.""".stripMargin + } } diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 861f9733ff62..a2019534f989 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2013,7 +2013,7 @@ class Typer extends Namer val stat1 = typed(stat)(ctx.exprContext(stat, exprOwner)) if (!ctx.isAfterTyper && isPureExpr(stat1) && !stat1.tpe.isRef(defn.UnitClass) && !isSelfOrSuperConstrCall(stat1)) - ctx.warning(em"a pure expression does nothing in statement position", stat.pos) + ctx.warning(PureExpressionInStatementPosition(stat, exprOwner), stat.pos) buf += stat1 traverse(rest) case nil => From dba854d9452f2b3548890860747765f359fa8f7b Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Thu, 4 Oct 2018 12:50:00 +0200 Subject: [PATCH 36/38] Filter pure expression warning in worksheets We now hide the warning the pure expression in statement position when the pure expression is an immediate children of the worksheet wrapper. --- .../languageserver/DottyLanguageServer.scala | 58 +++++++++++++++---- 1 file changed, 48 insertions(+), 10 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index 3ec566d38f4f..92449f2d1bca 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -21,7 +21,7 @@ import ast.{Trees, tpd} import core._, core.Decorators.{sourcePos => _, _} import Comments._, Contexts._, Flags._, Names._, NameOps._, Symbols._, SymDenotations._, Trees._, Types._ import classpath.ClassPathEntries -import reporting._, reporting.diagnostic.MessageContainer +import reporting._, reporting.diagnostic.{Message, MessageContainer, messages} import typer.Typer import util._ import interactive._, interactive.InteractiveDriver._ @@ -193,7 +193,7 @@ class DottyLanguageServer extends LanguageServer client.publishDiagnostics(new PublishDiagnosticsParams( document.getUri, - diags.flatMap(diagnostic(_, positionMapper)).asJava)) + diags.flatMap(diagnostic(_, positionMapper)(driver.currentCtx)).asJava)) } override def didChange(params: DidChangeTextDocumentParams): Unit = { @@ -221,7 +221,7 @@ class DottyLanguageServer extends LanguageServer client.publishDiagnostics(new PublishDiagnosticsParams( document.getUri, - diags.flatMap(diagnostic(_, positionMapper)).asJava)) + diags.flatMap(diagnostic(_, positionMapper)(driver.currentCtx)).asJava)) } } @@ -475,7 +475,9 @@ object DottyLanguageServer { * Convert a MessageContainer to an lsp4j.Diagnostic. The positions are transformed vy * `positionMapper`. */ - def diagnostic(mc: MessageContainer, positionMapper: Option[SourcePosition => SourcePosition] = None): Option[lsp4j.Diagnostic] = + def diagnostic(mc: MessageContainer, + positionMapper: Option[SourcePosition => SourcePosition] = None + )(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 { @@ -493,11 +495,40 @@ object DottyLanguageServer { } } - val code = mc.contained().errorId.errorNumber.toString - range(mc.pos, positionMapper).map(r => - new lsp4j.Diagnostic( - r, mc.message, severity(mc.level), /*source =*/ "", code)) + val message = mc.contained() + if (displayMessage(message, mc.pos.source)) { + val code = message.errorId.errorNumber.toString + range(mc.pos, positionMapper).map(r => + new lsp4j.Diagnostic( + r, mc.message, severity(mc.level), /*source =*/ "", code)) + } else { + None + } + } + + /** + * Check whether `message` should be displayed in the IDE. + * + * Currently we only filter out the warning about pure expressions in statement position when they + * are immediate children of the worksheet wrapper. + * + * @param message The message to filter. + * @param sourceFile The sourcefile from which `message` originates. + * @return true if the message should be displayed in the IDE, false otherwise. + */ + private def displayMessage(message: Message, sourceFile: SourceFile)(implicit ctx: Context): Boolean = { + if (isWorksheet(sourceFile)) { + message match { + case messages.PureExpressionInStatementPosition(_, exprOwner) => + val ownerSym = if (exprOwner.isLocalDummy) exprOwner.owner else exprOwner + !isWorksheetWrapper(ownerSym) + case _ => + true + } + } else { + true } + } /** Does this URI represent a worksheet? */ private def isWorksheet(uri: URI): Boolean = @@ -559,8 +590,15 @@ object DottyLanguageServer { * @see wrapWorksheet */ def isWorksheetWrapper(sourceTree: SourceTree)(implicit ctx: Context): Boolean = { - val symbol = sourceTree.tree.symbol - isWorksheet(sourceTree.source) && + isWorksheet(sourceTree.source) && isWorksheetWrapper(sourceTree.tree.symbol) + } + + /** + * Is this symbol the wrapper object that we put around worksheet sources? + * + * @see wrapWorksheet + */ + def isWorksheetWrapper(symbol: Symbol)(implicit ctx: Context): Boolean = { symbol.name.toString == "Worksheet$" && symbol.owner == ctx.definitions.EmptyPackageClass } From a2569c29a78a861504fb18c6aee3c494553e164e Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Tue, 9 Oct 2018 10:21:23 +0200 Subject: [PATCH 37/38] Factor `WorksheetWrapper` name to StdNames --- compiler/src/dotty/tools/dotc/core/StdNames.scala | 1 + .../tools/languageserver/DottyLanguageServer.scala | 4 ++-- .../dotty/tools/languageserver/WorksheetTest.scala | 10 ++++++---- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/StdNames.scala b/compiler/src/dotty/tools/dotc/core/StdNames.scala index d020b3eb7ffc..d1cbf31b00e6 100644 --- a/compiler/src/dotty/tools/dotc/core/StdNames.scala +++ b/compiler/src/dotty/tools/dotc/core/StdNames.scala @@ -548,6 +548,7 @@ object StdNames { val wait_ : N = "wait" val withFilter: N = "withFilter" val withFilterIfRefutable: N = "withFilterIfRefutable$" + val WorksheetWrapper: N = "WorksheetWrapper" val wrap: N = "wrap" val zero: N = "zero" val zip: N = "zip" diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index 92449f2d1bca..c307d884d857 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -540,7 +540,7 @@ object DottyLanguageServer { /** Wrap the source of a worksheet inside an `object`. */ private def wrapWorksheet(source: String): String = - s"""object Worksheet { + s"""object ${StdNames.nme.WorksheetWrapper} { |$source |}""".stripMargin @@ -599,7 +599,7 @@ object DottyLanguageServer { * @see wrapWorksheet */ def isWorksheetWrapper(symbol: Symbol)(implicit ctx: Context): Boolean = { - symbol.name.toString == "Worksheet$" && + symbol.name == StdNames.nme.WorksheetWrapper.moduleClassName && symbol.owner == ctx.definitions.EmptyPackageClass } diff --git a/language-server/test/dotty/tools/languageserver/WorksheetTest.scala b/language-server/test/dotty/tools/languageserver/WorksheetTest.scala index c7469f811a5e..5b2dc2644e62 100644 --- a/language-server/test/dotty/tools/languageserver/WorksheetTest.scala +++ b/language-server/test/dotty/tools/languageserver/WorksheetTest.scala @@ -3,6 +3,8 @@ package dotty.tools.languageserver import org.junit.Test import org.eclipse.lsp4j.{CompletionItemKind, DocumentHighlightKind, SymbolKind} +import dotty.tools.dotc.core.StdNames.nme.WorksheetWrapper +import dotty.tools.dotc.core.NameOps.NameDecorator import dotty.tools.languageserver.util.Code._ import dotty.tools.languageserver.util.embedded.CodeMarker @@ -168,9 +170,9 @@ class WorksheetTest { ws"""/** A class */ class ${m1}Foo${m2} { /** A method */ def ${m3}bar${m4} = 123 } val x = new ${m5}Foo${m6} x.${m7}bar${m8}""".withSource - .hover(m1 to m2, hoverContent("Worksheet.Foo", "/** A class */")) + .hover(m1 to m2, hoverContent(s"${WorksheetWrapper}.Foo", "/** A class */")) .hover(m3 to m4, hoverContent("Int", "/** A method */")) - .hover(m5 to m6, hoverContent("Worksheet.Foo", "/** A class */")) + .hover(m5 to m6, hoverContent(s"${WorksheetWrapper}.Foo", "/** A class */")) .hover(m7 to m8, hoverContent("Int", "/** A method */")) } @@ -178,7 +180,7 @@ class WorksheetTest { ws"""class ${m1}Foo${m2} { def ${m3}bar${m4} = 123 }""".withSource - .documentSymbol(m1, (m1 to m2).symInfo("Foo", SymbolKind.Class, "Worksheet$"), + .documentSymbol(m1, (m1 to m2).symInfo("Foo", SymbolKind.Class, WorksheetWrapper.moduleClassName.toString), (m3 to m4).symInfo("bar", SymbolKind.Method, "Foo")) } @@ -188,7 +190,7 @@ class WorksheetTest { def ${m3}bar${m4} = 123 }""", code"""class ${m5}Baz${m6}""" - ).symbol("Foo", (m1 to m2).symInfo("Foo", SymbolKind.Class, "Worksheet$")) + ).symbol("Foo", (m1 to m2).symInfo("Foo", SymbolKind.Class, WorksheetWrapper.moduleClassName.toString)) .symbol("bar", (m3 to m4).symInfo("bar", SymbolKind.Method, "Foo")) .symbol("Baz", (m5 to m6).symInfo("Baz", SymbolKind.Class)) } From 9a19df7a5c57e491c3ba1b17f59a954495618b2a Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Tue, 9 Oct 2018 10:30:09 +0200 Subject: [PATCH 38/38] Remove unnecessary cast --- .../dotty/tools/languageserver/DottyLanguageServer.scala | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index c307d884d857..63812f48ade4 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -42,8 +42,7 @@ import lsp4j.services._ * - This implementation is based on the LSP4J library: https://github.com/eclipse/lsp4j */ class DottyLanguageServer extends LanguageServer - with LanguageClientAware with TextDocumentService with WorkspaceService - with WorksheetService { thisServer => + with TextDocumentService with WorkspaceService with WorksheetService { thisServer => import ast.tpd._ import DottyLanguageServer._ @@ -122,8 +121,8 @@ class DottyLanguageServer extends LanguageServer } } - override def connect(client: LanguageClient): Unit = { - myClient = client.asInstanceOf[WorksheetClient] + def connect(client: WorksheetClient): Unit = { + myClient = client } override def exit(): Unit = {