Skip to content

Worksheet: improve hover support #5900

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions docs/docs/usage/worksheet-mode-implementation-details.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,12 @@ indicate that worksheet execution has produced some output.
* The worksheet that produced this output.
*/
textDocument: VersionedTextDocumentIdentifier;

/**
* The line number of the expression that produced this output.
* The range of the expression that produced this output.
*/
line: int;
range: Range;

/**
* The output that has been produced.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,17 +240,20 @@ class DottyLanguageServer extends LanguageServer
val document = params.getTextDocument
val uri = new URI(document.getUri)
val driver = driverFor(uri)
implicit def ctx: Context = driver.currentCtx
val worksheetMode = isWorksheet(uri)

val (text, positionMapper) =
if (worksheetMode) (wrapWorksheet(document.getText), Some(toUnwrappedPosition _))
else (document.getText, None)
val text =
if (worksheetMode)
wrapWorksheet(document.getText)
else
document.getText

val diags = driver.run(uri, text)

client.publishDiagnostics(new PublishDiagnosticsParams(
document.getUri,
diags.flatMap(diagnostic(_, positionMapper)(driver.currentCtx)).asJava))
diags.flatMap(diagnostic).asJava))
}

override def didChange(params: DidChangeTextDocumentParams): Unit = {
Expand All @@ -262,19 +265,22 @@ class DottyLanguageServer extends LanguageServer
checkMemory()

val driver = driverFor(uri)
implicit def ctx: Context = driver.currentCtx

val change = params.getContentChanges.get(0)
assert(change.getRange == null, "TextDocumentSyncKind.Incremental support is not implemented")

val (text, positionMapper) =
if (worksheetMode) (wrapWorksheet(change.getText), Some(toUnwrappedPosition _))
else (change.getText, None)
val text =
if (worksheetMode)
wrapWorksheet(change.getText)
else
change.getText

val diags = driver.run(uri, text)

client.publishDiagnostics(new PublishDiagnosticsParams(
document.getUri,
diags.flatMap(diagnostic(_, positionMapper)(driver.currentCtx)).asJava))
diags.flatMap(diagnostic).asJava))
}
}

Expand All @@ -299,7 +305,7 @@ class DottyLanguageServer extends LanguageServer
override def completion(params: CompletionParams) = computeAsync { cancelToken =>
val uri = new URI(params.getTextDocument.getUri)
val driver = driverFor(uri)
implicit val ctx = driver.currentCtx
implicit def ctx: Context = driver.currentCtx

val pos = sourcePosition(driver, uri, params.getPosition)
val items = driver.compilationUnits.get(uri) match {
Expand All @@ -318,13 +324,13 @@ class DottyLanguageServer extends LanguageServer
override def definition(params: TextDocumentPositionParams) = computeAsync { cancelToken =>
val uri = new URI(params.getTextDocument.getUri)
val driver = driverFor(uri)
implicit val ctx = driver.currentCtx
implicit def ctx: Context = driver.currentCtx

val pos = sourcePosition(driver, uri, params.getPosition)
val path = Interactive.pathTo(driver.openedTrees(uri), pos)

val definitions = Interactive.findDefinitions(path, pos, driver).toList
definitions.flatMap(d => location(d.namePos, positionMapperFor(d.source))).asJava
definitions.flatMap(d => location(d.namePos)).asJava
}

override def references(params: ReferenceParams) = computeAsync { cancelToken =>
Expand All @@ -341,7 +347,7 @@ class DottyLanguageServer extends LanguageServer
val pos = sourcePosition(driver, uri, params.getPosition)

val (definitions, originalSymbols) = {
implicit val ctx: Context = driver.currentCtx
implicit def ctx: Context = driver.currentCtx
val path = Interactive.pathTo(driver.openedTrees(uri), pos)
val definitions = Interactive.findDefinitions(path, pos, driver)
val originalSymbols = Interactive.enclosingSourceSymbols(path, pos)
Expand All @@ -359,7 +365,7 @@ class DottyLanguageServer extends LanguageServer
val name = definition.name(ctx).sourceModuleName.toString
val trees = remoteDriver.sourceTreesContaining(name)(ctx)
val matches = Interactive.findTreesMatching(trees, includes, definition)(ctx)
matches.map(tree => location(tree.namePos(ctx), positionMapperFor(tree.source)))
matches.map(tree => location(tree.namePos(ctx)))
}
}
}.toList
Expand All @@ -370,7 +376,7 @@ class DottyLanguageServer extends LanguageServer
override def rename(params: RenameParams) = computeAsync { cancelToken =>
val uri = new URI(params.getTextDocument.getUri)
val driver = driverFor(uri)
implicit val ctx = driver.currentCtx
implicit def ctx: Context = driver.currentCtx

val uriTrees = driver.openedTrees(uri)
val pos = sourcePosition(driver, uri, params.getPosition)
Expand Down Expand Up @@ -429,15 +435,15 @@ class DottyLanguageServer extends LanguageServer
.flatMap((uriOpt, ref) => uriOpt.map(uri => (uri.toString, ref)))
.mapValues(refs =>
refs.flatMap(ref =>
range(ref.namePos, positionMapperFor(ref.source)).map(nameRange => new TextEdit(nameRange, newName))).distinct.asJava)
range(ref.namePos).map(nameRange => new TextEdit(nameRange, newName))).distinct.asJava)

new WorkspaceEdit(changes.asJava)
}

override def documentHighlight(params: TextDocumentPositionParams) = computeAsync { cancelToken =>
val uri = new URI(params.getTextDocument.getUri)
val driver = driverFor(uri)
implicit val ctx = driver.currentCtx
implicit def ctx: Context = driver.currentCtx

val pos = sourcePosition(driver, uri, params.getPosition)
val uriTrees = driver.openedTrees(uri)
Expand All @@ -449,15 +455,15 @@ class DottyLanguageServer extends LanguageServer
val refs = Interactive.findTreesMatching(uriTrees, includes, sym)
(for {
ref <- refs
nameRange <- range(ref.namePos, positionMapperFor(ref.source))
nameRange <- range(ref.namePos)
} yield new DocumentHighlight(nameRange, DocumentHighlightKind.Read))
}.distinct.asJava
}

override def hover(params: TextDocumentPositionParams) = computeAsync { cancelToken =>
val uri = new URI(params.getTextDocument.getUri)
val driver = driverFor(uri)
implicit val ctx = driver.currentCtx
implicit def ctx: Context = driver.currentCtx

val pos = sourcePosition(driver, uri, params.getPosition)
val trees = driver.openedTrees(uri)
Expand All @@ -481,26 +487,26 @@ class DottyLanguageServer extends LanguageServer
override def documentSymbol(params: DocumentSymbolParams) = computeAsync { cancelToken =>
val uri = new URI(params.getTextDocument.getUri)
val driver = driverFor(uri)
implicit val ctx = driver.currentCtx
implicit def ctx: Context = driver.currentCtx

val uriTrees = driver.openedTrees(uri)

val defs = Interactive.namedTrees(uriTrees, Include.empty)
(for {
d <- defs if !isWorksheetWrapper(d)
info <- symbolInfo(d.tree.symbol, d.namePos, positionMapperFor(d.source))
info <- symbolInfo(d.tree.symbol, d.namePos)
} yield JEither.forLeft(info)).asJava
}

override def symbol(params: WorkspaceSymbolParams) = computeAsync { cancelToken =>
val query = params.getQuery

drivers.values.toList.flatMap { driver =>
implicit val ctx = driver.currentCtx
implicit def ctx: Context = driver.currentCtx

val trees = driver.sourceTreesContaining(query)
val defs = Interactive.namedTrees(trees, Include.empty, _.name.toString.contains(query))
defs.flatMap(d => symbolInfo(d.tree.symbol, d.namePos, positionMapperFor(d.source)))
defs.flatMap(d => symbolInfo(d.tree.symbol, d.namePos))
}.asJava
}

Expand All @@ -511,7 +517,7 @@ class DottyLanguageServer extends LanguageServer
val pos = sourcePosition(driver, uri, params.getPosition)

val (definitions, originalSymbols) = {
implicit val ctx: Context = driver.currentCtx
implicit def ctx: Context = driver.currentCtx
val path = Interactive.pathTo(driver.openedTrees(uri), pos)
val originalSymbols = Interactive.enclosingSourceSymbols(path, pos)
val definitions = Interactive.findDefinitions(path, pos, driver)
Expand All @@ -528,7 +534,7 @@ class DottyLanguageServer extends LanguageServer
tree => predicates.exists(_(tree))
}
val matches = Interactive.namedTrees(trees, Include.local, predicate)(ctx)
matches.map(tree => location(tree.namePos(ctx), positionMapperFor(tree.source)))
matches.map(tree => location(tree.namePos(ctx)))
}
}.toList

Expand All @@ -539,7 +545,7 @@ class DottyLanguageServer extends LanguageServer

val uri = new URI(params.getTextDocument.getUri)
val driver = driverFor(uri)
implicit val ctx = driver.currentCtx
implicit def ctx: Context = driver.currentCtx

val pos = sourcePosition(driver, uri, params.getPosition)
val trees = driver.openedTrees(uri)
Expand Down Expand Up @@ -653,9 +659,9 @@ object DottyLanguageServer {
}

/** Convert a SourcePosition to an lsp4j.Range */
def range(p: SourcePosition, positionMapper: Option[SourcePosition => SourcePosition] = None): Option[lsp4j.Range] =
def range(p: SourcePosition): Option[lsp4j.Range] =
if (p.exists) {
val mappedPosition = positionMapper.map(_(p)).getOrElse(p)
val mappedPosition = positionMapperFor(p.source).map(_(p)).getOrElse(p)
Some(new lsp4j.Range(
new lsp4j.Position(mappedPosition.startLine, mappedPosition.startColumn),
new lsp4j.Position(mappedPosition.endLine, mappedPosition.endColumn)
Expand All @@ -664,19 +670,16 @@ object DottyLanguageServer {
None

/** Convert a SourcePosition to an lsp4.Location */
def location(p: SourcePosition, positionMapper: Option[SourcePosition => SourcePosition] = None): Option[lsp4j.Location] =
def location(p: SourcePosition): Option[lsp4j.Location] =
for {
uri <- toUriOption(p.source)
r <- range(p, positionMapper)
r <- range(p)
} yield new lsp4j.Location(uri.toString, r)

/**
* Convert a MessageContainer to an lsp4j.Diagnostic. The positions are transformed vy
* `positionMapper`.
* Convert a MessageContainer to an lsp4j.Diagnostic.
*/
def diagnostic(mc: MessageContainer,
positionMapper: Option[SourcePosition => SourcePosition] = None
)(implicit ctx: Context): Option[lsp4j.Diagnostic] =
def diagnostic(mc: MessageContainer)(implicit ctx: Context): Option[lsp4j.Diagnostic] =
if (!mc.pos.exists)
None // diagnostics without positions are not supported: https://github.com/Microsoft/language-server-protocol/issues/249
else {
Expand All @@ -697,7 +700,7 @@ object DottyLanguageServer {
val message = mc.contained()
if (displayMessage(message, mc.pos.source)) {
val code = message.errorId.errorNumber.toString
range(mc.pos, positionMapper).map(r =>
range(mc.pos).map(r =>
new lsp4j.Diagnostic(
r, mc.message, severity(mc.level), /*source =*/ "", code))
} else {
Expand Down Expand Up @@ -866,7 +869,7 @@ object DottyLanguageServer {
}

/** Create an lsp4j.SymbolInfo from a Symbol and a SourcePosition */
def symbolInfo(sym: Symbol, pos: SourcePosition, positionMapper: Option[SourcePosition => SourcePosition])(implicit ctx: Context): Option[lsp4j.SymbolInformation] = {
def symbolInfo(sym: Symbol, pos: SourcePosition)(implicit ctx: Context): Option[lsp4j.SymbolInformation] = {
def symbolKind(sym: Symbol)(implicit ctx: Context): lsp4j.SymbolKind = {
import lsp4j.{SymbolKind => SK}

Expand All @@ -893,7 +896,7 @@ object DottyLanguageServer {
else
null

location(pos, positionMapper).map(l => new lsp4j.SymbolInformation(name, symbolKind(sym), l, containerName))
location(pos).map(l => new lsp4j.SymbolInformation(name, symbolKind(sym), l, containerName))
}

/** Convert `signature` to a `SignatureInformation` */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import dotty.tools.dotc.ast.tpd.{DefTree, Template, Tree, TypeDef}
import dotty.tools.dotc.core.Contexts.Context
import dotty.tools.dotc.interactive.SourceTree
import dotty.tools.dotc.util.Spans.Span
import dotty.tools.dotc.util.SourceFile
import dotty.tools.dotc.util.{ SourceFile, SourcePosition, NoSourcePosition }

import dotty.tools.dotc.core.Flags.Synthetic

Expand All @@ -22,15 +22,15 @@ object Worksheet {
*/
def run(tree: SourceTree,
treeLock: Object,
sendMessage: (Int, String) => Unit,
sendMessage: (SourcePosition, String) => Unit,
cancelChecker: CancelChecker)(
implicit ctx: Context): Unit = {
// For now, don't try to run multiple evaluators in parallel, this would require
// changes to the logic of Evaluator.get among other things.
Evaluator.synchronized {
Evaluator.get(cancelChecker) match {
case None =>
sendMessage(1, "Couldn't start the JVM.")
sendMessage(NoSourcePosition, "Couldn't start the JVM.")
case Some(evaluator) =>
val queries = treeLock.synchronized {
tree.tree match {
Expand Down Expand Up @@ -59,16 +59,15 @@ object Worksheet {
}

/**
* Extract the line number and source code corresponding to this tree
* Extract the position and source code corresponding to this tree
*
* @param evaluator The JVM that runs the REPL.
* @param tree The compiled tree to evaluate.
* @param sourcefile The sourcefile of the worksheet.
*/
private def query(tree: Tree, sourcefile: SourceFile): (Int, String) = {
val line = sourcefile.offsetToLine(tree.span.end)
private def query(tree: Tree, sourcefile: SourceFile)(implicit ctx: Context): (SourcePosition, String) = {
val source = sourcefile.content.slice(tree.span.start, tree.span.end).mkString
(line, source)
(tree.sourcePos, source)
}

private def bounds(span: Span): (Int, Int) = (span.start, span.end)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package dotty.tools.languageserver.worksheet

import org.eclipse.lsp4j.VersionedTextDocumentIdentifier
import org.eclipse.lsp4j.{ Range, VersionedTextDocumentIdentifier }

// All case classes in this file should have zero-parameters secondary
// constructors to allow Gson to reflectively create instances on
Expand All @@ -17,6 +17,6 @@ case class WorksheetRunResult(success: Boolean) {
}

/** The parameters to the `worksheet/publishOutput` notification. */
case class WorksheetRunOutput(textDocument: VersionedTextDocumentIdentifier, line: Int, content: String) {
def this() = this(null, 0, null)
case class WorksheetRunOutput(textDocument: VersionedTextDocumentIdentifier, range: Range, content: String) {
def this() = this(null, null, null)
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package dotty.tools.languageserver.worksheet

import dotty.tools.dotc.core.Contexts.Context
import dotty.tools.dotc.util.SourcePosition
import dotty.tools.dotc.interactive.InteractiveDriver
import dotty.tools.languageserver.DottyLanguageServer
import dotty.tools.languageserver.DottyLanguageServer.range

import org.eclipse.lsp4j.jsonrpc._//{CancelChecker, CompletableFutures}
import org.eclipse.lsp4j.jsonrpc.services._//{JsonSegment, JsonRequest}
Expand All @@ -19,8 +21,8 @@ trait WorksheetService { thisServer: DottyLanguageServer =>
val uri = new URI(params.textDocument.getUri)
try {
val driver = driverFor(uri)
val sendMessage =
(line: Int, msg: String) => client.publishOutput(WorksheetRunOutput(params.textDocument, line, msg))
val sendMessage = (pos: SourcePosition, msg: String) =>
client.publishOutput(WorksheetRunOutput(params.textDocument, range(pos).get, msg))

runWorksheet(driver, uri, sendMessage, cancelChecker)(driver.currentCtx)
cancelChecker.checkCanceled()
Expand All @@ -41,7 +43,7 @@ trait WorksheetService { thisServer: DottyLanguageServer =>
*/
private def runWorksheet(driver: InteractiveDriver,
uri: URI,
sendMessage: (Int, String) => Unit,
sendMessage: (SourcePosition, String) => Unit,
cancelChecker: CancelChecker)(
implicit ctx: Context): Unit = {
val treeOpt = thisServer.synchronized {
Expand Down
Loading