Skip to content

Commit 502d3f3

Browse files
committed
fix(lsp): Omit params field for shutdown/exit methods (oraios#870)
WHY: - HLS (Haskell) and rust-analyzer use Void/unit types for shutdown/exit - These strict-typed servers reject ANY params field (even empty {}) - PR oraios#851 changed to send params:{} for Delphi/FPC compatibility - This broke HLS/rust-analyzer with "Cannot parse Void" errors EXPECTED: - shutdown/exit methods: omit params field entirely (HLS/rust-analyzer compat) - Other methods with None: send params:{} (Delphi/FPC compat) - Other methods with params: include them unchanged (standard behavior) - Three-way compatibility preserved per JSON-RPC 2.0 spec Refs: oraios#870
1 parent 86cbcf0 commit 502d3f3

2 files changed

Lines changed: 312 additions & 6 deletions

File tree

src/solidlsp/lsp_protocol_handler/server.py

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -91,16 +91,40 @@ def make_error_response(request_id: Any, err: LSPError) -> StringDict:
9191
return {"jsonrpc": "2.0", "id": request_id, "error": err.to_lsp()}
9292

9393

94+
# LSP methods that expect NO params field at all (not even empty object).
95+
# These methods use Void/unit type in their protocol definition.
96+
# - shutdown: HLS uses Haskell's Void type, rust-analyzer expects unit
97+
# - exit: Similar - notification with no params
98+
# Sending params:{} to these methods causes parse errors like "Cannot parse Void"
99+
# See: https://www.jsonrpc.org/specification ("params MAY be omitted")
100+
_NO_PARAMS_METHODS = frozenset({"shutdown", "exit"})
101+
102+
103+
def _build_params_field(method: str, params: PayloadLike) -> StringDict:
104+
"""Build the params portion of a JSON-RPC message based on LSP method requirements.
105+
106+
LSP methods with Void/unit type (shutdown, exit) must omit params field entirely
107+
to satisfy HLS and rust-analyzer. Other methods send empty {} for None params
108+
to maintain Delphi/FPC LSP compatibility (PR #851).
109+
110+
Returns a dict that can be merged into the message using ** unpacking.
111+
"""
112+
if method in _NO_PARAMS_METHODS:
113+
return {} # Omit params entirely for Void-type methods
114+
elif params is not None:
115+
return {"params": params}
116+
else:
117+
return {"params": {}} # Keep {} for Delphi/FPC compatibility
118+
119+
94120
def make_notification(method: str, params: PayloadLike) -> StringDict:
95-
# JSON-RPC 2.0: params must be object or array if present, cannot be null
96-
# Some language servers require params to be present, so we send empty object instead of omitting
97-
return {"jsonrpc": "2.0", "method": method, "params": params if params is not None else {}}
121+
"""Create a JSON-RPC 2.0 notification message."""
122+
return {"jsonrpc": "2.0", "method": method, **_build_params_field(method, params)}
98123

99124

100125
def make_request(method: str, request_id: Any, params: PayloadLike) -> StringDict:
101-
# JSON-RPC 2.0: params must be object or array if present, cannot be null
102-
# Some language servers require params to be present, so we send empty object instead of omitting
103-
return {"jsonrpc": "2.0", "method": method, "id": request_id, "params": params if params is not None else {}}
126+
"""Create a JSON-RPC 2.0 request message."""
127+
return {"jsonrpc": "2.0", "method": method, "id": request_id, **_build_params_field(method, params)}
104128

105129

106130
class StopLoopException(Exception):
Lines changed: 282 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,282 @@
1+
"""
2+
Tests for JSON-RPC 2.0 params field handling in LSP protocol.
3+
4+
These tests verify the correct handling of the params field in LSP requests and notifications,
5+
specifically ensuring:
6+
- Void-type methods (shutdown, exit) omit params field entirely
7+
- Methods with explicit params include them unchanged
8+
- Methods with None params receive params: {} for Delphi/FPC compatibility
9+
10+
Reference: JSON-RPC 2.0 spec - params field is optional but must be object/array when present.
11+
"""
12+
13+
from typing import Any
14+
15+
import pytest
16+
17+
from solidlsp.lsp_protocol_handler.server import make_notification, make_request
18+
19+
# =============================================================================
20+
# Shared Assertion Helpers (DRY extraction per AI Panel recommendation)
21+
# =============================================================================
22+
23+
24+
def assert_jsonrpc_structure(
25+
result: dict[str, Any],
26+
expected_method: str,
27+
expected_keys: set[str],
28+
*,
29+
expected_id: Any | None = None,
30+
) -> None:
31+
"""Verify JSON-RPC 2.0 structural requirements with 5-point error messages.
32+
33+
Args:
34+
result: The dict returned by make_request/make_notification
35+
expected_method: The method name that should be in the result
36+
expected_keys: Exact set of keys that should be present
37+
expected_id: If provided, verify the id field matches (for requests)
38+
39+
"""
40+
# Verify jsonrpc field
41+
assert "jsonrpc" in result, (
42+
f"STRUCTURE ERROR: Missing required 'jsonrpc' field.\n"
43+
f"Expected: jsonrpc='2.0'\n"
44+
f"Actual keys: {list(result.keys())}\n"
45+
f"GUIDANCE: All JSON-RPC 2.0 messages must include jsonrpc field."
46+
)
47+
assert result["jsonrpc"] == "2.0", (
48+
f"STRUCTURE ERROR: Invalid jsonrpc version.\n"
49+
f"Expected: '2.0'\n"
50+
f"Actual: {result['jsonrpc']!r}\n"
51+
f"GUIDANCE: JSON-RPC 2.0 requires jsonrpc='2.0' exactly."
52+
)
53+
54+
# Verify method field
55+
assert "method" in result, (
56+
f"STRUCTURE ERROR: Missing required 'method' field.\n"
57+
f"Expected: method='{expected_method}'\n"
58+
f"Actual keys: {list(result.keys())}\n"
59+
f"GUIDANCE: All requests/notifications must include method field."
60+
)
61+
assert result["method"] == expected_method, (
62+
f"STRUCTURE ERROR: Method mismatch.\n"
63+
f"Expected: '{expected_method}'\n"
64+
f"Actual: {result['method']!r}\n"
65+
f"GUIDANCE: Method field must match the requested method name."
66+
)
67+
68+
# Verify id field if expected (requests only)
69+
if expected_id is not None:
70+
assert "id" in result, (
71+
f"STRUCTURE ERROR: Missing required 'id' field for request.\n"
72+
f"Expected: id={expected_id!r}\n"
73+
f"Actual keys: {list(result.keys())}\n"
74+
f"GUIDANCE: JSON-RPC 2.0 requests must include id field."
75+
)
76+
assert result["id"] == expected_id, (
77+
f"STRUCTURE ERROR: Request ID mismatch.\n"
78+
f"Expected: {expected_id!r}\n"
79+
f"Actual: {result['id']!r}\n"
80+
f"GUIDANCE: Request ID must be preserved exactly as provided."
81+
)
82+
83+
# Verify exact key set
84+
actual_keys = set(result.keys())
85+
if actual_keys != expected_keys:
86+
extra = sorted(actual_keys - expected_keys)
87+
missing = sorted(expected_keys - actual_keys)
88+
pytest.fail(
89+
f"STRUCTURE ERROR: Key set mismatch for method '{expected_method}'.\n"
90+
f"Expected keys: {sorted(expected_keys)}\n"
91+
f"Actual keys: {sorted(actual_keys)}\n"
92+
f"Extra keys: {extra}\n"
93+
f"Missing keys: {missing}\n"
94+
f"GUIDANCE: Verify key construction logic for Void-type vs normal methods."
95+
)
96+
97+
98+
def assert_params_omitted(result: dict[str, Any], method: str, req_id: str, input_params: Any = None) -> None:
99+
"""Assert that params field is NOT present (for Void-type methods).
100+
101+
Args:
102+
result: The dict returned by make_request/make_notification
103+
method: Method name for error message context
104+
req_id: Requirement ID (e.g., 'REQ-1', 'REQ-AI-PANEL-GAP')
105+
input_params: If provided, shows what params were passed (for explicit params tests)
106+
107+
"""
108+
if "params" in result:
109+
input_note = f"\nInput params: {input_params}" if input_params is not None else ""
110+
pytest.fail(
111+
f"{req_id} VIOLATED: {method} method MUST omit params field entirely.{input_note}\n"
112+
f"Expected: No 'params' key in result\n"
113+
f"Actual: params={result.get('params')!r}\n"
114+
f"Actual keys: {list(result.keys())}\n"
115+
f"REASON: HLS/rust-analyzer Void types reject any params field (even empty object).\n"
116+
f"GUIDANCE: Void-type constraint takes precedence - implementation must omit params entirely."
117+
)
118+
119+
120+
def assert_params_equal(result: dict[str, Any], expected_params: Any, req_id: str) -> None:
121+
"""Assert that params field equals expected value.
122+
123+
Args:
124+
result: The dict returned by make_request/make_notification
125+
expected_params: The exact params value expected
126+
req_id: Requirement ID for error message context
127+
128+
"""
129+
if "params" not in result:
130+
pytest.fail(
131+
f"{req_id} VIOLATED: params field missing.\n"
132+
f"Expected: params={expected_params!r}\n"
133+
f"Actual keys: {list(result.keys())}\n"
134+
f"GUIDANCE: Non-Void methods must include params field."
135+
)
136+
if result["params"] != expected_params:
137+
pytest.fail(
138+
f"{req_id} VIOLATED: params value mismatch.\n"
139+
f"Expected: {expected_params!r}\n"
140+
f"Actual: {result['params']!r}\n"
141+
f"GUIDANCE: Params must be included exactly as provided (or {{}} for None)."
142+
)
143+
144+
145+
class TestMakeNotificationParamsHandling:
146+
"""Test make_notification() params field handling per JSON-RPC 2.0 spec."""
147+
148+
def test_shutdown_method_omits_params_entirely(self) -> None:
149+
"""REQ-1: Void-type method 'shutdown' MUST omit params field entirely."""
150+
result = make_notification("shutdown", None)
151+
assert_jsonrpc_structure(result, "shutdown", {"jsonrpc", "method"})
152+
assert_params_omitted(result, "shutdown", "REQ-1")
153+
154+
def test_exit_method_omits_params_entirely(self) -> None:
155+
"""REQ-1: Void-type method 'exit' MUST omit params field entirely."""
156+
result = make_notification("exit", None)
157+
assert_jsonrpc_structure(result, "exit", {"jsonrpc", "method"})
158+
assert_params_omitted(result, "exit", "REQ-1")
159+
160+
def test_notification_with_explicit_params_dict(self) -> None:
161+
"""REQ-2: Methods with explicit params MUST include them unchanged."""
162+
test_params = {"uri": "file:///test.py", "languageId": "python"}
163+
result = make_notification("textDocument/didOpen", test_params)
164+
assert_jsonrpc_structure(result, "textDocument/didOpen", {"jsonrpc", "method", "params"})
165+
assert_params_equal(result, test_params, "REQ-2")
166+
167+
def test_notification_with_explicit_params_list(self) -> None:
168+
"""REQ-2: Methods with explicit params (list) MUST include them unchanged."""
169+
test_params = ["arg1", "arg2", "arg3"]
170+
result = make_notification("custom/method", test_params)
171+
assert_jsonrpc_structure(result, "custom/method", {"jsonrpc", "method", "params"})
172+
assert_params_equal(result, test_params, "REQ-2")
173+
174+
def test_notification_with_none_params_sends_empty_dict(self) -> None:
175+
"""REQ-3: Methods with None params MUST send params: {} (Delphi/FPC compat)."""
176+
result = make_notification("textDocument/didChange", None)
177+
assert_jsonrpc_structure(result, "textDocument/didChange", {"jsonrpc", "method", "params"})
178+
assert_params_equal(result, {}, "REQ-3")
179+
180+
def test_notification_with_empty_dict_params(self) -> None:
181+
"""REQ-2: Explicit empty dict params MUST be included unchanged."""
182+
result = make_notification("custom/notify", {})
183+
assert_jsonrpc_structure(result, "custom/notify", {"jsonrpc", "method", "params"})
184+
assert_params_equal(result, {}, "REQ-2")
185+
186+
187+
class TestMakeRequestParamsHandling:
188+
"""Test make_request() params field handling per JSON-RPC 2.0 spec."""
189+
190+
def test_shutdown_request_omits_params_entirely(self) -> None:
191+
"""REQ-1: Void-type method 'shutdown' MUST omit params field entirely (requests)."""
192+
result = make_request("shutdown", request_id=1, params=None)
193+
assert_jsonrpc_structure(result, "shutdown", {"jsonrpc", "method", "id"}, expected_id=1)
194+
assert_params_omitted(result, "shutdown", "REQ-1")
195+
196+
def test_request_with_explicit_params_dict(self) -> None:
197+
"""REQ-2: Requests with explicit params MUST include them unchanged."""
198+
test_params = {"textDocument": {"uri": "file:///test.py"}, "position": {"line": 10, "character": 5}}
199+
result = make_request("textDocument/hover", request_id=42, params=test_params)
200+
assert_jsonrpc_structure(result, "textDocument/hover", {"jsonrpc", "method", "id", "params"}, expected_id=42)
201+
assert_params_equal(result, test_params, "REQ-2")
202+
203+
def test_request_with_none_params_sends_empty_dict(self) -> None:
204+
"""REQ-3: Requests with None params MUST send params: {} (Delphi/FPC compat)."""
205+
result = make_request("workspace/configuration", request_id=100, params=None)
206+
assert_jsonrpc_structure(result, "workspace/configuration", {"jsonrpc", "method", "id", "params"}, expected_id=100)
207+
assert_params_equal(result, {}, "REQ-3")
208+
209+
def test_request_id_preservation(self) -> None:
210+
"""Verify request_id is correctly included in result (string ID)."""
211+
test_id = "unique-request-123"
212+
result = make_request("custom/request", request_id=test_id, params={"key": "value"})
213+
assert_jsonrpc_structure(result, "custom/request", {"jsonrpc", "method", "id", "params"}, expected_id=test_id)
214+
215+
def test_request_with_explicit_params_list(self) -> None:
216+
"""REQ-2: Requests with explicit params (list) MUST include them unchanged."""
217+
test_params = [1, 2, 3]
218+
result = make_request("custom/sum", request_id=99, params=test_params)
219+
assert_jsonrpc_structure(result, "custom/sum", {"jsonrpc", "method", "id", "params"}, expected_id=99)
220+
assert_params_equal(result, test_params, "REQ-2")
221+
222+
223+
class TestVoidMethodsExhaustive:
224+
"""Test all methods that should be treated as Void-type (no params)."""
225+
226+
def test_shutdown_request_ignores_explicit_params_dict(self) -> None:
227+
"""REQ-AI-PANEL-GAP: shutdown MUST omit params even when caller explicitly provides params."""
228+
explicit_params = {"key": "value", "another": "param"}
229+
result = make_request("shutdown", request_id=1, params=explicit_params)
230+
assert_jsonrpc_structure(result, "shutdown", {"jsonrpc", "method", "id"}, expected_id=1)
231+
assert_params_omitted(result, "shutdown", "REQ-AI-PANEL-GAP", input_params=explicit_params)
232+
233+
def test_exit_notification_ignores_explicit_params(self) -> None:
234+
"""REQ-AI-PANEL-GAP: exit MUST omit params even when caller explicitly provides params."""
235+
explicit_params = {"unexpected": "params"}
236+
result = make_notification("exit", explicit_params)
237+
assert_jsonrpc_structure(result, "exit", {"jsonrpc", "method"})
238+
assert_params_omitted(result, "exit", "REQ-AI-PANEL-GAP", input_params=explicit_params)
239+
240+
def test_only_shutdown_and_exit_are_void_methods(self) -> None:
241+
"""REQ-BOUNDARY: Verify EXACTLY shutdown/exit are Void-type - no more, no less."""
242+
# Positive verification: shutdown and exit MUST omit params
243+
shutdown_notif = make_notification("shutdown", None)
244+
exit_notif = make_notification("exit", None)
245+
shutdown_req = make_request("shutdown", 1, None)
246+
247+
assert "params" not in shutdown_notif, "shutdown notification should omit params"
248+
assert "params" not in exit_notif, "exit notification should omit params"
249+
assert "params" not in shutdown_req, "shutdown request should omit params"
250+
251+
# Negative verification: other methods MUST include params (even when None -> {})
252+
non_void_methods = [
253+
"initialize",
254+
"initialized",
255+
"textDocument/didOpen",
256+
"textDocument/didChange",
257+
"textDocument/didClose",
258+
"workspace/didChangeConfiguration",
259+
"workspace/didChangeWatchedFiles",
260+
]
261+
262+
for method in non_void_methods:
263+
result_notif = make_notification(method, None)
264+
result_req = make_request(method, 1, None)
265+
266+
if "params" not in result_notif:
267+
pytest.fail(
268+
f"BOUNDARY VIOLATION: '{method}' notification treated as Void-type.\n"
269+
f"Expected: params field present (should be {{}})\n"
270+
f"Actual keys: {list(result_notif.keys())}\n"
271+
f"GUIDANCE: Only 'shutdown' and 'exit' should omit params field."
272+
)
273+
assert_params_equal(result_notif, {}, f"REQ-3 ({method} notification)")
274+
275+
if "params" not in result_req:
276+
pytest.fail(
277+
f"BOUNDARY VIOLATION: '{method}' request treated as Void-type.\n"
278+
f"Expected: params field present (should be {{}})\n"
279+
f"Actual keys: {list(result_req.keys())}\n"
280+
f"GUIDANCE: Only 'shutdown' and 'exit' should omit params field."
281+
)
282+
assert_params_equal(result_req, {}, f"REQ-3 ({method} request)")

0 commit comments

Comments
 (0)