ocaml / ocaml-lsp

OCaml Language Server Protocol implementation
Other
749 stars 117 forks source link

ocamllsp/switchImplIntf results in "Jsonrpc: json conversion failed: missing field error" #1330

Open nemethf opened 2 months ago

nemethf commented 2 months ago

So, I know nothing about ocaml, but wanted to support ocaml-lsp extensions in an LSP client. I installed ocaml-lsp with these commands:

apt install opam
opam init
opam install ocaml-lsp-server

Then I created a project with opam exec -- dune init proj hello and connected to the server and sent an ocamllsp/switchImplIntf request, which made the server die with an uncaught exception. Probably, the client sent something invalid, but (I think) the server should handle the situation more gracefully.

This is the quest the client sends: {"jsonrpc":"2.0","id":5,"method":"ocamllsp/switchImplIntf","params":"file:///tmp/hello/bin/main.ml"}

On the other hand, I'm guessing there's nothing wrong with the request itself, because if I change it to some other format, I get a jsonrpc-error: "The input parameter for ocamllsp/switchImplIntf is invalid".

At any rate, the communication log is below.

Thanks!

communication log
[jsonrpc] D[15:33:04.274] Running language server: ocamllsp
[jsonrpc] e[15:33:04.275] --> initialize[1] {"jsonrpc":"2.0","id":1,"method":"initialize","params":{"processId":1150843,"clientInfo":{"name":"Eglot","version":"1.17"},"rootPath":"/tmp/hello/","rootUri":"file:///tmp/hello","initializationOptions":{},"capabilities":{"workspace":{"applyEdit":true,"executeCommand":{"dynamicRegistration":false},"workspaceEdit":{"documentChanges":true},"didChangeWatchedFiles":{"dynamicRegistration":true},"symbol":{"dynamicRegistration":false},"configuration":true,"workspaceFolders":true},"textDocument":{"synchronization":{"dynamicRegistration":false,"willSave":true,"willSaveWaitUntil":true,"didSave":true},"completion":{"dynamicRegistration":false,"completionItem":{"snippetSupport":false,"deprecatedSupport":true,"resolveSupport":{"properties":["documentation","details","additionalTextEdits"]},"tagSupport":{"valueSet":[1]}},"contextSupport":true},"hover":{"dynamicRegistration":false,"contentFormat":["plaintext"]},"signatureHelp":{"dynamicRegistration":false,"signatureInformation":{"parameterInformation":{"labelOffsetSupport":true},"documentationFormat":["plaintext"],"activeParameterSupport":true}},"references":{"dynamicRegistration":false},"definition":{"dynamicRegistration":false,"linkSupport":true},"declaration":{"dynamicRegistration":false,"linkSupport":true},"implementation":{"dynamicRegistration":false,"linkSupport":true},"typeDefinition":{"dynamicRegistration":false,"linkSupport":true},"documentSymbol":{"dynamicRegistration":false,"hierarchicalDocumentSymbolSupport":true,"symbolKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26]}},"documentHighlight":{"dynamicRegistration":false},"codeAction":{"dynamicRegistration":false,"resolveSupport":{"properties":["edit","command"]},"dataSupport":true,"codeActionLiteralSupport":{"codeActionKind":{"valueSet":["quickfix","refactor","refactor.extract","refactor.inline","refactor.rewrite","source","source.organizeImports"]}},"isPreferredSupport":true},"formatting":{"dynamicRegistration":false},"rangeFormatting":{"dynamicRegistration":false},"rename":{"dynamicRegistration":false},"inlayHint":{"dynamicRegistration":false},"publishDiagnostics":{"relatedInformation":false,"codeDescriptionSupport":false,"tagSupport":{"valueSet":[1,2]}}},"window":{"showDocument":{"support":true},"workDoneProgress":true},"general":{"positionEncodings":["utf-32","utf-8","utf-16"]},"experimental":{"snippetTextEdit":true,"serverStatusNotification":true,"colorDiagnosticOutput":true,"openServerLogs":true,"testExplorer":true,"localDocs":true},"offsetEncoding":["utf-32","utf-16"]},"workspaceFolders":[{"uri":"file:///tmp/hello","name":"/tmp/hello/"}]}}
[jsonrpc] e[15:33:04.282] <-- initialize[1] {"id":1,"jsonrpc":"2.0","result":{"capabilities":{"textDocumentSync":{"openClose":true,"change":2,"willSave":false,"willSaveWaitUntil":false,"save":true},"completionProvider":{"triggerCharacters":[".","#"],"resolveProvider":true},"hoverProvider":true,"signatureHelpProvider":{"triggerCharacters":[" ","~","?",":","("]},"declarationProvider":true,"definitionProvider":true,"typeDefinitionProvider":true,"referencesProvider":true,"documentHighlightProvider":true,"documentSymbolProvider":true,"codeActionProvider":{"codeActionKinds":["quickfix","construct","destruct","inferred_intf","put module name in identifiers","remove module name from identifiers","type-annotate"]},"codeLensProvider":{"resolveProvider":false},"documentFormattingProvider":true,"renameProvider":{"prepareProvider":true},"foldingRangeProvider":true,"executeCommandProvider":{"commands":["dune/promote"]},"selectionRangeProvider":true,"workspaceSymbolProvider":true,"workspace":{"workspaceFolders":{"supported":true,"changeNotifications":true}},"experimental":{"ocamllsp":{"interfaceSpecificLangId":true,"handleSwitchImplIntf":true,"handleInferIntf":true,"handleTypedHoles":true,"handleWrappingAstNode":true,"diagnostic_promotions":true}}},"serverInfo":{"name":"ocamllsp","version":"1.10.5"}}}
[jsonrpc] e[15:33:04.282] --> initialized {"jsonrpc":"2.0","method":"initialized","params":{}}
[jsonrpc] e[15:33:04.283] --> textDocument/didOpen {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///tmp/hello/bin/main.ml","version":0,"languageId":"ocaml","text":"let () = print_endline \"Hello, World!\"\n"}}}
[jsonrpc] e[15:33:04.283] --> workspace/didChangeConfiguration {"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{}}}
[jsonrpc] e[15:33:04.561] <-- textDocument/publishDiagnostics {"params":{"uri":"file:///tmp/hello/bin/main.ml","diagnostics":[{"range":{"start":{"line":0,"character":0},"end":{"line":1,"character":0}},"severity":1,"source":"ocamllsp","message":"No config found for file bin/main.ml. Try calling 'dune build'."}]},"method":"textDocument/publishDiagnostics","jsonrpc":"2.0"}
[jsonrpc] e[15:33:04.800] --> textDocument/hover[2] {"jsonrpc":"2.0","id":2,"method":"textDocument/hover","params":{"textDocument":{"uri":"file:///tmp/hello/bin/main.ml"},"position":{"line":0,"character":0}}}
[jsonrpc] e[15:33:04.800] --> textDocument/documentHighlight[3] {"jsonrpc":"2.0","id":3,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///tmp/hello/bin/main.ml"},"position":{"line":0,"character":0}}}
[jsonrpc] e[15:33:04.800] --> textDocument/signatureHelp[4] {"jsonrpc":"2.0","id":4,"method":"textDocument/signatureHelp","params":{"textDocument":{"uri":"file:///tmp/hello/bin/main.ml"},"position":{"line":0,"character":0}}}
[jsonrpc] e[15:33:04.807] <-- textDocument/hover[2] {"id":2,"jsonrpc":"2.0","result":null}
[jsonrpc] e[15:33:04.808] <-- textDocument/documentHighlight[3] {"id":3,"jsonrpc":"2.0","result":[]}
[jsonrpc] e[15:33:04.834] <-- textDocument/signatureHelp[4] {"id":4,"jsonrpc":"2.0","result":{"signatures":[{"label":"print_endline : string -> unit","documentation":{"kind":"plaintext","value":" Print a string, followed by a newline character, on\n   standard output and flush standard output. "},"parameters":[{"label":[16,22]}]}],"activeSignature":0,"activeParameter":0}}
[jsonrpc] e[15:33:07.997] --> ocamllsp/switchImplIntf[5] {"jsonrpc":"2.0","id":5,"method":"ocamllsp/switchImplIntf","params":"file:///tmp/hello/bin/main.ml"}
[stderr]  /-----------------------------------------------------------------------
[stderr]  | Internal error: Uncaught exception.
[stderr]  | Jsonrpc: json conversion failed: missing field error
[stderr]  | Raised at Jsonrpc__Import.Json.error in file "jsonrpc/src/import.ml", line 29, characters 23-50
[stderr]  | Called from Jsonrpc.Response.t_of_yojson in file "jsonrpc/src/jsonrpc.ml", line 247, characters 18-74
[stderr]  | Called from Lsp__Io.Make.read.resp in file "lsp/src/io.ml", line 104, characters 37-72
[stderr]  | Called from Lsp__Io.Make.read in file "lsp/src/io.ml" (inlined), line 107, characters 6-18
[stderr]  | Called from Lsp__Io.Make.read.(fun) in file "lsp/src/io.ml", line 120, characters 26-39
[stderr]  | Called from Fiber__Scheduler.exec in file "fiber/src/scheduler.ml", line 73, characters 8-11
[stderr]  | Re-raised at Stdune__Exn.raise_with_backtrace in file "otherlibs/stdune/src/exn.ml" (inlined), line 38, characters 27-56
[stderr]  | Called from Stdune__Exn_with_backtrace.reraise in file "otherlibs/stdune/src/exn_with_backtrace.ml", line 20, characters 33-71
[stderr]  | Called from Fiber__Scheduler.advance.(fun) in file "fiber/src/scheduler.ml", line 195, characters 2-58
[stderr]  | Called from Fiber.run.loop in file "fiber/src/fiber.ml", line 15, characters 30-61
[stderr]  | Called from Stdune__Exn_with_backtrace.try_with in file "otherlibs/stdune/src/exn_with_backtrace.ml", line 9, characters 8-12
[stderr]  \-----------------------------------------------------------------------
[stderr]  
[jsonrpc] D[15:33:08.004] Connection state change: `exited abnormally with code 1
'

----------b---y---e---b---y---e----------

(I get similar logs even if I start "dune build @check -w" beforehand.)

nemethf commented 2 weeks ago

It seems the documentation is not correct. Sending a DocumentUri as a parm of ocamllsp/switchImplIntf fails, but sending the DocumentUri in a list does work. Like this:
{"jsonrpc":"2.0","id":6,"method":"ocamllsp/switchImplIntf","params":["file:///tmp/hello/bin/main.ml"]}.

This is with "serverInfo":{"name":"ocamllsp","version":"1.10.5"} . Has something changed in this area recently, or was the documentation incorrect all along?

voodoos commented 2 weeks ago

Indeed, it looks like the query expects an (json) array with a single uri: https://github.com/ocaml/ocaml-lsp/blob/master/ocaml-lsp-server/src/custom_requests/req_switch_impl_intf.ml#L13-L14

This is explained by the base protocol LSP relies on: JSON-RPC

If present, parameters for the rpc call MUST be provided as a Structured value. Either by-position through an Array or by-name through an Object.

Do you want to open a PR to make it clearer in the documentation ?

nemethf commented 4 days ago

Do you want to open a PR to make it clearer in the documentation ?

I'm not really good at this, but I think it would be better to extend the error message to this: The input parameter for ocamllsp/switchImplIntf is invalid. See https://www.jsonrpc.org/specification#parameter_structures.