From 018886af121f696e8eae6b1f875b0f86dcd8f475 Mon Sep 17 00:00:00 2001 From: Aviad Shiber Date: Fri, 10 Apr 2026 09:22:47 +0300 Subject: [PATCH 1/2] fix: dynamically register jdtls capabilities to avoid blocking diagnostics Previously, hover/definition/references/completion/documentSymbol were advertised in the static InitializeResult even before jdtls started. This caused the IDE to defer hover to us, and when jdtls wasn't ready (or returned null), diagnostic tooltips were suppressed. Now these capabilities are only registered via client/registerCapability AFTER jdtls initializes successfully. The handlers are also registered at that point via server.feature() instead of module-level decorators, since pygls auto-advertises capabilities for decorated handlers. Result: diagnostic tooltips show immediately (our custom rules), and jdtls features activate only when jdtls is actually ready. 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 | 81 +++++++++++++++++++++++------ tests/test_server.py | 67 +++++++++++++++++++++++- 4 files changed, 134 insertions(+), 18 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 32ea578..ce82353 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "java-functional-lsp" -version = "0.7.2" +version = "0.7.3" 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 71a66c6..d4fd9aa 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.2" +__version__ = "0.7.3" diff --git a/src/java_functional_lsp/server.py b/src/java_functional_lsp/server.py index 266e450..fc37c71 100644 --- a/src/java_functional_lsp/server.py +++ b/src/java_functional_lsp/server.py @@ -228,11 +228,11 @@ def on_initialize(params: lsp.InitializeParams) -> lsp.InitializeResult: change=lsp.TextDocumentSyncKind.Full, save=lsp.SaveOptions(include_text=True), ), - completion_provider=lsp.CompletionOptions(trigger_characters=["."]), - hover_provider=True, - definition_provider=True, - references_provider=True, - document_symbol_provider=True, + # Only advertise capabilities we own (custom diagnostics + code actions). + # jdtls-dependent features (hover, definition, references, completion, + # documentSymbol) are registered dynamically after jdtls starts — see + # on_initialized(). This prevents us from claiming hover when jdtls + # isn't ready, which would suppress the IDE's diagnostic tooltips. code_action_provider=lsp.CodeActionOptions( code_action_kinds=[lsp.CodeActionKind.QuickFix], ), @@ -250,10 +250,59 @@ async def on_initialized(params: lsp.InitializedParams) -> None: started = await server._proxy.start(server._init_params) if started: logger.info("jdtls proxy active — full Java language support enabled") + await _register_jdtls_capabilities() else: logger.info("jdtls proxy unavailable — running with custom rules only") +_JAVA_SELECTOR = [lsp.TextDocumentFilterLanguage(language="java")] + +# jdtls-dependent capabilities registered dynamically after the proxy starts. +# Each entry: (id_suffix, LSP method, registration options class, extra kwargs). +_JDTLS_CAPABILITIES: list[tuple[str, str, type[Any], dict[str, Any]]] = [ + ("completion", lsp.TEXT_DOCUMENT_COMPLETION, lsp.CompletionRegistrationOptions, {"trigger_characters": ["."]}), + ("hover", lsp.TEXT_DOCUMENT_HOVER, lsp.HoverRegistrationOptions, {}), + ("definition", lsp.TEXT_DOCUMENT_DEFINITION, lsp.DefinitionRegistrationOptions, {}), + ("references", lsp.TEXT_DOCUMENT_REFERENCES, lsp.ReferenceRegistrationOptions, {}), + ("document-symbol", lsp.TEXT_DOCUMENT_DOCUMENT_SYMBOL, lsp.DocumentSymbolRegistrationOptions, {}), +] + + +def _build_jdtls_registrations() -> list[lsp.Registration]: + """Build LSP Registration objects for jdtls-dependent capabilities.""" + return [ + lsp.Registration( + id=f"jdtls-{suffix}", + method=method, + register_options=_converter.unstructure(opts_cls(document_selector=_JAVA_SELECTOR, **extra)), + ) + for suffix, method, opts_cls, extra in _JDTLS_CAPABILITIES + ] + + +async def _register_jdtls_capabilities() -> None: + """Dynamically register jdtls-dependent capabilities after the proxy starts. + + We don't advertise these in the static InitializeResult because doing so + would make the IDE defer hover/definition/etc to us even before jdtls is + ready, which suppresses the IDE's built-in diagnostic tooltips. + """ + # Register the request handlers so pygls dispatches incoming requests to them. + server.feature(lsp.TEXT_DOCUMENT_COMPLETION)(_on_completion) + server.feature(lsp.TEXT_DOCUMENT_HOVER)(_on_hover) + server.feature(lsp.TEXT_DOCUMENT_DEFINITION)(_on_definition) + server.feature(lsp.TEXT_DOCUMENT_REFERENCES)(_on_references) + server.feature(lsp.TEXT_DOCUMENT_DOCUMENT_SYMBOL)(_on_document_symbol) + + # Tell the client we now support these capabilities. + registrations = _build_jdtls_registrations() + try: + await server.client_register_capability_async(lsp.RegistrationParams(registrations=registrations)) + logger.info("Dynamically registered jdtls capabilities (hover, definition, references, completion, symbol)") + except Exception as e: + logger.warning("Failed to dynamically register jdtls capabilities: %s", e) + + # --- Document sync (forward to jdtls + run custom analyzers) --- @@ -314,8 +363,14 @@ async def on_did_close(params: lsp.DidCloseTextDocumentParams) -> None: # --- Forwarded features (jdtls passthrough) --- -@server.feature(lsp.TEXT_DOCUMENT_COMPLETION) -async def on_completion(params: lsp.CompletionParams) -> lsp.CompletionList | None: +# --- jdtls passthrough handlers (registered dynamically, NOT at module level) --- +# These are NOT decorated with @server.feature because pygls auto-advertises +# capabilities for decorated handlers. We register them manually via +# server.lsp.fm.feature() inside _register_jdtls_capabilities() so they only +# activate after jdtls starts. + + +async def _on_completion(params: lsp.CompletionParams) -> lsp.CompletionList | None: """Forward completion request to jdtls.""" if not server._proxy.is_available: return None @@ -328,8 +383,7 @@ async def on_completion(params: lsp.CompletionParams) -> lsp.CompletionList | No return None -@server.feature(lsp.TEXT_DOCUMENT_HOVER) -async def on_hover(params: lsp.HoverParams) -> lsp.Hover | None: +async def _on_hover(params: lsp.HoverParams) -> lsp.Hover | None: """Forward hover request to jdtls.""" if not server._proxy.is_available: return None @@ -342,8 +396,7 @@ async def on_hover(params: lsp.HoverParams) -> lsp.Hover | None: return None -@server.feature(lsp.TEXT_DOCUMENT_DEFINITION) -async def on_definition(params: lsp.DefinitionParams) -> list[lsp.Location] | None: +async def _on_definition(params: lsp.DefinitionParams) -> list[lsp.Location] | None: """Forward go-to-definition request to jdtls.""" if not server._proxy.is_available: return None @@ -358,8 +411,7 @@ async def on_definition(params: lsp.DefinitionParams) -> list[lsp.Location] | No return None -@server.feature(lsp.TEXT_DOCUMENT_REFERENCES) -async def on_references(params: lsp.ReferenceParams) -> list[lsp.Location] | None: +async def _on_references(params: lsp.ReferenceParams) -> list[lsp.Location] | None: """Forward find-references request to jdtls.""" if not server._proxy.is_available: return None @@ -372,8 +424,7 @@ async def on_references(params: lsp.ReferenceParams) -> list[lsp.Location] | Non return None -@server.feature(lsp.TEXT_DOCUMENT_DOCUMENT_SYMBOL) -async def on_document_symbol(params: lsp.DocumentSymbolParams) -> list[lsp.DocumentSymbol] | None: +async def _on_document_symbol(params: lsp.DocumentSymbolParams) -> list[lsp.DocumentSymbol] | None: """Forward document symbol request to jdtls.""" if not server._proxy.is_available: return None diff --git a/tests/test_server.py b/tests/test_server.py index cca9071..a7547da 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -290,6 +290,62 @@ def test_on_jdtls_diagnostics_callback(self) -> None: finally: server.workspace.remove_text_document(uri) + def test_init_capabilities_exclude_jdtls_features(self) -> None: + """Static capabilities must NOT include hover/definition/references/completion/documentSymbol. + + These are registered dynamically after jdtls starts, so the IDE doesn't + suppress diagnostic tooltips while jdtls is unavailable. + """ + from java_functional_lsp.server import on_initialize + + result = on_initialize( + lsp.InitializeParams( + process_id=1, + root_uri="file:///tmp", + capabilities=lsp.ClientCapabilities(), + ) + ) + caps = result.capabilities + assert caps.code_action_provider is not None + assert caps.text_document_sync is not None + assert caps.hover_provider is None + assert caps.definition_provider is None + assert caps.references_provider is None + assert caps.completion_provider is None + assert caps.document_symbol_provider is None + + def test_build_jdtls_registrations(self) -> None: + from java_functional_lsp.server import _build_jdtls_registrations + + regs = _build_jdtls_registrations() + assert len(regs) == 5 + methods = {r.method for r in regs} + assert lsp.TEXT_DOCUMENT_HOVER in methods + assert lsp.TEXT_DOCUMENT_DEFINITION in methods + assert lsp.TEXT_DOCUMENT_REFERENCES in methods + assert lsp.TEXT_DOCUMENT_COMPLETION in methods + assert lsp.TEXT_DOCUMENT_DOCUMENT_SYMBOL in methods + # All have java document selector + for r in regs: + assert r.register_options is not None + assert "documentSelector" in r.register_options + + async def test_register_jdtls_capabilities_logs_on_failure(self, caplog: Any) -> None: + """_register_jdtls_capabilities logs a warning when the client rejects.""" + import logging + from unittest.mock import AsyncMock, patch + + from java_functional_lsp.server import _register_jdtls_capabilities + from java_functional_lsp.server import server as srv + + mock = AsyncMock(side_effect=Exception("no")) + with ( + caplog.at_level(logging.WARNING, logger="java_functional_lsp.server"), + patch.object(srv, "client_register_capability_async", mock), + ): + await _register_jdtls_capabilities() + assert any("Failed to dynamically register" in r.getMessage() for r in caplog.records) + def test_serialize_params_camelcase(self) -> None: from java_functional_lsp.server import _serialize_params @@ -396,11 +452,20 @@ 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.""" + """Server advertises codeActionProvider and textDocumentSync but NOT jdtls features. + + jdtls-dependent capabilities (hover, definition, references, completion, + documentSymbol) are registered dynamically after jdtls starts, so they + should NOT appear in the static InitializeResult. + """ 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 + # jdtls features are NOT statically advertised (registered dynamically) + assert caps.hover_provider is None + assert caps.definition_provider is None + assert caps.references_provider is None async def test_null_return_diagnostic_published(self, lsp_client: LanguageClient) -> None: """didOpen a file with ``return null`` → server publishes null-return diagnostic.""" From fe9527bf721cf733cd60450c85fbea3e55edfe24 Mon Sep 17 00:00:00 2001 From: Aviad Shiber Date: Fri, 10 Apr 2026 10:03:10 +0300 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20address=20review=20findings=20?= =?UTF-8?q?=E2=80=94=20idempotency=20guard,=20test=20isolation,=20assertio?= =?UTF-8?q?ns?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add _jdtls_capabilities_registered flag to prevent FeatureAlreadyRegisteredError on double invocation (critical finding) - Move server.feature() calls inside try/except to prevent capability mismatch when client_register_capability_async fails (high finding) - Use _JDTLS_HANDLERS dict to collect handlers, iterated during registration - Extract _JDTLS_REG_PREFIX constant for registration IDs - Fix test_register_jdtls_capabilities_logs_on_failure: patch server.feature and reset _jdtls_capabilities_registered to avoid singleton mutation - Add test_register_jdtls_capabilities_happy_path: verifies handlers are registered and success log is emitted - Add test_register_jdtls_capabilities_idempotent: verifies second call is no-op - Strengthen test_build_jdtls_registrations: assert java language value, triggerCharacters for completion, id uniqueness and prefix - Add completion_provider/document_symbol_provider None assertions to integration test Co-Authored-By: Claude Opus 4.6 (1M context) --- src/java_functional_lsp/server.py | 58 +++++++++++++------ tests/test_server.py | 92 +++++++++++++++++++++++++++---- uv.lock | 2 +- 3 files changed, 122 insertions(+), 30 deletions(-) diff --git a/src/java_functional_lsp/server.py b/src/java_functional_lsp/server.py index fc37c71..648535b 100644 --- a/src/java_functional_lsp/server.py +++ b/src/java_functional_lsp/server.py @@ -257,6 +257,8 @@ async def on_initialized(params: lsp.InitializedParams) -> None: _JAVA_SELECTOR = [lsp.TextDocumentFilterLanguage(language="java")] +_JDTLS_REG_PREFIX = "jdtls-" + # jdtls-dependent capabilities registered dynamically after the proxy starts. # Each entry: (id_suffix, LSP method, registration options class, extra kwargs). _JDTLS_CAPABILITIES: list[tuple[str, str, type[Any], dict[str, Any]]] = [ @@ -267,12 +269,18 @@ async def on_initialized(params: lsp.InitializedParams) -> None: ("document-symbol", lsp.TEXT_DOCUMENT_DOCUMENT_SYMBOL, lsp.DocumentSymbolRegistrationOptions, {}), ] +# Maps LSP method → handler function for dynamic registration. +_JDTLS_HANDLERS: dict[str, Any] = {} + +# Set after first successful registration to prevent FeatureAlreadyRegisteredError. +_jdtls_capabilities_registered = False + def _build_jdtls_registrations() -> list[lsp.Registration]: """Build LSP Registration objects for jdtls-dependent capabilities.""" return [ lsp.Registration( - id=f"jdtls-{suffix}", + id=f"{_JDTLS_REG_PREFIX}{suffix}", method=method, register_options=_converter.unstructure(opts_cls(document_selector=_JAVA_SELECTOR, **extra)), ) @@ -286,21 +294,25 @@ async def _register_jdtls_capabilities() -> None: We don't advertise these in the static InitializeResult because doing so would make the IDE defer hover/definition/etc to us even before jdtls is ready, which suppresses the IDE's built-in diagnostic tooltips. + + Idempotent: safe to call multiple times (e.g., proxy restart). """ - # Register the request handlers so pygls dispatches incoming requests to them. - server.feature(lsp.TEXT_DOCUMENT_COMPLETION)(_on_completion) - server.feature(lsp.TEXT_DOCUMENT_HOVER)(_on_hover) - server.feature(lsp.TEXT_DOCUMENT_DEFINITION)(_on_definition) - server.feature(lsp.TEXT_DOCUMENT_REFERENCES)(_on_references) - server.feature(lsp.TEXT_DOCUMENT_DOCUMENT_SYMBOL)(_on_document_symbol) - - # Tell the client we now support these capabilities. - registrations = _build_jdtls_registrations() + global _jdtls_capabilities_registered + if _jdtls_capabilities_registered: + return + try: + # Register handlers so pygls dispatches incoming requests to them. + for method, handler in _JDTLS_HANDLERS.items(): + server.feature(method)(handler) + + # Tell the client we now support these capabilities. + registrations = _build_jdtls_registrations() await server.client_register_capability_async(lsp.RegistrationParams(registrations=registrations)) + _jdtls_capabilities_registered = True logger.info("Dynamically registered jdtls capabilities (hover, definition, references, completion, symbol)") - except Exception as e: - logger.warning("Failed to dynamically register jdtls capabilities: %s", e) + except Exception: + logger.warning("Failed to dynamically register jdtls capabilities", exc_info=True) # --- Document sync (forward to jdtls + run custom analyzers) --- @@ -360,14 +372,12 @@ async def on_did_close(params: lsp.DidCloseTextDocumentParams) -> None: await server._proxy.send_notification("textDocument/didClose", _serialize_params(params)) -# --- Forwarded features (jdtls passthrough) --- - - # --- jdtls passthrough handlers (registered dynamically, NOT at module level) --- +# # These are NOT decorated with @server.feature because pygls auto-advertises -# capabilities for decorated handlers. We register them manually via -# server.lsp.fm.feature() inside _register_jdtls_capabilities() so they only -# activate after jdtls starts. +# capabilities for decorated handlers. Instead, they are collected in +# _JDTLS_HANDLERS and registered inside _register_jdtls_capabilities() so +# they only activate after jdtls starts. async def _on_completion(params: lsp.CompletionParams) -> lsp.CompletionList | None: @@ -437,6 +447,18 @@ async def _on_document_symbol(params: lsp.DocumentSymbolParams) -> list[lsp.Docu return None +# Populate handler map for dynamic registration. +_JDTLS_HANDLERS.update( + { + lsp.TEXT_DOCUMENT_COMPLETION: _on_completion, + lsp.TEXT_DOCUMENT_HOVER: _on_hover, + lsp.TEXT_DOCUMENT_DEFINITION: _on_definition, + lsp.TEXT_DOCUMENT_REFERENCES: _on_references, + lsp.TEXT_DOCUMENT_DOCUMENT_SYMBOL: _on_document_symbol, + } +) + + # --- Code actions (quick fixes) --- # Human-readable titles for code actions diff --git a/tests/test_server.py b/tests/test_server.py index a7547da..9d2f441 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -315,7 +315,8 @@ def test_init_capabilities_exclude_jdtls_features(self) -> None: assert caps.document_symbol_provider is None def test_build_jdtls_registrations(self) -> None: - from java_functional_lsp.server import _build_jdtls_registrations + """_build_jdtls_registrations returns one Registration per jdtls capability, each scoped to java files.""" + from java_functional_lsp.server import _JDTLS_REG_PREFIX, _build_jdtls_registrations regs = _build_jdtls_registrations() assert len(regs) == 5 @@ -325,27 +326,94 @@ def test_build_jdtls_registrations(self) -> None: assert lsp.TEXT_DOCUMENT_REFERENCES in methods assert lsp.TEXT_DOCUMENT_COMPLETION in methods assert lsp.TEXT_DOCUMENT_DOCUMENT_SYMBOL in methods - # All have java document selector + # All IDs are unique and use the shared prefix + ids = {r.id for r in regs} + assert len(ids) == 5 + assert all(rid.startswith(_JDTLS_REG_PREFIX) for rid in ids) + # All have java document selector with correct language for r in regs: assert r.register_options is not None - assert "documentSelector" in r.register_options + selectors = r.register_options["documentSelector"] + assert any(s.get("language") == "java" for s in selectors) + # Completion has triggerCharacters + comp = next(r for r in regs if r.method == lsp.TEXT_DOCUMENT_COMPLETION) + assert comp.register_options.get("triggerCharacters") == ["."] async def test_register_jdtls_capabilities_logs_on_failure(self, caplog: Any) -> None: """_register_jdtls_capabilities logs a warning when the client rejects.""" import logging - from unittest.mock import AsyncMock, patch + from unittest.mock import AsyncMock, MagicMock, patch - from java_functional_lsp.server import _register_jdtls_capabilities + import java_functional_lsp.server as srv_mod from java_functional_lsp.server import server as srv - mock = AsyncMock(side_effect=Exception("no")) - with ( - caplog.at_level(logging.WARNING, logger="java_functional_lsp.server"), - patch.object(srv, "client_register_capability_async", mock), - ): - await _register_jdtls_capabilities() + # Patch both server.feature (to avoid FeatureAlreadyRegisteredError on + # the shared singleton) and client_register_capability_async (to trigger error). + mock_reg = AsyncMock(side_effect=Exception("no")) + mock_feature = MagicMock(return_value=lambda fn: fn) + old_flag = srv_mod._jdtls_capabilities_registered + srv_mod._jdtls_capabilities_registered = False + try: + with ( + caplog.at_level(logging.WARNING, logger="java_functional_lsp.server"), + patch.object(srv, "client_register_capability_async", mock_reg), + patch.object(srv, "feature", mock_feature), + ): + await srv_mod._register_jdtls_capabilities() + finally: + srv_mod._jdtls_capabilities_registered = old_flag assert any("Failed to dynamically register" in r.getMessage() for r in caplog.records) + async def test_register_jdtls_capabilities_happy_path(self, caplog: Any) -> None: + """On success, handlers are registered and info log is emitted.""" + import logging + from unittest.mock import AsyncMock, MagicMock, patch + + import java_functional_lsp.server as srv_mod + from java_functional_lsp.server import server as srv + + mock_reg = AsyncMock(return_value=None) + registered_methods: list[str] = [] + mock_feature = MagicMock(side_effect=lambda m: registered_methods.append(m) or (lambda fn: fn)) + old_flag = srv_mod._jdtls_capabilities_registered + srv_mod._jdtls_capabilities_registered = False + try: + with ( + caplog.at_level(logging.INFO, logger="java_functional_lsp.server"), + patch.object(srv, "client_register_capability_async", mock_reg), + patch.object(srv, "feature", mock_feature), + ): + await srv_mod._register_jdtls_capabilities() + finally: + srv_mod._jdtls_capabilities_registered = old_flag + # Handlers were registered for all 5 methods + assert lsp.TEXT_DOCUMENT_HOVER in registered_methods + assert lsp.TEXT_DOCUMENT_COMPLETION in registered_methods + assert lsp.TEXT_DOCUMENT_DEFINITION in registered_methods + assert lsp.TEXT_DOCUMENT_REFERENCES in registered_methods + assert lsp.TEXT_DOCUMENT_DOCUMENT_SYMBOL in registered_methods + # client_register_capability_async was called + mock_reg.assert_called_once() + # Success log emitted + assert any("Dynamically registered" in r.getMessage() for r in caplog.records) + + async def test_register_jdtls_capabilities_idempotent(self) -> None: + """Second call is a no-op (idempotency guard).""" + from unittest.mock import AsyncMock, patch + + import java_functional_lsp.server as srv_mod + from java_functional_lsp.server import server as srv + + mock_reg = AsyncMock() + old_flag = srv_mod._jdtls_capabilities_registered + srv_mod._jdtls_capabilities_registered = True + try: + with patch.object(srv, "client_register_capability_async", mock_reg): + await srv_mod._register_jdtls_capabilities() + finally: + srv_mod._jdtls_capabilities_registered = old_flag + mock_reg.assert_not_called() + def test_serialize_params_camelcase(self) -> None: from java_functional_lsp.server import _serialize_params @@ -466,6 +534,8 @@ async def test_initialize_reports_capabilities(self, lsp_client: LanguageClient) assert caps.hover_provider is None assert caps.definition_provider is None assert caps.references_provider is None + assert caps.completion_provider is None + assert caps.document_symbol_provider is None async def test_null_return_diagnostic_published(self, lsp_client: LanguageClient) -> None: """didOpen a file with ``return null`` → server publishes null-return diagnostic.""" diff --git a/uv.lock b/uv.lock index 43ed4e3..3919fb8 100644 --- a/uv.lock +++ b/uv.lock @@ -184,7 +184,7 @@ wheels = [ [[package]] name = "java-functional-lsp" -version = "0.7.2" +version = "0.7.3" source = { editable = "." } dependencies = [ { name = "pygls" },