Skip to content

PoC: VSCode LSP with Language Server (in TypeScript) - Diagnostics#811

Draft
ankitaS11 wants to merge 6 commits intolcompilers:mainfrom
ankitaS11:lsp/vscode-ts
Draft

PoC: VSCode LSP with Language Server (in TypeScript) - Diagnostics#811
ankitaS11 wants to merge 6 commits intolcompilers:mainfrom
ankitaS11:lsp/vscode-ts

Conversation

@ankitaS11
Copy link
Copy Markdown
Collaborator

@ankitaS11 ankitaS11 commented Jul 22, 2022

This PR aims to provide a PoC for using VSCode’s official language server module, which replaces the existing LSP’s JSONRPC and LPythonServer code base. Time spent on it was half a day, but the bugs it has fixed save us more than a week of implementing threading with the previous implementation. Here is the gist of what it does and what it does not: (note that this implementation only adds diagnostics to the extension, symbol look up will be next)

What does it add?

  • The Language Server is now written in TypeScript, which uses Microsoft’s official language server module.
  • This means that the JSON RPC module earlier written is now replaced with the imported JSON RPC Client from the module.
  • This also means that the code which “communicated” with the LPython compiler to get diagnostics is now reduced to a single compiler execution with a custom flag.
  • This PR adds a flag --show-errors which prints the diagnostics from the file to the console. This is used by the VSCode Language Server (written in TS) which catches the stdout and parses into JSON, and this is literally 5-6 lines of code.
  • This PR also fixes many bugs existing in the previous implementation of LSP, which include: 1. Absence of asynchronous launch to the compiler, and 2. Frequent bugs of the extension crashing while trying to show diagnostics to the VSCode window.
  • This PR also makes sure that the extension updates the diagnostics shown on the VSCode window after each edit, without needing to save the document explicitly. This improves the user experience.

What does it not do?

  • It does not replace MessageHandler code written previously to fetch diagnostics and is instead used by the flag --show-errors to get diagnostics -> convert to RapidJSON object -> convert the object to a string -> print (cout) to the console.

The goal of this PR is to show the demo of how it can make the experience really smooth by adding features to the VSCode extension. The steps are:

  1. Add a flag to the LPython compiler if it doesn’t exist for your feature yet. See: https://github.com/ankitaS11/lpython/blob/lsp/vscode-ts/src/bin/lpython.cpp#L944
  2. On running the lpython compiler command with the given flag, it should print out the information to stdout. See: https://github.com/ankitaS11/lpython/blob/lsp/vscode-ts/src/bin/lpython.cpp#L265
  3. The extension will catch the printed information into stdout, and parse it into a JSON object. See: https://github.com/ankitaS11/lpython/blob/lsp/vscode-ts/editor/vscode/lsp-sample/server/src/server.ts#L246 and https://github.com/ankitaS11/lpython/blob/lsp/vscode-ts/editor/vscode/lsp-sample/server/src/server.ts#L192
  4. The JSON object will be used to convert it into a relevant diagnostic object, which is then returned. See: https://github.com/ankitaS11/lpython/blob/lsp/vscode-ts/editor/vscode/lsp-sample/server/src/server.ts#L252

That’s it. :)

This reduces the efforts for anyone trying to implement features to just C++ (in LPython Compiler code-base), and for those trying to merge those features into LSP can just parse the information into JSON in the extension, and let the extension handle the rest.

In order to use it:

  1. Clone https://github.com/ankitaS11/lpython and check out to lsp/vscode-ts branch.
  2. Go to: editor/vscode/lsp-sample/server/src/server.ts file and replace the binary path of LPython in line number 109 and 201 to your binary path. For example, see: https://github.com/ankitaS11/lpython/blob/lsp/vscode-ts/editor/vscode/lsp-sample/server/src/server.ts#L201

Compile lpython with LSP flag on:

conda activate lp # or use your environment name here
cmake -DCMAKE_BUILD_TYPE=Debug -DWITH_LLVM=yes -DWITH_STACKTRACE=yes -DWITH_LFORTRAN_BINARY_MODFILES=no -DWITH_LSP=yes .
cmake --build . -j16

Now build the extension:

cd editor/vscode/lsp-sample/ && npm install && npm run compile

Now open VSCode in the extension folder (code editor/vscode/lsp-sample/) and run ctrl + shift + D, click on “Run and Debug” and choose VSCode Extension Development, and test the extension. :)

In case you want to package the extension, you can do:

npm install -g vsce
vsce package

This will generate a .vsix file in your editor/vscode/lsp-sample folder, which can then be imported as an extension. You can go to extensions in VSCode, click on the three dots on the top right, click on “Install from VSIX” and select the VSIX, and done (may require a reload). The extension has now been installed.

Please feel free to question/comment/suggest or give feedback on any of this work, as I look forward to implementing document symbol lookup if everything looks good as it is. The extension, as far as diagnostics are concerned, is fully fledged ready to be tried by any user.

cc: @certik

@ankitaS11
Copy link
Copy Markdown
Collaborator Author

Here is the demo if anyone is interested.

demo_lsp

@certik
Copy link
Copy Markdown
Contributor

certik commented Jul 23, 2022

Awesome, great job!

The VSCode extension should probably be maintained as a separate repository, but that would be an easy change. If this PR works, we can add it to a repository and only merge the LPython change.

@namannimmo10 would you please have time to test this PR out?

(Do not merge, we have to do the above refactoring.)

@ankitaS11 ankitaS11 marked this pull request as draft July 23, 2022 12:14
@gnikit
Copy link
Copy Markdown

