refactor(gopls): streamline gopls settings validation and JSON serialization

test(go): add assertions for SolidLanguageServer cache states

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]
 

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)