From 40ccd86c339a210da9c385a0bfa1570b59ea4c51 Mon Sep 17 00:00:00 2001 From: Aviad Shiber Date: Fri, 10 Apr 2026 06:35:02 +0300 Subject: [PATCH 01/14] fix: use lsprotocol-aware cattrs converter for jdtls request forwarding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The server used a vanilla cattrs.Converter() to serialize LSP request parameters before forwarding them to jdtls. This emits snake_case field names verbatim from the pygls/lsprotocol models: DefinitionParams(text_document=..., position=...) → {"text_document": {...}, "position": {...}} But the LSP JSON wire format requires camelCase: → {"textDocument": {...}, "position": {...}} jdtls's NavigateToDefinitionHandler (and every other position-based handler) then called TextDocumentPositionParams.getTextDocument(), which returned null because the JSON field was "text_document" not "textDocument", and blew up with: java.lang.NullPointerException: Cannot invoke "org.eclipse.lsp4j.TextDocumentIdentifier.getUri()" because the return value of "TextDocumentPositionParams.getTextDocument()" is null at NavigateToDefinitionHandler.definition(...:70) This broke every go-to-definition, find-references, hover, document symbol, and completion request forwarded to jdtls since the proxy feature was introduced. Users saw "loading..." / no results and the LSP log filled with stack traces. The fix is a one-line change: use lsprotocol.converters.get_converter() instead of cattrs.Converter(). The lsprotocol-aware converter: - Converts snake_case attributes to camelCase JSON fields - Handles lsprotocol's discriminated unions correctly - Drops None-valued optional fields (matching LSP convention) Also adds 7 regression tests in TestLspConverterCamelCase pinning the JSON shape of DefinitionParams, ReferenceParams (with nested context.includeDeclaration), HoverParams, DocumentSymbolParams, CompletionParams, DidOpenTextDocumentParams (with nested languageId), and the None-field-pruning behavior. A future regression to a vanilla cattrs converter would fail every one of these. Bumps version 0.7.1 -> 0.7.2. Verification: 299 passed (was 292), 77% coverage, pyright+ruff clean. Co-Authored-By: Claude Opus 4.6 (1M context) --- pyproject.toml | 2 +- src/java_functional_lsp/__init__.py | 2 +- src/java_functional_lsp/server.py | 11 ++- tests/test_proxy.py | 137 ++++++++++++++++++++++++++++ uv.lock | 2 +- 5 files changed, 149 insertions(+), 5 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 6c81b9a..d5d793a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "java-functional-lsp" -version = "0.7.1" +version = "0.7.2" description = "Java LSP server enforcing functional programming best practices — null safety, immutability, no exceptions" readme = "README.md" license = { text = "MIT" } diff --git a/src/java_functional_lsp/__init__.py b/src/java_functional_lsp/__init__.py index a24fac3..71a66c6 100644 --- a/src/java_functional_lsp/__init__.py +++ b/src/java_functional_lsp/__init__.py @@ -1,3 +1,3 @@ """java-functional-lsp: A Java LSP server enforcing functional programming best practices.""" -__version__ = "0.7.1" +__version__ = "0.7.2" diff --git a/src/java_functional_lsp/server.py b/src/java_functional_lsp/server.py index 493efbb..266e450 100644 --- a/src/java_functional_lsp/server.py +++ b/src/java_functional_lsp/server.py @@ -13,8 +13,8 @@ from pathlib import Path from typing import Any -import cattrs from lsprotocol import types as lsp +from lsprotocol.converters import get_converter from pygls.lsp.server import LanguageServer from pygls.uris import to_fs_path @@ -45,7 +45,14 @@ FunctionalChecker(), ] -_converter = cattrs.Converter() +#: LSP-aware cattrs converter. Unstructures to the LSP JSON shape +#: (camelCase field names, discriminated unions, None-field pruning) and +#: correspondingly structures from the same shape. Using a vanilla +#: ``cattrs.Converter()`` here emits snake_case field names (``text_document`` +#: instead of ``textDocument``), which breaks request forwarding to jdtls — +#: jdtls then sees a null ``TextDocumentIdentifier`` and throws NPEs during +#: go-to-definition, references, etc. +_converter = get_converter() class JavaFunctionalLspServer(LanguageServer): diff --git a/tests/test_proxy.py b/tests/test_proxy.py index 4b346b7..9c96b01 100644 --- a/tests/test_proxy.py +++ b/tests/test_proxy.py @@ -263,6 +263,143 @@ def test_analyze_document(self) -> None: assert "null-return" in codes +class TestLspConverterCamelCase: + """Regression tests for the LSP JSON shape of request forwarding. + + The LSP wire protocol uses camelCase field names (e.g. ``textDocument``). + pygls/lsprotocol models use snake_case attributes (``text_document``) and + rely on their own cattrs converter to handle the conversion. A vanilla + ``cattrs.Converter()`` emits snake_case literally, which causes jdtls to + throw ``NullPointerException`` on every ``textDocument/definition``, + ``textDocument/references``, ``textDocument/hover``, etc. because its + ``TextDocumentPositionParams.getTextDocument()`` returns null when the + field is named ``text_document``. + + These tests pin the converter behavior so a regression to a vanilla + cattrs converter would be caught immediately. + """ + + def test_definition_params_unstructures_to_camelcase(self) -> None: + from lsprotocol import types as lsp + + from java_functional_lsp.server import _serialize_params + + params = lsp.DefinitionParams( + text_document=lsp.TextDocumentIdentifier(uri="file:///foo.java"), + position=lsp.Position(line=10, character=5), + ) + result = _serialize_params(params) + assert isinstance(result, dict) + # jdtls requires camelCase: textDocument, NOT text_document + assert "textDocument" in result + assert "text_document" not in result + assert result["textDocument"]["uri"] == "file:///foo.java" + assert result["position"]["line"] == 10 + assert result["position"]["character"] == 5 + + def test_reference_params_unstructures_to_camelcase(self) -> None: + from lsprotocol import types as lsp + + from java_functional_lsp.server import _serialize_params + + params = lsp.ReferenceParams( + text_document=lsp.TextDocumentIdentifier(uri="file:///bar.java"), + position=lsp.Position(line=3, character=7), + context=lsp.ReferenceContext(include_declaration=True), + ) + result = _serialize_params(params) + assert isinstance(result, dict) + assert "textDocument" in result + assert "text_document" not in result + # Nested camelCase: context.includeDeclaration, NOT context.include_declaration + assert "context" in result + assert "includeDeclaration" in result["context"] + assert "include_declaration" not in result["context"] + + def test_hover_params_unstructures_to_camelcase(self) -> None: + from lsprotocol import types as lsp + + from java_functional_lsp.server import _serialize_params + + params = lsp.HoverParams( + text_document=lsp.TextDocumentIdentifier(uri="file:///baz.java"), + position=lsp.Position(line=0, character=0), + ) + result = _serialize_params(params) + assert "textDocument" in result + assert "text_document" not in result + + def test_document_symbol_params_unstructures_to_camelcase(self) -> None: + from lsprotocol import types as lsp + + from java_functional_lsp.server import _serialize_params + + params = lsp.DocumentSymbolParams( + text_document=lsp.TextDocumentIdentifier(uri="file:///quux.java"), + ) + result = _serialize_params(params) + assert "textDocument" in result + assert "text_document" not in result + + def test_did_open_params_unstructures_to_camelcase(self) -> None: + from lsprotocol import types as lsp + + from java_functional_lsp.server import _serialize_params + + params = lsp.DidOpenTextDocumentParams( + text_document=lsp.TextDocumentItem( + uri="file:///open.java", + language_id="java", + version=1, + text="class T {}", + ), + ) + result = _serialize_params(params) + assert "textDocument" in result + assert "text_document" not in result + # TextDocumentItem fields too: languageId, not language_id + assert "languageId" in result["textDocument"] + assert "language_id" not in result["textDocument"] + + def test_completion_params_unstructures_to_camelcase(self) -> None: + from lsprotocol import types as lsp + + from java_functional_lsp.server import _serialize_params + + params = lsp.CompletionParams( + text_document=lsp.TextDocumentIdentifier(uri="file:///c.java"), + position=lsp.Position(line=5, character=10), + ) + result = _serialize_params(params) + assert "textDocument" in result + assert "text_document" not in result + + def test_none_fields_are_omitted(self) -> None: + """Optional fields with None values should be dropped from the JSON. + + jdtls and other LSP servers treat the presence of a key with null + differently from the absence of the key. The LSP converter drops + None fields; a vanilla converter would emit ``"workDoneToken": null`` + which could confuse strict servers. + """ + from lsprotocol import types as lsp + + from java_functional_lsp.server import _serialize_params + + params = lsp.DefinitionParams( + text_document=lsp.TextDocumentIdentifier(uri="file:///foo.java"), + position=lsp.Position(line=0, character=0), + work_done_token=None, + partial_result_token=None, + ) + result = _serialize_params(params) + assert "workDoneToken" not in result + assert "partialResultToken" not in result + # Also the snake_case equivalents should not appear + assert "work_done_token" not in result + assert "partial_result_token" not in result + + class TestReadJavaMajorVersion: """Parsing of the ``java -version`` output. diff --git a/uv.lock b/uv.lock index 98c354c..43ed4e3 100644 --- a/uv.lock +++ b/uv.lock @@ -184,7 +184,7 @@ wheels = [ [[package]] name = "java-functional-lsp" -version = "0.7.1" +version = "0.7.2" source = { editable = "." } dependencies = [ { name = "pygls" }, From 9fa94442ba4d3105ee966099e60339b02029dcea Mon Sep 17 00:00:00 2001 From: Aviad Shiber Date: Fri, 10 Apr 2026 06:49:52 +0300 Subject: [PATCH 02/14] test: add end-to-end jdtls tests guarding the serialization pipeline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a new e2e test module (tests/test_e2e_jdtls.py) that spawns a real jdtls subprocess and exercises every forwarded LSP request through the actual wire. Unit tests with mocked subprocesses cannot catch JSON-shape regressions because the mocks never parse the bytes — we learned this the hard way twice in the last two releases: - v0.7.1: jdtls silently failed to start because inherited JAVA_HOME pointed at Java 8. Every unit test of find_jdtls_java_home passed while users had zero working jdtls features. - v0.7.2: every forwarded request NPE'd because a vanilla cattrs Converter emitted snake_case field names (text_document) instead of camelCase (textDocument). Every unit test passed while go-to-def, references, hover, completion, and document symbol were all broken. The new module has 7 tests covering every forwarded LSP method: - test_initialize_handshake_succeeds — proxy fixture asserts start() ok - test_document_symbol_round_trip — canonical camelCase regression - test_definition_request_does_not_npe — the exact v0.7.2 scenario - test_references_request_does_not_npe — exercises nested ReferenceContext with include_declaration → includeDeclaration - test_hover_request_does_not_npe - test_completion_request_does_not_npe — distinct content-assist path - test_did_open_notification_does_not_npe — notification path, catches TextDocumentItem.language_id → languageId regressions Each test: 1. Builds pygls-typed params (e.g. lsp.DefinitionParams(text_document=...)) 2. Passes them through _serialize_params — the real production path 3. Asserts the serialized dict has camelCase keys (wire-format sanity) 4. Sends via JdtlsProxy.send_request/send_notification 5. Captures jdtls stderr via caplog and asserts no NullPointerException appeared (catches regressions where send_request returns None from an error response, or where jdtls logs but recovers) Verified the tests actually catch the camelCase bug by temporarily reverting _converter to cattrs.Converter() — the first test failed immediately with: AssertionError: _serialize_params emitted wrong field names: dict_keys(['text_document', 'work_done_token', 'partial_result_token']) CI additions (.github/workflows/test.yml): - The unit matrix now passes `-m "not e2e"` to skip the new e2e tests. - New `e2e-test` job runs on both ubuntu-latest and macos-latest with a single Python version (3.12). Installs Java 21 via setup-java and jdtls via `brew install jdtls` on macOS or a direct tarball download from download.eclipse.org on Linux (wrapped in a shell launcher on $PATH). pyproject.toml registers the `e2e` pytest marker. Local verification: 306 tests pass (299 unit + 7 e2e), 80% coverage, 68s for e2e runs end-to-end. pyright + ruff clean. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/test.yml | 63 +++++- pyproject.toml | 3 + tests/test_e2e_jdtls.py | 425 +++++++++++++++++++++++++++++++++++++ 3 files changed, 490 insertions(+), 1 deletion(-) create mode 100644 tests/test_e2e_jdtls.py diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8992c96..e387396 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -31,6 +31,67 @@ jobs: - name: Type check run: uv run mypy src/ - name: Test - run: uv run pytest -q + run: uv run pytest -q -m "not e2e" + env: + NO_COLOR: "1" + + e2e-test: + name: End-to-End (jdtls) / ${{ matrix.os }} + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, macos-latest] + # One Python version is enough — the e2e tests verify the jdtls request/ + # response pipeline, not Python-version-specific behavior. The unit + # matrix above already exercises every Python version. + steps: + - uses: actions/checkout@v6 + - name: Set up Python 3.12 + uses: actions/setup-python@v6 + with: + python-version: "3.12" + - name: Install Java 21 + uses: actions/setup-java@v4 + with: + distribution: temurin + java-version: "21" + - name: Install jdtls (macOS) + if: runner.os == 'macOS' + run: brew install jdtls + - name: Install jdtls (Linux) + if: runner.os == 'Linux' + run: | + set -euo pipefail + # Download the Eclipse JDT Language Server milestone build directly. + # Pinned version for deterministic CI runs; bump as needed. + JDTLS_VERSION="1.47.0" + JDTLS_BUILD="202509181638" + JDTLS_URL="https://download.eclipse.org/jdtls/milestones/${JDTLS_VERSION}/jdt-language-server-${JDTLS_VERSION}-${JDTLS_BUILD}.tar.gz" + JDTLS_DIR="$HOME/.local/share/jdtls" + BIN_DIR="$HOME/.local/bin" + mkdir -p "$JDTLS_DIR" "$BIN_DIR" + # Follow redirects to reach the actual tarball through Eclipse's mirror. + curl -sSL -o /tmp/jdtls.tar.gz "$JDTLS_URL" + tar -xzf /tmp/jdtls.tar.gz -C "$JDTLS_DIR" + # Create a wrapper script on PATH that invokes the bundled Python launcher. + cat > "$BIN_DIR/jdtls" << SHELL_EOF + #!/bin/bash + exec python3 "$JDTLS_DIR/bin/jdtls" "\$@" + SHELL_EOF + chmod +x "$BIN_DIR/jdtls" + echo "$BIN_DIR" >> "$GITHUB_PATH" + - name: Install uv + uses: astral-sh/setup-uv@v7 + - name: Install dependencies + run: uv sync + - name: Verify jdtls is available + run: | + which jdtls + jdtls --help | head -5 || true + java -version + echo "JAVA_HOME=$JAVA_HOME" + - name: Run e2e tests + run: uv run pytest -v -m e2e env: NO_COLOR: "1" diff --git a/pyproject.toml b/pyproject.toml index d5d793a..0c06acb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -65,6 +65,9 @@ python_classes = ["Test*"] python_functions = ["test_*"] addopts = "--cov=java_functional_lsp --cov-report=term-missing --cov-fail-under=60" asyncio_mode = "auto" +markers = [ + "e2e: end-to-end tests that spawn a real jdtls subprocess (require jdtls + Java 21+; skipped when unavailable)", +] [tool.ruff] line-length = 120 diff --git a/tests/test_e2e_jdtls.py b/tests/test_e2e_jdtls.py new file mode 100644 index 0000000..ce3c652 --- /dev/null +++ b/tests/test_e2e_jdtls.py @@ -0,0 +1,425 @@ +"""End-to-end tests that spawn a real jdtls subprocess. + +These tests exercise the full request/response pipeline: + +1. Python LSP server builds pygls-typed params (``lsp.DefinitionParams(...)``) +2. ``server._serialize_params`` converts them via the lsprotocol converter +3. ``JdtlsProxy.send_request`` frames + writes bytes to the jdtls stdin +4. Real jdtls subprocess parses the JSON and handles the request +5. Response comes back through the proxy and is deserialized + +Unit tests with mocked subprocesses **cannot** catch JSON-shape regressions +because the mocks never parse the bytes. We learned this the hard way: + +- **v0.7.1 fix** (``fix/jdtls-java-home-detection``): got jdtls to start at all + after it was silently failing because the inherited ``JAVA_HOME`` pointed at + Java 8. Every unit test of ``find_jdtls_java_home`` passed while users had + no working jdtls. +- **v0.7.2 fix** (``fix/lsp-camelcase-serialization``): every forwarded request + to jdtls was failing with ``NullPointerException`` because a vanilla + ``cattrs.Converter()`` emitted snake_case field names (``text_document``) + instead of camelCase (``textDocument``). Every unit test passed while + go-to-definition, references, hover, and document symbol were all broken. + +These tests guard the end-to-end pipeline so the next such bug is caught +before release. + +Skip rules: the entire module is skipped when ``jdtls`` is not on PATH or +no Java 21+ installation can be found. Local developers with jdtls installed +see the tests run; CI installs jdtls in the dedicated ``e2e-test`` job. +""" + +from __future__ import annotations + +import asyncio +import logging +import os +import shutil +from collections.abc import AsyncIterator +from pathlib import Path + +import pytest +from lsprotocol import types as lsp + +from java_functional_lsp.proxy import JdtlsProxy, find_jdtls_java_home +from java_functional_lsp.server import _serialize_params + +# Module-level skip: jdtls and Java 21+ are required. Check once at import +# time so the skip reason is clear and the entire test file is skipped at +# collection time rather than failing each test independently. +_JDTLS_ON_PATH = shutil.which("jdtls") is not None +_JAVA_21_AVAILABLE = find_jdtls_java_home() is not None + +pytestmark = [ + pytest.mark.e2e, + pytest.mark.skipif( + not _JDTLS_ON_PATH, + reason="jdtls binary not found on PATH — install via `brew install jdtls` or equivalent", + ), + pytest.mark.skipif( + not _JAVA_21_AVAILABLE, + reason="no Java 21+ found — install via `brew install openjdk@21` or equivalent", + ), +] + +# jdtls cold-start is 10-20s; give each individual request a generous budget +# beyond the default REQUEST_TIMEOUT, and wrap the whole test in pytest-timeout +# so a hung jdtls doesn't wedge the suite. +_E2E_TEST_TIMEOUT_SEC = 120 +_JDTLS_PARSE_WAIT_SEC = 2.5 + +_HELLO_JAVA = """\ +public class Hello { + private String greeting; + + public Hello(String greeting) { + this.greeting = greeting; + } + + public String greet() { + return this.greeting; + } + + public static void main(String[] args) { + Hello h = new Hello("world"); + System.out.println(h.greet()); + } +} +""" + + +@pytest.fixture +def workspace(tmp_path: Path) -> tuple[Path, Path]: + """Create a minimal standalone Java workspace with a single Hello.java file. + + jdtls operates in "default project" mode for orphan files (no build config), + which is enough to parse the file and serve document symbols. No pom.xml, + no build.gradle, no .classpath — we don't need full classpath resolution + to verify that the request shape reaches jdtls correctly. + """ + src_file = tmp_path / "Hello.java" + src_file.write_text(_HELLO_JAVA) + return tmp_path, src_file + + +@pytest.fixture +async def proxy(workspace: tuple[Path, Path]) -> AsyncIterator[JdtlsProxy]: + """Start a real JdtlsProxy bound to the workspace fixture. + + Yields an initialized proxy and tears it down on test exit. Uses a minimal + ``InitializeParams`` dict that includes only the capabilities we exercise + in the tests below, to keep jdtls's initial workspace scan cheap. + """ + tmp_path, _ = workspace + p = JdtlsProxy() + + init_params = { + "processId": os.getpid(), + "rootUri": tmp_path.as_uri(), + "rootPath": str(tmp_path), + "capabilities": { + "textDocument": { + "synchronization": { + "dynamicRegistration": False, + "willSave": False, + "willSaveWaitUntil": False, + "didSave": True, + }, + "definition": {"dynamicRegistration": False, "linkSupport": False}, + "references": {"dynamicRegistration": False}, + "documentSymbol": { + "dynamicRegistration": False, + "hierarchicalDocumentSymbolSupport": True, + }, + "hover": {"dynamicRegistration": False, "contentFormat": ["plaintext"]}, + "completion": { + "dynamicRegistration": False, + "completionItem": {"snippetSupport": False}, + }, + }, + "workspace": {"configuration": False, "workspaceFolders": False}, + }, + "initializationOptions": {}, + "trace": "off", + } + + started = await p.start(init_params) + if not started: + pytest.fail( + "JdtlsProxy.start() returned False — jdtls failed to initialize. " + "Check the logs above for Java version / classpath issues." + ) + + try: + yield p + finally: + await p.stop() + + +async def _open_document(proxy: JdtlsProxy, src_file: Path) -> str: + """Send textDocument/didOpen for ``src_file`` and return its URI.""" + uri = src_file.as_uri() + await proxy.send_notification( + "textDocument/didOpen", + { + "textDocument": { + "uri": uri, + "languageId": "java", + "version": 1, + "text": src_file.read_text(), + } + }, + ) + # Give jdtls a moment to parse the file. We can't synchronously wait for + # parsing — LSP exposes no notification for that — so we sleep conservatively. + await asyncio.sleep(_JDTLS_PARSE_WAIT_SEC) + return uri + + +def _assert_no_npe_in_logs(caplog: pytest.LogCaptureFixture) -> None: + """Fail the test if any NullPointerException appeared in jdtls stderr. + + The proxy's ``_stderr_reader`` logs each stderr line via + ``logger.error("jdtls stderr: %s", line)``. We scan the captured records + for NPE markers — this catches regressions where the wire format is + wrong even when send_request returns a non-error response (e.g., when + jdtls falls back to a default value after logging the exception). + """ + npe_lines = [record.getMessage() for record in caplog.records if "NullPointerException" in record.getMessage()] + assert not npe_lines, ( + "jdtls threw NullPointerException — probably a request-shape bug " + "(camelCase vs snake_case). Captured lines:\n" + "\n".join(npe_lines) + ) + + +@pytest.mark.timeout(_E2E_TEST_TIMEOUT_SEC) +class TestJdtlsEndToEnd: + """End-to-end request/response tests against a real jdtls subprocess.""" + + async def test_initialize_handshake_succeeds(self, proxy: JdtlsProxy) -> None: + """Proves jdtls came up and announced its capabilities. + + This is implicit in the ``proxy`` fixture (which asserts start() + returned True), but an explicit test here makes the boundary clear: + if this fails, every other test will fail too, and the fixture is + to blame. + """ + assert proxy.is_available + caps = proxy.capabilities + # jdtls always advertises these — their presence confirms a clean init. + assert "definitionProvider" in caps + assert "documentSymbolProvider" in caps + + async def test_document_symbol_round_trip( + self, + workspace: tuple[Path, Path], + proxy: JdtlsProxy, + caplog: pytest.LogCaptureFixture, + ) -> None: + """Send textDocument/documentSymbol via _serialize_params and verify response. + + Document-symbol only needs the file to be parsed (no classpath resolution), + so it's the most reliable cross-environment e2e check. This is the + canonical camelCase regression test: if ``_serialize_params`` emits + ``text_document`` instead of ``textDocument``, jdtls logs NPE and + returns an error response, which we detect via caplog. + """ + _, src_file = workspace + uri = await _open_document(proxy, src_file) + + with caplog.at_level(logging.ERROR, logger="java_functional_lsp.proxy"): + params = lsp.DocumentSymbolParams( + text_document=lsp.TextDocumentIdentifier(uri=uri), + ) + serialized = _serialize_params(params) + + # Wire-format sanity: the serialization layer we send to jdtls MUST + # use camelCase field names. This is redundant with the unit tests + # in TestLspConverterCamelCase but pins the exact dict shape at the + # boundary of the real subprocess call. + assert "textDocument" in serialized, f"_serialize_params emitted wrong field names: {serialized.keys()}" + assert "text_document" not in serialized + + result = await proxy.send_request("textDocument/documentSymbol", serialized) + + # Primary assertion: jdtls returned something. With a correctly-shaped + # request, jdtls always returns a list (possibly empty) for a parsable + # file. None here means our proxy surfaced an error or timeout — both + # are regression signals. + assert result is not None, ( + "jdtls returned None for documentSymbol on a valid file. " + "This usually means the request shape was rejected — check caplog " + "for jdtls stderr output." + ) + assert isinstance(result, list) + # Hello.java declares a top-level class, so we expect at least one symbol. + assert len(result) > 0, ( + "jdtls returned an empty symbol list for a file with a top-level class. " + "Either jdtls didn't finish parsing or the file wasn't opened correctly." + ) + + _assert_no_npe_in_logs(caplog) + + async def test_definition_request_does_not_npe( + self, + workspace: tuple[Path, Path], + proxy: JdtlsProxy, + caplog: pytest.LogCaptureFixture, + ) -> None: + """The exact scenario from the v0.7.2 bug report: textDocument/definition. + + The NPE surfaced in ``NavigateToDefinitionHandler.definition`` when + ``TextDocumentPositionParams.getTextDocument()`` returned null. This + test sends a real definition request through the real serialization + path and asserts no NPE appears in jdtls stderr, regardless of whether + jdtls actually resolves the symbol (which depends on classpath state). + """ + _, src_file = workspace + uri = await _open_document(proxy, src_file) + + with caplog.at_level(logging.ERROR, logger="java_functional_lsp.proxy"): + # Position the cursor on the `greeting` identifier at line 8 col 20 + # inside `return this.greeting;`. Exact column doesn't matter — we + # care that the REQUEST reaches jdtls with a valid textDocument. + params = lsp.DefinitionParams( + text_document=lsp.TextDocumentIdentifier(uri=uri), + position=lsp.Position(line=8, character=20), + ) + serialized = _serialize_params(params) + assert "textDocument" in serialized + assert "text_document" not in serialized + + # We don't assert on the RESULT of this call — jdtls may return None + # or [] depending on classpath state. The critical assertion is that + # no NPE appeared in stderr, because an NPE would prove the camelCase + # regression has come back. + await proxy.send_request("textDocument/definition", serialized) + + _assert_no_npe_in_logs(caplog) + + async def test_hover_request_does_not_npe( + self, + workspace: tuple[Path, Path], + proxy: JdtlsProxy, + caplog: pytest.LogCaptureFixture, + ) -> None: + """Hover is another TextDocumentPositionParams-shaped request. + + Verifying hover in addition to definition catches regressions that + might only affect a subset of position-based handlers. + """ + _, src_file = workspace + uri = await _open_document(proxy, src_file) + + with caplog.at_level(logging.ERROR, logger="java_functional_lsp.proxy"): + params = lsp.HoverParams( + text_document=lsp.TextDocumentIdentifier(uri=uri), + position=lsp.Position(line=0, character=13), + ) + serialized = _serialize_params(params) + assert "textDocument" in serialized + + await proxy.send_request("textDocument/hover", serialized) + + _assert_no_npe_in_logs(caplog) + + async def test_did_open_notification_does_not_npe( + self, + workspace: tuple[Path, Path], + proxy: JdtlsProxy, + caplog: pytest.LogCaptureFixture, + ) -> None: + """Notifications go through the same serialization path as requests. + + didOpen uses ``DidOpenTextDocumentParams`` which wraps a + ``TextDocumentItem`` with snake_case ``language_id``. A camelCase bug + here would cause jdtls to fail parsing the notification silently + (no response, so no request-side error) but the NPE would appear in + stderr when jdtls tries to look up the file by URI later. + """ + _, src_file = workspace + uri = src_file.as_uri() + + with caplog.at_level(logging.ERROR, logger="java_functional_lsp.proxy"): + params = lsp.DidOpenTextDocumentParams( + text_document=lsp.TextDocumentItem( + uri=uri, + language_id="java", + version=1, + text=src_file.read_text(), + ), + ) + serialized = _serialize_params(params) + assert "textDocument" in serialized + assert "languageId" in serialized["textDocument"] + assert "language_id" not in serialized["textDocument"] + + await proxy.send_notification("textDocument/didOpen", serialized) + await asyncio.sleep(_JDTLS_PARSE_WAIT_SEC) + + _assert_no_npe_in_logs(caplog) + + async def test_references_request_does_not_npe( + self, + workspace: tuple[Path, Path], + proxy: JdtlsProxy, + caplog: pytest.LogCaptureFixture, + ) -> None: + """textDocument/references through _serialize_params. + + ReferenceParams is a compound type: it wraps ``text_document``, + ``position``, AND a nested ``context`` with its own snake_case field + ``include_declaration``. A vanilla cattrs converter would emit + ``context.include_declaration`` which jdtls would see as a null + ReferenceContext — guaranteed NPE. + """ + _, src_file = workspace + uri = await _open_document(proxy, src_file) + + with caplog.at_level(logging.ERROR, logger="java_functional_lsp.proxy"): + params = lsp.ReferenceParams( + text_document=lsp.TextDocumentIdentifier(uri=uri), + position=lsp.Position(line=8, character=20), # `greeting` inside greet() + context=lsp.ReferenceContext(include_declaration=True), + ) + serialized = _serialize_params(params) + assert "textDocument" in serialized + assert "text_document" not in serialized + # Nested field is also camelCase + assert "context" in serialized + assert "includeDeclaration" in serialized["context"] + assert "include_declaration" not in serialized["context"] + + await proxy.send_request("textDocument/references", serialized) + + _assert_no_npe_in_logs(caplog) + + async def test_completion_request_does_not_npe( + self, + workspace: tuple[Path, Path], + proxy: JdtlsProxy, + caplog: pytest.LogCaptureFixture, + ) -> None: + """textDocument/completion through _serialize_params. + + Completion exercises a distinct jdtls code path from definition/hover + (content assist rather than symbol resolution) and uses + ``CompletionParams`` which inherits TextDocumentPositionParams. A + request-shape bug here would NPE the completion handler. + """ + _, src_file = workspace + uri = await _open_document(proxy, src_file) + + with caplog.at_level(logging.ERROR, logger="java_functional_lsp.proxy"): + # Position after `h.` on the main() line where completion makes sense. + params = lsp.CompletionParams( + text_document=lsp.TextDocumentIdentifier(uri=uri), + position=lsp.Position(line=13, character=29), + ) + serialized = _serialize_params(params) + assert "textDocument" in serialized + assert "text_document" not in serialized + + await proxy.send_request("textDocument/completion", serialized) + + _assert_no_npe_in_logs(caplog) From e5c368b9e0c4d0a5616254fa9d3ef82895e2bbf4 Mon Sep 17 00:00:00 2001 From: Aviad Shiber Date: Fri, 10 Apr 2026 06:53:56 +0300 Subject: [PATCH 03/14] ci: pin Linux jdtls to 1.57.0 to match Homebrew formula MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit pinned 1.47.0 arbitrarily — that's 10 minor versions behind the current milestone. Switch to 1.57.0 (build 202602261110) to match what Homebrew ships on macOS, so the Linux and macOS e2e jobs exercise the same jdtls version. Also switch to the canonical Eclipse mirror download URL used by the Homebrew formula, which redirects through a mirror selector and gives more reliable downloads than hitting download.eclipse.org directly. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/test.yml | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e387396..c411ac0 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -64,17 +64,22 @@ jobs: run: | set -euo pipefail # Download the Eclipse JDT Language Server milestone build directly. - # Pinned version for deterministic CI runs; bump as needed. - JDTLS_VERSION="1.47.0" - JDTLS_BUILD="202509181638" - JDTLS_URL="https://download.eclipse.org/jdtls/milestones/${JDTLS_VERSION}/jdt-language-server-${JDTLS_VERSION}-${JDTLS_BUILD}.tar.gz" + # Pinned to match the Homebrew formula so Linux + macOS exercise the + # same version. Latest is 1.57.0 (build 202602261110); bump both + # JDTLS_VERSION and JDTLS_BUILD together when updating — get the + # current build suffix from the Homebrew formula's `url` line at + # https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/j/jdtls.rb + JDTLS_VERSION="1.57.0" + JDTLS_BUILD="202602261110" + JDTLS_URL="https://www.eclipse.org/downloads/download.php?file=/jdtls/milestones/${JDTLS_VERSION}/jdt-language-server-${JDTLS_VERSION}-${JDTLS_BUILD}.tar.gz&r=1" JDTLS_DIR="$HOME/.local/share/jdtls" BIN_DIR="$HOME/.local/bin" mkdir -p "$JDTLS_DIR" "$BIN_DIR" - # Follow redirects to reach the actual tarball through Eclipse's mirror. - curl -sSL -o /tmp/jdtls.tar.gz "$JDTLS_URL" + # -L follows Eclipse's mirror redirect; -f fails loudly on 404. + curl -sSLf -o /tmp/jdtls.tar.gz "$JDTLS_URL" tar -xzf /tmp/jdtls.tar.gz -C "$JDTLS_DIR" - # Create a wrapper script on PATH that invokes the bundled Python launcher. + # Wrapper script on PATH that invokes the bundled Python launcher + # with python3 (the tarball ships jdtls.py in bin/). cat > "$BIN_DIR/jdtls" << SHELL_EOF #!/bin/bash exec python3 "$JDTLS_DIR/bin/jdtls" "\$@" From 6158e6adae62a68a0f92ba808889c61e4f450d2c Mon Sep 17 00:00:00 2001 From: Aviad Shiber Date: Fri, 10 Apr 2026 06:56:06 +0300 Subject: [PATCH 04/14] ci: verify jdtls tarball against Homebrew-formula sha256 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without a hash check, a compromised Eclipse mirror could ship a tampered tarball and our CI would execute arbitrary code inside the e2e jdtls subprocess. Pin and verify the sha256 from the Homebrew formula — it's the canonical upstream hash for this milestone build. The bump instructions in the comment now list all three values that must be updated together (VERSION, BUILD, SHA256) to prevent future maintainers from introducing a hash/version skew. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/test.yml | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c411ac0..45a67bd 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -65,18 +65,26 @@ jobs: set -euo pipefail # Download the Eclipse JDT Language Server milestone build directly. # Pinned to match the Homebrew formula so Linux + macOS exercise the - # same version. Latest is 1.57.0 (build 202602261110); bump both - # JDTLS_VERSION and JDTLS_BUILD together when updating — get the - # current build suffix from the Homebrew formula's `url` line at - # https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/j/jdtls.rb + # same version. When bumping, update ALL THREE of JDTLS_VERSION, + # JDTLS_BUILD, and JDTLS_SHA256 together — the canonical source is + # the Homebrew formula: + # https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/j/jdtls.rb + # It has `url "...jdt-language-server--.tar.gz"` + # and `sha256 ""` which you can copy verbatim. JDTLS_VERSION="1.57.0" JDTLS_BUILD="202602261110" + JDTLS_SHA256="f7ffa93fe1bbbea95dac13dd97cdcd25c582d6e56db67258da0dcceb2302601e" JDTLS_URL="https://www.eclipse.org/downloads/download.php?file=/jdtls/milestones/${JDTLS_VERSION}/jdt-language-server-${JDTLS_VERSION}-${JDTLS_BUILD}.tar.gz&r=1" JDTLS_DIR="$HOME/.local/share/jdtls" BIN_DIR="$HOME/.local/bin" mkdir -p "$JDTLS_DIR" "$BIN_DIR" # -L follows Eclipse's mirror redirect; -f fails loudly on 404. curl -sSLf -o /tmp/jdtls.tar.gz "$JDTLS_URL" + # Verify the tarball integrity against the hash pinned in the + # Homebrew formula. This protects against mirror tampering — + # without it, a compromised Eclipse mirror could ship arbitrary + # code that our e2e tests would then execute. + echo "${JDTLS_SHA256} /tmp/jdtls.tar.gz" | sha256sum -c - tar -xzf /tmp/jdtls.tar.gz -C "$JDTLS_DIR" # Wrapper script on PATH that invokes the bundled Python launcher # with python3 (the tarball ships jdtls.py in bin/). From da7a5249a0474f51af6a9aef2e235d7c5d2421a9 Mon Sep 17 00:00:00 2001 From: Aviad Shiber Date: Fri, 10 Apr 2026 07:00:11 +0300 Subject: [PATCH 05/14] ci: disable coverage fail-under for e2e job MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The e2e job runs only 7 tests (``-m e2e``) which exercise the proxy + server layers but none of the analyzers, so total coverage lands at ~27% — below the 60% fail-under threshold enforced by pyproject.toml's default addopts. That made CI report "7 passed" and then fail with exit 1. Pass --no-cov to the e2e pytest invocation. The unit matrix already produces the authoritative coverage report, so the e2e job doesn't need its own. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/test.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 45a67bd..061a752 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -105,6 +105,11 @@ jobs: java -version echo "JAVA_HOME=$JAVA_HOME" - name: Run e2e tests - run: uv run pytest -v -m e2e + # --no-cov disables coverage reporting for this job. The e2e tests + # only exercise the proxy + server layers, so they cannot meet the + # 60% fail-under threshold enforced by pyproject.toml's default + # addopts. The unit matrix above already produces the authoritative + # coverage report. + run: uv run pytest -v -m e2e --no-cov env: NO_COLOR: "1" From ddbfe8aef033e11d55d684d0e1bc218596aea7ac Mon Sep 17 00:00:00 2001 From: Aviad Shiber Date: Fri, 10 Apr 2026 07:07:33 +0300 Subject: [PATCH 06/14] test: add major-feature sanity e2e tests + run full suite in integration CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds 4 new e2e tests in TestMajorFeatureSanity that verify major product features end-to-end through the real LanguageServer instance: - test_null_return_diagnostic_fires — analyzer pipeline produces null-return diagnostic for a Java file with return null - test_null_check_to_monadic_diagnostic_fires — if(x != null) pattern triggers the monadic-flow hint - test_code_action_returns_quickfix_with_edit — CodeActionParams with a null-return diagnostic returns a QuickFix with Option.none() edit + auto-import - test_code_action_ignores_non_java_functional_lsp_diagnostics — jdtls diagnostics are filtered out, not processed by our handler These tests bootstrap a real pygls workspace via server.protocol._workspace, inject documents via put_text_document, and call the on_code_action handler directly. They exercise the full pipeline: pygls workspace → analyzer chain → fix generator → lsp CodeAction → WorkspaceEdit. CI changes: - Renamed e2e-test job to "integration" and changed it to run the FULL test suite (uv run pytest -v) rather than just -m e2e. This means the integration job runs all 310 tests (unit + e2e) with jdtls installed, and coverage naturally reaches 82% without any --no-cov hack. - The unit matrix (test job) still excludes e2e tests via -m "not e2e" for fast cross-version feedback. Verification: 310 passed (was 306), 82% coverage, pyright + ruff clean. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/test.yml | 21 ++--- tests/test_e2e_jdtls.py | 186 +++++++++++++++++++++++++++++++++++++ 2 files changed, 195 insertions(+), 12 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 061a752..b231883 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -35,16 +35,18 @@ jobs: env: NO_COLOR: "1" - e2e-test: - name: End-to-End (jdtls) / ${{ matrix.os }} + integration: + name: Integration (with jdtls) / ${{ matrix.os }} runs-on: ${{ matrix.os }} strategy: fail-fast: false matrix: os: [ubuntu-latest, macos-latest] - # One Python version is enough — the e2e tests verify the jdtls request/ - # response pipeline, not Python-version-specific behavior. The unit - # matrix above already exercises every Python version. + # Runs the FULL test suite (unit + e2e) with jdtls installed, on one + # Python version per OS. This is a comprehensive sanity check that + # exercises: custom analyzer diagnostics, code action generation, + # AND jdtls request forwarding end-to-end. The unit matrix above + # provides fast Python-version-specific feedback without jdtls. steps: - uses: actions/checkout@v6 - name: Set up Python 3.12 @@ -104,12 +106,7 @@ jobs: jdtls --help | head -5 || true java -version echo "JAVA_HOME=$JAVA_HOME" - - name: Run e2e tests - # --no-cov disables coverage reporting for this job. The e2e tests - # only exercise the proxy + server layers, so they cannot meet the - # 60% fail-under threshold enforced by pyproject.toml's default - # addopts. The unit matrix above already produces the authoritative - # coverage report. - run: uv run pytest -v -m e2e --no-cov + - name: Run full test suite (unit + e2e) + run: uv run pytest -v env: NO_COLOR: "1" diff --git a/tests/test_e2e_jdtls.py b/tests/test_e2e_jdtls.py index ce3c652..ac34fa8 100644 --- a/tests/test_e2e_jdtls.py +++ b/tests/test_e2e_jdtls.py @@ -423,3 +423,189 @@ async def test_completion_request_does_not_npe( await proxy.send_request("textDocument/completion", serialized) _assert_no_npe_in_logs(caplog) + + +# -------------------------------------------------------------------------- +# Major-feature sanity checks (no jdtls required) +# -------------------------------------------------------------------------- +# +# These tests drive the real ``java_functional_lsp.server`` module-level +# LanguageServer instance with a real pygls workspace, verifying that the +# major features (custom analyzer diagnostics, QuickFix code actions) work +# end-to-end from pygls-typed LSP params through to published diagnostics +# and WorkspaceEdits. +# +# They are marked ``e2e`` because they test the server as an integrated unit +# — but unlike the jdtls tests above, they do NOT require jdtls and run on +# any platform in a few milliseconds. +# +# The two sibling ``pytest.mark.skipif`` guards above apply to every test in +# this file via ``pytestmark``, so these tests also auto-skip on CI runners +# without jdtls. For local runs without jdtls, use +# ``pytest tests/test_e2e_jdtls.py::TestMajorFeatureSanity`` to force-run +# just these. + +# We need to override the module-level skipif to let these sanity tests +# still run even without jdtls. They're in a separate class and we apply +# their own marks. +_SANITY_HELLO_WITH_BUGS = """\ +import java.util.List; + +public class BuggyExample { + public String firstOrNull(List xs) { + if (xs != null) { + return xs.get(0); + } else { + return null; + } + } +} +""" + + +@pytest.mark.e2e +class TestMajorFeatureSanity: + """End-to-end sanity checks for major features without jdtls. + + These tests drive ``server.on_did_open`` / ``server.on_code_action`` + through the real pygls workspace. They verify: + + 1. Opening a file with a known-bad pattern causes the custom analyzers + to publish the expected diagnostic. + 2. Requesting code actions on a diagnostic returns a valid WorkspaceEdit + with the QuickFix kind and the right rewrite title. + + These are the "does the product actually work end-to-end" tests that + complement the unit tests (which verify individual analyzers/fixes in + isolation) and the jdtls tests above (which verify request forwarding). + """ + + @staticmethod + def _ensure_workspace_initialized() -> None: + """Ensure the pygls server workspace is available. + + pygls' ``server.workspace`` property raises RuntimeError if the protocol + hasn't processed an ``initialize`` request. We bootstrap a minimal + workspace directly via the protocol's private API to avoid going through + the full JSON-RPC lifecycle in a unit test. + """ + from pygls.workspace import Workspace + + from java_functional_lsp.server import server + + if server.protocol._workspace is None: + server.protocol._workspace = Workspace( + root_uri="file:///test", + sync_kind=lsp.TextDocumentSyncKind.Full, + ) + + def test_null_return_diagnostic_fires(self) -> None: + """Opening a Java file with ``return null`` publishes a null-return diagnostic.""" + from java_functional_lsp.server import _run_analysis + + diags = _run_analysis(_SANITY_HELLO_WITH_BUGS, "file:///test/BuggyExample.java") + codes = [d.code for d in diags] + assert "null-return" in codes, f"Expected null-return diagnostic in {codes} — analyzer pipeline may be broken" + + def test_null_check_to_monadic_diagnostic_fires(self) -> None: + """The if(x != null) pattern should produce a null-check-to-monadic hint.""" + from java_functional_lsp.server import _run_analysis + + diags = _run_analysis(_SANITY_HELLO_WITH_BUGS, "file:///test/BuggyExample.java") + codes = [d.code for d in diags] + assert "null-check-to-monadic" in codes + + def test_code_action_returns_quickfix_with_edit(self) -> None: + """A CodeActionParams with a null-return diagnostic must yield a QuickFix edit. + + Drives the full code-action pipeline: parse → lookup fix generator → + apply → build lsp.CodeAction. A regression in fix registry lookup or + the server's code-action handler would fail here. + """ + from java_functional_lsp.server import on_code_action, server + + self._ensure_workspace_initialized() + uri = "file:///test/BuggyExample.java" + source = _SANITY_HELLO_WITH_BUGS + + # Inject the document into the pygls workspace so on_code_action + # can read its source. + server.workspace.put_text_document( + lsp.TextDocumentItem(uri=uri, language_id="java", version=1, text=source), + ) + try: + # Build a synthetic null-return diagnostic at the `null` literal + # in `return null;` (line 7, columns 19-23 in _SANITY_HELLO_WITH_BUGS). + null_return_diag = lsp.Diagnostic( + range=lsp.Range( + start=lsp.Position(line=7, character=19), + end=lsp.Position(line=7, character=23), + ), + message="Avoid returning null.", + severity=lsp.DiagnosticSeverity.Warning, + code="null-return", + source="java-functional-lsp", + ) + params = lsp.CodeActionParams( + text_document=lsp.TextDocumentIdentifier(uri=uri), + range=null_return_diag.range, + context=lsp.CodeActionContext(diagnostics=[null_return_diag]), + ) + + result = on_code_action(params) + finally: + server.workspace.remove_text_document(uri) + + assert result is not None, "on_code_action returned None for null-return diagnostic" + assert len(result) >= 1 + action = result[0] + assert action.kind == lsp.CodeActionKind.QuickFix + assert action.title == "Replace with Option.none()" + assert action.edit is not None + assert action.edit.changes is not None + edits = action.edit.changes[uri] + # We expect the null replacement plus an auto-import for Option. + assert any("Option.none()" in e.new_text for e in edits), ( + f"Expected Option.none() in edits, got: {[e.new_text for e in edits]}" + ) + assert any("import io.vavr.control.Option;" in e.new_text for e in edits), ( + "Expected auto-import of io.vavr.control.Option in the edits" + ) + + def test_code_action_ignores_non_java_functional_lsp_diagnostics(self) -> None: + """Diagnostics from other sources (e.g. jdtls) must be ignored by our handler. + + Regression guard for the ``if diag.source != "java-functional-lsp": continue`` + filter in on_code_action. Without it, our handler would try to look up + fix generators for jdtls diagnostic codes and pollute the action list. + """ + from java_functional_lsp.server import on_code_action, server + + self._ensure_workspace_initialized() + uri = "file:///test/BuggyExample.java" + server.workspace.put_text_document( + lsp.TextDocumentItem(uri=uri, language_id="java", version=1, text=_SANITY_HELLO_WITH_BUGS), + ) + try: + jdtls_diag = lsp.Diagnostic( + range=lsp.Range( + start=lsp.Position(line=7, character=19), + end=lsp.Position(line=7, character=23), + ), + message="Some jdtls warning", + severity=lsp.DiagnosticSeverity.Warning, + code="something-jdtls-specific", + source="Java", # NOT java-functional-lsp + ) + params = lsp.CodeActionParams( + text_document=lsp.TextDocumentIdentifier(uri=uri), + range=jdtls_diag.range, + context=lsp.CodeActionContext(diagnostics=[jdtls_diag]), + ) + + result = on_code_action(params) + finally: + server.workspace.remove_text_document(uri) + + # Filtered out → no actions returned at all. + assert result is None From 2787394667139dded2503394e6c2391401b53fa1 Mon Sep 17 00:00:00 2001 From: Aviad Shiber Date: Fri, 10 Apr 2026 07:11:21 +0300 Subject: [PATCH 07/14] chore: raise coverage threshold from 60% to 80% The test suite now achieves 82% coverage consistently (310 tests: unit + e2e). Raise the fail-under bar so future changes that drop coverage below 80% are caught by CI. Co-Authored-By: Claude Opus 4.6 (1M context) --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 0c06acb..32ea578 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -63,7 +63,7 @@ testpaths = ["tests"] python_files = ["test_*.py"] python_classes = ["Test*"] python_functions = ["test_*"] -addopts = "--cov=java_functional_lsp --cov-report=term-missing --cov-fail-under=60" +addopts = "--cov=java_functional_lsp --cov-report=term-missing --cov-fail-under=80" asyncio_mode = "auto" markers = [ "e2e: end-to-end tests that spawn a real jdtls subprocess (require jdtls + Java 21+; skipped when unavailable)", From a9f1bf27e3511c2ebe2fc79c4015b534af4cf949 Mon Sep 17 00:00:00 2001 From: Aviad Shiber Date: Fri, 10 Apr 2026 07:22:57 +0300 Subject: [PATCH 08/14] test: add server integration tests to meet 80% coverage bar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moves the major-feature sanity tests from test_e2e_jdtls.py (gated by jdtls skipif) to a new tests/test_server.py (no jdtls dependency). This lets them run in the regular unit matrix, boosting coverage from 77% to 80% without needing jdtls installed. New test file tests/test_server.py contains: TestServerHelpers (11 tests): - _load_config: no workspace, missing file, valid JSON, invalid JSON - _to_lsp_diagnostic: with/without DiagnosticData - _analyze_document: with excludes config, without excludes - _serialize_params: camelCase regression guard - _handle_exception: logs uncaught exceptions - _jdtls_raw_to_lsp_diagnostics: valid + malformed raw dicts - _on_jdtls_diagnostics callback: mocked publish verifies analyzer runs TestAnalyzerPipeline (4 tests): - null-return, null-check-to-monadic, try-catch-to-monadic diagnostics - Clean file produces no diagnostics TestCodeActionPipeline (4 tests): - null-return QuickFix → Option.none() + auto-import - try-catch-to-monadic QuickFix → Try.of() + auto-import - Non-java-functional-lsp diagnostics filtered out - Empty diagnostic context → no actions Coverage threshold raised to 80% in pyproject.toml. Unit matrix (without e2e) reaches 80.13%; full suite (with e2e) reaches 83.25%. Verification: 327 passed (320 unit + 7 e2e), pyright + ruff clean. Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/test_e2e_jdtls.py | 188 +------------------- tests/test_server.py | 371 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 375 insertions(+), 184 deletions(-) create mode 100644 tests/test_server.py diff --git a/tests/test_e2e_jdtls.py b/tests/test_e2e_jdtls.py index ac34fa8..88e593e 100644 --- a/tests/test_e2e_jdtls.py +++ b/tests/test_e2e_jdtls.py @@ -425,187 +425,7 @@ async def test_completion_request_does_not_npe( _assert_no_npe_in_logs(caplog) -# -------------------------------------------------------------------------- -# Major-feature sanity checks (no jdtls required) -# -------------------------------------------------------------------------- -# -# These tests drive the real ``java_functional_lsp.server`` module-level -# LanguageServer instance with a real pygls workspace, verifying that the -# major features (custom analyzer diagnostics, QuickFix code actions) work -# end-to-end from pygls-typed LSP params through to published diagnostics -# and WorkspaceEdits. -# -# They are marked ``e2e`` because they test the server as an integrated unit -# — but unlike the jdtls tests above, they do NOT require jdtls and run on -# any platform in a few milliseconds. -# -# The two sibling ``pytest.mark.skipif`` guards above apply to every test in -# this file via ``pytestmark``, so these tests also auto-skip on CI runners -# without jdtls. For local runs without jdtls, use -# ``pytest tests/test_e2e_jdtls.py::TestMajorFeatureSanity`` to force-run -# just these. - -# We need to override the module-level skipif to let these sanity tests -# still run even without jdtls. They're in a separate class and we apply -# their own marks. -_SANITY_HELLO_WITH_BUGS = """\ -import java.util.List; - -public class BuggyExample { - public String firstOrNull(List xs) { - if (xs != null) { - return xs.get(0); - } else { - return null; - } - } -} -""" - - -@pytest.mark.e2e -class TestMajorFeatureSanity: - """End-to-end sanity checks for major features without jdtls. - - These tests drive ``server.on_did_open`` / ``server.on_code_action`` - through the real pygls workspace. They verify: - - 1. Opening a file with a known-bad pattern causes the custom analyzers - to publish the expected diagnostic. - 2. Requesting code actions on a diagnostic returns a valid WorkspaceEdit - with the QuickFix kind and the right rewrite title. - - These are the "does the product actually work end-to-end" tests that - complement the unit tests (which verify individual analyzers/fixes in - isolation) and the jdtls tests above (which verify request forwarding). - """ - - @staticmethod - def _ensure_workspace_initialized() -> None: - """Ensure the pygls server workspace is available. - - pygls' ``server.workspace`` property raises RuntimeError if the protocol - hasn't processed an ``initialize`` request. We bootstrap a minimal - workspace directly via the protocol's private API to avoid going through - the full JSON-RPC lifecycle in a unit test. - """ - from pygls.workspace import Workspace - - from java_functional_lsp.server import server - - if server.protocol._workspace is None: - server.protocol._workspace = Workspace( - root_uri="file:///test", - sync_kind=lsp.TextDocumentSyncKind.Full, - ) - - def test_null_return_diagnostic_fires(self) -> None: - """Opening a Java file with ``return null`` publishes a null-return diagnostic.""" - from java_functional_lsp.server import _run_analysis - - diags = _run_analysis(_SANITY_HELLO_WITH_BUGS, "file:///test/BuggyExample.java") - codes = [d.code for d in diags] - assert "null-return" in codes, f"Expected null-return diagnostic in {codes} — analyzer pipeline may be broken" - - def test_null_check_to_monadic_diagnostic_fires(self) -> None: - """The if(x != null) pattern should produce a null-check-to-monadic hint.""" - from java_functional_lsp.server import _run_analysis - - diags = _run_analysis(_SANITY_HELLO_WITH_BUGS, "file:///test/BuggyExample.java") - codes = [d.code for d in diags] - assert "null-check-to-monadic" in codes - - def test_code_action_returns_quickfix_with_edit(self) -> None: - """A CodeActionParams with a null-return diagnostic must yield a QuickFix edit. - - Drives the full code-action pipeline: parse → lookup fix generator → - apply → build lsp.CodeAction. A regression in fix registry lookup or - the server's code-action handler would fail here. - """ - from java_functional_lsp.server import on_code_action, server - - self._ensure_workspace_initialized() - uri = "file:///test/BuggyExample.java" - source = _SANITY_HELLO_WITH_BUGS - - # Inject the document into the pygls workspace so on_code_action - # can read its source. - server.workspace.put_text_document( - lsp.TextDocumentItem(uri=uri, language_id="java", version=1, text=source), - ) - try: - # Build a synthetic null-return diagnostic at the `null` literal - # in `return null;` (line 7, columns 19-23 in _SANITY_HELLO_WITH_BUGS). - null_return_diag = lsp.Diagnostic( - range=lsp.Range( - start=lsp.Position(line=7, character=19), - end=lsp.Position(line=7, character=23), - ), - message="Avoid returning null.", - severity=lsp.DiagnosticSeverity.Warning, - code="null-return", - source="java-functional-lsp", - ) - params = lsp.CodeActionParams( - text_document=lsp.TextDocumentIdentifier(uri=uri), - range=null_return_diag.range, - context=lsp.CodeActionContext(diagnostics=[null_return_diag]), - ) - - result = on_code_action(params) - finally: - server.workspace.remove_text_document(uri) - - assert result is not None, "on_code_action returned None for null-return diagnostic" - assert len(result) >= 1 - action = result[0] - assert action.kind == lsp.CodeActionKind.QuickFix - assert action.title == "Replace with Option.none()" - assert action.edit is not None - assert action.edit.changes is not None - edits = action.edit.changes[uri] - # We expect the null replacement plus an auto-import for Option. - assert any("Option.none()" in e.new_text for e in edits), ( - f"Expected Option.none() in edits, got: {[e.new_text for e in edits]}" - ) - assert any("import io.vavr.control.Option;" in e.new_text for e in edits), ( - "Expected auto-import of io.vavr.control.Option in the edits" - ) - - def test_code_action_ignores_non_java_functional_lsp_diagnostics(self) -> None: - """Diagnostics from other sources (e.g. jdtls) must be ignored by our handler. - - Regression guard for the ``if diag.source != "java-functional-lsp": continue`` - filter in on_code_action. Without it, our handler would try to look up - fix generators for jdtls diagnostic codes and pollute the action list. - """ - from java_functional_lsp.server import on_code_action, server - - self._ensure_workspace_initialized() - uri = "file:///test/BuggyExample.java" - server.workspace.put_text_document( - lsp.TextDocumentItem(uri=uri, language_id="java", version=1, text=_SANITY_HELLO_WITH_BUGS), - ) - try: - jdtls_diag = lsp.Diagnostic( - range=lsp.Range( - start=lsp.Position(line=7, character=19), - end=lsp.Position(line=7, character=23), - ), - message="Some jdtls warning", - severity=lsp.DiagnosticSeverity.Warning, - code="something-jdtls-specific", - source="Java", # NOT java-functional-lsp - ) - params = lsp.CodeActionParams( - text_document=lsp.TextDocumentIdentifier(uri=uri), - range=jdtls_diag.range, - context=lsp.CodeActionContext(diagnostics=[jdtls_diag]), - ) - - result = on_code_action(params) - finally: - server.workspace.remove_text_document(uri) - - # Filtered out → no actions returned at all. - assert result is None +# Major-feature sanity checks (analyzer pipeline + code actions) now live in +# tests/test_server.py, which is NOT gated by the jdtls skipif above. They run +# in the regular unit matrix on every Python version and contribute to the 80% +# coverage floor without requiring jdtls to be installed. diff --git a/tests/test_server.py b/tests/test_server.py new file mode 100644 index 0000000..7851223 --- /dev/null +++ b/tests/test_server.py @@ -0,0 +1,371 @@ +"""Integration tests for the LanguageServer — analyzer pipeline + code actions. + +These tests drive the real ``java_functional_lsp.server`` module through the +same code paths IntelliJ/VS Code hit: ``_run_analysis`` for diagnostics and +``on_code_action`` for QuickFix generation. They bootstrap a real pygls +workspace, inject documents, and assert on the results. + +They run in the regular unit matrix (no jdtls required, no ``@pytest.mark.e2e``). +""" + +from __future__ import annotations + +from typing import Any + +from lsprotocol import types as lsp +from pygls.workspace import Workspace + +_BUGGY_JAVA = """\ +import java.util.List; + +public class BuggyExample { + public String firstOrNull(List xs) { + if (xs != null) { + return xs.get(0); + } else { + return null; + } + } +} +""" + +_TRY_CATCH_JAVA = """\ +import java.io.IOException; + +public class TryCatchExample { + public String read() { + try { + return riskyRead(); + } catch (IOException e) { + return "fallback"; + } + } +} +""" + + +def _ensure_workspace() -> None: + """Bootstrap a minimal pygls workspace if not already initialized.""" + from java_functional_lsp.server import server + + if server.protocol._workspace is None: + server.protocol._workspace = Workspace( + root_uri="file:///test", + sync_kind=lsp.TextDocumentSyncKind.Full, + ) + + +class TestServerHelpers: + """Tests for server.py internal helpers.""" + + def test_load_config_returns_empty_for_no_workspace(self) -> None: + from java_functional_lsp.server import _load_config + + assert _load_config(None) == {} + assert _load_config("") == {} + + def test_load_config_returns_empty_for_missing_file(self, tmp_path: Any) -> None: + from java_functional_lsp.server import _load_config + + assert _load_config(str(tmp_path)) == {} + + def test_load_config_reads_json(self, tmp_path: Any) -> None: + from java_functional_lsp.server import _load_config + + config_file = tmp_path / ".java-functional-lsp.json" + config_file.write_text('{"rules": {"null-return": "off"}}') + result = _load_config(str(tmp_path)) + assert result == {"rules": {"null-return": "off"}} + + def test_load_config_handles_invalid_json(self, tmp_path: Any) -> None: + from java_functional_lsp.server import _load_config + + config_file = tmp_path / ".java-functional-lsp.json" + config_file.write_text("not valid json {{{") + result = _load_config(str(tmp_path)) + assert result == {} + + def test_to_lsp_diagnostic_with_data(self) -> None: + from java_functional_lsp.analyzers.base import Diagnostic as LintDiag + from java_functional_lsp.analyzers.base import DiagnosticData, Severity + from java_functional_lsp.server import _to_lsp_diagnostic + + diag = LintDiag( + line=5, + col=10, + end_line=5, + end_col=20, + severity=Severity.HINT, + code="test-rule", + message="test message", + data=DiagnosticData(fix_type="FIX", target_library="lib", rationale="reason"), + ) + result = _to_lsp_diagnostic(diag) + assert result.severity == lsp.DiagnosticSeverity.Hint + assert result.data is not None + assert result.data["fixType"] == "FIX" + + def test_to_lsp_diagnostic_without_data(self) -> None: + from java_functional_lsp.analyzers.base import Diagnostic as LintDiag + from java_functional_lsp.analyzers.base import Severity + from java_functional_lsp.server import _to_lsp_diagnostic + + diag = LintDiag(line=0, col=0, end_line=0, end_col=5, severity=Severity.WARNING, code="x", message="msg") + result = _to_lsp_diagnostic(diag) + assert result.data is None + + def test_analyze_document_with_excludes(self) -> None: + from java_functional_lsp.server import _analyze_document, server + + old_config = server._config + server._config = {"excludes": ["**/generated/**"]} + try: + result = _analyze_document( + "class T { String f() { return null; } }", + "file:///project/src/main/generated/Foo.java", + ) + assert result == [] + finally: + server._config = old_config + + def test_analyze_document_without_excludes(self) -> None: + from java_functional_lsp.server import _analyze_document + + result = _analyze_document("class T { String f() { return null; } }", "file:///Foo.java") + assert any(d.code == "null-return" for d in result) + + def test_serialize_params_camelcase(self) -> None: + """The LSP converter must emit camelCase field names.""" + from java_functional_lsp.server import _serialize_params + + params = lsp.DefinitionParams( + text_document=lsp.TextDocumentIdentifier(uri="file:///x.java"), + position=lsp.Position(line=0, character=0), + ) + result = _serialize_params(params) + assert "textDocument" in result + assert "text_document" not in result + + def test_handle_exception_logs(self, caplog: Any) -> None: + """sys.excepthook is wired to _handle_exception for crash debugging.""" + import logging + + from java_functional_lsp.server import _handle_exception + + with caplog.at_level(logging.ERROR, logger="java_functional_lsp.server"): + _handle_exception(ValueError, ValueError("test crash"), None) + assert any("Uncaught exception" in r.getMessage() for r in caplog.records) + + def test_jdtls_raw_to_lsp_diagnostics_valid(self) -> None: + """Raw jdtls diagnostic dicts should be structured into lsp.Diagnostic.""" + from java_functional_lsp.server import _jdtls_raw_to_lsp_diagnostics + + raw = [ + { + "range": {"start": {"line": 1, "character": 0}, "end": {"line": 1, "character": 10}}, + "severity": 2, + "code": "jdt.warning", + "source": "Java", + "message": "Unused import", + } + ] + result = _jdtls_raw_to_lsp_diagnostics(raw) + assert len(result) == 1 + assert result[0].message == "Unused import" + assert result[0].source == "Java" + + def test_jdtls_raw_to_lsp_diagnostics_malformed(self) -> None: + """Completely broken raw diagnostics should not crash, just be skipped.""" + from java_functional_lsp.server import _jdtls_raw_to_lsp_diagnostics + + result = _jdtls_raw_to_lsp_diagnostics([42, None, "not a dict"]) + assert result == [] + + def test_on_jdtls_diagnostics_callback(self) -> None: + """The server's _on_jdtls_diagnostics callback re-analyzes the document.""" + from unittest.mock import patch + + from java_functional_lsp.server import server + + _ensure_workspace() + uri = "file:///test/Callback.java" + server.workspace.put_text_document( + lsp.TextDocumentItem( + uri=uri, language_id="java", version=1, text="class T { String f() { return null; } }" + ), + ) + try: + # Mock publish to avoid transport errors + with patch.object(server, "text_document_publish_diagnostics") as mock_pub: + server._on_jdtls_diagnostics(uri, []) + mock_pub.assert_called_once() + published = mock_pub.call_args[0][0] + # Verify our custom analyzer found the null-return + codes = [d.code for d in published.diagnostics] + assert "null-return" in codes + finally: + server.workspace.remove_text_document(uri) + + +class TestAnalyzerPipeline: + """Verify the full analyzer chain produces the expected diagnostics.""" + + def test_null_return_diagnostic(self) -> None: + from java_functional_lsp.server import _run_analysis + + diags = _run_analysis(_BUGGY_JAVA, "file:///test/BuggyExample.java") + codes = [d.code for d in diags] + assert "null-return" in codes + + def test_null_check_to_monadic_diagnostic(self) -> None: + from java_functional_lsp.server import _run_analysis + + diags = _run_analysis(_BUGGY_JAVA, "file:///test/BuggyExample.java") + codes = [d.code for d in diags] + assert "null-check-to-monadic" in codes + + def test_try_catch_to_monadic_diagnostic(self) -> None: + from java_functional_lsp.server import _run_analysis + + diags = _run_analysis(_TRY_CATCH_JAVA, "file:///test/TryCatch.java") + codes = [d.code for d in diags] + assert "try-catch-to-monadic" in codes + + def test_no_diagnostics_on_clean_file(self) -> None: + from java_functional_lsp.server import _run_analysis + + clean = "public class Clean {\n public int add(int a, int b) {\n return a + b;\n }\n}\n" + diags = _run_analysis(clean, "file:///test/Clean.java") + assert len(diags) == 0 + + +class TestCodeActionPipeline: + """Verify the code-action handler produces valid WorkspaceEdits.""" + + def test_null_return_quickfix(self) -> None: + """null-return diagnostic → QuickFix with Option.none() + auto-import.""" + from java_functional_lsp.server import on_code_action, server + + _ensure_workspace() + uri = "file:///test/BuggyExample.java" + server.workspace.put_text_document( + lsp.TextDocumentItem(uri=uri, language_id="java", version=1, text=_BUGGY_JAVA), + ) + try: + diag = lsp.Diagnostic( + range=lsp.Range(start=lsp.Position(line=7, character=19), end=lsp.Position(line=7, character=23)), + message="Avoid returning null.", + severity=lsp.DiagnosticSeverity.Warning, + code="null-return", + source="java-functional-lsp", + ) + result = on_code_action( + lsp.CodeActionParams( + text_document=lsp.TextDocumentIdentifier(uri=uri), + range=diag.range, + context=lsp.CodeActionContext(diagnostics=[diag]), + ) + ) + finally: + server.workspace.remove_text_document(uri) + + assert result is not None + assert len(result) >= 1 + action = result[0] + assert action.kind == lsp.CodeActionKind.QuickFix + assert action.title == "Replace with Option.none()" + assert action.edit is not None + assert action.edit.changes is not None + edits = action.edit.changes[uri] + assert any("Option.none()" in e.new_text for e in edits) + assert any("import io.vavr.control.Option;" in e.new_text for e in edits) + + def test_try_catch_to_monadic_quickfix(self) -> None: + """try-catch-to-monadic diagnostic → QuickFix with Try.of() + auto-import.""" + from java_functional_lsp.server import on_code_action, server + + _ensure_workspace() + uri = "file:///test/TryCatch.java" + server.workspace.put_text_document( + lsp.TextDocumentItem(uri=uri, language_id="java", version=1, text=_TRY_CATCH_JAVA), + ) + try: + # Diagnostic on the `try` keyword (line 4, cols 8-11) + diag = lsp.Diagnostic( + range=lsp.Range(start=lsp.Position(line=4, character=8), end=lsp.Position(line=4, character=11)), + message="Imperative try/catch.", + severity=lsp.DiagnosticSeverity.Hint, + code="try-catch-to-monadic", + source="java-functional-lsp", + ) + result = on_code_action( + lsp.CodeActionParams( + text_document=lsp.TextDocumentIdentifier(uri=uri), + range=diag.range, + context=lsp.CodeActionContext(diagnostics=[diag]), + ) + ) + finally: + server.workspace.remove_text_document(uri) + + assert result is not None + assert len(result) >= 1 + action = result[0] + assert action.kind == lsp.CodeActionKind.QuickFix + assert action.title == "Convert try/catch to Try monadic flow" + assert action.edit is not None + assert action.edit.changes is not None + edits = action.edit.changes[uri] + assert any("Try.of(() -> riskyRead())" in e.new_text for e in edits) + assert any("import io.vavr.control.Try;" in e.new_text for e in edits) + + def test_ignores_non_java_functional_lsp_diagnostics(self) -> None: + """Diagnostics from other sources (jdtls) are filtered out.""" + from java_functional_lsp.server import on_code_action, server + + _ensure_workspace() + uri = "file:///test/BuggyExample.java" + server.workspace.put_text_document( + lsp.TextDocumentItem(uri=uri, language_id="java", version=1, text=_BUGGY_JAVA), + ) + try: + diag = lsp.Diagnostic( + range=lsp.Range(start=lsp.Position(line=7, character=19), end=lsp.Position(line=7, character=23)), + message="Some jdtls warning", + severity=lsp.DiagnosticSeverity.Warning, + code="something-jdtls-specific", + source="Java", + ) + result = on_code_action( + lsp.CodeActionParams( + text_document=lsp.TextDocumentIdentifier(uri=uri), + range=diag.range, + context=lsp.CodeActionContext(diagnostics=[diag]), + ) + ) + finally: + server.workspace.remove_text_document(uri) + + assert result is None + + def test_code_action_with_no_diagnostics(self) -> None: + """Empty diagnostic context → no code actions.""" + from java_functional_lsp.server import on_code_action, server + + _ensure_workspace() + uri = "file:///test/Clean.java" + server.workspace.put_text_document( + lsp.TextDocumentItem(uri=uri, language_id="java", version=1, text="class Clean {}"), + ) + try: + result = on_code_action( + lsp.CodeActionParams( + text_document=lsp.TextDocumentIdentifier(uri=uri), + range=lsp.Range(start=lsp.Position(line=0, character=0), end=lsp.Position(line=0, character=5)), + context=lsp.CodeActionContext(diagnostics=[]), + ) + ) + finally: + server.workspace.remove_text_document(uri) + + assert result is None From 5db1c95bb8bbf557fe5b726d25071a162dd9593b Mon Sep 17 00:00:00 2001 From: Aviad Shiber Date: Fri, 10 Apr 2026 07:30:06 +0300 Subject: [PATCH 09/14] test: mock-free LSP lifecycle tests via real subprocess + stdio transport MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rewrites test_server.py with two complementary test tiers: **TestServerInternals** (16 tests) — direct-call tests exercising server.py helpers in-process for coverage: _load_config, _to_lsp_diagnostic, _analyze_document (with/without excludes), _handle_exception, _jdtls_raw_to_lsp_diagnostics, _on_jdtls_diagnostics callback, _serialize_params camelCase guard, and on_code_action for null-return, try-catch-to-monadic, and foreign-diagnostic filtering. One mock remains for text_document_publish_diagnostics (transport boundary). **TestLspLifecycle** (9 tests) — ZERO mocks. Spawns the real java-functional-lsp server as a subprocess via pygls LanguageClient, connects over stdio pipes, and drives the full LSP lifecycle: initialize → didOpen → wait for publishDiagnostics → codeAction. Tests cover: - initialize handshake - null-return / null-check-to-monadic / try-catch-to-monadic diagnostic publishing - Clean file → zero diagnostics - null-return QuickFix → Option.none() + auto-import - try-catch-to-monadic QuickFix → Try.of() + auto-import - Foreign-diagnostic filtering (jdtls diagnostics → no actions) - didChange with clean source → diagnostics cleared The subprocess tests run in ~3.4s total (real server startup + 9 requests). Coverage is provided by the direct-call tier (subprocess coverage cannot be captured by pytest-cov in the parent process). Unit matrix: 324 passed, 80.01% coverage (meets 80% bar). Full suite with e2e: 331 passed, 83%+ coverage. Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/test_server.py | 469 +++++++++++++++++++++++++++++-------------- 1 file changed, 315 insertions(+), 154 deletions(-) diff --git a/tests/test_server.py b/tests/test_server.py index 7851223..8d3426b 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -1,19 +1,27 @@ -"""Integration tests for the LanguageServer — analyzer pipeline + code actions. - -These tests drive the real ``java_functional_lsp.server`` module through the -same code paths IntelliJ/VS Code hit: ``_run_analysis`` for diagnostics and -``on_code_action`` for QuickFix generation. They bootstrap a real pygls -workspace, inject documents, and assert on the results. - -They run in the regular unit matrix (no jdtls required, no ``@pytest.mark.e2e``). +"""Integration tests for the LanguageServer — mock-free via real LSP transport. + +These tests spawn the actual ``java-functional-lsp`` server as a subprocess, +connect via pygls ``LanguageClient`` over stdio pipes, and drive the full LSP +lifecycle: initialize → didOpen → publishDiagnostics → codeAction. No mocks, +no patching — the same bytes flow that a real IDE sends. + +This is the layer that catches regressions invisible to unit tests: +- camelCase serialization (v0.7.2 bug: vanilla cattrs → snake_case) +- transport framing (Content-Length, JSON encoding) +- server initialization + workspace wiring +- diagnostic publishing timing """ from __future__ import annotations +import asyncio +import os +import sys from typing import Any +import pytest from lsprotocol import types as lsp -from pygls.workspace import Workspace +from pygls.lsp.client import LanguageClient _BUGGY_JAVA = """\ import java.util.List; @@ -43,9 +51,115 @@ } """ +_CLEAN_JAVA = """\ +public class Clean { + public int add(int a, int b) { + return a + b; + } +} +""" + + +@pytest.fixture +async def lsp_client(tmp_path: Any) -> LanguageClient: # type: ignore[misc] + """Spawn the real java-functional-lsp server and return an initialized client. + + Uses pygls ``LanguageClient.start_io`` to connect via stdio — the exact + transport IntelliJ/VS Code use. The server process is killed on teardown. + """ + client = LanguageClient("test-client", "1.0") + + # Collect published diagnostics so tests can assert on them. + client._published: dict[str, list[lsp.Diagnostic]] = {} # type: ignore[attr-defined] + + @client.feature(lsp.TEXT_DOCUMENT_PUBLISH_DIAGNOSTICS) + def on_publish(params: lsp.PublishDiagnosticsParams) -> None: + client._published[params.uri] = list(params.diagnostics) # type: ignore[attr-defined] + + await client.start_io(sys.executable, "-m", "java_functional_lsp") + + result = await client.initialize_async( + lsp.InitializeParams( + process_id=os.getpid(), + root_uri=tmp_path.as_uri(), + root_path=str(tmp_path), + capabilities=lsp.ClientCapabilities( + text_document=lsp.TextDocumentClientCapabilities( + code_action=lsp.CodeActionClientCapabilities( + code_action_literal_support=lsp.ClientCodeActionLiteralOptions( + code_action_kind=lsp.ClientCodeActionKindOptions( + value_set=[lsp.CodeActionKind.QuickFix], + ), + ), + ), + publish_diagnostics=lsp.PublishDiagnosticsClientCapabilities(), + ), + ), + ) + ) + assert result.capabilities is not None + client.initialized(lsp.InitializedParams()) + + try: + yield client + finally: + try: + await client.shutdown_async(None) + client.exit(None) + except Exception: + pass + await client.stop() + + +async def _open_and_wait_for_diagnostics( + client: LanguageClient, + uri: str, + source: str, + *, + timeout: float = 10.0, +) -> list[lsp.Diagnostic]: + """Open a document and wait until publishDiagnostics arrives for its URI. + + The server publishes diagnostics asynchronously after didOpen. We poll + the client's collected notifications until the URI appears or timeout. + """ + client._published.pop(uri, None) # type: ignore[attr-defined] + + client.text_document_did_open( + lsp.DidOpenTextDocumentParams( + text_document=lsp.TextDocumentItem( + uri=uri, + language_id="java", + version=1, + text=source, + ), + ) + ) + + deadline = asyncio.get_event_loop().time() + timeout + while asyncio.get_event_loop().time() < deadline: + if uri in client._published: # type: ignore[attr-defined] + return client._published[uri] # type: ignore[attr-defined] + await asyncio.sleep(0.1) + + pytest.fail(f"Timed out waiting for publishDiagnostics on {uri}") + return [] # unreachable, but satisfies type checker + + +# -------------------------------------------------------------------------- +# Direct-call tests — exercise server internals for coverage +# -------------------------------------------------------------------------- +# +# The subprocess tests below (TestLspLifecycle) are the real e2e tests, but +# pytest-cov can only instrument the test process, not spawned subprocesses. +# These direct-call tests exercise the same server.py code paths in-process +# so the coverage counter credits them. + def _ensure_workspace() -> None: """Bootstrap a minimal pygls workspace if not already initialized.""" + from pygls.workspace import Workspace + from java_functional_lsp.server import server if server.protocol._workspace is None: @@ -55,35 +169,30 @@ def _ensure_workspace() -> None: ) -class TestServerHelpers: - """Tests for server.py internal helpers.""" +class TestServerInternals: + """Direct-call tests for server.py helpers — provides in-process coverage.""" - def test_load_config_returns_empty_for_no_workspace(self) -> None: + def test_load_config_no_workspace(self) -> None: from java_functional_lsp.server import _load_config assert _load_config(None) == {} - assert _load_config("") == {} - def test_load_config_returns_empty_for_missing_file(self, tmp_path: Any) -> None: + def test_load_config_missing_file(self, tmp_path: Any) -> None: from java_functional_lsp.server import _load_config assert _load_config(str(tmp_path)) == {} - def test_load_config_reads_json(self, tmp_path: Any) -> None: + def test_load_config_valid_json(self, tmp_path: Any) -> None: from java_functional_lsp.server import _load_config - config_file = tmp_path / ".java-functional-lsp.json" - config_file.write_text('{"rules": {"null-return": "off"}}') - result = _load_config(str(tmp_path)) - assert result == {"rules": {"null-return": "off"}} + (tmp_path / ".java-functional-lsp.json").write_text('{"rules": {"null-return": "off"}}') + assert _load_config(str(tmp_path)) == {"rules": {"null-return": "off"}} - def test_load_config_handles_invalid_json(self, tmp_path: Any) -> None: + def test_load_config_invalid_json(self, tmp_path: Any) -> None: from java_functional_lsp.server import _load_config - config_file = tmp_path / ".java-functional-lsp.json" - config_file.write_text("not valid json {{{") - result = _load_config(str(tmp_path)) - assert result == {} + (tmp_path / ".java-functional-lsp.json").write_text("not json {{{") + assert _load_config(str(tmp_path)) == {} def test_to_lsp_diagnostic_with_data(self) -> None: from java_functional_lsp.analyzers.base import Diagnostic as LintDiag @@ -96,9 +205,9 @@ def test_to_lsp_diagnostic_with_data(self) -> None: end_line=5, end_col=20, severity=Severity.HINT, - code="test-rule", - message="test message", - data=DiagnosticData(fix_type="FIX", target_library="lib", rationale="reason"), + code="test", + message="msg", + data=DiagnosticData(fix_type="FIX", target_library="lib", rationale="r"), ) result = _to_lsp_diagnostic(diag) assert result.severity == lsp.DiagnosticSeverity.Hint @@ -110,151 +219,100 @@ def test_to_lsp_diagnostic_without_data(self) -> None: from java_functional_lsp.analyzers.base import Severity from java_functional_lsp.server import _to_lsp_diagnostic - diag = LintDiag(line=0, col=0, end_line=0, end_col=5, severity=Severity.WARNING, code="x", message="msg") - result = _to_lsp_diagnostic(diag) - assert result.data is None + diag = LintDiag(line=0, col=0, end_line=0, end_col=5, severity=Severity.WARNING, code="x", message="m") + assert _to_lsp_diagnostic(diag).data is None def test_analyze_document_with_excludes(self) -> None: from java_functional_lsp.server import _analyze_document, server - old_config = server._config + old = server._config server._config = {"excludes": ["**/generated/**"]} try: - result = _analyze_document( - "class T { String f() { return null; } }", - "file:///project/src/main/generated/Foo.java", - ) - assert result == [] + assert _analyze_document("class T { String f() { return null; } }", "file:///generated/F.java") == [] finally: - server._config = old_config + server._config = old - def test_analyze_document_without_excludes(self) -> None: + def test_analyze_document_produces_diagnostics(self) -> None: from java_functional_lsp.server import _analyze_document - result = _analyze_document("class T { String f() { return null; } }", "file:///Foo.java") + result = _analyze_document("class T { String f() { return null; } }", "file:///F.java") assert any(d.code == "null-return" for d in result) - def test_serialize_params_camelcase(self) -> None: - """The LSP converter must emit camelCase field names.""" - from java_functional_lsp.server import _serialize_params - - params = lsp.DefinitionParams( - text_document=lsp.TextDocumentIdentifier(uri="file:///x.java"), - position=lsp.Position(line=0, character=0), - ) - result = _serialize_params(params) - assert "textDocument" in result - assert "text_document" not in result - def test_handle_exception_logs(self, caplog: Any) -> None: - """sys.excepthook is wired to _handle_exception for crash debugging.""" import logging from java_functional_lsp.server import _handle_exception with caplog.at_level(logging.ERROR, logger="java_functional_lsp.server"): - _handle_exception(ValueError, ValueError("test crash"), None) + _handle_exception(ValueError, ValueError("crash"), None) assert any("Uncaught exception" in r.getMessage() for r in caplog.records) - def test_jdtls_raw_to_lsp_diagnostics_valid(self) -> None: - """Raw jdtls diagnostic dicts should be structured into lsp.Diagnostic.""" + def test_jdtls_raw_to_lsp_diagnostics(self) -> None: from java_functional_lsp.server import _jdtls_raw_to_lsp_diagnostics raw = [ { "range": {"start": {"line": 1, "character": 0}, "end": {"line": 1, "character": 10}}, "severity": 2, - "code": "jdt.warning", + "code": "x", "source": "Java", - "message": "Unused import", + "message": "warn", } ] result = _jdtls_raw_to_lsp_diagnostics(raw) assert len(result) == 1 - assert result[0].message == "Unused import" - assert result[0].source == "Java" + assert result[0].message == "warn" def test_jdtls_raw_to_lsp_diagnostics_malformed(self) -> None: - """Completely broken raw diagnostics should not crash, just be skipped.""" from java_functional_lsp.server import _jdtls_raw_to_lsp_diagnostics - result = _jdtls_raw_to_lsp_diagnostics([42, None, "not a dict"]) - assert result == [] + assert _jdtls_raw_to_lsp_diagnostics([42, None, "bad"]) == [] def test_on_jdtls_diagnostics_callback(self) -> None: - """The server's _on_jdtls_diagnostics callback re-analyzes the document.""" from unittest.mock import patch from java_functional_lsp.server import server _ensure_workspace() - uri = "file:///test/Callback.java" + uri = "file:///test/Cb.java" server.workspace.put_text_document( lsp.TextDocumentItem( uri=uri, language_id="java", version=1, text="class T { String f() { return null; } }" ), ) try: - # Mock publish to avoid transport errors with patch.object(server, "text_document_publish_diagnostics") as mock_pub: server._on_jdtls_diagnostics(uri, []) mock_pub.assert_called_once() - published = mock_pub.call_args[0][0] - # Verify our custom analyzer found the null-return - codes = [d.code for d in published.diagnostics] + codes = [d.code for d in mock_pub.call_args[0][0].diagnostics] assert "null-return" in codes finally: server.workspace.remove_text_document(uri) + def test_serialize_params_camelcase(self) -> None: + from java_functional_lsp.server import _serialize_params -class TestAnalyzerPipeline: - """Verify the full analyzer chain produces the expected diagnostics.""" - - def test_null_return_diagnostic(self) -> None: - from java_functional_lsp.server import _run_analysis - - diags = _run_analysis(_BUGGY_JAVA, "file:///test/BuggyExample.java") - codes = [d.code for d in diags] - assert "null-return" in codes - - def test_null_check_to_monadic_diagnostic(self) -> None: - from java_functional_lsp.server import _run_analysis - - diags = _run_analysis(_BUGGY_JAVA, "file:///test/BuggyExample.java") - codes = [d.code for d in diags] - assert "null-check-to-monadic" in codes - - def test_try_catch_to_monadic_diagnostic(self) -> None: - from java_functional_lsp.server import _run_analysis - - diags = _run_analysis(_TRY_CATCH_JAVA, "file:///test/TryCatch.java") - codes = [d.code for d in diags] - assert "try-catch-to-monadic" in codes - - def test_no_diagnostics_on_clean_file(self) -> None: - from java_functional_lsp.server import _run_analysis - - clean = "public class Clean {\n public int add(int a, int b) {\n return a + b;\n }\n}\n" - diags = _run_analysis(clean, "file:///test/Clean.java") - assert len(diags) == 0 - - -class TestCodeActionPipeline: - """Verify the code-action handler produces valid WorkspaceEdits.""" + result = _serialize_params( + lsp.DefinitionParams( + text_document=lsp.TextDocumentIdentifier(uri="file:///x.java"), + position=lsp.Position(line=0, character=0), + ) + ) + assert "textDocument" in result + assert "text_document" not in result - def test_null_return_quickfix(self) -> None: - """null-return diagnostic → QuickFix with Option.none() + auto-import.""" + def test_code_action_null_return(self) -> None: from java_functional_lsp.server import on_code_action, server _ensure_workspace() - uri = "file:///test/BuggyExample.java" + uri = "file:///test/CA1.java" server.workspace.put_text_document( lsp.TextDocumentItem(uri=uri, language_id="java", version=1, text=_BUGGY_JAVA), ) try: diag = lsp.Diagnostic( range=lsp.Range(start=lsp.Position(line=7, character=19), end=lsp.Position(line=7, character=23)), - message="Avoid returning null.", + message="m", severity=lsp.DiagnosticSeverity.Warning, code="null-return", source="java-functional-lsp", @@ -268,32 +326,21 @@ def test_null_return_quickfix(self) -> None: ) finally: server.workspace.remove_text_document(uri) - assert result is not None - assert len(result) >= 1 - action = result[0] - assert action.kind == lsp.CodeActionKind.QuickFix - assert action.title == "Replace with Option.none()" - assert action.edit is not None - assert action.edit.changes is not None - edits = action.edit.changes[uri] - assert any("Option.none()" in e.new_text for e in edits) - assert any("import io.vavr.control.Option;" in e.new_text for e in edits) + assert result[0].title == "Replace with Option.none()" - def test_try_catch_to_monadic_quickfix(self) -> None: - """try-catch-to-monadic diagnostic → QuickFix with Try.of() + auto-import.""" + def test_code_action_try_catch(self) -> None: from java_functional_lsp.server import on_code_action, server _ensure_workspace() - uri = "file:///test/TryCatch.java" + uri = "file:///test/CA2.java" server.workspace.put_text_document( lsp.TextDocumentItem(uri=uri, language_id="java", version=1, text=_TRY_CATCH_JAVA), ) try: - # Diagnostic on the `try` keyword (line 4, cols 8-11) diag = lsp.Diagnostic( range=lsp.Range(start=lsp.Position(line=4, character=8), end=lsp.Position(line=4, character=11)), - message="Imperative try/catch.", + message="m", severity=lsp.DiagnosticSeverity.Hint, code="try-catch-to-monadic", source="java-functional-lsp", @@ -307,33 +354,23 @@ def test_try_catch_to_monadic_quickfix(self) -> None: ) finally: server.workspace.remove_text_document(uri) - assert result is not None - assert len(result) >= 1 - action = result[0] - assert action.kind == lsp.CodeActionKind.QuickFix - assert action.title == "Convert try/catch to Try monadic flow" - assert action.edit is not None - assert action.edit.changes is not None - edits = action.edit.changes[uri] - assert any("Try.of(() -> riskyRead())" in e.new_text for e in edits) - assert any("import io.vavr.control.Try;" in e.new_text for e in edits) + assert any("Try.of" in e.new_text for e in result[0].edit.changes[uri]) - def test_ignores_non_java_functional_lsp_diagnostics(self) -> None: - """Diagnostics from other sources (jdtls) are filtered out.""" + def test_code_action_filters_foreign(self) -> None: from java_functional_lsp.server import on_code_action, server _ensure_workspace() - uri = "file:///test/BuggyExample.java" + uri = "file:///test/CA3.java" server.workspace.put_text_document( lsp.TextDocumentItem(uri=uri, language_id="java", version=1, text=_BUGGY_JAVA), ) try: diag = lsp.Diagnostic( - range=lsp.Range(start=lsp.Position(line=7, character=19), end=lsp.Position(line=7, character=23)), - message="Some jdtls warning", + range=lsp.Range(start=lsp.Position(line=0, character=0), end=lsp.Position(line=0, character=5)), + message="x", severity=lsp.DiagnosticSeverity.Warning, - code="something-jdtls-specific", + code="jdtls-thing", source="Java", ) result = on_code_action( @@ -345,27 +382,151 @@ def test_ignores_non_java_functional_lsp_diagnostics(self) -> None: ) finally: server.workspace.remove_text_document(uri) - assert result is None - def test_code_action_with_no_diagnostics(self) -> None: - """Empty diagnostic context → no code actions.""" - from java_functional_lsp.server import on_code_action, server - _ensure_workspace() +# -------------------------------------------------------------------------- +# Subprocess-based tests — zero mocks, real LSP transport +# -------------------------------------------------------------------------- + + +@pytest.mark.timeout(30) +class TestLspLifecycle: + """Full LSP lifecycle tests via real stdio transport — zero mocks.""" + + async def test_initialize_reports_capabilities(self, lsp_client: LanguageClient) -> None: + """Server advertises codeActionProvider and textDocumentSync.""" + # The fixture already initialized — just verify the stored capabilities. + assert lsp_client is not None # fixture didn't fail + + async def test_null_return_diagnostic_published(self, lsp_client: LanguageClient) -> None: + """didOpen a file with ``return null`` → server publishes null-return diagnostic.""" + uri = "file:///test/BuggyExample.java" + diags = await _open_and_wait_for_diagnostics(lsp_client, uri, _BUGGY_JAVA) + codes = [d.code for d in diags] + assert "null-return" in codes + + async def test_null_check_to_monadic_diagnostic_published(self, lsp_client: LanguageClient) -> None: + """The if(x != null) pattern produces a null-check-to-monadic hint.""" + uri = "file:///test/BuggyExample2.java" + diags = await _open_and_wait_for_diagnostics(lsp_client, uri, _BUGGY_JAVA) + codes = [d.code for d in diags] + assert "null-check-to-monadic" in codes + + async def test_try_catch_to_monadic_diagnostic_published(self, lsp_client: LanguageClient) -> None: + """try/catch with single return produces a try-catch-to-monadic hint.""" + uri = "file:///test/TryCatch.java" + diags = await _open_and_wait_for_diagnostics(lsp_client, uri, _TRY_CATCH_JAVA) + codes = [d.code for d in diags] + assert "try-catch-to-monadic" in codes + + async def test_clean_file_produces_no_diagnostics(self, lsp_client: LanguageClient) -> None: + """A clean Java file should produce zero diagnostics.""" uri = "file:///test/Clean.java" - server.workspace.put_text_document( - lsp.TextDocumentItem(uri=uri, language_id="java", version=1, text="class Clean {}"), + diags = await _open_and_wait_for_diagnostics(lsp_client, uri, _CLEAN_JAVA) + assert len(diags) == 0 + + async def test_null_return_code_action_quickfix(self, lsp_client: LanguageClient) -> None: + """Request code action on null-return diagnostic → QuickFix with Option.none(). + + This is the full round-trip: didOpen → publishDiagnostics → codeAction + request with the real diagnostic → server returns a WorkspaceEdit. + """ + uri = "file:///test/BuggyAction.java" + diags = await _open_and_wait_for_diagnostics(lsp_client, uri, _BUGGY_JAVA) + null_diag = next((d for d in diags if d.code == "null-return"), None) + assert null_diag is not None + + actions = await lsp_client.text_document_code_action_async( + lsp.CodeActionParams( + text_document=lsp.TextDocumentIdentifier(uri=uri), + range=null_diag.range, + context=lsp.CodeActionContext(diagnostics=[null_diag]), + ) ) - try: - result = on_code_action( - lsp.CodeActionParams( - text_document=lsp.TextDocumentIdentifier(uri=uri), - range=lsp.Range(start=lsp.Position(line=0, character=0), end=lsp.Position(line=0, character=5)), - context=lsp.CodeActionContext(diagnostics=[]), - ) + + assert actions is not None + assert len(actions) >= 1 + action = actions[0] + assert action.title == "Replace with Option.none()" + assert action.kind == lsp.CodeActionKind.QuickFix + assert action.edit is not None + assert action.edit.changes is not None + edits = action.edit.changes[uri] + assert any("Option.none()" in e.new_text for e in edits) + assert any("import io.vavr.control.Option;" in e.new_text for e in edits) + + async def test_try_catch_code_action_quickfix(self, lsp_client: LanguageClient) -> None: + """Request code action on try-catch-to-monadic → QuickFix with Try.of().""" + uri = "file:///test/TryCatchAction.java" + diags = await _open_and_wait_for_diagnostics(lsp_client, uri, _TRY_CATCH_JAVA) + try_diag = next((d for d in diags if d.code == "try-catch-to-monadic"), None) + assert try_diag is not None + + actions = await lsp_client.text_document_code_action_async( + lsp.CodeActionParams( + text_document=lsp.TextDocumentIdentifier(uri=uri), + range=try_diag.range, + context=lsp.CodeActionContext(diagnostics=[try_diag]), ) - finally: - server.workspace.remove_text_document(uri) + ) - assert result is None + assert actions is not None + assert len(actions) >= 1 + action = actions[0] + assert action.title == "Convert try/catch to Try monadic flow" + assert action.edit is not None + assert action.edit.changes is not None + edits = action.edit.changes[uri] + assert any("Try.of(() -> riskyRead())" in e.new_text for e in edits) + assert any("import io.vavr.control.Try;" in e.new_text for e in edits) + + async def test_code_action_ignores_foreign_diagnostics(self, lsp_client: LanguageClient) -> None: + """Diagnostics from other sources get no code actions from our server.""" + uri = "file:///test/Foreign.java" + await _open_and_wait_for_diagnostics(lsp_client, uri, _BUGGY_JAVA) + + foreign_diag = lsp.Diagnostic( + range=lsp.Range(start=lsp.Position(line=0, character=0), end=lsp.Position(line=0, character=5)), + message="Some jdtls warning", + severity=lsp.DiagnosticSeverity.Warning, + code="something-jdtls", + source="Java", + ) + actions = await lsp_client.text_document_code_action_async( + lsp.CodeActionParams( + text_document=lsp.TextDocumentIdentifier(uri=uri), + range=foreign_diag.range, + context=lsp.CodeActionContext(diagnostics=[foreign_diag]), + ) + ) + assert actions is None or len(actions) == 0 + + async def test_diagnostics_update_on_file_change(self, lsp_client: LanguageClient) -> None: + """didChange with a fixed file should clear diagnostics. + + Opens a buggy file, verifies diagnostics arrive, then sends a + didChange with clean source and verifies diagnostics are cleared. + """ + uri = "file:///test/Changing.java" + diags = await _open_and_wait_for_diagnostics(lsp_client, uri, _BUGGY_JAVA) + assert len(diags) > 0 + + # Clear the notification cache and send a change to clean source. + lsp_client._published.pop(uri, None) # type: ignore[attr-defined] + lsp_client.text_document_did_change( + lsp.DidChangeTextDocumentParams( + text_document=lsp.VersionedTextDocumentIdentifier(uri=uri, version=2), + content_changes=[lsp.TextDocumentContentChangeWholeDocument(text=_CLEAN_JAVA)], + ) + ) + + # Wait for fresh diagnostics (debounced, ~150ms + processing). + deadline = asyncio.get_event_loop().time() + 5.0 + while asyncio.get_event_loop().time() < deadline: + if uri in lsp_client._published: # type: ignore[attr-defined] + break + await asyncio.sleep(0.1) + + new_diags = lsp_client._published.get(uri, diags) # type: ignore[attr-defined] + assert len(new_diags) == 0, f"Expected zero diagnostics after fixing, got {[d.code for d in new_diags]}" From d5e1c92edbf9038e6686750465e380e2f732b01b Mon Sep 17 00:00:00 2001 From: Aviad Shiber Date: Fri, 10 Apr 2026 07:36:54 +0300 Subject: [PATCH 10/14] docs: update README, CONTRIBUTING, SKILL for try-catch-to-monadic + test architecture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - README: add try-catch-to-monadic to rules table (15 → 16 rules) and code actions table (3 patterns documented). Update jdtls section to mention auto-detection of Java 21+ via JDTLS_JAVA_HOME / java_home. Update Spring-aware suppression list. - CONTRIBUTING: add Test Architecture section documenting the 4-tier test suite (unit, server internals, LSP lifecycle, jdtls e2e) and the 80% coverage threshold. Document _FIX_TITLES assertion guard. - SKILL: same rule table + code action updates as README, plus updated @Bean suppression list. Co-Authored-By: Claude Opus 4.6 (1M context) --- CONTRIBUTING.md | 13 ++++++++++++- README.md | 10 ++++++---- SKILL.md | 12 +++++++----- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9edaa27..81b2f8a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -48,9 +48,20 @@ uv run pytest 4. Add a `DiagnosticData` entry to the module's `_DATA` dict with `fix_type`, `target_library`, and `rationale` 5. Pass `data=_DATA["rule-id"]` when creating the `Diagnostic` 6. Add tests in `tests/test_.py` (including a test verifying the `data` field) -7. Optionally add a quick fix generator in `src/java_functional_lsp/fixes.py` and register it in `_FIX_REGISTRY` +7. Optionally add a quick fix generator in `src/java_functional_lsp/fixes.py` and register it in `_FIX_REGISTRY` + add its title to `_FIX_TITLES` in `server.py` (an import-time assertion catches mismatches) 8. Update the rules table in `README.md` +## Test Architecture + +The project has a layered test suite: + +- **Unit tests** (`tests/test_*_checker.py`, `tests/test_fixes.py`, `tests/test_proxy.py`) — fast, focused, run in the main CI matrix across Python 3.10-3.13 on Ubuntu + macOS +- **Server integration tests** (`tests/test_server.py: TestServerInternals`) — exercise the server pipeline (config loading, diagnostic conversion, code actions) in-process +- **LSP lifecycle tests** (`tests/test_server.py: TestLspLifecycle`) — **zero mocks** — spawn the real server as a subprocess via pygls `LanguageClient`, connect over stdio, exercise the full LSP round-trip (initialize, didOpen, publishDiagnostics, codeAction, didChange) +- **jdtls e2e tests** (`tests/test_e2e_jdtls.py`) — **zero mocks** — spawn real jdtls, exercise definition/references/hover/completion/documentSymbol forwarding. Auto-skip when jdtls is not installed. Run in a dedicated CI integration job. + +Coverage threshold is **80%**. Bump the version in both `pyproject.toml` and `src/java_functional_lsp/__init__.py` when making source changes (a pre-commit hook enforces this). + ## Reporting Issues - Use the [bug report template](https://github.com/aviadshiber/java-functional-lsp/issues/new?template=bug-report.md) diff --git a/README.md b/README.md index 5fa2aa3..0891e67 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ A Java Language Server that provides two things in one: 1. **Full Java language support** — completions, hover, go-to-definition, compile errors, missing imports — by proxying [Eclipse jdtls](https://github.com/eclipse-jdtls/eclipse.jdt.ls) under the hood -2. **15 functional programming rules** — catches anti-patterns and suggests Vavr/Lombok/Spring alternatives, all before compilation +2. **16 functional programming rules** — catches anti-patterns and suggests Vavr/Lombok/Spring alternatives, all before compilation 3. **Code actions (quick fixes)** — automated refactoring via LSP `textDocument/codeAction`, with machine-readable diagnostic metadata for AI agents Designed for teams using **Vavr**, **Lombok**, and **Spring** with a functional-first approach. @@ -24,7 +24,7 @@ When [jdtls](https://github.com/eclipse-jdtls/eclipse.jdt.ls) is installed, the - Type mismatches - Completions, hover, go-to-definition, find references -Install jdtls separately: `brew install jdtls` (requires JDK 21+). Without jdtls, the server runs in standalone mode — the 12 custom rules still work, but you won't get compile errors or completions. +Install jdtls separately: `brew install jdtls` (requires JDK 21+). The server auto-detects a Java 21+ installation even when the IDE's project SDK is older (e.g., Java 8) by probing `JDTLS_JAVA_HOME`, `JAVA_HOME`, `/usr/libexec/java_home -v 21+` (macOS), and `java` on PATH. Without jdtls, the server runs in standalone mode — the 16 custom rules still work, but you won't get compile errors or completions. ### Functional programming rules @@ -44,6 +44,7 @@ Install jdtls separately: `brew install jdtls` (requires JDK 21+). Without jdtls | `component-annotation` | `@Component`/`@Service`/`@Repository` | `@Configuration` + `@Bean` | — | | `frozen-mutation` | Mutation on `List.of()`/`Collections.unmodifiable*` | `io.vavr.collection.List` | ✅ | | `null-check-to-monadic` | `if (x != null) { return x.foo(); }` | `Option.of(x).map(...)` | ✅ | +| `try-catch-to-monadic` | `try { return x(); } catch (E e) { return d; }` | `Try.of(() -> x()).getOrElse(d)` | ✅ | | `impure-method` | Method mixing pure logic with side-effects | Extract pure logic; wrap IO in `Try` | — | ## Install @@ -203,7 +204,7 @@ Create `.java-functional-lsp.json` in your project root to customize rules: - `rules` — per-rule severity: `error`, `warning` (default), `info`, `hint`, `off` **Spring-aware behavior:** -- `throw-statement` and `catch-rethrow` are automatically suppressed inside `@Bean` methods +- `throw-statement`, `catch-rethrow`, and `try-catch-to-monadic` are automatically suppressed inside `@Bean` methods - `mutable-dto` suggests `@ConstructorBinding` instead of `@Value` when the class has `@ConfigurationProperties` **Inline suppression** with `@SuppressWarnings`: @@ -231,8 +232,9 @@ The server provides LSP code actions (`textDocument/codeAction`) that automatica | Rule | Code Action | What it does | |------|-------------|--------------| | `frozen-mutation` | Switch to Vavr Immutable Collection | Rewrites `List.of()` → `io.vavr.collection.List.of()`, `.add(x)` → `= list.append(x)`, adds import | -| `null-check-to-monadic` | Convert to Option monadic flow | Rewrites `if (x != null) { return x.foo(); }` → `Option.of(x).map(...).getOrNull()`, adds import | +| `null-check-to-monadic` | Convert to Option monadic flow | Rewrites `if (x != null) { return x.foo(); }` → `Option.of(x).map(...)`, supports chained fallbacks via `.orElse()`, adds import | | `null-return` | Replace with Option.none() | Rewrites `return null` → `return Option.none()`, adds import | +| `try-catch-to-monadic` | Convert try/catch to Try monadic flow | Rewrites `try { return expr; } catch (E e) { return default; }` → `Try.of(() -> expr).getOrElse(default)`. Supports 3 patterns: simple default (eager/lazy `.getOrElse`), logging + default (`.onFailure().getOrElse`), and exception-dependent recovery (`.recover(E.class, ...).get()`). Skips try-with-resources, finally, multi-catch, and union types. Adds import. | Quick fixes automatically add the required Vavr import if it's not already present. Disable auto-import with `"autoImportVavr": false` in config. diff --git a/SKILL.md b/SKILL.md index 879bdce..7190580 100644 --- a/SKILL.md +++ b/SKILL.md @@ -1,13 +1,13 @@ --- name: java-functional-lsp -description: Java LSP with full language support (completions, hover, go-to-def, compile errors) plus 15 functional programming rules with automated quick fixes. Auto-invoke when setting up Java language support or discussing Java linting configuration. +description: Java LSP with full language support (completions, hover, go-to-def, compile errors) plus 16 functional programming rules with automated quick fixes. Auto-invoke when setting up Java language support or discussing Java linting configuration. allowed-tools: Bash disable-model-invocation: true --- # Java Functional LSP -A Java LSP server that wraps jdtls and adds 15 functional programming rules with code actions (quick fixes). Gives you **full Java language support** (completions, hover, go-to-def, compile errors) **plus** custom diagnostics with machine-readable metadata for AI agents — all before compilation. +A Java LSP server that wraps jdtls and adds 16 functional programming rules with code actions (quick fixes). Gives you **full Java language support** (completions, hover, go-to-def, compile errors) **plus** custom diagnostics with machine-readable metadata for AI agents — all before compilation. ## Prerequisites @@ -22,7 +22,7 @@ brew install jdtls Without jdtls, the server runs in standalone mode — custom rules still work, but no completions/hover/compile errors. -## Rules (15 checks) +## Rules (16 checks) | Rule | Detects | Suggests | Quick Fix | |------|---------|----------|-----------| @@ -40,6 +40,7 @@ Without jdtls, the server runs in standalone mode — custom rules still work, b | `component-annotation` | `@Component`/`@Service`/`@Repository` | `@Configuration` + `@Bean` | — | | `frozen-mutation` | Mutation on `List.of()`/`Collections.unmodifiable*` | `io.vavr.collection.List` | ✅ | | `null-check-to-monadic` | `if (x != null) { return x.foo(); }` | `Option.of(x).map(...)` | ✅ | +| `try-catch-to-monadic` | `try { return x(); } catch (E e) { return d; }` | `Try.of(() -> x()).getOrElse(d)` | ✅ | | `impure-method` | Method mixing pure logic with side-effects | Extract pure logic; wrap IO in `Try` | — | ## Code Actions (Quick Fixes) @@ -47,8 +48,9 @@ Without jdtls, the server runs in standalone mode — custom rules still work, b Rules marked ✅ provide automated `textDocument/codeAction` fixes: - **frozen-mutation** → "Switch to Vavr Immutable Collection" — rewrites type, init, and mutation call to Vavr persistent API, adds import -- **null-check-to-monadic** → "Convert to Option monadic flow" — rewrites `if (x != null)` to `Option.of(x).map(...)`, adds import +- **null-check-to-monadic** → "Convert to Option monadic flow" — rewrites `if (x != null)` to `Option.of(x).map(...)`, supports chained fallbacks via `.orElse()`, adds import - **null-return** → "Replace with Option.none()" — replaces `null` with `Option.none()`, adds import +- **try-catch-to-monadic** → "Convert try/catch to Try monadic flow" — rewrites `try { return expr; } catch (E e) { return default; }` to `Try.of(() -> expr).getOrElse(default)`. Supports 3 patterns: simple default, logging + default (`.onFailure().getOrElse`), and exception-dependent recovery (`.recover(E.class, ...).get()`). Skips try-with-resources, finally, multi-catch, union types. Adds import. ## Agent-Ready Diagnostics @@ -85,7 +87,7 @@ Create `.java-functional-lsp.json` in your project root: - `rules` — per-rule severity: `error`, `warning` (default), `info`, `hint`, `off` - `autoImportVavr` — quick fixes auto-add Vavr imports (default: `true`) - `strictPurity` — `impure-method` uses WARNING instead of HINT (default: `false`) -- `throw-statement`/`catch-rethrow` auto-suppressed in `@Bean` methods +- `throw-statement`/`catch-rethrow`/`try-catch-to-monadic` auto-suppressed in `@Bean` methods - `mutable-dto` suggests `@ConstructorBinding` for `@ConfigurationProperties` classes - Inline suppression: `@SuppressWarnings("java-functional-lsp:rule-id")` on any declaration From d52197902c226f34d93e64f5f4b04b6b38d45ddd Mon Sep 17 00:00:00 2001 From: Aviad Shiber Date: Fri, 10 Apr 2026 07:58:35 +0300 Subject: [PATCH 11/14] =?UTF-8?q?fix:=20address=20review=20ensemble=20find?= =?UTF-8?q?ings=20=E2=80=94=20heredoc,=20test=20correctness,=20deprecation?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix heredoc indentation in CI (SHELL_EOF was indented, bash requires col 0) Replaced with printf to avoid heredoc pitfalls entirely - Replace false-confidence test_initialize_reports_capabilities with real assertions on code_action_provider and text_document_sync - Fix deprecated asyncio.get_event_loop() → get_running_loop() (Python 3.10+) - Add explicit timeout failure instead of silent .get() fallback in test_diagnostics_update_on_file_change - Log proxy cleanup errors instead of silently swallowing in e2e fixture - Assert required fields survive None-field pruning in test_none_fields_are_omitted Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/test.yml | 5 +---- tests/test_e2e_jdtls.py | 5 ++++- tests/test_proxy.py | 4 ++++ tests/test_server.py | 19 ++++++++++++------- 4 files changed, 21 insertions(+), 12 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b231883..29964f2 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -90,10 +90,7 @@ jobs: tar -xzf /tmp/jdtls.tar.gz -C "$JDTLS_DIR" # Wrapper script on PATH that invokes the bundled Python launcher # with python3 (the tarball ships jdtls.py in bin/). - cat > "$BIN_DIR/jdtls" << SHELL_EOF - #!/bin/bash - exec python3 "$JDTLS_DIR/bin/jdtls" "\$@" - SHELL_EOF + printf '#!/bin/bash\nexec python3 "%s/bin/jdtls" "$@"\n' "$JDTLS_DIR" > "$BIN_DIR/jdtls" chmod +x "$BIN_DIR/jdtls" echo "$BIN_DIR" >> "$GITHUB_PATH" - name: Install uv diff --git a/tests/test_e2e_jdtls.py b/tests/test_e2e_jdtls.py index 88e593e..8f8d633 100644 --- a/tests/test_e2e_jdtls.py +++ b/tests/test_e2e_jdtls.py @@ -153,7 +153,10 @@ async def proxy(workspace: tuple[Path, Path]) -> AsyncIterator[JdtlsProxy]: try: yield p finally: - await p.stop() + try: + await p.stop() + except Exception: + logging.getLogger(__name__).warning("proxy.stop() failed during teardown", exc_info=True) async def _open_document(proxy: JdtlsProxy, src_file: Path) -> str: diff --git a/tests/test_proxy.py b/tests/test_proxy.py index 9c96b01..446245b 100644 --- a/tests/test_proxy.py +++ b/tests/test_proxy.py @@ -393,6 +393,10 @@ def test_none_fields_are_omitted(self) -> None: partial_result_token=None, ) result = _serialize_params(params) + # Required fields must survive pruning + assert "textDocument" in result + assert "position" in result + # Optional None fields must be omitted assert "workDoneToken" not in result assert "partialResultToken" not in result # Also the snake_case equivalents should not appear diff --git a/tests/test_server.py b/tests/test_server.py index 8d3426b..cca9071 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -98,6 +98,7 @@ def on_publish(params: lsp.PublishDiagnosticsParams) -> None: ) ) assert result.capabilities is not None + client._server_capabilities = result.capabilities # type: ignore[attr-defined] client.initialized(lsp.InitializedParams()) try: @@ -136,8 +137,8 @@ async def _open_and_wait_for_diagnostics( ) ) - deadline = asyncio.get_event_loop().time() + timeout - while asyncio.get_event_loop().time() < deadline: + deadline = asyncio.get_running_loop().time() + timeout + while asyncio.get_running_loop().time() < deadline: if uri in client._published: # type: ignore[attr-defined] return client._published[uri] # type: ignore[attr-defined] await asyncio.sleep(0.1) @@ -396,8 +397,10 @@ class TestLspLifecycle: async def test_initialize_reports_capabilities(self, lsp_client: LanguageClient) -> None: """Server advertises codeActionProvider and textDocumentSync.""" - # The fixture already initialized — just verify the stored capabilities. - assert lsp_client is not None # fixture didn't fail + caps = lsp_client._server_capabilities # type: ignore[attr-defined] + assert caps is not None + assert caps.code_action_provider is not None + assert caps.text_document_sync is not None async def test_null_return_diagnostic_published(self, lsp_client: LanguageClient) -> None: """didOpen a file with ``return null`` → server publishes null-return diagnostic.""" @@ -522,11 +525,13 @@ async def test_diagnostics_update_on_file_change(self, lsp_client: LanguageClien ) # Wait for fresh diagnostics (debounced, ~150ms + processing). - deadline = asyncio.get_event_loop().time() + 5.0 - while asyncio.get_event_loop().time() < deadline: + loop = asyncio.get_running_loop() + deadline = loop.time() + 5.0 + while loop.time() < deadline: if uri in lsp_client._published: # type: ignore[attr-defined] break await asyncio.sleep(0.1) - new_diags = lsp_client._published.get(uri, diags) # type: ignore[attr-defined] + assert uri in lsp_client._published, "Timed out waiting for updated diagnostics after didChange" # type: ignore[attr-defined] + new_diags = lsp_client._published[uri] # type: ignore[attr-defined] assert len(new_diags) == 0, f"Expected zero diagnostics after fixing, got {[d.code for d in new_diags]}" From 5715e1801e64eae25b65a1fc89eb9785682a10cf Mon Sep 17 00:00:00 2001 From: Aviad Shiber Date: Fri, 10 Apr 2026 08:04:28 +0300 Subject: [PATCH 12/14] =?UTF-8?q?perf:=20class-scoped=20e2e=20fixtures=20?= =?UTF-8?q?=E2=80=94=207=E2=86=921=20jdtls=20cold-starts=20(70s=E2=86=9210?= =?UTF-8?q?s)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Promote proxy and workspace fixtures to scope="class" with loop_scope="class" so all TestJdtlsEndToEnd tests share a single jdtls subprocess and event loop. Open the document once in fixture setup instead of per-test. The loop_scope parameter is required because pytest-asyncio creates a new event loop per test by default — without it, the asyncio subprocess handles from proxy setup break in subsequent tests. Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/test_e2e_jdtls.py | 83 +++++++++++++++++++++++++---------------- 1 file changed, 50 insertions(+), 33 deletions(-) diff --git a/tests/test_e2e_jdtls.py b/tests/test_e2e_jdtls.py index 8f8d633..da20761 100644 --- a/tests/test_e2e_jdtls.py +++ b/tests/test_e2e_jdtls.py @@ -39,6 +39,7 @@ from pathlib import Path import pytest +import pytest_asyncio from lsprotocol import types as lsp from java_functional_lsp.proxy import JdtlsProxy, find_jdtls_java_home @@ -88,27 +89,37 @@ """ -@pytest.fixture -def workspace(tmp_path: Path) -> tuple[Path, Path]: +@pytest.fixture(scope="class") +def workspace(tmp_path_factory: pytest.TempPathFactory) -> tuple[Path, Path]: """Create a minimal standalone Java workspace with a single Hello.java file. + Class-scoped so the same workspace is reused across all tests in + TestJdtlsEndToEnd, avoiding redundant tmp_path creation per test. + jdtls operates in "default project" mode for orphan files (no build config), which is enough to parse the file and serve document symbols. No pom.xml, no build.gradle, no .classpath — we don't need full classpath resolution to verify that the request shape reaches jdtls correctly. """ + tmp_path = tmp_path_factory.mktemp("jdtls_workspace") src_file = tmp_path / "Hello.java" src_file.write_text(_HELLO_JAVA) return tmp_path, src_file -@pytest.fixture +@pytest_asyncio.fixture(scope="class", loop_scope="class") async def proxy(workspace: tuple[Path, Path]) -> AsyncIterator[JdtlsProxy]: """Start a real JdtlsProxy bound to the workspace fixture. - Yields an initialized proxy and tears it down on test exit. Uses a minimal - ``InitializeParams`` dict that includes only the capabilities we exercise - in the tests below, to keep jdtls's initial workspace scan cheap. + Class-scoped with ``loop_scope="class"`` so all tests in the class share + a single jdtls subprocess AND a single event loop. This cuts e2e wall-clock + time by ~6x (1 cold-start instead of 7). Without ``loop_scope``, + pytest-asyncio creates a new event loop per test, breaking the asyncio + subprocess handles created during proxy setup. + + Yields an initialized proxy and tears it down on class exit. Uses a + minimal ``InitializeParams`` dict that includes only the capabilities + we exercise in the tests below, to keep jdtls's initial workspace scan cheap. """ tmp_path, _ = workspace p = JdtlsProxy() @@ -150,19 +161,10 @@ async def proxy(workspace: tuple[Path, Path]) -> AsyncIterator[JdtlsProxy]: "Check the logs above for Java version / classpath issues." ) - try: - yield p - finally: - try: - await p.stop() - except Exception: - logging.getLogger(__name__).warning("proxy.stop() failed during teardown", exc_info=True) - - -async def _open_document(proxy: JdtlsProxy, src_file: Path) -> str: - """Send textDocument/didOpen for ``src_file`` and return its URI.""" + # Open the workspace file once so all tests can use it immediately. + _, src_file = workspace uri = src_file.as_uri() - await proxy.send_notification( + await p.send_notification( "textDocument/didOpen", { "textDocument": { @@ -173,10 +175,24 @@ async def _open_document(proxy: JdtlsProxy, src_file: Path) -> str: } }, ) - # Give jdtls a moment to parse the file. We can't synchronously wait for - # parsing — LSP exposes no notification for that — so we sleep conservatively. await asyncio.sleep(_JDTLS_PARSE_WAIT_SEC) - return uri + + try: + yield p + finally: + try: + await p.stop() + except Exception: + logging.getLogger(__name__).warning("proxy.stop() failed during teardown", exc_info=True) + + +def _document_uri(src_file: Path) -> str: + """Return the URI for the workspace document. + + With class-scoped fixtures the document is opened once in the proxy fixture + setup, so individual tests just need the URI. + """ + return src_file.as_uri() def _assert_no_npe_in_logs(caplog: pytest.LogCaptureFixture) -> None: @@ -196,6 +212,7 @@ def _assert_no_npe_in_logs(caplog: pytest.LogCaptureFixture) -> None: @pytest.mark.timeout(_E2E_TEST_TIMEOUT_SEC) +@pytest.mark.asyncio(loop_scope="class") class TestJdtlsEndToEnd: """End-to-end request/response tests against a real jdtls subprocess.""" @@ -228,7 +245,7 @@ async def test_document_symbol_round_trip( returns an error response, which we detect via caplog. """ _, src_file = workspace - uri = await _open_document(proxy, src_file) + uri = _document_uri(src_file) with caplog.at_level(logging.ERROR, logger="java_functional_lsp.proxy"): params = lsp.DocumentSymbolParams( @@ -278,7 +295,7 @@ async def test_definition_request_does_not_npe( jdtls actually resolves the symbol (which depends on classpath state). """ _, src_file = workspace - uri = await _open_document(proxy, src_file) + uri = _document_uri(src_file) with caplog.at_level(logging.ERROR, logger="java_functional_lsp.proxy"): # Position the cursor on the `greeting` identifier at line 8 col 20 @@ -312,7 +329,7 @@ async def test_hover_request_does_not_npe( might only affect a subset of position-based handlers. """ _, src_file = workspace - uri = await _open_document(proxy, src_file) + uri = _document_uri(src_file) with caplog.at_level(logging.ERROR, logger="java_functional_lsp.proxy"): params = lsp.HoverParams( @@ -328,7 +345,6 @@ async def test_hover_request_does_not_npe( async def test_did_open_notification_does_not_npe( self, - workspace: tuple[Path, Path], proxy: JdtlsProxy, caplog: pytest.LogCaptureFixture, ) -> None: @@ -339,17 +355,18 @@ async def test_did_open_notification_does_not_npe( here would cause jdtls to fail parsing the notification silently (no response, so no request-side error) but the NPE would appear in stderr when jdtls tries to look up the file by URI later. - """ - _, src_file = workspace - uri = src_file.as_uri() + This test verifies the serialization shape only — the proxy fixture + already opened Hello.java, so we use a synthetic URI to avoid + interfering with other tests. + """ with caplog.at_level(logging.ERROR, logger="java_functional_lsp.proxy"): params = lsp.DidOpenTextDocumentParams( text_document=lsp.TextDocumentItem( - uri=uri, + uri="file:///tmp/DidOpenTest.java", language_id="java", version=1, - text=src_file.read_text(), + text="public class DidOpenTest {}", ), ) serialized = _serialize_params(params) @@ -358,7 +375,7 @@ async def test_did_open_notification_does_not_npe( assert "language_id" not in serialized["textDocument"] await proxy.send_notification("textDocument/didOpen", serialized) - await asyncio.sleep(_JDTLS_PARSE_WAIT_SEC) + await asyncio.sleep(0.5) _assert_no_npe_in_logs(caplog) @@ -377,7 +394,7 @@ async def test_references_request_does_not_npe( ReferenceContext — guaranteed NPE. """ _, src_file = workspace - uri = await _open_document(proxy, src_file) + uri = _document_uri(src_file) with caplog.at_level(logging.ERROR, logger="java_functional_lsp.proxy"): params = lsp.ReferenceParams( @@ -411,7 +428,7 @@ async def test_completion_request_does_not_npe( request-shape bug here would NPE the completion handler. """ _, src_file = workspace - uri = await _open_document(proxy, src_file) + uri = _document_uri(src_file) with caplog.at_level(logging.ERROR, logger="java_functional_lsp.proxy"): # Position after `h.` on the main() line where completion makes sense. From 53c66d2d3a8ff3da22ea47a50424575ad93f4503 Mon Sep 17 00:00:00 2001 From: Aviad Shiber Date: Fri, 10 Apr 2026 08:05:56 +0300 Subject: [PATCH 13/14] =?UTF-8?q?chore:=20upgrade=20CI=20actions=20?= =?UTF-8?q?=E2=80=94=20setup-uv=20v7=E2=86=92v8,=20setup-java=20v4?= =?UTF-8?q?=E2=86=92v5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/test.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 29964f2..d39f6ac 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -21,7 +21,7 @@ jobs: with: python-version: ${{ matrix.python-version }} - name: Install uv - uses: astral-sh/setup-uv@v7 + uses: astral-sh/setup-uv@v8 - name: Install dependencies run: uv sync - name: Lint @@ -54,7 +54,7 @@ jobs: with: python-version: "3.12" - name: Install Java 21 - uses: actions/setup-java@v4 + uses: actions/setup-java@v5 with: distribution: temurin java-version: "21" @@ -94,7 +94,7 @@ jobs: chmod +x "$BIN_DIR/jdtls" echo "$BIN_DIR" >> "$GITHUB_PATH" - name: Install uv - uses: astral-sh/setup-uv@v7 + uses: astral-sh/setup-uv@v8 - name: Install dependencies run: uv sync - name: Verify jdtls is available From e8fea459eaba24844388b5b8dd83bb1ab2779ab8 Mon Sep 17 00:00:00 2001 From: Aviad Shiber Date: Fri, 10 Apr 2026 08:39:14 +0300 Subject: [PATCH 14/14] =?UTF-8?q?fix:=20revert=20setup-uv=20to=20v7=20?= =?UTF-8?q?=E2=80=94=20v8=20floating=20tag=20doesn't=20exist=20yet?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d39f6ac..a621786 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -21,7 +21,7 @@ jobs: with: python-version: ${{ matrix.python-version }} - name: Install uv - uses: astral-sh/setup-uv@v8 + uses: astral-sh/setup-uv@v7 - name: Install dependencies run: uv sync - name: Lint @@ -94,7 +94,7 @@ jobs: chmod +x "$BIN_DIR/jdtls" echo "$BIN_DIR" >> "$GITHUB_PATH" - name: Install uv - uses: astral-sh/setup-uv@v8 + uses: astral-sh/setup-uv@v7 - name: Install dependencies run: uv sync - name: Verify jdtls is available