From 1643c14aaecb77a001a11a3199c8f2c5759019df Mon Sep 17 00:00:00 2001 From: Roman Nikulenkov Date: Mon, 5 Jan 2026 17:00:54 +0300 Subject: [PATCH] refactor(gopls): streamline gopls settings validation and JSON serialization test(go): add assertions for SolidLanguageServer cache states --- src/solidlsp/language_servers/gopls.py | 47 ++++++++++++-------------- test/solidlsp/go/test_go_basic.py | 28 ++++++++++----- 2 files changed, 41 insertions(+), 34 deletions(-) diff --git a/src/solidlsp/language_servers/gopls.py b/src/solidlsp/language_servers/gopls.py index aaa59793..419109ef 100644 --- a/src/solidlsp/language_servers/gopls.py +++ b/src/solidlsp/language_servers/gopls.py @@ -3,7 +3,7 @@ import os import pathlib import subprocess import threading -from typing import cast +from typing import Any, cast from overrides import override @@ -133,21 +133,12 @@ class Gopls(SolidLanguageServer): # (Access settings directly to avoid extra INFO logging from CustomLSSettings.get.) gopls_settings = self._custom_settings.settings.get("gopls_settings") if gopls_settings: - if not isinstance(gopls_settings, dict): - raise TypeError( - f"gopls_settings must be a dict, got {type(gopls_settings).__name__}. " - "Expected structure: {'buildFlags': ['-tags=foo'], 'env': {...}, ...}" - ) + gopls_settings = self._validate_gopls_settings_dict(gopls_settings) # Validate JSON-serializability early: initializationOptions is sent over JSON-RPC. import json - try: - json.dumps(gopls_settings, sort_keys=True, separators=(",", ":")) - except (TypeError, ValueError) as exc: - raise TypeError( - "gopls_settings must be JSON-serializable (json.dumps). Use JSON-compatible values (dict/list/str/int/float/bool/null) and prefer string keys." - ) from exc + self._canonical_json_or_raise(json, gopls_settings) # Log keys only (and at DEBUG) to avoid leaking sensitive values and to reduce startup noise. log.debug("Applying gopls settings via initializationOptions: keys=%s", list(gopls_settings.keys())) @@ -155,6 +146,23 @@ class Gopls(SolidLanguageServer): return cast(InitializeParams, initialize_params) + def _validate_gopls_settings_dict(self, gopls_settings: object) -> dict: + if not isinstance(gopls_settings, dict): + raise TypeError( + f"gopls_settings must be a dict, got {type(gopls_settings).__name__}. " + "Expected structure: {'buildFlags': ['-tags=foo'], 'env': {...}, ...}" + ) + + return gopls_settings + + def _canonical_json_or_raise(self, json_module: Any, data: object) -> str: + try: + return json_module.dumps(data, sort_keys=True, separators=(",", ":")) + except (TypeError, ValueError) as exc: + raise TypeError( + "gopls_settings must be JSON-serializable (json.dumps). Use JSON-compatible values (dict/list/str/int/float/bool/null) and prefer string keys." + ) from exc + # Environment variables that influence Go build context and affect cached symbols. _CACHE_CONTEXT_ENV_KEYS = ("GOFLAGS", "GOOS", "GOARCH", "CGO_ENABLED") @@ -174,14 +182,8 @@ class Gopls(SolidLanguageServer): if gopls_settings_raw is None: gopls_settings = None else: - if not isinstance(gopls_settings_raw, dict): - raise TypeError( - f"gopls_settings must be a dict, got {type(gopls_settings_raw).__name__}. " - "Expected structure: {'buildFlags': ['-tags=foo'], 'env': {...}, ...}" - ) - # Treat an explicitly empty dict the same as not providing settings at all. - gopls_settings = gopls_settings_raw or None + gopls_settings = self._validate_gopls_settings_dict(gopls_settings_raw) or None # Only include env vars that are set to a non-empty value. env_subset: dict[str, str] = {} @@ -198,12 +200,7 @@ class Gopls(SolidLanguageServer): if gopls_settings is not None: fingerprint_data["gopls_settings"] = gopls_settings - try: - canonical_json = json.dumps(fingerprint_data, sort_keys=True, separators=(",", ":")) - except (TypeError, ValueError) as exc: - raise TypeError( - "gopls_settings must be JSON-serializable (json.dumps). Use JSON-compatible values (dict/list/str/int/float/bool/null) and prefer string keys." - ) from exc + canonical_json = self._canonical_json_or_raise(json, fingerprint_data) return hashlib.sha256(canonical_json.encode("utf-8")).hexdigest()[:16] diff --git a/test/solidlsp/go/test_go_basic.py b/test/solidlsp/go/test_go_basic.py index f1255eba..cd7e8a86 100644 --- a/test/solidlsp/go/test_go_basic.py +++ b/test/solidlsp/go/test_go_basic.py @@ -113,6 +113,22 @@ class TestGoBuildTags: main_go = os.path.join("main.go") + def _assert_caches_loaded_and_clean(ls: SolidLanguageServer) -> None: + # White-box assertions: SolidLanguageServer currently has no public API to verify that + # caches were loaded from disk vs created lazily on first request. + assert ls._raw_document_symbols_cache, "Expected raw document-symbol cache to load from disk" + assert ls._document_symbols_cache, "Expected document-symbol cache to load from disk" + assert not ls._raw_document_symbols_cache_is_modified + assert not ls._document_symbols_cache_is_modified + + def _assert_caches_empty(ls: SolidLanguageServer) -> None: + assert ls._raw_document_symbols_cache == {} + assert ls._document_symbols_cache == {} + + def _assert_caches_modified(ls: SolidLanguageServer) -> None: + assert ls._raw_document_symbols_cache_is_modified + assert ls._document_symbols_cache_is_modified + # Run 1 (default context): populate caches and persist them to disk. with start_ls_context(Language.GO, repo_path=str(repo_path), solidlsp_dir=tmp_path) as ls_default: _ = ls_default.request_document_symbols(main_go) @@ -146,11 +162,7 @@ class TestGoBuildTags: with start_ls_context(Language.GO, repo_path=str(repo_path), solidlsp_dir=tmp_path) as ls_default_again: assert ls_default_again.cache_dir == cache_dir - assert ls_default_again._raw_document_symbols_cache, "Expected raw document-symbol cache to load from disk" - assert ls_default_again._document_symbols_cache, "Expected document-symbol cache to load from disk" - - assert not ls_default_again._raw_document_symbols_cache_is_modified - assert not ls_default_again._document_symbols_cache_is_modified + _assert_caches_loaded_and_clean(ls_default_again) _ = ls_default_again.request_document_symbols(main_go) @@ -174,11 +186,9 @@ class TestGoBuildTags: assert foo_doc_cache_version != default_doc_cache_version # Different build context => persisted caches must not be loaded. - assert ls_foo._raw_document_symbols_cache == {} - assert ls_foo._document_symbols_cache == {} + _assert_caches_empty(ls_foo) _ = ls_foo.request_document_symbols(main_go) # A cache miss should repopulate and mark caches modified. - assert ls_foo._raw_document_symbols_cache_is_modified - assert ls_foo._document_symbols_cache_is_modified + _assert_caches_modified(ls_foo)