[WIP] Implement LSP #16

cardoso · 2023-06-11T11:28:19Z

Closes #3

Super WIP, but thought it might be good to have some initial feedback on the approach.

For now, https://github.com/bytecodealliance/wasm-tools is used, but I'm thinking about adding support for https://github.com/bytecodealliance/cargo-component since currently it has its own logic for resolving wit packages, which seems to have some advantages.

To test it out, have a recent version of wasm-tools installed, run npm install, npm start & reload the vscode window.

Preview

Diagnostics

Hover

TODO

lint correctly when inside deps folder
allow configuring problem matcher & pattern
run wit-deps when appropriate
clean up package.json scripts
clean up changes as much as possible

eduardomourar

@cardoso , thanks for your contribution! I will review this more throughly soon. I feel some tests are needed in this PR. Ideally we need both unit tests and some integration tests. Could you create some unit tests in the LSP rust code, please? After this is merged, I will work on the integration tests.

cardoso · 2023-06-14T21:08:01Z

Thanks @eduardomourar - I'll get started on the tests

Some updates: Ultimately I decided to fork wit-parser so I could make some items public for more advanced features than linting such as semantic tokens, contextual hovering, etc.

This code would more appropriately live in the wasm-tools repository alongside wit-parser.

My current thinking is for this to become a binary crate "wit-lsp" in wasm-tools with some of the logic pushed to the wit-parser crate under an "lsp" feature flag.

This isn't blocking as I can keep using the fork for now, but I think moving to that approach would make the LSP more robust and easier to keep up-to-date. Also, keeping it separate from the VSCode extension would make it easier to port for other editors.

I can make that move if it makes sense.

eduardomourar · 2023-06-14T21:51:08Z

@cardoso , let's keep the LSP code here for now in order to iterate more quickly (specially because I don't have permissions to the wasm-tools repository). To simplify that migration later on, we need to move all LSP related files to a folder called ./lsp.

eduardomourar · 2023-06-14T22:01:48Z

package.json

+    "build:lsp:debug": "cargo build",
+    "build:lsp:release": "cargo build --release",
+    "build:ext:debug": "esbuild ./extension.ts --bundle --outfile=out/main.js --external:vscode --format=cjs --platform=node",
+    "build:ext:release": "esbuild ./extension.ts --bundle --outfile=out/main.js --external:vscode --format=cjs --platform=node --minify",
+    "build:debug": "npm run build:lsp:debug && npm run build:ext:debug",
+    "build:release": "npm run build:lsp:release && npm run build:ext:release",
+    "build": "npm run build:debug",
+    "package:lsp:debug": "cp ./target/debug/wit-lsp ./out/wit-lsp",
+    "package:lsp:release": "cp ./target/release/wit-lsp ./out/wit-lsp",
+    "package:ext:debug": "vsce package -o wit-idl.vsix",
+    "package:ext:release": "vsce package -o wit-idl.vsix",
+    "package:debug": "npm run package:lsp:debug && npm run package:ext:debug",
+    "package:release": "npm run package:lsp:release && npm run package:ext:release",
+    "package": "npm run package:debug",
+    "install:insiders": "code-insiders --install-extension wit-idl.vsix",
+    "install:stable": "code --install-extension wit-idl.vsix",
+    "install": "npm run install:stable || npm run install:insiders",


After the move to the lsp folder:

Suggested change

"build:lsp:debug": "cargo build",

"build:lsp:release": "cargo build --release",

"build:ext:debug": "esbuild ./extension.ts --bundle --outfile=out/main.js --external:vscode --format=cjs --platform=node",

"build:ext:release": "esbuild ./extension.ts --bundle --outfile=out/main.js --external:vscode --format=cjs --platform=node --minify",

"build:debug": "npm run build:lsp:debug && npm run build:ext:debug",

"build:release": "npm run build:lsp:release && npm run build:ext:release",

"build": "npm run build:debug",

"package:lsp:debug": "cp ./target/debug/wit-lsp ./out/wit-lsp",

"package:lsp:release": "cp ./target/release/wit-lsp ./out/wit-lsp",

"package:ext:debug": "vsce package -o wit-idl.vsix",

"package:ext:release": "vsce package -o wit-idl.vsix",

"package:debug": "npm run package:lsp:debug && npm run package:ext:debug",

"package:release": "npm run package:lsp:release && npm run package:ext:release",

"package": "npm run package:debug",

"install:insiders": "code-insiders --install-extension wit-idl.vsix",

"install:stable": "code --install-extension wit-idl.vsix",

"install": "npm run install:stable || npm run install:insiders",

"build:lsp:debug": "cargo build --manifest-path=lsp/Cargo.toml",

"build:lsp:release": "cargo build --manifest-path=lsp/Cargo.toml --release",

"build:ext:debug": "esbuild ./extension.ts --bundle --outfile=out/main.js --external:vscode --format=cjs --platform=node",

"build:ext:release": "esbuild ./extension.ts --bundle --outfile=out/main.js --external:vscode --format=cjs --platform=node --minify",

"build:debug": "npm run build:lsp:debug && npm run build:ext:debug",

"build:release": "npm run build:lsp:release && npm run build:ext:release",

"build": "npm run build:debug",

"package:lsp:debug": "cp ./lsp/target/debug/wit-lsp ./out/wit-lsp",

"package:lsp:release": "cp ./lsp/target/release/wit-lsp ./out/wit-lsp",

"package:ext:debug": "vsce package -o wit-idl.vsix",

"package:ext:release": "vsce package -o wit-idl.vsix",

"package:debug": "npm run package:lsp:debug && npm run package:ext:debug",

"package:release": "npm run package:lsp:release && npm run package:ext:release",

"package": "npm run package:debug",

"install:plugin:insiders": "code-insiders --install-extension wit-idl.vsix",

"install:plugin:stable": "code --install-extension wit-idl.vsix",

"install:plugin": "npm run install:plugin:stable",

eduardomourar · 2023-06-14T22:05:11Z

package.json

+  "dependencies": {
+    "vscode-languageclient": "8.1.0"
+  },
  "devDependencies": {
-    "@vscode/vsce": "^2.19.0",
-    "vscode-tmgrammar-test": "^0.1.1"
+    "@types/node": "20.3.0",
+    "@types/vscode": "1.79.0",
+    "@typescript-eslint/eslint-plugin": "5.59.9",
+    "@typescript-eslint/parser": "5.59.9",
+    "@vscode/vsce": "2.19.0",
+    "esbuild": "0.18.0",
+    "eslint": "8.42.0",
+    "eslint-config-xo": "0.43.1",
+    "eslint-config-xo-typescript": "0.57.0",
+    "tmlanguage-generator": "0.4.1",
+    "typescript": "5.1.3",
+    "vscode-tmgrammar-test": "0.1.1"


Let's keep all of these with a caret (^) to allow security patches to go through. That is until we have dependabot enabled and fully automated release process.

eduardomourar · 2023-06-14T22:19:56Z

.vscodeignore

+target/**
+node_modules/**
+
+commitlint.config.js
+
+Cargo.lock
+Cargo.toml
+.DS_Store


Suggested change

target/**

node_modules/**

commitlint.config.js

Cargo.lock

Cargo.toml

.DS_Store

node_modules/**

commitlint.config.js

lsp/**

.DS_Store

eduardomourar · 2023-06-16T15:07:14Z

.editorconfig

@@ -1,2 +1,2 @@
 [*]
-indent_size = 2
+indent_size = 4


do we really need to change the indentation?

eduardomourar · 2023-06-16T15:08:58Z

Cargo.toml

+lazy_static = "1"
+wit-parser = { git = "https://github.com/cardoso/wasm-tools", branch = "lsp" }
+ropey = { version = "1.6" }
+pulldown-cmark = "0.9"


the logger can be used add debug information specially in this exploratory phase

Suggested change

pulldown-cmark = "0.9"

pulldown-cmark = "0.9"

log = "0.4"

env_logger = "0.10"

eduardomourar · 2023-06-16T15:10:30Z

src/main.rs

+use tower_lsp::{LspService, Server};
+use wit_lsp::WitLsp;
+
+async fn start() {
+    let stdin = tokio::io::stdin();
+    let stdout = tokio::io::stdout();
+    let (service, socket) = LspService::new(WitLsp::new);
+    let server = Server::new(stdin, stdout, socket);
+    server.serve(service).await;
+}
+
+#[tokio::main]
+async fn main() {
+    start().await;
+}


Suggested change

use tower_lsp::{LspService, Server};

use wit_lsp::WitLsp;

async fn start() {

let stdin = tokio::io::stdin();

let stdout = tokio::io::stdout();

let (service, socket) = LspService::new(WitLsp::new);

let server = Server::new(stdin, stdout, socket);

server.serve(service).await;

}

#[tokio::main]

async fn main() {

start().await;

}

#[macro_use]

extern crate log;

use tower_lsp::{LspService, Server};

use wit_lsp::WitLsp;

async fn start() {

let stdin = tokio::io::stdin();

let stdout = tokio::io::stdout();

let (service, socket) = LspService::new(WitLsp::new);

let server = Server::new(stdin, stdout, socket);

server.serve(service).await;

}

#[tokio::main]

async fn main() {

env_logger::init();

debug!("starting language server up...");

start().await;

}

eduardomourar · 2023-06-16T15:12:37Z

extension.ts

+import {workspace, type ExtensionContext} from 'vscode';
+import {LanguageClient} from 'vscode-languageclient/node';
+
+let client: LanguageClient;
+
+export function activate(context: ExtensionContext) {
+	client = new LanguageClient(
+		'witLanguageClient',
+		'WIT Language Client',
+		{
+			command: context.asAbsolutePath('./out/wit-lsp'),
+		},
+		{
+			documentSelector: [{
+				language: 'wit',
+				pattern: '**/*.wit',
+			}],
+			synchronize: {
+				fileEvents: workspace.createFileSystemWatcher('**/*.wit'),
+			},
+		},
+	);
+
+	// eslint-disable-next-line @typescript-eslint/no-floating-promises
+	client.start();
+}


Use an environment variable, for now, to change the log level.

Suggested change

import {workspace, type ExtensionContext} from 'vscode';

import {LanguageClient} from 'vscode-languageclient/node';

let client: LanguageClient;

export function activate(context: ExtensionContext) {

client = new LanguageClient(

'witLanguageClient',

'WIT Language Client',

{

command: context.asAbsolutePath('./out/wit-lsp'),

},

{

documentSelector: [{

language: 'wit',

pattern: '**/*.wit',

}],

synchronize: {

fileEvents: workspace.createFileSystemWatcher('**/*.wit'),

},

},

);

// eslint-disable-next-line @typescript-eslint/no-floating-promises

client.start();

}

import {window, workspace, type ExtensionContext} from 'vscode';

import {LanguageClient} from 'vscode-languageclient/node';

let client: LanguageClient;

export async function activate(context: ExtensionContext): Promise<void> {

const traceOutputChannel = window.createOutputChannel('WIT Language Server trace');

client = new LanguageClient(

'witLanguageClient',

'WIT Language Client',

{

command: context.asAbsolutePath('./out/wit-lsp'),

options: {

env: {

...process.env,

// eslint-disable-next-line @typescript-eslint/naming-convention

RUST_LOG: 'debug',

},

},

},

{

documentSelector: [{

scheme: 'file',

language: 'wit',

}],

synchronize: {

fileEvents: workspace.createFileSystemWatcher('**/*.wit'),

},

traceOutputChannel,

},

);

await client.start();

}

eduardomourar · 2023-06-16T15:22:44Z

src/handler/capabilities.rs

+        semantic_tokens_provider: Some(SemanticTokensServerCapabilities::SemanticTokensOptions(
+            SemanticTokensOptions {
+                work_done_progress_options: Default::default(),
+                legend: wit::token::legend(),
+                full: Some(SemanticTokensFullOptions::Delta { delta: Some(false) }),
+                range: None,
+            },
+        )),


by adding the semantic tokens provider here, the TextMate scopes are being overwritten. consequently, the highlighting is different before and after the LSP kicks in. while keeping the aesthetic as close as possible to the current setup, the scopes within syntaxes/wit.tmLanguage.json have to be modified to match this expected mapping table or we have to define a custom mapping.

cardoso added 11 commits June 11, 2023 08:19

feat: implement lsp

8f87407

chore: remove wit-parser dependency for now

0b369d5

chore: remove unused branch

8c8e17a

chore: remove unneeded reference

4d59921

fix: wrong path install script

a8fc313

feat: add hover capability

000d25a

chore: move documentation to markdown

4130a71

make package_id and items in ast public

4093f71

implement semantic tokens

fd032d2

cargo fmt

782ec6a

update docs

8bb6487

eduardomourar reviewed Jun 14, 2023

View reviewed changes

eduardomourar reviewed Jun 16, 2023

View reviewed changes

.editorconfig

@@ -1,2 +1,2 @@

[*]

indent_size = 2

indent_size = 4

Copy link

Collaborator

eduardomourar Jun 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we really need to change the indentation?

eduardomourar reviewed Jun 16, 2023

View reviewed changes

eduardomourar requested changes Jun 16, 2023

View reviewed changes

cardoso mentioned this pull request Sep 4, 2023

Add serialize to all the exported structs of wit-parser bytecodealliance/wasm-tools#1177

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Implement LSP #16

[WIP] Implement LSP #16

cardoso commented Jun 11, 2023 •

edited

eduardomourar left a comment

cardoso commented Jun 14, 2023

eduardomourar commented Jun 14, 2023

eduardomourar Jun 14, 2023

eduardomourar Jun 14, 2023

eduardomourar Jun 14, 2023

eduardomourar Jun 16, 2023

eduardomourar Jun 16, 2023

eduardomourar Jun 16, 2023

eduardomourar Jun 16, 2023

eduardomourar Jun 16, 2023

[WIP] Implement LSP #16

Are you sure you want to change the base?

[WIP] Implement LSP #16

Conversation

cardoso commented Jun 11, 2023 • edited

Preview

Diagnostics

Hover

TODO

eduardomourar left a comment

Choose a reason for hiding this comment

cardoso commented Jun 14, 2023

eduardomourar commented Jun 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cardoso commented Jun 11, 2023 •

edited