mirror of
https://github.com/tiennm99/serena.git
synced 2026-10-13 04:15:11 +00:00
Solidity: await validation completion for diagnostics instead of a fixed wait
The Nomic solidity-language-server validates asynchronously: document
changes are debounced (~250ms), validation runs a forge/solc compile,
and completion is signalled with the custom notification
custom/validation-job-status -- emitted on every completion path
(success, validation failure, job errors; verified in the bundled 0.8.4
package, which publishes the document's diagnostics before the signal).
Serena ignored the signal ("Unhandled method
'custom/validation-job-status'") and waited a fixed 2.5s for a
non-empty publishDiagnostics, returning [] whenever validation finished
later. Warm environments fit the window; slow or cold ones (macOS and
Windows CI runners, which download solc 0.8.20 on every run -- the CI
cache only covers ~/.serena/language_servers/static, not
~/.local/share/svm) often do not. This is the root cause of the
empty-diagnostics failures of test_file_diagnostics[solidity]: xfailed
on macOS since its introduction (bd7e64e8) and failing on Windows on
Tests run #4236, job catch-all (windows-latest). Shrinking the wait
reproduces the exact failure signature (AssertionError: []) on Linux.
Changes:
- Handle custom/validation-job-status (threading.Event + last payload).
The payload carries no document URI, so the event is global to the
server instance.
- request_text_document_diagnostics keeps the 2.5s non-empty publish
wait as the fast path. When it misses, it awaits the completion
signal against a 60s deadline (aligned with the indexing wait) and
takes the generation-advancing publication for the request URI,
accepting an empty result as the validated answer; a completion that
brings no publication for the URI (e.g. one belonging to another
document) re-arms the wait until the deadline. If the signal never
arrives, a warning is logged and the previous empty-result behavior
applies, bounded by the deadline.
- Remove the darwin xfail from test_file_diagnostics; add regression
tests for the fallback delivery (fast path forced to miss), for the
never-signalled bound, and for re-arming on a spurious completion
belonging to another document.
- CHANGELOG entry under Language Servers.
Design notes:
- Mirrors #1639 (TypeScript/VTS: event-based indexing wait replacing a
fixed sleep).
- The simpler alternative -- accepting empty publications and raising
the fixed wait -- was rejected: without the completion signal, an
early clear-on-open publication would be taken as the answer, and
every clean file would stall for the full wait.
- Supersedes the quarantine approach of draft PR #1674 (broadening the
xfail to non-Linux); whichever lands second resolves the trivial
conflict on the xfail block.
This commit is contained in:
1 parent
e08e964d0c
commit
90334cc131
3 files changed
+164
-8
No files matched your search
@@ -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`.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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"]
|
||||
Reference in new issue
Block a user