gnikit commented Jul 23, 2022

This is looking great @ankitaS11!! From my cursory reading of this PR you are adding a second Language Server that is meant to serve the diagnostic messages for LPython. If that is the only thing you are planning on doing, might I suggest you implement this as a linter instead? Basically the only thing that you need is an interface to provideCodeActions. I have drafted a mostly complete extension doing that (can't compile lpython with the above instructions for some reason, need to investigate further, probably I am missing something in my conda env).

Basically the gist of it is, that if you want a standalone feature from LSP e.g. diagnostics, you don't have to implement the entire server, you can just implement the interface to that single feature and VS Code is capable of merging all the providers.

@ankitaS11
Copy link
Copy Markdown
Collaborator Author

This is looking great @ankitaS11!! From my cursory reading of this PR you are adding a second Language Server that is meant to serve the diagnostic messages for LPython. If that is the only thing you are planning on doing, might I suggest you implement this as a linter instead? Basically the only thing that you need is an interface to provideCodeActions. I have drafted a mostly complete extension doing that (can't compile lpython with the above instructions for some reason, need to investigate further, probably I am missing something in my conda env).

Basically the gist of it is, that if you want a standalone feature from LSP e.g. diagnostics, you don't have to implement the entire server, you can just implement the interface to that single feature and VS Code is capable of merging all the providers.

Hi @gnikit

First of all, thanks for the discussion in the previous PR about using the VSCode's language server existing module, and that helped a lot. :)

Just to clarify, this will be the only language server (for LPython) once, and if this PR is merged (I'll remove the previous implementation of JSONRPC and LpythonServer in C++ in the next commits), and will consequently also add other features like document symbol lookup. Do you think that solves your question?

@gnikit
Copy link
Copy Markdown

gnikit commented Jul 23, 2022

Hi @gnikit

First of all, thanks for the discussion in the previous PR about using the VSCode's language server existing module, and that helped a lot. :)

No worries, it's my pleasure helping out, feel free to drop me an email if you have any questions with LSP.

Just to clarify, this will be the only language server (for LPython) once, and if this PR is merged (I'll remove the previous implementation of JSONRPC and LpythonServer in C++ in the next commits), and will consequently also add other features like document symbol lookup. Do you think that solves your question?

I see. Removing the JSONRPC from C++ sounds like a good idea but if your parser (libASR) is in C++ you will still have to communicate the results from C++ to whatever language you choose to implement your LS. You can use normal JSON for that like you are doing here, or TOML, or some other custom format.

From my understanding you are currently sending your diagnostics from C++ in JSON, then parsing them into a vscode.Diagnostic[] in Typescript. If you want to to that with symbols, hover, rename or any other LSP request you will have to create your own interface that will carry all the necessary info to then be converted into a JSONRPC request in Typescript.

The underlying problem basically is that your semantics (libASR) are in C++ and there is no straightforward way to interface with Typescript. To do that you would basically have to reinvent the JSON requests already present in the LSP. Does that make a bit more sense?

I will drop a message to the Microsoft Team that produced proprietary Language Servers in C++ to see if they can disclose any details.

@certik
Copy link
Copy Markdown
Contributor

certik commented Jul 23, 2022

Right now the current code is still maintainable, so I suggest we follow our current design and get everything working.

I agree with @gnikit that there is probably a better way to do this, and if so, we should definitely refactor our code to use the better approach. I'll meet with @gnikit next week to discuss this more, and we can even meet all three of us to brainstorm the design.

We'll use exactly the same approach with LFortran also later on.

@ankitaS11
Copy link
Copy Markdown
Collaborator Author

Hi, @certik and @gnikit

Let me quickly explain how we are communicating the TypeScript Language Server with C++ libASR: (https://github.com/ankitaS11/lpython/blob/lsp/vscode-ts/editor/vscode/lsp-sample/server/src/server.ts#L246)

const stdout = await runCompiler(text, "--show-errors ", settings);

What this means is, that we communicate with the compiler and not with libASR, so we rely on the compiler to have an appropriate flag that will just print the output to the console. Yes, we'll expect it to print us the necessary information in a decided structure (example: it should have line details if it's diagnostics, and with specific names to those values), which is (in my opinion) alright since we'll have to have a standard for our own communication at some stage.

This function runCompiler is implemented here, which is essentially an asynchronous function.

This means, that when we try to add support for document symbol lookup, we'll just need a flag in LPython compiler, something like: --show-symbols which will print (to stdout) a structure in the form of a string (converted from a JSON object) - and this will be caught by our server in TypeScript, parsed and sent to the appropriate callback function in our extension.

I haven't looked yet at what information is required for hover or other features, but I plan to look at them next week, just to get an idea of what structure we can have.

For LFortran, this will be the same, that we just have the same flags, and honestly speaking, I found this as the only way (to my knowledge - which might be limited at this point in time) to have a server in TS (which enables us to do more and better) communicating with the compiler.

Right now the current code is still maintainable, so I suggest we follow our current design and get everything working.

@certik - By "current code", do you mean the code in this PR or the LSP code we had merged before? Just wanted to clarify!

I'll meet with @gnikit next week to discuss this more, and we can even meet all three of us to brainstorm the design.

I think a discussion will help! I'll be happy to join and discuss or brainstorm the ideas/design.

@namannimmo10
Copy link
Copy Markdown
Collaborator

Great work. Thanks, @ankitaS11. I tested this locally and works as expected.
To me, this looks more like a linter; so if we are going to eventually have just one language server, and we add various functionality to it, e.g., code formatting, strict type checking, etc, that act as a sort of plugin, then it would be great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants