Skip to content
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

Unable to use Sorbet LSP for Ruby (LSP invalid method call id=Num(0)) #2786

Closed
klardotsh opened this issue Jun 16, 2022 · 7 comments · Fixed by #2801
Closed

Unable to use Sorbet LSP for Ruby (LSP invalid method call id=Num(0)) #2786

klardotsh opened this issue Jun 16, 2022 · 7 comments · Fixed by #2801
Labels
A-language-server Area: Language server client C-bug Category: This is a bug upstream

Comments

@klardotsh
Copy link

Summary

Trying to use Sorbet as a Ruby LSP so I can move over from Neovim for work as I already have with my side projects, but alas, I can't get the LSP to work. Here's my ~/.config/helix/languages.toml:

[[language]]
name = "ruby"
language-server = { command = "srb", args = ["tc", "--lsp"] }

Reproduction Steps

I tried this:

  1. hx -vvvvvvvvvvvvvvvv lib/example/typing_example_3.rb

I expected this to happen:

  1. See the type errors in the file light up, much as they do in eg. Rust projects on this system, and much as they do with Sorbet in my Neovim setup
  2. Be able to Space-K to hover a variable and see its type, much as I can do in Rust projects in Helix, and with Sorbet in Neovim

Instead, this happened:

  • None of the above
  • I'm editing plain old Ruby with syntax highlighting but no LSP integration
  • Language server not active for current buffer when attempting to Space-K for hover

Helix log

~/.cache/helix/helix.log
2022-06-15T20:25:55.860 helix_loader [DEBUG] Located configuration folders: ["/home/j/src/redacted/redacted-repo/.helix"]
2022-06-15T20:25:55.937 mio::poll [TRACE] registering event source with poller: token=Token(1), interests=READABLE | WRITABLE
2022-06-15T20:25:55.937 mio::poll [TRACE] registering event source with poller: token=Token(2), interests=READABLE | WRITABLE
2022-06-15T20:25:55.937 mio::poll [TRACE] registering event source with poller: token=Token(3), interests=READABLE | WRITABLE
2022-06-15T20:25:55.937 helix_lsp::client [INFO] Using custom LSP config: {}
2022-06-15T20:25:55.937 mio::poll [TRACE] registering event source with poller: token=Token(4), interests=READABLE | WRITABLE
2022-06-15T20:25:55.937 mio::poll [TRACE] registering event source with poller: token=Token(5), interests=READABLE | WRITABLE
2022-06-15T20:25:55.937 helix_lsp::transport [INFO] -> {"jsonrpc":"2.0","method":"initialize","params":{"capabilities":{"textDocument":{"codeAction":{"codeActionLiteralSupport":{"codeActionKind":{"valueSet":["","quickfix","refactor","refactor.extract","refactor.inline","refactor.rewrite","source","source.organizeImports"]}}},"completion":{"completionItem":{"resolveSupport":{"properties":["documentation","detail","additionalTextEdits"]},"snippetSupport":false},"completionItemKind":{}},"hover":{"contentFormat":["markdown"]},"publishDiagnostics":{},"rename":{"dynamicRegistration":false,"honorsChangeAnnotations":false,"prepareSupport":false}},"window":{"workDoneProgress":true},"workspace":{"configuration":true,"didChangeConfiguration":{"dynamicRegistration":false},"workspaceFolders":true}},"initializationOptions":{},"processId":7167,"rootPath":"/home/j/src/redacted/redacted-repo","rootUri":"file:///home/j/src/redacted/redacted-repo","workspaceFolders":[{"name":"redacted-repo","uri":"file:///home/j/src/redacted/redacted-repo"}]},"id":0}
2022-06-15T20:25:55.937 mio::poll [TRACE] registering event source with poller: token=Token(0), interests=READABLE
2022-06-15T20:25:55.937 mio::poll [TRACE] registering event source with poller: token=Token(1), interests=READABLE
2022-06-15T20:25:57.815 helix_lsp::transport [INFO] <- {"jsonrpc":"2.0","id":0,"requestMethod":"initialize","result":{"capabilities":{"textDocumentSync":1,"hoverProvider":true,"completionProvider":{"triggerCharacters":[".",":"]},"definitionProvider":true,"typeDefinitionProvider":true,"implementationProvider":true,"referencesProvider":true,"documentHighlightProvider":false,"documentSymbolProvider":false,"workspaceSymbolProvider":true,"codeActionProvider":{"codeActionKinds":["quickfix","source.fixAll.sorbet"]},"documentFormattingProvider":false,"renameProvider":{"prepareProvider":true},"sorbetShowSymbolProvider":true}}}
2022-06-15T20:25:57.815 helix_term::application [ERROR] LSP invalid method call id=Num(0)
2022-06-15T20:26:09.184 helix_lsp::transport [INFO] Language server not initialized, delaying request
2022-06-15T20:26:09.686 helix_term::application [ERROR] Timed out waiting for language servers to shutdown
2022-06-15T20:26:09.687 mio::poll [TRACE] deregistering event source from poller
2022-06-15T20:26:09.688 mio::poll [TRACE] deregistering event source from poller
2022-06-15T20:26:09.688 mio::poll [TRACE] deregistering event source from poller
2022-06-15T20:26:09.688 mio::poll [TRACE] deregistering event source from poller
2022-06-15T20:26:09.688 mio::poll [TRACE] deregistering event source from poller

Platform

Linux

Terminal Emulator

foot-1.12.1_1

Helix Version

helix 22.05

@klardotsh klardotsh added the C-bug Category: This is a bug label Jun 16, 2022
@bjorn-ove
Copy link
Contributor

The problem here is that Sorbet includes a field not mentioned by the jsonrpc specification. The field requestMethod is not mentioned anywhere as far as I can tell, and this is also what throws off the jsonrpc parsing library.

I'm not sure how best to solve this problem though. Technically it might be the fault of Sorbet, but maybe the jsonrpc library is stricter than it has to?

@the-mikedavis the-mikedavis added the A-language-server Area: Language server client label Jun 16, 2022
@the-mikedavis
Copy link
Member

Yeah it looks like it's just that requestMethod field. A normal initialization RPC:

-> {"jsonrpc":"2.0","method":"initialize","params":{...},"id":0}
<- {"id":0,"jsonrpc":"2.0","result":{...}}

with sorbet:

-> {"jsonrpc":"2.0","method":"initialize","params":{...},"id":0}
<- {"jsonrpc":"2.0","id":0,"requestMethod":"initialize","result":{...}}

From the source it looks like it includes requestMethod on the response to the shutdown RPC as well. I suspect there's a #[serde(deny_unknown_fields)] somewhere that we could remove to allow these.

@the-mikedavis
Copy link
Member

Could be solved by this issue upstream: paritytech/jsonrpc#595

Or I suppose we might be able to implement jsonrpc within helix itself and drop the #[serde(deny_unknown_fields)]. The codebase we end up using from jsonrpc seems quite small and straightforward: we essentially just use the types as far as I can see.

@bjorn-ove
Copy link
Contributor

It might not be as easy as removing deny_unkown_fields though. That could make the untagged feature confused.

@the-mikedavis
Copy link
Member

Looks like the compiler is happy to accept the untagged enum without denying unknown fields. Some cases are a bit confused like https://github.com/paritytech/jsonrpc/blob/0a2702595b8831a9a30dec7dc476157c648c361c/core/src/types/response.rs#L260-L281 but I think that's acceptable.

@klardotsh
Copy link
Author

Hmmmm I wonder if a stopgap measure for my own personal use would be to wrap sorbet in a jq pipe to filter out the malformed/extraneous field. I already use srb through a wrapper script to Dockerize it, so this wouldn't be a huge leap.

Thanks for the quick investigation all! Please let me know if there's anything I can do to help (might have some time for small code contributions, or at a minimum I'm happy to test branches along the way)!

the-mikedavis added a commit to the-mikedavis/helix that referenced this issue Jun 18, 2022
We should not depend on jsonrpc-core anymore:

* The project just announced it's no longer actively maintained[^1],
  preferring their new implementation in `jsonrpsee`.
* The types are too strict: we would benefit from removing some
  `#[serde(deny_unknown_fields)]` annotations to allow language
  servers that disrespect the spec[^2].
* We don't use much of the project. Just the types out of core.
  These are easy to embed directly into the `helix-lsp` crate.

[^1]: paritytech/jsonrpc#674
[^2]: helix-editor#2786
@the-mikedavis the-mikedavis linked a pull request Jun 18, 2022 that will close this issue
archseer pushed a commit that referenced this issue Jun 18, 2022
We should not depend on jsonrpc-core anymore:

* The project just announced it's no longer actively maintained[^1],
  preferring their new implementation in `jsonrpsee`.
* The types are too strict: we would benefit from removing some
  `#[serde(deny_unknown_fields)]` annotations to allow language
  servers that disrespect the spec[^2].
* We don't use much of the project. Just the types out of core.
  These are easy to embed directly into the `helix-lsp` crate.

[^1]: paritytech/jsonrpc#674
[^2]: #2786
lazytanuki pushed a commit to lazytanuki/helix that referenced this issue Jun 21, 2022
We should not depend on jsonrpc-core anymore:

* The project just announced it's no longer actively maintained[^1],
  preferring their new implementation in `jsonrpsee`.
* The types are too strict: we would benefit from removing some
  `#[serde(deny_unknown_fields)]` annotations to allow language
  servers that disrespect the spec[^2].
* We don't use much of the project. Just the types out of core.
  These are easy to embed directly into the `helix-lsp` crate.

[^1]: paritytech/jsonrpc#674
[^2]: helix-editor#2786
@klardotsh
Copy link
Author

Just wanted to mention - I pulled and built the latest HEAD and wrote some wrapper scripts to use it over the system-wide installation. Works great with my Sorbet setup for work, so I think I can finally daily-driver Helix! Thanks folks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-language-server Area: Language server client C-bug Category: This is a bug upstream
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants