Merge pull request #1675 from xormania/fix/solidity-diagnostics-validation-wait

Solidity: await validation completion for diagnostics instead of a fixed wait

CHANGELOG.md

@@ -51,6 +51,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`.

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

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"]