diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ac7d1a1..1275013b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ Status of the `main` branch. Changes prior to the next official version change w - Make tool call errors surface explicitly as errors at the MCP protocol level * Language Servers: + - Solidity: fix diagnostics intermittently coming back empty on slow or cold environments (macOS/Windows CI). - C/C++ (clangd): improve support and documentation for Unreal Engine 5 projects. - HLSL (`shader-language-server`): pass `--locked` to `cargo install` when building from source on macOS (and in the manual-install instructions), honoring the crate's packaged `Cargo.lock`. diff --git a/src/solidlsp/language_servers/solidity_language_server.py b/src/solidlsp/language_servers/solidity_language_server.py index 515bd5ce..434c536a 100644 --- a/src/solidlsp/language_servers/solidity_language_server.py +++ b/src/solidlsp/language_servers/solidity_language_server.py @@ -10,7 +10,7 @@ import pathlib import platform import shutil import threading -from time import sleep +from time import monotonic, sleep from typing import Any from overrides import override @@ -40,6 +40,14 @@ class SolidityLanguageServer(SolidLanguageServer): Requires Node.js and npm to be installed. """ + _VALIDATION_COMPLETION_TIMEOUT = 60.0 + """upper bound for awaiting the validation-completion signal (``custom/validation-job-status``), + aligned with the indexing wait; on cold environments validation includes downloading the pinned + solc version""" + + _POST_VALIDATION_PUBLISH_WAIT = 2.5 + """grace period for the diagnostics publication that accompanies the completion signal""" + @staticmethod def _determine_log_level(line: str) -> int: """Suppress known non-critical stderr output from the Solidity language server.""" @@ -66,6 +74,11 @@ class SolidityLanguageServer(SolidLanguageServer): "solidity", solidlsp_settings, ) + self._validation_completed = threading.Event() + """set when the language server reports an asynchronous validation run as finished + (``custom/validation-job-status``); cleared at the start of each diagnostics request""" + self._last_validation_status: Any | None = None + """payload of the most recent ``custom/validation-job-status`` notification, kept for logging""" def _create_dependency_provider(self) -> LanguageServerDependencyProvider: return self.DependencyProvider(self._custom_settings, self._ls_resources_dir) @@ -242,11 +255,19 @@ class SolidityLanguageServer(SolidLanguageServer): if indexed_count[0] >= expected_count: all_indexed.set() + def on_validation_job_status(params: Any) -> None: + # signals completion of an asynchronous validation run (a forge/solc compile); + # the payload carries no document URI, so the event is global to the server instance + self._last_validation_status = params + log.debug(f"Solidity LSP: validation job finished: {params}") + self._validation_completed.set() + self.server.on_request("client/registerCapability", register_capability_handler) self.server.on_notification("window/logMessage", window_log_message) self.server.on_notification("$/progress", do_nothing) self.server.on_notification("textDocument/publishDiagnostics", do_nothing) self.server.on_notification("custom/file-indexed", on_file_indexed) + self.server.on_notification("custom/validation-job-status", on_validation_job_status) log.info("Starting Solidity language server process") self.server.start() @@ -300,6 +321,9 @@ class SolidityLanguageServer(SolidLanguageServer): "character": len(lines[-1]), } + # any validation that finishes after this point may satisfy the fallback wait below + self._validation_completed.clear() + self.server.notify.did_open_text_document( { # ty: ignore[invalid-argument-type] # dict built from LSPConstants keys; shape matches the TypedDict LSPConstants.TEXT_DOCUMENT: { @@ -328,12 +352,52 @@ class SolidityLanguageServer(SolidLanguageServer): ], } ) + # fast path: wait for a non-empty diagnostics publication (warm environments) diagnostics = self._wait_for_relevant_published_diagnostics( uri=uri, after_generation=diagnostics_before_request, timeout=self._get_published_diagnostics_wait_timeout(True), allow_cached=True, ) + + # fallback: validation (a forge/solc compile) runs asynchronously in the server and can + # exceed the initial wait on slow or cold environments (e.g. CI runners downloading the + # pinned solc); await its completion signal, then take the publication made for this + # request, accepting an empty result as the validated answer. The signal carries no + # document URI, so a completion belonging to another document can wake the wait early; + # re-arm and keep waiting until the deadline in that case. + if diagnostics is None: + # fast path missed; validation is still running -- await its completion signal so the + # wait below is visible in logs on slow/cold environments + log.debug( + f"Solidity diagnostics not yet published for {uri}; " + f"awaiting validation completion (up to {self._VALIDATION_COMPLETION_TIMEOUT:.0f}s)" + ) + deadline = monotonic() + self._VALIDATION_COMPLETION_TIMEOUT + while diagnostics is None: + remaining = deadline - monotonic() + if remaining <= 0 or not self._validation_completed.wait(timeout=remaining): + log.warning( + f"Solidity validation completion was not signalled within " + f"{self._VALIDATION_COMPLETION_TIMEOUT:.0f}s for {uri} " + f"(last status: {self._last_validation_status}); returning without diagnostics" + ) + break + diagnostics = self._wait_for_published_diagnostics( + uri=uri, + after_generation=diagnostics_before_request, + timeout=self._POST_VALIDATION_PUBLISH_WAIT, + ) + if diagnostics is None: + # a completion fired but published nothing for our URI (e.g. a run for another + # document); re-arm and keep waiting so the ongoing wait stays visible + log.debug(f"Solidity validation completed without diagnostics for {uri} yet; continuing to wait") + self._validation_completed.clear() + else: + log.debug( + f"Solidity diagnostics resolved via validation completion " + f"(status={self._last_validation_status}): {len(diagnostics)} diagnostic(s)" + ) finally: self.server.notify.did_close_text_document( { # ty: ignore[invalid-argument-type] # dict built from LSPConstants keys; shape matches the TypedDict diff --git a/test/solidlsp/solidity/test_solidity_diagnostics.py b/test/solidlsp/solidity/test_solidity_diagnostics.py index fd3cd6ba..21a11f3c 100644 --- a/test/solidlsp/solidity/test_solidity_diagnostics.py +++ b/test/solidlsp/solidity/test_solidity_diagnostics.py @@ -1,19 +1,42 @@ -import sys - import pytest from solidlsp import SolidLanguageServer +from solidlsp.language_servers.solidity_language_server import SolidityLanguageServer from solidlsp.ls_config import Language from test.solidlsp.util.diagnostics import assert_file_diagnostics +class _NeverSetEvent: + """Stand-in for :class:`threading.Event` whose wait never succeeds.""" + + def wait(self, timeout: float | None = None) -> bool: + return False + + def clear(self) -> None: + return None + + def set(self) -> None: + return None + + +class _AlwaysSignalledEvent: + """Stand-in for :class:`threading.Event` whose wait always reports a completion; counts clears.""" + + def __init__(self) -> None: + self.cleared = 0 + + def wait(self, timeout: float | None = None) -> bool: + return True + + def clear(self) -> None: + self.cleared += 1 + + def set(self) -> None: + return None + + @pytest.mark.solidity class TestSolidityDiagnostics: - @pytest.mark.xfail( - sys.platform == "darwin", - reason="Unknown — passes on Ubuntu but consistently fails on macOS CI; root cause not yet identified.", - strict=False, - ) @pytest.mark.parametrize("language_server", [Language.SOLIDITY], indirect=True) def test_file_diagnostics(self, language_server: SolidLanguageServer) -> None: assert_file_diagnostics( @@ -22,3 +45,71 @@ class TestSolidityDiagnostics: (), min_count=1, ) + + @pytest.mark.parametrize("language_server", [Language.SOLIDITY], indirect=True) + def test_file_diagnostics_via_validation_completion( + self, language_server: SolidLanguageServer, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Diagnostics must still arrive via the validation-completion fallback when the initial + publish wait does not observe them (the failure mode of cold/slow CI runners). + """ + assert isinstance(language_server, SolidityLanguageServer) + + # force the fast path to miss so that only the validation-completion fallback can deliver + monkeypatch.setattr(language_server, "_wait_for_relevant_published_diagnostics", lambda **kwargs: None) + + assert_file_diagnostics( + language_server, + "contracts/DiagnosticsSample.sol", + (), + min_count=1, + ) + + @pytest.mark.parametrize("language_server", [Language.SOLIDITY], indirect=True) + def test_file_diagnostics_without_validation_signal( + self, language_server: SolidLanguageServer, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A server that never signals validation completion must yield an empty result within the + bound instead of hanging or raising. + """ + assert isinstance(language_server, SolidityLanguageServer) + + # force the fast path to miss and make the completion signal unobtainable + monkeypatch.setattr(language_server, "_wait_for_relevant_published_diagnostics", lambda **kwargs: None) + monkeypatch.setattr(language_server, "_VALIDATION_COMPLETION_TIMEOUT", 0.2) + monkeypatch.setattr(language_server, "_validation_completed", _NeverSetEvent()) + + diagnostics = language_server.request_text_document_diagnostics("contracts/DiagnosticsSample.sol", min_severity=1) + assert diagnostics == [] + + @pytest.mark.parametrize("language_server", [Language.SOLIDITY], indirect=True) + def test_file_diagnostics_rearms_after_spurious_completion( + self, language_server: SolidLanguageServer, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A completion signal belonging to another document (the payload carries no URI) must + re-arm the fallback wait instead of ending the request without diagnostics. + """ + assert isinstance(language_server, SolidityLanguageServer) + + expected_diagnostic = { + "uri": "file:///spurious-completion-test", + "severity": 1, + "message": "delivered after re-arm", + "range": {"start": {"line": 0, "character": 0}, "end": {"line": 0, "character": 1}}, + "code": None, + } + # first wake is spurious (no publication for the request URI), second delivers + grace_reads: list[list | None] = [None, [expected_diagnostic]] + event = _AlwaysSignalledEvent() + + # force the fast path to miss and script the fallback's wake/read sequence + monkeypatch.setattr(language_server, "_wait_for_relevant_published_diagnostics", lambda **kwargs: None) + monkeypatch.setattr(language_server, "_validation_completed", event) + monkeypatch.setattr(language_server, "_wait_for_published_diagnostics", lambda **kwargs: grace_reads.pop(0)) + + diagnostics = language_server.request_text_document_diagnostics("contracts/DiagnosticsSample.sol", min_severity=1) + + # one clear at request start plus exactly one re-arm after the spurious wake + assert event.cleared == 2 + assert not grace_reads # both scripted reads consumed + assert [diagnostic["message"] for diagnostic in diagnostics] == ["delivered after re-arm"]