Proposal: migrate LSP transport from sourcegraph/jsonrpc2 to golang-jsonrpc2/v2

[gumeniukcom]

Proposal: migrate the LSP transport to golang-jsonrpc2/v2

Hi @kitagry — thanks for bqls, it's a genuinely useful BigQuery LSP.

I maintain golang-jsonrpc2 (a JSON-RPC 2.0 library for Go) and its jsonrpcstdio transport (LSP Content-Length framing built in). I put together a complete, working migration of bqls off sourcegraph/jsonrpc2 and wanted to check whether you'd be open to it before sending a PR.

Branch (builds green, go test ./langserver/... passes): https://github.com/gumeniukcom/bqls/tree/migrate-to-golang-jsonrpc2

Why it might be worth it (honest framing)

This is not primarily a performance pitch — an LSP is parser-bound (your ZetaSQL/tree work dominates), so the transport is not your bottleneck. The real wins are simplicity and spec-conformance:

• −138 lines net (15 files, +115/−253). The repeated if req.Params == nil { … } + json.Unmarshal(*req.Params, …) + &jsonrpc2.Error{…} boilerplate in all 9 handler files collapses into typed handler signatures: func(ctx, params P) (R, error).
• The manual switch req.Method dispatch and the hand-rolled defer recover() in Handle() go away — method-not-found (-32601) and panic recovery are handled by the library.
• Server push (publishDiagnostics, $/progress, window/showMessage) goes through a Pusher captured from the request context at initialize, replacing the stored *jsonrpc2.Conn.
• Dispatch stays strictly sequential (the library default), so LSP ordering is unchanged.

Behavior deltas (full disclosure — these are what a reviewer needs)

1. Handler error text is no longer sent to the client. A returned error maps to -32603 with a generic message; the real text goes to the server-side (stderr) log. Your user-facing error path (window/showMessage) is unchanged. This is the one delta I'd want your call on.
2. Malformed (non-empty, wrong-shape) params now return -32602 instead of -32603 (more spec-correct).
3. Absent/null params no longer short-circuit to -32602; the handler runs with a zero-value param struct. Real editors always send params, so this is a malformed-client edge case.
4. A panicking request handler now produces a proper -32603 reply instead of hanging the client (the stack-trace log line is dropped; the panic value is still logged).
5. New safety limits that didn't exist before: a 15-minute per-request timeout (needed for long workspace/executeCommand BigQuery runs) → -32605 on exceed, and a 256 MiB inbound frame cap (DoS ceiling, far above any real LSP payload).

On the dependency-trust question (fair to ask)

It's a smaller, individually-maintained library, so: it's covered by an OpenSSF Scorecard (~8) and a passing Best Practices badge, has a SECURITY.md with private reporting, pinned/reproducible releases, and zero-dependency core aside from mailru/easyjson. Happy to pin to a specific tag (v2.7.0) and keep the surface auditable.

What I have NOT verified live

I smoke-tested the transport end-to-end (Content-Length round-trip, method-not-found, -32602/-32603 mapping, notification-produces-no-response, clean shutdown on stdin close — all pass). I could not run a full initialize → didOpen → hover → publishDiagnostics flow because that needs GCP credentials + a real BigQuery project; the async-push wiring is verified by code review. Your CI / a real editor session would cover that path.

Totally fine if this isn't a direction you want to take — no worries either way. If you're open to it I'll polish the branch into a PR. Thanks!