scalameta / metals

Scala language server with rich IDE features 🚀
https://scalameta.org/metals/
Apache License 2.0
2.07k stars 323 forks source link

Design doc for Language support for VSCode notebook #4434

Closed tanishiking closed 1 year ago

tanishiking commented 1 year ago

See more discussions from https://github.com/scalameta/metals/pull/4309

Language support for VSCode notebook

Overview

We can run Scala programs on Jupyter Notebooks using a Jupyter Kernel for Scala called Almond.

Currently, when using notebook cells in VSCode, Metals cannot provide language support such as code completion (by Metals) and code navigations.

This design document describes how Metals can support many language features for the notebook environment in VSCode

Scope

The scope of this document is limited to notebook in VSCode. It doesn't cover LSP support for jupyter notebook in general (like integration with jupyter-lsp).

However, supporting the notebooks in VSCode would be a good first step for integrations with other notebook environments.

Goal

Non-Goal

Context

LSP 3.17 NotebookCellTextDocumentFilter

Starting from LSP 3.17, LSP clients can recognize notebook cells as documents and request textDocument/* to LSP servers. For example, the following change unlocks the textDocument/* requests for jupyter-notebook (of Scala cells) in VSCode.

diff --git a/src/extension.ts b/src/extension.ts
index 13b49f6f..32a99c36 100644
--- a/src/extension.ts
+++ b/src/extension.ts
@@ -291,6 +291,10 @@ function launchMetals(
       { scheme: "file", language: "java" },
       { scheme: "jar", language: "scala" },
       { scheme: "jar", language: "java" },
+      {
+        notebook: { scheme: "file", notebookType: "jupyter-notebook" },
+        language: "scala",
+      },
     ],
     synchronize: {
       configurationSection: "metals",

see: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#notebookCellTextDocumentFilter

Now, notebook on VSCode starts sending the textDocument/* request to LSP server.

[Trace - 01:09:28 PM] Received request 'textDocument/completion - (140)'
Params: {
  "context": {
    "triggerKind": 2,
    "triggerCharacter": "."
  },
  "textDocument": {
    "uri": "vscode-notebook-cell:/Users/tanishiking/src/github.com/tanishiking/scala3-playground/src/main/scala/Untitled-1.ipynb#W2sZmlsZQ%3D%3D"
  },
  "position": {
    "line": 0,
    "character": 2
  }
}

LSP 3.17 Notebook Document syncronization

In addition to NotebookCellTextDocumentFilter, LSP 3.17 provides a feature called Notebook Document syncronization.

see: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#notebookDocument_synchronization

By setting server capabilities in scalameta/metals

diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala
index 00aa1d791f..35b0ea2dc9 100644
--- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala
+++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala
@@ -860,6 +860,16 @@ class MetalsLanguageServer(
             ServerCommands.all.map(_.id).asJava
           )
         )
+        val selector = new NotebookSelector()
+        selector.setNotebook(
+          JEither.forLeft[String, NotebookDocumentFilter]("*")
+        )
+        selector.setCells(List(new NotebookSelectorCell("scala")).asJava)
+        capabilities.setNotebookDocumentSync(
+          new NotebookDocumentSyncRegistrationOptions(
+            List(selector).asJava
+          )
+        )
         capabilities.setFoldingRangeProvider(true)
         capabilities.setSelectionRangeProvider(true)
         capabilities.setCodeLensProvider(new CodeLensOptions(false))

Notebook LSP client start sending several notifications such as notebookDocument/didOpen, and notebookDocument/didChange. LSP 3.17 expects LSP servers to syncronize the notebook contents by these notifications, and provide language features based on the synchronized content.

Problems

Though LSP 3.17 provides some specifications around notebook support, it doesn't work out of the box with Metals for mainly two reasons.

Metals doesn't understand the Scala program in a cell

As you found, LSP client (VSCode) sends textDocument/* request with

"textDocument": {
  "uri": "vscode-notebook-cell:/Users/tanishiking/src/github.com/tanishiking/scala3-playground/src/main/scala/Untitled-1.ipynb#W2sZmlsZQ%3D%3D"
},
"position": {
  "line": 0,
  "character": 2
}

However, Metals can't understand each cell of notebook. For example, in the following setting, Metals can't handle cell2 and cell3 (in Scala2, Metals can't cell1 too). Because each cell doesn't compile.

// cell1
def add(a: Int, b: Int): Int = a + b

// cell2
add(1, 2)

// cell3
import $ivy.`io.circe::circe-generic:0.11.1`

In order to analyze Scala code and provide compiler-based language features, Metals have to convert cells into compilable Scala program.

Metals server cannot recognise *.ipynb file as a compile target

Metals server provide language features to the files that are recognized as a buildTarget/sources of Build Server Protocol.

However, current Metals doesn't recognize *.ipynb (or combined Scala source, that we'll mention later) for Build Servers.

*Metals have to run build server that recognizes `.ipynbfiles as aSourceItem`**. Otherwise, Metals cannot tell which Scala version of presentation compiler should run against the notebook (Kernel).

Solution / Technical Architecture

In order to provide language support for notebooks, two things are needed.

Components

System flow

When VSCode start running Almond Kernel on VSCode

sequenceDiagram
  participant AlmondKernel
  actor VSCode
  participant Metals
  participant AlmondBSP
  VSCode->>AlmondKernel: (run and) connect to the kernel
  VSCode->>Metals: notebookDocument/didOpen
  Metals->>AlmondBSP: connect to AlmondBSP
  Metals->>AlmondBSP: workspace/buildTargets
  AlmondBSP->>Metals: buildTarget for the notebook

When editing notebook

sequenceDiagram
  actor VSCode
  participant Metals
  participant AlmondBSP
  VSCode->>Metals: notebookDocument/didChange
  Metals->>AlmondBSP: buildTarget/compile

When VSCode request textDocument/*

Alternative Solution

Can we just combine cells into Ammonite script, instead of communicating with Almond BSP?

While Almond can generate accurate Scala source program, it takes many steps for providing notebook support.

Instead of using Almond, Metals can combine cells into *.sc file.

For example, given the following notebook cells,

// cell1
import $ivy.`io.circe::circe-generic:0.11.1`

// cell2
def add(a: Int, b: Int): Int = a + b

// cell3
add(1, 2)

Metals can combine those cells into combined.sc. The combine source will be a valid Ammmonite script file. Now Metals can re-use the Ammonite BSP for the language support.

import $ivy.`io.circe::circe-generic:0.11.1`
def add(a: Int, b: Int): Int = a + b
add(1, 2)

Run Almond BSP only when it's from VSCode

Instead of running Almond BSP all the time, maybe we can run Almond BSP only when it's from VSCode? I don't think we can, because:

Almond BSP shouldn't accept buildTarget/compile?

Instead of accepting buildTarget/compile and generate Scala sources on notebook changes, we can let Kernel compile Scala sources (enabling SemanticDB and writing out the generated Scala sources to the filesystem).

It works only if Almond Kernel can detect it's running for Metals, but it looks like impossible as mentioned in the above section.

Milestones

Concerns

tanishiking commented 1 year ago

Closing, this is just for saving / indexing this doc, for more discussion, it should go https://github.com/scalameta/metals-feature-requests/issues/236

tadeohepperle commented 1 year ago

Hey, is there any update/expected timeline on this? The feature would be awesome, I'd give anyone $50 who gets this done in the next couple of months.

tgodzik commented 1 year ago

No timeline yet, this requires quite a bit if work and currently we need to prioritize stability issues.

alexarchambault commented 1 year ago

@tadeohepperle Surely you meant $50k?

tadeohepperle commented 1 year ago

@tadeohepperle Surely you meant $50k?

@alexarchambault Look, I just wanted to demonstrate that this is a valuable feature I'd be willing to pay for. If we had 1000 people interested in it like me, we'd have the $50k. I know $50 doesn't go anywhere on it's own. I'm not expecting any dev to get excited by $50...

tgodzik commented 1 year ago

Let's park the discussion about money here. As I said this is a non trivial matter and we don't plan on crowdfunding the efforts in the repo. Currently, we don't have the capacity nor time to work on it, but this might change in the future.

aishfenton commented 11 months ago

Another negative to using AlmondBSP that I want to point out @tgodzik (and this would be major issue for our setup), is that we want to have our local code available to the notebook (and keep our existing build tool). If you use AlmondBSP then you introduce a separate issue: how do you then export your non-notebook code/libs into Almond's build.