Refactor/lsp by JudaNanaa · Pull Request #175 · z-libs/Zen-C

JudaNanaa · 2026-02-02T19:06:08Z

This PR refactors and improves the LSP core implementation.

Refactored the initialize, textDocument/didOpen, and textDocument/didChange handlers for better structure and correctness.
Implemented proper handling of shutdown and exit according to the LSP / JSON-RPC specification.
Added dedicated JSON-RPC error handling functions to return consistent errors when requests are invalid or malformed.
Fixed an issue where an id was incorrectly included in publishDiagnostics messages, which must be sent as notifications.
Added LSP tests with standardized and readable output, consistent with the existing test conventions.

This is an initial step toward a more complete and robust LSP implementation, and further features and improvements will follow.

Feel free to share any feedback or suggestions.

refactor the code and for initialize for more readability and fix the id problem

…num in case of problem

Copilot

Pull request overview

Refactors the Zen C language server’s JSON-RPC/LSP core to improve request routing, error handling, and protocol compliance, and adds an initial LSP test runner.

Changes:

Refactored JSON-RPC dispatch into dedicated handler functions and added helpers for emitting JSON-RPC responses.
Added a JSON-RPC error module + shared error codes to standardize error responses.
Fixed publishDiagnostics to be a notification (no id) and integrated a new LSP test script into make test.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
tests/run_lsp_tests.sh	Adds a shell-based LSP/JSON-RPC compliance smoke test runner.
src/lsp/lsp_analysis.c	Removes `id` from `publishDiagnostics` and updates `lsp_check_file` signature accordingly.
src/lsp/json_rpc_error.c	Introduces helpers for consistent JSON-RPC error responses.
src/lsp/json_rpc.h	Adds JSON-RPC error code enum and declares new JSON-RPC helper APIs.
src/lsp/json_rpc.c	Refactors request handling, adds `initialize`/`didOpen`/`didChange`/`shutdown`/`exit` handlers, and adds JSON response helpers.
Makefile	Adds new JSON-RPC error source file and runs LSP tests as part of `make test`.

Comments suppressed due to low confidence (1)

src/lsp/json_rpc.c:256

On parse failure, handle_request returns silently. JSON-RPC requires a Parse error response with id: null (-32700). Call parse_error(NULL) (or equivalent) before returning so clients can recover/debug.

    cJSON *json = cJSON_Parse(json_str);
    if (!json)
    {
        return;
    }

Copilot · 2026-02-02T19:16:54Z

tests/run_lsp_tests.sh

+        echo -e "Testing $name... PASS"
+        ((PASSED++))
+    else
+        echo -e "Testing $name... FAIL$"


The failure output string includes an extra trailing "$", which makes the test output inconsistent and harder to parse/grep.

Suggested change

echo -e "Testing $name... FAIL$"

echo -e "Testing $name... FAIL"

Copilot · 2026-02-02T19:16:54Z

src/lsp/json_rpc.c

+	cJSON *text = cJSON_GetObjectItem(changes, "text");
+	if (!text || !cJSON_IsString(text))
+	{


contentChanges in textDocument/didChange is an array per LSP; using cJSON_GetObjectItem(changes, "text") will always return NULL and the change will be ignored. Extract the first array element (or iterate) and read its text field.

Copilot · 2026-02-02T19:16:54Z

src/lsp/json_rpc.c

+    assert(json);
+
+    body = cJSON_PrintUnformatted(json);
+
+    len = strlen(body);
+
+    fprintf(stdout, "Content-Length: %zu\r\n\r\n", len);
+    fwrite(body, 1, len, stdout);
+    fflush(stdout);


send_lsp_message_json assumes cJSON_PrintUnformatted always succeeds; if it returns NULL (OOM), strlen(body) will dereference NULL and crash. Handle a NULL return (e.g., bail out or send an internal error) before calling strlen/fwrite.

Copilot · 2026-02-02T19:16:55Z

src/lsp/json_rpc.c

+    cJSON_AddItemToObject(res, "id", cJSON_Duplicate(id_item, 1));
+
+    if (result)
+	{
+        cJSON_AddItemToObject(res, "result", cJSON_Duplicate(result, 1));
+    }
+    else {


create_response drops the id field when id_item is NULL because cJSON_AddItemToObject(..., NULL) fails (see src/lsp/cJSON.c:2108). JSON-RPC error responses must include an explicit "id": null in this case. Use cJSON_AddNullToObject(res, "id") when id_item is NULL.

Suggested change

cJSON_AddItemToObject(res, "id", cJSON_Duplicate(id_item, 1));

if (result)

{

cJSON_AddItemToObject(res, "result", cJSON_Duplicate(result, 1));

}

else {

if (id_item)

{

cJSON_AddItemToObject(res, "id", cJSON_Duplicate(id_item, 1));

}

else

{

cJSON_AddNullToObject(res, "id");

}

if (result)

{

cJSON_AddItemToObject(res, "result", cJSON_Duplicate(result, 1));

}

else

{

Copilot · 2026-02-02T19:16:55Z

src/lsp/json_rpc.c

+	if (id_item)
+	{
+		// FIXME: not always int but can be string too
+		id = id_item->valueint;
+	}


This code accepts string IDs but still reads id_item->valueint and passes an int id to most handlers; for string IDs valueint becomes 0, causing responses with the wrong id. To be JSON-RPC compliant, propagate the original id_item (number or string) into responses (e.g., change handler signatures to take const cJSON *id_item, or convert to a string/variant) instead of truncating to int.

Copilot · 2026-02-02T19:16:55Z

src/lsp/json_rpc.c

+void handle_exit(void)
+{
+    fprintf(stderr, "zls: exit received\n");
+    // TODO: add the lsp clean here
+
+    // TODO: exit 0 if shutdown is call before exit else exit 1 
+    exit(0);
+}


The PR description mentions spec-compliant shutdown/exit handling, but handle_exit always exits with code 0 and still has TODOs. LSP requires exit code 0 only if a prior shutdown request was received; otherwise exit code 1. Track shutdown state and implement the correct exit code behavior.

Copilot · 2026-02-02T19:16:55Z

src/lsp/json_rpc_error.c

+void send_error(const cJSON *id_item, const jsonrpc_error_t err_code, const char *err_msg)
+{
+	cJSON *err = create_error(err_code, err_msg);
+	cJSON *res = create_response(id_item, NULL, err);
+
+	send_lsp_message_json(res);
+
+	cJSON_Delete(err);
+	cJSON_Delete(res);


send_error assumes create_response always returns a valid object; if it returns NULL, send_lsp_message_json will assert/crash. Add NULL checks and fall back to a minimal error response or at least avoid dereferencing NULL.

Copilot · 2026-02-02T19:16:55Z

src/lsp/json_rpc.c

+    cJSON *response = create_response(id_item, result, NULL);
+    send_lsp_message_json(response);
+
+    cJSON_Delete(result);
+    cJSON_Delete(response);


handle_initialize calls create_response(id_item, ...) without checking whether id_item is present. If initialize is sent as a notification (no id), this will hit the create_response assertions (or produce an invalid response). Guard this path: either treat missing id as a notification (initialize but do not respond), or return a JSON-RPC Invalid Request error with id: null.

Suggested change

cJSON *response = create_response(id_item, result, NULL);

send_lsp_message_json(response);

cJSON_Delete(result);

cJSON_Delete(response);

if (id_item)

{

cJSON *response = create_response(id_item, result, NULL);

send_lsp_message_json(response);

cJSON_Delete(response);

}

cJSON_Delete(result);

Copilot · 2026-02-02T19:16:56Z

tests/run_lsp_tests.sh

+# ALL tests
+
+response=$(send_lsp_message '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"rootUri":"file:///tmp"}}')


The suite currently validates initialize/shutdown/exit/unknown-method, but it doesn’t exercise textDocument/didOpen / textDocument/didChange or verify that publishDiagnostics is emitted as a notification (no id). Adding coverage for those flows would catch regressions in the handlers.

Copilot · 2026-02-02T19:16:56Z

tests/run_lsp_tests.sh

+echo "Summary:"
+echo "-> Passed: $PASSED"
+echo "-> Failed: $FAILED"
+echo "----------------------------------------"


The script prints a summary but never exits non-zero when failures occur, so make test will still pass even if LSP tests fail. Mirror the other test runners by exit 1 when $FAILED is non-zero (and optionally print FAILED_TESTS).

Suggested change

echo "----------------------------------------"

echo "----------------------------------------"

if [ "$FAILED" -ne 0 ]; then

if [ "${#FAILED_TESTS[@]}" -ne 0 ]; then

echo "Failed tests:"

for test_name in "${FAILED_TESTS[@]}"; do

echo " - $test_name"

done

fi

exit 1

fi

exit 0

Zuhaitz-dev · 2026-02-04T16:45:07Z

Hey! Tonight I will check this PR (sorry for the delay, a lot of things to do xD)

I see there are conflicts (which makes sense after my changes in the LSP) but I am sure there are some nice additions. (:

JudaNanaa added 5 commits February 2, 2026 00:22

refactor and fix the initialize lsp method

7c6b06b

refactor the code and for initialize for more readability and fix the id problem

start add handle_shutdown

10dd37a

start implementing shutdown and exit method, add error function and e…

3f0890b

…num in case of problem

add comment and fix id bug

d2715e1

refactor didOpen/didChange part for more readability

0a94c99

Copilot AI review requested due to automatic review settings February 2, 2026 19:06

Copilot started reviewing on behalf of JudaNanaa February 2, 2026 19:06 View session

Copilot AI reviewed Feb 2, 2026

View reviewed changes

Zuhaitz-dev added the enhancement New feature or request label Feb 6, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Refactor/lsp#175

Refactor/lsp#175
JudaNanaa wants to merge 5 commits intoz-libs:mainfrom
JudaNanaa:refactor/lsp

JudaNanaa commented Feb 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Zuhaitz-dev commented Feb 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	echo -e "Testing $name... FAIL$"
	echo -e "Testing $name... FAIL"

		# ALL tests

		response=$(send_lsp_message '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"rootUri":"file:///tmp"}}')

-echo "----------------------------------------"
+echo "----------------------------------------"
+if [ "$FAILED" -ne 0 ]; then
+    if [ "${#FAILED_TESTS[@]}" -ne 0 ]; then
+        echo "Failed tests:"
+        for test_name in "${FAILED_TESTS[@]}"; do
+            echo " - $test_name"
+        done
+    fi
+    exit 1
+fi
+exit 0

Uh oh!

Conversation

JudaNanaa commented Feb 2, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Zuhaitz-dev commented Feb 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants